[RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[roopa]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This patch adds new flags in netlink header nlmsg_flags to signal if the
message is for the kernel, hw or both.

This can be used to indicate hw offload for all kind of objects
routes, fdb entries, neighs, link objects like bonds, bridges, vxlan.

Adding it in the header makes it possible to use it accross all objects and
across all messages (sets/gets/deletes).

Other alternative to this is a per kernel object netlink attribute/flag.
But that leads to duplicating the attribute in different subsystems.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 include/uapi/linux/netlink.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index 1a85940..f78522d 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -54,6 +54,8 @@ struct nlmsghdr {
 #define NLM_F_ACK		4	/* Reply with ack, with zero or error code */
 #define NLM_F_ECHO		8	/* Echo this request 		*/
 #define NLM_F_DUMP_INTR		16	/* Dump was inconsistent due to sequence change */
++#define NLM_F_KERNEL       32      /* This msg is only for the kernel */
+#define NLM_F_HW_OFFLOAD	64	/* offload this msg to hw */
 
 /* Modifiers to GET request */
 #define NLM_F_ROOT	0x100	/* specify tree	root	*/
-- 
1.7.10.4

Re: [RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[Thomas Graf]

On 11/21/14 at 02:49pm, roopa@cumulusnetworks.com wrote:
> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
> index 1a85940..f78522d 100644
> --- a/include/uapi/linux/netlink.h
> +++ b/include/uapi/linux/netlink.h
> @@ -54,6 +54,8 @@ struct nlmsghdr {
>  #define NLM_F_ACK		4	/* Reply with ack, with zero or error code */
>  #define NLM_F_ECHO		8	/* Echo this request 		*/
>  #define NLM_F_DUMP_INTR		16	/* Dump was inconsistent due to sequence change */
> ++#define NLM_F_KERNEL       32      /* This msg is only for the kernel */
> +#define NLM_F_HW_OFFLOAD	64	/* offload this msg to hw */
>  
>  /* Modifiers to GET request */
>  #define NLM_F_ROOT	0x100	/* specify tree	root	*/

The NLM_F_ flag space applies to all Netlink messages including non
networking bits and is reserved for flags vital to the functioning
of the Netlink protocol itself. I suggest you move this to a
RTNETLINK specific flags space.

Re: [RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[Roopa Prabhu]

On 11/21/14, 3:12 PM, Thomas Graf wrote:
> On 11/21/14 at 02:49pm, roopa@cumulusnetworks.com wrote:
>> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
>> index 1a85940..f78522d 100644
>> --- a/include/uapi/linux/netlink.h
>> +++ b/include/uapi/linux/netlink.h
>> @@ -54,6 +54,8 @@ struct nlmsghdr {
>>   #define NLM_F_ACK		4	/* Reply with ack, with zero or error code */
>>   #define NLM_F_ECHO		8	/* Echo this request 		*/
>>   #define NLM_F_DUMP_INTR		16	/* Dump was inconsistent due to sequence change */
>> ++#define NLM_F_KERNEL       32      /* This msg is only for the kernel */
>> +#define NLM_F_HW_OFFLOAD	64	/* offload this msg to hw */
>>   
>>   /* Modifiers to GET request */
>>   #define NLM_F_ROOT	0x100	/* specify tree	root	*/
> The NLM_F_ flag space applies to all Netlink messages including non
> networking bits and is reserved for flags vital to the functioning
> of the Netlink protocol itself. I suggest you move this to a
> RTNETLINK specific flags space.

I did try to add it at a layer lower than the netlink header. But, 
nothing else fits so well as this :).
I did not find a place where i could make it a global flag for just 
networking objects. As I mention in my patch description,
If not here it will be a per subsystem flag/attribute. I can post a 
patch showing that approach as well.

Thanks!.

Re: [RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[Thomas Graf]

On 11/21/14 at 04:10pm, Roopa Prabhu wrote:
> On 11/21/14, 3:12 PM, Thomas Graf wrote:
> >On 11/21/14 at 02:49pm, roopa@cumulusnetworks.com wrote:
> >>diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
> >>index 1a85940..f78522d 100644
> >>--- a/include/uapi/linux/netlink.h
> >>+++ b/include/uapi/linux/netlink.h
> >>@@ -54,6 +54,8 @@ struct nlmsghdr {
> >>  #define NLM_F_ACK		4	/* Reply with ack, with zero or error code */
> >>  #define NLM_F_ECHO		8	/* Echo this request 		*/
> >>  #define NLM_F_DUMP_INTR		16	/* Dump was inconsistent due to sequence change */
> >>++#define NLM_F_KERNEL       32      /* This msg is only for the kernel */
> >>+#define NLM_F_HW_OFFLOAD	64	/* offload this msg to hw */
> >>  /* Modifiers to GET request */
> >>  #define NLM_F_ROOT	0x100	/* specify tree	root	*/
> >The NLM_F_ flag space applies to all Netlink messages including non
> >networking bits and is reserved for flags vital to the functioning
> >of the Netlink protocol itself. I suggest you move this to a
> >RTNETLINK specific flags space.
> 
> I did try to add it at a layer lower than the netlink header. But, nothing
> else fits so well as this :).
> I did not find a place where i could make it a global flag for just
> networking objects. As I mention in my patch description,
> If not here it will be a per subsystem flag/attribute. I can post a patch
> showing that approach as well.

Global variables have their appeal ;-) The issue I see with this
besides polluting a wide namespace is that it is not extendable.
We get a single bit and if a single bit is not enough we'll start
adding things in lower layers anyway (as you already do).

I think this should really be an IFF_ flag for net_device so its
state is exposed to user space as well.

Re: [RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[Roopa Prabhu]

On 11/22/14, 4:29 AM, Thomas Graf wrote:
> On 11/21/14 at 04:10pm, Roopa Prabhu wrote:
>> On 11/21/14, 3:12 PM, Thomas Graf wrote:
>>> On 11/21/14 at 02:49pm, roopa@cumulusnetworks.com wrote:
>>>> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
>>>> index 1a85940..f78522d 100644
>>>> --- a/include/uapi/linux/netlink.h
>>>> +++ b/include/uapi/linux/netlink.h
>>>> @@ -54,6 +54,8 @@ struct nlmsghdr {
>>>>   #define NLM_F_ACK		4	/* Reply with ack, with zero or error code */
>>>>   #define NLM_F_ECHO		8	/* Echo this request 		*/
>>>>   #define NLM_F_DUMP_INTR		16	/* Dump was inconsistent due to sequence change */
>>>> ++#define NLM_F_KERNEL       32      /* This msg is only for the kernel */
>>>> +#define NLM_F_HW_OFFLOAD	64	/* offload this msg to hw */
>>>>   /* Modifiers to GET request */
>>>>   #define NLM_F_ROOT	0x100	/* specify tree	root	*/
>>> The NLM_F_ flag space applies to all Netlink messages including non
>>> networking bits and is reserved for flags vital to the functioning
>>> of the Netlink protocol itself. I suggest you move this to a
>>> RTNETLINK specific flags space.
>> I did try to add it at a layer lower than the netlink header. But, nothing
>> else fits so well as this :).
>> I did not find a place where i could make it a global flag for just
>> networking objects. As I mention in my patch description,
>> If not here it will be a per subsystem flag/attribute. I can post a patch
>> showing that approach as well.
> Global variables have their appeal ;-) The issue I see with this
> besides polluting a wide namespace is that it is not extendable.
> We get a single bit and if a single bit is not enough we'll start
> adding things in lower layers anyway (as you already do).
>
> I think this should really be an IFF_ flag for net_device so its
> state is exposed to user space as well.
I did start with a IFF_HW_OFFLOAD flag. This is going exactly in the 
direction i was thinking it will go. :). I had to start at the top, 
hence started with the netlink header first.

Now, going one step lower, It can be a flag or  a netlink attribute.

Going with the flag,
Doing a quick survey of all the rtnetlink handlers..(keeping future in 
mind)....for the top objects in the current context (link, route, fdb 
and neigh)
we can add it in the below flags.

struct ifinfomsg -> ifi_flags
struct ndmsg ->ndm_flags
struct rtmsg  -> rtm_flags

If this is preferred .., I can repost the series with these changes.

Thanks,
Roopa

Re: [RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[Vincent JARDIN]

On 22/11/2014 20:37, Roopa Prabhu wrote:
> struct ifinfomsg -> ifi_flags
> struct ndmsg ->ndm_flags
> struct rtmsg  -> rtm_flags
>
> If this is preferred .., I can repost the series with these changes.

Yes, it would be my preferred too.

Thank you,
   Vincent

Re: [RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[Sergei Shtylyov]

Hello.

On 11/22/2014 1:49 AM, roopa@cumulusnetworks.com wrote:

> From: Roopa Prabhu <roopa@cumulusnetworks.com>

> This patch adds new flags in netlink header nlmsg_flags to signal if the
> message is for the kernel, hw or both.

> This can be used to indicate hw offload for all kind of objects
> routes, fdb entries, neighs, link objects like bonds, bridges, vxlan.

> Adding it in the header makes it possible to use it accross all objects and
> across all messages (sets/gets/deletes).

> Other alternative to this is a per kernel object netlink attribute/flag.
> But that leads to duplicating the attribute in different subsystems.

> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
>   include/uapi/linux/netlink.h |    2 ++
>   1 file changed, 2 insertions(+)

> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
> index 1a85940..f78522d 100644
> --- a/include/uapi/linux/netlink.h
> +++ b/include/uapi/linux/netlink.h
> @@ -54,6 +54,8 @@ struct nlmsghdr {
>   #define NLM_F_ACK		4	/* Reply with ack, with zero or error code */
>   #define NLM_F_ECHO		8	/* Echo this request 		*/
>   #define NLM_F_DUMP_INTR		16	/* Dump was inconsistent due to sequence change */
> ++#define NLM_F_KERNEL       32      /* This msg is only for the kernel */

    I don't think you really meant double '+'.

> +#define NLM_F_HW_OFFLOAD	64	/* offload this msg to hw */
[...]

WBR, Sergei

Re: [RFC PATCH 1/4] rtnetlink: new flag NLM_F_HW_OFFLOAD to indicate kernel object offload to hardware

[Roopa Prabhu]

On 11/22/14, 12:08 PM, Sergei Shtylyov wrote:
> Hello.
>
> On 11/22/2014 1:49 AM, roopa@cumulusnetworks.com wrote:
>
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
>> This patch adds new flags in netlink header nlmsg_flags to signal if the
>> message is for the kernel, hw or both.
>
>> This can be used to indicate hw offload for all kind of objects
>> routes, fdb entries, neighs, link objects like bonds, bridges, vxlan.
>
>> Adding it in the header makes it possible to use it accross all 
>> objects and
>> across all messages (sets/gets/deletes).
>
>> Other alternative to this is a per kernel object netlink attribute/flag.
>> But that leads to duplicating the attribute in different subsystems.
>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>>   include/uapi/linux/netlink.h |    2 ++
>>   1 file changed, 2 insertions(+)
>
>> diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
>> index 1a85940..f78522d 100644
>> --- a/include/uapi/linux/netlink.h
>> +++ b/include/uapi/linux/netlink.h
>> @@ -54,6 +54,8 @@ struct nlmsghdr {
>>   #define NLM_F_ACK        4    /* Reply with ack, with zero or error 
>> code */
>>   #define NLM_F_ECHO        8    /* Echo this request         */
>>   #define NLM_F_DUMP_INTR        16    /* Dump was inconsistent due 
>> to sequence change */
>> ++#define NLM_F_KERNEL       32      /* This msg is only for the 
>> kernel */
>
>    I don't think you really meant double '+'.
certainly not.  will fix it. thanks.
>
>> +#define NLM_F_HW_OFFLOAD    64    /* offload this msg to hw */
> [...]
>
> WBR, Sergei
>
> -- 
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html