Messages in this thread | | | Date | Tue, 27 Sep 2022 18:09:21 +0300 | From | Vladimir Oltean <> | Subject | Re: [PATCH v2 net-next 01/12] net/sched: taprio: allow user input of per-tc max SDU |
| |
On Mon, Sep 26, 2022 at 01:38:29PM -0700, Jakub Kicinski wrote: > On Fri, 23 Sep 2022 19:32:59 +0300 Vladimir Oltean wrote: > > + if (!tb[TCA_TAPRIO_TC_ENTRY_INDEX]) { > > + NL_SET_ERR_MSG_MOD(extack, "TC entry index missing"); > > NL_SET_ERR_ATTR_MISS() ? > > > + return -EINVAL; > > + } > > + > > + tc = nla_get_u32(tb[TCA_TAPRIO_TC_ENTRY_INDEX]); > > + if (tc >= TC_QOPT_MAX_QUEUE) { > > + NL_SET_ERR_MSG_MOD(extack, "TC entry index out of range"); > > NLA_POLICY_MAX() > > Are you not using those on purpose? :(
I don't exactly see it as being super user friendly to leave it to the policy validator (or to use NL_SET_ERR_ATTR_MISS()) because all that will be reported back to user space will be the offset to the original attribute in the nlmsghdr, which is pretty hard to retrieve and re-interpret (at least in the iproute2 tc source code, I can't seem to find a way to stringify it or something like that). For the NLA_POLICY_MAX(), all I'll get now is an uninformative "Error: integer out of range." What integer? What range?
I don't understand what is the gain of removing extack message strings and just pointing to the netlink attribute via NLMSGERR_ATTR_OFFS? Could I at least use the NL_SET_ERR_ATTR_MISS() helper *and* set a custom message? That's for the missing nlattr. Regarding the range checking in the policy, I'd like a custom message there as well, but the NLA_POLICY_MAX() doesn't provide one. However, I see that struct nla_policy has a const char *reject_message for NLA_REJECT types. Would it be an abuse to move this outside of the union and allow U32 policies and such to also provide it?
| |