[PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[roopa]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This is a generic high level feature flag for all switch asic features today.

switch drivers set this flag on switch ports. Logical devices like
bridge, bonds, vxlans can inherit this flag from their slaves/ports.

I had to use SWITCH in the name to avoid ambiguity with other feature
flags. But, since i have been harping about not calling it 'switch',
I am welcome to any suggestions :)

An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
in net_device_flags.
---
 include/linux/netdev_features.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
index 8e30685..68db1de 100644
--- a/include/linux/netdev_features.h
+++ b/include/linux/netdev_features.h
@@ -66,6 +66,7 @@ enum {
 	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
 	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
 	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
+	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
 
 	/*
 	 * Add your fresh new feature above and remember to update
@@ -124,6 +125,7 @@ enum {
 #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
 #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
 #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
+#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
 
 /* Features valid for ethtool to change */
 /* = all defined minus driver/device-class-related */
-- 
1.7.10.4

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Jianhua Xie]

在 2014年12月05日 10:26, roopa@cumulusnetworks.com 写道:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This is a generic high level feature flag for all switch asic features today.
>
> switch drivers set this flag on switch ports. Logical devices like
> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>
> I had to use SWITCH in the name to avoid ambiguity with other feature
> flags. But, since i have been harping about not calling it 'switch',
> I am welcome to any suggestions :)
>
> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
> in net_device_flags.
> ---
>   include/linux/netdev_features.h |    2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 8e30685..68db1de 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -66,6 +66,7 @@ enum {
>   	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>   	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
>   	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
> +	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
I am interested in this flag very much, but I am not very clear
how many offload capabilities does this flag imply.  If this flag
belongs to a general flag and can be accepted by all vendors,
I will reuse this flag to introduce another out going data traffics
distribution offload method to bonding driver.

Thanks & B.R.
Jianhua
>   
>   	/*
>   	 * Add your fresh new feature above and remember to update
> @@ -124,6 +125,7 @@ enum {
>   #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
>   #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
>   #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
> +#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
>   
>   /* Features valid for ethtool to change */
>   /* = all defined minus driver/device-class-related */

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Roopa Prabhu]

On 12/4/14, 7:21 PM, Jianhua Xie wrote:
>
> 在 2014年12月05日 10:26, roopa@cumulusnetworks.com 写道:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This is a generic high level feature flag for all switch asic 
>> features today.
>>
>> switch drivers set this flag on switch ports. Logical devices like
>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>
>> I had to use SWITCH in the name to avoid ambiguity with other feature
>> flags. But, since i have been harping about not calling it 'switch',
>> I am welcome to any suggestions :)
>>
>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>> in net_device_flags.
>> ---
>>   include/linux/netdev_features.h |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/netdev_features.h 
>> b/include/linux/netdev_features.h
>> index 8e30685..68db1de 100644
>> --- a/include/linux/netdev_features.h
>> +++ b/include/linux/netdev_features.h
>> @@ -66,6 +66,7 @@ enum {
>>       NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN 
>> STAGs */
>>       NETIF_F_HW_L2FW_DOFFLOAD_BIT,    /* Allow L2 Forwarding in 
>> Hardware */
>>       NETIF_F_BUSY_POLL_BIT,        /* Busy poll */
>> +    NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
> I am interested in this flag very much, but I am not very clear
> how many offload capabilities does this flag imply.  If this flag
> belongs to a general flag and can be accepted by all vendors,
> I will reuse this flag to introduce another out going data traffics
> distribution offload method to bonding driver.
Right now its a one global offload flag for switch asics. Some may not 
want to tune it more. But others may and so i do expect more flags in 
this category.

Thanks,
Roopa

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Jianhua Xie]

在 2014年12月05日 12:17, Roopa Prabhu 写道:
> On 12/4/14, 7:21 PM, Jianhua Xie wrote:
>>
>> 在 2014年12月05日 10:26, roopa@cumulusnetworks.com 写道:
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This is a generic high level feature flag for all switch asic 
>>> features today.
>>>
>>> switch drivers set this flag on switch ports. Logical devices like
>>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>>
>>> I had to use SWITCH in the name to avoid ambiguity with other feature
>>> flags. But, since i have been harping about not calling it 'switch',
>>> I am welcome to any suggestions :)
>>>
>>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>>> in net_device_flags.
>>> ---
>>>   include/linux/netdev_features.h |    2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/linux/netdev_features.h 
>>> b/include/linux/netdev_features.h
>>> index 8e30685..68db1de 100644
>>> --- a/include/linux/netdev_features.h
>>> +++ b/include/linux/netdev_features.h
>>> @@ -66,6 +66,7 @@ enum {
>>>       NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN 
>>> STAGs */
>>>       NETIF_F_HW_L2FW_DOFFLOAD_BIT,    /* Allow L2 Forwarding in 
>>> Hardware */
>>>       NETIF_F_BUSY_POLL_BIT,        /* Busy poll */
>>> +    NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
>> I am interested in this flag very much, but I am not very clear
>> how many offload capabilities does this flag imply.  If this flag
>> belongs to a general flag and can be accepted by all vendors,
>> I will reuse this flag to introduce another out going data traffics
>> distribution offload method to bonding driver.
> Right now its a one global offload flag for switch asics. Some may not 
> want to tune it more. But others may and so i do expect more flags in 
> this category.
>
Got it with thanks.
Best Regards.
> Thanks,
> Roopa
>

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Scott Feldman]

On Thu, Dec 4, 2014 at 6:26 PM,  <roopa@cumulusnetworks.com> wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This is a generic high level feature flag for all switch asic features today.
>
> switch drivers set this flag on switch ports. Logical devices like
> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>
> I had to use SWITCH in the name to avoid ambiguity with other feature
> flags. But, since i have been harping about not calling it 'switch',
> I am welcome to any suggestions :)
>
> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
> in net_device_flags.
> ---
>  include/linux/netdev_features.h |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 8e30685..68db1de 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -66,6 +66,7 @@ enum {
>         NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>         NETIF_F_HW_L2FW_DOFFLOAD_BIT,   /* Allow L2 Forwarding in Hardware */
>         NETIF_F_BUSY_POLL_BIT,          /* Busy poll */
> +       NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
>
>         /*
>          * Add your fresh new feature above and remember to update
> @@ -124,6 +125,7 @@ enum {
>  #define NETIF_F_HW_VLAN_STAG_TX        __NETIF_F(HW_VLAN_STAG_TX)
>  #define NETIF_F_HW_L2FW_DOFFLOAD       __NETIF_F(HW_L2FW_DOFFLOAD)
>  #define NETIF_F_BUSY_POLL      __NETIF_F(BUSY_POLL)
> +#define NETIF_F_HW_SWITCH_OFFLOAD      __NETIF_F(HW_SWITCH_OFFLOAD)

Can we use existing flag NETIF_F_HW_L2FW_DOFFLOAD?  The comment above
on that flag suggests it would be perfect for what we're doing with L2
bridging: offloading the L2 forwarding path in HW.

NETIF_F_HW_L2FW_DOFFLOAD seems to be orphaned in current net-next, so
I'm not sure of it's status, but see if it makes sense to recycle it.

I could see a NETIF_F_HW_L3FW_DOFFLOAD for L3 fwd offload, down the road.


>  /* Features valid for ethtool to change */
>  /* = all defined minus driver/device-class-related */
> --
> 1.7.10.4
>

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[John Fastabend]

On 12/04/2014 10:08 PM, Scott Feldman wrote:
> On Thu, Dec 4, 2014 at 6:26 PM,  <roopa@cumulusnetworks.com> wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This is a generic high level feature flag for all switch asic features today.
>>
>> switch drivers set this flag on switch ports. Logical devices like
>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>
>> I had to use SWITCH in the name to avoid ambiguity with other feature
>> flags. But, since i have been harping about not calling it 'switch',
>> I am welcome to any suggestions :)
>>
>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>> in net_device_flags.
>> ---
>>   include/linux/netdev_features.h |    2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>> index 8e30685..68db1de 100644
>> --- a/include/linux/netdev_features.h
>> +++ b/include/linux/netdev_features.h
>> @@ -66,6 +66,7 @@ enum {
>>          NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>>          NETIF_F_HW_L2FW_DOFFLOAD_BIT,   /* Allow L2 Forwarding in Hardware */
>>          NETIF_F_BUSY_POLL_BIT,          /* Busy poll */
>> +       NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
>>
>>          /*
>>           * Add your fresh new feature above and remember to update
>> @@ -124,6 +125,7 @@ enum {
>>   #define NETIF_F_HW_VLAN_STAG_TX        __NETIF_F(HW_VLAN_STAG_TX)
>>   #define NETIF_F_HW_L2FW_DOFFLOAD       __NETIF_F(HW_L2FW_DOFFLOAD)
>>   #define NETIF_F_BUSY_POLL      __NETIF_F(BUSY_POLL)
>> +#define NETIF_F_HW_SWITCH_OFFLOAD      __NETIF_F(HW_SWITCH_OFFLOAD)
>
> Can we use existing flag NETIF_F_HW_L2FW_DOFFLOAD?  The comment above
> on that flag suggests it would be perfect for what we're doing with L2
> bridging: offloading the L2 forwarding path in HW.
>
> NETIF_F_HW_L2FW_DOFFLOAD seems to be orphaned in current net-next, so
> I'm not sure of it's status, but see if it makes sense to recycle it.
>
> I could see a NETIF_F_HW_L3FW_DOFFLOAD for L3 fwd offload, down the road.
>

Its not orphaned its used by the macvlan device to indicate that the
port switching should be done in hardware. For example put macvlan in
VEPA mode and the hardware will deliver the packets directly to the
netdev and get the pruning right. In VEB mode the hardware will forward
packets correctly between macvlan netdevs. The implementation on ixgbe
for example is to give a set of hardware queues per macvlan netdev. I
still need to add offloaded QOS to make this more interesting.

I could support a l3vlan device as well even on some of the host nics.

The issue with generalizing it is at the moment it is used by ixgbe
which can't forward between two physical ports. It works on macvlan
because each macvlan netdev uses the same physical uplink. I think 
switching between different ports is going to require another feature
bit like its done here.

.John

>
>>   /* Features valid for ethtool to change */
>>   /* = all defined minus driver/device-class-related */
>> --
>> 1.7.10.4
>>


-- 
John Fastabend         Intel Corporation

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Scott Feldman]

On Thu, Dec 4, 2014 at 10:32 PM, John Fastabend
<john.fastabend@gmail.com> wrote:
> On 12/04/2014 10:08 PM, Scott Feldman wrote:
>>
>> On Thu, Dec 4, 2014 at 6:26 PM,  <roopa@cumulusnetworks.com> wrote:
>>>
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This is a generic high level feature flag for all switch asic features
>>> today.
>>>
>>> switch drivers set this flag on switch ports. Logical devices like
>>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>>
>>> I had to use SWITCH in the name to avoid ambiguity with other feature
>>> flags. But, since i have been harping about not calling it 'switch',
>>> I am welcome to any suggestions :)
>>>
>>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>>> in net_device_flags.
>>> ---
>>>   include/linux/netdev_features.h |    2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/linux/netdev_features.h
>>> b/include/linux/netdev_features.h
>>> index 8e30685..68db1de 100644
>>> --- a/include/linux/netdev_features.h
>>> +++ b/include/linux/netdev_features.h
>>> @@ -66,6 +66,7 @@ enum {
>>>          NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN
>>> STAGs */
>>>          NETIF_F_HW_L2FW_DOFFLOAD_BIT,   /* Allow L2 Forwarding in
>>> Hardware */
>>>          NETIF_F_BUSY_POLL_BIT,          /* Busy poll */
>>> +       NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
>>>
>>>          /*
>>>           * Add your fresh new feature above and remember to update
>>> @@ -124,6 +125,7 @@ enum {
>>>   #define NETIF_F_HW_VLAN_STAG_TX        __NETIF_F(HW_VLAN_STAG_TX)
>>>   #define NETIF_F_HW_L2FW_DOFFLOAD       __NETIF_F(HW_L2FW_DOFFLOAD)
>>>   #define NETIF_F_BUSY_POLL      __NETIF_F(BUSY_POLL)
>>> +#define NETIF_F_HW_SWITCH_OFFLOAD      __NETIF_F(HW_SWITCH_OFFLOAD)
>>
>>
>> Can we use existing flag NETIF_F_HW_L2FW_DOFFLOAD?  The comment above
>> on that flag suggests it would be perfect for what we're doing with L2
>> bridging: offloading the L2 forwarding path in HW.
>>
>> NETIF_F_HW_L2FW_DOFFLOAD seems to be orphaned in current net-next, so
>> I'm not sure of it's status, but see if it makes sense to recycle it.
>>
>> I could see a NETIF_F_HW_L3FW_DOFFLOAD for L3 fwd offload, down the road.
>>
>
> Its not orphaned its used by the macvlan device to indicate that the
> port switching should be done in hardware. For example put macvlan in
> VEPA mode and the hardware will deliver the packets directly to the
> netdev and get the pruning right. In VEB mode the hardware will forward
> packets correctly between macvlan netdevs. The implementation on ixgbe
> for example is to give a set of hardware queues per macvlan netdev. I
> still need to add offloaded QOS to make this more interesting.
>
> I could support a l3vlan device as well even on some of the host nics.
>
> The issue with generalizing it is at the moment it is used by ixgbe
> which can't forward between two physical ports. It works on macvlan
> because each macvlan netdev uses the same physical uplink. I think switching
> between different ports is going to require another feature
> bit like its done here.


My bad...I grepped for the _BIT variant and missed the usage in
macvlan, etc.  Sorry about that.

>
> .John
>
>
>>
>>>   /* Features valid for ethtool to change */
>>>   /* = all defined minus driver/device-class-related */
>>> --
>>> 1.7.10.4
>>>
>
>
> --
> John Fastabend         Intel Corporation

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Jiri Pirko]

Fri, Dec 05, 2014 at 03:26:39AM CET, roopa@cumulusnetworks.com wrote:
>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
>This is a generic high level feature flag for all switch asic features today.
>
>switch drivers set this flag on switch ports. Logical devices like
>bridge, bonds, vxlans can inherit this flag from their slaves/ports.


Can you please elaborate on how exactly would this inheritance look
like?


>
>I had to use SWITCH in the name to avoid ambiguity with other feature
>flags. But, since i have been harping about not calling it 'switch',
>I am welcome to any suggestions :)
>
>An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>in net_device_flags.
>---
> include/linux/netdev_features.h |    2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>index 8e30685..68db1de 100644
>--- a/include/linux/netdev_features.h
>+++ b/include/linux/netdev_features.h
>@@ -66,6 +66,7 @@ enum {
> 	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
> 	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
> 	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
>+	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
> 
> 	/*
> 	 * Add your fresh new feature above and remember to update
>@@ -124,6 +125,7 @@ enum {
> #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
> #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
> #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
>+#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
> 
> /* Features valid for ethtool to change */
> /* = all defined minus driver/device-class-related */
>-- 
>1.7.10.4
>

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Jamal Hadi Salim]

On 12/04/14 21:26, roopa@cumulusnetworks.com wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> This is a generic high level feature flag for all switch asic features today.
>
> switch drivers set this flag on switch ports. Logical devices like
> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>
> I had to use SWITCH in the name to avoid ambiguity with other feature
> flags. But, since i have been harping about not calling it 'switch',
> I am welcome to any suggestions :)
>

I know  in this use case it is a switch. But please dont use that term
for things that we are going to use as generic offload descriptors.
How about just simple: NETIF_F_HW_DEV_OFFLOAD_BIT
BTW: Didnt you already have a netdev specific offload feature flag?

cheers,
jamal

> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
> in net_device_flags.
> ---
>   include/linux/netdev_features.h |    2 ++
>   1 file changed, 2 insertions(+)
>
> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
> index 8e30685..68db1de 100644
> --- a/include/linux/netdev_features.h
> +++ b/include/linux/netdev_features.h
> @@ -66,6 +66,7 @@ enum {
>   	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>   	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
>   	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
> +	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
>
>   	/*
>   	 * Add your fresh new feature above and remember to update
> @@ -124,6 +125,7 @@ enum {
>   #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
>   #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
>   #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
> +#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
>
>   /* Features valid for ethtool to change */
>   /* = all defined minus driver/device-class-related */
>

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Jiri Pirko]

Fri, Dec 05, 2014 at 01:06:50PM CET, jhs@mojatatu.com wrote:
>On 12/04/14 21:26, roopa@cumulusnetworks.com wrote:
>>From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>>This is a generic high level feature flag for all switch asic features today.
>>
>>switch drivers set this flag on switch ports. Logical devices like
>>bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>
>>I had to use SWITCH in the name to avoid ambiguity with other feature
>>flags. But, since i have been harping about not calling it 'switch',
>>I am welcome to any suggestions :)
>>
>
>I know  in this use case it is a switch. But please dont use that term
>for things that we are going to use as generic offload descriptors.
>How about just simple: NETIF_F_HW_DEV_OFFLOAD_BIT

What that should tell it stands for? I think we need something more
specific.


>BTW: Didnt you already have a netdev specific offload feature flag?
>
>cheers,
>jamal
>
>>An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>>in net_device_flags.
>>---
>>  include/linux/netdev_features.h |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>>diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>>index 8e30685..68db1de 100644
>>--- a/include/linux/netdev_features.h
>>+++ b/include/linux/netdev_features.h
>>@@ -66,6 +66,7 @@ enum {
>>  	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>>  	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
>>  	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
>>+	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
>>
>>  	/*
>>  	 * Add your fresh new feature above and remember to update
>>@@ -124,6 +125,7 @@ enum {
>>  #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
>>  #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
>>  #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
>>+#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
>>
>>  /* Features valid for ethtool to change */
>>  /* = all defined minus driver/device-class-related */
>>
>

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Jamal Hadi Salim]

On 12/05/14 07:17, Jiri Pirko wrote:
> Fri, Dec 05, 2014 at 01:06:50PM CET, jhs@mojatatu.com wrote:
>> On 12/04/14 21:26, roopa@cumulusnetworks.com wrote:
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This is a generic high level feature flag for all switch asic features today.
>>>
>>> switch drivers set this flag on switch ports. Logical devices like
>>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>>
>>> I had to use SWITCH in the name to avoid ambiguity with other feature
>>> flags. But, since i have been harping about not calling it 'switch',
>>> I am welcome to any suggestions :)
>>>
>>
>> I know  in this use case it is a switch. But please dont use that term
>> for things that we are going to use as generic offload descriptors.
>> How about just simple: NETIF_F_HW_DEV_OFFLOAD_BIT
>
> What that should tell it stands for? I think we need something more
> specific.
>

Ok, the use case in this instance is very specific. My blinders
go off anytime i see switch or sw or swdev.
In which case maybe what Scott suggested but changing L2FWD
with FDB?

cheers,
jamal

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Roopa Prabhu]

On 12/4/14, 11:41 PM, Jiri Pirko wrote:
> Fri, Dec 05, 2014 at 03:26:39AM CET, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This is a generic high level feature flag for all switch asic features today.
>>
>> switch drivers set this flag on switch ports. Logical devices like
>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>
> Can you please elaborate on how exactly would this inheritance look
> like?

My thought there was, when a port with the hw offload flag is added to 
the bridge, the same flag gets set on the bridge. And, for any bridge 
attributes (not port attributes), this flag on the bridge can be used to 
offload those bridge attributes.
bridge attribute examples: IFLA_BR_FORWARD_DELAY, IFLA_BR_HELLO_TIME, 
IFLA_BR_MAX_AGE.
I don't think offloads for these are handled today. I was going to look 
at them as part of continued work on this.
The current patches only target bridge port attributes and the flag for 
this is already set by the port driver.

I believe netdev_update_features() takes care of the updating the flag 
on the bridge part. I plan to check on that.


>
>
>> I had to use SWITCH in the name to avoid ambiguity with other feature
>> flags. But, since i have been harping about not calling it 'switch',
>> I am welcome to any suggestions :)
>>
>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>> in net_device_flags.
>> ---
>> include/linux/netdev_features.h |    2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>> index 8e30685..68db1de 100644
>> --- a/include/linux/netdev_features.h
>> +++ b/include/linux/netdev_features.h
>> @@ -66,6 +66,7 @@ enum {
>> 	NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>> 	NETIF_F_HW_L2FW_DOFFLOAD_BIT,	/* Allow L2 Forwarding in Hardware */
>> 	NETIF_F_BUSY_POLL_BIT,		/* Busy poll */
>> +	NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
>>
>> 	/*
>> 	 * Add your fresh new feature above and remember to update
>> @@ -124,6 +125,7 @@ enum {
>> #define NETIF_F_HW_VLAN_STAG_TX	__NETIF_F(HW_VLAN_STAG_TX)
>> #define NETIF_F_HW_L2FW_DOFFLOAD	__NETIF_F(HW_L2FW_DOFFLOAD)
>> #define NETIF_F_BUSY_POLL	__NETIF_F(BUSY_POLL)
>> +#define NETIF_F_HW_SWITCH_OFFLOAD	__NETIF_F(HW_SWITCH_OFFLOAD)
>>
>> /* Features valid for ethtool to change */
>> /* = all defined minus driver/device-class-related */
>> -- 
>> 1.7.10.4
>>

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Scott Feldman]

On Fri, Dec 5, 2014 at 6:16 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
> On 12/4/14, 11:41 PM, Jiri Pirko wrote:
>>
>> Fri, Dec 05, 2014 at 03:26:39AM CET, roopa@cumulusnetworks.com wrote:
>>>
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This is a generic high level feature flag for all switch asic features
>>> today.
>>>
>>> switch drivers set this flag on switch ports. Logical devices like
>>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>
>>
>> Can you please elaborate on how exactly would this inheritance look
>> like?
>
>
> My thought there was, when a port with the hw offload flag is added to the
> bridge, the same flag gets set on the bridge. And, for any bridge attributes
> (not port attributes), this flag on the bridge can be used to offload those
> bridge attributes.
> bridge attribute examples: IFLA_BR_FORWARD_DELAY, IFLA_BR_HELLO_TIME,
> IFLA_BR_MAX_AGE.

Ah, wait, why do those need to be pushed down to driver/HW?  Letting
the bridge (or external process like mstpd) own the ctrl-plane means
HW isn't running STP machine or aging out FDB entries.  Let Linux take
care of that.

Same goes for bonding, since that was mentioned earlier.  Keep LACP
ctrl processing in the kernel/bonding driver, and there is no need to
push bonding settings down to port driver/hw.  driver/hw just need to
know port membership and LACP status.

> I don't think offloads for these are handled today. I was going to look at
> them as part of continued work on this.
> The current patches only target bridge port attributes and the flag for this
> is already set by the port driver.
>
> I believe netdev_update_features() takes care of the updating the flag on
> the bridge part. I plan to check on that.
>
>
>
>>
>>
>>> I had to use SWITCH in the name to avoid ambiguity with other feature
>>> flags. But, since i have been harping about not calling it 'switch',
>>> I am welcome to any suggestions :)
>>>
>>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>>> in net_device_flags.
>>> ---
>>> include/linux/netdev_features.h |    2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/include/linux/netdev_features.h
>>> b/include/linux/netdev_features.h
>>> index 8e30685..68db1de 100644
>>> --- a/include/linux/netdev_features.h
>>> +++ b/include/linux/netdev_features.h
>>> @@ -66,6 +66,7 @@ enum {
>>>         NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN
>>> STAGs */
>>>         NETIF_F_HW_L2FW_DOFFLOAD_BIT,   /* Allow L2 Forwarding in
>>> Hardware */
>>>         NETIF_F_BUSY_POLL_BIT,          /* Busy poll */
>>> +       NETIF_F_HW_SWITCH_OFFLOAD_BIT,  /* HW switch offload */
>>>
>>>         /*
>>>          * Add your fresh new feature above and remember to update
>>> @@ -124,6 +125,7 @@ enum {
>>> #define NETIF_F_HW_VLAN_STAG_TX __NETIF_F(HW_VLAN_STAG_TX)
>>> #define NETIF_F_HW_L2FW_DOFFLOAD        __NETIF_F(HW_L2FW_DOFFLOAD)
>>> #define NETIF_F_BUSY_POLL       __NETIF_F(BUSY_POLL)
>>> +#define NETIF_F_HW_SWITCH_OFFLOAD      __NETIF_F(HW_SWITCH_OFFLOAD)
>>>
>>> /* Features valid for ethtool to change */
>>> /* = all defined minus driver/device-class-related */
>>> --
>>> 1.7.10.4
>>>
>

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Roopa Prabhu]

On 12/5/14, 10:53 AM, Scott Feldman wrote:
> On Fri, Dec 5, 2014 at 6:16 AM, Roopa Prabhu <roopa@cumulusnetworks.com> wrote:
>> On 12/4/14, 11:41 PM, Jiri Pirko wrote:
>>> Fri, Dec 05, 2014 at 03:26:39AM CET, roopa@cumulusnetworks.com wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This is a generic high level feature flag for all switch asic features
>>>> today.
>>>>
>>>> switch drivers set this flag on switch ports. Logical devices like
>>>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>>
>>> Can you please elaborate on how exactly would this inheritance look
>>> like?
>>
>> My thought there was, when a port with the hw offload flag is added to the
>> bridge, the same flag gets set on the bridge. And, for any bridge attributes
>> (not port attributes), this flag on the bridge can be used to offload those
>> bridge attributes.
>> bridge attribute examples: IFLA_BR_FORWARD_DELAY, IFLA_BR_HELLO_TIME,
>> IFLA_BR_MAX_AGE.
> Ah, wait, why do those need to be pushed down to driver/HW?  Letting
> the bridge (or external process like mstpd) own the ctrl-plane means
> HW isn't running STP machine or aging out FDB entries.  Let Linux take
> care of that.
>
> Same goes for bonding, since that was mentioned earlier.  Keep LACP
> ctrl processing in the kernel/bonding driver, and there is no need to
> push bonding settings down to port driver/hw.  driver/hw just need to
> know port membership and LACP status.

scott, agreed these are stp attributes. Wont go into hardware. I picked 
the wrong example.
But, I was just trying to point out that there maybe bridge attributes 
that need to be passed to hw.
Like the bridge ageing timer etc. We don't use it today. But others may 
in the future.
I was just trying to say that ...this maybe useful in the future. The 
current in-kernel implementation
nor my patches do anything for this. We can add it in the future if need 
be.

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Thomas Graf]

On 12/04/14 at 06:26pm, roopa@cumulusnetworks.com wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> This is a generic high level feature flag for all switch asic features today.
> 
> switch drivers set this flag on switch ports. Logical devices like
> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
> 
> I had to use SWITCH in the name to avoid ambiguity with other feature
> flags. But, since i have been harping about not calling it 'switch',
> I am welcome to any suggestions :)
> 
> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
> in net_device_flags.

What does this flag indicate specifically? What driver would
implement ndo_bridge_setlink() but not set this flag?

I think it should be clearly documented when this flag is to bet set.

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Roopa Prabhu]

On 12/5/14, 2:43 PM, Thomas Graf wrote:
> On 12/04/14 at 06:26pm, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This is a generic high level feature flag for all switch asic features today.
>>
>> switch drivers set this flag on switch ports. Logical devices like
>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>
>> I had to use SWITCH in the name to avoid ambiguity with other feature
>> flags. But, since i have been harping about not calling it 'switch',
>> I am welcome to any suggestions :)
>>
>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>> in net_device_flags.
> What does this flag indicate specifically? What driver would
> implement ndo_bridge_setlink() but not set this flag?
>
> I think it should be clearly documented when this flag is to bet set.
I mentioned it as an alternative because it was there in my RFC patch. 
There is no code for it yet.
And I will get rid of the comment in v2.

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Thomas Graf]

On 12/05/14 at 11:46pm, Roopa Prabhu wrote:
> On 12/5/14, 2:43 PM, Thomas Graf wrote:
> >On 12/04/14 at 06:26pm, roopa@cumulusnetworks.com wrote:
> >>From: Roopa Prabhu <roopa@cumulusnetworks.com>
> >>
> >>This is a generic high level feature flag for all switch asic features today.
> >>
> >>switch drivers set this flag on switch ports. Logical devices like
> >>bridge, bonds, vxlans can inherit this flag from their slaves/ports.
> >>
> >>I had to use SWITCH in the name to avoid ambiguity with other feature
> >>flags. But, since i have been harping about not calling it 'switch',
> >>I am welcome to any suggestions :)
> >>
> >>An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
> >>in net_device_flags.
> >What does this flag indicate specifically? What driver would
> >implement ndo_bridge_setlink() but not set this flag?
> >
> >I think it should be clearly documented when this flag is to bet set.
> I mentioned it as an alternative because it was there in my RFC patch. There
> is no code for it yet.
> And I will get rid of the comment in v2.

Sorry, I was referring to NETIF_F_HW_SWITCH_OFFLOAD. I do not
understand the scope of this bit/flag yet. Can you give examples
when to set this bit? At what point would the existing ixgbe FDB
offload set this bit? Is there a case when ndo_bridge_setlink()
is implemented but this bit is not set?

Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads

[Roopa Prabhu]

On 12/6/14, 2:14 AM, Thomas Graf wrote:
> On 12/05/14 at 11:46pm, Roopa Prabhu wrote:
>> On 12/5/14, 2:43 PM, Thomas Graf wrote:
>>> On 12/04/14 at 06:26pm, roopa@cumulusnetworks.com wrote:
>>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>>
>>>> This is a generic high level feature flag for all switch asic features today.
>>>>
>>>> switch drivers set this flag on switch ports. Logical devices like
>>>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>>>>
>>>> I had to use SWITCH in the name to avoid ambiguity with other feature
>>>> flags. But, since i have been harping about not calling it 'switch',
>>>> I am welcome to any suggestions :)
>>>>
>>>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>>>> in net_device_flags.
>>> What does this flag indicate specifically? What driver would
>>> implement ndo_bridge_setlink() but not set this flag?
>>>
>>> I think it should be clearly documented when this flag is to bet set.
>> I mentioned it as an alternative because it was there in my RFC patch. There
>> is no code for it yet.
>> And I will get rid of the comment in v2.
> Sorry, I was referring to NETIF_F_HW_SWITCH_OFFLOAD.
Ah, ok.

> I do not
> understand the scope of this bit/flag yet.
> Can you give examples
> when to set this bit? At what point would the existing ixgbe FDB
> offload set this bit? Is there a case when ndo_bridge_setlink()
> is implemented but this bit is not set?
The switch asic drivers will set this flag on the switch ports. This 
flag will be used when you want to offload
kernel networking to hardware or in other words accelerate  the 
corresponding kernel network function.
  In the context of current patches it is the bridge driver. A kernel 
bridge that has any of these switch asic ports
  as bridge ports can inherit this flag and use this flag on the ports to
call the switch driver ndo ops to offload state to hw and thus hw 
accelerate the bridging function.

The existing l2 offload flags, example the hwmode and self flags used 
for nic drivers bridge/l2 offload,
does not help switch asic hardware completely. AFAICT, those are used 
today to manage the bridge in nic hw directly.
They don't involve the bridge driver.

And the most important thing is they require the user to specify this 
additional flag to offload.


In my proposal,
- The flag/bit helps switch asic drivers to indicate that they 
accelerate the kernel networking objects/functions
- The user does not have to specify a new flag to do so. A bridge 
created with switch asic ports will thus be accelerated if the switch 
driver supports it.
- The user can continue to directly manage l2 in nics (ixgbe) using the 
existing hwmode/self flags
- And it also does not stop users from using the 'self' flag to talk to 
the switch asic driver directly
- And involving the bridge driver makes sure the add/del notifications 
to user space go out after both kernel and hardware are programmed

Regarding ixgbe driver setting this flag: For the current vepa and veb 
modes I don't see a need for it.
But if there is a use case in the future to indicate hw accelerating the 
bridge driver, ixgbe driver can set this flag