On Wed, Sep 29, 2021 at 10:55:37AM -0700, Jakub Kicinski wrote:

On Wed, 29 Sep 2021 18:31:51 +0300 Leon Romanovsky wrote:

quoted

On Wed, Sep 29, 2021 at 07:39:40AM -0700, Jakub Kicinski wrote:

quoted

On Wed, 29 Sep 2021 17:13:28 +0300 Leon Romanovsky wrote:  

quoted

We don't need to advertise counters for feature that is not supported.
In multiport mlx5 devices, the reload functionality is not supported, so
this change at least make that device to behave like all other netdev
devices that don't support devlink reload.

The ops structure is set very early to make sure that internal devlink
routines will be able access driver back during initialization (btw very
questionable design choice)  

Indeed, is this fixable? Or now that devlink_register() was moved to 
the end of probe netdev can call ops before instance is registered?
  

quoted

and at that stage the driver doesn't know
yet which device type it is going to drive.

So the answer is:
1. Can't have two structures.  

I still don't understand why. To be clear - swapping full op structures
is probably acceptable if it's a pure upgrade (existing pointers match).
Poking new ops into a structure (in alphabetical order if I understand
your reply to Greg, not destructor-before-contructor) is what I deem
questionable.  

It is sorted simply for readability and not for any other technical
reason.

Regarding new ops, this is how we are setting callbacks in RDMA based on
actual device support. It works like a charm.

quoted

quoted

2. Same behaviour across all netdev devices.  

Unclear what this is referring to.  

If your device doesn't support devlink reload, it won't print any
reload counters at all. It is not the case for the multiport mlx5
device. It doesn't support, but still present these counters.

There's myriad ways you can hide features.

Swapping ops is heavy handed and prone to data races, I don't like it.

I'm not swapping, but setting only in supported devices.

Anyway, please give me a chance to present improved version of this
mechanism and we will continue from there.

Thanks