Re: [PATCH net-next v3 5/7] net: lan966x: Add lag support for lan966x.
From: Vladimir Oltean <vladimir.oltean@nxp.com>
Date: 2022-07-02 14:12:32
Also in:
lkml
On Fri, Jul 01, 2022 at 10:52:25PM +0200, Horatiu Vultur wrote:
Add link aggregation hardware offload support for lan966x Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com> ---
Superficially the code looks OK (maybe if you could only remove the trailing "." from the commit message). A comment below.
+int lan966x_lag_port_prechangeupper(struct net_device *dev,
+ struct netdev_notifier_changeupper_info *info)
+{
+ struct lan966x_port *port = netdev_priv(dev);
+ struct lan966x *lan966x = port->lan966x;
+ struct netdev_lag_upper_info *lui;
+ struct netlink_ext_ack *extack;
+
+ extack = netdev_notifier_info_to_extack(&info->info);
+ lui = info->upper_info;
+ if (!lui)
+ return NOTIFY_DONE;
+
+ if (lui->tx_type != NETDEV_LAG_TX_TYPE_HASH) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "LAG device using unsupported Tx type");
+ return notifier_from_errno(-EINVAL);
+ }
+
+ switch (lui->hash_type) {
+ case NETDEV_LAG_HASH_L2:
+ lan_wr(ANA_AGGR_CFG_AC_DMAC_ENA_SET(1) |
+ ANA_AGGR_CFG_AC_SMAC_ENA_SET(1),
+ lan966x, ANA_AGGR_CFG);
+ break;
+ case NETDEV_LAG_HASH_L34:
+ lan_wr(ANA_AGGR_CFG_AC_IP6_TCPUDP_ENA_SET(1) |
+ ANA_AGGR_CFG_AC_IP4_TCPUDP_ENA_SET(1) |
+ ANA_AGGR_CFG_AC_IP4_SIPDIP_ENA_SET(1),
+ lan966x, ANA_AGGR_CFG);
+ break;
+ case NETDEV_LAG_HASH_L23:
+ lan_wr(ANA_AGGR_CFG_AC_DMAC_ENA_SET(1) |
+ ANA_AGGR_CFG_AC_SMAC_ENA_SET(1) |
+ ANA_AGGR_CFG_AC_IP6_TCPUDP_ENA_SET(1) |
+ ANA_AGGR_CFG_AC_IP4_TCPUDP_ENA_SET(1),
+ lan966x, ANA_AGGR_CFG);ANA_AGGR_CFG is global to the switch, yet it constantly changes with the hash_type of each new offloaded LAG. So a LAG that hashes on L3/L4 will break the hash mode of an existing L2 LAG. I think you should implement a system that disallows changes to ANA_AGGR_CFG as long as there are users of a certain mode.
+ break; + default: + NL_SET_ERR_MSG_MOD(extack, + "LAG device using unsupported hash type"); + return notifier_from_errno(-EINVAL); + } + + return NOTIFY_OK; +}