summaryrefslogtreecommitdiff
path: root/net
diff options
context:
space:
mode:
authorVladimir Oltean <vladimir.oltean@nxp.com>2021-02-12 17:15:56 +0200
committerDavid S. Miller <davem@davemloft.net>2021-02-12 17:08:04 -0800
commita8b659e7ff75a6e766bc5691df57ceb26018db9f (patch)
tree758c90dc40e3e8dfb44b6e5dd029a080863995fc /net
parente18f4c18ab5b0dd47caaf8377c2e36d66f632a8c (diff)
net: dsa: act as passthrough for bridge port flags
There are multiple ways in which a PORT_BRIDGE_FLAGS attribute can be expressed by the bridge through switchdev, and not all of them can be emulated by DSA mid-layer API at the same time. One possible configuration is when the bridge offloads the port flags using a mask that has a single bit set - therefore only one feature should change. However, DSA currently groups together unicast and multicast flooding in the .port_egress_floods method, which limits our options when we try to add support for turning off broadcast flooding: do we extend .port_egress_floods with a third parameter which b53 and mv88e6xxx will ignore? But that means that the DSA layer, which currently implements the PRE_BRIDGE_FLAGS attribute all by itself, will see that .port_egress_floods is implemented, and will report that all 3 types of flooding are supported - not necessarily true. Another configuration is when the user specifies more than one flag at the same time, in the same netlink message. If we were to create one individual function per offloadable bridge port flag, we would limit the expressiveness of the switch driver of refusing certain combinations of flag values. For example, a switch may not have an explicit knob for flooding of unknown multicast, just for flooding in general. In that case, the only correct thing to do is to allow changes to BR_FLOOD and BR_MCAST_FLOOD in tandem, and never allow mismatched values. But having a separate .port_set_unicast_flood and .port_set_multicast_flood would not allow the driver to possibly reject that. Also, DSA doesn't consider it necessary to inform the driver that a SWITCHDEV_ATTR_ID_BRIDGE_MROUTER attribute was offloaded, because it just calls .port_egress_floods for the CPU port. When we'll add support for the plain SWITCHDEV_ATTR_ID_PORT_MROUTER, that will become a real problem because the flood settings will need to be held statefully in the DSA middle layer, otherwise changing the mrouter port attribute will impact the flooding attribute. And that's _assuming_ that the underlying hardware doesn't have anything else to do when a multicast router attaches to a port than flood unknown traffic to it. If it does, there will need to be a dedicated .port_set_mrouter anyway. So we need to let the DSA drivers see the exact form that the bridge passes this switchdev attribute in, otherwise we are standing in the way. Therefore we also need to use this form of language when communicating to the driver that it needs to configure its initial (before bridge join) and final (after bridge leave) port flags. The b53 and mv88e6xxx drivers are converted to the passthrough API and their implementation of .port_egress_floods is split into two: a function that configures unicast flooding and another for multicast. The mv88e6xxx implementation is quite hairy, and it turns out that the implementations of unknown unicast flooding are actually the same for 6185 and for 6352: behind the confusing names actually lie two individual bits: NO_UNKNOWN_MC -> FLOOD_UC = 0x4 = BIT(2) NO_UNKNOWN_UC -> FLOOD_MC = 0x8 = BIT(3) so there was no reason to entangle them in the first place. Whereas the 6185 writes to MV88E6185_PORT_CTL0_FORWARD_UNKNOWN of PORT_CTL0, which has the exact same bit index. I have left the implementations separate though, for the only reason that the names are different enough to confuse me, since I am not able to double-check with a user manual. The multicast flooding setting for 6185 is in a different register than for 6352 though. Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r--net/dsa/dsa_priv.h9
-rw-r--r--net/dsa/port.c31
-rw-r--r--net/dsa/slave.c7
3 files changed, 24 insertions, 23 deletions
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index bc835f3de2be..f5949b39f6f7 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -184,10 +184,13 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
int dsa_port_mdb_del(const struct dsa_port *dp,
const struct switchdev_obj_port_mdb *mdb);
int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
- struct switchdev_brport_flags flags);
+ struct switchdev_brport_flags flags,
+ struct netlink_ext_ack *extack);
int dsa_port_bridge_flags(const struct dsa_port *dp,
- struct switchdev_brport_flags flags);
-int dsa_port_mrouter(struct dsa_port *dp, bool mrouter);
+ struct switchdev_brport_flags flags,
+ struct netlink_ext_ack *extack);
+int dsa_port_mrouter(struct dsa_port *dp, bool mrouter,
+ struct netlink_ext_ack *extack);
int dsa_port_vlan_add(struct dsa_port *dp,
const struct switchdev_obj_port_vlan *vlan);
int dsa_port_vlan_del(struct dsa_port *dp,
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 368064dfd93e..80e6471a7a5c 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -140,7 +140,7 @@ static void dsa_port_change_brport_flags(struct dsa_port *dp,
tmp.val = flags.val & BIT(flag);
tmp.mask = BIT(flag);
- dsa_port_bridge_flags(dp, tmp);
+ dsa_port_bridge_flags(dp, tmp, NULL);
}
}
@@ -425,41 +425,38 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock)
}
int dsa_port_pre_bridge_flags(const struct dsa_port *dp,
- struct switchdev_brport_flags flags)
+ struct switchdev_brport_flags flags,
+ struct netlink_ext_ack *extack)
{
struct dsa_switch *ds = dp->ds;
- if (!ds->ops->port_egress_floods ||
- (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD)))
+ if (!ds->ops->port_pre_bridge_flags)
return -EINVAL;
- return 0;
+ return ds->ops->port_pre_bridge_flags(ds, dp->index, flags, extack);
}
int dsa_port_bridge_flags(const struct dsa_port *dp,
- struct switchdev_brport_flags flags)
+ struct switchdev_brport_flags flags,
+ struct netlink_ext_ack *extack)
{
struct dsa_switch *ds = dp->ds;
- int port = dp->index;
- int err = 0;
- if (ds->ops->port_egress_floods)
- err = ds->ops->port_egress_floods(ds, port,
- flags.val & BR_FLOOD,
- flags.val & BR_MCAST_FLOOD);
+ if (!ds->ops->port_bridge_flags)
+ return -EINVAL;
- return err;
+ return ds->ops->port_bridge_flags(ds, dp->index, flags, extack);
}
-int dsa_port_mrouter(struct dsa_port *dp, bool mrouter)
+int dsa_port_mrouter(struct dsa_port *dp, bool mrouter,
+ struct netlink_ext_ack *extack)
{
struct dsa_switch *ds = dp->ds;
- int port = dp->index;
- if (!ds->ops->port_egress_floods)
+ if (!ds->ops->port_set_mrouter)
return -EOPNOTSUPP;
- return ds->ops->port_egress_floods(ds, port, true, mrouter);
+ return ds->ops->port_set_mrouter(ds, dp->index, mrouter, extack);
}
int dsa_port_mtu_change(struct dsa_port *dp, int new_mtu,
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index be29008477d3..8c9a41a7209a 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -292,13 +292,14 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
ret = dsa_port_ageing_time(dp, attr->u.ageing_time);
break;
case SWITCHDEV_ATTR_ID_PORT_PRE_BRIDGE_FLAGS:
- ret = dsa_port_pre_bridge_flags(dp, attr->u.brport_flags);
+ ret = dsa_port_pre_bridge_flags(dp, attr->u.brport_flags,
+ extack);
break;
case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
- ret = dsa_port_bridge_flags(dp, attr->u.brport_flags);
+ ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, extack);
break;
case SWITCHDEV_ATTR_ID_BRIDGE_MROUTER:
- ret = dsa_port_mrouter(dp->cpu_dp, attr->u.mrouter);
+ ret = dsa_port_mrouter(dp->cpu_dp, attr->u.mrouter, extack);
break;
default:
ret = -EOPNOTSUPP;