Re: [PATCH net-next] net/smc: introduce shadow sockets for fallback connections

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

On 24.03.23 08:26, Kai wrote:
> 
> 
> 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.

I'm wondering how the usecase works? How are the server-client 
applications get accelerated by using SMC-R? If your case rely on the 
fallback, why don't use TCP/IP directly?

> 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