Re: [PATCH net-next 0/8] netlink: align attributes when needed (patchset #3)
From: Lars EllenbergDate: Tue, 26 Apr 2016 13:54:27 +0200 > On Tue, Apr 26, 2016 at 10:06:10AM +0200, Nicolas Dichtel wrote: >> >> This is the continuation (series #3) of the work done to align netlink >> attributes when these attributes contain some 64-bit fields. >> >> It's the last patchset from what I've seen. >> >> The last user of nla_put_u64() is block/drbd. This module does not use >> standard netlink API (see all the stuff in include/linux/genl_magic_struct.h >> and include/linux/genl_magic_func.h). I didn't modify it because it's seems >> hard to do it whithout testing and fully understanding the context > > Something like this should just work. Unfortunately we had problems using unspec, that's why an explicit new padding attribute is added for each netlink attribute set.
Re: [PATCH net-next 0/8] netlink: align attributes when needed (patchset #3)
From: Nicolas DichtelDate: Tue, 26 Apr 2016 10:06:10 +0200 > The last user of nla_put_u64() is block/drbd. This module does not use > standard netlink API (see all the stuff in include/linux/genl_magic_struct.h > and include/linux/genl_magic_func.h). Yet another example where doing things in a special unique way creates headaches and pain for everyone... sigh. > I didn't modify it because it's seems hard to do it whithout testing > and fully understanding the context (for example, why > include/linux/drbd_genl.h is not part of uapi?). Any thoughts? I think you'll need to work with the drbd maintainer(s) to resolve this and test the result. Series applied, thanks.
Re: [Drbd-dev] [PATCH net-next 0/8] netlink: align attributes when needed (patchset #3)
On Tue, Apr 26, 2016 at 01:54:27PM +0200, Lars Ellenberg wrote: > On Tue, Apr 26, 2016 at 10:06:10AM +0200, Nicolas Dichtel wrote: > > > > This is the continuation (series #3) of the work done to align netlink > > attributes when these attributes contain some 64-bit fields. > > > > It's the last patchset from what I've seen. > > > > The last user of nla_put_u64() is block/drbd. This module does not use > > standard netlink API (see all the stuff in include/linux/genl_magic_struct.h > > and include/linux/genl_magic_func.h). I didn't modify it because it's seems > > hard to do it whithout testing and fully understanding the context > > Something like this should just work. > + * @attrtype: attribute type > + * @value: numeric value > + */ > +static inline int nla_put_u64_64bit_unspec(struct sk_buff *skb, int attrtype, > + u64 value) > +{ > + return nla_put_64bit(skb, attrtype, sizeof(u64), , NLA_UNSPEC); Ok, I confused attribute and policy type there for a sec. Anyways, 0 works just fine, all our nested attribute enums are != 0, because nla_parse skips type 0. Lars
Re: [PATCH net-next 0/8] netlink: align attributes when needed (patchset #3)
On Tue, Apr 26, 2016 at 10:06:10AM +0200, Nicolas Dichtel wrote: > > This is the continuation (series #3) of the work done to align netlink > attributes when these attributes contain some 64-bit fields. > > It's the last patchset from what I've seen. > > The last user of nla_put_u64() is block/drbd. This module does not use > standard netlink API (see all the stuff in include/linux/genl_magic_struct.h > and include/linux/genl_magic_func.h). I didn't modify it because it's seems > hard to do it whithout testing and fully understanding the context Something like this should just work. diff --git a/include/linux/genl_magic_struct.h b/include/linux/genl_magic_struct.h index eecd19b..5715dac 100644 --- a/include/linux/genl_magic_struct.h +++ b/include/linux/genl_magic_struct.h @@ -80,7 +80,7 @@ extern void CONCAT_(GENL_MAGIC_FAMILY, _genl_unregister)(void); nla_get_u32, nla_put_u32, true) #define __u64_field(attr_nr, attr_flag, name) \ __field(attr_nr, attr_flag, name, NLA_U64, __u64, \ - nla_get_u64, nla_put_u64, false) + nla_get_u64, nla_put_u64_64bit_unspec, false) #define __str_field(attr_nr, attr_flag, name, maxlen) \ __array(attr_nr, attr_flag, name, NLA_NUL_STRING, char, maxlen, \ nla_strlcpy, nla_put, false) diff --git a/include/net/netlink.h b/include/net/netlink.h index e589cb3..38ba770 100644 --- a/include/net/netlink.h +++ b/include/net/netlink.h @@ -871,6 +871,18 @@ static inline int nla_put_u64_64bit(struct sk_buff *skb, int attrtype, } /** + * nla_put_u64_64bit_unspec - nla_put_u64_64bit() with padattr = 0 + * @skb: socket buffer to add attribute to + * @attrtype: attribute type + * @value: numeric value + */ +static inline int nla_put_u64_64bit_unspec(struct sk_buff *skb, int attrtype, + u64 value) +{ + return nla_put_64bit(skb, attrtype, sizeof(u64), , NLA_UNSPEC); +} + +/** * nla_put_be64 - Add a __be64 netlink attribute to a socket buffer and align it * @skb: socket buffer to add attribute to * @attrtype: attribute type > (for > example, why include/linux/drbd_genl.h is not part of uapi?). > Any thoughts? probably should be. Thanks, Lars
[PATCH net-next 0/8] netlink: align attributes when needed (patchset #3)
This is the continuation (series #3) of the work done to align netlink attributes when these attributes contain some 64-bit fields. It's the last patchset from what I've seen. The last user of nla_put_u64() is block/drbd. This module does not use standard netlink API (see all the stuff in include/linux/genl_magic_struct.h and include/linux/genl_magic_func.h). I didn't modify it because it's seems hard to do it whithout testing and fully understanding the context (for example, why include/linux/drbd_genl.h is not part of uapi?). Any thoughts? Documentation/networking/gen_stats.txt | 6 +- drivers/net/macsec.c| 121 +++- drivers/net/wireless/mac80211_hwsim.c | 2 +- drivers/net/wireless/mac80211_hwsim.h | 1 + fs/quota/netlink.c | 12 ++-- include/net/gen_stats.h | 6 +- include/uapi/linux/gen_stats.h | 1 + include/uapi/linux/if_link.h| 1 + include/uapi/linux/if_macsec.h | 6 ++ include/uapi/linux/inet_diag.h | 4 +- include/uapi/linux/openvswitch.h| 2 + include/uapi/linux/pkt_cls.h| 2 + include/uapi/linux/quota.h | 1 + include/uapi/linux/rtnetlink.h | 1 + include/uapi/linux/tc_act/tc_bpf.h | 1 + include/uapi/linux/tc_act/tc_connmark.h | 1 + include/uapi/linux/tc_act/tc_csum.h | 1 + include/uapi/linux/tc_act/tc_defact.h | 1 + include/uapi/linux/tc_act/tc_gact.h | 1 + include/uapi/linux/tc_act/tc_ife.h | 1 + include/uapi/linux/tc_act/tc_ipt.h | 1 + include/uapi/linux/tc_act/tc_mirred.h | 1 + include/uapi/linux/tc_act/tc_nat.h | 1 + include/uapi/linux/tc_act/tc_pedit.h| 1 + include/uapi/linux/tc_act/tc_skbedit.h | 1 + include/uapi/linux/tc_act/tc_vlan.h | 1 + net/core/gen_stats.c| 35 + net/core/neighbour.c| 3 +- net/core/rtnetlink.c| 4 +- net/core/sock_diag.c| 2 +- net/ipv4/inet_diag.c| 9 ++- net/openvswitch/datapath.c | 27 +++ net/sched/act_api.c | 7 +- net/sched/act_bpf.c | 3 +- net/sched/act_connmark.c| 3 +- net/sched/act_csum.c| 2 +- net/sched/act_gact.c| 2 +- net/sched/act_ife.c | 2 +- net/sched/act_ipt.c | 2 +- net/sched/act_mirred.c | 2 +- net/sched/act_nat.c | 2 +- net/sched/act_pedit.c | 2 +- net/sched/act_simple.c | 2 +- net/sched/act_skbedit.c | 2 +- net/sched/act_vlan.c| 2 +- net/sched/cls_u32.c | 7 +- net/sched/sch_api.c | 6 +- net/sctp/sctp_diag.c| 5 +- 48 files changed, 211 insertions(+), 98 deletions(-) Comments are welcomed, Regards, Nicolas