netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] Bonding: Fix support for gso_partial_features
@ 2025-01-21  8:55 Hangbin Liu
  2025-01-21  9:55 ` Hangbin Liu
  0 siblings, 1 reply; 2+ messages in thread
From: Hangbin Liu @ 2025-01-21  8:55 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Nikolay Aleksandrov, Simon Horman,
	Jianbo Liu, Boris Pismenny, Tariq Toukan, linux-kselftest,
	linux-kernel, Hangbin Liu, Liang Li

The fixed commit adds NETIF_F_GSO_ESP bit for bonding gso_partial_features.
However, if we don't set the dev NETIF_F_GSO_PARTIAL bit, the later
netdev_change_features() -> netdev_fix_features() will remove the
NETIF_F_GSO_ESP bit from the dev features. This causes ethtool to show
that the bond does not support tx-esp-segmentation. For example

 # ethtool -k bond0 | grep esp
 tx-esp-segmentation: off [requested on]
 esp-hw-offload: on
 esp-tx-csum-hw-offload: on

Add the NETIF_F_GSO_PARTIAL bit to bond dev features when set
gso_partial_features to fix this issue.

Fixes: 4861333b4217 ("bonding: add ESP offload features when slaves support")
Reported-by: Liang Li <liali@redhat.com>
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 drivers/net/bonding/bond_main.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 7b78c2bada81..e1c054416d5e 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1598,10 +1598,12 @@ static void bond_compute_features(struct bonding *bond)
 	}
 	bond_dev->hard_header_len = max_hard_header_len;
 
-	if (gso_partial_features & NETIF_F_GSO_ESP)
+	if (gso_partial_features & NETIF_F_GSO_ESP) {
 		bond_dev->gso_partial_features |= NETIF_F_GSO_ESP;
-	else
+		bond_dev->features |= NETIF_F_GSO_PARTIAL;
+	} else {
 		bond_dev->gso_partial_features &= ~NETIF_F_GSO_ESP;
+	}
 
 done:
 	bond_dev->vlan_features = vlan_features;
-- 
2.39.5 (Apple Git-154)


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

* Re: [PATCH net] Bonding: Fix support for gso_partial_features
  2025-01-21  8:55 [PATCH net] Bonding: Fix support for gso_partial_features Hangbin Liu
@ 2025-01-21  9:55 ` Hangbin Liu
  0 siblings, 0 replies; 2+ messages in thread
From: Hangbin Liu @ 2025-01-21  9:55 UTC (permalink / raw)
  To: netdev
  Cc: Jay Vosburgh, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Nikolay Aleksandrov, Simon Horman,
	Jianbo Liu, Boris Pismenny, Tariq Toukan, linux-kselftest,
	linux-kernel, Liang Li

On Tue, Jan 21, 2025 at 08:55:25AM +0000, Hangbin Liu wrote:
> The fixed commit adds NETIF_F_GSO_ESP bit for bonding gso_partial_features.
> However, if we don't set the dev NETIF_F_GSO_PARTIAL bit, the later
> netdev_change_features() -> netdev_fix_features() will remove the
> NETIF_F_GSO_ESP bit from the dev features. This causes ethtool to show
> that the bond does not support tx-esp-segmentation. For example
> 
>  # ethtool -k bond0 | grep esp
>  tx-esp-segmentation: off [requested on]
>  esp-hw-offload: on
>  esp-tx-csum-hw-offload: on
> 
> Add the NETIF_F_GSO_PARTIAL bit to bond dev features when set
> gso_partial_features to fix this issue.
> 
> Fixes: 4861333b4217 ("bonding: add ESP offload features when slaves support")
> Reported-by: Liang Li <liali@redhat.com>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
>  drivers/net/bonding/bond_main.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 7b78c2bada81..e1c054416d5e 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1598,10 +1598,12 @@ static void bond_compute_features(struct bonding *bond)
>  	}
>  	bond_dev->hard_header_len = max_hard_header_len;
>  
> -	if (gso_partial_features & NETIF_F_GSO_ESP)
> +	if (gso_partial_features & NETIF_F_GSO_ESP) {
>  		bond_dev->gso_partial_features |= NETIF_F_GSO_ESP;
> -	else
> +		bond_dev->features |= NETIF_F_GSO_PARTIAL;
> +	} else {
>  		bond_dev->gso_partial_features &= ~NETIF_F_GSO_ESP;
> +	}

Hmm, looks we need to remove NETIF_F_GSO_PARTIAL when remove the
gso_partial_features.

BTW, I saw some drivers set NETIF_F_GSO_PARTIAL on dev->features. Some
other drivers set NETIF_F_GSO_PARTIAL on dev->hw_enc_features. I haven't
see a doc about where we should set. Please tell me if you have any comments.

Thanks
Hangbin

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

end of thread, other threads:[~2025-01-21  9:55 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-21  8:55 [PATCH net] Bonding: Fix support for gso_partial_features Hangbin Liu
2025-01-21  9:55 ` Hangbin Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).