Re: [net-next 14/15] net/mlx5: Light probe local SFs

[Jiri Pirko <jiri@resnulli.us>]

Thu, Jun 15, 2023 at 09:33:25PM CEST, kuba@kernel.org wrote:
>On Thu, 15 Jun 2023 19:37:23 +0200 Jiri Pirko wrote:
>> Thu, Jun 15, 2023 at 06:37:01PM CEST, kuba@kernel.org wrote:
>> >On Thu, 15 Jun 2023 12:51:09 +0200 Jiri Pirko wrote:  
>> >> The problem is scalability. SFs could be activated in parallel, but the
>> >> cmd that is doing that holds devlink instance lock. That serializes it.
>> >> So we need to either:
>> >> 1) change the devlink locking to be able to execute some of the cmds in
>> >>    parallel and leave the activation sync
>> >> 2) change the activation to be async and work with notifications
>> >> 
>> >> I like 2) better, as the 1) maze we just got out of recently :)
>> >> WDYT?  
>> >
>> >I guess we don't need to wait for the full activation. Is the port
>> >creation also async, then, or just the SF devlink instance creation?  
>> 
>> I'm not sure I follow :/
>> The activation is when the SF auxiliary device is created. The driver then
>> probes the SF auxiliary device and instantiates everything, SF devlink,
>> SF netdev, etc.
>
>Sorry, maybe let's look at an example:
>
>$ devlink port add pci/0000:08:00.0 flavour pcisf pfnum 0 sfnum 11
>
>needs to print / return the handle of the created port.

Yep, that is what happens now.


>
>$ devlink port function set pci/0000:08:00.0/32768 \
>               hw_addr 00:00:00:00:00:11 state active
>
>needs to print / return the handle of the devlink instance

Right, that makes sense. I will look into adding that. It's a bit
tricky, as the instance might actually appear on a different host, then,
there is nothing to return here.


>
>> We need wait/notification for 2 reasons
>> 1) to get the auxiliary device name for the activated
>>    SF. It is needed for convenience of the orchestration tools.
>> 2) to get the result of the activation (success/fail)
>>    It is also needed for convenience of the orchestration tools.
>
>Are you saying the activation already waits for the devlink instance to
>be spawned? If so that's great, all we need to do is for the:

Yep.


>
>$ devlink port function set pci/0000:08:00.0/32768 \
>               hw_addr 00:00:00:00:00:11 state active
>
>to either return sufficient info for the orchestration to know what the
>resulting SF / SF devlink instance is. Most likely indirectly by adding
>that info to the port so that the PORT_NEW notification carries it.

Yeah, we need to add the same info to the GET cmd.


>
>Did I confuse things even more?

No, no confusion. But, the problem with this is that devlink port function set
active blocks until the activation is done holding the devlink instance
lock. That prevents other ports from being activated in parallel. From
driver/FW/HW perspective, we can do that.

So the question is, how to allow this parallelism?



>
>As a reminder what sparked this convo is that user specifies "sfnum 11"
>in the example, and the sf device gets called "sf.1".

Yeah, will look into that as well.