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

From: Jakub Kicinski <kuba@kernel.org>
To: Jiri Pirko <jiri@resnulli.us>
Cc: "Kubalewski, Arkadiusz" <arkadiusz.kubalewski@intel.com>,
	Vadim Fedorenko <vfedorenko@novek.ru>,
	Jonathan Lemon <jonathan.lemon@gmail.com>,
	Paolo Abeni <pabeni@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Vadim Fedorenko <vadfed@fb.com>,
	linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org,
	"Olech, Milena" <milena.olech@intel.com>,
	"Michalik, Michal" <michal.michalik@intel.com>
Subject: Re: [RFC PATCH v4 2/4] dpll: Add DPLL framework base functions
Date: Fri, 9 Dec 2022 08:19:42 -0800	[thread overview]
Message-ID: <20221209081942.565bc422@kernel.org> (raw)
In-Reply-To: <Y5MAEQ74trsNFQQc@nanopsycho>

On Fri, 9 Dec 2022 10:29:53 +0100 Jiri Pirko wrote:
> Thu, Dec 08, 2022 at 06:05:17PM CET, kuba@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,

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.

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.