All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload()
@ 2026-08-07  7:09 Rongguang Wei
  2026-08-08 15:39 ` Willem de Bruijn
  2026-08-10 23:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 6+ messages in thread
From: Rongguang Wei @ 2026-08-07  7:09 UTC (permalink / raw)
  To: willemdebruijn.kernel, kuba, jasowangio, andrew+netdev
  Cc: netdev, Rongguang Wei, Willem de Bruijn

From: Rongguang Wei <weirongguang@kylinos.cn>

The USO features in set_offload() incorrectly uses feature_mask and
features argument.

The USO feature was written to the local features variable instead of
feature_mask. All other offload bits (TSO, TSO_ECN) are stored in
feature_mask which becomes tap->tap_features and is used by
tap_handle_frame() for GSO segmentation. Without NETIF_F_GSO_UDP_L4
in tap->tap_features, making USO on tap effectively non-functional.

Keeping the USO handling inside the TUN_F_CSUM block avoids enabling
GRO/LRO when userspace requests USO without CSUM.

Fixes: 399e0827642f ("driver/net/tun: Added features for USO.")
Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
Reviewed-by: Willem de Bruijn <willemb@google.com>
---
v2: Fix more incorrect variable in USO check.
    https://lore.kernel.org/netdev/20260806163949.2807698-1-kuba@kernel.org/
v1: https://lore.kernel.org/netdev/20260805072121.117472-1-clementwei90@163.com/
---
 drivers/net/tap.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 5d2d34d24ce8..d4ca2fee538b 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -883,7 +883,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
 
 		/* TODO: for now USO4 and USO6 should work simultaneously */
 		if ((arg & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
-			features |= NETIF_F_GSO_UDP_L4;
+			feature_mask |= NETIF_F_GSO_UDP_L4;
 	}
 
 	/* tun/tap driver inverts the usage for TSO offloads, where
@@ -894,8 +894,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
 	 * When user space turns off TSO, we turn off GSO/LRO so that
 	 * user-space will not receive TSO frames.
 	 */
-	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6) ||
-	    (feature_mask & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
+	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4))
 		features |= RX_OFFLOADS;
 	else
 		features &= ~RX_OFFLOADS;
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload()
  2026-08-07  7:09 [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload() Rongguang Wei
@ 2026-08-08 15:39 ` Willem de Bruijn
  2026-08-10  1:10   ` Rongguang Wei
  2026-08-10  2:00   ` Rongguang Wei
  2026-08-10 23:40 ` patchwork-bot+netdevbpf
  1 sibling, 2 replies; 6+ messages in thread
From: Willem de Bruijn @ 2026-08-08 15:39 UTC (permalink / raw)
  To: Rongguang Wei, willemdebruijn.kernel, kuba, jasowangio,
	andrew+netdev
  Cc: netdev, Rongguang Wei, Willem de Bruijn

Rongguang Wei wrote:
> From: Rongguang Wei <weirongguang@kylinos.cn>
> 
> The USO features in set_offload() incorrectly uses feature_mask and
> features argument.
> 
> The USO feature was written to the local features variable instead of
> feature_mask. All other offload bits (TSO, TSO_ECN) are stored in
> feature_mask which becomes tap->tap_features and is used by
> tap_handle_frame() for GSO segmentation. Without NETIF_F_GSO_UDP_L4
> in tap->tap_features, making USO on tap effectively non-functional.
> 
> Keeping the USO handling inside the TUN_F_CSUM block avoids enabling
> GRO/LRO when userspace requests USO without CSUM.
> 
> Fixes: 399e0827642f ("driver/net/tun: Added features for USO.")
> Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
> Reviewed-by: Willem de Bruijn <willemb@google.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Substantially changed patch, please don't keep Reviewed-by tags across
non-trivial changes. That said, I do agree, so adding it again.

> ---
> v2: Fix more incorrect variable in USO check.
>     https://lore.kernel.org/netdev/20260806163949.2807698-1-kuba@kernel.org/
> v1: https://lore.kernel.org/netdev/20260805072121.117472-1-clementwei90@163.com/
> ---
>  drivers/net/tap.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> index 5d2d34d24ce8..d4ca2fee538b 100644
> --- a/drivers/net/tap.c
> +++ b/drivers/net/tap.c
> @@ -883,7 +883,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>  
>  		/* TODO: for now USO4 and USO6 should work simultaneously */
>  		if ((arg & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
> -			features |= NETIF_F_GSO_UDP_L4;
> +			feature_mask |= NETIF_F_GSO_UDP_L4;

Drivers/net/tun seems to have this same issue.

Do you want to take a look or shall I?


>  	}
>  
>  	/* tun/tap driver inverts the usage for TSO offloads, where
> @@ -894,8 +894,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>  	 * When user space turns off TSO, we turn off GSO/LRO so that
>  	 * user-space will not receive TSO frames.
>  	 */
> -	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6) ||
> -	    (feature_mask & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
> +	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4))
>  		features |= RX_OFFLOADS;
>  	else
>  		features &= ~RX_OFFLOADS;

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload()
  2026-08-08 15:39 ` Willem de Bruijn
@ 2026-08-10  1:10   ` Rongguang Wei
  2026-08-10  2:00   ` Rongguang Wei
  1 sibling, 0 replies; 6+ messages in thread
From: Rongguang Wei @ 2026-08-10  1:10 UTC (permalink / raw)
  To: Willem de Bruijn, kuba, jasowangio, andrew+netdev
  Cc: netdev, Rongguang Wei, Willem de Bruijn



Willem de Bruijn wrote:
> Rongguang Wei wrote:
>> From: Rongguang Wei <weirongguang@kylinos.cn>
>>
>> The USO features in set_offload() incorrectly uses feature_mask and
>> features argument.
>>
>> The USO feature was written to the local features variable instead of
>> feature_mask. All other offload bits (TSO, TSO_ECN) are stored in
>> feature_mask which becomes tap->tap_features and is used by
>> tap_handle_frame() for GSO segmentation. Without NETIF_F_GSO_UDP_L4
>> in tap->tap_features, making USO on tap effectively non-functional.
>>
>> Keeping the USO handling inside the TUN_F_CSUM block avoids enabling
>> GRO/LRO when userspace requests USO without CSUM.
>>
>> Fixes: 399e0827642f ("driver/net/tun: Added features for USO.")
>> Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
>> Reviewed-by: Willem de Bruijn <willemb@google.com>
> 
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> 
> Substantially changed patch, please don't keep Reviewed-by tags across
> non-trivial changes. That said, I do agree, so adding it again.

I am sorry for that and thanks for the reminder.
> 
>> ---
>> v2: Fix more incorrect variable in USO check.
>>     https://lore.kernel.org/netdev/20260806163949.2807698-1-kuba@kernel.org/
>> v1: https://lore.kernel.org/netdev/20260805072121.117472-1-clementwei90@163.com/
>> ---
>>  drivers/net/tap.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>> index 5d2d34d24ce8..d4ca2fee538b 100644
>> --- a/drivers/net/tap.c
>> +++ b/drivers/net/tap.c
>> @@ -883,7 +883,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>>  
>>  		/* TODO: for now USO4 and USO6 should work simultaneously */
>>  		if ((arg & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
>> -			features |= NETIF_F_GSO_UDP_L4;
>> +			feature_mask |= NETIF_F_GSO_UDP_L4;
> 
> Drivers/net/tun seems to have this same issue.
> 
> Do you want to take a look or shall I?
Sure, I'd be glad to do with that. Should the tun modifications be combined with this patch?
> 
> 
>>  	}
>>  
>>  	/* tun/tap driver inverts the usage for TSO offloads, where
>> @@ -894,8 +894,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>>  	 * When user space turns off TSO, we turn off GSO/LRO so that
>>  	 * user-space will not receive TSO frames.
>>  	 */
>> -	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6) ||
>> -	    (feature_mask & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
>> +	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4))
>>  		features |= RX_OFFLOADS;
>>  	else
>>  		features &= ~RX_OFFLOADS;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload()
  2026-08-08 15:39 ` Willem de Bruijn
  2026-08-10  1:10   ` Rongguang Wei
@ 2026-08-10  2:00   ` Rongguang Wei
  2026-08-10 19:55     ` Willem de Bruijn
  1 sibling, 1 reply; 6+ messages in thread
From: Rongguang Wei @ 2026-08-10  2:00 UTC (permalink / raw)
  To: Willem de Bruijn, kuba, jasowangio, andrew+netdev
  Cc: netdev, Rongguang Wei, Willem de Bruijn

Willem de Bruijn wrote:
> Rongguang Wei wrote:
>> From: Rongguang Wei <weirongguang@kylinos.cn>
>>
>> The USO features in set_offload() incorrectly uses feature_mask and
>> features argument.
>>
>> The USO feature was written to the local features variable instead of
>> feature_mask. All other offload bits (TSO, TSO_ECN) are stored in
>> feature_mask which becomes tap->tap_features and is used by
>> tap_handle_frame() for GSO segmentation. Without NETIF_F_GSO_UDP_L4
>> in tap->tap_features, making USO on tap effectively non-functional.
>>
>> Keeping the USO handling inside the TUN_F_CSUM block avoids enabling
>> GRO/LRO when userspace requests USO without CSUM.
>>
>> Fixes: 399e0827642f ("driver/net/tun: Added features for USO.")
>> Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
>> Reviewed-by: Willem de Bruijn <willemb@google.com>
> 
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> 
> Substantially changed patch, please don't keep Reviewed-by tags across
> non-trivial changes. That said, I do agree, so adding it again.
> 
>> ---
>> v2: Fix more incorrect variable in USO check.
>>     https://lore.kernel.org/netdev/20260806163949.2807698-1-kuba@kernel.org/
>> v1: https://lore.kernel.org/netdev/20260805072121.117472-1-clementwei90@163.com/
>> ---
>>  drivers/net/tap.c | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
>> index 5d2d34d24ce8..d4ca2fee538b 100644
>> --- a/drivers/net/tap.c
>> +++ b/drivers/net/tap.c
>> @@ -883,7 +883,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>>  
>>  		/* TODO: for now USO4 and USO6 should work simultaneously */
>>  		if ((arg & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
>> -			features |= NETIF_F_GSO_UDP_L4;
>> +			feature_mask |= NETIF_F_GSO_UDP_L4;
> 
> Drivers/net/tun seems to have this same issue.
> 
> Do you want to take a look or shall I?
> 
Hi,
I have check the code in drivers/net/tun.c.
In tun.c, the tun's all offload bits go into features argument and
I think tun.c does not have this issue.
> 
>>  	}
>>  
>>  	/* tun/tap driver inverts the usage for TSO offloads, where
>> @@ -894,8 +894,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
>>  	 * When user space turns off TSO, we turn off GSO/LRO so that
>>  	 * user-space will not receive TSO frames.
>>  	 */
>> -	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6) ||
>> -	    (feature_mask & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
>> +	if (feature_mask & (NETIF_F_TSO | NETIF_F_TSO6 | NETIF_F_GSO_UDP_L4))
>>  		features |= RX_OFFLOADS;
>>  	else
>>  		features &= ~RX_OFFLOADS;


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload()
  2026-08-10  2:00   ` Rongguang Wei
@ 2026-08-10 19:55     ` Willem de Bruijn
  0 siblings, 0 replies; 6+ messages in thread
From: Willem de Bruijn @ 2026-08-10 19:55 UTC (permalink / raw)
  To: Rongguang Wei, Willem de Bruijn, kuba, jasowangio, andrew+netdev
  Cc: netdev, Rongguang Wei, Willem de Bruijn

Rongguang Wei wrote:
> Willem de Bruijn wrote:
> > Rongguang Wei wrote:
> >> From: Rongguang Wei <weirongguang@kylinos.cn>
> >>
> >> The USO features in set_offload() incorrectly uses feature_mask and
> >> features argument.
> >>
> >> The USO feature was written to the local features variable instead of
> >> feature_mask. All other offload bits (TSO, TSO_ECN) are stored in
> >> feature_mask which becomes tap->tap_features and is used by
> >> tap_handle_frame() for GSO segmentation. Without NETIF_F_GSO_UDP_L4
> >> in tap->tap_features, making USO on tap effectively non-functional.
> >>
> >> Keeping the USO handling inside the TUN_F_CSUM block avoids enabling
> >> GRO/LRO when userspace requests USO without CSUM.
> >>
> >> Fixes: 399e0827642f ("driver/net/tun: Added features for USO.")
> >> Signed-off-by: Rongguang Wei <weirongguang@kylinos.cn>
> >> Reviewed-by: Willem de Bruijn <willemb@google.com>
> > 
> > Reviewed-by: Willem de Bruijn <willemb@google.com>
> > 
> > Substantially changed patch, please don't keep Reviewed-by tags across
> > non-trivial changes. That said, I do agree, so adding it again.
> > 
> >> ---
> >> v2: Fix more incorrect variable in USO check.
> >>     https://lore.kernel.org/netdev/20260806163949.2807698-1-kuba@kernel.org/
> >> v1: https://lore.kernel.org/netdev/20260805072121.117472-1-clementwei90@163.com/
> >> ---
> >>  drivers/net/tap.c | 5 ++---
> >>  1 file changed, 2 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/net/tap.c b/drivers/net/tap.c
> >> index 5d2d34d24ce8..d4ca2fee538b 100644
> >> --- a/drivers/net/tap.c
> >> +++ b/drivers/net/tap.c
> >> @@ -883,7 +883,7 @@ static int set_offload(struct tap_queue *q, unsigned long arg)
> >>  
> >>  		/* TODO: for now USO4 and USO6 should work simultaneously */
> >>  		if ((arg & (TUN_F_USO4 | TUN_F_USO6)) == (TUN_F_USO4 | TUN_F_USO6))
> >> -			features |= NETIF_F_GSO_UDP_L4;
> >> +			feature_mask |= NETIF_F_GSO_UDP_L4;
> > 
> > Drivers/net/tun seems to have this same issue.
> > 
> > Do you want to take a look or shall I?
> > 
> Hi,
> I have check the code in drivers/net/tun.c.
> In tun.c, the tun's all offload bits go into features argument and
> I think tun.c does not have this issue.

Oh indeed. Thanks for checking.

To repeat, this patch LGTM.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload()
  2026-08-07  7:09 [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload() Rongguang Wei
  2026-08-08 15:39 ` Willem de Bruijn
@ 2026-08-10 23:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-10 23:40 UTC (permalink / raw)
  To: Rongguang Wei
  Cc: willemdebruijn.kernel, kuba, jasowangio, andrew+netdev, netdev,
	weirongguang, willemb

Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Fri,  7 Aug 2026 15:09:14 +0800 you wrote:
> From: Rongguang Wei <weirongguang@kylinos.cn>
> 
> The USO features in set_offload() incorrectly uses feature_mask and
> features argument.
> 
> The USO feature was written to the local features variable instead of
> feature_mask. All other offload bits (TSO, TSO_ECN) are stored in
> feature_mask which becomes tap->tap_features and is used by
> tap_handle_frame() for GSO segmentation. Without NETIF_F_GSO_UDP_L4
> in tap->tap_features, making USO on tap effectively non-functional.
> 
> [...]

Here is the summary with links:
  - [net,v2] tap: fix incorrect variable used for USO check in set_offload()
    https://git.kernel.org/netdev/net-next/c/fac7973f004d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-08-10 23:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-07  7:09 [PATCH net v2] tap: fix incorrect variable used for USO check in set_offload() Rongguang Wei
2026-08-08 15:39 ` Willem de Bruijn
2026-08-10  1:10   ` Rongguang Wei
2026-08-10  2:00   ` Rongguang Wei
2026-08-10 19:55     ` Willem de Bruijn
2026-08-10 23:40 ` patchwork-bot+netdevbpf

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.