All of lore.kernel.org
 help / color / mirror / Atom feed
From: Johannes Berg <johannes@sipsolutions.net>
To: Jakub Kicinski <kuba@kernel.org>, Jiri Pirko <jiri@resnulli.us>,
	Michal Kubecek <mkubecek@suse.cz>,
	dsahern@kernel.org, pablo@netfilter.org
Cc: netdev@vger.kernel.org
Subject: Re: Genetlink per cmd policies
Date: Wed, 30 Sep 2020 18:06:28 +0200	[thread overview]
Message-ID: <fce613c2b4c797de4be413afddf872fd6dae9ef8.camel@sipsolutions.net> (raw)
In-Reply-To: <20200930084955.71a8c0ba@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

Hi Jakub,

> I'd like to be able to dump ethtool nl policies, but right now policy
> dumping is only supported for "global" family policies.

Did ethtool add per-command policies?

> Is there any reason not to put the policy in struct genl_ops, 
> or just nobody got to it, yet?
> 
> I get the feeling that this must have been discussed before...

Sort of, yeah.

We actually *had* per-command policies, and I removed it in commit
3b0f31f2b8c9 ("genetlink: make policy common to family"), mostly because
the maxattr is actually in the family not the op, so having a policy in
each op was never really a good idea and well-supported.

Additionally, having the pointer in each op (and if you want to do it
right you also need maxattr in each op) significantly increases the
binary size there, as well as boilerplate code to type it out, though
the latter could be avoided by "falling back" to the family policy.

So at the time, that reduced nl80211 size by 824 bytes, which I guess
means we had 103 ops at the time. Doing it right with maxattr would cost
twice as much as just the policy pointer, and we probably have a few
more ops now, so we're looking at a cost of ~1.6k for it.

Personally, I'm not really convinced that there really is a need for it,
but then I haven't really looked that much at ethtool. In most cases,
you want some "common" attributes for the family, even if it's only the
interface index or such, because also on the userspace side that's
awkward if you have to really build *everything* including such
identifying information per command. The one or two families that
actually had different policies per command (at the time I removed it)
actually solved this by "aliasing" the same attribute number between the
different policies, but again that feels rather awkward ...

That's the historic info I guess - I'll take a look at ethtool later and
see what it's doing there.

johannes


  parent reply	other threads:[~2020-09-30 16:06 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-30 15:49 Genetlink per cmd policies Jakub Kicinski
2020-09-30 16:03 ` Michal Kubecek
2020-09-30 16:11   ` Jakub Kicinski
2020-09-30 16:06 ` Johannes Berg [this message]
2020-09-30 16:17   ` Johannes Berg
2020-09-30 16:44     ` Jakub Kicinski
2020-09-30 18:36       ` Johannes Berg
2020-09-30 18:42         ` Michal Kubecek
2020-09-30 18:42           ` Johannes Berg
2020-09-30 19:01         ` Jakub Kicinski
2020-09-30 19:03           ` Johannes Berg
2020-09-30 19:14             ` Jakub Kicinski
2020-09-30 19:15               ` Johannes Berg
2020-09-30 19:46                 ` Jakub Kicinski
2020-09-30 20:13                   ` Johannes Berg
2020-09-30 20:47                     ` Jakub Kicinski
2020-09-30 23:38                       ` Andrew Lunn
2020-10-01  0:23                         ` Jakub Kicinski
2020-10-01  1:53                           ` Andrew Lunn
2020-10-01 15:50                             ` Jakub Kicinski
2020-10-01 15:52                               ` Johannes Berg
2020-09-30 16:18   ` Jakub Kicinski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=fce613c2b4c797de4be413afddf872fd6dae9ef8.camel@sipsolutions.net \
    --to=johannes@sipsolutions.net \
    --cc=dsahern@kernel.org \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=mkubecek@suse.cz \
    --cc=netdev@vger.kernel.org \
    --cc=pablo@netfilter.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.