[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4ECA8D50.9080603@intel.com>
Date: Mon, 21 Nov 2011 09:41:36 -0800
From: Greg Rose <gregory.v.rose@...el.com>
To: Ben Hutchings <bhutchings@...arflare.com>
CC: Roopa Prabhu <roprabhu@...co.com>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"chrisw@...hat.com" <chrisw@...hat.com>,
"sri@...ibm.com" <sri@...ibm.com>,
"dragos.tatulea@...il.com" <dragos.tatulea@...il.com>,
"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
"arnd@...db.de" <arnd@...db.de>, "mst@...hat.com" <mst@...hat.com>,
"mchan@...adcom.com" <mchan@...adcom.com>,
"dwang2@...co.com" <dwang2@...co.com>,
"shemminger@...tta.com" <shemminger@...tta.com>,
"eric.dumazet@...il.com" <eric.dumazet@...il.com>,
"kaber@...sh.net" <kaber@...sh.net>,
"benve@...co.com" <benve@...co.com>
Subject: Re: [net-next-2.6 PATCH 0/6 v4] macvlan: MAC Address filtering support
for passthru mode
On 11/18/2011 9:40 AM, Ben Hutchings wrote:
> On Fri, 2011-11-18 at 08:58 -0800, Greg Rose wrote:
>> On 11/17/2011 4:44 PM, Ben Hutchings wrote:
>>> On Thu, 2011-11-17 at 16:32 -0800, Greg Rose wrote:
>>>> On 11/17/2011 4:15 PM, Ben Hutchings wrote:
>>>>> Sorry to come to this rather late.
>>>>>
>>>>> On Tue, 2011-11-08 at 23:55 -0800, Roopa Prabhu wrote:
>>>>> [...]
>>>>>> v2 -> v3
>>>>>> - Moved set and get filter ops from rtnl_link_ops to netdev_ops
>>>>>> - Support for SRIOV VFs.
>>>>>> [Note: The get filters msg (in the way current get rtnetlink handles
>>>>>> it) might get too big for SRIOV vfs. This patch follows existing sriov
>>>>>> vf get code and tries to accomodate filters for all VF's in a PF.
>>>>>> And for the SRIOV case I have only tested the fact that the VF
>>>>>> arguments are getting delivered to rtnetlink correctly. The code
>>>>>> follows existing sriov vf handling code so rest of it should work fine]
>>>>> [...]
>>>>>
>>>>> This is already broken for large numbers of VFs, and increasing the
>>>>> amount of information per VF is going to make the situation worse. I am
>>>>> no netlink expert but I think that the current approach of bundling all
>>>>> information about an interface in a single message may not be
>>>>> sustainable.
>>>>>
>>>>> Also, I'm unclear on why this interface is to be used to set filtering
>>>>> for the (PF) net device as well as for related VFs. Doesn't that
>>>>> duplicate the functionality of ndo_set_rx_mode and
>>>>> ndo_vlan_rx_{add,kill}_vid?
>>>>
>>>> Functionally yes but contextually no. This allows the PF driver to know
>>>> that it is setting these filters in the context of the existence of VFs,
>>>> allowing it to take appropriate action. The other two functions may be
>>>> called without the presence of SR-IOV enablement and the existence of VFs.
>>>>
>>>> Anyway, that's why I asked Roopa to add that capability.
>>>
>>> I don't follow. The PF driver already knows whether it has enabled VFs.
>>>
>>> How do filters set this way interact with filters set through the
>>> existing operations? Should they override promiscuous mode? None of
>>> this has been specified.
>>
>> Promiscuous mode is exactly the issue this feature is intended for. I'm
>> not familiar with the solarflare device but Intel HW promiscuous mode is
>> only promiscuous on the physical port, not on the VEB. So a packet sent
>> from a VF will not be captured by the PF across the VEB unless the MAC
>> and VLAN filters have been programmed into the HW.
>
> Yes, I get it. The hardware bridge needs to know more about the address
> configuration on the host than the driver is getting at the moment.
>
>> So you may not need
>> the feature for your devices but it is required for Intel devices.
>
> Well we don't have the hardware bridge but that means each VF driver
> needs to know whether to fall back to the software bridge. The net
> driver needs much the same additional information.
>
>> And
>> it's a fairly simple request, just allow -1 to indicate that the target
>> of the filter requests is for the PF itself. Using the already existing
>> set_rx_mode function wont' work because the PF driver will look at it
>> and figure it's in promiscuous mode anyway, so it won't set the filters
>> into the HW. At least that is how it is in the case of our HW and
>> driver. Again, the behavior of your HW and driver is unknown to me and
>> thus you may not require this feature.
>
> What concerns me is that this seems to be a workaround rather than a fix
> for over-use of promiscuous mode, and it changes the semantics of
> filtering modes in ways that haven't been well-specified.
I feel the opposite is true. It allows a known set of receive filters
so that you don't have to use promiscuous mode, which cuts down on
overhead from processing packets the upper layer stack isn't really
interested in.
>
> What if there's a software bridge between two net devices corresponding
> to separate physical ports, so that they really need to be promiscuous?
> What if the administrator runs tcpdump and really wants the (PF) net
> device to be promiscuous?
I don't believe there is anything in this patch set that removes
promiscuous mode operation as it is commonly used. Perhaps I've missed
something.
>
> These cases shouldn't break because of VF acceleration.
I don't see how they would in the case of Intel HW. And these new ops
to set rx filters are something that Intel HW needs because it
implements a VEB that is *not* a learning bridge and we don't want it to
be a learning bridge because of security concerns. It is better for our
requirements to be allowed to set the MAC/VLAN filters that we want a VF
to be able to see.
> Or at least we
> should make a conscious and documented decision that 'promiscuous'
> doesn't mean that if you enable it on your network adapter.
Again, I don't know of any plans to change anything relating to putting
a device in promiscuous mode or changing the semantics of what
promiscuous mode means.
- Greg
--
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
Powered by blists - more mailing lists