On 2024/6/5 00:16, Gerd Bayer wrote:
Hi Wen Gu, sorry for the late reply, I'm just catching up after a bit of a vacation.
No worries at all, I hope you had a great vacation!
On Fri, 2024-05-31 at 16:54 +0800, Wen Gu wrote:When copying smc settings to clcsock, avoid setting clcsock's sk_sndbuf to sysctl_tcp_wmem[1], since this may overwrite the value set by tcp_sndbuf_expand() in TCP connection establishment. And the other setting sk_{snd|rcv}buf to sysctl value in smc_adjust_sock_bufsizes() can also be omitted since the initialization of smc sock and clcsock has set sk_{snd|rcv}buf to smc.sysctl_{w|r}mem or ipv4_sysctl_tcp_{w|r}mem[1]. Fixes: 30c3c4a4497c ("net/smc: Use correct buffer sizes when switching between TCP and SMC") Link: https://lore.kernel.org/r/5eaf3858-e7fd-4db8-83e8-3d7a3e0e9ae2@xxxxxxxxxxxxxxxxx Signed-off-by: Wen Gu <guwen@xxxxxxxxxxxxxxxxx> --- FYI, The detailed motivation and testing can be found in the link above. ---
<...>
As Wenjia already said, we've discussed this a bit. As I remember, I've added the sections to copy over the sysctl values as a "safety measure" when moving between smc/clc sockets - but had the wrong assumption in mind that e.g. in a fall-back a new TCP handshake would be done. Apparently, we didn't test the buffer size behavior in these scenarios enough to notice the "weird" behavior. So we reviewed your initial report of the oddity per your message in the link above, too. We fully agree that if no connection at the SMC level could be established, you should expect the socket buffersizes be used that had been established for the TCP connection - regardless if the fallback is due to the server or the client. So feel free to add my Reviewed-by: Gerd Bayer <gbayer@xxxxxxxxxxxxx>, too.
Hi, Gerd and Wenjia. Thanks a lot for your confirmation. And as for the last question in the initial report (link above), that why the server does not call smc_copy_sock_settings_to_clc() like the client when fallback happens, I guess it is because at the time that server fallback, the new_smc sock has not been accepted, so there will be no user's sock settings that needs to be copied to clcsock. Thanks!
Thanks, Gerd
