netdev - Re: [PATCH net-next v4 00/18] net/smc: implement virtual ISM extension and loopback-ism

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 27 Sep 2023 17:16:38 +0200
From: Alexandra Winter <wintera@...ux.ibm.com>
To: Wen Gu <guwen@...ux.alibaba.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 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



When I installed the patches, I noticed that 
> smcd info
showed an SEID, even though I had no ISM device --> good


> 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.


I was able to do SMC transfers via the smcd-loopback feature :-D