Re: [PATCH 3/5] netlink: rework policy dump to support multiple policies

[Johannes Berg <johannes@sipsolutions.net>]

On Fri, 2020-10-02 at 08:39 -0700, Jakub Kicinski wrote:
> 
> > -	ctx->state = netlink_policy_dump_start(op.policy, op.maxattr);
> > -	if (IS_ERR(ctx->state))
> > -		return PTR_ERR(ctx->state);
> > -	return 0;
> > +	return netlink_policy_dump_add_policy(&ctx->state, op.policy,
> > +					      op.maxattr);
> 
> Looks like we flip-flopped between int and pointer return between
> patches 1 and this one?

Huh, yeah, that was kinda dumb. I started going down one path and then
...

I'll probably just squash the first patch or something. Will figure
something out, thanks.

> >  }
> > +int netlink_policy_dump_get_policy_idx(struct netlink_policy_dump_state *state,
> > +				       const struct nla_policy *policy,
> > +				       unsigned int maxtype)
> > +{
> > +	unsigned int i;
> > +
> > +	if (WARN_ON(!policy || !maxtype))
> > +                return 0;
> 
> Would this warning make sense in add() (if not already there)?
> If null/0 is never added it can't match and we'd just hit the
> warning below.

It's not there, because had originally thought it should be OK to just
blindly add a policy of a family even if it has none. But that makes no
sense.

However, it's not true that it can't match, because

> > +	for (i = 0; i < state->n_alloc; i++) {

we go to n_alloc here, and don't separately track n_used, but n_alloc
grows in tens (or so), not singles.

johannes