| Message ID | 1433208470-25338-4-git-send-email-vivien.didelot@savoirfairelinux.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
Vivien, On 06/01/2015 06:27 PM, Vivien Didelot wrote: > This commit implements the port_vlan_add and port_vlan_del functions in > the dsa_switch_driver structure for Marvell 88E6xxx compatible switches. > > This allows to access a switch VLAN Table Unit, and thus define VLANs > from standard userspace commands such as "bridge vlan". > > Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> > --- [ ... ] > + > +int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid, > + u16 bridge_flags) > +{ > + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); > + struct mv88e6xxx_vtu_entry entry = { 0 }; > + int prev_vid = vid ? vid - 1 : 4095; > + int i, ret; > + > + /* Bringing an interface up adds it to the VLAN 0. Ignore this. */ > + if (!vid) > + return 0; > + Me puzzled ;-). I brought this and the fid question up before. No idea if my e-mail got lost or what happened. Can you explain why we don't need a configuration for vlan 0 ? > + /* The DSA port-based VLAN setup reserves FID 0 to DSA_MAX_PORTS; > + * we will use the next FIDs for 802.1q; > + * thus, forbid the last DSA_MAX_PORTS VLANs. > + */ > + if (vid > 4095 - DSA_MAX_PORTS) > + return -EINVAL; > + > + mutex_lock(&ps->smi_mutex); > + ret = _mv88e6xxx_vtu_getnext(ds, prev_vid, &entry); > + if (ret < 0) > + goto unlock; > + > + /* If the VLAN does not exist, re-initialize the entry for addition */ > + if (entry.vid != vid || !entry.valid) { > + memset(&entry, 0, sizeof(entry)); > + entry.valid = true; > + entry.vid = vid; > + entry.fid = DSA_MAX_PORTS + vid; I brought this up before. No idea if my e-mail got lost or what happened. We use a fid per port, and a fid per bridge group. With VLANs, this is completely ignored, ahd there is only a single fid per vlan for the entire switch. Either per-port fids are unnecessary as well, or something is wrong here, or I am missing something. Can you explain why we only need a single fid per vlan, even if we have multiple bridge groups and the same vlan is configured in all of them ? Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jun 1, 2015 at 11:50 PM, Guenter Roeck <linux@roeck-us.net> wrote: [cut] > I brought this up before. No idea if my e-mail got lost or what happened. > > We use a fid per port, and a fid per bridge group. With VLANs, this is > completely > ignored, ahd there is only a single fid per vlan for the entire switch. > > Either per-port fids are unnecessary as well, or something is wrong here, > or I am missing something. Can you explain why we only need a single fid > per vlan, even if we have multiple bridge groups and the same vlan is > configured in all of them ? That brings up an interesting point about having multiple bridges with the same vlan configured. I struggled with that problem with rocker also and I don't have an answer other than "don't do that". Or, better put, if you have multiple bridge on the same vlan, just use one bridge for that vlan. Otherwise, I don't know how at the device level to partition the vlan between the bridges. Maybe that's what Vivien is facing also? I can see how this works for software-only bridges, because they should be isolated from each other and independent. But when offloading to a device which sees VLAN XXX global across the entire switch, I don't see how we can preserve the bridge boundaries. I hope I'm not misunderstanding the issue here; if I am, I apologize. -scott -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> +int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid, > + u16 bridge_flags) > +{ > + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); > + struct mv88e6xxx_vtu_entry entry = { 0 }; > + int prev_vid = vid ? vid - 1 : 4095; > + int i, ret; > + > + /* Bringing an interface up adds it to the VLAN 0. Ignore this. */ > + if (!vid) > + return 0; > + > + /* The DSA port-based VLAN setup reserves FID 0 to DSA_MAX_PORTS; > + * we will use the next FIDs for 802.1q; > + * thus, forbid the last DSA_MAX_PORTS VLANs. > + */ > + if (vid > 4095 - DSA_MAX_PORTS) > + return -EINVAL; I'm not sure about this. I've setup systems where i've used VLANs from the top down to differentiate them from other VLANs going from the bottom up. We might want to dynamically assign FIDs to VLANs rather than have a static mapping. Andrew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/02/2015 12:44 AM, Scott Feldman wrote: > On Mon, Jun 1, 2015 at 11:50 PM, Guenter Roeck <linux@roeck-us.net> wrote: > > [cut] > >> I brought this up before. No idea if my e-mail got lost or what happened. >> >> We use a fid per port, and a fid per bridge group. With VLANs, this is >> completely >> ignored, ahd there is only a single fid per vlan for the entire switch. >> >> Either per-port fids are unnecessary as well, or something is wrong here, >> or I am missing something. Can you explain why we only need a single fid >> per vlan, even if we have multiple bridge groups and the same vlan is >> configured in all of them ? > > That brings up an interesting point about having multiple bridges with > the same vlan configured. I struggled with that problem with rocker > also and I don't have an answer other than "don't do that". Or, > better put, if you have multiple bridge on the same vlan, just use one > bridge for that vlan. Otherwise, I don't know how at the device level > to partition the vlan between the bridges. Maybe that's what Vivien > is facing also? I can see how this works for software-only bridges, > because they should be isolated from each other and independent. But > when offloading to a device which sees VLAN XXX global across the > entire switch, I don't see how we can preserve the bridge boundaries. > One solution would be to use separate fids, like we do for non-vlan bridges. That makes fid management more complex, sure, but it would work. Otherwise we might have to explicitly disable support for more than one bridge group on a switch, which seems a bit artificial to me. Also, we already have cases where the switch is connected to the CPU with more than one Ethernet port. It is easy to imagine that the user of such a system might want to configure two bridge groups. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Also, we already have cases where the switch is connected to the CPU with > more than one Ethernet port. It is easy to imagine that the user of such > a system might want to configure two bridge groups. Hi Guenter I think that is orthogonal. Having multiple CPU ports should really only be seen as increased throughput with load sharing. It makes no different to the basic user use cases. They can all be done with a single CPU port, but with less bandwidth. Andrew -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/02/2015 06:42 AM, Andrew Lunn wrote: >> Also, we already have cases where the switch is connected to the CPU with >> more than one Ethernet port. It is easy to imagine that the user of such >> a system might want to configure two bridge groups. > > Hi Guenter > > I think that is orthogonal. Having multiple CPU ports should really > only be seen as increased throughput with load sharing. It makes no > different to the basic user use cases. They can all be done with a > single CPU port, but with less bandwidth. > Hi Andrew, quite possibly, but for my part I usually try not to restrict use cases. Some people may feel uncomfortable with load sharing and rather use the separate cpu ports to connect to dedicated external ports on the switch. Sure, that may reduce overall throughput, but it would provide a cleaner separation of traffic and guarantee that each of the ports gets its full bandwidth and is not starved by the other. Yes, I am sure that is all configurable, but it adds more and more complexity for the user. Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/02/2015 12:44 AM, Scott Feldman wrote: > That brings up an interesting point about having multiple bridges with > the same vlan configured. I struggled with that problem with rocker > also and I don't have an answer other than "don't do that". Or, > better put, if you have multiple bridge on the same vlan, just use one > bridge for that vlan. Otherwise, I don't know how at the device level > to partition the vlan between the bridges. Maybe that's what Vivien > is facing also? I can see how this works for software-only bridges, > because they should be isolated from each other and independent. But > when offloading to a device which sees VLAN XXX global across the > entire switch, I don't see how we can preserve the bridge boundaries. Scott, I'm confused by this. I think you're saying this config is problematic: br0: eth0.100, eth1.100 br1: eth2.100, eth3.100 But this works fine today. Could you clarify the issue you're referring to? Thanks, - nolan -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Guenter, On Jun 2, 2015, at 2:50 AM, Guenter Roeck linux@roeck-us.net wrote: > On 06/01/2015 06:27 PM, Vivien Didelot wrote: >> + /* Bringing an interface up adds it to the VLAN 0. Ignore this. */ >> + if (!vid) >> + return 0; >> + > > Me puzzled ;-). I brought this and the fid question up before. > No idea if my e-mail got lost or what happened. > > Can you explain why we don't need a configuration for vlan 0 ? Sorry for late reply. Initially, when issuing "ip link set up dev swp0", ndo_vlan_rx_add_vid was called to add the interface in the VLAN 0. 2 things happen here: * this is inconsistent with the "bridge vlan" output which doesn't seem to know about a VID 0; * VID 0 seems special for this switch: if an ingressing frame has VID 0, the tagged port will override the VID bits with the port default VID at egress. Thanks, -v -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 02, 2015 at 09:39:50PM -0400, Vivien Didelot wrote: > Guenter, > > On Jun 2, 2015, at 2:50 AM, Guenter Roeck linux@roeck-us.net wrote: > > On 06/01/2015 06:27 PM, Vivien Didelot wrote: > >> + /* Bringing an interface up adds it to the VLAN 0. Ignore this. */ > >> + if (!vid) > >> + return 0; > >> + > > > > Me puzzled ;-). I brought this and the fid question up before. > > No idea if my e-mail got lost or what happened. > > > > Can you explain why we don't need a configuration for vlan 0 ? > > Sorry for late reply. Initially, when issuing "ip link set up dev swp0", > ndo_vlan_rx_add_vid was called to add the interface in the VLAN 0. > Loading the 802.1q module has the same effect. I think this may be on purpose; it is telling the switch to accept packets with vid==0 (and untagged packets). > 2 things happen here: > > * this is inconsistent with the "bridge vlan" output which doesn't seem to > know about a VID 0; > * VID 0 seems special for this switch: if an ingressing frame has VID 0, the > tagged port will override the VID bits with the port default VID at egress. > As far as I can see, the switch treats packets with vid==0 and untaged packets as unknown if VLAN support is enabled. Anyway, sounds odd. Sure this isn't a configuration problem somethere ? Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jun 2, 2015 at 3:31 PM, nolan <nolan@cumulusnetworks.com> wrote: > On 06/02/2015 12:44 AM, Scott Feldman wrote: >> >> That brings up an interesting point about having multiple bridges with >> the same vlan configured. I struggled with that problem with rocker >> also and I don't have an answer other than "don't do that". Or, >> better put, if you have multiple bridge on the same vlan, just use one >> bridge for that vlan. Otherwise, I don't know how at the device level >> to partition the vlan between the bridges. Maybe that's what Vivien >> is facing also? I can see how this works for software-only bridges, >> because they should be isolated from each other and independent. But >> when offloading to a device which sees VLAN XXX global across the >> entire switch, I don't see how we can preserve the bridge boundaries. > > > Scott, > > I'm confused by this. I think you're saying this config is problematic: > > br0: eth0.100, eth1.100 > br1: eth2.100, eth3.100 > > > But this works fine today. Ya, this is going to work because br0 and br1 are bridging untagged traffic, but br0 and br1 have different internal VLAN ids for untagged traffic, so there is no crosstalk between bridges. (I'm assuming since you used the ethX.100 format, you've vconfig created a vlan interface on ethX and added the vlan interface to brY). The vlan interface eats the vlan tag and the bridge sees untagged traffic. > Could you clarify the issue you're referring to? I'm talking about bridging tagged traffic. E.g.: ip link add name br0 type bridge ip link add name br1 type bridge ip link set dev sw1p1 master br0 ip link set dev sw1p2 master br0 ip link set dev sw1p3 master br1 ip link set dev sw1p4 master br1 bridge vlan add vid 100 dev sw1p1 bridge vlan add vid 100 dev sw1p2 bridge vlan add vid 100 dev sw1p3 bridge vlan add vid 100 dev sw1p4 Now the ports are trunking vlan 100 and the bridge/device see tagged traffic. If the device used floods vlan 100 pkt to all ports in vlan 100, it'll flood to a port outside the bridge. Oops! For the device I'm using (rocker w/ OF-DPA) the bridging table matches on vlan ID and mac_dst. There is no prevision to isolate vlans per bridge. How do you solve the above case with your hardware? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/02/2015 11:53 PM, Scott Feldman wrote: > On Tue, Jun 2, 2015 at 3:31 PM, nolan <nolan@cumulusnetworks.com> wrote: >> On 06/02/2015 12:44 AM, Scott Feldman wrote: >>> >>> That brings up an interesting point about having multiple bridges with >>> the same vlan configured. I struggled with that problem with rocker >>> also and I don't have an answer other than "don't do that". Or, >>> better put, if you have multiple bridge on the same vlan, just use one >>> bridge for that vlan. Otherwise, I don't know how at the device level >>> to partition the vlan between the bridges. Maybe that's what Vivien >>> is facing also? I can see how this works for software-only bridges, >>> because they should be isolated from each other and independent. But >>> when offloading to a device which sees VLAN XXX global across the >>> entire switch, I don't see how we can preserve the bridge boundaries. >> >> >> Scott, >> >> I'm confused by this. I think you're saying this config is problematic: >> >> br0: eth0.100, eth1.100 >> br1: eth2.100, eth3.100 >> >> >> But this works fine today. > > Ya, this is going to work because br0 and br1 are bridging untagged > traffic, but br0 and br1 have different internal VLAN ids for untagged > traffic, so there is no crosstalk between bridges. > > (I'm assuming since you used the ethX.100 format, you've vconfig > created a vlan interface on ethX and added the vlan interface to brY). > The vlan interface eats the vlan tag and the bridge sees untagged > traffic. > >> Could you clarify the issue you're referring to? > > I'm talking about bridging tagged traffic. E.g.: > > ip link add name br0 type bridge > ip link add name br1 type bridge > > ip link set dev sw1p1 master br0 > ip link set dev sw1p2 master br0 > ip link set dev sw1p3 master br1 > ip link set dev sw1p4 master br1 > > bridge vlan add vid 100 dev sw1p1 > bridge vlan add vid 100 dev sw1p2 > bridge vlan add vid 100 dev sw1p3 > bridge vlan add vid 100 dev sw1p4 > > Now the ports are trunking vlan 100 and the bridge/device see tagged > traffic. If the device used floods vlan 100 pkt to all ports in vlan > 100, it'll flood to a port outside the bridge. Oops! For the device > I'm using (rocker w/ OF-DPA) the bridging table matches on vlan ID and > mac_dst. There is no prevision to isolate vlans per bridge. > > How do you solve the above case with your hardware? > We could use separate FIDs per vlan/bridge group, ie don't assume that fid == vid. A simple solution might be to just set the fid to the fid used by the bridge. That would not support 802.1s, though. Guenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Guenter, On Jun 2, 2015, at 10:17 PM, Guenter Roeck linux@roeck-us.net wrote: > On Tue, Jun 02, 2015 at 09:39:50PM -0400, Vivien Didelot wrote: >> Guenter, >> >> On Jun 2, 2015, at 2:50 AM, Guenter Roeck linux@roeck-us.net wrote: >> > On 06/01/2015 06:27 PM, Vivien Didelot wrote: >> >> + /* Bringing an interface up adds it to the VLAN 0. Ignore this. */ >> >> + if (!vid) >> >> + return 0; >> >> + >> > >> > Me puzzled ;-). I brought this and the fid question up before. >> > No idea if my e-mail got lost or what happened. >> > >> > Can you explain why we don't need a configuration for vlan 0 ? >> >> Sorry for late reply. Initially, when issuing "ip link set up dev swp0", >> ndo_vlan_rx_add_vid was called to add the interface in the VLAN 0. >> > Loading the 802.1q module has the same effect. > > I think this may be on purpose; it is telling the switch to accept > packets with vid==0 (and untagged packets). > >> 2 things happen here: >> >> * this is inconsistent with the "bridge vlan" output which doesn't seem to >> know about a VID 0; >> * VID 0 seems special for this switch: if an ingressing frame has VID 0, the >> tagged port will override the VID bits with the port default VID at egress. >> > As far as I can see, the switch treats packets with vid==0 and untaged packets > as unknown if VLAN support is enabled. I am not sure about the untagged frames. But for tagged frames, the documentation says that frames with vid 0 will be overridden with the port's default VID. > Anyway, sounds odd. Sure this isn't a configuration problem somethere ? If I'm not mistaken, other drivers do that. e.g. Rocker deals with VID >= 1: for (vid = 1; vid < VLAN_N_VID; vid++) Maybe this VID overriding feature is what we want? But it doesn't look right to me, even more since it is not exposed to the user. Thanks, -v -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 06/03/2015 07:56 AM, Vivien Didelot wrote: > Hi Guenter, > > On Jun 2, 2015, at 10:17 PM, Guenter Roeck linux@roeck-us.net wrote: >> On Tue, Jun 02, 2015 at 09:39:50PM -0400, Vivien Didelot wrote: >>> Guenter, >>> >>> On Jun 2, 2015, at 2:50 AM, Guenter Roeck linux@roeck-us.net wrote: >>>> On 06/01/2015 06:27 PM, Vivien Didelot wrote: >>>>> + /* Bringing an interface up adds it to the VLAN 0. Ignore this. */ >>>>> + if (!vid) >>>>> + return 0; >>>>> + >>>> >>>> Me puzzled ;-). I brought this and the fid question up before. >>>> No idea if my e-mail got lost or what happened. >>>> >>>> Can you explain why we don't need a configuration for vlan 0 ? >>> >>> Sorry for late reply. Initially, when issuing "ip link set up dev swp0", >>> ndo_vlan_rx_add_vid was called to add the interface in the VLAN 0. >>> >> Loading the 802.1q module has the same effect. >> >> I think this may be on purpose; it is telling the switch to accept >> packets with vid==0 (and untagged packets). >> >>> 2 things happen here: >>> >>> * this is inconsistent with the "bridge vlan" output which doesn't seem to >>> know about a VID 0; >>> * VID 0 seems special for this switch: if an ingressing frame has VID 0, the >>> tagged port will override the VID bits with the port default VID at egress. >>> >> As far as I can see, the switch treats packets with vid==0 and untaged packets >> as unknown if VLAN support is enabled. > > I am not sure about the untagged frames. But for tagged frames, the > documentation says that frames with vid 0 will be overridden with the port's > default VID. > The documentation says that untagged frames get the port's default VID assigned. >> Anyway, sounds odd. Sure this isn't a configuration problem somethere ? > > If I'm not mistaken, other drivers do that. e.g. Rocker deals with VID >= 1: > > for (vid = 1; vid < VLAN_N_VID; vid++) > Yes, but that won't help us. Again, the problem is that the switch drops untagged packets if VLAN mode is set to secure and there is no VID table entry for VID 0 (or, rather, the port's default VID, which happens to be 0 in our case). We'll have to solve that problem somehow. Using fallback mode isn't really a good solution since it still treats untagged packets (and priority tagged packets with vid==0) as membership violations. It might make sense to set all ports to secure mode, but then we would always have to create a VID table entry for vid=0 (or vid=default vid). Maybe we should just do that, and/or keep 802.1q support on a port disabled unless it is explicitly enabled by some means (such as adding an entry to the port's VLAN filter table). Since we actually set the default VLAN to 0 in mv88e6xxx_setup_port_common(), I wonder if you actually _see_ a problem with replaced VIDs, or if you just assume that there would be one. Can you clarify ? Thanks, Guenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 02/06/15 23:53, Scott Feldman wrote: > On Tue, Jun 2, 2015 at 3:31 PM, nolan <nolan@cumulusnetworks.com> wrote: >> On 06/02/2015 12:44 AM, Scott Feldman wrote: >>> >>> That brings up an interesting point about having multiple bridges with >>> the same vlan configured. I struggled with that problem with rocker >>> also and I don't have an answer other than "don't do that". Or, >>> better put, if you have multiple bridge on the same vlan, just use one >>> bridge for that vlan. Otherwise, I don't know how at the device level >>> to partition the vlan between the bridges. Maybe that's what Vivien >>> is facing also? I can see how this works for software-only bridges, >>> because they should be isolated from each other and independent. But >>> when offloading to a device which sees VLAN XXX global across the >>> entire switch, I don't see how we can preserve the bridge boundaries. >> >> >> Scott, >> >> I'm confused by this. I think you're saying this config is problematic: >> >> br0: eth0.100, eth1.100 >> br1: eth2.100, eth3.100 >> >> >> But this works fine today. > > Ya, this is going to work because br0 and br1 are bridging untagged > traffic, but br0 and br1 have different internal VLAN ids for untagged > traffic, so there is no crosstalk between bridges. > > (I'm assuming since you used the ethX.100 format, you've vconfig > created a vlan interface on ethX and added the vlan interface to brY). > The vlan interface eats the vlan tag and the bridge sees untagged > traffic. > >> Could you clarify the issue you're referring to? > > I'm talking about bridging tagged traffic. E.g.: > > ip link add name br0 type bridge > ip link add name br1 type bridge > > ip link set dev sw1p1 master br0 > ip link set dev sw1p2 master br0 > ip link set dev sw1p3 master br1 > ip link set dev sw1p4 master br1 > > bridge vlan add vid 100 dev sw1p1 > bridge vlan add vid 100 dev sw1p2 > bridge vlan add vid 100 dev sw1p3 > bridge vlan add vid 100 dev sw1p4 > > Now the ports are trunking vlan 100 and the bridge/device see tagged > traffic. If the device used floods vlan 100 pkt to all ports in vlan > 100, it'll flood to a port outside the bridge. Oops! For the device > I'm using (rocker w/ OF-DPA) the bridging table matches on vlan ID and > mac_dst. There is no prevision to isolate vlans per bridge. > > How do you solve the above case with your hardware? For switches that support double-tagging, I suppose you could utilize the fact that packets need to be normalized to include an internal outer tag as well (for both tagged and untagged packets), and utilize that to make sure there is no cross-talk. That way, you can have the same inner VLAN tags on both of your bridges but the internal outer tag can be different. There might be a better solution, like having proper partitioning based on whatever the switch is capable of?
On 06/02/2015 11:53 PM, Scott Feldman wrote: > I'm talking about bridging tagged traffic. E.g.: > > ip link add name br0 type bridge > ip link add name br1 type bridge > > ip link set dev sw1p1 master br0 > ip link set dev sw1p2 master br0 > ip link set dev sw1p3 master br1 > ip link set dev sw1p4 master br1 > > bridge vlan add vid 100 dev sw1p1 > bridge vlan add vid 100 dev sw1p2 > bridge vlan add vid 100 dev sw1p3 > bridge vlan add vid 100 dev sw1p4 > > Now the ports are trunking vlan 100 and the bridge/device see tagged > traffic. If the device used floods vlan 100 pkt to all ports in vlan > 100, it'll flood to a port outside the bridge. Oops! For the device > I'm using (rocker w/ OF-DPA) the bridging table matches on vlan ID and > mac_dst. There is no prevision to isolate vlans per bridge. > > How do you solve the above case with your hardware? Ah, understood, thanks for clarifying. Right now, we only support this case with vconfig-style vlans. I don't see any conceptual reason it couldn't be supported in the vlan-aware bridge model as well. The HW configuration would be essentially identical to the equivalent vconfig-style config. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/dsa/mv88e6xxx.c b/drivers/net/dsa/mv88e6xxx.c index 7fba330..49ef2f8 100644 --- a/drivers/net/dsa/mv88e6xxx.c +++ b/drivers/net/dsa/mv88e6xxx.c @@ -2,6 +2,9 @@ * net/dsa/mv88e6xxx.c - Marvell 88e6xxx switch chip support * Copyright (c) 2008 Marvell Semiconductor * + * Copyright (c) 2015 CMC Electronics, Inc. + * Added support for 802.1q VLAN Table Unit operations + * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by * the Free Software Foundation; either version 2 of the License, or @@ -1348,6 +1351,281 @@ static void mv88e6xxx_bridge_work(struct work_struct *work) } } +static int _mv88e6xxx_vtu_wait(struct dsa_switch *ds) +{ + return _mv88e6xxx_wait(ds, REG_GLOBAL, GLOBAL_VTU_OP, + GLOBAL_VTU_OP_BUSY); +} + +static int _mv88e6xxx_vtu_cmd(struct dsa_switch *ds, u16 op) +{ + int ret; + + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_OP, op); + if (ret < 0) + return ret; + + return _mv88e6xxx_vtu_wait(ds); +} + +static int _mv88e6xxx_stu_loadpurge(struct dsa_switch *ds, u8 sid, bool valid) +{ + int ret, data; + + ret = _mv88e6xxx_vtu_wait(ds); + if (ret < 0) + return ret; + + data = sid & GLOBAL_VTU_SID_MASK; + if (valid) + data |= GLOBAL_VTU_VID_VALID; + + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID, data); + if (ret < 0) + return ret; + + /* Unused (yet) data registers */ + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3, 0); + if (ret < 0) + return ret; + + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7, 0); + if (ret < 0) + return ret; + + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_8_11, 0); + if (ret < 0) + return ret; + + return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_STU_LOAD_PURGE); +} + +static int _mv88e6xxx_vtu_getnext(struct dsa_switch *ds, u16 vid, + struct mv88e6xxx_vtu_entry *entry) +{ + int ret, i; + + ret = _mv88e6xxx_vtu_wait(ds); + if (ret < 0) + return ret; + + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID, + vid & GLOBAL_VTU_VID_MASK); + if (ret < 0) + return ret; + + ret = _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_GET_NEXT); + if (ret < 0) + return ret; + + ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_VID); + if (ret < 0) + return ret; + + entry->vid = ret & GLOBAL_VTU_VID_MASK; + entry->valid = !!(ret & GLOBAL_VTU_VID_VALID); + + if (entry->valid) { + /* Ports 0-3, offsets 0, 4, 8, 12 */ + ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3); + if (ret < 0) + return ret; + + for (i = 0; i < 4; ++i) + entry->tags[i] = (ret >> (i * 4)) & 3; + + /* Ports 4-6, offsets 0, 4, 8 */ + ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7); + if (ret < 0) + return ret; + + for (i = 4; i < 7; ++i) + entry->tags[i] = (ret >> ((i - 4) * 4)) & 3; + + ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_FID); + if (ret < 0) + return ret; + + entry->fid = ret & GLOBAL_VTU_FID_MASK; + + ret = _mv88e6xxx_reg_read(ds, REG_GLOBAL, GLOBAL_VTU_SID); + if (ret < 0) + return ret; + + entry->sid = ret & GLOBAL_VTU_SID_MASK; + } + + return 0; +} + +static int _mv88e6xxx_vtu_loadpurge(struct dsa_switch *ds, + struct mv88e6xxx_vtu_entry *entry) +{ + u16 data = 0; + int ret, i; + + ret = _mv88e6xxx_vtu_wait(ds); + if (ret < 0) + return ret; + + if (entry->valid) { + /* Set Data Register, ports 0-3, offsets 0, 4, 8, 12 */ + for (data = i = 0; i < 4; ++i) + data |= entry->tags[i] << (i * 4); + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_0_3, + data); + if (ret < 0) + return ret; + + /* Set Data Register, ports 4-6, offsets 0, 4, 8 */ + for (data = 0, i = 4; i < 7; ++i) + data |= entry->tags[i] << ((i - 4) * 4); + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_4_7, + data); + if (ret < 0) + return ret; + + /* Unused Data Register */ + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_DATA_8_11, + 0); + if (ret < 0) + return ret; + + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_SID, + entry->sid & GLOBAL_VTU_SID_MASK); + if (ret < 0) + return ret; + + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_FID, + entry->fid & GLOBAL_VTU_FID_MASK); + if (ret < 0) + return ret; + + /* Valid bit set means load, unset means purge */ + data = GLOBAL_VTU_VID_VALID; + } + + data |= entry->vid & GLOBAL_VTU_VID_MASK; + ret = _mv88e6xxx_reg_write(ds, REG_GLOBAL, GLOBAL_VTU_VID, data); + if (ret < 0) + return ret; + + return _mv88e6xxx_vtu_cmd(ds, GLOBAL_VTU_OP_VTU_LOAD_PURGE); +} + +int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid, + u16 bridge_flags) +{ + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); + struct mv88e6xxx_vtu_entry entry = { 0 }; + int prev_vid = vid ? vid - 1 : 4095; + int i, ret; + + /* Bringing an interface up adds it to the VLAN 0. Ignore this. */ + if (!vid) + return 0; + + /* The DSA port-based VLAN setup reserves FID 0 to DSA_MAX_PORTS; + * we will use the next FIDs for 802.1q; + * thus, forbid the last DSA_MAX_PORTS VLANs. + */ + if (vid > 4095 - DSA_MAX_PORTS) + return -EINVAL; + + mutex_lock(&ps->smi_mutex); + ret = _mv88e6xxx_vtu_getnext(ds, prev_vid, &entry); + if (ret < 0) + goto unlock; + + /* If the VLAN does not exist, re-initialize the entry for addition */ + if (entry.vid != vid || !entry.valid) { + memset(&entry, 0, sizeof(entry)); + entry.valid = true; + entry.vid = vid; + entry.fid = DSA_MAX_PORTS + vid; + entry.sid = 0; /* We don't use 802.1s (yet) */ + + /* A VTU entry must have a valid STU entry (undocumented). + * The default STU pointer for a VTU entry is 0. If per VLAN + * spanning tree is not used then only one STU entry is needed + * to cover all VTU entries. Thus, validate the STU entry 0. + */ + ret = _mv88e6xxx_stu_loadpurge(ds, 0, true); + if (ret < 0) + goto unlock; + + for (i = 0; i < ps->num_ports; ++i) + entry.tags[i] = dsa_is_cpu_port(ds, i) ? + GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED : + GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER; + } + + entry.tags[port] = bridge_flags & BRIDGE_VLAN_INFO_UNTAGGED ? + GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED : + GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED; + + ret = _mv88e6xxx_vtu_loadpurge(ds, &entry); + + /* Set port default VID */ + if (bridge_flags & BRIDGE_VLAN_INFO_PVID) + ret = _mv88e6xxx_reg_write(ds, REG_PORT(port), + PORT_DEFAULT_VLAN, + vid & PORT_DEFAULT_VLAN_MASK); +unlock: + mutex_unlock(&ps->smi_mutex); + + return ret; +} + +int mv88e6xxx_port_vlan_del(struct dsa_switch *ds, int port, u16 vid) +{ + struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); + struct mv88e6xxx_vtu_entry entry = { 0 }; + int i, ret, prev_vid = vid ? vid - 1 : 4095; + bool keep = false; + + /* Bringing an interface up adds it to the VLAN 0. Ignore this. */ + if (!vid) + return 0; + + mutex_lock(&ps->smi_mutex); + ret = _mv88e6xxx_vtu_getnext(ds, prev_vid, &entry); + if (ret < 0) + goto unlock; + + if (entry.vid != vid || !entry.valid) { + ret = -EINVAL; + goto unlock; + } + + entry.tags[port] = GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER; + + /* keep the VLAN unless all ports are excluded */ + for (i = 0; i < ps->num_ports; ++i) { + if (dsa_is_cpu_port(ds, i)) + continue; + + if (entry.tags[i] != GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER) { + keep = true; + break; + } + } + + entry.valid = keep; + ret = _mv88e6xxx_vtu_loadpurge(ds, &entry); + if (ret < 0) + goto unlock; + + /* TODO reset PVID if it was this vid? */ + + if (!keep) + ret = _mv88e6xxx_update_bridge_config(ds, entry.fid); +unlock: + mutex_unlock(&ps->smi_mutex); + + return ret; +} + static int mv88e6xxx_setup_port(struct dsa_switch *ds, int port) { struct mv88e6xxx_priv_state *ps = ds_to_priv(ds); diff --git a/drivers/net/dsa/mv88e6xxx.h b/drivers/net/dsa/mv88e6xxx.h index e10ccdb..42032c3 100644 --- a/drivers/net/dsa/mv88e6xxx.h +++ b/drivers/net/dsa/mv88e6xxx.h @@ -120,6 +120,7 @@ #define PORT_CONTROL_1 0x05 #define PORT_BASE_VLAN 0x06 #define PORT_DEFAULT_VLAN 0x07 +#define PORT_DEFAULT_VLAN_MASK 0xfff #define PORT_CONTROL_2 0x08 #define PORT_CONTROL_2_IGNORE_FCS BIT(15) #define PORT_CONTROL_2_VTU_PRI_OVERRIDE BIT(14) @@ -160,6 +161,10 @@ #define GLOBAL_MAC_01 0x01 #define GLOBAL_MAC_23 0x02 #define GLOBAL_MAC_45 0x03 +#define GLOBAL_VTU_FID 0x02 /* 6352 only? */ +#define GLOBAL_VTU_FID_MASK 0xfff +#define GLOBAL_VTU_SID 0x03 /* 6352 only? */ +#define GLOBAL_VTU_SID_MASK 0x3f #define GLOBAL_CONTROL 0x04 #define GLOBAL_CONTROL_SW_RESET BIT(15) #define GLOBAL_CONTROL_PPU_ENABLE BIT(14) @@ -176,9 +181,20 @@ #define GLOBAL_CONTROL_TCAM_EN BIT(1) #define GLOBAL_CONTROL_EEPROM_DONE_EN BIT(0) #define GLOBAL_VTU_OP 0x05 +#define GLOBAL_VTU_OP_BUSY BIT(15) +#define GLOBAL_VTU_OP_FLUSH_ALL ((1 << 12) | GLOBAL_VTU_OP_BUSY) +#define GLOBAL_VTU_OP_VTU_LOAD_PURGE ((3 << 12) | GLOBAL_VTU_OP_BUSY) +#define GLOBAL_VTU_OP_VTU_GET_NEXT ((4 << 12) | GLOBAL_VTU_OP_BUSY) +#define GLOBAL_VTU_OP_STU_LOAD_PURGE ((5 << 12) | GLOBAL_VTU_OP_BUSY) #define GLOBAL_VTU_VID 0x06 +#define GLOBAL_VTU_VID_MASK 0xfff +#define GLOBAL_VTU_VID_VALID BIT(12) #define GLOBAL_VTU_DATA_0_3 0x07 #define GLOBAL_VTU_DATA_4_7 0x08 +#define GLOBAL_VTU_DATA_MEMBER_TAG_UNMODIFIED 0 +#define GLOBAL_VTU_DATA_MEMBER_TAG_UNTAGGED 1 +#define GLOBAL_VTU_DATA_MEMBER_TAG_TAGGED 2 +#define GLOBAL_VTU_DATA_MEMBER_TAG_NON_MEMBER 3 #define GLOBAL_VTU_DATA_8_11 0x09 #define GLOBAL_ATU_CONTROL 0x0a #define GLOBAL_ATU_CONTROL_LEARN2ALL BIT(3) @@ -293,6 +309,14 @@ #define GLOBAL2_QOS_WEIGHT 0x1c #define GLOBAL2_MISC 0x1d +struct mv88e6xxx_vtu_entry { + u16 vid; + u16 fid; + u8 sid; + bool valid; + u8 tags[DSA_MAX_PORTS]; +}; + struct mv88e6xxx_priv_state { /* When using multi-chip addressing, this mutex protects * access to the indirect access registers. (In single-chip @@ -397,6 +421,9 @@ int mv88e6xxx_port_fdb_getnext(struct dsa_switch *ds, int port, int mv88e6xxx_phy_page_read(struct dsa_switch *ds, int port, int page, int reg); int mv88e6xxx_phy_page_write(struct dsa_switch *ds, int port, int page, int reg, int val); +int mv88e6xxx_port_vlan_add(struct dsa_switch *ds, int port, u16 vid, + u16 bridge_flags); +int mv88e6xxx_port_vlan_del(struct dsa_switch *ds,int port, u16 vid); extern struct dsa_switch_driver mv88e6131_switch_driver; extern struct dsa_switch_driver mv88e6123_61_65_switch_driver; extern struct dsa_switch_driver mv88e6352_switch_driver;
This commit implements the port_vlan_add and port_vlan_del functions in the dsa_switch_driver structure for Marvell 88E6xxx compatible switches. This allows to access a switch VLAN Table Unit, and thus define VLANs from standard userspace commands such as "bridge vlan". Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com> --- drivers/net/dsa/mv88e6xxx.c | 278 ++++++++++++++++++++++++++++++++++++++++++++ drivers/net/dsa/mv88e6xxx.h | 27 +++++ 2 files changed, 305 insertions(+)