From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3D35F19BD8 for ; Mon, 31 Jul 2023 17:03:42 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4157CC433CA; Mon, 31 Jul 2023 17:03:42 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1690823022; bh=BHKJOJ2tohKYMhbBN1dMeyRlyJh9OizTUmw+8ksIt/c=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=YJQ6TfheP/NEzwcZD3EDwNjtGIIu6Bu1hVTzjEw657o/yAyj/3UDvHlhy3QdH8Uow o3P/31dnfVwOPhj1sLbq/ZL1qFVhz/hA78CBYYlemS7RqoSlAc0UzHp4ikUfxlLE6j NdnR6NY1XezMVWKJS3mcIYE0Udlg8fRu4HvRwR/wJ+CkNmzwMmbqVmbezF+jXORNWs 5tHzCLh3zjJO+sr5cEXgth68jGUfH/SXKPVZpozuwr44oJ4C8hqJcOZcW35sMNLCEW zKWcDFz0MBD41pL7zQg7rIU4UVNrctlkpZ/MHCF1HsPlk5vLFFGpDXdwSNf/YFueqr /jTR0S7vBZ4Yg== Date: Mon, 31 Jul 2023 10:03:41 -0700 From: Jakub Kicinski To: Jiri Pirko Cc: netdev@vger.kernel.org, pabeni@redhat.com, davem@davemloft.net, edumazet@google.com, moshe@nvidia.com, saeedm@nvidia.com, idosch@nvidia.com, petrm@nvidia.com Subject: Re: [patch net-next v2 10/11] devlink: introduce dump selector attr and use it for per-instance dumps Message-ID: <20230731100341.4809a372@kernel.org> In-Reply-To: References: <20230720121829.566974-1-jiri@resnulli.us> <20230720121829.566974-11-jiri@resnulli.us> <20230725114044.402450df@kernel.org> Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Mon, 31 Jul 2023 14:47:08 +0200 Jiri Pirko wrote: > >Why not declare a fully nested policy with just the two attrs? > > Not sure I follow. But the nest under DEVLINK_ATTR_DUMP_SELECTOR has > its own policy, generated by devlink_nl_dump_selector_policy_init(). I > did it this way instead of separate policy array for 2 reasons: > 1) We don't have duplicate and possibly conflicting policies for devlink > root and selector > 2) It is easy for specific object type to pass attrs that are included > in the policy initialization (see the health reporter extension later > in this patchset). There are couple of object to benefit from this, > for example "sb". > 3) It is I think a bit nicer for specific object type to pass array of > attrs, instead of a policy array that would be exported from netlink.c > > If you insist on separate policy arrays, I can do it though. IMO the contents of the series do not justify the complexity. > I had it like that initially, I just decided to go this way for the 3 reasons > listed above. > > >Also - do you know of any userspace which would pass garbage attrs > >to the dumps? Do we really need to accept all attributes, or can > >we trim the dump policies to what's actually supported? > > That's what this patch is doing. It only accepts what the kernel > understands. It gives the object types (as for example health reporter) > option to extend the attr set to accept them into selectors as well, if > they know how to handle them. I'm talking about the "outer" policy, the level at which DEVLINK_ATTR_DUMP_SELECTOR is defined.