netdev - Re: [PATCH net-next v3 3/5] net: gso: restore ids of outer ip headers correctly

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: <73927f0c-f6aa-464b-ab20-559196e015a8@gmail.com>
Date: Thu, 21 Aug 2025 17:20:24 +0100
From: Edward Cree <ecree.xilinx@...il.com>
To: Richard Gobert <richardbgobert@...il.com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, horms@...nel.org, corbet@....net, saeedm@...dia.com,
 tariqt@...dia.com, mbloch@...dia.com, leon@...nel.org, dsahern@...nel.org,
 ncardwell@...gle.com, kuniyu@...gle.com, shuah@...nel.org, sdf@...ichev.me,
 aleksander.lobakin@...el.com, florian.fainelli@...adcom.com,
 willemdebruijn.kernel@...il.com, alexander.duyck@...il.com,
 linux-kernel@...r.kernel.org, linux-net-drivers@....com
Subject: Re: [PATCH net-next v3 3/5] net: gso: restore ids of outer ip headers
 correctly

On 21/08/2025 08:30, Richard Gobert wrote:
> Currently, NETIF_F_TSO_MANGLEID indicates that the inner-most ID can
> be mangled. Outer IDs can always be mangled.
> 
> Make GSO preserve outer IDs by default, with NETIF_F_TSO_MANGLEID allowing
> both inner and outer IDs to be mangled. In the future, we could add
> NETIF_F_TSO_MANGLEID_INNER to provide more granular control to
> drivers.
> 
> This commit also modifies a few drivers that use SKB_GSO_FIXEDID directly.
> 
> Signed-off-by: Richard Gobert <richardbgobert@...il.com>
...
> diff --git a/drivers/net/ethernet/sfc/ef100_tx.c b/drivers/net/ethernet/sfc/ef100_tx.c
> index e6b6be549581..4efd22b44986 100644
> --- a/drivers/net/ethernet/sfc/ef100_tx.c
> +++ b/drivers/net/ethernet/sfc/ef100_tx.c
> @@ -189,7 +189,8 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
>  {
>  	bool gso_partial = skb_shinfo(skb)->gso_type & SKB_GSO_PARTIAL;
>  	unsigned int len, ip_offset, tcp_offset, payload_segs;
> -	u32 mangleid = ESE_GZ_TX_DESC_IP4_ID_INC_MOD16;
> +	u32 mangleid_outer = ESE_GZ_TX_DESC_IP4_ID_INC_MOD16;
> +	u32 mangleid_inner = ESE_GZ_TX_DESC_IP4_ID_INC_MOD16;
>  	unsigned int outer_ip_offset, outer_l4_offset;
>  	u16 vlan_tci = skb_vlan_tag_get(skb);
>  	u32 mss = skb_shinfo(skb)->gso_size;
> @@ -201,7 +202,9 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
>  	u32 paylen;
>  
>  	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID)
> -		mangleid = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
> +		mangleid_outer = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
> +	if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_FIXEDID_INNER)
> +		mangleid_inner = ESE_GZ_TX_DESC_IP4_ID_NO_OP;
>  	if (efx->net_dev->features & NETIF_F_HW_VLAN_CTAG_TX)
>  		vlan_enable = skb_vlan_tag_present(skb);
>  
> @@ -239,14 +242,13 @@ static void ef100_make_tso_desc(struct efx_nic *efx,
>  			      ESF_GZ_TX_TSO_CSO_INNER_L4, 1,
>  			      ESF_GZ_TX_TSO_INNER_L3_OFF_W, ip_offset >> 1,
>  			      ESF_GZ_TX_TSO_INNER_L4_OFF_W, tcp_offset >> 1,
> -			      ESF_GZ_TX_TSO_ED_INNER_IP4_ID, mangleid,
> +			      ESF_GZ_TX_TSO_ED_INNER_IP4_ID, mangleid_inner,
>  			      ESF_GZ_TX_TSO_ED_INNER_IP_LEN, 1,
>  			      ESF_GZ_TX_TSO_OUTER_L3_OFF_W, outer_ip_offset >> 1,
>  			      ESF_GZ_TX_TSO_OUTER_L4_OFF_W, outer_l4_offset >> 1,
>  			      ESF_GZ_TX_TSO_ED_OUTER_UDP_LEN, udp_encap && !gso_partial,
>  			      ESF_GZ_TX_TSO_ED_OUTER_IP_LEN, encap && !gso_partial,
> -			      ESF_GZ_TX_TSO_ED_OUTER_IP4_ID, encap ? mangleid :
> -								     ESE_GZ_TX_DESC_IP4_ID_NO_OP,
> +			      ESF_GZ_TX_TSO_ED_OUTER_IP4_ID, mangleid_outer,
>  			      ESF_GZ_TX_TSO_VLAN_INSERT_EN, vlan_enable,
>  			      ESF_GZ_TX_TSO_VLAN_INSERT_TCI, vlan_tci
>  		);

AFAICT this will now, in the case when FIXEDID isn't set, set
 ESF_GZ_TX_TSO_ED_OUTER_IP4_ID on non-encapsulated frames, for which
 ESF_GZ_TX_TSO_OUTER_L3_OFF_W has been set to 0.  I'm not 100% sure,
 but I think that will cause the NIC to do an INC_MOD16 on octets 4
 and 5 of the packet, corrupting the Ethernet header.
Please retain the existing logic whereby ED_OUTER_IP4_ID is set to
 NO_OP in the !encap case.
Note that the EF100 host interface's semantics take the view that an
 unencapsulated packet has an INNER and no OUTER header, which AIUI
 is the opposite to how your new gso_type flags are defined, so I
 think for !encap you also need to set mangleid_inner based on
 SKB_GSO_TCP_FIXEDID, rather than SKB_GSO_TCP_FIXEDID_INNER.

My apologies for not spotting this in earlier versions.