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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZVtcEwICZHsTtija@nanopsycho>
Date: Mon, 20 Nov 2023 14:16:03 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Jamal Hadi Salim <jhs@...atatu.com>
Cc: netdev@...r.kernel.org, deb.chatterjee@...el.com,
	anjali.singhai@...el.com, namrata.limaye@...el.com, tom@...anda.io,
	mleitner@...hat.com, Mahesh.Shirshyad@....com,
	tomasz.osinski@...el.com, xiyou.wangcong@...il.com,
	davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, vladbu@...dia.com, horms@...nel.org,
	daniel@...earbox.net, bpf@...r.kernel.org, khalidm@...dia.com,
	toke@...hat.com, mattyk@...dia.com, David Ahern <dsahern@...il.com>,
	Stephen Hemminger <stephen@...workplumber.org>
Subject: Re: [PATCH net-next v8 09/15] p4tc: add template pipeline create,
 get, update, delete

Mon, Nov 20, 2023 at 01:48:14PM CET, jhs@...atatu.com wrote:
>On Mon, Nov 20, 2023 at 3:18 AM Jiri Pirko <jiri@...nulli.us> wrote:
>>
>> Fri, Nov 17, 2023 at 01:09:45PM CET, jhs@...atatu.com wrote:
>> >On Thu, Nov 16, 2023 at 11:11 AM Jiri Pirko <jiri@...nulli.us> wrote:
>> >>
>> >> Thu, Nov 16, 2023 at 03:59:42PM CET, jhs@...atatu.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.

You don't need the header, 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.
>
>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...


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

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.


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