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

["D. Wythe" <alibuda@linux.alibaba.com>]

Hi Martin,

Sorry to have been responding so late,  I've been working on the 
link_update you mentioned in last week,
I have completed the support and testing of the related functions of it. 
and it is expected to be released in the
next few days.

As you mentioned, I do have much experience in kernel network 
development, so I plan to resend the PATCH in the form of RFC.
I really hope to receive your suggestions in next serials. Thank you.😉

Best wishes.
D. Wythe


On 3/25/23 7:27 AM, Martin KaFai Lau wrote:
> 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.