Date:   Fri, 24 Mar 2023 15:26:44 +0800
From:   Kai <KaiShen@...ux.alibaba.com>
To:     Wenjia Zhang <wenjia@...ux.ibm.com>, kgraul@...ux.ibm.com,
        jaka@...ux.ibm.com, kuba@...nel.org, davem@...emloft.net,
        dsahern@...nel.org
Cc:     netdev@...r.kernel.org, linux-s390@...r.kernel.org,
        linux-rdma@...r.kernel.org
Subject: Re: [PATCH net-next] net/smc: introduce shadow sockets for fallback
 connections



On 3/23/23 1:09 AM, Wenjia Zhang wrote:
> 
> 
> On 21.03.23 08:19, Kai Shen wrote:
>> SMC-R performs not so well on fallback situations right now,
>> especially on short link server fallback occasions. We are planning
>> to make SMC-R widely used and handling this fallback performance
>> issue is really crucial to us. Here we introduce a shadow socket
>> method to try to relief this problem.
>>
> Could you please elaborate the problem?

Here is the background. We are using SMC-R to accelerate server-client 
applications by using SMC-R on server side, but not all clients use 
SMC-R. So in these occasions we hope that the clients using SMC-R get 
acceleration while the clients that fallback to TCP will get the 
performance no worse than TCP.
What's more, in short link scenario we may use fallback on purpose for
SMC-R perform badly with its highly cost connection establishing path.
So it is very important that SMC-R perform similarly as TCP on fallback 
occasions since we use SMC-R as a acceleration method and performance 
compromising should not happen when user use TCP client to connect a 
SMC-R server.
In our tests, fallback SMC-R accepting path on server-side contribute to 
the performance gap compared to TCP a lot as mentioned in the patch and 
we are trying to solve this problem.

> 
> Generally, I don't have a good feeling about the two non-listenning 
> sockets, and I can not see why it is necessary to introduce the socket 
> actsock instead of using the clcsock itself. Maybe you can convince me 
> with a good reason.
>
First let me explain why we use two sockets here.
We want the fallback accept path to be similar as TCP so all the 
fallback connection requests should go to the fallback sock(accept 
queue) and go a shorter path (bypass tcp_listen_work) while clcsock 
contains both requests with syn_smc and fallback requests. So we pick 
requests with syn_smc to actsock and fallback requests to fbsock.
I think it is the right strategy that we have two queues for two types 
of incoming requests (which will lead to good performance too).
On the other hand, the implementation of this strategy is worth discussing.
As Paolo said, in this implementation only the shadow socket's receive 
queue is needed. I use this two non-listenning sockets for these 
following reasons.
1. If we implement a custom accept, some of the symbols are not 
accessible since they are not exported(like mem_cgroup_charge_skmem).
2. Here we reuse the accept path of TCP so that the future update of TCP
may not lead to problems caused by the difference between the custom 
accept and future TCP accept.
3. SMC-R is trying to behave like TCP and if we implement custom accept, 
there may be repeated code and looks not cool.

Well, i think two queues is the right strategy but I am not so sure 
about which implement is better and we really want to solve this 
problem. Please give advice.

>> +static inline bool tcp_reqsk_queue_empty(struct sock *sk)
>> +{
>> +    struct inet_connection_sock *icsk = inet_csk(sk);
>> +    struct request_sock_queue *queue = &icsk->icsk_accept_queue;
>> +
>> +    return reqsk_queue_empty(queue);
>> +}
>> +
> Since this is only used by smc, I'd like to suggest to use 
> smc_tcp_reqsk_queue_empty instead of tcp_reqsk_queue_empty.
>
Will do.

Thanks

Kai

Powered by blists - more mailing lists