Re: [Bridge] [PATCH] bridge: relay bridge multicast pkgs if !STP

[Stephen Hemminger <shemminger@vyatta.com>]

On Tue, 12 May 2009 19:17:15 +0200
Joakim Tjernlund <joakim.tjernlund@transmode.se> wrote:

> Stephen Hemminger <shemminger@vyatta.com> wrote on 12/05/2009 17:26:52:
> >
> > On Tue, 12 May 2009 16:02:14 +0200
> > Joakim Tjernlund <Joakim.Tjernlund@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@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.

--