Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Message-ID: <e3819550-7b10-4f9c-7347-dcf1f97b8e6b@linux.alibaba.com>
Date:   Wed, 4 Oct 2023 17:05:57 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.15.1
Subject: Re: [PATCH net-next v4 09/18] net/smc: introduce SMC-D loopback
 device
To:     Alexandra Winter <wintera@linux.ibm.com>,
        Jan Karcher <jaka@linux.ibm.com>, dust.li@linux.alibaba.com,
        kgraul@linux.ibm.com, wenjia@linux.ibm.com, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com
Cc:     schnelle@linux.ibm.com, gbayer@linux.ibm.com, pasic@linux.ibm.com,
        alibuda@linux.alibaba.com, tonylu@linux.alibaba.com,
        linux-s390@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <1695568613-125057-1-git-send-email-guwen@linux.alibaba.com>
 <1695568613-125057-10-git-send-email-guwen@linux.alibaba.com>
 <3febdf3e-e213-7acf-7dd4-75d177676c3e@linux.ibm.com>
 <20230925151816.GC92403@linux.alibaba.com>
 <3f71928e-157a-748e-42ee-4de3c80ed109@linux.ibm.com>
 <e85fe903-a025-a693-906b-834ff2a2a812@linux.ibm.com>
 <22858b56-dee0-e65f-a698-b0f2090a872d@linux.alibaba.com>
 <c2117ecb-466c-cc45-04c5-f12adedec217@linux.ibm.com>
From:   Wen Gu <guwen@linux.alibaba.com>
In-Reply-To: <c2117ecb-466c-cc45-04c5-f12adedec217@linux.ibm.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk


On 2023/9/29 22:08, Alexandra Winter wrote:
> 
> 
> On 28.09.23 20:35, Wen Gu wrote:
>>
>>
>> On 2023/9/28 11:16, Jan Karcher wrote:
>>>
>>>
>>> On 26/09/2023 09:24, Alexandra Winter wrote:
>>>>
>>>>
>>>> On 25.09.23 17:18, Dust Li wrote:
>>>>>> Hello Wen Gu,
>>>>>>
>>>>>> thank you for adding the Kconfig, so the distributions can decide when to offer this feature.
>>>>>>
>>>>>> I propose you add some kind of runtime switch as well. Not every user who loads the SMC module
>>>>>> may want to exploit smcd-loopback. Especially in native environements without containers.
>>>>>>
>>>>>> If no RoCE interfaces or no ISM interfaces exist, the respective handling is skipped in SMC.
>>>>>> If loopback is always created unconditionally, there is no way to opt-out.
>>>>> Hi Sandy,
>>>>>
>>>>> After talking to Wen Gu offline, I think the real issue here might be
>>>>> we don't have an abstract layer in SMC, something like net/core/dev.c
>>>>>
>>>>> Without this, we cannot do:
>>>>>
>>>>> 1. Enable/disable those devices dynamically
>>>>>      Currently, If we want to disable a SMC-R device to communicate with
>>>>>      others, we need to refer to 'ip link set dev xxx down' to disable the
>>>>>      netdevice, then Infiniband subsystem will notify SMC that the state of
>>>>>      the IB device has changed. We cannot explicitly choose not to use some
>>>>>      specific IB/RoCE devices without disable totally.
>>>>>      If the loopback device need to support enable/disable itself, I
>>>>>      think it might be better to enable this feature for all SMC devices.
>>>>>
>>>>> 2. Do statistics per device
>>>>>      Now, we have to relay on IB/RoCE devices' hardware statistics to see
>>>>>      how many packets/bytes we have sent through this device.
>>>>>
>>>>> Both the above issues get worse when the IB/RoCE device is shared by SMC
>>>>> and userspace RDMA applications. If SMC-R and userspace RDMA applications
>>>>> run at the same time, we can't enable the device to run userspace RDMA
>>>>> applications while block it from running SMC. For statistics, we cannot
>>>>> tell how many packets/bytes were sent by SMC and how many were sent by
>>>>> userspace RDMA applications.
>>>>>
>>>>> So I think those are better to support in the SMC layer.
>>>>>
>>>>> Best regards!
>>>>> Dust
>>>>
>>>> Thank you very much for your considerations. I also think a generic handling
>>>> of these requirements in the smc layer would be best. Especially, if we want
>>>> to add virtio-ism support soon. There we will face the same issues again.
>>>> Let's hear what others think about this.
>>>>
>>>>
>>>
>>> Thanks you Sandy for bringing it up and Dust Li & Wen Gu for your thoughts.
>>> I agree that such a runtime switch is needed and also that this generic handling would be good in the smc layer.
>>
>> Right. runtime switch is necessary. I'm trying some ways to see which one is more suitable.
>>
>>
>> As for implementing a abstract layer that capable of handling 1) enable/disable SMC usage of
>> RDMA/ISM devices. 2) count packets/bytes of RDMA/ISM devices that generated/consumed by SMC,
>> I believe it would be helpful, and IMHO its architecture may be:
>>
>> ----------------------------------------------
>>                    SMC protocol
>>      (af_smc.c / smc_core.c / smc_clc.c ...)
>> ----------------------------------------------
>>            Abstract layer of SMC device
>>        (define SMC device common operations)
>> ----------------------------------------------
>>    RDMA device |        (virt) ISM device
>>    (smc_ib.c)  |   (smc_ism.c / smc_loopback.c)
>> ----------------------------------------------
>>
>> But I also believe this may require a lot of works and may be a long-term job.
>>
> 
> I like that concept a lot. If we can agree on a direction, we can define
> meaningful pieces and approach it piece by piece.
> 

Yes. It can be added to our interlock's backup list.

> 
>> If only for the virtual ISM device, e.g.loopback-ism, I am considering adding it to the Linux
>> device tree (/sys/devices/virtual/) to make it more 'device-like', and controlling its
>> enable/disable and get the statistics through some files, such as
>> echo 1 > /sys/devices/virtual/loopback-ism/alive
>> or
>> cat /sys/devices/virtual/loopback-ism/statistics/{rx|tx}_{bytes|packets}
>> (similar to what tcp lo have in /sys/devices/virtual/net/lo)
>>
>> What are your thoughts on it? Thanks.
>>
> 
> Makes sense to me, but I don't have too much experience in that area.
> I have never seen an attribute called 'alive' before.
> I think attributes like 'power', 'enable' or 'online' are used for other device types.
> 

Thanks. I will refer to existing devices for reference.

>>
>> -- 
>> A little off-topic, it's currently China's National Day holiday, which lasts for about a week,
>> so we are now on vacation. As a result, my responses might be a bit slower, but I will still
>> make time to check/reply the mail and prepare for my new version. Thank you all very much!
>>
>> Regards,
>> Wen Gu
> 
> Next week is Germany's national holiday, so many of us are out as well.

Have a nice holiday! :)