| Message ID | 20201208120802.1268708-2-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: 0 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, 78 lines checked |
| netdev/build_allmodconfig_warn | fail | Errors and warnings before: 0 this patch: 4 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On 08/12/2020 14:07:47+0200, Vladimir Oltean wrote: > We should not be unconditionally enabling address learning, since doing > that is actively detrimential when a port is standalone and not offloading > a bridge. Namely, if a port in the switch is standalone and others are > offloading the bridge, then we could enter a situation where we learn an > address towards the standalone port, but the bridged ports could not > forward the packet there, because the CPU is the only path between the > standalone and the bridged ports. The solution of course is to not > enable address learning unless the bridge asks for it. Currently this is > the only bridge port flag we are looking at. The others (flooding etc) > are TBD. > > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com> > --- > drivers/net/ethernet/mscc/ocelot.c | 21 ++++++++++++++++++++- > drivers/net/ethernet/mscc/ocelot.h | 3 +++ > drivers/net/ethernet/mscc/ocelot_net.c | 4 ++++ > include/soc/mscc/ocelot.h | 2 ++ > 4 files changed, 29 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c > index b9626eec8db6..7a5c534099d3 100644 > --- a/drivers/net/ethernet/mscc/ocelot.c > +++ b/drivers/net/ethernet/mscc/ocelot.c > @@ -883,6 +883,7 @@ EXPORT_SYMBOL(ocelot_get_ts_info); > > void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state) > { > + struct ocelot_port *ocelot_port = ocelot->ports[port]; > u32 port_cfg; > int p, i; > > @@ -896,7 +897,8 @@ void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state) > ocelot->bridge_fwd_mask |= BIT(port); > fallthrough; > case BR_STATE_LEARNING: > - port_cfg |= ANA_PORT_PORT_CFG_LEARN_ENA; > + if (ocelot_port->brport_flags & BR_LEARNING) > + port_cfg |= ANA_PORT_PORT_CFG_LEARN_ENA; > break; > > default: > @@ -1178,6 +1180,7 @@ EXPORT_SYMBOL(ocelot_port_bridge_join); > int ocelot_port_bridge_leave(struct ocelot *ocelot, int port, > struct net_device *bridge) > { > + struct ocelot_port *ocelot_port = ocelot->ports[port]; > struct ocelot_vlan pvid = {0}, native_vlan = {0}; > struct switchdev_trans trans; > int ret; > @@ -1200,6 +1203,10 @@ int ocelot_port_bridge_leave(struct ocelot *ocelot, int port, > ocelot_port_set_pvid(ocelot, port, pvid); > ocelot_port_set_native_vlan(ocelot, port, native_vlan); > > + ocelot_port->brport_flags = 0; > + ocelot_rmw_gix(ocelot, 0, ANA_PORT_PORT_CFG_LEARN_ENA, > + ANA_PORT_PORT_CFG, port); > + > return 0; > } > EXPORT_SYMBOL(ocelot_port_bridge_leave); > @@ -1391,6 +1398,18 @@ int ocelot_get_max_mtu(struct ocelot *ocelot, int port) > } > EXPORT_SYMBOL(ocelot_get_max_mtu); > > +void ocelot_port_bridge_flags(struct ocelot *ocelot, int port, > + unsigned long flags, > + struct switchdev_trans *trans) > +{ > + struct ocelot_port *ocelot_port = ocelot->ports[port]; > + > + if (switchdev_trans_ph_prepare(trans)) > + return; > + > + ocelot_port->brport_flags = flags; > +} > + > void ocelot_init_port(struct ocelot *ocelot, int port) > { > struct ocelot_port *ocelot_port = ocelot->ports[port]; > diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h > index 291d39d49c4e..739bd201d951 100644 > --- a/drivers/net/ethernet/mscc/ocelot.h > +++ b/drivers/net/ethernet/mscc/ocelot.h > @@ -102,6 +102,9 @@ struct ocelot_multicast { > struct ocelot_pgid *pgid; > }; > > +void ocelot_port_bridge_flags(struct ocelot *ocelot, int port, > + unsigned long flags, > + struct switchdev_trans *trans); > int ocelot_port_fdb_do_dump(const unsigned char *addr, u16 vid, > bool is_static, void *data); > int ocelot_mact_learn(struct ocelot *ocelot, int port, > diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c > index 9ba7e2b166e9..93ecd5274156 100644 > --- a/drivers/net/ethernet/mscc/ocelot_net.c > +++ b/drivers/net/ethernet/mscc/ocelot_net.c > @@ -882,6 +882,10 @@ static int ocelot_port_attr_set(struct net_device *dev, > case SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED: > ocelot_port_attr_mc_set(ocelot, port, !attr->u.mc_disabled); > break; > + case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS: > + ocelot_port_bridge_flags(ocelot, port, attr->u.brport_flags, > + trans); > + break; > default: > err = -EOPNOTSUPP; > break; > diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h > index 2f4cd3288bcc..50514c087231 100644 > --- a/include/soc/mscc/ocelot.h > +++ b/include/soc/mscc/ocelot.h > @@ -581,6 +581,8 @@ struct ocelot_port { > > struct regmap *target; > > + unsigned long brport_flags; > + > bool vlan_aware; > /* VLAN that untagged frames are classified to, on ingress */ > struct ocelot_vlan pvid_vlan; > -- > 2.25.1 >
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c index b9626eec8db6..7a5c534099d3 100644 --- a/drivers/net/ethernet/mscc/ocelot.c +++ b/drivers/net/ethernet/mscc/ocelot.c @@ -883,6 +883,7 @@ EXPORT_SYMBOL(ocelot_get_ts_info); void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state) { + struct ocelot_port *ocelot_port = ocelot->ports[port]; u32 port_cfg; int p, i; @@ -896,7 +897,8 @@ void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state) ocelot->bridge_fwd_mask |= BIT(port); fallthrough; case BR_STATE_LEARNING: - port_cfg |= ANA_PORT_PORT_CFG_LEARN_ENA; + if (ocelot_port->brport_flags & BR_LEARNING) + port_cfg |= ANA_PORT_PORT_CFG_LEARN_ENA; break; default: @@ -1178,6 +1180,7 @@ EXPORT_SYMBOL(ocelot_port_bridge_join); int ocelot_port_bridge_leave(struct ocelot *ocelot, int port, struct net_device *bridge) { + struct ocelot_port *ocelot_port = ocelot->ports[port]; struct ocelot_vlan pvid = {0}, native_vlan = {0}; struct switchdev_trans trans; int ret; @@ -1200,6 +1203,10 @@ int ocelot_port_bridge_leave(struct ocelot *ocelot, int port, ocelot_port_set_pvid(ocelot, port, pvid); ocelot_port_set_native_vlan(ocelot, port, native_vlan); + ocelot_port->brport_flags = 0; + ocelot_rmw_gix(ocelot, 0, ANA_PORT_PORT_CFG_LEARN_ENA, + ANA_PORT_PORT_CFG, port); + return 0; } EXPORT_SYMBOL(ocelot_port_bridge_leave); @@ -1391,6 +1398,18 @@ int ocelot_get_max_mtu(struct ocelot *ocelot, int port) } EXPORT_SYMBOL(ocelot_get_max_mtu); +void ocelot_port_bridge_flags(struct ocelot *ocelot, int port, + unsigned long flags, + struct switchdev_trans *trans) +{ + struct ocelot_port *ocelot_port = ocelot->ports[port]; + + if (switchdev_trans_ph_prepare(trans)) + return; + + ocelot_port->brport_flags = flags; +} + void ocelot_init_port(struct ocelot *ocelot, int port) { struct ocelot_port *ocelot_port = ocelot->ports[port]; diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h index 291d39d49c4e..739bd201d951 100644 --- a/drivers/net/ethernet/mscc/ocelot.h +++ b/drivers/net/ethernet/mscc/ocelot.h @@ -102,6 +102,9 @@ struct ocelot_multicast { struct ocelot_pgid *pgid; }; +void ocelot_port_bridge_flags(struct ocelot *ocelot, int port, + unsigned long flags, + struct switchdev_trans *trans); int ocelot_port_fdb_do_dump(const unsigned char *addr, u16 vid, bool is_static, void *data); int ocelot_mact_learn(struct ocelot *ocelot, int port, diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c index 9ba7e2b166e9..93ecd5274156 100644 --- a/drivers/net/ethernet/mscc/ocelot_net.c +++ b/drivers/net/ethernet/mscc/ocelot_net.c @@ -882,6 +882,10 @@ static int ocelot_port_attr_set(struct net_device *dev, case SWITCHDEV_ATTR_ID_BRIDGE_MC_DISABLED: ocelot_port_attr_mc_set(ocelot, port, !attr->u.mc_disabled); break; + case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS: + ocelot_port_bridge_flags(ocelot, port, attr->u.brport_flags, + trans); + break; default: err = -EOPNOTSUPP; break; diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h index 2f4cd3288bcc..50514c087231 100644 --- a/include/soc/mscc/ocelot.h +++ b/include/soc/mscc/ocelot.h @@ -581,6 +581,8 @@ struct ocelot_port { struct regmap *target; + unsigned long brport_flags; + bool vlan_aware; /* VLAN that untagged frames are classified to, on ingress */ struct ocelot_vlan pvid_vlan;
We should not be unconditionally enabling address learning, since doing that is actively detrimential when a port is standalone and not offloading a bridge. Namely, if a port in the switch is standalone and others are offloading the bridge, then we could enter a situation where we learn an address towards the standalone port, but the bridged ports could not forward the packet there, because the CPU is the only path between the standalone and the bridged ports. The solution of course is to not enable address learning unless the bridge asks for it. Currently this is the only bridge port flag we are looking at. The others (flooding etc) are TBD. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> --- drivers/net/ethernet/mscc/ocelot.c | 21 ++++++++++++++++++++- drivers/net/ethernet/mscc/ocelot.h | 3 +++ drivers/net/ethernet/mscc/ocelot_net.c | 4 ++++ include/soc/mscc/ocelot.h | 2 ++ 4 files changed, 29 insertions(+), 1 deletion(-)