netdev - Re: Genetlink per cmd policies

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200930091825.27d14d1d@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Wed, 30 Sep 2020 09:18:25 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     Jiri Pirko <jiri@...nulli.us>, Michal Kubecek <mkubecek@...e.cz>,
        dsahern@...nel.org, pablo@...filter.org, netdev@...r.kernel.org
Subject: Re: Genetlink per cmd policies

On Wed, 30 Sep 2020 18:06:28 +0200 Johannes Berg wrote:
> 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?

Yup.

> > 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.

Yup, it's like 2 or 3 "ops->family ? : rt->family" and we're good on
fallback AFAICT.

> 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.

Hm. If we really care it wouldn't be too crazy to have "lightweight"
ops and normal ops. At a cost of an extra pointer in the family.
Core genl can deal with that. And we could save another 16B per op if
we drop .start and .done which nl80211 barely uses.

> 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 ...

ethtool has common and per command parts. Global policies don't jell
well with strict checking in my mind.

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