Mon, Nov 20, 2023 at 04:30:11PM CET, jhs@xxxxxxxxxxxx wrote: >On Mon, Nov 20, 2023 at 8:16 AM Jiri Pirko <jiri@xxxxxxxxxxx> wrote: >> >> Mon, Nov 20, 2023 at 01:48:14PM CET, jhs@xxxxxxxxxxxx wrote: >> >On Mon, Nov 20, 2023 at 3:18 AM Jiri Pirko <jiri@xxxxxxxxxxx> wrote: >> >> >> >> Fri, Nov 17, 2023 at 01:09:45PM CET, jhs@xxxxxxxxxxxx wrote: >> >> >On Thu, Nov 16, 2023 at 11:11 AM Jiri Pirko <jiri@xxxxxxxxxxx> wrote: >> >> >> >> >> >> Thu, Nov 16, 2023 at 03:59:42PM CET, jhs@xxxxxxxxxxxx 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. >> >> You don't need the header, that's my point. >> > >Let me see if i understand you: >We have multiple object types per pipeline - this info is _omni >present and it is never going to change_. >Your view is, have a hierarchy of attributes and put this subheader in >probably one attribute at the root. That or use genetlink to have per-cmd attributes. >You parse the root, you find the obj and pipeid and then you use that >to parse the rest of the per-object specific >attributes? > >I dont know if a hierarchical attribute layout gives you any advantage >over the subheader approach - unless we figure a way to annotate >attributes as "optional" vs "must be present". I agree that getting >the validation for free is a bonus .. > > >> > >> >> >> >> >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). >> >> I don't see why iproute2 user facing interface would be any different >> depending on if you user RTnetlink or genetlink as backend channel... >> > >iproute2 supports plenty of genetlink already. >We need to find a way to have the best of both worlds. > >> >> > >> >> >> >> >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. >> >> Right, but in real life, netlink is used directly by many apps. I don't >> see why this is any different. >> > >Not sure if you were referring to what i said about the json file or >something else. The main value is not just kernel independence but >also iproute2 independence i.e not need to compile any code. > >> Plus, the very best part of yaml from user perpective I see is, >> you just need the kernel-git yaml file and you can submit all commands. >> No userspace implementation needed. > >Two different tacts: i can see this as being developer friendly (and >we are more trying to be operator friendly). >I need to take a closer look. Sounds like it should be polyglot >friendly as well. If i am not mistaken you still have to compile code >as a result of generation from the yaml? Nope, you can run ynl.py and let it parse the yaml on fly. > >cheers, >jamal > >> >> > >> > >> >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 >> >> >> >> >> >> [...]