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

Jamal Hadi Salim
2023-11-17T12:09:57Z

On Thu, Nov 16, 2023 at 11:11 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Thu, Nov 16, 2023 at 03:59:42PM CET, jhs@mojatatu.com 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?
>

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

cheers,
jamal

>
> >+
> >+#define P4TC_MAXPIPELINE_COUNT 32
> >+#define P4TC_MAXTABLES_COUNT 32
> >+#define P4TC_MINTABLES_COUNT 0
> >+#define P4TC_MSGBATCH_SIZE 16
> >+
> > #define P4TC_MAX_KEYSZ 512
> >
> >+#define TEMPLATENAMSZ 32
> >+#define PIPELINENAMSIZ TEMPLATENAMSZ
>
> ugh. A prefix please?
>
> pw-bot: cr
>
> …