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

Jamal Hadi Salim
2023-11-20T20:12:19Z

On Mon, Nov 20, 2023 at 1:20 PM David Ahern <dsahern@kernel.org> wrote:
>
> 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.
>

Big question is: how does the genl (which i am assuming you mean the
ynl stuff) fit back into iproute2?
The yaml files approach is a great deal of help for maintenance IMO (a
lot of repetitive code gone). But do we leave the rest of the masses
out? What is the motivation for pushing anything to be shared? And if
the answer is to convert everything onwards into genl then where is
the central location to grab that code from? Is it still iproute2 or
the kernel? etc

cheers,
jamal

> >
> > 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..
> >
>