Subject: Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan mode

From: "Michael S. Tsirkin" <[email-masked]>
Date: Tue, 10 Apr 2012 16:43:22 +0300

On Tue, Apr 10, 2012 at 06:27:32AM -0700, John Fastabend wrote:
> On 4/10/2012 1:09 AM, Michael S. Tsirkin wrote:
> > On Mon, Apr 09, 2012 at 03:00:54PM -0700, John Fastabend wrote:
> >> This adds a new macvlan mode MACVLAN_PASSTHRU_NOPROMISC
> >> this mode acts the same as the original passthru mode _except_
> >> it does not set promiscuous mode on the lowerdev. Because the
> >> lowerdev is not put in promiscuous mode any unicast or multicast
> >> addresses the device should receive must be explicitely added
> >> with the FDB bridge ops. In many use cases the management stack
> >> will know the mac addresses needed (maybe negotiated via EVB/VDP)
> >> or may require only receiving known "good" mac addresses. This
> >> mode with the FDB ops supports this usage model.
> > 
> > 
> > Looks good to me. Some questions below:
> > 
> >> This patch is a result of Roopa Prabhu's work. Follow up
> >> patches are needed for VEPA and VEB macvlan modes.
> > 
> > And bridge too?
> > 
> 
> Yes I called this mode VEB here but this is defined in if_link.h
> as IFLA_MACVLAN_MODE_BRIDGE. From a IEEE point of view I think
> the macvlan bridge mode acts more like a 802.1Q VEB then a 802.1d
> bridge.

grep didn't find IFLA_MACVLAN_MODE_BRIDGE - which kernel
are you looking at?

> > Also, my understanding is that other modes won't need a flag
> > like this since they don't put the device in promisc mode initially,
> > so no assumptions are broken if we require all addresses
> > to be declared, right?
> > 
> 
> correct. But requires extra work to the hash table so the forwarding
> works correctly.
> 
> > A final question: I think we'll later add a macvlan mode
> > that does not flood all multicasts. This would change behaviour
> > in an incompatible way so we'll probably need yet another
> > flag. Would it make sense to combine this functionality
> > with nopromisc so we have less modes to support?
> > 
> 
> For VEPA and bridge modes this makes sense to me.

Hmm okay, but this would mean we should convert
MACVLAN_MODE_PASSTHRU_NOPROMISC to something
that can combined with all modes. E.g.
MACVLAN_MODE_BRIDGE | MACVLAN_MODE_FLAG_XXXXX

and document that it does not promise to flood
multicast.

> If you want
> the flood behavior you can create it by adding the addr to all
> the devices or just to a subset of them to get the non-flooding
> capabilities.
> 
> .John

BTW we seem to try to flood in pass-through too, not sure why.

-- 
MST