| Message ID | 1549326187-82147-2-git-send-email-dlu998@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [ovs-dev,v4,1/2] dp-packet: Add 'dp_packet_l3_size()'. | expand |
On Mon, Feb 04, 2019 at 04:23:07PM -0800, Darrell Ball wrote: > 'conn_key_extract()' in userspace conntrack is including L2 > (Ethernet) pad bytes for both L3 and L4 sizes. One problem is > any packet with non-zero L2 padding can incorrectly fail L4 > checksum validation. > > This patch fixes conn_key_extract() by ignoring L2 pad bytes. > > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") > CC: Daniele Di Proietto <diproiettod@ovn.org> > Co-authored-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> > Co-authored-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com> > Co-authored-by: Nitin Katiyar <nitin.katiyar@ericsson.com> > Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> > Signed-off-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com> > Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com> > Signed-off-by: Darrell Ball <dlu998@gmail.com> Thanks. I applied this series to master. Please let me know what branches I should backport it to.
Hi, We would like to get it backported till OVS 2.6 at least. Regards, Nitin > -----Original Message----- > From: Ben Pfaff [mailto:blp@ovn.org] > Sent: Tuesday, February 12, 2019 9:13 AM > To: Darrell Ball <dlu998@gmail.com> > Cc: dev@openvswitch.org; Nitin Katiyar <nitin.katiyar@ericsson.com> > Subject: Re: [ovs-dev] [patch v4 2/2] conntrack: Exclude l2 padding in > 'conn_key_extract()'. > > On Mon, Feb 04, 2019 at 04:23:07PM -0800, Darrell Ball wrote: > > 'conn_key_extract()' in userspace conntrack is including L2 > > (Ethernet) pad bytes for both L3 and L4 sizes. One problem is any > > packet with non-zero L2 padding can incorrectly fail L4 checksum > > validation. > > > > This patch fixes conn_key_extract() by ignoring L2 pad bytes. > > > > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") > > CC: Daniele Di Proietto <diproiettod@ovn.org> > > Co-authored-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> > > Co-authored-by: Venkatesan Pradeep > <venkatesan.pradeep@ericsson.com> > > Co-authored-by: Nitin Katiyar <nitin.katiyar@ericsson.com> > > Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> > > Signed-off-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com> > > Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com> > > Signed-off-by: Darrell Ball <dlu998@gmail.com> > > Thanks. > > I applied this series to master. Please let me know what branches I should > backport it to.
I backported as far as 2.9. For 2.8, it didn't apply cleanly--please post backports. On Wed, Feb 13, 2019 at 06:09:03AM +0000, Nitin Katiyar wrote: > Hi, > We would like to get it backported till OVS 2.6 at least. > > Regards, > Nitin > > > -----Original Message----- > > From: Ben Pfaff [mailto:blp@ovn.org] > > Sent: Tuesday, February 12, 2019 9:13 AM > > To: Darrell Ball <dlu998@gmail.com> > > Cc: dev@openvswitch.org; Nitin Katiyar <nitin.katiyar@ericsson.com> > > Subject: Re: [ovs-dev] [patch v4 2/2] conntrack: Exclude l2 padding in > > 'conn_key_extract()'. > > > > On Mon, Feb 04, 2019 at 04:23:07PM -0800, Darrell Ball wrote: > > > 'conn_key_extract()' in userspace conntrack is including L2 > > > (Ethernet) pad bytes for both L3 and L4 sizes. One problem is any > > > packet with non-zero L2 padding can incorrectly fail L4 checksum > > > validation. > > > > > > This patch fixes conn_key_extract() by ignoring L2 pad bytes. > > > > > > Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") > > > CC: Daniele Di Proietto <diproiettod@ovn.org> > > > Co-authored-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> > > > Co-authored-by: Venkatesan Pradeep > > <venkatesan.pradeep@ericsson.com> > > > Co-authored-by: Nitin Katiyar <nitin.katiyar@ericsson.com> > > > Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> > > > Signed-off-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com> > > > Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com> > > > Signed-off-by: Darrell Ball <dlu998@gmail.com> > > > > Thanks. > > > > I applied this series to master. Please let me know what branches I should > > backport it to.
diff --git a/lib/conntrack.c b/lib/conntrack.c index 0fedddd..e1f4041 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1875,6 +1875,9 @@ extract_l4_icmp6(struct conn_key *key, const void *data, size_t size, * processed, the function will extract the key from the packet nested * in the ICMP payload and set '*related' to true. * + * 'size' here is the layer 4 size, which can be a nested size if parsing + * an ICMP or ICMP6 header. + * * If 'related' is NULL, it means that we're already parsing a header nested * in an ICMP error. In this case, we skip checksum and length validation. */ static inline bool @@ -1949,7 +1952,6 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, * we use a sparse representation (miniflow). * */ - const char *tail = dp_packet_tail(pkt); bool ok; ctx->key.dl_type = dl_type; @@ -1960,11 +1962,11 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, } else { bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt); /* Validate the checksum only when hwol is not supported. */ - ok = extract_l3_ipv4(&ctx->key, l3, tail - (char *) l3, NULL, + ok = extract_l3_ipv4(&ctx->key, l3, dp_packet_l3_size(pkt), NULL, !hwol_good_l3_csum); } } else if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { - ok = extract_l3_ipv6(&ctx->key, l3, tail - (char *) l3, NULL); + ok = extract_l3_ipv6(&ctx->key, l3, dp_packet_l3_size(pkt), NULL); } else { ok = false; } @@ -1974,8 +1976,8 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, if (!hwol_bad_l4_csum) { bool hwol_good_l4_csum = dp_packet_l4_checksum_valid(pkt); /* Validate the checksum only when hwol is not supported. */ - if (extract_l4(&ctx->key, l4, tail - l4, &ctx->icmp_related, l3, - !hwol_good_l4_csum)) { + if (extract_l4(&ctx->key, l4, dp_packet_l4_size(pkt), + &ctx->icmp_related, l3, !hwol_good_l4_csum)) { ctx->hash = conn_key_hash(&ctx->key, ct->hash_basis); return true; }
'conn_key_extract()' in userspace conntrack is including L2 (Ethernet) pad bytes for both L3 and L4 sizes. One problem is any packet with non-zero L2 padding can incorrectly fail L4 checksum validation. This patch fixes conn_key_extract() by ignoring L2 pad bytes. Fixes: a489b16854b5 ("conntrack: New userspace connection tracker.") CC: Daniele Di Proietto <diproiettod@ovn.org> Co-authored-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Co-authored-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com> Co-authored-by: Nitin Katiyar <nitin.katiyar@ericsson.com> Signed-off-by: Vishal Deep Ajmera <vishal.deep.ajmera@ericsson.com> Signed-off-by: Venkatesan Pradeep <venkatesan.pradeep@ericsson.com> Signed-off-by: Nitin Katiyar <nitin.katiyar@ericsson.com> Signed-off-by: Darrell Ball <dlu998@gmail.com> --- v3: Removed line inadvertently merged from another patch. lib/conntrack.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)