On Mon, Sep 30, 2024 at 06:25:17PM +0200, Petr Machata wrote:
>
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> writes:
>
> > On Mon, Sep 30, 2024 at 01:45:09PM +0200, Jakub Kicinski wrote:
> >> On Mon, 30 Sep 2024 12:56:20 +0200 Pablo Neira Ayuso wrote:
> >> > On Mon, Sep 30, 2024 at 12:28:08PM +0200, Pablo Neira Ayuso wrote:
> >> > > On Sun, Sep 29, 2024 at 10:42:44AM +0000, Danielle Ratson wrote:
> >> > > > Hi,
> >> > > >
> >> > > > Is there a plan to build a new version soon?
> >> > > > I am asking since I am planning to use this function in ethtool.
> >> > >
> >> > > ASAP
> >> >
> >> > but one question before... Is this related to NLA_UINT in the kernel?
> >> >
> >> > /**
> >> > * nla_put_uint - Add a variable-size unsigned int to a socket buffer
> >> > * @skb: socket buffer to add attribute to
> >> > * @attrtype: attribute type
> >> > * @value: numeric value
> >> > */
> >> > static inline int nla_put_uint(struct sk_buff *skb, int attrtype, u64 value)
> >> > {
> >> > u64 tmp64 = value;
> >> > u32 tmp32 = value;
> >> >
> >> > if (tmp64 == tmp32)
> >> > return nla_put_u32(skb, attrtype, tmp32);
> >> > return nla_put(skb, attrtype, sizeof(u64), &tmp64);
> >> > }
> >> >
> >> > if I'm correct, it seems kernel always uses either u32 or u64.
> >> >
> >> > Userspace assumes u8 and u16 are possible though:
> >> >
> >> > +/**
> >> > + * mnl_attr_get_uint - returns 64-bit unsigned integer attribute.
> >> > + * \param attr pointer to netlink attribute
> >> > + *
> >> > + * This function returns the 64-bit value of the attribute payload.
> >> > + */
> >> > +EXPORT_SYMBOL uint64_t mnl_attr_get_uint(const struct nlattr *attr)
> >> > +{
> >> > + switch (mnl_attr_get_payload_len(attr)) {
> >> > + case sizeof(uint8_t):
> >> > + return mnl_attr_get_u8(attr);
> >> > + case sizeof(uint16_t):
> >> > + return mnl_attr_get_u16(attr);
> >> > + case sizeof(uint32_t):
> >> > + return mnl_attr_get_u32(attr);
> >> > + case sizeof(uint64_t):
> >> > + return mnl_attr_get_u64(attr);
> >> > + }
> >> > +
> >> > + return -1ULL;
> >> > +}
> >> >
> >> > Or this is an attempt to provide a helper that allows you fetch for
> >> > payload value of 2^3..2^6 bytes?
> >>
> >> No preference here, FWIW. Looks like this patch does a different thing
> >> than the kernel. But maybe a broader "automatic" helper is useful for
> >> user space code.
> >
> > Not sure. @Danielle: could you clarify your intention?
>
> This follows the iproute2 helper, where I was asked to support >32-bit
> fields purely as a service to the users, so that one helper can be used
> for any integral field.
Which helper are your referring to? Is it modeled after NLA_UINT?
I don't think this patch is fine. This also returns -1ULL so there is
no way to know if size is not correct or payload length is 64 bits
using UINT64_MAX?
Thanks.
