On 2025-01-23 09:59:39, 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@xxxxxxxxxxxxxxxxx> >--- > include/net/netns/smc.h | 3 ++ > include/net/smc.h | 53 ++++++++++++++++++++++++ > net/ipv4/tcp_output.c | 18 +++++++-- > net/smc/Kconfig | 12 ++++++ > net/smc/Makefile | 1 + > net/smc/smc_ops.c | 53 ++++++++++++++++++++++++ > net/smc/smc_ops.h | 28 +++++++++++++ > net/smc/smc_sysctl.c | 90 +++++++++++++++++++++++++++++++++++++++++ > 8 files changed, 254 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..81b3fdb39cd2 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; >+#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..844f98a6296a 100644 >--- a/include/net/smc.h >+++ b/include/net/smc.h >@@ -18,6 +18,8 @@ > #include "linux/ism.h" > > struct sock; >+struct tcp_sock; >+struct inet_request_sock; > > #define SMC_MAX_PNETID_LEN 16 /* Max. length of PNET id */ > >@@ -97,4 +99,55 @@ struct smcd_dev { > u8 going_away : 1; > }; > >+#define SMC_OPS_NAME_MAX 16 >+ >+enum { >+ /* ops can be inherit from init_net */ >+ SMC_OPS_FLAG_INHERITABLE = 0x1, >+ >+ SMC_OPS_ALL_FLAGS = SMC_OPS_FLAG_INHERITABLE, >+}; >+ >+struct smc_ops { >+ /* priavte */ >+ >+ struct list_head list; >+ struct module *owner; >+ >+ /* public */ >+ >+ /* unique name */ >+ char name[SMC_OPS_NAME_MAX]; >+ int flags; >+ >+ /* Invoked before computing SMC option for SYN packets. >+ * We can control whether to set SMC options by returning varios value. >+ * Return 0 to disable SMC, or return any other value to enable it. >+ */ >+ int (*set_option)(struct tcp_sock *tp); >+ >+ /* Invoked before Set up SMC options for SYN-ACK packets >+ * We can control whether to respond SMC options by returning varios >+ * value. Return 0 to disable SMC, or return any other value to enable >+ * it. >+ */ >+ int (*set_option_cond)(const struct tcp_sock *tp, >+ struct inet_request_sock *ireq); >+}; >+ >+#if IS_ENABLED(CONFIG_SMC_OPS) >+#define smc_call_retops(init_val, sk, func, ...) ({ \ >+ typeof(init_val) __ret = (init_val); \ >+ struct smc_ops *ops; \ >+ rcu_read_lock(); \ >+ ops = READ_ONCE(sock_net(sk)->smc.ops); \ >+ if (ops && ops->func) \ >+ __ret = ops->func(__VA_ARGS__); \ >+ rcu_read_unlock(); \ >+ __ret; \ >+}) >+#else >+#define smc_call_retops(init_val, ...) (init_val) >+#endif /* CONFIG_SMC_OPS */ >+ > #endif /* _SMC_H */ >diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c >index 0e5b9a654254..f62e30b4ffc8 100644 >--- a/net/ipv4/tcp_output.c >+++ b/net/ipv4/tcp_output.c >@@ -40,6 +40,7 @@ > #include <net/tcp.h> > #include <net/mptcp.h> > #include <net/proto_memory.h> >+#include <net/smc.h> > > #include <linux/compiler.h> > #include <linux/gfp.h> >@@ -759,14 +760,18 @@ 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) >+ struct sock *sk = &tp->inet_conn.icsk_inet.sk; > if (static_branch_unlikely(&tcp_have_smc)) { > if (tp->syn_smc) { >- if (*remaining >= TCPOLEN_EXP_SMC_BASE_ALIGNED) { >+ tp->syn_smc = !!smc_call_retops(1, sk, set_option, tp); >+ /* re-check syn_smc */ >+ if (tp->syn_smc && >+ *remaining >= TCPOLEN_EXP_SMC_BASE_ALIGNED) { > opts->options |= OPTION_SMC; > *remaining -= TCPOLEN_EXP_SMC_BASE_ALIGNED; > } >@@ -776,14 +781,19 @@ static void smc_set_option(const struct tcp_sock *tp, > } > > static void smc_set_option_cond(const struct tcp_sock *tp, >- const struct inet_request_sock *ireq, >+ struct inet_request_sock *ireq, > struct tcp_out_options *opts, > unsigned int *remaining) > { > #if IS_ENABLED(CONFIG_SMC) >+ const struct sock *sk = &tp->inet_conn.icsk_inet.sk; > if (static_branch_unlikely(&tcp_have_smc)) { > if (tp->syn_smc && ireq->smc_ok) { >- if (*remaining >= TCPOLEN_EXP_SMC_BASE_ALIGNED) { >+ ireq->smc_ok = !!smc_call_retops(1, sk, set_option_cond, >+ tp, ireq); >+ /* re-check smc_ok */ >+ if (ireq->smc_ok && >+ *remaining >= TCPOLEN_EXP_SMC_BASE_ALIGNED) { > opts->options |= OPTION_SMC; > *remaining -= TCPOLEN_EXP_SMC_BASE_ALIGNED; > } >diff --git a/net/smc/Kconfig b/net/smc/Kconfig >index ba5e6a2dd2fd..27f35064d04c 100644 >--- a/net/smc/Kconfig >+++ b/net/smc/Kconfig >@@ -33,3 +33,15 @@ config SMC_LO > of architecture or hardware. > > if unsure, say N. >+ >+config SMC_OPS >+ bool "Generic hook for SMC subsystem" >+ depends on SMC && BPF_SYSCALL >+ default n >+ help >+ SMC_OPS enables support to register generic hook via eBPF programs >+ 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. I'm still not completely satisfied with the name smc_ops. Since this will be the API for our users, we need to be carefull on the name. It seems like you're aiming to define a common set of operations, but the implementation appears to be intertwined with BPF. If this is intended to be a common interface, and if we are using another operation, there shouldn’t be a need to hold a BPF reference. As your 'help' sugguest, What about smc_hook ? Best regards, Dust