Message-ID: <577446FE.7060402@gmail.com>
Date:	Wed, 29 Jun 2016 15:09:02 -0700
From:	John Fastabend <john.fastabend@...il.com>
To:	Or Gerlitz <gerlitz.or@...il.com>
Cc:	Or Gerlitz <ogerlitz@...lanox.com>, Jiri Pirko <jiri@...nulli.us>,
	"Samudrala, Sridhar" <sridhar.samudrala@...el.com>,
	Saeed Mahameed <saeedm@...lanox.com>,
	"David S. Miller" <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Hadar Hen-Zion <hadarh@...lanox.com>,
	Jiri Pirko <jiri@...lanox.com>,
	Andy Gospodarek <gospo@...ulusnetworks.com>,
	Jesse Brandeburg <jesse.brandeburg@...el.com>,
	John Fastabend <john.r.fastabend@...el.com>,
	Ido Schimmel <idosch@...lanox.com>,
	Tal Anker <Ankertal@...lanox.com>
Subject: Re: [PATCH net-next 08/16] net/devlink: Add E-Switch mode control

On 16-06-29 02:33 PM, Or Gerlitz wrote:
> On Wed, Jun 29, 2016 at 7:35 PM, John Fastabend
> <john.fastabend@...il.com> wrote:
>> On 16-06-29 07:48 AM, Or Gerlitz wrote:
>>> On 6/28/2016 10:31 PM, John Fastabend wrote:
>>>> On 16-06-28 12:12 PM, Jiri Pirko wrote:
> 
>>>>> Why?! Please, leave legacy be legacy. Use the new mode for
>>>>> implementing new features. Don't make things any more complicated :(
> 
> [...]
>>>> Maybe I'm reading to much into the devlink flag names and if instead
>>>> you use a switch like the following,
> 
>>>>    VF representer : enable/disable the creation VF netdev's to represent
>>>>                     the virtual functions on the PF
> 
>>>> Much less complicated then magic switching between forwarding logic IMO
>>>> and you don't whack a default configuration that an entire stack (e.g.
>>>> libvirt) has been built to use.
> 
>>> Re letting the user to observe/modify the rules added by the
>>> driver/firmware while legacy mode. Even if possible with bridge/fdb, it
>>> will be really pragmatical and doesn't make sense to get that donefor
>>> the TC subsystem. So this isn't a well defined solution and anyway, as
>>> you said, legacy mode enhancements is a different exercise. Personally,
>>> I agree with Jiri, that we should legacy be legacyand focus on adding
>>> the new model.
> 
>> The ixgbe driver already supports bridge and tc commands without the VF
>> representer.  Adding the VF representer to these drivers just extends
>> the existing support so we have an identifier for VFs and now the
>> redirect action works and the fdb commands can specify the VF netdevs.
>> I don't see this as a problem because we already do it today with
>> 'ip' and bridge tools.
> 
> To be precise, for both ixgbe and mlx5, the existing tc support
> (u32/ixgbe, flower/mlx5) is not for switching functionality but rather
> for NIC-ish one, e.g drop, mark, etc. Indeed in ixgbe you added
> redirect to VF, but this is only for south --> north (wire --> VF)
> traffic, w.o the VF rep you can't do the other way around.
> 

Correct which is why we need the VF rep. So we are completely in
sync there.

> Just to clarify, to what exact bridge command support did you refer for ixgbe?

'bridge fdb' commands are supported today on the PF. But its the
same story as above we need the VF rep to also use it on the
VF representer

Also 'bridge link' command for veb/vepa modes is supported and the
other link attributes could be supported with additional driver
support. No need for core changes here. But again yes only on the
PF so again we need the VF reps.

> 
> The forwarding done in the legacy mode is not well defined, and
> different across vendors, adding there the VF reps will not make it
> any better b/c some steering rules will be set by tc/bridge offloads
> while other rules will be put by the driver.
> I don't see how this takes us to better place.

In legacy mode or any other mode you are defining some default policy
and rules.

In the legacy mode we use mac/vlan assigned l2 forwarding entries in the
hardware fdb which are seen when you query 'ip link' and 'bridge fdb'
today. And similarly can be modified today using 'ip link' and 'bridge
fdb' at least on the intel devices. Its not undefined in any way with
a quick query of the tools we can learn exactly what the configuration
is and even change it. This works fairly well with existing controllers
and stacks.

The limitations are 'ip' only supports a single MAC address per VF and
'tc' doesn't work on VF ports because when the VF is assigned to a VM
or namespace we lose visibility of it. Providing a VF rep for this
solves both of those problems.

In this new mode the default policy is to create a default miss rule
and implement no l2 forwarding rules. Unfortunately not all hardware
in use supports this default miss rule case but would still benefit
from having a VF rep. So we shouldn't make this a stipulation for
enabling VF reps. It also changes a default policy that has been in
place for years without IMO at least any compelling reason. It will
be easy enough to change the default l2 policy to a flow based model
with a few bridge/tc commands.

> 
>> We are also slightly in disagreement about what the default should be
>> with VF netdevs. I think the default should be the same L2 mac/vlan
>> switch behavior and see no reason to change it by default just because
>> we added VF netdevs. The infrastructure libvirt/openstack/etc are built
>> around this default today. But I guess nothing in this series specifies
>> what the defaults of any given driver will be. VF netdevs are still
>> useful even on older hardware that only supports mac/vlan forwarding to
>> expose statistics and send/receive control frames such as lldp.
> 
> Again, this is not about default engineering... and using the VF reps
> (not VF netdevs) in legacy mode only make it more cryptic to my
> opinion. I agree some changes would be needed in openstack to support
> the new model, but this is how progress is made... you can't always
> make all layer above you unchanged. Note that the VF reps behave the
> same as tap devices (v-switch doing xmit on tap --> recv in VM, VM
> sends --> recv on tap into the v-switch), so the change in open-stack
> would not be that big.
> 

But in this case we have no reason to break the stack above us. The
currently deployed usage is L2 mac/vlan. As soon as you bind a vSwitch
or whatever mgmt agent to the device it can go ahead and manage the
switch putting it in the correct mode using the tooling in 'bridge' and
'tc'.


> [...]
> 
>> Why I think the VF representer is a per port ethtool flag and not a
>> devlink option is my use case might be to assign a PF into a VM or
>> namespace where I don't want VF netdevs.
> 
> again, we think the correct place to set how the eswitch is managed is
> through eswitch manager PCI devices and not net devices and hence
> ethtool is not the way to go.
> 
> Also, how do you want your e-switch to be managed in this case?
> 

In the case where I don't create vf netdevs on one of the PFs I'll
manage the forwarding tables via the existing mechanisms 'ip' and
'bridge'. However its likely not a big deal because 'ip' and 'bridge'
will continue to work even if VF reps are around. The ethtool/devlink
comment was more about pointing out that creating VFs does not
require you to manage your switch any differently. Its useful even on
devices that can't support flow based forwarding for statistics and
setting port attributes like mtu, etc.

.John

Powered by blists - more mailing lists