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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4F843AB2.5060901@intel.com>
Date:	Tue, 10 Apr 2012 06:50:42 -0700
From:	John Fastabend <john.r.fastabend@...el.com>
To:	"Michael S. Tsirkin" <mst@...hat.com>
CC:	roprabhu@...co.com, stephen.hemminger@...tta.com,
	davem@...emloft.net, hadi@...erus.ca, bhutchings@...arflare.com,
	jeffrey.t.kirsher@...el.com, netdev@...r.kernel.org,
	gregory.v.rose@...el.com, krkumar2@...ibm.com, sri@...ibm.com
Subject: Re: [net-next PATCH v1 7/7] macvlan: add FDB bridge ops and new macvlan
 mode

On 4/10/2012 1:14 AM, Michael S. Tsirkin wrote:
> On Tue, Apr 10, 2012 at 11:09:16AM +0300, 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?
>>
>> 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?
>>
>> 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?
> 
> One other question I forgot:
> 

[...]

>>>  
>>> @@ -344,12 +346,15 @@ static int macvlan_stop(struct net_device *dev)
>>>  	struct macvlan_dev *vlan = netdev_priv(dev);
>>>  	struct net_device *lowerdev = vlan->lowerdev;
>>>  
>>> +	dev_uc_unsync(lowerdev, dev);
>>> +	dev_mc_unsync(lowerdev, dev);
>>> +
>>>  	if (vlan->port->passthru) {
>>> -		dev_set_promiscuity(lowerdev, -1);
>>> +		if (vlan->mode == MACVLAN_MODE_PASSTHRU)
>>> +			dev_set_promiscuity(lowerdev, 1);
>>>  		goto hash_del;
>>>  	}
>>>  
>>> -	dev_mc_unsync(lowerdev, dev);
>>>  	if (dev->flags & IFF_ALLMULTI)
>>>  		dev_set_allmulti(lowerdev, -1);
>>>  
>>> @@ -399,10 +404,11 @@ static void macvlan_change_rx_flags(struct net_device *dev, int change)
>>>  		dev_set_allmulti(lowerdev, dev->flags & IFF_ALLMULTI ? 1 : -1);
> 
> In the new mode, do we want to have promisc on lowerdev follow whatever
> is set on the macvlan, like we do for allmulti?
> I'm not sure at this point - what do others think?
> 

Just to enumerate why you would need this: (1) socket set with
PACKET_MR_MULTICAST and (2) something like mrouted is running
on the macvlan (3) maybe some case I missed?

Don't you need CAP_NET_RAW to set these though anyways? So I
wouldn't think it would be a problem. I assume if a user has
CAP_NET_RAW or UUID 0 they really should be able to set this
up.

.John
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html