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

From: Jakub Kicinski <[email-masked]>
Date: Fri, 20 Jan 2023 09:23:23 -0800

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.