[RFC PATCH v4 2/4] dpll: Add DPLL framework base functions

[Jiri Pirko]

Fri, Dec 09, 2022 at 05:19:42PM CET, kuba at kernel.org wrote:
>On Fri, 9 Dec 2022 10:29:53 +0100 Jiri Pirko wrote:
>> Thu, Dec 08, 2022 at 06:05:17PM CET, kuba at kernel.org wrote:
>> >On Thu, 8 Dec 2022 17:33:28 +0100 Jiri Pirko wrote:  
>> >> For any synce pin manipulation over dpll netlink, we can use the netns
>> >> check of the linked netdev. This is the netns aware leg of the dpll,
>> >> it should be checked for.  
>> >
>> >The OCP card is an atomic clock, it does not have any networking.  
>> 
>> Sure, so why it has to be netns aware if it has nothing to do with
>> networking?
>
>That's a larger question, IDK if broadening the scope of the discussion
>will help us reach a conclusion. 
>
>The patchset as is uses network namespaces for permissions:
>
>+		.flags	= GENL_UNS_ADMIN_PERM,

Yeah, I wonder if just GENL_ADMIN_PERM wuldn't be more suitable here...


>
>so that's what I'm commenting on - aligning visibility of objects with
>already used permissions.
>
>> >> I can't imagine practically havind the whole dpll instance netns aware.
>> >> Omitting the fact that it really has no meaning for non-synce pins, what
>> >> would be the behaviour when for example pin 1 is in netns a, pin 2 in
>> >> netns b and dpll itself in netns c?  
>> >
>> >To be clear I don't think it's a bad idea in general, I've done 
>> >the same thing for my WIP PSP patches. But we already have one
>> >device without netdevs, hence I thought maybe devlink. So maybe
>> >we do the same thing with devlink? I mean - allow multiple devlink
>> >instances to be linked and require caps on any of them?  
>> 
>> I read this 5 times, I'm lost, don't understand what you mean :/
>
>Sorry I was replying to both paragraphs here, sorry.
>What I thought you suggested is we scope the DPLL to whatever the
>linked netdevs are scoped to? If netns has any of the netdevs attached
>to the DPLL then it can see the DPLL and control it as well.

Okay, that would make sense.
GENL_UNS_ADMIN_PERM | GENL_UNS_ADMIN_PERM
then.

>
>What I was saying is some DPLL have no netdevs. So we can do the same
>thing with devlinks. Let the driver link the DPLL to one or more
>devlink instances, and if any of the devlink instances is in current
>netns then you can see the DPLL.

I don't think that would be needed to pull devlink into the picture.
If not netdev is linked to dpll, GENL_ADMIN_PERM would apply.