Re: [PATCH bpf-next v5 2/5] net/smc: Introduce generic hook smc_ops

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

On Mon, Jan 13, 2025 at 07:40:12PM +0800, Dust Li wrote:
> On 2025-01-07 12:17:12, D. Wythe wrote:
> >The introduction of IPPROTO_SMC enables eBPF programs to determine
> >whether to use SMC based on the context of socket creation, such as
> >network namespaces, PID and comm name, etc.
> >
> >As a subsequent enhancement, to introduce a new generic hook that
> >allows decisions on whether to use SMC or not at runtime, including
> >but not limited to local/remote IP address or ports.
> >
> >Moreover, in the future, we can achieve more complex extensions to the
> >protocol stack by extending this ops.
> >
> >Signed-off-by: D. Wythe <alibuda@linux.alibaba.com>
> >---
> > include/net/netns/smc.h |  3 ++
> > include/net/smc.h       | 51 ++++++++++++++++++++++
> > net/ipv4/tcp_output.c   | 15 +++++--
> > net/smc/Kconfig         | 12 ++++++
> > net/smc/Makefile        |  1 +
> > net/smc/smc_ops.c       | 51 ++++++++++++++++++++++
> > net/smc/smc_ops.h       | 25 +++++++++++
> > net/smc/smc_sysctl.c    | 95 +++++++++++++++++++++++++++++++++++++++++
> > 8 files changed, 249 insertions(+), 4 deletions(-)
> > create mode 100644 net/smc/smc_ops.c
> > create mode 100644 net/smc/smc_ops.h
> >
> >diff --git a/include/net/netns/smc.h b/include/net/netns/smc.h
> >index fc752a50f91b..59d069f56b2d 100644
> >--- a/include/net/netns/smc.h
> >+++ b/include/net/netns/smc.h
> >@@ -17,6 +17,9 @@ struct netns_smc {
> > #ifdef CONFIG_SYSCTL
> > 	struct ctl_table_header		*smc_hdr;
> > #endif
> >+#if IS_ENABLED(CONFIG_SMC_OPS)
> >+	struct smc_ops __rcu *ops;
>                               ^^^
> Align with other fields
> 
Oops, nice catch.
> >+#endif /* CONFIG_SMC_OPS */
> > 	unsigned int			sysctl_autocorking_size;
> > 	unsigned int			sysctl_smcr_buf_type;
> > 	int				sysctl_smcr_testlink_time;
> >diff --git a/include/net/smc.h b/include/net/smc.h
> >index db84e4e35080..326a217001d4 100644
> >--- a/include/net/smc.h
> >+++ b/include/net/smc.h
> >+#include <net/smc.h>
> 
> Put this include right after #include <net/proto_memory.h> ?
> I notice all the includes here are well classified.
> 
> 
Done, I didn't notice this detail before.
> > 
> > /* Refresh clocks of a TCP socket,
> >  * ensuring monotically increasing values.
> >@@ -759,14 +760,17 @@ 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)
> > {
> >+	  SMC_OPS enables support to register genericfor hook via eBPF programs
> 
> genericfor ?
> 
generic.. I will fix it in next version. 
> >+	  for SMC subsystem. eBPF programs offer much greater flexibility
> >+	  in modifying the behavior of the SMC protocol stack compared
> >+	  to a complete kernel-based approach.
> >+
> >+	  if unsure, say N.
> >diff --git a/net/smc/Makefile b/net/smc/Makefile
> >index 60f1c87d5212..5dd706b2927a 100644
> >--- a/net/smc/Makefile
> >+++ b/net/smc/Makefile
> >+	struct net *net = container_of(ctl->data, struct net,
> >+				       smc.ops);
> 
> This line won't exceed 80 characters, no need to split into 2 lines.
> 
Take it. Thanks for all the advices.

Best wishes,
D. Wythe

> 
> Best regards,
> Dust