Re: [PATCH net-next v6 0/3] Introduce IPPROTO_SMC

[Wenjia Zhang <wenjia@linux.ibm.com>]

On 05.06.24 14:56, D. Wythe wrote:
> From: "D. Wythe" <alibuda@linux.alibaba.com>
> 
> This patch allows to create smc socket via AF_INET,
> similar to the following code,
> 
> /* create v4 smc sock */
> v4 = socket(AF_INET, SOCK_STREAM, IPPROTO_SMC);
> 
> /* create v6 smc sock */
> v6 = socket(AF_INET6, SOCK_STREAM, IPPROTO_SMC);
> 
> There are several reasons why we believe it is appropriate here:
> 
> 1. For smc sockets, it actually use IPv4 (AF-INET) or IPv6 (AF-INET6)
> address. There is no AF_SMC address at all.
> 
> 2. Create smc socket in the AF_INET(6) path, which allows us to reuse
> the infrastructure of AF_INET(6) path, such as common ebpf hooks.
> Otherwise, smc have to implement it again in AF_SMC path. Such as:
>    1. Replace IPPROTO_TCP with IPPROTO_SMC in the socket() syscall
>       initiated by the user, without the use of LD-PRELOAD.
>    2. Select whether immediate fallback is required based on peer's port/ip
>       before connect().
> 
> A very significant result is that we can now use eBPF to implement smc_run
> instead of LD_PRELOAD, who is completely ineffective in scenarios of static
> linking.
> 
> Another potential value is that we are attempting to optimize the
> performance of fallback socks, where merging socks is an important part,
> and it relies on the creation of SMC sockets under the AF_INET path.
> (More information :
> https://lore.kernel.org/netdev/1699442703-25015-1-git-send-email-alibuda@linux.alibaba.com/T/)
> 
> v2 -> v1:
> 
> - Code formatting, mainly including alignment and annotation repair.
> - move inet_smc proto ops to inet_smc.c, avoiding af_smc.c becoming too bulky.
> - Fix the issue where refactoring affects the initialization order.
> - Fix compile warning (unused out_inet_prot) while CONFIG_IPV6 was not set.
> 
> v3 -> v2:
> 
> - Add Alibaba's copyright information to the newfile
> 
> v4 -> v3:
> 
> - Fix some spelling errors
> - Align function naming style with smc_sock_init() to smc_sk_init()
> - Reversing the order of the conditional checks on clcsock to make the code more intuitive
> 
> v5 -> v4:
> 
> - Fix some spelling errors
> - Added comment, "/* CONFIG_IPV6 */", after the final #endif directive.
> - Rename smc_inet.h and smc_inet.c to smc_inet.h and smc_inet.c
> - Encapsulate the initialization and destruction of inet_smc in inet_smc.c,
>    rather than implementing it directly in af_smc.c.
> - Remove useless header files in smc_inet.h
> - Make smc_inet_prot_xxx and smc_inet_sock_init() to be static, since it's
>    only used in smc_inet.c
> 
> 
> v6 -> v5:
> 
> - Wrapping lines to not exceed 80 characters
> - Combine initialization and error handling of smc_inet6 into the same #if
>    macro block.
> 
> D. Wythe (3):
>    net/smc: refactoring initialization of smc sock
>    net/smc: expose smc proto operations
>    net/smc: Introduce IPPROTO_SMC
> 
>   include/uapi/linux/in.h |   2 +
>   net/smc/Makefile        |   2 +-
>   net/smc/af_smc.c        | 162 ++++++++++++++++++++++++++--------------------
>   net/smc/smc.h           |  38 +++++++++++
>   net/smc/smc_inet.c      | 169 ++++++++++++++++++++++++++++++++++++++++++++++++
>   net/smc/smc_inet.h      |  22 +++++++
>   6 files changed, 324 insertions(+), 71 deletions(-)
>   create mode 100644 net/smc/smc_inet.c
>   create mode 100644 net/smc/smc_inet.h
> 
Hi D.Wythe,

This version of the code looks good to me!
And I played with it on our platform, and did some basic testing for 
SMCR and SMCD. It works pretty well. I like it. Thank you for your effort!

Please feel free to add my signs for the whole patches series.
Reviewed-by: Wenjia Zhang <wenjia@linux.ibm.com>
Tested-by: Wenjia Zhang <wenjia@linux.ibm.com>

Thanks,
Wenjia