Patchwork [BUG] bd4265fe36 bridge: Only flood unreg groups... breaks DHCP setup

login
register
mail settings
Submitter stephen hemminger
Date July 6, 2011, 5:35 a.m.
Message ID <20110705223551.5e1053af@nehalam.ftrdhcpuser.net>
Download mbox | patch
Permalink /patch/103415/
State RFC
Delegated to: David Miller
Headers show

Comments

stephen hemminger - July 6, 2011, 5:35 a.m.
On Wed, 6 Jul 2011 13:08:48 +0800
Herbert Xu <herbert@gondor.apana.org.au> wrote:

> On Tue, Jul 05, 2011 at 10:06:36PM -0700, Stephen Hemminger wrote:
> > On Tue, 05 Jul 2011 18:40:44 -0700 (PDT)
> > David Miller <davem@davemloft.net> wrote:
> > 
> > > From: Herbert Xu <herbert@gondor.hengli.com.au>
> > > Date: Wed, 6 Jul 2011 07:58:33 +0800
> > > 
> > > > bridge: Always flood broadcast packets
> > > > 
> > > > As is_multicast_ether_addr returns true on broadcast packets as
> > > > well, we need to explicitly exclude broadcast packets so that
> > > > they're always flooded.  This wasn't an issue before as broadcast
> > > > packets were considered to be an unregistered multicast group,
> > > > which were always flooded.  However, as we now only flood such
> > > > packets to router ports, this is no longer acceptable.
> > > > 
> > > > Reported-by: Michael Guntsche <mike@it-loops.com>
> > > > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> > > 
> > > Applied.
> > 
> > Obviously needs to go to stable as well.
> 
> I don't think the buggy patch has made it to a release kernel
> yet.
> 
> Thanks,

The bisected commit bd4265fe36 is in 3.0-rc4 but the input code
path still treats multicast and broadcast the same which means
there are some other possible cases where broadcast doesn't get
forwarded.

Wouldn't it make more sense to force input path to always forward
broadcasts. It would also save a lookup of mdb entry.


--
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
Herbert Xu - July 6, 2011, 5:45 a.m.
On Tue, Jul 05, 2011 at 10:35:51PM -0700, Stephen Hemminger wrote:
>
> The bisected commit bd4265fe36 is in 3.0-rc4 but the input code
> path still treats multicast and broadcast the same which means
> there are some other possible cases where broadcast doesn't get
> forwarded.

AFAIK it worked properly prior to bd4265fe36 because broadcast
addresses simply get treated as an unregistered multicast group
and ends up being flooded.

So while yes we do do an extra mdb lookup I don't think there is
any visible problem in stable.

Cheers,

Patch

--- a/net/bridge/br_input.c	2011-07-05 22:28:30.111995701 -0700
+++ b/net/bridge/br_input.c	2011-07-05 22:34:08.259995671 -0700
@@ -77,7 +77,9 @@  int br_handle_frame_finish(struct sk_buf
 
 	dst = NULL;
 
-	if (is_multicast_ether_addr(dest)) {
+	if (is_broadcast_ether_addr(dest))
+		skb2 = skb;
+	else if (is_multicast_ether_addr(dest)) {
 		mdst = br_mdb_get(br, skb);
 		if (mdst || BR_INPUT_SKB_CB_MROUTERS_ONLY(skb)) {
 			if ((mdst && mdst->mglist) ||