Re: [PATCH ipsec] xfrm: Fix inner mode lookup in tunnel mode GSO segmentation

[Sabrina Dubroca <sd@queasysnail.net>]

2025-11-14, 05:56:17 +0200, Jianbo Liu wrote:
> Commit 61fafbee6cfe ("xfrm: Determine inner GSO type from packet
> inner protocol") attempted to fix GSO segmentation by reading the
> inner protocol from XFRM_MODE_SKB_CB(skb)->protocol. This was
> incorrect as the XFRM_MODE_SKB_CB(skb)->protocol field is not assigned
> a value in this code path and led to selecting the wrong inner mode.

Your testing didn't catch it before the patch was submitted? :(


> The correct value is in xfrm_offload(skb)->proto, which is set from
> the outer tunnel header's protocol field by esp[4|6]_gso_encap(). It
> is initialized by xfrm[4|6]_tunnel_encap_add() to either IPPROTO_IPIP
> or IPPROTO_IPV6, using xfrm_af2proto() and correctly reflects the
> inner packet's address family.

What's the call sequence that leads to calling
xfrm4_tunnel_gso_segment without setting
XFRM_MODE_SKB_CB(skb)->protocol? I'm seeing

xfrm_output -> xfrm_output2 -> xfrm_output_one
 -> xfrm_outer_mode_output -> xfrm4_prepare_output
 -> xfrm_inner_extract_output -> xfrm4_extract_output

(almost same as what ends up calling xfrm[4|6]_tunnel_encap_add)
so XFRM_MODE_SKB_CB(skb)->protocol should be set?


Also, after thinking about it more, I'm not so sure that
xfrm_ip2inner_mode is wanted/needed in this context. Since we already
have the inner protocol (whether it's via xo->proto or
XFRM_MODE_SKB_CB(skb)->protocol), and all we care about is the inner
family (to get the corresponding ethertype), we can just get it
directly from the inner protocol without looking at
x->inner_mode{,_iaf}? (pretty much just the reverse of xfrm_af2proto)

-- 
Sabrina