[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d6a53bed-b197-432c-84e5-ac324b36137e@linux.dev>
Date: Wed, 5 Nov 2025 20:16:45 -0800
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: "D. Wythe" <alibuda@...ux.alibaba.com>
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org,
pabeni@...hat.com, song@...nel.org, sdf@...gle.com, haoluo@...gle.com,
yhs@...com, edumazet@...gle.com, john.fastabend@...il.com,
kpsingh@...nel.org, jolsa@...nel.org, mjambigi@...ux.ibm.com,
wenjia@...ux.ibm.com, wintera@...ux.ibm.com, dust.li@...ux.alibaba.com,
tonylu@...ux.alibaba.com, guwen@...ux.alibaba.com, bpf@...r.kernel.org,
davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org,
sidraya@...ux.ibm.com, jaka@...ux.ibm.com
Subject: Re: [PATCH bpf-next v4 2/3] net/smc: bpf: Introduce generic hook for
handshake flow
On 11/5/25 6:33 PM, D. Wythe wrote:
> On Wed, Nov 05, 2025 at 02:58:48PM -0800, Martin KaFai Lau wrote:
>>
>>
>> On 11/4/25 11:01 PM, D. Wythe wrote:
>>> On Tue, Nov 04, 2025 at 04:03:46PM -0800, Martin KaFai Lau wrote:
>>>>
>>>>
>>>> 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__); \
>>>>> +
>>>>> + 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?
>>>>
>>>> [ ... ]
>>>>
>>>
>>> You're right, it is a bit redundant. However, if we merge the parameters,
>>> every user of this macro will be forced to pass tp. In fact, we’re
>>> already considering adding some callback functions that don’t take tp as
>>> a parameter.
>>
>> If the struct_ops callback does not take tp, then don't pass it to the
>> callback. I have a hard time to imagine why the bpf prog will not be
>> interested in the tp/sk pointer though.
>>
>> or you meant the caller does not have tp? and where is the future caller?
>
> My initial concern was that certain ctrl->func callbacks might
> eventually need to operate on an smc_sock rather than a tcp_sock.
hmm...in that case, I think it first needs to understand where else the
smc struct_ops is planned to be called in the future. I thought the smc
struct_ops is something unique to the af_smc address family but I
suspect the future ops addition may not be the case. Can you share some
details on where the future callback will be? e.g. in smc_{connect,
sendmsg, recvmsg...} that has the smc_sock?
Powered by blists - more mailing lists