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

Jamal Hadi Salim
2023-11-20T12:48:27Z

On Mon, Nov 20, 2023 at 3:18 AM Jiri Pirko <jiri@resnulli.us> wrote:
>
> Fri, Nov 17, 2023 at 01:09:45PM CET, jhs@mojatatu.com wrote:
> >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.
>
> Yeah, you don't need to carry 20year old burden in newly introduced
> interface. That's my point.

Having a demux sub header is 20 year old burden? I didnt follow.

>
> >b) Where does this leave iproute2? +Cc David and Stephen. Do other
> >generic netlink conversions get contributed back to iproute2?
>
> There is no conversion afaik, only extensions. And they has to be,
> otherwise the user would not be able to use the newly introduced
> features.

The big question is does the collective who use iproute2 still get to
use the same tooling or now they have to go and learn some new
tooling. I understand the value of the new approach but is it a
revolution or an evolution? We opted to put thing in iproute2 instead
for example because that is widely available (and used).

>
> >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.
>
> I'm pretty sure you can achieve the same over genetlink.
>

I think you are right.

>
> >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..
>
> All the benefits of ynl yaml tooling, at least.
>

Did you pay close attention to what we have? The user space code is
written once into iproute2 and subsequent to that there is no
recompilation of any iproute2 code. The compiler generates a json
file specific to a P4 program which is then introspected by the
iproute2 code.

cheers,
jamal

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