| Message ID | 20240222235614.180876-2-kuba@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | tools: ynl: stop using libmnl | expand |
Le 23/02/2024 à 00:56, Jakub Kicinski a écrit : > The temporary auto-int helpers are not really correct. > We can't treat signed and unsigned ints the same when > determining whether we need full 8B. I realized this > before sending the patch to add support in libmnl. > Unfortunately, that patch has not been merged, > so time to fix our local helpers. Use the mnl* name > for now, subsequent patches will address that. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org>> --- > tools/net/ynl/lib/ynl-priv.h | 45 ++++++++++++++++++++++++++++-------- > 1 file changed, 36 insertions(+), 9 deletions(-) > > diff --git a/tools/net/ynl/lib/ynl-priv.h b/tools/net/ynl/lib/ynl-priv.h > index 7491da8e7555..eaa0d432366c 100644 > --- a/tools/net/ynl/lib/ynl-priv.h > +++ b/tools/net/ynl/lib/ynl-priv.h > @@ -125,20 +125,47 @@ int ynl_exec_dump(struct ynl_sock *ys, struct nlmsghdr *req_nlh, > void ynl_error_unknown_notification(struct ynl_sock *ys, __u8 cmd); > int ynl_error_parse(struct ynl_parse_arg *yarg, const char *msg); > > -#ifndef MNL_HAS_AUTO_SCALARS > -static inline uint64_t mnl_attr_get_uint(const struct nlattr *attr) > +/* Attribute helpers */ > + > +static inline __u64 mnl_attr_get_uint(const struct nlattr *attr) > { > - if (mnl_attr_get_payload_len(attr) == 4) > + switch (mnl_attr_get_payload_len(attr)) { > + case 4: > return mnl_attr_get_u32(attr); > - return mnl_attr_get_u64(attr); > + case 8: > + return mnl_attr_get_u64(attr); > + default: > + return 0; > + } > +} > + > +static inline __s64 mnl_attr_get_sint(const struct nlattr *attr) > +{ > + switch (mnl_attr_get_payload_len(attr)) { > + case 4: > + return mnl_attr_get_u32(attr); > + case 8: > + return mnl_attr_get_u64(attr); > + default: > + return 0; > + } > } mnl_attr_get_uint() and mnl_attr_get_sint() are identical. What about #define mnl_attr_get_sint mnl_attr_get_uint ? > > static inline void > -mnl_attr_put_uint(struct nlmsghdr *nlh, uint16_t type, uint64_t data) > +mnl_attr_put_uint(struct nlmsghdr *nlh, __u16 type, __u64 data) Is there a reason to switch from uint*_t to __u* types? > { > - if ((uint32_t)data == (uint64_t)data) > - return mnl_attr_put_u32(nlh, type, data); > - return mnl_attr_put_u64(nlh, type, data); > + if ((__u32)data == (__u64)data) > + mnl_attr_put_u32(nlh, type, data); > + else > + mnl_attr_put_u64(nlh, type, data); > +} > + > +static inline void > +mnl_attr_put_sint(struct nlmsghdr *nlh, __u16 type, __s64 data) > +{ > + if ((__s32)data == (__s64)data) > + mnl_attr_put_u32(nlh, type, data); > + else > + mnl_attr_put_u64(nlh, type, data); > } > #endif > -#endif
On Fri, 23 Feb 2024 14:51:12 +0100 Nicolas Dichtel wrote: > > +static inline __s64 mnl_attr_get_sint(const struct nlattr *attr) > > +{ > > + switch (mnl_attr_get_payload_len(attr)) { > > + case 4: > > + return mnl_attr_get_u32(attr); > > + case 8: > > + return mnl_attr_get_u64(attr); > > + default: > > + return 0; > > + } > > } > mnl_attr_get_uint() and mnl_attr_get_sint() are identical. What about > #define mnl_attr_get_sint mnl_attr_get_uint > ? I like to have the helpers written out
Le 23/02/2024 à 15:35, Jakub Kicinski a écrit : > On Fri, 23 Feb 2024 14:51:12 +0100 Nicolas Dichtel wrote: >>> +static inline __s64 mnl_attr_get_sint(const struct nlattr *attr) >>> +{ >>> + switch (mnl_attr_get_payload_len(attr)) { >>> + case 4: >>> + return mnl_attr_get_u32(attr); >>> + case 8: >>> + return mnl_attr_get_u64(attr); >>> + default: >>> + return 0; >>> + } >>> } >> mnl_attr_get_uint() and mnl_attr_get_sint() are identical. What about >> #define mnl_attr_get_sint mnl_attr_get_uint >> ? > > I like to have the helpers written out
Jakub Kicinski <kuba@kernel.org> writes: > The temporary auto-int helpers are not really correct. > We can't treat signed and unsigned ints the same when > determining whether we need full 8B. I realized this > before sending the patch to add support in libmnl. > Unfortunately, that patch has not been merged, > so time to fix our local helpers. Use the mnl* name > for now, subsequent patches will address that. > > Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Donald Hunter <donald.hunter@gmail.com>
diff --git a/tools/net/ynl/lib/ynl-priv.h b/tools/net/ynl/lib/ynl-priv.h index 7491da8e7555..eaa0d432366c 100644 --- a/tools/net/ynl/lib/ynl-priv.h +++ b/tools/net/ynl/lib/ynl-priv.h @@ -125,20 +125,47 @@ int ynl_exec_dump(struct ynl_sock *ys, struct nlmsghdr *req_nlh, void ynl_error_unknown_notification(struct ynl_sock *ys, __u8 cmd); int ynl_error_parse(struct ynl_parse_arg *yarg, const char *msg); -#ifndef MNL_HAS_AUTO_SCALARS -static inline uint64_t mnl_attr_get_uint(const struct nlattr *attr) +/* Attribute helpers */ + +static inline __u64 mnl_attr_get_uint(const struct nlattr *attr) { - if (mnl_attr_get_payload_len(attr) == 4) + switch (mnl_attr_get_payload_len(attr)) { + case 4: return mnl_attr_get_u32(attr); - return mnl_attr_get_u64(attr); + case 8: + return mnl_attr_get_u64(attr); + default: + return 0; + } +} + +static inline __s64 mnl_attr_get_sint(const struct nlattr *attr) +{ + switch (mnl_attr_get_payload_len(attr)) { + case 4: + return mnl_attr_get_u32(attr); + case 8: + return mnl_attr_get_u64(attr); + default: + return 0; + } } static inline void -mnl_attr_put_uint(struct nlmsghdr *nlh, uint16_t type, uint64_t data) +mnl_attr_put_uint(struct nlmsghdr *nlh, __u16 type, __u64 data) { - if ((uint32_t)data == (uint64_t)data) - return mnl_attr_put_u32(nlh, type, data); - return mnl_attr_put_u64(nlh, type, data); + if ((__u32)data == (__u64)data) + mnl_attr_put_u32(nlh, type, data); + else + mnl_attr_put_u64(nlh, type, data); +} + +static inline void +mnl_attr_put_sint(struct nlmsghdr *nlh, __u16 type, __s64 data) +{ + if ((__s32)data == (__s64)data) + mnl_attr_put_u32(nlh, type, data); + else + mnl_attr_put_u64(nlh, type, data); } #endif -#endif
The temporary auto-int helpers are not really correct. We can't treat signed and unsigned ints the same when determining whether we need full 8B. I realized this before sending the patch to add support in libmnl. Unfortunately, that patch has not been merged, so time to fix our local helpers. Use the mnl* name for now, subsequent patches will address that. Signed-off-by: Jakub Kicinski <kuba@kernel.org> --- tools/net/ynl/lib/ynl-priv.h | 45 ++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 9 deletions(-)