netdev - Re: [PATCH net-next v9 14/15] p4tc: add set of P4TC table kfuncs

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: <87lea5j8ys.fsf@toke.dk>
Date: Fri, 08 Dec 2023 11:15:55 +0100
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Martin KaFai Lau <martin.lau@...ux.dev>, Jamal Hadi Salim
 <jhs@...atatu.com>
Cc: deb.chatterjee@...el.com, anjali.singhai@...el.com,
 namrata.limaye@...el.com, mleitner@...hat.com, Mahesh.Shirshyad@....com,
 tomasz.osinski@...el.com, jiri@...nulli.us, xiyou.wangcong@...il.com,
 davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, vladbu@...dia.com, horms@...nel.org,
 khalidm@...dia.com, daniel@...earbox.net, bpf@...r.kernel.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH net-next v9 14/15] p4tc: add set of P4TC table kfuncs

Martin KaFai Lau <martin.lau@...ux.dev> writes:

> On 12/1/23 10:29 AM, Jamal Hadi Salim wrote:
>> We add an initial set of kfuncs to allow interactions from eBPF programs
>> to the P4TC domain.
>> 
>> - bpf_p4tc_tbl_read: Used to lookup a table entry from a BPF
>> program installed in TC. To find the table entry we take in an skb, the
>> pipeline ID, the table ID, a key and a key size.
>> We use the skb to get the network namespace structure where all the
>> pipelines are stored. After that we use the pipeline ID and the table
>> ID, to find the table. We then use the key to search for the entry.
>> We return an entry on success and NULL on failure.
>> 
>> - xdp_p4tc_tbl_read: Used to lookup a table entry from a BPF
>> program installed in XDP. To find the table entry we take in an xdp_md,
>> the pipeline ID, the table ID, a key and a key size.
>> We use struct xdp_md to get the network namespace structure where all
>> the pipelines are stored. After that we use the pipeline ID and the table
>> ID, to find the table. We then use the key to search for the entry.
>> We return an entry on success and NULL on failure.
>> 
>> - bpf_p4tc_entry_create: Used to create a table entry from a BPF
>> program installed in TC. To create the table entry we take an skb, the
>> pipeline ID, the table ID, a key and its size, and an action which will
>> be associated with the new entry.
>> We return 0 on success and a negative errno on failure
>> 
>> - xdp_p4tc_entry_create: Used to create a table entry from a BPF
>> program installed in XDP. To create the table entry we take an xdp_md, the
>> pipeline ID, the table ID, a key and its size, and an action which will
>> be associated with the new entry.
>> We return 0 on success and a negative errno on failure
>> 
>> - bpf_p4tc_entry_create_on_miss: conforms to PNA "add on miss".
>> First does a lookup using the passed key and upon a miss will add the entry
>> to the table.
>> We return 0 on success and a negative errno on failure
>> 
>> - xdp_p4tc_entry_create_on_miss: conforms to PNA "add on miss".
>> First does a lookup using the passed key and upon a miss will add the entry
>> to the table.
>> We return 0 on success and a negative errno on failure
>> 
>> - bpf_p4tc_entry_update: Used to update a table entry from a BPF
>> program installed in TC. To update the table entry we take an skb, the
>> pipeline ID, the table ID, a key and its size, and an action which will
>> be associated with the new entry.
>> We return 0 on success and a negative errno on failure
>> 
>> - xdp_p4tc_entry_update: Used to update a table entry from a BPF
>> program installed in XDP. To update the table entry we take an xdp_md, the
>> pipeline ID, the table ID, a key and its size, and an action which will
>> be associated with the new entry.
>> We return 0 on success and a negative errno on failure
>> 
>> - bpf_p4tc_entry_delete: Used to delete a table entry from a BPF
>> program installed in TC. To delete the table entry we take an skb, the
>> pipeline ID, the table ID, a key and a key size.
>> We return 0 on success and a negative errno on failure
>> 
>> - xdp_p4tc_entry_delete: Used to delete a table entry from a BPF
>> program installed in XDP. To delete the table entry we take an xdp_md, the
>> pipeline ID, the table ID, a key and a key size.
>> We return 0 on success and a negative errno on failure
>
> [ ... ]
>
>> +BTF_SET8_START(p4tc_kfunc_check_tbl_set_skb)
>> +BTF_ID_FLAGS(func, bpf_p4tc_tbl_read, KF_RET_NULL);
>> +BTF_ID_FLAGS(func, bpf_p4tc_entry_create);
>> +BTF_ID_FLAGS(func, bpf_p4tc_entry_create_on_miss);
>> +BTF_ID_FLAGS(func, bpf_p4tc_entry_update);
>> +BTF_ID_FLAGS(func, bpf_p4tc_entry_delete);
>> +BTF_SET8_END(p4tc_kfunc_check_tbl_set_skb)
>
> These create/read/update/delete kfuncs are like defining a new hidden bpf map 
> type in the kernel. bpf prog can now create its own link-list and rbtree. 
> sched_ext has already been using it. This is the way the bpf prog should use 
> instead of creating a new map type.

I don't really think this is an accurate assessment, given Jamal's use
case. These kfuncs are more akin to the FIB lookup helper, or the
netfilter kfuncs: they provide lookup into a kernel-internal data
structure, so that BPF can access that data structure while staying in
sync with the rest of the kernel.

If this was a BPF-only implementation you'd be right, but given the
constraint of having the P4 objects represented in the kernel[0], I
think this is a perfectly reasonable use of kfuncs, even though they
happen to look like the map API.

-Toke

[0] Whether having those objects represented at all is reasonable is a
separate discussion, which I believe John et al are having with Jamal in
a separate subthread. I don't personally have any strong objections to
doing that.