netdev - Re: [PATCH net-next v3 1/8] docs: add more netlink docs (incl. spec docs)

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]

Message-ID: <20230120092323.39d3787e@kernel.org>
Date:   Fri, 20 Jan 2023 09:23:23 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Johannes Berg <johannes@...solutions.net>
Cc:     davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
        pabeni@...hat.com, robh@...nel.org, stephen@...workplumber.org,
        ecree.xilinx@...il.com, sdf@...gle.com, f.fainelli@...il.com,
        fw@...len.de, linux-doc@...r.kernel.org, razor@...ckwall.org,
        nicolas.dichtel@...nd.com, Bagas Sanjaya <bagasdotme@...il.com>
Subject: Re: [PATCH net-next v3 1/8] docs: add more netlink docs (incl. spec
 docs)

On Fri, 20 Jan 2023 10:15:39 +0100 Johannes Berg wrote:
> > > > +Support dump consistency
> > > > +------------------------
> > > > +
> > > > +If iterating over objects during dump may skip over objects or repeat
> > > > +them - make sure to report dump inconsistency with ``NLM_F_DUMP_INTR``.    
> > > 
> > > That could be a bit more fleshed out on _how_ to do that, if it's not
> > > somewhere else?  
> > 
> > I was thinking about adding a sentence like "To avoid consistency
> > issues store your objects in an Xarray and correctly use the ID during
> > iteration".. but it seems to hand-wavy. Really the coder needs to
> > understand dumps quite well to get what's going on, and then the
> > consistency is kinda obvious. IDK. Almost nobody gets this right :(  
> 
> Yeah agree, it's tricky one way or the other. To be honest I was
> thinking less of documenting the mechanics of the underlying code to
> ensure that, but rather of the mechanics of using the APIs to ensure
> that, i.e. how to use cb->seq and friends.

I see. Let me add that.
My hope was to steer people towards data structures with stable
indexes, so the problem doesn't occur. But I'll add a mention of 
the helpers.

> > > Unrelated to this particular document, but ...
> > > 
> > > I'm all for this, btw, but maybe we should have a way of representing in
> > > the policy that an attribute is used as multi-attr for an array, and a
> > > way of exposing that in the policy export? Hmm. Haven't thought about
> > > this for a while.  
> > 
> > Informational-only or enforced? Enforcing this now would be another
> > backward-compat nightmare :(  
> 
> More informational - for userspace to know from policy dump that certain
> attributes have that property. With nested it's easy to know (there's a
> special nested-array type), but multi-attr there's no way to distinguish
> "is this one" and "is this multiple".

Makes sense.

> Now ... you might say you don't really care now since you want
> everything to be auto-generated and then you have it in the docs
> (actually, do you?), and that's a fair point.

Have in the docs that we want everything to be auto-generated?

> > FWIW I have a set parked on a branch to add "required" bit to policies,
> > so for per-op policies one can reject requests with missing attrs
> > during validation.  
> 
> Nice. That might yet convince me of per-op policies ;-)
> 
> Though IMHO the namespace issue remains - I'd still not like to have 100
> definitions of NL80211_ATTR_IFINDEX or similar.

Yeah, there's different ways of dealing with it. The ethtool way is
pretty neat - have a nest in each command for "common attrs" with
ifindex and stuff in it.