* [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers @ 2015-08-14 1:30 Simon Horman 2015-08-17 18:33 ` Pravin Shelar 0 siblings, 1 reply; 5+ messages in thread From: Simon Horman @ 2015-08-14 1:30 UTC (permalink / raw) To: Pravin Shelar Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA, Simon Horman When an error occurs skipping IPv6 extension headers retain the already parsed IP protocol and IPv6 addresses in the flow. Also assume that the packet is not a fragment in the absence of information to the contrary; that is always use the frag_off value set by ipv6_skip_exthdr(). This allows matching on the IP protocol and IPv6 addresses of packets with malformed extension headers. Signed-off-by: Simon Horman <simon.horman@netronome.com> --- * Some consideration should be given to unwanted side effects of this patch as it affects the handling of malformed packets. Signed-off-by: Simon Horman <simon.horman@netronome.com> --- net/openvswitch/flow.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index 8db22ef73626..3c5336c5d4ea 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c @@ -271,8 +271,6 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) key->ipv6.addr.dst = nh->daddr; payload_ofs = ipv6_skip_exthdr(skb, payload_ofs, &nexthdr, &frag_off); - if (unlikely(payload_ofs < 0)) - return -EINVAL; if (frag_off) { if (frag_off & htons(~0x7)) @@ -283,6 +281,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) key->ip.frag = OVS_FRAG_TYPE_NONE; } + /* Delayed handling of error in ipv6_skip_exthdr() as it + * always sets frag_off to a valid value which may be + * used to set key->ip.frag above. + */ + if (unlikely(payload_ofs < 0)) + return -EINVAL; + nh_len = payload_ofs - nh_ofs; skb_set_transport_header(skb, nh_ofs + nh_len); key->ip.proto = nexthdr; @@ -622,8 +627,6 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key) nh_len = parse_ipv6hdr(skb, key); if (unlikely(nh_len < 0)) { - memset(&key->ip, 0, sizeof(key->ip)); - memset(&key->ipv6.addr, 0, sizeof(key->ipv6.addr)); if (nh_len == -EINVAL) { skb->transport_header = skb->network_header; error = 0; -- 2.1.4 _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers 2015-08-14 1:30 [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers Simon Horman @ 2015-08-17 18:33 ` Pravin Shelar 2015-08-27 7:10 ` Simon Horman 0 siblings, 1 reply; 5+ messages in thread From: Pravin Shelar @ 2015-08-17 18:33 UTC (permalink / raw) To: Simon Horman; +Cc: Jesse Gross, netdev, dev@openvswitch.org On Thu, Aug 13, 2015 at 6:30 PM, Simon Horman <simon.horman@netronome.com> wrote: > When an error occurs skipping IPv6 extension headers retain the already > parsed IP protocol and IPv6 addresses in the flow. Also assume that the > packet is not a fragment in the absence of information to the contrary; > that is always use the frag_off value set by ipv6_skip_exthdr(). > > This allows matching on the IP protocol and IPv6 addresses of packets > with malformed extension headers. > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > --- > > * Some consideration should be given to unwanted side effects of this patch > as it affects the handling of malformed packets. > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > --- > net/openvswitch/flow.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c > index 8db22ef73626..3c5336c5d4ea 100644 > --- a/net/openvswitch/flow.c > +++ b/net/openvswitch/flow.c > @@ -271,8 +271,6 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) > key->ipv6.addr.dst = nh->daddr; > > payload_ofs = ipv6_skip_exthdr(skb, payload_ofs, &nexthdr, &frag_off); > - if (unlikely(payload_ofs < 0)) > - return -EINVAL; > > if (frag_off) { > if (frag_off & htons(~0x7)) > @@ -283,6 +281,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) > key->ip.frag = OVS_FRAG_TYPE_NONE; > } > > + /* Delayed handling of error in ipv6_skip_exthdr() as it > + * always sets frag_off to a valid value which may be > + * used to set key->ip.frag above. > + */ > + if (unlikely(payload_ofs < 0)) > + return -EINVAL; > + check_header() function called above also return -EINVAL, That could confuse caller checking for error code. So we need different error code here. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers 2015-08-17 18:33 ` Pravin Shelar @ 2015-08-27 7:10 ` Simon Horman 2015-08-28 18:11 ` Pravin Shelar 0 siblings, 1 reply; 5+ messages in thread From: Simon Horman @ 2015-08-27 7:10 UTC (permalink / raw) To: Pravin Shelar; +Cc: Jesse Gross, netdev, dev@openvswitch.org Hi Pravin, On Mon, Aug 17, 2015 at 11:33:59AM -0700, Pravin Shelar wrote: > On Thu, Aug 13, 2015 at 6:30 PM, Simon Horman > <simon.horman@netronome.com> wrote: > > When an error occurs skipping IPv6 extension headers retain the already > > parsed IP protocol and IPv6 addresses in the flow. Also assume that the > > packet is not a fragment in the absence of information to the contrary; > > that is always use the frag_off value set by ipv6_skip_exthdr(). > > > > This allows matching on the IP protocol and IPv6 addresses of packets > > with malformed extension headers. > > > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > > > --- > > > > * Some consideration should be given to unwanted side effects of this patch > > as it affects the handling of malformed packets. > > > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > --- > > net/openvswitch/flow.c | 11 +++++++---- > > 1 file changed, 7 insertions(+), 4 deletions(-) > > > > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c > > index 8db22ef73626..3c5336c5d4ea 100644 > > --- a/net/openvswitch/flow.c > > +++ b/net/openvswitch/flow.c > > @@ -271,8 +271,6 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) > > key->ipv6.addr.dst = nh->daddr; > > > > payload_ofs = ipv6_skip_exthdr(skb, payload_ofs, &nexthdr, &frag_off); > > - if (unlikely(payload_ofs < 0)) > > - return -EINVAL; > > > > if (frag_off) { > > if (frag_off & htons(~0x7)) > > @@ -283,6 +281,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) > > key->ip.frag = OVS_FRAG_TYPE_NONE; > > } > > > > + /* Delayed handling of error in ipv6_skip_exthdr() as it > > + * always sets frag_off to a valid value which may be > > + * used to set key->ip.frag above. > > + */ > > + if (unlikely(payload_ofs < 0)) > > + return -EINVAL; > > + > check_header() function called above also return -EINVAL, That could > confuse caller checking for error code. So we need different error > code here. Thanks for pointing that out and sorry for taking so long to get back to you. How about something like this? -- >8 -- Subject: [PATCH v1.1] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers When an error occurs skipping IPv6 extension headers retain the already parsed IP protocol and IPv6 addresses in the flow. Also assume that the packet is not a fragment in the absence of information to the contrary; that is always use the frag_off value set by ipv6_skip_exthdr(). This allows matching on the IP protocol and IPv6 addresses of packets with malformed extension headers. Signed-off-by: Simon Horman <simon.horman@netronome.com> --- * Some consideration should be given to unwanted side effects of this patch as it affects the handling of malformed packets. Signed-off-by: Simon Horman <simon.horman@netronome.com> --- net/openvswitch/flow.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index 8db22ef73626..de4366f81b11 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c @@ -271,8 +271,6 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) key->ipv6.addr.dst = nh->daddr; payload_ofs = ipv6_skip_exthdr(skb, payload_ofs, &nexthdr, &frag_off); - if (unlikely(payload_ofs < 0)) - return -EINVAL; if (frag_off) { if (frag_off & htons(~0x7)) @@ -283,6 +281,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) key->ip.frag = OVS_FRAG_TYPE_NONE; } + /* Delayed handling of error in ipv6_skip_exthdr() as it + * always sets frag_off to a valid value which may be + * used to set key->ip.frag above. + */ + if (unlikely(payload_ofs < 0)) + return -EPROTO; + nh_len = payload_ofs - nh_ofs; skb_set_transport_header(skb, nh_ofs + nh_len); key->ip.proto = nexthdr; @@ -622,12 +627,16 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key) nh_len = parse_ipv6hdr(skb, key); if (unlikely(nh_len < 0)) { - memset(&key->ip, 0, sizeof(key->ip)); - memset(&key->ipv6.addr, 0, sizeof(key->ipv6.addr)); - if (nh_len == -EINVAL) { + switch (nh_len) { + case -EINVAL: + memset(&key->ip, 0, sizeof(key->ip)); + memset(&key->ipv6.addr, 0, sizeof(key->ipv6.addr)); + /* fall-through */ + case -EPROTO: skb->transport_header = skb->network_header; error = 0; - } else { + break; + default: error = nh_len; } return error; -- 2.1.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers 2015-08-27 7:10 ` Simon Horman @ 2015-08-28 18:11 ` Pravin Shelar 2015-08-28 23:31 ` Simon Horman 0 siblings, 1 reply; 5+ messages in thread From: Pravin Shelar @ 2015-08-28 18:11 UTC (permalink / raw) To: Simon Horman; +Cc: Jesse Gross, netdev, dev@openvswitch.org On Thu, Aug 27, 2015 at 12:10 AM, Simon Horman <simon.horman@netronome.com> wrote: > Hi Pravin, > > On Mon, Aug 17, 2015 at 11:33:59AM -0700, Pravin Shelar wrote: >> On Thu, Aug 13, 2015 at 6:30 PM, Simon Horman >> <simon.horman@netronome.com> wrote: >> > When an error occurs skipping IPv6 extension headers retain the already >> > parsed IP protocol and IPv6 addresses in the flow. Also assume that the >> > packet is not a fragment in the absence of information to the contrary; ... > -- >8 -- > Subject: [PATCH v1.1] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers > > When an error occurs skipping IPv6 extension headers retain the already > parsed IP protocol and IPv6 addresses in the flow. Also assume that the > packet is not a fragment in the absence of information to the contrary; > that is always use the frag_off value set by ipv6_skip_exthdr(). > > This allows matching on the IP protocol and IPv6 addresses of packets > with malformed extension headers. > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > > --- > > * Some consideration should be given to unwanted side effects of this patch > as it affects the handling of malformed packets. > > Signed-off-by: Simon Horman <simon.horman@netronome.com> > --- > net/openvswitch/flow.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c > index 8db22ef73626..de4366f81b11 100644 > --- a/net/openvswitch/flow.c > +++ b/net/openvswitch/flow.c > @@ -271,8 +271,6 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) > key->ipv6.addr.dst = nh->daddr; > > payload_ofs = ipv6_skip_exthdr(skb, payload_ofs, &nexthdr, &frag_off); > - if (unlikely(payload_ofs < 0)) > - return -EINVAL; > > if (frag_off) { > if (frag_off & htons(~0x7)) > @@ -283,6 +281,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key) > key->ip.frag = OVS_FRAG_TYPE_NONE; > } > > + /* Delayed handling of error in ipv6_skip_exthdr() as it > + * always sets frag_off to a valid value which may be > + * used to set key->ip.frag above. > + */ > + if (unlikely(payload_ofs < 0)) > + return -EPROTO; > + > nh_len = payload_ofs - nh_ofs; > skb_set_transport_header(skb, nh_ofs + nh_len); > key->ip.proto = nexthdr; > @@ -622,12 +627,16 @@ static int key_extract(struct sk_buff *skb, struct sw_flow_key *key) > > nh_len = parse_ipv6hdr(skb, key); > if (unlikely(nh_len < 0)) { > - memset(&key->ip, 0, sizeof(key->ip)); > - memset(&key->ipv6.addr, 0, sizeof(key->ipv6.addr)); > - if (nh_len == -EINVAL) { > + switch (nh_len) { > + case -EINVAL: > + memset(&key->ip, 0, sizeof(key->ip)); > + memset(&key->ipv6.addr, 0, sizeof(key->ipv6.addr)); > + /* fall-through */ > + case -EPROTO: > skb->transport_header = skb->network_header; > error = 0; > - } else { > + break; > + default: > error = nh_len; > } > return error; Looks good to me. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers 2015-08-28 18:11 ` Pravin Shelar @ 2015-08-28 23:31 ` Simon Horman 0 siblings, 0 replies; 5+ messages in thread From: Simon Horman @ 2015-08-28 23:31 UTC (permalink / raw) To: Pravin Shelar; +Cc: Jesse Gross, netdev, dev@openvswitch.org On Fri, Aug 28, 2015 at 11:11:08AM -0700, Pravin Shelar wrote: [snip] > Looks good to me. Thanks, I will make a formal submission of it as v2. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-08-28 23:31 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-14 1:30 [PATCH/RFC] openvswitch: Retain parsed IPv6 header fields in flow on error skipping extension headers Simon Horman 2015-08-17 18:33 ` Pravin Shelar 2015-08-27 7:10 ` Simon Horman 2015-08-28 18:11 ` Pravin Shelar 2015-08-28 23:31 ` Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).