Re: [PATCH net-next 00/15] net/smc: implement loopback-ism used by SMC-D

[Alexandra Winter <wintera@linux.ibm.com>]

On 11.01.24 13:00, Wen Gu wrote:
> This patch set acts as the second part of the new version of [1] (The first
> part can be referred from [2]), the updated things of this version are listed
> at the end.
> 
> # Background
> 
> SMC-D is now used in IBM z with ISM function to optimize network interconnect
> for intra-CPC communications. Inspired by this, we try to make SMC-D available
> on the non-s390 architecture through a software-implemented virtual ISM device,
> that is the loopback-ism device here, to accelerate inter-process or
> inter-containers communication within the same OS instance.


Hello Wen Gu,

thank you very much for this patchset. I have been looking at it a bit.
I installed in on a testserver, but did not yet excercise the loopback-ism device.
I want to continue investigations, but daily work interferes, so I thought I
send you some comments now. So this is not at all a code review, but some 
thoughts and observations about the general concept.


In [1] there was a discussion about an abstraction layer between smc-d and the
ism devices. 
I am not sure what you are proposing now, is it an smc-d feature or independent of smc?
In 3/15 you say it is part of the SMC module, but then it has its own device entry.
Didn't you want to use it for other things as well? Or is it an SMC-D only feature?
Is it a device (Config help: "kind of virtual device")? Or an SMC-D feature?

Will we have a class of ism devices (s390 ism, ism-loopback, virtio-ism)
That share common properties (internal API?)
and smc-d will work with any of those?
But they all can exist without smc ?! BTW: This is what we want for s390-ism.
The client-registration interface [2] is currently the way to achieve this. 
But maybe we need a more general concept?

Maybe first a preparation patchset that introduces a class/ism
Together with an improved API?
In case you want to use ISM devices for other purposes as well..
But then the whole picture of ism-loopback in one patchset (RFC?) 
has its benefits as well.


Other points that I noticed:

Naming: smc loopback, ism-loopback, loopback-ism ?

config: why not tristate? Why under net/smc?

/sys/devices/virtual/smc  does not initially show up in my installation!!!
root@t35lp50:/sys/devices/virtual/> ls
3270/  bdi/  block/  graphics/  iommu/  mem/  memory_tiering/  misc/  net/  tty/  vc/  vtconsole/  workqueue/
root@t35lp50:/sys/devices/virtual/> ls smc/loopback-ism
active  dmb_copy  dmbs_cnt  dmb_type  subsystem@  uevent  xfer_bytes
root@t35lp50:/sys/devices/virtual/> ls
3270/  bdi/  block/  graphics/  iommu/  mem/  memory_tiering/  misc/  net/  smc/  tty/  vc/  vtconsole/  workqueue/
Is that normal behaviour?

You introduced a class/smc
Maybe class/ism would be better?
The other smc interfaces do not show up in class/smc!! Not so good

Why doesn't it show in smc_rnics?
(Maybe some deficiency of smc_rnics?)

But then it shows in other smc-tools:
root@t35lp50:/sys/> smcd device
FID  Type  PCI-ID        PCHID  InUse  #LGs  PNET-ID
0000 0     loopback-ism  ffff   No        0
0029 ISM   0000:00:00.0  07c1   No        0  NET1
Nice!

Kind regards
Sandy


[1] https://lore.kernel.org/lkml/e3819550-7b10-4f9c-7347-dcf1f97b8e6b@linux.alibaba.com/
[2] 89e7d2ba61b7 ("net/ism: Add new API for client registration")