On 10.02.25 12:16, Guangguan Wang wrote:

On 2025/1/15 19:53, Guangguan Wang wrote:

quoted

On 2025/1/14 20:07, Halil Pasic wrote:

quoted

On Fri, 10 Jan 2025 13:43:44 +0800
Guangguan Wang [off-list ref] wrote:

quoted

quoted

I think I showed a valid and practical setup that would break with your
patch as is. Do you agree with that statement?

Did you mean
"
Now for something like a bond of two OSA
interfaces, I would expect the two legs of the bond to probably have a
"HW PNETID", but the netdev representing the bond itself won't have one
unless the Linux admin defines a software PNETID, which is work, and
can't have a HW PNETID because it is a software construct within Linux.
Breaking for example an active-backup bond setup where the legs have
HW PNETIDs and the admin did not bother to specify a PNETID for the bond
is not acceptable.
" ?
If the legs have HW pnetids, add pnetid to bond netdev will fail as
smc_pnet_add_eth will check whether the base_ndev already have HW pnetid.

If the legs without HW pnetids, and admin add pnetids to legs through smc_pnet.
Yes, my patch will break the setup. What Paolo suggests(both checking ndev and
base_ndev, and replace || by && )can help compatible with the setup.

I'm glad we agree on that part. Things are much more acceptable if we
are doing both base and ndev.

It is also acceptable for me.

quoted

Nevertheless I would like to understand
your problem better, and talk about it to my team. I will also ask some
questions in another email.

Questions are welcome.

quoted

That said having things work differently if there is a HW PNETID on
the base, and different if there is none is IMHO wonky and again
asymmetric.

Imagine the following you have your nice little setup with a PNETID on
a non-leaf and a base_ndev that has no PNETID. Then your HW admin
configures a PNETID to your base_ndev, a different one. Suddenly
your ndev PNETID is ignored for reasons not obvious to you. Yes it is
similar to having a software PNETID on the base_ndev and getting it
overruled by a HW PNETID, but much less obvious IMHO. I am wondering if there are any scenarios that require setting different

pnetids for different net devices in one netdev hierarchy. If no, maybe
we should limit that only one pnetid can be set to one netdev hierarchy.

quoted

I also think
a software PNETID of the base should probably take precedence over over
the software pnetid of ndev.

Agree!

Thanks,
Guangguan Wang

quoted

Regards,
Halil

Hi Halil,

Are there any questions or further discussions about this patch? If no, I will
send a v2 patch, in which software pnetid will be searched in both base_ndev and ndev,
and base_ndev will take precedence over ndev.

Thanks,
Guangguan Wang

Hi Guangguan,

Thank you for the detailed description and examples; I understand your 
situation better now. Paolo's suggestions (checking both ndev and 
base_ndev, and replacing || with &&) could indeed serve as a workaround 
for certain setups like yours. However, they might also introduce 
invalid topologies, one example is what Halil mentioned.

Therefore, neither suggestion is fully acceptable, whether from you or 
from Paolo. I agree that we should restrict it so that only one pnetid 
can be assigned to a single netdev hierarchy, based on the base ndev.

One preliminary idea I have is to enhance the smc_pnet -a -I <ethx> 
<pnetid> command to analyze the entire hierarchy first, ensuring that 
only one pnetid is assigned per netdev hierarchy.

Thanks,
Wenjia