Joakim Tjernlund
2009-May-12 14:02 UTC
[Bridge] [PATCH] bridge: relay bridge multicast pkgs if !STP
Currently the bridge drops all STP pkgs when
STP is off. This makes the bridge relay the Bridge Group
multicast address, 01:80:c2:00:00:00, when STP is off.
This allows for loop detection by other bridges doing STP.
Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se>
---
net/bridge/br_input.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 30b8877..8197823 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -112,6 +112,17 @@ static inline int is_link_local(const unsigned char *dest)
return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | ((a[2] ^ b[2]) & m)) == 0;
}
+/* Does address match the Bride Group multicast address.
+ * 01:80:c2:00:00:00
+ */
+static inline int is_bridge_group(const unsigned char *dest)
+{
+ __be16 *a = (__be16 *)dest;
+ static const __be16 *b = (const __be16 *)br_group_address;
+
+ return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) == 0;
+}
+
/*
* Called via br_handle_frame_hook.
* Return NULL if skb is handled
@@ -137,7 +148,8 @@ struct sk_buff *br_handle_frame(struct net_bridge_port *p,
struct sk_buff *skb)
if (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
NULL, br_handle_local_finish))
return NULL; /* frame consumed by filter */
- else
+ /* Relay Bridge Group address is STP is off */
+ if (!(p->br->stp_enabled == BR_NO_STP &&
is_bridge_group(dest)))
return skb; /* continue processing */
}
--
1.6.2.3
Stephen Hemminger
2009-May-12 15:26 UTC
[Bridge] [PATCH] bridge: relay bridge multicast pkgs if !STP
On Tue, 12 May 2009 16:02:14 +0200 Joakim Tjernlund <Joakim.Tjernlund at transmode.se> wrote:> Currently the bridge drops all STP pkgs when > STP is off. This makes the bridge relay the Bridge Group > multicast address, 01:80:c2:00:00:00, when STP is off. > This allows for loop detection by other bridges doing STP. > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se> > --- > net/bridge/br_input.c | 14 +++++++++++++- > 1 files changed, 13 insertions(+), 1 deletions(-) > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > index 30b8877..8197823 100644 > --- a/net/bridge/br_input.c > +++ b/net/bridge/br_input.c > @@ -112,6 +112,17 @@ static inline int is_link_local(const unsigned char *dest) > return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | ((a[2] ^ b[2]) & m)) == 0; > } > > +/* Does address match the Bride Group multicast address. > + * 01:80:c2:00:00:00 > + */ > +static inline int is_bridge_group(const unsigned char *dest) > +{ > + __be16 *a = (__be16 *)dest; > + static const __be16 *b = (const __be16 *)br_group_address; > + > + return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) == 0; > +} > + > /* > * Called via br_handle_frame_hook. > * Return NULL if skb is handled > @@ -137,7 +148,8 @@ struct sk_buff *br_handle_frame(struct net_bridge_port *p, struct sk_buff *skb) > if (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev, > NULL, br_handle_local_finish)) > return NULL; /* frame consumed by filter */ > - else > + /* Relay Bridge Group address is STP is off */ > + if (!(p->br->stp_enabled == BR_NO_STP && is_bridge_group(dest))) > return skb; /* continue processing */ > } >First, you should have used compare_ether_addr. Second, it needs to still obey state rules on port, so the return skb should fall through to the switch statement. --
Joakim Tjernlund
2009-May-12 17:17 UTC
[Bridge] [PATCH] bridge: relay bridge multicast pkgs if !STP
Stephen Hemminger <shemminger at vyatta.com> wrote on 12/05/2009 17:26:52:> > On Tue, 12 May 2009 16:02:14 +0200 > Joakim Tjernlund <Joakim.Tjernlund at transmode.se> wrote: > > > Currently the bridge drops all STP pkgs when > > STP is off. This makes the bridge relay the Bridge Group > > multicast address, 01:80:c2:00:00:00, when STP is off. > > This allows for loop detection by other bridges doing STP. > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se> > > --- > > net/bridge/br_input.c | 14 +++++++++++++- > > 1 files changed, 13 insertions(+), 1 deletions(-) > > > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > > index 30b8877..8197823 100644 > > --- a/net/bridge/br_input.c > > +++ b/net/bridge/br_input.c > > @@ -112,6 +112,17 @@ static inline int is_link_local(const unsigned char *dest) > > return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | ((a[2] ^ b[2]) & m)) == 0; > > } > > > > +/* Does address match the Bride Group multicast address. > > + * 01:80:c2:00:00:00 > > + */ > > +static inline int is_bridge_group(const unsigned char *dest) > > +{ > > + __be16 *a = (__be16 *)dest; > > + static const __be16 *b = (const __be16 *)br_group_address; > > + > > + return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) == 0; > > +} > > + > > /* > > * Called via br_handle_frame_hook. > > * Return NULL if skb is handled > > @@ -137,7 +148,8 @@ struct sk_buff *br_handle_frame(struct net_bridge_port *p, struct sk_buff *skb) > > if (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev, > > NULL, br_handle_local_finish)) > > return NULL; /* frame consumed by filter */ > > - else > > + /* Relay Bridge Group address is STP is off */ > > + if (!(p->br->stp_enabled == BR_NO_STP && is_bridge_group(dest))) > > return skb; /* continue processing */ > > } > > > > First, you should have used compare_ether_addr.Ah, right. Will fix.> Second, it needs to still obey state rules on port, so the return skb > should fall through to the switch statement.But is does, the return is executed when STP is on and has been there all the time. Jocke
Joakim Tjernlund
2009-May-12 17:34 UTC
[Bridge] [PATCH] bridge: Relay bridge multicast pkgs if !STP
Currently the bridge drops all STP pkgs when STP is off. This makes the bridge relay the Bridge Group multicast address, 01:80:c2:00:00:00, when STP is off. This allows for loop detection by other bridges doing STP. Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se> --- V2: - Use compare_ether_address() - Invert if logic, hopefully easier to read? net/bridge/br_input.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c index 30b8877..0bd175c 100644 --- a/net/bridge/br_input.c +++ b/net/bridge/br_input.c @@ -137,7 +137,9 @@ struct sk_buff *br_handle_frame(struct net_bridge_port *p, struct sk_buff *skb) if (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev, NULL, br_handle_local_finish)) return NULL; /* frame consumed by filter */ - else + /* Relay Bridge Group address if STP is off */ + if (p->br->stp_enabled != BR_NO_STP || + compare_ether_addr(dest, br_group_address)) return skb; /* continue processing */ } -- 1.6.2.3
Stephen Hemminger
2009-May-12 17:36 UTC
[Bridge] [PATCH] bridge: relay bridge multicast pkgs if !STP
On Tue, 12 May 2009 19:17:15 +0200 Joakim Tjernlund <joakim.tjernlund at transmode.se> wrote:> Stephen Hemminger <shemminger at vyatta.com> wrote on 12/05/2009 17:26:52: > > > > On Tue, 12 May 2009 16:02:14 +0200 > > Joakim Tjernlund <Joakim.Tjernlund at transmode.se> wrote: > > > > > Currently the bridge drops all STP pkgs when > > > STP is off. This makes the bridge relay the Bridge Group > > > multicast address, 01:80:c2:00:00:00, when STP is off. > > > This allows for loop detection by other bridges doing STP. > > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se> > > > --- > > > net/bridge/br_input.c | 14 +++++++++++++- > > > 1 files changed, 13 insertions(+), 1 deletions(-) > > > > > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > > > index 30b8877..8197823 100644 > > > --- a/net/bridge/br_input.c > > > +++ b/net/bridge/br_input.c > > > @@ -112,6 +112,17 @@ static inline int is_link_local(const unsigned char *dest) > > > return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | ((a[2] ^ b[2]) & m)) == 0; > > > } > > > > > > +/* Does address match the Bride Group multicast address. > > > + * 01:80:c2:00:00:00 > > > + */ > > > +static inline int is_bridge_group(const unsigned char *dest) > > > +{ > > > + __be16 *a = (__be16 *)dest; > > > + static const __be16 *b = (const __be16 *)br_group_address; > > > + > > > + return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) == 0; > > > +} > > > + > > > /* > > > * Called via br_handle_frame_hook. > > > * Return NULL if skb is handled > > > @@ -137,7 +148,8 @@ struct sk_buff *br_handle_frame(struct net_bridge_port *p, struct sk_buff *skb) > > > if (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev, > > > NULL, br_handle_local_finish)) > > > return NULL; /* frame consumed by filter */ > > > - else > > > + /* Relay Bridge Group address is STP is off */ > > > + if (!(p->br->stp_enabled == BR_NO_STP && is_bridge_group(dest))) > > > return skb; /* continue processing */ > > > } > > > > > > > First, you should have used compare_ether_addr. > > Ah, right. Will fix. > > > Second, it needs to still obey state rules on port, so the return skb > > should fall through to the switch statement. > > But is does, the return is executed when STP is on and has been there > all the time. > > Jocke >If STP packet arrives on a disabled or learning port, it should not be forwarded. --
Joakim Tjernlund
2009-May-13 15:29 UTC
[Bridge] [PATCH] bridge: relay bridge multicast pkgs if !STP
Joakim Tjernlund/Transmode wrote on 12/05/2009 19:46:21:> > Stephen Hemminger <shemminger at vyatta.com> wrote on 12/05/2009 19:36:27: > > > > On Tue, 12 May 2009 19:17:15 +0200 > > Joakim Tjernlund <joakim.tjernlund at transmode.se> wrote: > > > > > Stephen Hemminger <shemminger at vyatta.com> wrote on 12/05/2009 17:26:52: > > > > > > > > On Tue, 12 May 2009 16:02:14 +0200 > > > > Joakim Tjernlund <Joakim.Tjernlund at transmode.se> wrote: > > > > > > > > > Currently the bridge drops all STP pkgs when > > > > > STP is off. This makes the bridge relay the Bridge Group > > > > > multicast address, 01:80:c2:00:00:00, when STP is off. > > > > > This allows for loop detection by other bridges doing STP. > > > > > > > > > > Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund at transmode.se> > > > > > --- > > > > > net/bridge/br_input.c | 14 +++++++++++++- > > > > > 1 files changed, 13 insertions(+), 1 deletions(-) > > > > > > > > > > diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c > > > > > index 30b8877..8197823 100644 > > > > > --- a/net/bridge/br_input.c > > > > > +++ b/net/bridge/br_input.c > > > > > @@ -112,6 +112,17 @@ static inline int is_link_local(const unsigned char *dest) > > > > > return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | ((a[2] ^ b[2]) & m)) == 0; > > > > > } > > > > > > > > > > +/* Does address match the Bride Group multicast address. > > > > > + * 01:80:c2:00:00:00 > > > > > + */ > > > > > +static inline int is_bridge_group(const unsigned char *dest) > > > > > +{ > > > > > + __be16 *a = (__be16 *)dest; > > > > > + static const __be16 *b = (const __be16 *)br_group_address; > > > > > + > > > > > + return ((a[0] ^ b[0]) | (a[1] ^ b[1]) | (a[2] ^ b[2])) == 0; > > > > > +} > > > > > + > > > > > /* > > > > > * Called via br_handle_frame_hook. > > > > > * Return NULL if skb is handled > > > > > @@ -137,7 +148,8 @@ struct sk_buff *br_handle_frame(struct net_bridge_port *p, struct sk_buff *skb) > > > > > if (NF_HOOK(PF_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev, > > > > > NULL, br_handle_local_finish)) > > > > > return NULL; /* frame consumed by filter */ > > > > > - else > > > > > + /* Relay Bridge Group address is STP is off */ > > > > > + if (!(p->br->stp_enabled == BR_NO_STP && is_bridge_group(dest))) > > > > > return skb; /* continue processing */ > > > > > } > > > > > > > > > > > > > First, you should have used compare_ether_addr. > > > > > > Ah, right. Will fix. > > > > > > > Second, it needs to still obey state rules on port, so the return skb > > > > should fall through to the switch statement. > > > > > > But is does, the return is executed when STP is on and has been there > > > all the time. > > > > > > Jocke > > > > > > > If STP packet arrives on a disabled or learning port, it should not > > be forwarded. > hmm, should this particular multicast pkg be handled different from other multicast pkgs? > I don't see why/how. My patch treats the pkg like any other and fall > down to the switch stmt where br_hande_frame_finish will deal with it(drop it > if in LEARNING)Ping? Did I loose you here or did you see what I meant? Jocke