| Message ID | 1456510568-13679-8-git-send-email-vivien.didelot@savoirfairelinux.com |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Hi Vivien, On 02/26/2016 12:16 PM, Vivien Didelot wrote: > + /* allow CPU port or DSA link(s) to send frames to every port */ > + if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) { > + output_ports = mask; > + } else { Is this always correct? Are there situations where a CPU or neighboring switch should not be allowed to access another port? (e.g. Figure 6 or 7 in the 88E6352 functional specification). Thanks, Kevin
On Fri, Feb 26, 2016 at 08:45:28PM +0000, Kevin Smith wrote: > Hi Vivien, > > On 02/26/2016 12:16 PM, Vivien Didelot wrote: > > + /* allow CPU port or DSA link(s) to send frames to every port */ > > + if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) { > > + output_ports = mask; > > + } else { > Is this always correct? Are there situations where a CPU or neighboring > switch should not be allowed to access another port? (e.g. Figure 6 or 7 > in the 88E6352 functional specification). What do these figures show? The CPU port needs to be able to send to each external port. The whole DSA concept is that Linux has a netdev per external port, and can send frames using the netdev out a specific port. Such frames have a DSA header indicating which port they are destined to. When you have a multi chip setup, the frame needs to traverse DSA ports. Andrew
Hi Kevin, Andrew, Andrew Lunn <andrew@lunn.ch> writes: > On Fri, Feb 26, 2016 at 08:45:28PM +0000, Kevin Smith wrote: >> Hi Vivien, >> >> On 02/26/2016 12:16 PM, Vivien Didelot wrote: >> > + /* allow CPU port or DSA link(s) to send frames to every port */ >> > + if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) { >> > + output_ports = mask; >> > + } else { > >> Is this always correct? Are there situations where a CPU or neighboring >> switch should not be allowed to access another port? (e.g. Figure 6 or 7 >> in the 88E6352 functional specification). Given Linux expectations (described below by Andrew) I'd say yes, this is always correct. But I'd be curious to know if someone has counter examples for this. > What do these figures show? The figure shows the following VLANTable config: Port 0 1 2 3 4 5 6 0 - * * * - - * 1 * - * * - - * 2 * * - * - - * 3 * * * - - - * 4 - - - - - * - 5 - - - - * - - 6 * * * * - - - There is two independant groups: 0, 1, 2, 3, 6 (LAN, 6 is CPU/Router), and 4, 5 (4 is WAN and 5 is CPU/Router): Port # Port Type VLANTable Setting 0 LAN 0x4E 1 LAN 0x4D 2 LAN 0x4B 3 LAN 0x47 4 WAN 0x20 5 CPU 0x10 6 CPU 0x0F > The CPU port needs to be able to send to each external port. The whole > DSA concept is that Linux has a netdev per external port, and can send > frames using the netdev out a specific port. Such frames have a DSA > header indicating which port they are destined to. When you have a > multi chip setup, the frame needs to traverse DSA ports. This current patch produces to following setup at setup: Port 0 1 2 3 4 5 6 0 - - - - - * * 1 - - - - - * * 2 - - - - - * * 3 - - - - - * * 4 - - - - - * * 5 * * * * * - * 6 * * * * * * - Here, 5 is the CPU port and 6 is a DSA port. After joining ports 0, 1, 2 in the same bridge, we end up with: Port 0 1 2 3 4 5 6 0 - * * - - * * 1 * - * - - * * 2 * * - - - * * 3 - - - - - * * 4 - - - - - * * 5 * * * * * - * 6 * * * * * * - Thanks, -v
On Fri, Feb 26, 2016 at 04:37:39PM -0500, Vivien Didelot wrote: > Hi Kevin, Andrew, > > Andrew Lunn <andrew@lunn.ch> writes: > > > On Fri, Feb 26, 2016 at 08:45:28PM +0000, Kevin Smith wrote: > >> Hi Vivien, > >> > >> On 02/26/2016 12:16 PM, Vivien Didelot wrote: > >> > + /* allow CPU port or DSA link(s) to send frames to every port */ > >> > + if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) { > >> > + output_ports = mask; > >> > + } else { > > > >> Is this always correct? Are there situations where a CPU or neighboring > >> switch should not be allowed to access another port? (e.g. Figure 6 or 7 > >> in the 88E6352 functional specification). > > Given Linux expectations (described below by Andrew) I'd say yes, this > is always correct. But I'd be curious to know if someone has counter > examples for this. > > > What do these figures show? > > The figure shows the following VLANTable config: > > Port 0 1 2 3 4 5 6 > 0 - * * * - - * > 1 * - * * - - * > 2 * * - * - - * > 3 * * * - - - * > 4 - - - - - * - > 5 - - - - * - - > 6 * * * * - - - > > There is two independant groups: 0, 1, 2, 3, 6 (LAN, 6 is CPU/Router), > and 4, 5 (4 is WAN and 5 is CPU/Router): Ah, two CPU interfaces. We don't support that yet. I do have patches, but i took a different approach. They just load balance, by some definition of 'load balance' between the two CPU ports. Andrew
Hi Vivien, Andrew, On 02/26/2016 03:37 PM, Vivien Didelot wrote: > Here, 5 is the CPU port and 6 is a DSA port. > > After joining ports 0, 1, 2 in the same bridge, we end up with: > > Port 0 1 2 3 4 5 6 > 0 - * * - - * * > 1 * - * - - * * > 2 * * - - - * * > 3 - - - - - * * > 4 - - - - - * * > 5 * * * * * - * > 6 * * * * * * - The case I am concerned about is if the switch connected over DSA in this example has a WAN port on it, which can legitimately route to the CPU on port 5 but should not route to the LAN ports 0, 1, and 2. Does this VLAN allow direct communication between the WAN and LAN? Or is this prevented by DSA or some other mechanism? Thanks, Kevin
On Fri, Feb 26, 2016 at 10:12:28PM +0000, Kevin Smith wrote: > Hi Vivien, Andrew, > > On 02/26/2016 03:37 PM, Vivien Didelot wrote: > > Here, 5 is the CPU port and 6 is a DSA port. > > > > After joining ports 0, 1, 2 in the same bridge, we end up with: > > > > Port 0 1 2 3 4 5 6 > > 0 - * * - - * * > > 1 * - * - - * * > > 2 * * - - - * * > > 3 - - - - - * * > > 4 - - - - - * * > > 5 * * * * * - * > > 6 * * * * * * - > The case I am concerned about is if the switch connected over DSA in > this example has a WAN port on it, which can legitimately route to the > CPU on port 5 but should not route to the LAN ports 0, 1, and 2. Does > this VLAN allow direct communication between the WAN and LAN? Or is > this prevented by DSA or some other mechanism? A typical WIFI access point with a connection to a cable modem. So in linux you have interfaces like lan0, lan1, lan2, lan3, wan0 DSA provides you these interface. And by default they are all separated. There is no path between them. You can consider them as being separate physical ethernet cards, just like all other interfaces in linux. What you would typically do is: brctl addbr br0 brctl addif br0 lan0 brctl addif br0 lan1 brctl addif br0 lan2 brctl addif br0 lan3 to create a bridge between the lan ports. The linux kernel will then push this bridge configuration down into the hardware, so the switch can forward frames between these ports. The wan port is not part of the bridge, so there is no L2 path to the WAN port. You need to do IP routing on the CPU. Linux takes the stance that switch ports interfaces should act just like any other linux interface and you configure them in the normal linux way. Andrew
Hi Andrew, On 02/26/2016 04:35 PM, Andrew Lunn wrote: > On Fri, Feb 26, 2016 at 10:12:28PM +0000, Kevin Smith wrote: >> Hi Vivien, Andrew, >> >> On 02/26/2016 03:37 PM, Vivien Didelot wrote: >>> Here, 5 is the CPU port and 6 is a DSA port. >>> >>> After joining ports 0, 1, 2 in the same bridge, we end up with: >>> >>> Port 0 1 2 3 4 5 6 >>> 0 - * * - - * * >>> 1 * - * - - * * >>> 2 * * - - - * * >>> 3 - - - - - * * >>> 4 - - - - - * * >>> 5 * * * * * - * >>> 6 * * * * * * - >> The case I am concerned about is if the switch connected over DSA in >> this example has a WAN port on it, which can legitimately route to the >> CPU on port 5 but should not route to the LAN ports 0, 1, and 2. Does >> this VLAN allow direct communication between the WAN and LAN? Or is >> this prevented by DSA or some other mechanism? > A typical WIFI access point with a connection to a cable modem. > > So in linux you have interfaces like > > lan0, lan1, lan2, lan3, wan0 > > DSA provides you these interface. And by default they are all > separated. There is no path between them. You can consider them as > being separate physical ethernet cards, just like all other interfaces > in linux. > > What you would typically do is: > > brctl addbr br0 > brctl addif br0 lan0 > brctl addif br0 lan1 > brctl addif br0 lan2 > brctl addif br0 lan3 > > to create a bridge between the lan ports. The linux kernel will then > push this bridge configuration down into the hardware, so the switch > can forward frames between these ports. > > The wan port is not part of the bridge, so there is no L2 path to the > WAN port. You need to do IP routing on the CPU. > > Linux takes the stance that switch ports interfaces should act just > like any other linux interface and you configure them in the normal > linux way. > > Andrew Thanks for the explanation. I am a bit befuddled by the combination of all the possible configurations of the switch and how they interact with Linux. :) I think I understand what is happening now. Kevin
On Fri, Feb 26, 2016 at 10:47:38PM +0000, Kevin Smith wrote: > Hi Andrew, > > On 02/26/2016 04:35 PM, Andrew Lunn wrote: > > On Fri, Feb 26, 2016 at 10:12:28PM +0000, Kevin Smith wrote: > >> Hi Vivien, Andrew, > >> > >> On 02/26/2016 03:37 PM, Vivien Didelot wrote: > >>> Here, 5 is the CPU port and 6 is a DSA port. > >>> > >>> After joining ports 0, 1, 2 in the same bridge, we end up with: > >>> > >>> Port 0 1 2 3 4 5 6 > >>> 0 - * * - - * * > >>> 1 * - * - - * * > >>> 2 * * - - - * * > >>> 3 - - - - - * * > >>> 4 - - - - - * * > >>> 5 * * * * * - * > >>> 6 * * * * * * - > >> The case I am concerned about is if the switch connected over DSA in > >> this example has a WAN port on it, which can legitimately route to the > >> CPU on port 5 but should not route to the LAN ports 0, 1, and 2. Does > >> this VLAN allow direct communication between the WAN and LAN? Or is > >> this prevented by DSA or some other mechanism? > > A typical WIFI access point with a connection to a cable modem. > > > > So in linux you have interfaces like > > > > lan0, lan1, lan2, lan3, wan0 > > > > DSA provides you these interface. And by default they are all > > separated. There is no path between them. You can consider them as > > being separate physical ethernet cards, just like all other interfaces > > in linux. > > > > What you would typically do is: > > > > brctl addbr br0 > > brctl addif br0 lan0 > > brctl addif br0 lan1 > > brctl addif br0 lan2 > > brctl addif br0 lan3 > > > > to create a bridge between the lan ports. The linux kernel will then > > push this bridge configuration down into the hardware, so the switch > > can forward frames between these ports. > > > > The wan port is not part of the bridge, so there is no L2 path to the > > WAN port. You need to do IP routing on the CPU. > > > > Linux takes the stance that switch ports interfaces should act just > > like any other linux interface and you configure them in the normal > > linux way. > > > > Andrew > > Thanks for the explanation. I am a bit befuddled by the combination of > all the possible configurations of the switch and how they interact with > Linux. :) I think I understand what is happening now. You might also be looking at this the wrong way around. It is best to think of the switch as a hardware accelerator. It offers functions to the linux network stack to accelerate part of the linux network stack. We only push out to the hardware functions it is capable of accelerating. What it cannot accelerate stays in software. Think of it as a GPU, but for networking... Andrew
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c index 0f16911..7f3036b 100644 --- a/drivers/net/dsa/mv88e6xxx.c +++ b/drivers/net/dsa/mv88e6xxx.c @@ -1087,12 +1087,32 @@ abort: return ret; } -static int _mv88e6xxx_port_vlan_map_set(struct dsa_switch *ds, int port, - u16 output_ports) +static int _mv88e6xxx_port_based_vlan_map(struct dsa_switch *ds, int port) { struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); + struct net_device *bridge = ps->ports[port].bridge_dev; const u16 mask = (1 << ps->num_ports) - 1; + u16 output_ports = 0; int reg; + int i; + + /* allow CPU port or DSA link(s) to send frames to every port */ + if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)) { + output_ports = mask; + } else { + for (i = 0; i < ps->num_ports; ++i) { + /* allow sending frames to every group member */ + if (bridge && ps->ports[i].bridge_dev == bridge) + output_ports |= BIT(i); + + /* allow sending frames to CPU port and DSA link(s) */ + if (dsa_is_cpu_port(ds, i) || dsa_is_dsa_port(ds, i)) + output_ports |= BIT(i); + } + } + + /* prevent frames from going back out of the port they came in on */ + output_ports &= ~BIT(port); reg = _mv88e6xxx_reg_read(ds, REG_PORT(port), PORT_BASE_VLAN); if (reg < 0) @@ -2114,7 +2134,17 @@ int mv88e6xxx_port_bridge_join(struct dsa_switch *ds, int port, if (err) goto unlock; + /* Assign the bridge and remap each port's VLANTable */ ps->ports[port].bridge_dev = bridge; + + for (i = 0; i < ps->num_ports; ++i) { + if (ps->ports[i].bridge_dev == bridge) { + err = _mv88e6xxx_port_based_vlan_map(ds, i); + if (err) + break; + } + } + unlock: mutex_unlock(&ps->smi_mutex); @@ -2124,8 +2154,9 @@ unlock: int mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port) { struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); + struct net_device *bridge = ps->ports[port].bridge_dev; u16 fid; - int err; + int i, err; mutex_lock(&ps->smi_mutex); @@ -2138,7 +2169,17 @@ int mv88e6xxx_port_bridge_leave(struct dsa_switch *ds, int port) if (err) goto unlock; + /* Unassign the bridge and remap each port's VLANTable */ ps->ports[port].bridge_dev = NULL; + + for (i = 0; i < ps->num_ports; ++i) { + if (i == port || ps->ports[i].bridge_dev == bridge) { + err = _mv88e6xxx_port_based_vlan_map(ds, i); + if (err) + break; + } + } + unlock: mutex_unlock(&ps->smi_mutex); @@ -2402,15 +2443,14 @@ static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port) goto abort; /* Port based VLAN map: give each port its own address - * database, and allow every port to egress frames on all other ports. + * database, and allow bidirectional communication between the + * CPU and DSA port(s), and the other ports. */ ret = _mv88e6xxx_port_fid_set(ds, port, port + 1); if (ret) goto abort; - reg = BIT(ps->num_ports) - 1; /* all ports */ - reg &= ~BIT(port); /* except itself */ - ret = _mv88e6xxx_port_vlan_map_set(ds, port, reg); + ret = _mv88e6xxx_port_based_vlan_map(ds, port); if (ret) goto abort;
The In Chip Port Based VLAN Table contains bits used to restrict which output ports this input port can send frames to. With the VLAN filtering enabled, these tables work in conjunction with the VLAN Table Unit to allow egressing frames. In order to remove the current dependency to BRIDGE_VLAN_FILTERING for basic hardware bridging to work, it is necessary to restore a fine control of each port's VLANTable, on setup and when a port joins or leaves a bridge. Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> --- drivers/net/dsa/mv88e6xxx.c | 54 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 47 insertions(+), 7 deletions(-)