netdev - Re: [net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

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]

Message-ID: <20230129114821.GF74595@linux.alibaba.com>
Date:   Sun, 29 Jan 2023 19:48:21 +0800
From:   Dust Li <dust.li@...ux.alibaba.com>
To:     Jan Karcher <jaka@...ux.ibm.com>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>
Cc:     netdev@...r.kernel.org, linux-s390@...r.kernel.org,
        Heiko Carstens <hca@...ux.ibm.com>,
        Alexandra Winter <wintera@...ux.ibm.com>,
        Wenjia Zhang <wenjia@...ux.ibm.com>,
        Thorsten Winkler <twinkler@...ux.ibm.com>,
        Stefan Raspl <raspl@...ux.ibm.com>,
        Karsten Graul <kgraul@...ux.ibm.com>,
        Nils Hoppmann <niho@...ux.ibm.com>,
        Halil Pasic <pasic@...ux.ibm.com>,
        Tony Lu <tonylu@...ux.alibaba.com>,
        Wen Gu <guwen@...ux.alibaba.com>
Subject: Re: [net-next v2 0/8] drivers/s390/net/ism: Add generalized interface

On Mon, Jan 23, 2023 at 07:17:44PM +0100, Jan Karcher wrote:
>Previously, there was no clean separation between SMC-D code and the ISM
>device driver.This patch series addresses the situation to make ISM available
>for uses outside of SMC-D.
>In detail: SMC-D offers an interface via struct smcd_ops, which only the
>ISM module implements so far. However, there is no real separation between
>the smcd and ism modules, which starts right with the ISM device
>initialization, which calls directly into the SMC-D code.
>This patch series introduces a new API in the ISM module, which allows
>registration of arbitrary clients via include/linux/ism.h: struct ism_client.
>Furthermore, it introduces a "pure" struct ism_dev (i.e. getting rid of
>dependencies on SMC-D in the device structure), and adds a number of API
>calls for data transfers via ISM (see ism_register_dmb() & friends).
>Still, the ISM module implements the SMC-D API, and therefore has a number
>of internal helper functions for that matter.
>Note that the ISM API is consciously kept thin for now (as compared to the
>SMC-D API calls), as a number of API calls are only used with SMC-D and
>hardly have any meaningful usage beyond SMC-D, e.g. the VLAN-related calls.

Hi,

Great work ! This makes the SMC & ISM code much more clear !

I like this patchset, just some questions on this refactor.
I still see there are some SMC related code in
'drivers/s390/net/ism_drv.c', mainly to implement smcd_ops.

As ISM is the lower layer of SMC, I think remove the dependency
on SMC would be better ? Do you have any plan to do that ?

One more thing:
I didn't find any call for smcd_ops->set_vlan_required/reset_vlan_required,
looks it's not needed, so why not remove it, am I missed something ?

Thanks!

>
>v1 -> v2:
>  Removed s390x dependency which broke config for other archs.
>
>Stefan Raspl (8):
>  net/smc: Terminate connections prior to device removal
>  net/ism: Add missing calls to disable bus-mastering
>  s390/ism: Introduce struct ism_dmb
>  net/ism: Add new API for client registration
>  net/smc: Register SMC-D as ISM client
>  net/smc: Separate SMC-D and ISM APIs
>  s390/ism: Consolidate SMC-D-related code
>  net/smc: De-tangle ism and smc device initialization
>
> drivers/s390/net/ism.h     |  19 +-
> drivers/s390/net/ism_drv.c | 376 ++++++++++++++++++++++++++++++-------
> include/linux/ism.h        |  98 ++++++++++
> include/net/smc.h          |  24 +--
> net/smc/af_smc.c           |   9 +-
> net/smc/smc_clc.c          |  11 +-
> net/smc/smc_core.c         |  13 +-
> net/smc/smc_diag.c         |   3 +-
> net/smc/smc_ism.c          | 180 ++++++++++--------
> net/smc/smc_ism.h          |   3 +-
> net/smc/smc_pnet.c         |  40 ++--
> 11 files changed, 560 insertions(+), 216 deletions(-)
> create mode 100644 include/linux/ism.h
>
>-- 
>2.25.1