[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1a6ef713-e5fd-3af7-fd57-966d6ddda9a4@linux.alibaba.com>
Date: Fri, 29 Sep 2023 00:42:17 +0800
From: Wen Gu <guwen@...ux.alibaba.com>
To: Alexandra Winter <wintera@...ux.ibm.com>, kgraul@...ux.ibm.com,
wenjia@...ux.ibm.com, jaka@...ux.ibm.com, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com
Cc: schnelle@...ux.ibm.com, gbayer@...ux.ibm.com, pasic@...ux.ibm.com,
alibuda@...ux.alibaba.com, tonylu@...ux.alibaba.com,
dust.li@...ux.alibaba.com, linux-s390@...r.kernel.org,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next v4 00/18] net/smc: implement virtual ISM
extension and loopback-ism
On 2023/9/27 23:16, Alexandra Winter wrote:
>
>
> On 24.09.23 17:16, Wen Gu wrote:
>> Wen Gu (18):
>> net/smc: decouple ism_dev from SMC-D device dump
>> net/smc: decouple ism_dev from SMC-D DMB registration
>> net/smc: extract v2 check helper from SMC-D device registration
>> net/smc: support SMCv2.x supplemental features negotiation
>> net/smc: reserve CHID range for SMC-D virtual device
>> net/smc: extend GID to 128bits only for virtual ISM device
>> net/smc: disable SEID on non-s390 architecture
>> net/smc: enable virtual ISM device feature bit
>> net/smc: introduce SMC-D loopback device
>> net/smc: implement ID-related operations of loopback
>> net/smc: implement some unsupported operations of loopback
>> net/smc: implement DMB-related operations of loopback
>> net/smc: register loopback device as SMC-Dv2 device
>> net/smc: add operation for getting DMB attribute
>> net/smc: add operations for DMB attach and detach
>> net/smc: avoid data copy from sndbuf to peer RMB in SMC-D
>> net/smc: modify cursor update logic when sndbuf mapped to RMB
>> net/smc: add interface implementation of loopback device
>>
>> drivers/s390/net/ism_drv.c | 20 +-
>> include/net/smc.h | 32 ++-
>> include/uapi/linux/smc.h | 3 +
>> include/uapi/linux/smc_diag.h | 2 +
>> net/smc/Kconfig | 13 ++
>> net/smc/Makefile | 2 +-
>> net/smc/af_smc.c | 88 ++++++--
>> net/smc/smc.h | 7 +
>> net/smc/smc_cdc.c | 56 ++++-
>> net/smc/smc_cdc.h | 1 +
>> net/smc/smc_clc.c | 64 ++++--
>> net/smc/smc_clc.h | 10 +-
>> net/smc/smc_core.c | 111 +++++++++-
>> net/smc/smc_core.h | 9 +-
>> net/smc/smc_diag.c | 11 +-
>> net/smc/smc_ism.c | 100 ++++++---
>> net/smc/smc_ism.h | 24 ++-
>> net/smc/smc_loopback.c | 489 ++++++++++++++++++++++++++++++++++++++++++
>> net/smc/smc_loopback.h | 54 +++++
>> net/smc/smc_pnet.c | 4 +-
>> 20 files changed, 996 insertions(+), 104 deletions(-)
>> create mode 100644 net/smc/smc_loopback.c
>> create mode 100644 net/smc/smc_loopback.h
>
>
> Hello Wen Gu,
>
> I applied and built your patches and noticed some things that you may want to consider in the next version:
>
> Series should be split up [2]
>
> Several lines exceed 80 columns [1][3]
>
> 'git clang-format HEAD~18' finds several formatting issues.
> Maybe not all of them need to be fixed.
>
> codespell *.patch
> 0006-net-smc-extend-GID-to-128bits-only-for-virtual-ISM-d.patch:7: protocal ==> protocol
>
> With your patches applied I get some new warnings [4]:
> Seems there are some ntoh conversions missing
>
> CHECK net/smc/af_smc.c
> net/smc/af_smc.c:723:32: warning: cast to restricted __be64
> net/smc/af_smc.c:1427:52: warning: cast to restricted __be64
> CHECK net/smc/smc_pnet.c
> CHECK net/smc/smc_ib.c
> CHECK net/smc/smc_clc.c
> net/smc/smc_clc.c:954:72: warning: incorrect type in argument 1 (different base types)
> net/smc/smc_clc.c:954:72: expected unsigned short [usertype] chid
> net/smc/smc_clc.c:954:72: got restricted __be16 [usertype] chid
> net/smc/smc_clc.c:1050:29: warning: incorrect type in assignment (different base types)
> net/smc/smc_clc.c:1050:29: expected unsigned long long [usertype] gid
> net/smc/smc_clc.c:1050:29: got restricted __be64 [usertype]
> net/smc/smc_clc.c:1051:31: warning: incorrect type in assignment (different base types)
> net/smc/smc_clc.c:1051:31: expected unsigned long long [usertype] token
> net/smc/smc_clc.c:1051:31: got restricted __be64 [usertype]
>
>
> [1] linux/Documentation/process/coding-style.rst
> [2] https://www.kernel.org/doc/html/v6.3/process/maintainer-netdev.html?highlight=network
> [3] scripts/checkpatch.pl --strict --max-line-length=80 --git HEAD-18
> [4] make C=2 CF=-D__CHECK_ENDIAN__ M=net/smc -Wunused-function -Wimplicit-fallthrough -Wincompatible-function-pointer-types-strict
>
>
Hi Sandy,
Thank you very much for your detailed comments. They are really helpful.
I will check and fix them in my next version.
>
> When I installed the patches, I noticed that
>> smcd info
> showed an SEID, even though I had no ISM device --> good
>
Yes, virtual ISM device will return the right SEID (same as that returned
by ISM device) on s390 arch.
>
>> smcd device
> FID Type PCI-ID PCHID InUse #LGs PNET-ID
> 0000 0 0000 Yes 2
>
> This needs some improvements.., but I'm not sure what is the best way to display virtual smcd interfaces in the smc-tools.
>
Right. My idea is to add the display of the device's name and GID
along with the PCI information (all zero) to indicate it's virtual
smcd device.
>
> I was able to do SMC transfers via the smcd-loopback feature :-D
>
Glad to hear this. I will improve the issues you mentioned in the next
version and next version will be split up.
Thank you very much.
Wen Gu
Powered by blists - more mailing lists