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