[net-next] openvswitch: Derive IP protocol number for IPv6 later frags

Currently, OVS only parses the IP protocol number for the first
IPv6 fragment, but sets the IP protocol number for the later fragments
to be NEXTHDF_FRAGMENT.  This patch tries to derive the IP protocol
number for the IPV6 later frags so that we can match that.

Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
---
 net/openvswitch/flow.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

On Fri, Aug 10, 2018 at 10:19 AM, Yi-Hung Wei <yihung.wei@gmail.com> wrote:
> Currently, OVS only parses the IP protocol number for the first
> IPv6 fragment, but sets the IP protocol number for the later fragments
> to be NEXTHDF_FRAGMENT.  This patch tries to derive the IP protocol
> number for the IPV6 later frags so that we can match that.
>
> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> ---
>  net/openvswitch/flow.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> index 56b8e7167790..3d654c4f71be 100644
> --- a/net/openvswitch/flow.c
> +++ b/net/openvswitch/flow.c
> @@ -297,7 +297,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key)
>
>         nh_len = payload_ofs - nh_ofs;
>         skb_set_transport_header(skb, nh_ofs + nh_len);
> -       key->ip.proto = nexthdr;
> +       if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
> +               unsigned int offset = 0;
> +
> +               key->ip.proto = ipv6_find_hdr(skb, &offset, -1, NULL, NULL);
> +       } else {
> +               key->ip.proto = nexthdr;
> +       }
parsing ipv6 ipv6_skip_exthdr() is called to find fragment hdr and
then this patch calls ipv6_find_hdr() to find next protocol. I think
we could call ipv6_find_hdr() to get fragment type and next hdr, that
would save parsing same packet twice in some cases.

Other option would be calling ipv6_find_hdr() after setting OVS_FRAG_TYPE_LATER.

On Sun, Aug 12, 2018 at 6:09 PM Pravin Shelar <pshelar@ovn.org> wrote:
>
> On Fri, Aug 10, 2018 at 10:19 AM, Yi-Hung Wei <yihung.wei@gmail.com> wrote:
> > Currently, OVS only parses the IP protocol number for the first
> > IPv6 fragment, but sets the IP protocol number for the later fragments
> > to be NEXTHDF_FRAGMENT.  This patch tries to derive the IP protocol
> > number for the IPV6 later frags so that we can match that.
> >
> > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com>
> > ---
> >  net/openvswitch/flow.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
> > index 56b8e7167790..3d654c4f71be 100644
> > --- a/net/openvswitch/flow.c
> > +++ b/net/openvswitch/flow.c
> > @@ -297,7 +297,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key)
> >
> >         nh_len = payload_ofs - nh_ofs;
> >         skb_set_transport_header(skb, nh_ofs + nh_len);
> > -       key->ip.proto = nexthdr;
> > +       if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
> > +               unsigned int offset = 0;

How about we start the 2nd time parsing from
unsigned int offset = payload_ofs;

> > +
> > +               key->ip.proto = ipv6_find_hdr(skb, &offset, -1, NULL, NULL);

Then we only find the last header from previous parsed offset.

William

> > +       } else {
> > +               key->ip.proto = nexthdr;
> > +       }
> parsing ipv6 ipv6_skip_exthdr() is called to find fragment hdr and
> then this patch calls ipv6_find_hdr() to find next protocol. I think
> we could call ipv6_find_hdr() to get fragment type and next hdr, that
> would save parsing same packet twice in some cases.
>
> Other option would be calling ipv6_find_hdr() after setting OVS_FRAG_TYPE_LATER.

On Mon, Aug 13, 2018 at 10:48 AM William Tu <u9012063@gmail.com> wrote:
> > > --- a/net/openvswitch/flow.c
> > > +++ b/net/openvswitch/flow.c
> > > @@ -297,7 +297,13 @@ static int parse_ipv6hdr(struct sk_buff *skb, struct sw_flow_key *key)
> > >
> > >         nh_len = payload_ofs - nh_ofs;
> > >         skb_set_transport_header(skb, nh_ofs + nh_len);
> > > -       key->ip.proto = nexthdr;
> > > +       if (key->ip.frag == OVS_FRAG_TYPE_LATER) {
> > > +               unsigned int offset = 0;
>
> How about we start the 2nd time parsing from
> unsigned int offset = payload_ofs;
>
> > > +
> > > +               key->ip.proto = ipv6_find_hdr(skb, &offset, -1, NULL, NULL);
>
> Then we only find the last header from previous parsed offset.
>
> William
>
> > > +       } else {
> > > +               key->ip.proto = nexthdr;
> > > +       }
> > parsing ipv6 ipv6_skip_exthdr() is called to find fragment hdr and
> > then this patch calls ipv6_find_hdr() to find next protocol. I think
> > we could call ipv6_find_hdr() to get fragment type and next hdr, that
> > would save parsing same packet twice in some cases.
> >
> > Other option would be calling ipv6_find_hdr() after setting OVS_FRAG_TYPE_LATER.

Thanks Pravin and William's feedback.

After looking into ipv6_find_hdr() more closely, I think we can just
call ipv6_find_hdr() once and derive everything we need.

I will submit the new patch once net-next is open.

Thanks,

-Yi-Hung