Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <0300acca47b10384e6181516f32caddda043f3e4.camel@redhat.com>
Subject: Re: [PATCH] net: fix wrong network header length
From:   Paolo Abeni <pabeni@redhat.com>
To:     Lina Wang <lina.wang@mediatek.com>,
        "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        Matthias Brugger <matthias.bgg@gmail.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        bpf@vger.kernel.org, Martin KaFai Lau <kafai@fb.com>,
        Song Liu <songliubraving@fb.com>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, maze@google.com,
        willemb@google.com, edumazet@google.com
Date:   Tue, 08 Feb 2022 09:25:45 +0100
In-Reply-To: <20220208025511.1019-1-lina.wang@mediatek.com>
References: <20220208025511.1019-1-lina.wang@mediatek.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.42.3 (3.42.3-1.fc35) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Hello,

On Tue, 2022-02-08 at 10:55 +0800, Lina Wang wrote:
> When clatd starts with ebpf offloaing, and NETIF_F_GRO_FRAGLIST is enable,
> several skbs are gathered in skb_shinfo(skb)->frag_list. The first skb's
> ipv6 header will be changed to ipv4 after bpf_skb_proto_6_to_4,
> network_header\transport_header\mac_header have been updated as ipv4 acts,
> but other skbs in frag_list didnot update anything, just ipv6 packets.
> 
> udp_queue_rcv_skb will call skb_segment_list to traverse other skbs in
> frag_list and make sure right udp payload is delivered to user space.
> Unfortunately, other skbs in frag_list who are still ipv6 packets are
> updated like the first skb and will have wrong transport header length.
> 
> e.g.before bpf_skb_proto_6_to_4,the first skb and other skbs in frag_list
> has the same network_header(24)& transport_header(64), after
> bpf_skb_proto_6_to_4, ipv6 protocol has been changed to ipv4, the first
> skb's network_header is 44,transport_header is 64, other skbs in frag_list
> didnot change.After skb_segment_list, the other skbs in frag_list has
> different network_header(24) and transport_header(44), so there will be 20
> bytes difference,that is difference between ipv6 header and ipv4 header.

> Actually, there are two solutions to fix it, one is traversing all skbs
> and changing every skb header in bpf_skb_proto_6_to_4, the other is
> modifying frag_list skb's header in skb_segment_list. 

I don't think the above should be addressed into the GSO layer. The
ebpf program is changing the GRO packet in arbitrary way violating the
GSO packet constraint - arguably, it's corrupting the packet.

I think it would be better change the bpf_skb_proto_6_to_4() to
properly handle FRAGLIST GSO packets.

If traversing the segments become too costly, you can try replacing
GRO_FRAGLIST with GRO_UDP_FWD.

Thanks!

Paolo