diff mbox

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

Message ID 20110705223551.5e1053af@nehalam.ftrdhcpuser.net
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

stephen hemminger July 6, 2011, 5:35 a.m. UTC 
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

Comments

Herbert Xu July 6, 2011, 5:45 a.m. UTC | #1 
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,
diff mbox

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) ||