Re: [PATCH bpf-next v4 2/3] net/smc: bpf: Introduce generic hook for handshake flow

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

On 11/2/25 11:31 PM, D. Wythe wrote:
> +#if IS_ENABLED(CONFIG_SMC_HS_CTRL_BPF)
> +#define smc_call_hsbpf(init_val, sk, func, ...) ({		\
> +	typeof(init_val) __ret = (init_val);			\
> +	struct smc_hs_ctrl *ctrl;				\
> +	rcu_read_lock();					\
> +	ctrl = rcu_dereference(sock_net(sk)->smc.hs_ctrl);	\

The smc_hs_ctrl (and its ops) is called from the netns, so the 
bpf_struct_ops is attached to a netns. Attaching bpf_struct_ops to a 
netns has not been done before. More on this later.

> +	if (ctrl && ctrl->func)					\
> +		__ret = ctrl->func(__VA_ARGS__);		\
> +	rcu_read_unlock();					\
> +	__ret;							\
> +})
> +#else
> +#define smc_call_hsbpf(init_val, sk, ...)  ({ (void)(sk); (init_val); })
> +#endif /* CONFIG_SMC_HS_CTRL_BPF */
> +
>   #endif	/* _SMC_H */
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 7f5df7a71f62..1a3234729a29 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -40,6 +40,7 @@
>   #include <net/tcp.h>
>   #include <net/tcp_ecn.h>
>   #include <net/mptcp.h>
> +#include <net/smc.h>
>   #include <net/proto_memory.h>
>   #include <net/psp.h>
>   
> @@ -802,34 +803,41 @@ static void tcp_options_write(struct tcphdr *th, struct tcp_sock *tp,
>   	mptcp_options_write(th, ptr, tp, opts);
>   }
>   
> -static void smc_set_option(const struct tcp_sock *tp,
> +static void smc_set_option(struct tcp_sock *tp,
>   			   struct tcp_out_options *opts,
>   			   unsigned int *remaining)
>   {
>   #if IS_ENABLED(CONFIG_SMC)
> -	if (static_branch_unlikely(&tcp_have_smc)) {
> -		if (tp->syn_smc) {
> -			if (*remaining >= TCPOLEN_EXP_SMC_BASE_ALIGNED) {
> -				opts->options |= OPTION_SMC;
> -				*remaining -= TCPOLEN_EXP_SMC_BASE_ALIGNED;
> -			}
> +	struct sock *sk = &tp->inet_conn.icsk_inet.sk;

the sk is tp ...

> +
> +	if (static_branch_unlikely(&tcp_have_smc) && tp->syn_smc) {
> +		tp->syn_smc = !!smc_call_hsbpf(1, sk, syn_option, tp);

... so just pass tp instead of passing both sk and tp?

[ ... ]

> +static int smc_bpf_hs_ctrl_init(struct btf *btf) { return 0; }
> +
> +static int smc_bpf_hs_ctrl_reg(void *kdata, struct bpf_link *link)

More on attaching to netns. There is discussion on how to attach a 
bpf_struct_ops to a particular cgroup in a link. I think the link should 
be able to attach a bpf_struct_ops to a particular netns also.

I would suggest to reject link now. Later, link support can be added to 
attach to a particular netns. This will be the last non-link-only 
bpf_struct_ops addition, considering the blast radius is limited on 
smc_hs_ctrl and the smc effort was started a while ago. I could have 
missed things here. Other experts could chime in.

	if (link)
		return -EOPNOTSUPP;


pw-bot: cr

> +{
> +	return smc_hs_ctrl_reg(kdata);
> +}
> +
> +static void smc_bpf_hs_ctrl_unreg(void *kdata, struct bpf_link *link)
> +{
> +	smc_hs_ctrl_unreg(kdata);
> +}
> +