Messages in this thread | | | From | Vladimir Oltean <> | Subject | Re: [PATCH net-next v3 5/7] net: lan966x: Add lag support for lan966x. | Date | Sat, 2 Jul 2022 14:12:20 +0000 |
| |
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; > +}
| |