| Message ID | 20201208120802.1268708-12-vladimir.oltean@nxp.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | LAG offload for Ocelot DSA switches | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | fail | Series longer than 15 patches |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | fail | Errors and warnings before: 4 this patch: 4 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 77 lines checked |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 4 this patch: 4 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 08/12/2020 14:07:57+0200, Vladimir Oltean wrote: > The setup of logical port IDs is done in two places: from the inconclusively > named ocelot_setup_lag and from ocelot_port_lag_leave, a function that > also calls ocelot_setup_lag (which apparently does an incomplete setup > of the LAG). > > To improve this situation, we can rename ocelot_setup_lag into > ocelot_setup_logical_port_ids, and drop the "lag" argument. It will now > set up the logical port IDs of all switch ports, which may be just > slightly more inefficient but more maintainable. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > drivers/net/ethernet/mscc/ocelot.c | 47 ++++++++++++++++++------------ > 1 file changed, 28 insertions(+), 19 deletions(-) > > diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c > index ee0fcee8e09a..1a98c24af056 100644 > --- a/drivers/net/ethernet/mscc/ocelot.c > +++ b/drivers/net/ethernet/mscc/ocelot.c > @@ -1279,20 +1279,36 @@ static void ocelot_set_aggr_pgids(struct ocelot *ocelot) > } > } > > -static void ocelot_setup_lag(struct ocelot *ocelot, int lag) > +/* When offloading a bonding interface, the switch ports configured under the > + * same bond must have the same logical port ID, equal to the physical port ID > + * of the lowest numbered physical port in that bond. Otherwise, in standalone/ > + * bridged mode, each port has a logical port ID equal to its physical port ID. > + */ > +static void ocelot_setup_logical_port_ids(struct ocelot *ocelot) > { > - unsigned long bond_mask = ocelot->lags[lag]; > - unsigned int p; > + int port; > > - for_each_set_bit(p, &bond_mask, ocelot->num_phys_ports) { > - u32 port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG, p); > + for (port = 0; port < ocelot->num_phys_ports; port++) { > + struct ocelot_port *ocelot_port = ocelot->ports[port]; > + struct net_device *bond; > + > + if (!ocelot_port) > + continue; > > - port_cfg &= ~ANA_PORT_PORT_CFG_PORTID_VAL_M; > + bond = ocelot_port->bond; > + if (bond) { > + int lag = __ffs(ocelot_get_bond_mask(ocelot, bond)); > > - /* Use lag port as logical port for port i */ > - ocelot_write_gix(ocelot, port_cfg | > - ANA_PORT_PORT_CFG_PORTID_VAL(lag), > - ANA_PORT_PORT_CFG, p); > + ocelot_rmw_gix(ocelot, > + ANA_PORT_PORT_CFG_PORTID_VAL(lag), > + ANA_PORT_PORT_CFG_PORTID_VAL_M, > + ANA_PORT_PORT_CFG, port); > + } else { > + ocelot_rmw_gix(ocelot, > + ANA_PORT_PORT_CFG_PORTID_VAL(port), > + ANA_PORT_PORT_CFG_PORTID_VAL_M, > + ANA_PORT_PORT_CFG, port); > + } > } > } > > @@ -1320,7 +1336,7 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port, > ocelot->lags[lag] |= BIT(port); > } > > - ocelot_setup_lag(ocelot, lag); > + ocelot_setup_logical_port_ids(ocelot); > ocelot_apply_bridge_fwd_mask(ocelot); > ocelot_set_aggr_pgids(ocelot); > > @@ -1331,7 +1347,6 @@ EXPORT_SYMBOL(ocelot_port_lag_join); > void ocelot_port_lag_leave(struct ocelot *ocelot, int port, > struct net_device *bond) > { > - u32 port_cfg; > int i; > > ocelot->ports[port]->bond = NULL; > @@ -1348,15 +1363,9 @@ void ocelot_port_lag_leave(struct ocelot *ocelot, int port, > > ocelot->lags[n] = ocelot->lags[port]; > ocelot->lags[port] = 0; > - > - ocelot_setup_lag(ocelot, n); > } > > - port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG, port); > - port_cfg &= ~ANA_PORT_PORT_CFG_PORTID_VAL_M; > - ocelot_write_gix(ocelot, port_cfg | ANA_PORT_PORT_CFG_PORTID_VAL(port), > - ANA_PORT_PORT_CFG, port); > - > + ocelot_setup_logical_port_ids(ocelot); > ocelot_apply_bridge_fwd_mask(ocelot); > ocelot_set_aggr_pgids(ocelot); > } > -- > 2.25.1 >
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index ee0fcee8e09a..1a98c24af056 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -1279,20 +1279,36 @@ static void ocelot_set_aggr_pgids(struct ocelot *ocelot) } } -static void ocelot_setup_lag(struct ocelot *ocelot, int lag) +/* When offloading a bonding interface, the switch ports configured under the + * same bond must have the same logical port ID, equal to the physical port ID + * of the lowest numbered physical port in that bond. Otherwise, in standalone/ + * bridged mode, each port has a logical port ID equal to its physical port ID. + */ +static void ocelot_setup_logical_port_ids(struct ocelot *ocelot) { - unsigned long bond_mask = ocelot->lags[lag]; - unsigned int p; + int port; - for_each_set_bit(p, &bond_mask, ocelot->num_phys_ports) { - u32 port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG, p); + for (port = 0; port < ocelot->num_phys_ports; port++) { + struct ocelot_port *ocelot_port = ocelot->ports[port]; + struct net_device *bond; + + if (!ocelot_port) + continue; - port_cfg &= ~ANA_PORT_PORT_CFG_PORTID_VAL_M; + bond = ocelot_port->bond; + if (bond) { + int lag = __ffs(ocelot_get_bond_mask(ocelot, bond)); - /* Use lag port as logical port for port i */ - ocelot_write_gix(ocelot, port_cfg | - ANA_PORT_PORT_CFG_PORTID_VAL(lag), - ANA_PORT_PORT_CFG, p); + ocelot_rmw_gix(ocelot, + ANA_PORT_PORT_CFG_PORTID_VAL(lag), + ANA_PORT_PORT_CFG_PORTID_VAL_M, + ANA_PORT_PORT_CFG, port); + } else { + ocelot_rmw_gix(ocelot, + ANA_PORT_PORT_CFG_PORTID_VAL(port), + ANA_PORT_PORT_CFG_PORTID_VAL_M, + ANA_PORT_PORT_CFG, port); + } } } @@ -1320,7 +1336,7 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port, ocelot->lags[lag] |= BIT(port); } - ocelot_setup_lag(ocelot, lag); + ocelot_setup_logical_port_ids(ocelot); ocelot_apply_bridge_fwd_mask(ocelot); ocelot_set_aggr_pgids(ocelot); @@ -1331,7 +1347,6 @@ EXPORT_SYMBOL(ocelot_port_lag_join); void ocelot_port_lag_leave(struct ocelot *ocelot, int port, struct net_device *bond) { - u32 port_cfg; int i; ocelot->ports[port]->bond = NULL; @@ -1348,15 +1363,9 @@ void ocelot_port_lag_leave(struct ocelot *ocelot, int port, ocelot->lags[n] = ocelot->lags[port]; ocelot->lags[port] = 0; - - ocelot_setup_lag(ocelot, n); } - port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG, port); - port_cfg &= ~ANA_PORT_PORT_CFG_PORTID_VAL_M; - ocelot_write_gix(ocelot, port_cfg | ANA_PORT_PORT_CFG_PORTID_VAL(port), - ANA_PORT_PORT_CFG, port); - + ocelot_setup_logical_port_ids(ocelot); ocelot_apply_bridge_fwd_mask(ocelot); ocelot_set_aggr_pgids(ocelot); }
The setup of logical port IDs is done in two places: from the inconclusively named ocelot_setup_lag and from ocelot_port_lag_leave, a function that also calls ocelot_setup_lag (which apparently does an incomplete setup of the LAG). To improve this situation, we can rename ocelot_setup_lag into ocelot_setup_logical_port_ids, and drop the "lag" argument. It will now set up the logical port IDs of all switch ports, which may be just slightly more inefficient but more maintainable. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/ethernet/mscc/ocelot.c | 47 ++++++++++++++++++------------ 1 file changed, 28 insertions(+), 19 deletions(-)