netdev - Re: [PATCH bpf-next v2 1/2] net/smc: Introduce BPF injection capability for SMC

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: <fe3db636-2f89-3175-a605-2124b43ae4fa@linux.dev>
Date:   Fri, 24 Mar 2023 16:27:51 -0700
From:   Martin KaFai Lau <martin.lau@...ux.dev>
To:     "D. Wythe" <alibuda@...ux.alibaba.com>
Cc:     kuba@...nel.org, davem@...emloft.net, netdev@...r.kernel.org,
        linux-s390@...r.kernel.org, linux-rdma@...r.kernel.org,
        bpf@...r.kernel.org, kgraul@...ux.ibm.com, wenjia@...ux.ibm.com,
        jaka@...ux.ibm.com, ast@...nel.org, daniel@...earbox.net,
        andrii@...nel.org
Subject: Re: [PATCH bpf-next v2 1/2] net/smc: Introduce BPF injection
 capability for SMC

On 3/23/23 9:08 PM, D. Wythe wrote:
> 
> The latest design is that users can register a negotiator implementation indexed 
> by name, smc_sock can use bpf_setsockopt to specify
> whether a specific negotiation implementation is required via name. If there are 
> no settings, there will be no negotiators.
> 
> What do you think?

tbh, bpf_setsockopt is many steps away. It needs to begin with a syscall 
setsockopt first. There is little reason it can only be done with a bpf prog. 
and how does the user know which negotiator a smc sock is using? Currently, ss 
can learn the tcp-cc of a sk.

~~~~~~~~

If this effort is serious, the code quality has to be much improved. The obvious 
bug and unused variables make this set at most a RFC.

 From the bpf perspective, it is ok-ish to start with a global negotiator first 
and skip the setsockopt details for now. However, it needs to be have a name. 
The new link_update 
(https://lore.kernel.org/bpf/20230323032405.3735486-1-kuifeng@meta.com/) has to 
work also. The struct_ops is rcu reader safe, so leverage it whenever it can 
instead of the read/write lock. It is how struct_ops work for tcp, so try to 
stay consistent as much as possible in the networking stack.

> 
> In addition, I am very sorry that I have not issued my implementation for such a 
> long time, and I have encountered some problems with the implementation because
> the SMC needs to be built as kernel module, I have struggled with the 
> bpf_setsockopt implementation, and there are some new self-testes that need to 
> be written.
> 

Regarding compiling as module,

+ifneq ($(CONFIG_SMC),)
+ifeq ($(CONFIG_BPF_SYSCALL),y)
+obj-y				+= smc/bpf_smc_struct_ops.o
+endif

struct_ops does not support module now. It is on the todo list. The 
bpf_smc_struct_ops.o above can only be used when CONFIG_SMC=y. Otherwise, the 
bpf_smc_struct_ops is always built in while most users will never load the smc 
module.