Skip to content

Commit

Permalink
net: dsa: microchip: implement multi-bridge support
Browse files Browse the repository at this point in the history 
Current driver version is able to handle only one bridge at time.
Configuring two bridges on two different ports would end up shorting this
bridges by HW. To reproduce it:

	ip l a name br0 type bridge
	ip l a name br1 type bridge
	ip l s dev br0 up
	ip l s dev br1 up
	ip l s lan1 master br0
	ip l s dev lan1 up
	ip l s lan2 master br1
	ip l s dev lan2 up

	Ping on lan1 and get response on lan2, which should not happen.

This happened, because current driver version is storing one global "Port VLAN
Membership" and applying it to all ports which are members of any
bridge.
To solve this issue, we need to handle each port separately.

This patch is dropping the global port member storage and calculating
membership dynamically depending on STP state and bridge participation.

Note: STP support was broken before this patch and should be fixed
separately.

Fixes: c2e8669 ("net: dsa: microchip: break KSZ9477 DSA driver into two files")
Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
Reviewed-by: Vladimir Oltean <olteanv@gmail.com>
Link: https://lore.kernel.org/r/20211126123926.2981028-1-o.rempel@pengutronix.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
Oleksij Rempel authored and Jakub Kicinski committed Nov 26, 2021
1 parent 32c5449 commit b3612cc
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 133 deletions.
56 changes: 7 additions & 49 deletions drivers/net/dsa/microchip/ksz8795.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1002,80 +1002,44 @@ static void ksz8_cfg_port_member(struct ksz_device *dev, int port, u8 member)
data &= ~PORT_VLAN_MEMBERSHIP;
data |= (member & dev->port_mask);
ksz_pwrite8(dev, port, P_MIRROR_CTRL, data);
dev->ports[port].member = member;
}

static void ksz8_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
{
struct ksz_device *dev = ds->priv;
int forward = dev->member;
struct ksz_port *p;
int member = -1;
u8 data;

p = &dev->ports[port];

ksz_pread8(dev, port, P_STP_CTRL, &data);
data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);

switch (state) {
case BR_STATE_DISABLED:
data |= PORT_LEARN_DISABLE;
if (port < dev->phy_port_cnt)
member = 0;
break;
case BR_STATE_LISTENING:
data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
if (port < dev->phy_port_cnt &&
p->stp_state == BR_STATE_DISABLED)
member = dev->host_mask | p->vid_member;
break;
case BR_STATE_LEARNING:
data |= PORT_RX_ENABLE;
break;
case BR_STATE_FORWARDING:
data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);

/* This function is also used internally. */
if (port == dev->cpu_port)
break;

/* Port is a member of a bridge. */
if (dev->br_member & BIT(port)) {
dev->member |= BIT(port);
member = dev->member;
} else {
member = dev->host_mask | p->vid_member;
}
break;
case BR_STATE_BLOCKING:
data |= PORT_LEARN_DISABLE;
if (port < dev->phy_port_cnt &&
p->stp_state == BR_STATE_DISABLED)
member = dev->host_mask | p->vid_member;
break;
default:
dev_err(ds->dev, "invalid STP state: %d\n", state);
return;
}

ksz_pwrite8(dev, port, P_STP_CTRL, data);

p = &dev->ports[port];
p->stp_state = state;
/* Port membership may share register with STP state. */
if (member >= 0 && member != p->member)
ksz8_cfg_port_member(dev, port, (u8)member);

/* Check if forwarding needs to be updated. */
if (state != BR_STATE_FORWARDING) {
if (dev->br_member & BIT(port))
dev->member &= ~BIT(port);
}

/* When topology has changed the function ksz_update_port_member
* should be called to modify port forwarding behavior.
*/
if (forward != dev->member)
ksz_update_port_member(dev, port);
ksz_update_port_member(dev, port);
}

static void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port)
Expand Down Expand Up @@ -1341,7 +1305,7 @@ static void ksz8795_cpu_interface_select(struct ksz_device *dev, int port)

static void ksz8_port_setup(struct ksz_device *dev, int port, bool cpu_port)
{
struct ksz_port *p = &dev->ports[port];
struct dsa_switch *ds = dev->ds;
struct ksz8 *ksz8 = dev->priv;
const u32 *masks;
u8 member;
Expand All @@ -1368,10 +1332,11 @@ static void ksz8_port_setup(struct ksz_device *dev, int port, bool cpu_port)
if (!ksz_is_ksz88x3(dev))
ksz8795_cpu_interface_select(dev, port);

member = dev->port_mask;
member = dsa_user_ports(ds);
} else {
member = dev->host_mask | p->vid_member;
member = BIT(dsa_upstream_port(ds, port));
}

ksz8_cfg_port_member(dev, port, member);
}

Expand All @@ -1392,20 +1357,13 @@ static void ksz8_config_cpu_port(struct dsa_switch *ds)
ksz_cfg(dev, regs[S_TAIL_TAG_CTRL], masks[SW_TAIL_TAG_ENABLE], true);

p = &dev->ports[dev->cpu_port];
p->vid_member = dev->port_mask;
p->on = 1;

ksz8_port_setup(dev, dev->cpu_port, true);
dev->member = dev->host_mask;

for (i = 0; i < dev->phy_port_cnt; i++) {
p = &dev->ports[i];

/* Initialize to non-zero so that ksz_cfg_port_member() will
* be called.
*/
p->vid_member = BIT(i);
p->member = dev->port_mask;
ksz8_port_stp_state_set(ds, i, BR_STATE_DISABLED);

/* Last port may be disabled. */
Expand Down
66 changes: 8 additions & 58 deletions drivers/net/dsa/microchip/ksz9477.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,6 @@ static void ksz9477_cfg_port_member(struct ksz_device *dev, int port,
u8 member)
{
ksz_pwrite32(dev, port, REG_PORT_VLAN_MEMBERSHIP__4, member);
dev->ports[port].member = member;
}

static void ksz9477_port_stp_state_set(struct dsa_switch *ds, int port,
Expand All @@ -400,49 +399,25 @@ static void ksz9477_port_stp_state_set(struct dsa_switch *ds, int port,
struct ksz_device *dev = ds->priv;
struct ksz_port *p = &dev->ports[port];
u8 data;
int member = -1;
int forward = dev->member;

ksz_pread8(dev, port, P_STP_CTRL, &data);
data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);

switch (state) {
case BR_STATE_DISABLED:
data |= PORT_LEARN_DISABLE;
if (port != dev->cpu_port)
member = 0;
break;
case BR_STATE_LISTENING:
data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
if (port != dev->cpu_port &&
p->stp_state == BR_STATE_DISABLED)
member = dev->host_mask | p->vid_member;
break;
case BR_STATE_LEARNING:
data |= PORT_RX_ENABLE;
break;
case BR_STATE_FORWARDING:
data |= (PORT_TX_ENABLE | PORT_RX_ENABLE);

/* This function is also used internally. */
if (port == dev->cpu_port)
break;

member = dev->host_mask | p->vid_member;
mutex_lock(&dev->dev_mutex);

/* Port is a member of a bridge. */
if (dev->br_member & (1 << port)) {
dev->member |= (1 << port);
member = dev->member;
}
mutex_unlock(&dev->dev_mutex);
break;
case BR_STATE_BLOCKING:
data |= PORT_LEARN_DISABLE;
if (port != dev->cpu_port &&
p->stp_state == BR_STATE_DISABLED)
member = dev->host_mask | p->vid_member;
break;
default:
dev_err(ds->dev, "invalid STP state: %d\n", state);
Expand All @@ -451,23 +426,8 @@ static void ksz9477_port_stp_state_set(struct dsa_switch *ds, int port,

ksz_pwrite8(dev, port, P_STP_CTRL, data);
p->stp_state = state;
mutex_lock(&dev->dev_mutex);
/* Port membership may share register with STP state. */
if (member >= 0 && member != p->member)
ksz9477_cfg_port_member(dev, port, (u8)member);

/* Check if forwarding needs to be updated. */
if (state != BR_STATE_FORWARDING) {
if (dev->br_member & (1 << port))
dev->member &= ~(1 << port);
}

/* When topology has changed the function ksz_update_port_member
* should be called to modify port forwarding behavior.
*/
if (forward != dev->member)
ksz_update_port_member(dev, port);
mutex_unlock(&dev->dev_mutex);
ksz_update_port_member(dev, port);
}

static void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port)
Expand Down Expand Up @@ -1168,10 +1128,10 @@ static void ksz9477_phy_errata_setup(struct ksz_device *dev, int port)

static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
{
u8 data8;
u8 member;
u16 data16;
struct ksz_port *p = &dev->ports[port];
struct dsa_switch *ds = dev->ds;
u8 data8, member;
u16 data16;

/* enable tag tail for host port */
if (cpu_port)
Expand Down Expand Up @@ -1250,12 +1210,12 @@ static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
ksz_pwrite8(dev, port, REG_PORT_XMII_CTRL_1, data8);
p->phydev.duplex = 1;
}
mutex_lock(&dev->dev_mutex);

if (cpu_port)
member = dev->port_mask;
member = dsa_user_ports(ds);
else
member = dev->host_mask | p->vid_member;
mutex_unlock(&dev->dev_mutex);
member = BIT(dsa_upstream_port(ds, port));

ksz9477_cfg_port_member(dev, port, member);

/* clear pending interrupts */
Expand All @@ -1276,8 +1236,6 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds)
const char *prev_mode;

dev->cpu_port = i;
dev->host_mask = (1 << dev->cpu_port);
dev->port_mask |= dev->host_mask;
p = &dev->ports[i];

/* Read from XMII register to determine host port
Expand Down Expand Up @@ -1312,23 +1270,15 @@ static void ksz9477_config_cpu_port(struct dsa_switch *ds)

/* enable cpu port */
ksz9477_port_setup(dev, i, true);
p->vid_member = dev->port_mask;
p->on = 1;
}
}

dev->member = dev->host_mask;

for (i = 0; i < dev->port_cnt; i++) {
if (i == dev->cpu_port)
continue;
p = &dev->ports[i];

/* Initialize to non-zero so that ksz_cfg_port_member() will
* be called.
*/
p->vid_member = (1 << i);
p->member = dev->port_mask;
ksz9477_port_stp_state_set(ds, i, BR_STATE_DISABLED);
p->on = 1;
if (i < dev->phy_port_cnt)
Expand Down
50 changes: 28 additions & 22 deletions drivers/net/dsa/microchip/ksz_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,21 +22,40 @@

void ksz_update_port_member(struct ksz_device *dev, int port)
{
struct ksz_port *p;
struct ksz_port *p = &dev->ports[port];
struct dsa_switch *ds = dev->ds;
u8 port_member = 0, cpu_port;
const struct dsa_port *dp;
int i;

for (i = 0; i < dev->port_cnt; i++) {
if (i == port || i == dev->cpu_port)
if (!dsa_is_user_port(ds, port))
return;

dp = dsa_to_port(ds, port);
cpu_port = BIT(dsa_upstream_port(ds, port));

for (i = 0; i < ds->num_ports; i++) {
const struct dsa_port *other_dp = dsa_to_port(ds, i);
struct ksz_port *other_p = &dev->ports[i];
u8 val = 0;

if (!dsa_is_user_port(ds, i))
continue;
p = &dev->ports[i];
if (!(dev->member & (1 << i)))
if (port == i)
continue;
if (!dp->bridge_dev || dp->bridge_dev != other_dp->bridge_dev)
continue;

/* Port is a member of the bridge and is forwarding. */
if (p->stp_state == BR_STATE_FORWARDING &&
p->member != dev->member)
dev->dev_ops->cfg_port_member(dev, i, dev->member);
if (other_p->stp_state == BR_STATE_FORWARDING &&
p->stp_state == BR_STATE_FORWARDING) {
val |= BIT(port);
port_member |= BIT(i);
}

dev->dev_ops->cfg_port_member(dev, i, val | cpu_port);
}

dev->dev_ops->cfg_port_member(dev, port, port_member | cpu_port);
}
EXPORT_SYMBOL_GPL(ksz_update_port_member);

Expand Down Expand Up @@ -175,12 +194,6 @@ EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
int ksz_port_bridge_join(struct dsa_switch *ds, int port,
struct net_device *br)
{
struct ksz_device *dev = ds->priv;

mutex_lock(&dev->dev_mutex);
dev->br_member |= (1 << port);
mutex_unlock(&dev->dev_mutex);

/* port_stp_state_set() will be called after to put the port in
* appropriate state so there is no need to do anything.
*/
Expand All @@ -192,13 +205,6 @@ EXPORT_SYMBOL_GPL(ksz_port_bridge_join);
void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
struct net_device *br)
{
struct ksz_device *dev = ds->priv;

mutex_lock(&dev->dev_mutex);
dev->br_member &= ~(1 << port);
dev->member &= ~(1 << port);
mutex_unlock(&dev->dev_mutex);

/* port_stp_state_set() will be called after to put the port in
* forwarding state so there is no need to do anything.
*/
Expand Down
4 changes: 0 additions & 4 deletions drivers/net/dsa/microchip/ksz_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ struct ksz_port_mib {
};

struct ksz_port {
u16 member;
u16 vid_member;
bool remove_tag; /* Remove Tag flag set, for ksz8795 only */
int stp_state;
struct phy_device phydev;
Expand Down Expand Up @@ -83,8 +81,6 @@ struct ksz_device {
struct ksz_port *ports;
struct delayed_work mib_read;
unsigned long mib_read_interval;
u16 br_member;
u16 member;
u16 mirror_rx;
u16 mirror_tx;
u32 features; /* chip specific features */
Expand Down

0 comments on commit b3612cc

Please sign in to comment.