* [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
@ 2024-07-31 6:35 Danielle Ratson
2024-07-31 7:14 ` Phil Sutter
0 siblings, 1 reply; 14+ messages in thread
From: Danielle Ratson @ 2024-07-31 6:35 UTC (permalink / raw)
To: netfilter-devel; +Cc: pablo, phil, fw, mlxsw, Danielle Ratson
NLA_UINT attributes have a 4-byte payload if possible, and an 8-byte one
if necessary.
There are some NLA_UINT attributes that lack an appropriate getter function.
Add a function mnl_attr_get_uint() to cover that extract these. Since we
need to dispatch on length anyway, make the getter truly universal by
supporting also u8 and u16.
Signed-off-by: Danielle Ratson <danieller@nvidia.com>
---
include/libmnl/libmnl.h | 1 +
src/attr.c | 22 ++++++++++++++++++++++
src/libmnl.map | 4 ++++
3 files changed, 27 insertions(+)
diff --git a/include/libmnl/libmnl.h b/include/libmnl/libmnl.h
index 4bd0b92..9c03280 100644
--- a/include/libmnl/libmnl.h
+++ b/include/libmnl/libmnl.h
@@ -92,6 +92,7 @@ extern uint8_t mnl_attr_get_u8(const struct nlattr *attr);
extern uint16_t mnl_attr_get_u16(const struct nlattr *attr);
extern uint32_t mnl_attr_get_u32(const struct nlattr *attr);
extern uint64_t mnl_attr_get_u64(const struct nlattr *attr);
+extern uint64_t mnl_attr_get_uint(const struct nlattr *attr);
extern const char *mnl_attr_get_str(const struct nlattr *attr);
/* TLV attribute putters */
diff --git a/src/attr.c b/src/attr.c
index bc39df4..399318e 100644
--- a/src/attr.c
+++ b/src/attr.c
@@ -389,6 +389,28 @@ EXPORT_SYMBOL uint64_t mnl_attr_get_u64(const struct nlattr *attr)
return tmp;
}
+/**
+ * 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;
+}
+
/**
* mnl_attr_get_str - get pointer to string attribute
* \param attr pointer to netlink attribute
diff --git a/src/libmnl.map b/src/libmnl.map
index e5920e5..cd58863 100644
--- a/src/libmnl.map
+++ b/src/libmnl.map
@@ -77,3 +77,7 @@ LIBMNL_1.2 {
mnl_socket_open2;
mnl_socket_fdopen;
} LIBMNL_1.1;
+
+LIBMNL_1.3 {
+ mnl_attr_get_uint;
+} LIBMNL_1.2;
--
2.45.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-07-31 6:35 [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function Danielle Ratson
@ 2024-07-31 7:14 ` Phil Sutter
2024-09-29 10:42 ` Danielle Ratson
0 siblings, 1 reply; 14+ messages in thread
From: Phil Sutter @ 2024-07-31 7:14 UTC (permalink / raw)
To: Danielle Ratson; +Cc: netfilter-devel, pablo, fw, mlxsw
On Wed, Jul 31, 2024 at 09:35:51AM +0300, Danielle Ratson wrote:
> NLA_UINT attributes have a 4-byte payload if possible, and an 8-byte one
> if necessary.
>
> There are some NLA_UINT attributes that lack an appropriate getter function.
>
> Add a function mnl_attr_get_uint() to cover that extract these. Since we
> need to dispatch on length anyway, make the getter truly universal by
> supporting also u8 and u16.
>
> Signed-off-by: Danielle Ratson <danieller@nvidia.com>
Patch applied, thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-07-31 7:14 ` Phil Sutter
@ 2024-09-29 10:42 ` Danielle Ratson
2024-09-30 10:28 ` Pablo Neira Ayuso
0 siblings, 1 reply; 14+ messages in thread
From: Danielle Ratson @ 2024-09-29 10:42 UTC (permalink / raw)
To: Phil Sutter
Cc: netfilter-devel@vger.kernel.org, pablo@netfilter.org,
fw@strlen.de, mlxsw
Hi,
Is there a plan to build a new version soon?
I am asking since I am planning to use this function in ethtool.
Thanks,
Danielle
> -----Original Message-----
> From: Phil Sutter <phil@nwl.cc>
> Sent: Wednesday, 31 July 2024 10:15
> To: Danielle Ratson <danieller@nvidia.com>
> Cc: netfilter-devel@vger.kernel.org; pablo@netfilter.org; fw@strlen.de; mlxsw
> <mlxsw@nvidia.com>
> Subject: Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
>
> On Wed, Jul 31, 2024 at 09:35:51AM +0300, Danielle Ratson wrote:
> > NLA_UINT attributes have a 4-byte payload if possible, and an 8-byte
> > one if necessary.
> >
> > There are some NLA_UINT attributes that lack an appropriate getter
> function.
> >
> > Add a function mnl_attr_get_uint() to cover that extract these. Since
> > we need to dispatch on length anyway, make the getter truly universal
> > by supporting also u8 and u16.
> >
> > Signed-off-by: Danielle Ratson <danieller@nvidia.com>
>
> Patch applied, thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-29 10:42 ` Danielle Ratson
@ 2024-09-30 10:28 ` Pablo Neira Ayuso
2024-09-30 10:56 ` Pablo Neira Ayuso
0 siblings, 1 reply; 14+ messages in thread
From: Pablo Neira Ayuso @ 2024-09-30 10:28 UTC (permalink / raw)
To: Danielle Ratson
Cc: Phil Sutter, netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw
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
> Thanks,
> Danielle
>
> > -----Original Message-----
> > From: Phil Sutter <phil@nwl.cc>
> > Sent: Wednesday, 31 July 2024 10:15
> > To: Danielle Ratson <danieller@nvidia.com>
> > Cc: netfilter-devel@vger.kernel.org; pablo@netfilter.org; fw@strlen.de; mlxsw
> > <mlxsw@nvidia.com>
> > Subject: Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
> >
> > On Wed, Jul 31, 2024 at 09:35:51AM +0300, Danielle Ratson wrote:
> > > NLA_UINT attributes have a 4-byte payload if possible, and an 8-byte
> > > one if necessary.
> > >
> > > There are some NLA_UINT attributes that lack an appropriate getter
> > function.
> > >
> > > Add a function mnl_attr_get_uint() to cover that extract these. Since
> > > we need to dispatch on length anyway, make the getter truly universal
> > > by supporting also u8 and u16.
> > >
> > > Signed-off-by: Danielle Ratson <danieller@nvidia.com>
> >
> > Patch applied, thanks!
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 10:28 ` Pablo Neira Ayuso
@ 2024-09-30 10:56 ` Pablo Neira Ayuso
2024-09-30 11:45 ` Jakub Kicinski
0 siblings, 1 reply; 14+ messages in thread
From: Pablo Neira Ayuso @ 2024-09-30 10:56 UTC (permalink / raw)
To: Danielle Ratson
Cc: Phil Sutter, netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw,
kuba
Cc'ing Jakub.
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?
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 10:56 ` Pablo Neira Ayuso
@ 2024-09-30 11:45 ` Jakub Kicinski
2024-09-30 12:48 ` Pablo Neira Ayuso
0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2024-09-30 11:45 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Danielle Ratson, Phil Sutter, netfilter-devel@vger.kernel.org,
fw@strlen.de, mlxsw
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 11:45 ` Jakub Kicinski
@ 2024-09-30 12:48 ` Pablo Neira Ayuso
2024-09-30 16:25 ` Petr Machata
2024-10-01 9:19 ` Danielle Ratson
0 siblings, 2 replies; 14+ messages in thread
From: Pablo Neira Ayuso @ 2024-09-30 12:48 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Danielle Ratson, Phil Sutter, netfilter-devel@vger.kernel.org,
fw@strlen.de, mlxsw
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?
If this is to support NLA_UINT, I'd prefer to stick to NLA_UINT semantics.
@Jakub: is there any plan to augment NLA_UINT in the future? What
the assumption from userspace that this will always return 32-bits
else 64-bits value?
Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 12:48 ` Pablo Neira Ayuso
@ 2024-09-30 16:25 ` Petr Machata
2024-09-30 17:01 ` Pablo Neira Ayuso
2024-10-01 9:19 ` Danielle Ratson
1 sibling, 1 reply; 14+ messages in thread
From: Petr Machata @ 2024-09-30 16:25 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Jakub Kicinski, Danielle Ratson, Phil Sutter,
netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw
Pablo Neira Ayuso <pablo@netfilter.org> 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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 16:25 ` Petr Machata
@ 2024-09-30 17:01 ` Pablo Neira Ayuso
2024-09-30 17:10 ` Pablo Neira Ayuso
0 siblings, 1 reply; 14+ messages in thread
From: Pablo Neira Ayuso @ 2024-09-30 17:01 UTC (permalink / raw)
To: Petr Machata
Cc: Jakub Kicinski, Danielle Ratson, Phil Sutter,
netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw
On Mon, Sep 30, 2024 at 06:25:17PM +0200, Petr Machata wrote:
>
> Pablo Neira Ayuso <pablo@netfilter.org> 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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 17:01 ` Pablo Neira Ayuso
@ 2024-09-30 17:10 ` Pablo Neira Ayuso
2024-09-30 17:52 ` Pablo Neira Ayuso
2024-10-01 7:51 ` Petr Machata
0 siblings, 2 replies; 14+ messages in thread
From: Pablo Neira Ayuso @ 2024-09-30 17:10 UTC (permalink / raw)
To: Petr Machata
Cc: Jakub Kicinski, Danielle Ratson, Phil Sutter,
netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw
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@netfilter.org> 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?
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?
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 17:10 ` Pablo Neira Ayuso
@ 2024-09-30 17:52 ` Pablo Neira Ayuso
2024-10-01 8:16 ` Petr Machata
2024-10-01 7:51 ` Petr Machata
1 sibling, 1 reply; 14+ messages in thread
From: Pablo Neira Ayuso @ 2024-09-30 17:52 UTC (permalink / raw)
To: Petr Machata
Cc: Jakub Kicinski, Danielle Ratson, Phil Sutter,
netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw
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@netfilter.org> 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@nvidia.com>
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.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 17:10 ` Pablo Neira Ayuso
2024-09-30 17:52 ` Pablo Neira Ayuso
@ 2024-10-01 7:51 ` Petr Machata
1 sibling, 0 replies; 14+ messages in thread
From: Petr Machata @ 2024-10-01 7:51 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Petr Machata, Jakub Kicinski, Danielle Ratson, Phil Sutter,
netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw
Pablo Neira Ayuso <pablo@netfilter.org> writes:
> 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@netfilter.org> 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?
That's no different from the other mnl_attr_get_uX() helpers that just
assume the user has passed in the right attribute type.
> I found it:
>
> static inline __u64 rta_getattr_uint(const struct rtattr *rta)
That's the one.
> This only has one user in the tree so far, right?
Well, two, but yeah, the u8 / u16 stuff is not currently used.
> 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?
Yeah, both NHA_GROUP_STATS_ENTRY_PACKETS and
NHA_GROUP_STATS_ENTRY_PACKETS_HW are NLA_UINT.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 17:52 ` Pablo Neira Ayuso
@ 2024-10-01 8:16 ` Petr Machata
0 siblings, 0 replies; 14+ messages in thread
From: Petr Machata @ 2024-10-01 8:16 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: Petr Machata, Jakub Kicinski, Danielle Ratson, Phil Sutter,
netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw
Pablo Neira Ayuso <pablo@netfilter.org> writes:
> 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@netfilter.org> 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@nvidia.com>
> 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.
Yeah, sorry, I wrote this to explain the rationale:
> 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.
... but of course I should have written <32-bit payloads, not >32-bit.
^ permalink raw reply [flat|nested] 14+ messages in thread
* RE: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
2024-09-30 12:48 ` Pablo Neira Ayuso
2024-09-30 16:25 ` Petr Machata
@ 2024-10-01 9:19 ` Danielle Ratson
1 sibling, 0 replies; 14+ messages in thread
From: Danielle Ratson @ 2024-10-01 9:19 UTC (permalink / raw)
To: Pablo Neira Ayuso, Jakub Kicinski
Cc: Phil Sutter, netfilter-devel@vger.kernel.org, fw@strlen.de, mlxsw
> From: Pablo Neira Ayuso <pablo@netfilter.org>
> Sent: Monday, 30 September 2024 15:48
> To: Jakub Kicinski <kuba@kernel.org>
> Cc: Danielle Ratson <danieller@nvidia.com>; Phil Sutter <phil@nwl.cc>;
> netfilter-devel@vger.kernel.org; fw@strlen.de; mlxsw <mlxsw@nvidia.com>
> Subject: Re: [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function
>
> 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?
Hi,
Thanks for all the comments.
As I see it, there are at least two occurrences in ethtool for a code that tries to overcome the lack of this kind of uint helper when the uint attribute type exists in the kernel and seems like more recommended to use then before.
Therefore, as I see it, this helper seems more reasonable to have.
But I am trying to understand, are we discussing the need of the patch at all? Or some nits about the way it is implemented? Or something else?
I am asking since the patch was already taken by @Pablo Neira Ayuso.
>
> If this is to support NLA_UINT, I'd prefer to stick to NLA_UINT semantics.
>
> @Jakub: is there any plan to augment NLA_UINT in the future? What the
> assumption from userspace that this will always return 32-bits else 64-bits
> value?
>
> Thanks.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-10-01 9:19 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-31 6:35 [PATCH libmnl] src: attr: Add mnl_attr_get_uint() function Danielle Ratson
2024-07-31 7:14 ` Phil Sutter
2024-09-29 10:42 ` Danielle Ratson
2024-09-30 10:28 ` Pablo Neira Ayuso
2024-09-30 10:56 ` Pablo Neira Ayuso
2024-09-30 11:45 ` Jakub Kicinski
2024-09-30 12:48 ` Pablo Neira Ayuso
2024-09-30 16:25 ` Petr Machata
2024-09-30 17:01 ` Pablo Neira Ayuso
2024-09-30 17:10 ` Pablo Neira Ayuso
2024-09-30 17:52 ` Pablo Neira Ayuso
2024-10-01 8:16 ` Petr Machata
2024-10-01 7:51 ` Petr Machata
2024-10-01 9:19 ` Danielle Ratson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.