| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250123073034.GQ89233@linux.alibaba.com>
Date: Thu, 23 Jan 2025 15:30:34 +0800
From: Dust Li <dust.li@...ux.alibaba.com>
To: "D. Wythe" <alibuda@...ux.alibaba.com>, kgraul@...ux.ibm.com,
wenjia@...ux.ibm.com, jaka@...ux.ibm.com, ast@...nel.org,
daniel@...earbox.net, andrii@...nel.org, martin.lau@...ux.dev,
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,
guwen@...ux.alibaba.com
Cc: kuba@...nel.org, davem@...emloft.net, netdev@...r.kernel.org,
linux-s390@...r.kernel.org, linux-rdma@...r.kernel.org,
bpf@...r.kernel.org
Subject: Re: [PATCH bpf-next v7 3/6] net/smc: Introduce generic hook smc_ops
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@...ux.alibaba.com>
>---
> 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
Powered by blists - more mailing lists