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

[Martin KaFai Lau <martin.lau@linux.dev>]

On 12/8/23 2:15 AM, Toke Høiland-Jørgensen wrote:
> Martin KaFai Lau <martin.lau@linux.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.

I might not be clear. It was my question on why it has to be in the kernel 
instead of in the bpf map, so the earlier bpf link-list and rbtree example just 
in case this recent bpf capability has not been considered.

If it is an existing kernel infra-structure, kfunc is a reasonable use.

The P4 objects are newly added to this set with bpf program as its user. It can 
be represented in the bpf map as well instead of in the kernel.

or is it fair to say that bpf prog is not the primary consumer of the P4 
objects. Instead kernel is the primary user of the p4 objects such that p4tc can 
work independently without the bpf piece to begin with and bpf could be 
considered as an extension later?