Re: [PATCH net-next 3/5] team: use common function to compute the features

On 08/29, Hangbin Liu wrote:

Use the new helper netdev_compute_features_from_lowers() to compute the team device features. This helper performs both the feature computation and the netdev_change_features() call.

Note that such change replace the lower layer traversing currently done using team->port_list with netdev_for_each_lower_dev(). Such change is safe as `port_list` contains exactly the same elements as `team->dev->adj_list.lower` and the helper is always invoked under the RTNL lock.

With this change, the explicit netdev_change_features() in team_add_slave() can be safely removed, as team_port_add() already takes care of the notification via netdev_compute_features_from_lowers(), and same thing for team_del_slave()

This also fixes missing computations for MPLS, XFRM, and TSO/GSO partial features.

Signed-off-by: Hangbin Liu liuhangbin@gmail.com

drivers/net/team/team_core.c | 73 ++---------------------------------- 1 file changed, 4 insertions(+), 69 deletions(-)

diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c index 17f07eb0ee52..018d876e140a 100644 --- a/drivers/net/team/team_core.c +++ b/drivers/net/team/team_core.c @@ -982,63 +982,6 @@ static void team_port_disable(struct team *team, team_lower_state_changed(port); } -#define TEAM_VLAN_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \

• 	    NETIF_F_FRAGLIST | NETIF_F_GSO_SOFTWARE | \
  

• 	    NETIF_F_HIGHDMA | NETIF_F_LRO | \
  

• 	    NETIF_F_GSO_ENCAP_ALL)
  

-#define TEAM_ENC_FEATURES (NETIF_F_HW_CSUM | NETIF_F_SG | \

• 		 NETIF_F_RXCSUM | NETIF_F_GSO_SOFTWARE)
  

-static void __team_compute_features(struct team *team) -{

• struct team_port *port;
• netdev_features_t vlan_features = TEAM_VLAN_FEATURES;
• netdev_features_t enc_features = TEAM_ENC_FEATURES;
• unsigned short max_hard_header_len = ETH_HLEN;
• unsigned int dst_release_flag = IFF_XMIT_DST_RELEASE |

• 			IFF_XMIT_DST_RELEASE_PERM;
  

• rcu_read_lock();
• if (list_empty(&team->port_list))

• goto done;
  

• vlan_features = netdev_base_features(vlan_features);
• enc_features = netdev_base_features(enc_features);
• list_for_each_entry_rcu(port, &team->port_list, list) {

• vlan_features = netdev_increment_features(vlan_features,
  

• 			port->dev->vlan_features,
  

• 			TEAM_VLAN_FEATURES);
  

• enc_features =
  

• 	netdev_increment_features(enc_features,
  

• 				  port->dev->hw_enc_features,
  

• 				  TEAM_ENC_FEATURES);
  

• dst_release_flag &= port->dev->priv_flags;
  

• if (port->dev->hard_header_len > max_hard_header_len)
  

• 	max_hard_header_len = port->dev->hard_header_len;
  

• }

-done:

• rcu_read_unlock();
• 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;
  

• team->dev->hard_header_len = max_hard_header_len;
• team->dev->priv_flags &= ~IFF_XMIT_DST_RELEASE;
• if (dst_release_flag == (IFF_XMIT_DST_RELEASE | IFF_XMIT_DST_RELEASE_PERM))

• team->dev->priv_flags |= IFF_XMIT_DST_RELEASE;
  

-}

-static void team_compute_features(struct team *team) -{

• __team_compute_features(team);
• netdev_change_features(team->dev);

-}

static int team_port_enter(struct team *team, struct team_port *port) { int err = 0; @@ -1300,7 +1243,7 @@ static int team_port_add(struct team *team, struct net_device *port_dev, port->index = -1; list_add_tail_rcu(&port->list, &team->port_list); team_port_enable(team, port);

• __team_compute_features(team);

• netdev_compute_features_from_lowers(team->dev); __team_port_change_port_added(port, !!netif_oper_up(port_dev)); __team_options_change_check(team);

@@ -1382,7 +1325,7 @@ static int team_port_del(struct team *team, struct net_device *port_dev) dev_set_mtu(port_dev, port->orig.mtu); kfree_rcu(port, rcu); netdev_info(dev, "Port device %s removed\n", portname);

• __team_compute_features(team);

• netdev_compute_features_from_lowers(team->dev);

return 0; }

[..]

@@ -1976,9 +1919,6 @@ static int team_add_slave(struct net_device *dev, struct net_device *port_dev, err = team_port_add(team, port_dev, extack);

• if (!err)

• netdev_change_features(dev);
  

• return err;

} @@ -1991,11 +1931,6 @@ static int team_del_slave(struct net_device *dev, struct net_device *port_dev) err = team_port_del(team, port_dev);

• if (err)

• return err;
  

• netdev_change_features(dev);
• return err;

}

nit: change these to 'return team_port_xxx()' ? Can drop the 'err' as well, don't think it's needed anymore?