linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 net-next 2/2] net: bonding, dummy, ifb, team: advertise NETIF_F_GSO_SOFTWARE
@ 2020-11-01 13:17 Alexander Lobakin
  2020-11-02 16:30 ` Willem de Bruijn
  0 siblings, 1 reply; 4+ messages in thread
From: Alexander Lobakin @ 2020-11-01 13:17 UTC (permalink / raw)
  To: David S. Miller, Jakub Kicinski
  Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, Jiri Pirko,
	Steffen Klassert, Willem de Bruijn, Alexander Lobakin,
	Miaohe Lin, Antoine Tenart, Mauro Carvalho Chehab, netdev,
	linux-kernel

Virtual netdevs should use NETIF_F_GSO_SOFTWARE to forward GSO skbs
as-is and let the final drivers deal with them when supported.
Also remove NETIF_F_GSO_UDP_L4 from bonding and team drivers as it's
now included in the "software" list.

Suggested-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Alexander Lobakin <alobakin@pm.me>
---
 drivers/net/bonding/bond_main.c | 11 +++++------
 drivers/net/dummy.c             |  2 +-
 drivers/net/ifb.c               |  3 +--
 drivers/net/team/team.c         |  9 ++++-----
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 84ecbc6fa0ff..71c9677d135f 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1228,14 +1228,14 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
 }
 
 #define BOND_VLAN_FEATURES	(NETIF_F_HW_CSUM | NETIF_F_SG | \
-				 NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
+				 NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \
 				 NETIF_F_HIGHDMA | NETIF_F_LRO)
 
 #define BOND_ENC_FEATURES	(NETIF_F_HW_CSUM | NETIF_F_SG | \
-				 NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
+				 NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE)
 
 #define BOND_MPLS_FEATURES	(NETIF_F_HW_CSUM | NETIF_F_SG | \
-				 NETIF_F_ALL_TSO)
+				 NETIF_F_GSO_SOFTWARE)
 
 
 static void bond_compute_features(struct bonding *bond)
@@ -1291,8 +1291,7 @@ static void bond_compute_features(struct bonding *bond)
 	bond_dev->vlan_features = vlan_features;
 	bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
 				    NETIF_F_HW_VLAN_CTAG_TX |
-				    NETIF_F_HW_VLAN_STAG_TX |
-				    NETIF_F_GSO_UDP_L4;
+				    NETIF_F_HW_VLAN_STAG_TX;
 #ifdef CONFIG_XFRM_OFFLOAD
 	bond_dev->hw_enc_features |= xfrm_features;
 #endif /* CONFIG_XFRM_OFFLOAD */
@@ -4721,7 +4720,7 @@ void bond_setup(struct net_device *bond_dev)
 				NETIF_F_HW_VLAN_CTAG_RX |
 				NETIF_F_HW_VLAN_CTAG_FILTER;
 
-	bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4;
+	bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
 #ifdef CONFIG_XFRM_OFFLOAD
 	bond_dev->hw_features |= BOND_XFRM_FEATURES;
 #endif /* CONFIG_XFRM_OFFLOAD */
diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
index bab3a9bb5e6f..f82ad7419508 100644
--- a/drivers/net/dummy.c
+++ b/drivers/net/dummy.c
@@ -124,7 +124,7 @@ static void dummy_setup(struct net_device *dev)
 	dev->flags &= ~IFF_MULTICAST;
 	dev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE;
 	dev->features	|= NETIF_F_SG | NETIF_F_FRAGLIST;
-	dev->features	|= NETIF_F_ALL_TSO;
+	dev->features	|= NETIF_F_GSO_SOFTWARE;
 	dev->features	|= NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX;
 	dev->features	|= NETIF_F_GSO_ENCAP_ALL;
 	dev->hw_features |= dev->features;
diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 7fe306e76281..fa63d4dee0ba 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -187,8 +187,7 @@ static const struct net_device_ops ifb_netdev_ops = {
 };
 
 #define IFB_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG  | NETIF_F_FRAGLIST	| \
-		      NETIF_F_TSO_ECN | NETIF_F_TSO | NETIF_F_TSO6	| \
-		      NETIF_F_GSO_ENCAP_ALL 				| \
+		      NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ENCAP_ALL	| \
 		      NETIF_F_HIGHDMA | NETIF_F_HW_VLAN_CTAG_TX		| \
 		      NETIF_F_HW_VLAN_STAG_TX)
 
diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
index 07f1f3933927..b4092127a92c 100644
--- a/drivers/net/team/team.c
+++ b/drivers/net/team/team.c
@@ -975,11 +975,11 @@ static void team_port_disable(struct team *team,
 }
 
 #define TEAM_VLAN_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
-			    NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
+			    NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \
 			    NETIF_F_HIGHDMA | NETIF_F_LRO)
 
 #define TEAM_ENC_FEATURES	(NETIF_F_HW_CSUM | NETIF_F_SG | \
-				 NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
+				 NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE)
 
 static void __team_compute_features(struct team *team)
 {
@@ -1009,8 +1009,7 @@ static void __team_compute_features(struct team *team)
 	team->dev->vlan_features = vlan_features;
 	team->dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
 				     NETIF_F_HW_VLAN_CTAG_TX |
-				     NETIF_F_HW_VLAN_STAG_TX |
-				     NETIF_F_GSO_UDP_L4;
+				     NETIF_F_HW_VLAN_STAG_TX;
 	team->dev->hard_header_len = max_hard_header_len;
 
 	team->dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
@@ -2175,7 +2174,7 @@ static void team_setup(struct net_device *dev)
 			   NETIF_F_HW_VLAN_CTAG_RX |
 			   NETIF_F_HW_VLAN_CTAG_FILTER;
 
-	dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4;
+	dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
 	dev->features |= dev->hw_features;
 	dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
 }
-- 
2.29.2



^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next 2/2] net: bonding, dummy, ifb, team: advertise NETIF_F_GSO_SOFTWARE
  2020-11-01 13:17 [PATCH v2 net-next 2/2] net: bonding, dummy, ifb, team: advertise NETIF_F_GSO_SOFTWARE Alexander Lobakin
@ 2020-11-02 16:30 ` Willem de Bruijn
  2020-11-02 19:25   ` Alexander Lobakin
  0 siblings, 1 reply; 4+ messages in thread
From: Willem de Bruijn @ 2020-11-02 16:30 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: David S. Miller, Jakub Kicinski, Jay Vosburgh, Veaceslav Falico,
	Andy Gospodarek, Jiri Pirko, Steffen Klassert, Miaohe Lin,
	Antoine Tenart, Mauro Carvalho Chehab, Network Development,
	linux-kernel

On Sun, Nov 1, 2020 at 8:17 AM Alexander Lobakin <alobakin@pm.me> wrote:
>
> Virtual netdevs should use NETIF_F_GSO_SOFTWARE to forward GSO skbs
> as-is and let the final drivers deal with them when supported.
> Also remove NETIF_F_GSO_UDP_L4 from bonding and team drivers as it's
> now included in the "software" list.

The rationale is that it is okay to advertise these features with
software fallback as bonding/teaming "hardware" features, because
there will always be a downstream device for which they will be
implemented, possibly in the software fallback, correct?

That does not apply to dummy or IFB. I guess dummy is fine, because
xmit is a black hole, and IFB because ingress can safely handle these
packets? How did you arrive at the choice of changing these two, of
all virtual devices?

>
> Suggested-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
> ---
>  drivers/net/bonding/bond_main.c | 11 +++++------
>  drivers/net/dummy.c             |  2 +-
>  drivers/net/ifb.c               |  3 +--
>  drivers/net/team/team.c         |  9 ++++-----
>  4 files changed, 11 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 84ecbc6fa0ff..71c9677d135f 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1228,14 +1228,14 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
>  }
>
>  #define BOND_VLAN_FEATURES     (NETIF_F_HW_CSUM | NETIF_F_SG | \
> -                                NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
> +                                NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \
>                                  NETIF_F_HIGHDMA | NETIF_F_LRO)
>
>  #define BOND_ENC_FEATURES      (NETIF_F_HW_CSUM | NETIF_F_SG | \
> -                                NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
> +                                NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE)
>
>  #define BOND_MPLS_FEATURES     (NETIF_F_HW_CSUM | NETIF_F_SG | \
> -                                NETIF_F_ALL_TSO)
> +                                NETIF_F_GSO_SOFTWARE)
>
>
>  static void bond_compute_features(struct bonding *bond)
> @@ -1291,8 +1291,7 @@ static void bond_compute_features(struct bonding *bond)
>         bond_dev->vlan_features = vlan_features;
>         bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
>                                     NETIF_F_HW_VLAN_CTAG_TX |
> -                                   NETIF_F_HW_VLAN_STAG_TX |
> -                                   NETIF_F_GSO_UDP_L4;
> +                                   NETIF_F_HW_VLAN_STAG_TX;
>  #ifdef CONFIG_XFRM_OFFLOAD
>         bond_dev->hw_enc_features |= xfrm_features;
>  #endif /* CONFIG_XFRM_OFFLOAD */
> @@ -4721,7 +4720,7 @@ void bond_setup(struct net_device *bond_dev)
>                                 NETIF_F_HW_VLAN_CTAG_RX |
>                                 NETIF_F_HW_VLAN_CTAG_FILTER;
>
> -       bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4;
> +       bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
>  #ifdef CONFIG_XFRM_OFFLOAD
>         bond_dev->hw_features |= BOND_XFRM_FEATURES;
>  #endif /* CONFIG_XFRM_OFFLOAD */
> diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
> index bab3a9bb5e6f..f82ad7419508 100644
> --- a/drivers/net/dummy.c
> +++ b/drivers/net/dummy.c
> @@ -124,7 +124,7 @@ static void dummy_setup(struct net_device *dev)
>         dev->flags &= ~IFF_MULTICAST;
>         dev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE;
>         dev->features   |= NETIF_F_SG | NETIF_F_FRAGLIST;
> -       dev->features   |= NETIF_F_ALL_TSO;
> +       dev->features   |= NETIF_F_GSO_SOFTWARE;
>         dev->features   |= NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX;
>         dev->features   |= NETIF_F_GSO_ENCAP_ALL;
>         dev->hw_features |= dev->features;
> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
> index 7fe306e76281..fa63d4dee0ba 100644
> --- a/drivers/net/ifb.c
> +++ b/drivers/net/ifb.c
> @@ -187,8 +187,7 @@ static const struct net_device_ops ifb_netdev_ops = {
>  };
>
>  #define IFB_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG  | NETIF_F_FRAGLIST | \
> -                     NETIF_F_TSO_ECN | NETIF_F_TSO | NETIF_F_TSO6      | \
> -                     NETIF_F_GSO_ENCAP_ALL                             | \
> +                     NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ENCAP_ALL      | \
>                       NETIF_F_HIGHDMA | NETIF_F_HW_VLAN_CTAG_TX         | \
>                       NETIF_F_HW_VLAN_STAG_TX)
>
> diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
> index 07f1f3933927..b4092127a92c 100644
> --- a/drivers/net/team/team.c
> +++ b/drivers/net/team/team.c
> @@ -975,11 +975,11 @@ static void team_port_disable(struct team *team,
>  }
>
>  #define TEAM_VLAN_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
> -                           NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
> +                           NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \
>                             NETIF_F_HIGHDMA | NETIF_F_LRO)
>
>  #define TEAM_ENC_FEATURES      (NETIF_F_HW_CSUM | NETIF_F_SG | \
> -                                NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
> +                                NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE)
>
>  static void __team_compute_features(struct team *team)
>  {
> @@ -1009,8 +1009,7 @@ static void __team_compute_features(struct team *team)
>         team->dev->vlan_features = vlan_features;
>         team->dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
>                                      NETIF_F_HW_VLAN_CTAG_TX |
> -                                    NETIF_F_HW_VLAN_STAG_TX |
> -                                    NETIF_F_GSO_UDP_L4;
> +                                    NETIF_F_HW_VLAN_STAG_TX;
>         team->dev->hard_header_len = max_hard_header_len;
>
>         team->dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
> @@ -2175,7 +2174,7 @@ static void team_setup(struct net_device *dev)
>                            NETIF_F_HW_VLAN_CTAG_RX |
>                            NETIF_F_HW_VLAN_CTAG_FILTER;
>
> -       dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4;
> +       dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
>         dev->features |= dev->hw_features;
>         dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
>  }
> --
> 2.29.2
>
>

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next 2/2] net: bonding, dummy, ifb, team: advertise NETIF_F_GSO_SOFTWARE
  2020-11-02 16:30 ` Willem de Bruijn
@ 2020-11-02 19:25   ` Alexander Lobakin
  2020-11-02 20:04     ` Willem de Bruijn
  0 siblings, 1 reply; 4+ messages in thread
From: Alexander Lobakin @ 2020-11-02 19:25 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Alexander Lobakin, David S. Miller, Jakub Kicinski, Jay Vosburgh,
	Veaceslav Falico, Andy Gospodarek, Jiri Pirko, Steffen Klassert,
	Miaohe Lin, Antoine Tenart, Mauro Carvalho Chehab,
	Network Development, linux-kernel

From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Mon, 2 Nov 2020 11:30:17 -0500

Hi!
Thanks for the Ack.

> On Sun, Nov 1, 2020 at 8:17 AM Alexander Lobakin <alobakin@pm.me> wrote:
>>
>> Virtual netdevs should use NETIF_F_GSO_SOFTWARE to forward GSO skbs
>> as-is and let the final drivers deal with them when supported.
>> Also remove NETIF_F_GSO_UDP_L4 from bonding and team drivers as it's
>> now included in the "software" list.
>
> The rationale is that it is okay to advertise these features with
> software fallback as bonding/teaming "hardware" features, because
> there will always be a downstream device for which they will be
> implemented, possibly in the software fallback, correct?
>
> That does not apply to dummy or IFB. I guess dummy is fine, because
> xmit is a black hole, and IFB because ingress can safely handle these
> packets? How did you arrive at the choice of changing these two, of
> all virtual devices?

Two points:
1. Exactly, dummy is just dummy, while ifb is an intermediate netdev to
   share resources, so it should be as fine as with other virtual devs.
2. They both advertise NETIF_F_ALL_TSO | NETIF_F_GSO_ENCAP_ALL, which
   assumes that they handle all GSO skbs just like the others (pass
   them as is to the real drivers in case with ifb).

>>
>> Suggested-by: Willem de Bruijn <willemb@google.com>
>> Signed-off-by: Alexander Lobakin <alobakin@pm.me>
>> ---
>>  drivers/net/bonding/bond_main.c | 11 +++++------
>>  drivers/net/dummy.c             |  2 +-
>>  drivers/net/ifb.c               |  3 +--
>>  drivers/net/team/team.c         |  9 ++++-----
>>  4 files changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 84ecbc6fa0ff..71c9677d135f 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -1228,14 +1228,14 @@ static netdev_features_t bond_fix_features(struct net_device *dev,
>>  }
>>
>>  #define BOND_VLAN_FEATURES     (NETIF_F_HW_CSUM | NETIF_F_SG | \
>> -                                NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
>> +                                NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \
>>                                  NETIF_F_HIGHDMA | NETIF_F_LRO)
>>
>>  #define BOND_ENC_FEATURES      (NETIF_F_HW_CSUM | NETIF_F_SG | \
>> -                                NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
>> +                                NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE)
>>
>>  #define BOND_MPLS_FEATURES     (NETIF_F_HW_CSUM | NETIF_F_SG | \
>> -                                NETIF_F_ALL_TSO)
>> +                                NETIF_F_GSO_SOFTWARE)
>>
>>
>>  static void bond_compute_features(struct bonding *bond)
>> @@ -1291,8 +1291,7 @@ static void bond_compute_features(struct bonding *bond)
>>         bond_dev->vlan_features = vlan_features;
>>         bond_dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
>>                                     NETIF_F_HW_VLAN_CTAG_TX |
>> -                                   NETIF_F_HW_VLAN_STAG_TX |
>> -                                   NETIF_F_GSO_UDP_L4;
>> +                                   NETIF_F_HW_VLAN_STAG_TX;
>>  #ifdef CONFIG_XFRM_OFFLOAD
>>         bond_dev->hw_enc_features |= xfrm_features;
>>  #endif /* CONFIG_XFRM_OFFLOAD */
>> @@ -4721,7 +4720,7 @@ void bond_setup(struct net_device *bond_dev)
>>                                 NETIF_F_HW_VLAN_CTAG_RX |
>>                                 NETIF_F_HW_VLAN_CTAG_FILTER;
>>
>> -       bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4;
>> +       bond_dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
>>  #ifdef CONFIG_XFRM_OFFLOAD
>>         bond_dev->hw_features |= BOND_XFRM_FEATURES;
>>  #endif /* CONFIG_XFRM_OFFLOAD */
>> diff --git a/drivers/net/dummy.c b/drivers/net/dummy.c
>> index bab3a9bb5e6f..f82ad7419508 100644
>> --- a/drivers/net/dummy.c
>> +++ b/drivers/net/dummy.c
>> @@ -124,7 +124,7 @@ static void dummy_setup(struct net_device *dev)
>>         dev->flags &= ~IFF_MULTICAST;
>>         dev->priv_flags |= IFF_LIVE_ADDR_CHANGE | IFF_NO_QUEUE;
>>         dev->features   |= NETIF_F_SG | NETIF_F_FRAGLIST;
>> -       dev->features   |= NETIF_F_ALL_TSO;
>> +       dev->features   |= NETIF_F_GSO_SOFTWARE;
>>         dev->features   |= NETIF_F_HW_CSUM | NETIF_F_HIGHDMA | NETIF_F_LLTX;
>>         dev->features   |= NETIF_F_GSO_ENCAP_ALL;
>>         dev->hw_features |= dev->features;
>> diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
>> index 7fe306e76281..fa63d4dee0ba 100644
>> --- a/drivers/net/ifb.c
>> +++ b/drivers/net/ifb.c
>> @@ -187,8 +187,7 @@ static const struct net_device_ops ifb_netdev_ops = {
>>  };
>>
>>  #define IFB_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG  | NETIF_F_FRAGLIST | \
>> -                     NETIF_F_TSO_ECN | NETIF_F_TSO | NETIF_F_TSO6      | \
>> -                     NETIF_F_GSO_ENCAP_ALL                             | \
>> +                     NETIF_F_GSO_SOFTWARE | NETIF_F_GSO_ENCAP_ALL      | \
>>                       NETIF_F_HIGHDMA | NETIF_F_HW_VLAN_CTAG_TX         | \
>>                       NETIF_F_HW_VLAN_STAG_TX)
>>
>> diff --git a/drivers/net/team/team.c b/drivers/net/team/team.c
>> index 07f1f3933927..b4092127a92c 100644
>> --- a/drivers/net/team/team.c
>> +++ b/drivers/net/team/team.c
>> @@ -975,11 +975,11 @@ static void team_port_disable(struct team *team,
>>  }
>>
>>  #define TEAM_VLAN_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \
>> -                           NETIF_F_FRAGLIST | NETIF_F_ALL_TSO | \
>> +                           NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \
>>                             NETIF_F_HIGHDMA | NETIF_F_LRO)
>>
>>  #define TEAM_ENC_FEATURES      (NETIF_F_HW_CSUM | NETIF_F_SG | \
>> -                                NETIF_F_RXCSUM | NETIF_F_ALL_TSO)
>> +                                NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE)
>>
>>  static void __team_compute_features(struct team *team)
>>  {
>> @@ -1009,8 +1009,7 @@ static void __team_compute_features(struct team *team)
>>         team->dev->vlan_features = vlan_features;
>>         team->dev->hw_enc_features = enc_features | NETIF_F_GSO_ENCAP_ALL |
>>                                      NETIF_F_HW_VLAN_CTAG_TX |
>> -                                    NETIF_F_HW_VLAN_STAG_TX |
>> -                                    NETIF_F_GSO_UDP_L4;
>> +                                    NETIF_F_HW_VLAN_STAG_TX;
>>         team->dev->hard_header_len = max_hard_header_len;
>>
>>         team->dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
>> @@ -2175,7 +2174,7 @@ static void team_setup(struct net_device *dev)
>>                            NETIF_F_HW_VLAN_CTAG_RX |
>>                            NETIF_F_HW_VLAN_CTAG_FILTER;
>>
>> -       dev->hw_features |= NETIF_F_GSO_ENCAP_ALL | NETIF_F_GSO_UDP_L4;
>> +       dev->hw_features |= NETIF_F_GSO_ENCAP_ALL;
>>         dev->features |= dev->hw_features;
>>         dev->features |= NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;
>>  }
>> --
>> 2.29.2

Al


^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next 2/2] net: bonding, dummy, ifb, team: advertise NETIF_F_GSO_SOFTWARE
  2020-11-02 19:25   ` Alexander Lobakin
@ 2020-11-02 20:04     ` Willem de Bruijn
  0 siblings, 0 replies; 4+ messages in thread
From: Willem de Bruijn @ 2020-11-02 20:04 UTC (permalink / raw)
  To: Alexander Lobakin
  Cc: David S. Miller, Jakub Kicinski, Jay Vosburgh, Veaceslav Falico,
	Andy Gospodarek, Jiri Pirko, Steffen Klassert, Miaohe Lin,
	Antoine Tenart, Mauro Carvalho Chehab, Network Development,
	linux-kernel

On Mon, Nov 2, 2020 at 2:26 PM Alexander Lobakin <alobakin@pm.me> wrote:
>
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Date: Mon, 2 Nov 2020 11:30:17 -0500
>
> Hi!
> Thanks for the Ack.
>
> > On Sun, Nov 1, 2020 at 8:17 AM Alexander Lobakin <alobakin@pm.me> wrote:
> >>
> >> Virtual netdevs should use NETIF_F_GSO_SOFTWARE to forward GSO skbs
> >> as-is and let the final drivers deal with them when supported.
> >> Also remove NETIF_F_GSO_UDP_L4 from bonding and team drivers as it's
> >> now included in the "software" list.
> >
> > The rationale is that it is okay to advertise these features with
> > software fallback as bonding/teaming "hardware" features, because
> > there will always be a downstream device for which they will be
> > implemented, possibly in the software fallback, correct?
> >
> > That does not apply to dummy or IFB. I guess dummy is fine, because
> > xmit is a black hole, and IFB because ingress can safely handle these
> > packets? How did you arrive at the choice of changing these two, of
> > all virtual devices?
>
> Two points:
> 1. Exactly, dummy is just dummy, while ifb is an intermediate netdev to
>    share resources, so it should be as fine as with other virtual devs.
> 2. They both advertise NETIF_F_ALL_TSO | NETIF_F_GSO_ENCAP_ALL, which
>    assumes that they handle all GSO skbs just like the others (pass
>    them as is to the real drivers in case with ifb).

There is no real driver in the case of ifb if it forwards to the
ingress path. But as discussed before, that can handle gso packets for
all these protocols, too.

> >>
> >> Suggested-by: Willem de Bruijn <willemb@google.com>
> >> Signed-off-by: Alexander Lobakin <alobakin@pm.me>

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

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-11-02 20:06 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-01 13:17 [PATCH v2 net-next 2/2] net: bonding, dummy, ifb, team: advertise NETIF_F_GSO_SOFTWARE Alexander Lobakin
2020-11-02 16:30 ` Willem de Bruijn
2020-11-02 19:25   ` Alexander Lobakin
2020-11-02 20:04     ` Willem de Bruijn

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).