netdev - Re: [PATCH net-next v2 00/10] genetlink: support per-command policy dump

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 02 Oct 2020 22:27:19 +0200
From:   Johannes Berg <johannes@...solutions.net>
To:     Jakub Kicinski <kuba@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, andrew@...n.ch,
        jiri@...nulli.us, mkubecek@...e.cz, dsahern@...nel.org,
        pablo@...filter.org
Subject: Re: [PATCH net-next v2 00/10] genetlink: support per-command policy
 dump

On Fri, 2020-10-02 at 08:09 -0700, Jakub Kicinski wrote:
> On Fri, 02 Oct 2020 17:04:11 +0200 Johannes Berg wrote:
> > > > Yeah, that'd work. I'd probably wonder if we shouldn't do
> > > > 
> > > > [OP_POLICY]
> > > >   [OP] -> (u32, u32)
> > > > 
> > > > in a struct with two u32's, since that's quite a bit more compact.  
> > > 
> > > What do we do if the op doesn't have a dump or do callback?
> > > 0 is a valid policy ID, sadly :(  
> > 
> > Hm, good point. We could do -1 since that can't ever be reached though.
> > 
> > But compactness isn't really that necessary here anyway, so ...
> 
> Cool, sounds like a plan.
> 
> This series should be good to merge, then.

So I'm having second thoughts on this now :)

If you ask me to split the policy dump to do/dump, like we discussed
above, then what you did here for "retrieve a single policy" doesn't
really make any sense? Because you'd be able to do that properly only
for do, or you need my patches to get both?

Perhaps it would make sense if you removed patch 10 from your set, and
we add it back after my patches?

Or I could submit my patches right after yours, but that leaves the code
between the commits doing something weird, in that it would only give
you the policies but no indication of which is for do/dump? Obviously
today it'd only be one, but still, from a uAPI perspective.

I guess it doesn't matter too much though, we get to the state that we
want to be in, just the intermediate steps won't necessarily make much
sense.

For now I'll respin my patches so we see how the above do/dump
separating looks.

johannes