Re: [PATCH net-next v8 09/15] p4tc: add template pipeline create, get, update, delete

David Ahern
2023-11-20T18:20:48Z

On 11/17/23 4:09 AM, Jamal Hadi Salim wrote:
>>> diff --git a/include/uapi/linux/p4tc.h b/include/uapi/linux/p4tc.h
>>> index ba32dba66..4d33f44c1 100644
>>> --- a/include/uapi/linux/p4tc.h
>>> +++ b/include/uapi/linux/p4tc.h
>>> @@ -2,8 +2,71 @@
>>> #ifndef __LINUX_P4TC_H
>>> #define __LINUX_P4TC_H
>>>
>>> +#include <linux/types.h>
>>> +#include <linux/pkt_sched.h>
>>> +
>>> +/* pipeline header */
>>> +struct p4tcmsg {
>>> + __u32 pipeid;
>>> + __u32 obj;
>>> +};
>>
>> I don't follow. Is there any sane reason to use header instead of normal
>> netlink attribute? Moveover, you extend the existing RT netlink with
>> a huge amout of p4 things. Isn't this the good time to finally introduce
>> generic netlink TC family with proper yaml spec with all the benefits it
>> brings and implement p4 tc uapi there? Please?
>>

There is precedence (new netdev APIs) to move new infra to genl, but it
is not clear to me if extending existing functionality should fall into
that required conversion.

>
> Several reasons:
> a) We are similar to current tc messaging with the subheader being
> there for multiplexing.
> b) Where does this leave iproute2? +Cc David and Stephen. Do other
> generic netlink conversions get contributed back to iproute2?
> c) note: Our API is CRUD-ish instead of RPC(per generic netlink)
> based. i.e you have:
> COMMAND <PATH/TO/OBJECT> [optional data] so we can support arbitrary
> P4 programs from the control plane.
> d) we have spent many hours optimizing the control to the kernel so i
> am not sure what it would buy us to switch to generic netlink..
>