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

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

On 29.01.23 12:48, Dust Li wrote:
> 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 ?
> 
Since SMC is the main user of the ISM currently, we still want to keep 
the dependency for now, Sure, I agree with you, in the future we should 
remove the dependency.

> 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 ?
> 
You didn't miss anything, that’s just for the usage in case

> 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