| Message ID | BANLkTikDqjErqBmfwrN6SJPgPjmmMfJw7g@mail.gmail.com |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, Jun 28, 2011 at 07:34:57PM +0100, Nick Carter wrote: > > Yes, the default is a dumb hub (IMHO correctly so). And a dumb hub will > > forward 802.1X packets (IMHO also correctly so). > > > > Why should we specifically add a knob for EAPOL? Next we're adding one > > for STP itself, then one for LLDP, then one for Cisco's deprecated > > crap (CDP, DTP, ...) etc. > > > > If you want a dumb hub that drops EAPOL, use ebtables. > > > > -David > > > > > If we are not going to have an EAPOL knob, but we are going to act as > a repeater when STP is off then we still need these diffs to forward > the PAE group address. > (In fact we cant just act as a repeater because of the recent ethernet > bonding regression) > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index 90e985b..267f581 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -163,7 +163,8 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb) > goto drop; > > /* If STP is turned off, then forward */ > - if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0) > + if (p->br->stp_enabled == BR_NO_STP && > + (dest[5] == 0 || skb->protocol == htons(ETH_P_PAE))) > goto forward; > Nick That code actually looks quite wrong to me, we should be forwarding all of the 01:80:C2:00:00:0x groups in non-STP mode, especially :0E and :0D. (LLDP and GVRP/MVRP) Pause frames are the one exception that makes the rule, but as the comment a few lines above states, "Pause frames shouldn't be passed up by driver anyway". Btw, what might make sense is a general knob for forwarding those link-local groups, split off from the STP switch so the STP switch controls only the :00 (STP) group. That way you can decide separately whether you want to be LLDP/GVRP/802.1X/... transparent and whether you want to run STP. Not sure if it's needed, it can always be done with ebtables... -David -- 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 28 June 2011 19:58, David Lamparter <equinox@diac24.net> wrote: > On Tue, Jun 28, 2011 at 07:34:57PM +0100, Nick Carter wrote: >> > Yes, the default is a dumb hub (IMHO correctly so). And a dumb hub will >> > forward 802.1X packets (IMHO also correctly so). >> > >> > Why should we specifically add a knob for EAPOL? Next we're adding one >> > for STP itself, then one for LLDP, then one for Cisco's deprecated >> > crap (CDP, DTP, ...) etc. >> > >> > If you want a dumb hub that drops EAPOL, use ebtables. >> > >> > -David >> > >> > >> If we are not going to have an EAPOL knob, but we are going to act as >> a repeater when STP is off then we still need these diffs to forward >> the PAE group address. >> (In fact we cant just act as a repeater because of the recent ethernet >> bonding regression) >> >> diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c >> index 90e985b..267f581 100644 >> --- a/net/bridge/br_input.c >> +++ b/net/bridge/br_input.c >> @@ -163,7 +163,8 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb) >> goto drop; >> >> /* If STP is turned off, then forward */ >> - if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0) >> + if (p->br->stp_enabled == BR_NO_STP && >> + (dest[5] == 0 || skb->protocol == htons(ETH_P_PAE))) >> goto forward; >> Nick > > That code actually looks quite wrong to me, we should be forwarding all of > the 01:80:C2:00:00:0x groups in non-STP mode, especially :0E and :0D. > (LLDP and GVRP/MVRP) > > Pause frames are the one exception that makes the rule, but as the > comment a few lines above states, "Pause frames shouldn't be passed up by > driver anyway". > > Btw, what might make sense is a general knob for forwarding those > link-local groups, split off from the STP switch so the STP switch > controls only the :00 (STP) group. That way you can decide separately > whether you want to be LLDP/GVRP/802.1X/... transparent and whether you > want to run STP. Sounds good to me. So we go for :03, :0D, and :0E. We cant touch :02 see: commit f01cb5fbea1c1613621f9f32f385e12c1a29dde0 Revert "bridge: Forward reserved group addresses if !STP" > Not sure if it's needed, it can always be done with ebtables... What would be the ebtables rules to achieve the forwarding of :03 ? I asked this question on the netfilter list and the only response I got said ebtables was a filter and could not do this. :03 is hitting NF_BR_LOCAL_IN. How would you 'reinject' it so it is forwarded ? Thanks Nick > > > -David > > -- 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 28, 2011 at 09:00:16PM +0100, Nick Carter wrote: > >> /* If STP is turned off, then forward */ > >> - if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0) > >> + if (p->br->stp_enabled == BR_NO_STP && > >> + (dest[5] == 0 || skb->protocol == htons(ETH_P_PAE))) > >> goto forward; > >> Nick > > > > That code actually looks quite wrong to me, we should be forwarding all of > > the 01:80:C2:00:00:0x groups in non-STP mode, especially :0E and :0D. > > (LLDP and GVRP/MVRP) > > > > Pause frames are the one exception that makes the rule, but as the > > comment a few lines above states, "Pause frames shouldn't be passed up by > > driver anyway". > > > > Btw, what might make sense is a general knob for forwarding those > > link-local groups, split off from the STP switch so the STP switch > > controls only the :00 (STP) group. That way you can decide separately > > whether you want to be LLDP/GVRP/802.1X/... transparent and whether you > > want to run STP. > > Sounds good to me. So we go for :03, :0D, and :0E. We cant touch :02 see: > commit f01cb5fbea1c1613621f9f32f385e12c1a29dde0 > Revert "bridge: Forward reserved group addresses if !STP" > > > Not sure if it's needed, it can always be done with ebtables... > What would be the ebtables rules to achieve the forwarding of :03 ? I > asked this question on the netfilter list and the only response I got > said ebtables was a filter and could not do this. :03 is hitting > NF_BR_LOCAL_IN. How would you 'reinject' it so it is forwarded ? 'reinject' isn't possible when it hits that code path - which is pretty much why I'm saying we should be forwarding everything in the non-STP case. I have to read up on the bonding interactions, but to my understanding the only reasonable usage case is to have the bond below the bridge like eth0 \ |- bond0 - br0 eth1 / then the bonding should receive (and consume) the packets before they reach the bridge. (Some quick googling reveals that hardware switch chips special-drop 01:80:c2:00:00:01 [802.3x/pause] and :02 [802.3ad/lacp] and nothing else - for the dumb ones anyway. It also seems like the match for pause frames usually works on the address, not on the protocol field like we do it...) -David -- 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 28 June 2011 21:22, David Lamparter <equinox@diac24.net> wrote: > On Tue, Jun 28, 2011 at 09:00:16PM +0100, Nick Carter wrote: >> >> /* If STP is turned off, then forward */ >> >> - if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0) >> >> + if (p->br->stp_enabled == BR_NO_STP && >> >> + (dest[5] == 0 || skb->protocol == htons(ETH_P_PAE))) >> >> goto forward; >> >> Nick >> > >> > That code actually looks quite wrong to me, we should be forwarding all of >> > the 01:80:C2:00:00:0x groups in non-STP mode, especially :0E and :0D. >> > (LLDP and GVRP/MVRP) >> > >> > Pause frames are the one exception that makes the rule, but as the >> > comment a few lines above states, "Pause frames shouldn't be passed up by >> > driver anyway". >> > >> > Btw, what might make sense is a general knob for forwarding those >> > link-local groups, split off from the STP switch so the STP switch >> > controls only the :00 (STP) group. That way you can decide separately >> > whether you want to be LLDP/GVRP/802.1X/... transparent and whether you >> > want to run STP. >> >> Sounds good to me. So we go for :03, :0D, and :0E. We cant touch :02 see: >> commit f01cb5fbea1c1613621f9f32f385e12c1a29dde0 >> Revert "bridge: Forward reserved group addresses if !STP" >> >> > Not sure if it's needed, it can always be done with ebtables... >> What would be the ebtables rules to achieve the forwarding of :03 ? I >> asked this question on the netfilter list and the only response I got >> said ebtables was a filter and could not do this. :03 is hitting >> NF_BR_LOCAL_IN. How would you 'reinject' it so it is forwarded ? > > 'reinject' isn't possible when it hits that code path - which is pretty > much why I'm saying we should be forwarding everything in the non-STP > case. I'm not sure I like this turn off STP and suddenly start forwarding random groups. There is no connection between wanting STP on / off and forwarding pae on / off. There is no dependencies between the protocols. Also on reflection I think a knob per mac group would be better. We are only interested in 3 and if I enable pae forwarding so I can connect virtual machine supplicants, i don't then want to turn on LDP forwarding which will needlessly hit my virtual machines. So how about sysfs ../bridge/pae_forwarding ../bridge/ldp_forwarding ../bridge/mvrp_forwarding > > I have to read up on the bonding interactions, but to my understanding > the only reasonable usage case is to have the bond below the bridge like > eth0 \ > |- bond0 - br0 > eth1 / > then the bonding should receive (and consume) the packets before they > reach the bridge. > > (Some quick googling reveals that hardware switch chips special-drop > 01:80:c2:00:00:01 [802.3x/pause] and :02 [802.3ad/lacp] and nothing > else - for the dumb ones anyway. It also seems like the match for pause > frames usually works on the address, not on the protocol field like we > do it...) 'Enterprise' switches drop :03 [802.1x] Nick > > > -David > > -- 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 28, 2011 at 09:54:01PM +0100, Nick Carter wrote: > > 'reinject' isn't possible when it hits that code path - which is pretty > > much why I'm saying we should be forwarding everything in the non-STP > > case. > I'm not sure I like this turn off STP and suddenly start forwarding > random groups. There is no connection between wanting STP on / off > and forwarding pae on / off. I beg to differ, there very much is. You never ever ever want to be running STP with 802.1X packets passing through... some client will shut down your upstream port, your STP will break and you will die in a fire. The general idea, though, is that a STP-enabled switch is an intelligent switch. And an intelligent switch can speak all those pesky small side-dish protocols. With STP disabled on the other hand, it depends on site policy. Now policy... > There is no dependencies between the protocols. > Also on reflection I think a knob per mac group would be better. .... policy can be done nice and good with ebtables. You can match the groups you want, or the protocols, or the phase of the moon. > We are only interested in 3 and if I enable pae forwarding so I can > connect virtual machine supplicants, i don't then want to turn on LDP > forwarding which will needlessly hit my virtual machines. > So how about sysfs > ../bridge/pae_forwarding > ../bridge/ldp_forwarding > ../bridge/mvrp_forwarding It's not like either LLDP or MVRP will trash your VMs. Those protocols send a packet once per a few seconds. MVRP is interesting for the STP-enabled case though. I'm not aware of any userspace *VRP implementations, and dropping *VRP without an userspace daemon to speak it on our behalf means we're creating a *VRP border/break. I would however say that doing an userspace *VRP implementation is a better solution than kernel hacks for this particular case. > > (Some quick googling reveals that hardware switch chips special-drop > > 01:80:c2:00:00:01 [802.3x/pause] and :02 [802.3ad/lacp] and nothing > > else - for the dumb ones anyway. It also seems like the match for pause > > frames usually works on the address, not on the protocol field like we > > do it...) > 'Enterprise' switches drop :03 [802.1x] They also speak STP, see above about never STP+1X :) -David -- 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 28 June 2011 22:04, David Lamparter <equinox@diac24.net> wrote: > On Tue, Jun 28, 2011 at 09:54:01PM +0100, Nick Carter wrote: >> > 'reinject' isn't possible when it hits that code path - which is pretty >> > much why I'm saying we should be forwarding everything in the non-STP >> > case. >> I'm not sure I like this turn off STP and suddenly start forwarding >> random groups. There is no connection between wanting STP on / off >> and forwarding pae on / off. > > I beg to differ, there very much is. You never ever ever want to be > running STP with 802.1X packets passing through... some client will shut > down your upstream port, your STP will break and you will die in a fire. > > The general idea, though, is that a STP-enabled switch is an intelligent > switch. And an intelligent switch can speak all those pesky small > side-dish protocols. > > With STP disabled on the other hand, it depends on site policy. Now > policy... > >> There is no dependencies between the protocols. >> Also on reflection I think a knob per mac group would be better. > > .... policy can be done nice and good with ebtables. You can match the > groups you want, or the protocols, or the phase of the moon. > >> We are only interested in 3 and if I enable pae forwarding so I can >> connect virtual machine supplicants, i don't then want to turn on LDP >> forwarding which will needlessly hit my virtual machines. >> So how about sysfs >> ../bridge/pae_forwarding >> ../bridge/ldp_forwarding >> ../bridge/mvrp_forwarding > > It's not like either LLDP or MVRP will trash your VMs. Those protocols > send a packet once per a few seconds. > > MVRP is interesting for the STP-enabled case though. I'm not aware of > any userspace *VRP implementations, and dropping *VRP without an > userspace daemon to speak it on our behalf means we're creating a *VRP > border/break. > > I would however say that doing an userspace *VRP implementation is a > better solution than kernel hacks for this particular case. > >> > (Some quick googling reveals that hardware switch chips special-drop >> > 01:80:c2:00:00:01 [802.3x/pause] and :02 [802.3ad/lacp] and nothing >> > else - for the dumb ones anyway. It also seems like the match for pause >> > frames usually works on the address, not on the protocol field like we >> > do it...) >> 'Enterprise' switches drop :03 [802.1x] > > They also speak STP, see above about never STP+1X :) But if you turn off STP they wont start forwarding 802.1x. Also having STP on and forwarding 802.1x would be useful (but non-standard) in some cheap redundant periphery switches, connecting to a couple of 'core' switches acting as 802.1x authenticators. Nick > > -David > > -- 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 28, 2011 at 10:22:53PM +0100, Nick Carter wrote: > > I beg to differ, there very much is. You never ever ever want to be > > running STP with 802.1X packets passing through... some client will shut > > down your upstream port, your STP will break and you will die in a fire. > > > > The general idea, though, is that a STP-enabled switch is an intelligent > > switch. And an intelligent switch can speak all those pesky small > > side-dish protocols. [...] > >> > (Some quick googling reveals that hardware switch chips special-drop > >> > 01:80:c2:00:00:01 [802.3x/pause] and :02 [802.3ad/lacp] and nothing > >> > else - for the dumb ones anyway. It also seems like the match for pause > >> > frames usually works on the address, not on the protocol field like we > >> > do it...) > >> 'Enterprise' switches drop :03 [802.1x] > > > > They also speak STP, see above about never STP+1X :) > But if you turn off STP they wont start forwarding 802.1x. Yes, hence my suggestion to have a knob for all of the link-local ethernet groups. (Which I'm still not actually endorsing, just placing the idea) > Also having STP on and forwarding 802.1x would be useful (but > non-standard) in some cheap redundant periphery switches, connecting > to a couple of 'core' switches acting as 802.1x authenticators. That wouldn't really make much sense since those central 802.1X authenticators wouldn't be able switch the client-facing ports on and off. Instead, you now have to (1) disable the port switching to make sure your upstreams stay on and (2) deal with 802.1X clients being re"routed" by STP to different authenticators that don't know them. -David -- 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 28 June 2011 22:46, David Lamparter <equinox@diac24.net> wrote: > On Tue, Jun 28, 2011 at 10:22:53PM +0100, Nick Carter wrote: >> > I beg to differ, there very much is. You never ever ever want to be >> > running STP with 802.1X packets passing through... some client will shut >> > down your upstream port, your STP will break and you will die in a fire. >> > >> > The general idea, though, is that a STP-enabled switch is an intelligent >> > switch. And an intelligent switch can speak all those pesky small >> > side-dish protocols. > [...] >> >> > (Some quick googling reveals that hardware switch chips special-drop >> >> > 01:80:c2:00:00:01 [802.3x/pause] and :02 [802.3ad/lacp] and nothing >> >> > else - for the dumb ones anyway. It also seems like the match for pause >> >> > frames usually works on the address, not on the protocol field like we >> >> > do it...) >> >> 'Enterprise' switches drop :03 [802.1x] >> > >> > They also speak STP, see above about never STP+1X :) >> But if you turn off STP they wont start forwarding 802.1x. > > Yes, hence my suggestion to have a knob for all of the link-local > ethernet groups. (Which I'm still not actually endorsing, just placing > the idea) > >> Also having STP on and forwarding 802.1x would be useful (but >> non-standard) in some cheap redundant periphery switches, connecting >> to a couple of 'core' switches acting as 802.1x authenticators. > > That wouldn't really make much sense since those central 802.1X > authenticators wouldn't be able switch the client-facing ports on and > off. Although its non standard, it is common for authenticators to do 802.1X per source mac rather than per port. Also the central authenticator ports can be routed not bridged. So i dont think you can rule out the "STP on plus 802.1x being forwarded" requirement. > Instead, you now have to (1) disable the port switching to make > sure your upstreams stay on and (2) deal with 802.1X clients being > re"routed" by STP to different authenticators that don't know them. Well if the authenticators are pointed at a remote ACS then they will know them. And again even though non-standard, 802.1X 'mac move' functionality exists. Nick > > > -David > > -- 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
The problem is that the damn 802.1 committees keep loading up protocols on the same multicast address range. Trying to solve a design committee problem in the kernel is not going to make anybody happy. I am happy with the simple solution of: no STP == Hub STP == Bridge These are both well know configurations and are blessed by standards. -- 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 Wed, Jun 29, 2011 at 04:34:23PM -0700, Stephen Hemminger wrote: > The problem is that the damn 802.1 committees keep loading up protocols > on the same multicast address range. Trying to solve a design committee > problem in the kernel is not going to make anybody happy. > > I am happy with the simple solution of: > no STP == Hub > STP == Bridge > These are both well know configurations and are blessed by standards. I agree, that is how we should behave by default, and we'll match most admin's expectations. Regarding multicast groups, I would summarise like this: 1. any multicast gets forwarded by default, 2. unless it is 01:80:c2:00:00:01 or :02 (pause/bonding) (this rule applies regardless of STP state) 3. if STP is on: 4. 01:80:c2:00:00:00 (STP) never gets forwarded 5. 01:80:c2:00:00:03-0f don't get forwarded by default What we can do is add a switch to disable the #5 rule. The way I see it is that that switch would remove an exception from the rule and turn it back to the default #1; that's acceptable for making a new knob in my eyes. (Adding an 802.1X knob would be an exception to the exception for me, which is why I'm against it.) I'll cook up a patch in a few minutes, we really need to get rule #2 right anyway. We _MUST_NOT_ pass bonding frames in any case, but we currently do that if STP is off. (cf. my earlier patch 1/2) -David -- 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
2011/7/1 David Lamparter <equinox@diac24.net>: > On Wed, Jun 29, 2011 at 04:34:23PM -0700, Stephen Hemminger wrote: >> The problem is that the damn 802.1 committees keep loading up protocols >> on the same multicast address range. Trying to solve a design committee >> problem in the kernel is not going to make anybody happy. >> >> I am happy with the simple solution of: >> no STP == Hub >> STP == Bridge >> These are both well know configurations and are blessed by standards. > > I agree, that is how we should behave by default, and we'll match most > admin's expectations. > > Regarding multicast groups, I would summarise like this: > 1. any multicast gets forwarded by default, > 2. unless it is 01:80:c2:00:00:01 or :02 (pause/bonding) > (this rule applies regardless of STP state) > 3. if STP is on: > 4. 01:80:c2:00:00:00 (STP) never gets forwarded > 5. 01:80:c2:00:00:03-0f don't get forwarded by default > > What we can do is add a switch to disable the #5 rule. The way I see it > is that that switch would remove an exception from the rule and turn it > back to the default #1; that's acceptable for making a new knob in my > eyes. > > (Adding an 802.1X knob would be an exception to the exception for me, > which is why I'm against it.) > > I'll cook up a patch in a few minutes, we really need to get rule #2 > right anyway. We _MUST_NOT_ pass bonding frames in any case, but we > currently do that if STP is off. (cf. my earlier patch 1/2) If you use linux box as a (invisible) L2 network tap, then you want to pass everything in the hub mode (including LACP/whatever). Best Regards, Michał Mirosław -- 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 Fri, Jul 01, 2011 at 04:58:56PM +0200, Michał Mirosław wrote: [...] > > We _MUST_NOT_ pass bonding frames in any case, but we > > currently do that if STP is off. (cf. my earlier patch 1/2) > > If you use linux box as a (invisible) L2 network tap, then you want to > pass everything in the hub mode (including LACP/whatever). We must not do that by default, this breaks bridges with bonding devices as ports. I'm actively band-aiding that problem with ebtables on one of my boxes currently. How about I change "stp_forward_802local" to "forward_802local" and it gets 3 values like: - 0 (default) behave like a switch, if STP is on then drop all 16 groups, if STP is off then drop :01 and :02 - 1 forward regular groups - drop :01 and :02, forward everything else - 2 forward everything ("invisible tap mode") optional: - -1 drop all 16 groups even if STP is off (not needed, can be done with ebtables...) btw, since the drivers should eat up pause frames, you're not a fully invisible L2 tap anyway. -David -- 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
W dniu 1 lipca 2011 17:16 użytkownik David Lamparter <equinox@diac24.net> napisał: > On Fri, Jul 01, 2011 at 04:58:56PM +0200, Michał Mirosław wrote: > [...] >> > We _MUST_NOT_ pass bonding frames in any case, but we >> > currently do that if STP is off. (cf. my earlier patch 1/2) >> >> If you use linux box as a (invisible) L2 network tap, then you want to >> pass everything in the hub mode (including LACP/whatever). > > We must not do that by default, this breaks bridges with bonding devices > as ports. I'm actively band-aiding that problem with ebtables on one of > my boxes currently. > > How about I change "stp_forward_802local" to "forward_802local" and it > gets 3 values like: > - 0 (default) behave like a switch, if STP is on then drop all 16 > groups, if STP is off then drop :01 and :02 > - 1 forward regular groups - drop :01 and :02, forward everything else > - 2 forward everything ("invisible tap mode") > optional: > - -1 drop all 16 groups even if STP is off (not needed, can be done with > ebtables...) > > btw, since the drivers should eat up pause frames, you're not a fully > invisible L2 tap anyway. If -1 can be done with ebtables what is different for 0 and 1 cases? Another idea: you could make this a 16-bit bitmap (bit per group) x2 (STP vs non-STP) - that would cover all uses with the same amount of code. Best Regards, Michał Mirosław -- 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
2011/7/1 Michał Mirosław <mirqus@gmail.com>: > W dniu 1 lipca 2011 17:16 użytkownik David Lamparter > <equinox@diac24.net> napisał: >> On Fri, Jul 01, 2011 at 04:58:56PM +0200, Michał Mirosław wrote: >> [...] >>> > We _MUST_NOT_ pass bonding frames in any case, but we >>> > currently do that if STP is off. (cf. my earlier patch 1/2) >>> >>> If you use linux box as a (invisible) L2 network tap, then you want to >>> pass everything in the hub mode (including LACP/whatever). >> >> We must not do that by default, this breaks bridges with bonding devices >> as ports. I'm actively band-aiding that problem with ebtables on one of >> my boxes currently. >> >> How about I change "stp_forward_802local" to "forward_802local" and it >> gets 3 values like: >> - 0 (default) behave like a switch, if STP is on then drop all 16 >> groups, if STP is off then drop :01 and :02 >> - 1 forward regular groups - drop :01 and :02, forward everything else >> - 2 forward everything ("invisible tap mode") >> optional: >> - -1 drop all 16 groups even if STP is off (not needed, can be done with >> ebtables...) >> >> btw, since the drivers should eat up pause frames, you're not a fully >> invisible L2 tap anyway. > > If -1 can be done with ebtables what is different for 0 and 1 cases? > > Another idea: you could make this a 16-bit bitmap (bit per group) x2 > (STP vs non-STP) - that would cover all uses with the same amount of > code. That is exactly what I thought yesterday and I wrote and tested the code today :) +++ b/net/bridge/br_input.c @@ -166,6 +166,9 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb) if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0) goto forward; + if (p->br->group_fwd_mask & (1 << dest[5])) + goto forward; +++ b/net/bridge/br_private.h @@ -244,6 +244,13 @@ struct net_bridge struct timer_list multicast_query_timer; #endif + /* Each bit used to match the LSB of the IEEE 802.1D group address + * 01-80-C2-00-00-00 bit 0 + * .. + * 01-80-C2-00-00-0F bit 15 + */ + u16 group_fwd_mask; + I will post the full diffs to netdev now. With this 'knob' users can have any behaviour they require. Nick > > Best Regards, > Michał Mirosław > -- 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/net/bridge/br_input.c b/net/bridge/br_input.c index 90e985b..267f581 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -163,7 +163,8 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb) goto drop; /* If STP is turned off, then forward */ - if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0) + if (p->br->stp_enabled == BR_NO_STP && + (dest[5] == 0 || skb->protocol == htons(ETH_P_PAE))) goto forward; Nick -- To unsubscribe from this list: send the line "unsubscribe netdev" in