On 2023/1/20 00:25, Alexandra Winter wrote:
On 20.12.22 04:21, Wen Gu wrote:
This patch introduces a kind of loopback device for SMC-D, thus
enabling the SMC communication between two local sockets in one
kernel.
The loopback device supports basic capabilities defined by SMC-D,
including registering DMB, unregistering DMB and moving data.
Considering that there is no ism device on other servers expect
IBM z13,
Please use the wording 'on other architectures except s390'.
That is how IBM Z is referred to in the Linux kernel.
Thanks, will use words consistent with this.
the loopback device can be used as a dummy device to
test SMC-D logic for the broad community.
Signed-off-by: Wen Gu <guwen@xxxxxxxxxxxxxxxxx>
---
Hello Wen Gu,
as the general design discussions are ongoing, I didn't
do a thorough review. But here are some general remarks
that you may want to consider for future versions.
I would propose to add a module parameter (default off) to enable
SMC-D loopback.
OK, will add a module parameter in the future version.
include/net/smc.h | 1 +
net/smc/Makefile | 2 +-
net/smc/af_smc.c | 12 ++-
net/smc/smc_cdc.c | 6 ++
net/smc/smc_cdc.h | 1 +
net/smc/smc_loopback.c | 282 +++++++++++++++++++++++++++++++++++++++++++++++++
net/smc/smc_loopback.h | 59 +++++++++++
7 files changed, 361 insertions(+), 2 deletions(-)
create mode 100644 net/smc/smc_loopback.c
create mode 100644 net/smc/smc_loopback.h
I am not convinced that this warrants a separate file.
IMHO, the dummy device used by smcd loopback is corresponding to ISM device.
So I put the dummy device implementation in smc_loopback.c alone, imitating
drivers/s390/net/ism_drv.c. I think it maybe clearer to do so.
[...]
+}
+
+static int lo_add_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
+{
+ return 0;
+}
+
+static int lo_del_vlan_id(struct smcd_dev *smcd, u64 vlan_id)
+{
+ return 0;
+}
+
+static int lo_set_vlan_required(struct smcd_dev *smcd)
+{
+ return 0;
+}
+
+static int lo_reset_vlan_required(struct smcd_dev *smcd)
+{
+ return 0;
+}
The VLAN functions are only required for SMC-Dv1
Seems you want to provide v1 support for loopback?
May be nice for testing v1 VLAN support.
But then you need proper VLAN support.
Based on the current discussion, I tend to only provide v2 support for loopback
since v2 is the general trend. So I will fix this in the future version.
[...]
+
+static u8 *lo_get_system_eid(void)
+{
+ return &LO_SYSTEM_EID.seid_string[0];
+}
SEID is for the whole system not per device.
We probably need to register a different function
for each architecture.
Yes, I agree.
+
+static u16 lo_get_chid(struct smcd_dev *smcd)
+{
+ return 0;
+}
+
Shouldn't this return 0xFFFF in your current concept?
Yes, this should return 0xFFFF.
I supplemented a patch as attachment in this earlier reply:
https://lore.kernel.org/netdev/b25f56de-7913-2a56-950f-dfe0defd6936@xxxxxxxxxxxxxxxxx/
and have amended this.
