On 07.01.25 20:32, Halil Pasic wrote:

On Tue, 7 Jan 2025 09:44:30 +0100
Paolo Abeni [off-list ref] wrote:

quoted

On 12/27/24 5:04 AM, Guangguan Wang wrote:

...

quoted

quoted

The command 'smc_pnet -a -I <ethx> <pnetid>' will add <pnetid>
to the pnetid table and will attach the <pnetid> to net device
whose name is <ethx>. But When do SMCR by <ethx>, in function
smc_pnet_find_roce_by_pnetid, it will use <ethx>'s base ndev's
pnetid to match rdma device, not <ethx>'s pnetid. The asymmetric
use of the pnetid seems weird. Sometimes it is difficult to know
the hierarchy of net device what may make it difficult to configure
the pnetid and to use the pnetid. Looking into the history of
commit, it was the commit 890a2cb4a966 ("net/smc: rework pnet table")
that changes the ndev from the <ethx> to the <ethx>'s base ndev
when finding pnetid by pnetid table. It seems a mistake.

This patch changes the ndev back to the <ethx> when finding pnetid
by pnetid table.

Fixes: 890a2cb4a966 ("net/smc: rework pnet table")
Signed-off-by: Guangguan Wang <redacted>  

If I read correctly, this will break existing applications using the
lookup schema introduced by the blamed commit - which is not very
recent.

I agree that this patch may break existing applications or existing
configuration automation scripts.

...

PNETID stands for "Physical Network Identifier" and the idea is that iff
two ports are connected to the same physical network then they should
have the same PNETID. And on s390 PNETID can come and often is comming
"from the hardware". 

...


HW pnetids (smc_pnetid_by_dev_port()) are only visible at the base netdevice.
It seems that the pnetid table, managed by the smc_pnet tool, tries to mimick
that behaviour, and the concept (recommendation?) would be to set the 
user-defined pnetid also for the base netdevice and then use the upper
level netdevices for the tcp connection. Which makes some sense, 
all upper level devices have the same connectivity as the base device.

So this patch would break a setup that follows this concept and only sets the 
pnetid at the base netdevice.


Optionally you can set a user-defined pnetid on upper level devices (maybe for
usability??), but as Guangguan noticed, that has no practical impact.
In the documentation I see examples where the same pnetid is set for upper
and base device. 
You cannot set a user-defined pnetid on a upper level device, if the base
device has a HW pnetid (smc_pnet_add_eth()) which makes some sense,
not even the same pnetid (makes less sense IMO).
However you can set different user-defined pnetids on the upper netdevices
and the base device, which makes no sense to me.

quoted

Perhaps for a net patch would be better to support both lookup schemas
i.e.

	(smc_pnet_find_ndev_pnetid_by_table(ndev, ndev_pnetid) ||
	 smc_pnet_find_ndev_pnetid_by_table(base_ndev, ndev_pnetid))

?

This may yield undesirable results, if base pnetid and upper pnetid differ..
But then maybe GIGO?


... 

BTW to implement the logic proposed by you Paolo, as understood by me,
we would have to use "&&" instead of "||". 

+1


Another idea may be to change the setting of a user-defined pnetid
on an upper level netdevice to
- fail if the base netdevice has a different pnetid
- set the pnetid of the base device , if it is currently unset.