On Fri, 2 Oct 2020 11:09:42 +0200 Johannes Berg wrote: > From: Johannes Berg <[email protected]> > > Rework the policy dump code a bit to support adding multiple > policies to a single dump, in order to e.g. support per-op > policies in generic netlink. > > Signed-off-by: Johannes Berg <[email protected]>
Reviewed-by: Jakub Kicinski <[email protected]> with a side of nits.. > diff --git a/include/net/netlink.h b/include/net/netlink.h > index 4be0ad237e57..a929759a03f5 100644 > --- a/include/net/netlink.h > +++ b/include/net/netlink.h > @@ -1937,12 +1937,68 @@ void nla_get_range_signed(const struct nla_policy *pt, > > struct netlink_policy_dump_state; > > -struct netlink_policy_dump_state * > -netlink_policy_dump_start(const struct nla_policy *policy, > - unsigned int maxtype); > +/** > + * netlink_policy_dump_add_policy - add a policy to the dump > + * @pstate: state to add to, may be reallocated, must be %NULL the first time > + * @policy: the new policy to add to the dump > + * @maxtype: the new policy's max attr type > + * > + * Returns: 0 on success, a negative error code otherwise. > + * > + * Call this to allocate a policy dump state, and to add policies to it. This > + * should be called from the dump start() callback. > + * > + * Note: on failures, any previously allocated state is freed. > + */ > +int netlink_policy_dump_add_policy(struct netlink_policy_dump_state **pstate, > + const struct nla_policy *policy, > + unsigned int maxtype); Personal preference perhaps, but I prefer kdoc with the definition. > diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c > index 537472342781..42777749d4d8 100644 > --- a/net/netlink/genetlink.c > +++ b/net/netlink/genetlink.c > @@ -1164,10 +1164,8 @@ static int ctrl_dumppolicy_start(struct > netlink_callback *cb) > if (!op.policy) > return -ENODATA; > > - 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? > } > +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. > + for (i = 0; i < state->n_alloc; i++) { > + if (state->policies[i].policy == policy && > + state->policies[i].maxtype == maxtype) > + return i; > + } > + > + WARN_ON(1); > + return 0; > } > > static bool
