On Mon, Sep 30, 2024 at 07:10:27PM +0200, Pablo Neira Ayuso wrote:
> On Mon, Sep 30, 2024 at 07:01:42PM +0200, Pablo Neira Ayuso wrote:
> > 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?
>
> I found it:
>
> static inline __u64 rta_getattr_uint(const struct rtattr *rta)
>
> This only has one user in the tree so far, right?
Well, this is a matter of documenting behaviour.
> include/libnetlink.h:static inline __u64 rta_getattr_uint(const struct rtattr *rta)
> ip/ipnexthop.c: nh_grp_stats->packets = rta_getattr_uint(rta);
> ip/ipnexthop.c: nh_grp_stats->packets_hw = rta_getattr_uint(rta);
>
> is this attribute for ipnexthop of NLA_UINT type?
But it seems intention is to support NLA_UINT according to iproute's
commit.
commit 95836fbf35d352f7c031ddac2e6093a935308cc9
Author: Petr Machata <petrm@xxxxxxxxxx>
Date: Thu Mar 14 15:52:12 2024 +0100
libnetlink: Add rta_getattr_uint()
NLA_UINT attributes have a 4-byte payload if possible, and an 8-byte one if
necessary. Add a function to extract these. Since we need to dispatch on
length anyway, make the getter truly universal by supporting also u8 and
u16.
so it went further to make it universal for 2^3..2^6 values.
I am going to submit a patch to provide more info on this helper function.
