netdev - Re: [PATCH bpf-next v4 1/7] netkit, bpf: Add bpf programmable net device

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: <28a19c46-2ee0-01db-cf88-6c9007e97c82@iogearbox.net>
Date: Thu, 26 Oct 2023 08:20:24 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Kui-Feng Lee <sinquersw@...il.com>,
 Martin KaFai Lau <martin.lau@...ux.dev>
Cc: netdev@...r.kernel.org, razor@...ckwall.org, ast@...nel.org,
 andrii@...nel.org, john.fastabend@...il.com, sdf@...gle.com,
 toke@...nel.org, kuba@...nel.org, andrew@...n.ch,
 Toke Høiland-Jørgensen <toke@...hat.com>,
 bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 1/7] netkit, bpf: Add bpf programmable net
 device

Hi Kui-Feng,

On 10/26/23 3:18 AM, Kui-Feng Lee wrote:
> On 10/25/23 18:15, Kui-Feng Lee wrote:
>> On 10/25/23 15:09, Martin KaFai Lau wrote:
>>> On 10/25/23 2:24 PM, Kui-Feng Lee wrote:
>>>> On 10/24/23 14:48, Daniel Borkmann wrote:
>>>>> This work adds a new, minimal BPF-programmable device called "netkit"
>>>>> (former PoC code-name "meta") we recently presented at LSF/MM/BPF. The
>>>>> core idea is that BPF programs are executed within the drivers xmit routine
>>>>> and therefore e.g. in case of containers/Pods moving BPF processing closer
>>>>> to the source.
>>>>
>>>> Sorry for intruding into this discussion! Although it is too late to
>>>> mentioned this since this patchset have been v4 already.
>>>>
>>>> I notice netkit has introduced a new attach type. I wonder if it
>>>> possible to implement it as a new struct_ops type.
>>>
>>> Could your elaborate more about what does this struct_ops type do and how is it different from the SCHED_CLS bpf prog that the netkit is running?
>>
>> I found the code has been landed.
>> Basing on the landed code and
>> the patchset of registering bpf struct_ops from modules that I
>> am working on, it will looks like what is done in following patch.
>> No changes on syscall, uapi and libbpf are required.
>>
>> diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
>> index 7e484f9fd3ae..e4eafaf397bf 100644
>> --- a/drivers/net/netkit.c
>> +++ b/drivers/net/netkit.c
>> @@ -20,6 +20,7 @@ struct netkit {
>>       struct bpf_mprog_entry __rcu *active;
>>       enum netkit_action policy;
>>       struct bpf_mprog_bundle    bundle;
>> +    struct hlist_head ops_list;
>>
>>       /* Needed in slow-path */
>>       enum netkit_mode mode;
>> @@ -27,6 +28,13 @@ struct netkit {
>>       u32 headroom;
>>   };
>>
>> +struct netkit_ops {
>> +    struct hlist_node node;
>> +    int ifindex;
>> +
>> +    int (*xmit)(struct sk_buff *skb);
>> +};
>> +
>>   struct netkit_link {
>>       struct bpf_link link;
>>       struct net_device *dev;
>> @@ -46,6 +54,22 @@ netkit_run(const struct bpf_mprog_entry *entry, struct sk_buff *skb,
>>           if (ret != NETKIT_NEXT)
>>               break;
>>       }
>> +
>> +    return ret;
>> +}
>> +
>> +static __always_inline int
>> +netkit_run_st_ops(const struct netkit *nk, struct sk_buff *skb,
>> +       enum netkit_action ret)
>> +{
>> +    struct netkit_ops *ops;
>> +
>> +    hlist_for_each_entry_rcu(ops, &nk->ops_list, node) {
>> +        ret = ops->xmit(skb);
>> +        if (ret != NETKIT_NEXT)
>> +            break;
>> +    }
>> +
>>       return ret;
>>   }
>>
>> @@ -80,6 +104,8 @@ static netdev_tx_t netkit_xmit(struct sk_buff *skb, struct net_device *dev)
>>       entry = rcu_dereference(nk->active);
>>       if (entry)
>>           ret = netkit_run(entry, skb, ret);
>> +    if (ret == NETKIT_NEXT)
>> +        ret = netkit_run_st_ops(nk, skb, ret);
>>       switch (ret) {
>>       case NETKIT_NEXT:
>>       case NETKIT_PASS:

I don't think it makes sense to cramp struct ops in here for what has been
solved already with the bpf_mprog interface in a more efficient way and with
control dependencies for the insertion (before/after relative programs/links).
The latter is in particular crucial for a multi-user interface when dealing
with network traffic (think for example: policy, forwarder, observability
prog, etc).

Thanks,
Daniel