Re: [PATCH ipsec v2] xfrm: check MAC header is shown with both skb->mac_len and skb_mac_header_was_set()

[Eric Dumazet]

On Fri, Oct 18, 2024 at 3:22 PM En-Wei WU <en-wei.wu@xxxxxxxxxxxxx> wrote:
>
> > Seems like the __netif_receive_skb_core() and dev_gro_receive() are
> > the places where it calls skb_reset_mac_len() with skb->mac_header =
> > ~0U.
> I believe it's the root cause.
>
> My concern is that if we put something like:
> +       if (!skb_mac_header_was_set(skb)) {
> +               DEBUG_NET_WARN_ON_ONCE(1);
> +               skb->mac_len = 0;
> in skb_reset_mac_len(), it may degrade the RX path a bit.

I do not have such concerns. Note this is temporary until we fix the root cause.

>
> Catching the bug in xfrm4_remove_tunnel_encap() and
> xfrm6_remove_tunnel_encap() (the original patch) is nice because it
> won't affect the systems which are not using the xfrm.
>

Somehow xfrm is feeding to gro_cells_receive() packets without the mac
header being set, this is the bug that needs to be fixed.

GRO needs skb_mac_header() to return the correct pointer.

For normal GRO, it is set either in :

1) napi_gro_frags : napi_frags_skb()  calls skb_reset_mac_header(skb);

2) napi_gro_receive() : callers are supposed to call eth_type_trans()
before calling napi_gro_receive().
    eth_type_trans() calls skb_reset_mac_header() as expected.

xfrm calls skb_mac_header_rebuild(), but it might be a NOP if MAC
header was never set.