Skip to content

Commit

Permalink
net: dsa: act as passthrough for bridge port flags
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
vladimiroltean authored and davem330 committed Feb 13, 2021
1 parent e18f4c1 commit a8b659e
Show file tree
Hide file tree
Showing 10 changed files with 263 additions and 127 deletions.
91 changes: 64 additions & 27 deletions drivers/net/dsa/b53/b53_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -510,6 +510,39 @@ void b53_imp_vlan_setup(struct dsa_switch *ds, int cpu_port)
}
EXPORT_SYMBOL(b53_imp_vlan_setup);

static void b53_port_set_ucast_flood(struct b53_device *dev, int port,
bool unicast)
{
u16 uc;

b53_read16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, &uc);
if (unicast)
uc |= BIT(port);
else
uc &= ~BIT(port);
b53_write16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, uc);
}

static void b53_port_set_mcast_flood(struct b53_device *dev, int port,
bool multicast)
{
u16 mc;

b53_read16(dev, B53_CTRL_PAGE, B53_MC_FLOOD_MASK, &mc);
if (multicast)
mc |= BIT(port);
else
mc &= ~BIT(port);
b53_write16(dev, B53_CTRL_PAGE, B53_MC_FLOOD_MASK, mc);

b53_read16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, &mc);
if (multicast)
mc |= BIT(port);
else
mc &= ~BIT(port);
b53_write16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, mc);
}

int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
{
struct b53_device *dev = ds->priv;
Expand All @@ -522,7 +555,8 @@ int b53_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)

cpu_port = dsa_to_port(ds, port)->cpu_dp->index;

b53_br_egress_floods(ds, port, true, true);
b53_port_set_ucast_flood(dev, port, true);
b53_port_set_mcast_flood(dev, port, true);

if (dev->ops->irq_enable)
ret = dev->ops->irq_enable(dev, port);
Expand Down Expand Up @@ -655,7 +689,8 @@ static void b53_enable_cpu_port(struct b53_device *dev, int port)

b53_brcm_hdr_setup(dev->ds, port);

b53_br_egress_floods(dev->ds, port, true, true);
b53_port_set_ucast_flood(dev, port, true);
b53_port_set_mcast_flood(dev, port, true);
}

static void b53_enable_mib(struct b53_device *dev)
Expand Down Expand Up @@ -1916,37 +1951,37 @@ void b53_br_fast_age(struct dsa_switch *ds, int port)
}
EXPORT_SYMBOL(b53_br_fast_age);

int b53_br_egress_floods(struct dsa_switch *ds, int port,
bool unicast, bool multicast)
static int b53_br_flags_pre(struct dsa_switch *ds, int port,
struct switchdev_brport_flags flags,
struct netlink_ext_ack *extack)
{
struct b53_device *dev = ds->priv;
u16 uc, mc;

b53_read16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, &uc);
if (unicast)
uc |= BIT(port);
else
uc &= ~BIT(port);
b53_write16(dev, B53_CTRL_PAGE, B53_UC_FLOOD_MASK, uc);
if (flags.mask & ~(BR_FLOOD | BR_MCAST_FLOOD))
return -EINVAL;

b53_read16(dev, B53_CTRL_PAGE, B53_MC_FLOOD_MASK, &mc);
if (multicast)
mc |= BIT(port);
else
mc &= ~BIT(port);
b53_write16(dev, B53_CTRL_PAGE, B53_MC_FLOOD_MASK, mc);
return 0;
}

b53_read16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, &mc);
if (multicast)
mc |= BIT(port);
else
mc &= ~BIT(port);
b53_write16(dev, B53_CTRL_PAGE, B53_IPMC_FLOOD_MASK, mc);
static int b53_br_flags(struct dsa_switch *ds, int port,
struct switchdev_brport_flags flags,
struct netlink_ext_ack *extack)
{
if (flags.mask & BR_FLOOD)
b53_port_set_ucast_flood(ds->priv, port,
!!(flags.val & BR_FLOOD));
if (flags.mask & BR_MCAST_FLOOD)
b53_port_set_mcast_flood(ds->priv, port,
!!(flags.val & BR_MCAST_FLOOD));

return 0;
}

static int b53_set_mrouter(struct dsa_switch *ds, int port, bool mrouter,
struct netlink_ext_ack *extack)
{
b53_port_set_mcast_flood(ds->priv, port, mrouter);

return 0;
}
EXPORT_SYMBOL(b53_br_egress_floods);

static bool b53_possible_cpu_port(struct dsa_switch *ds, int port)
{
Expand Down Expand Up @@ -2187,9 +2222,11 @@ static const struct dsa_switch_ops b53_switch_ops = {
.set_mac_eee = b53_set_mac_eee,
.port_bridge_join = b53_br_join,
.port_bridge_leave = b53_br_leave,
.port_pre_bridge_flags = b53_br_flags_pre,
.port_bridge_flags = b53_br_flags,
.port_set_mrouter = b53_set_mrouter,
.port_stp_state_set = b53_br_set_stp_state,
.port_fast_age = b53_br_fast_age,
.port_egress_floods = b53_br_egress_floods,
.port_vlan_filtering = b53_vlan_filtering,
.port_vlan_add = b53_vlan_add,
.port_vlan_del = b53_vlan_del,
Expand Down
2 changes: 0 additions & 2 deletions drivers/net/dsa/b53/b53_priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,6 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *bridge);
void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *bridge);
void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
void b53_br_fast_age(struct dsa_switch *ds, int port);
int b53_br_egress_floods(struct dsa_switch *ds, int port,
bool unicast, bool multicast);
int b53_setup_devlink_resources(struct dsa_switch *ds);
void b53_port_event(struct dsa_switch *ds, int port);
void b53_phylink_validate(struct dsa_switch *ds, int port,
Expand Down

0 comments on commit a8b659e

Please sign in to comment.