netdev - Re: [PATCH net-next] openvswitch: correctly fragment packet with mpls headers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAOrHB_Dh=OPUTbbN7QYo9Nz4cV99-k_fT4z7jSOEcnFEDKXQEg@mail.gmail.com>
Date:   Mon, 3 Oct 2016 11:04:46 -0700
From:   Pravin Shelar <pshelar@....org>
To:     Jiri Benc <jbenc@...hat.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        David Ahern <dsa@...ulusnetworks.com>
Subject: Re: [PATCH net-next] openvswitch: correctly fragment packet with mpls headers

On Mon, Oct 3, 2016 at 9:33 AM, Jiri Benc <jbenc@...hat.com> wrote:
> If mpls headers were pushed to a defragmented packet, the refragmentation no
> longer works correctly after 48d2ab609b6b ("net: mpls: Fixups for GSO"). The
> network header has to be shifted after the mpls headers for the
> fragmentation and restored afterwards.
>
> Fixes: 48d2ab609b6b ("net: mpls: Fixups for GSO")
> Signed-off-by: Jiri Benc <jbenc@...hat.com>
> ---
>  net/openvswitch/actions.c | 26 +++++++++++++++++++++-----
>  1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index 4e03f64709bc..370b2ba3df4c 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -62,7 +62,8 @@ struct ovs_frag_data {
>         struct vport *vport;
>         struct ovs_skb_cb cb;
>         __be16 inner_protocol;
> -       __u16 vlan_tci;
> +       u16 network_offset;     /* valid only if inner_protocol is set */
> +       u16 vlan_tci;
>         __be16 vlan_proto;
>         unsigned int l2_len;
>         u8 l2_data[MAX_L2_LEN];
> @@ -656,7 +657,6 @@ static int ovs_vport_output(struct net *net, struct sock *sk, struct sk_buff *sk
>
>         __skb_dst_copy(skb, data->dst);
>         *OVS_CB(skb) = data->cb;
> -       skb->inner_protocol = data->inner_protocol;
>         skb->vlan_tci = data->vlan_tci;
>         skb->vlan_proto = data->vlan_proto;
>
> @@ -666,6 +666,13 @@ static int ovs_vport_output(struct net *net, struct sock *sk, struct sk_buff *sk
>         skb_postpush_rcsum(skb, skb->data, data->l2_len);
>         skb_reset_mac_header(skb);
>
> +       if (data->inner_protocol) {
> +               skb->inner_protocol = data->inner_protocol;
> +               skb->inner_network_header = skb->network_header;
> +               skb_set_network_header(skb, data->network_offset);
> +       }
> +       skb_reset_mac_len(skb);
> +
>         ovs_vport_send(vport, skb);
>         return 0;
>  }
> @@ -684,7 +691,8 @@ static struct dst_ops ovs_dst_ops = {
>  /* prepare_frag() is called once per (larger-than-MTU) frame; its inverse is
>   * ovs_vport_output(), which is called once per fragmented packet.
>   */
> -static void prepare_frag(struct vport *vport, struct sk_buff *skb)
> +static void prepare_frag(struct vport *vport, struct sk_buff *skb,
> +                        u16 orig_network_offset)
>  {
>         unsigned int hlen = skb_network_offset(skb);
>         struct ovs_frag_data *data;
> @@ -694,6 +702,7 @@ static void prepare_frag(struct vport *vport, struct sk_buff *skb)
>         data->vport = vport;
>         data->cb = *OVS_CB(skb);
>         data->inner_protocol = skb->inner_protocol;
> +       data->network_offset = orig_network_offset;
>         data->vlan_tci = skb->vlan_tci;
>         data->vlan_proto = skb->vlan_proto;
>         data->l2_len = hlen;
> @@ -706,6 +715,13 @@ static void prepare_frag(struct vport *vport, struct sk_buff *skb)
>  static void ovs_fragment(struct net *net, struct vport *vport,
>                          struct sk_buff *skb, u16 mru, __be16 ethertype)
>  {
> +       u16 orig_network_offset = 0;
> +
> +       if (skb->inner_protocol) {
> +               orig_network_offset = skb_network_offset(skb);
> +               skb->network_header = skb->inner_network_header;
> +       }
> +
This is not correct way to detect MPLS packet. inner_protocol can be
set by any tunnel device for using tunnel offloads. So this would
break the fragmentation for encapsulated packets. How about using
eth_p_mpls() as done in do-output()?