2016-04-22 15:41:57

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 0/9] netlink: align attributes when needed (patchset #1)


This is the continuation of the work done to align netlink attributes
when these attributes contain some 64-bit fields.

David, if the third patch is too big (or maybe the series), I can split it.
Just tell me what you prefer.

include/linux/netfilter/ipset/ip_set.h | 9 ++--
include/net/netlink.h | 60 ++++++++++++++++------
include/net/nl802154.h | 6 +++
include/uapi/linux/fib_rules.h | 1 +
include/uapi/linux/l2tp.h | 1 +
include/uapi/linux/lwtunnel.h | 2 +
include/uapi/linux/neighbour.h | 2 +
include/uapi/linux/netfilter/ipset/ip_set.h | 1 +
include/uapi/linux/netfilter/nf_tables.h | 8 +++
include/uapi/linux/netfilter/nfnetlink_acct.h | 1 +
include/uapi/linux/netfilter/nfnetlink_conntrack.h | 3 ++
include/uapi/linux/openvswitch.h | 1 +
include/uapi/linux/tcp_metrics.h | 1 +
include/uapi/linux/xfrm.h | 1 +
kernel/taskstats.c | 37 ++-----------
lib/nlattr.c | 8 ++-
net/core/fib_rules.c | 4 +-
net/core/neighbour.c | 19 +++----
net/ieee802154/nl802154.c | 13 +++--
net/ipv4/ip_tunnel_core.c | 10 ++--
net/ipv4/tcp_metrics.c | 6 ++-
net/l2tp/l2tp_netlink.c | 3 +-
net/netfilter/nf_conntrack_netlink.c | 18 ++++---
net/netfilter/nf_conntrack_proto_dccp.c | 4 +-
net/netfilter/nf_tables_api.c | 24 ++++++---
net/netfilter/nf_tables_trace.c | 5 +-
net/netfilter/nfnetlink_acct.c | 9 ++--
net/netfilter/nft_counter.c | 6 ++-
net/netfilter/nft_dynset.c | 3 +-
net/netfilter/nft_limit.c | 6 ++-
net/openvswitch/flow_netlink.c | 5 +-
net/xfrm/xfrm_user.c | 10 ++--
32 files changed, 178 insertions(+), 109 deletions(-)

Comments are welcomed,
Regards,
Nicolas


2016-04-22 15:42:08

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 9/9] taskstats: use the libnl API to align nlattr on 64-bit

Goal of this patch is to use the new libnl API to align netlink attribute
when needed.
The layout of the netlink message will be a bit different after the patch,
because the padattr (TASKSTATS_TYPE_STATS) will be inside the nested
attribute instead of before it.

Signed-off-by: Nicolas Dichtel <[email protected]>
---
kernel/taskstats.c | 37 +++++--------------------------------
1 file changed, 5 insertions(+), 32 deletions(-)

diff --git a/kernel/taskstats.c b/kernel/taskstats.c
index 21f82c29c914..b3f05ee20d18 100644
--- a/kernel/taskstats.c
+++ b/kernel/taskstats.c
@@ -357,10 +357,6 @@ static int parse(struct nlattr *na, struct cpumask *mask)
return ret;
}

-#if defined(CONFIG_64BIT) && !defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS)
-#define TASKSTATS_NEEDS_PADDING 1
-#endif
-
static struct taskstats *mk_reply(struct sk_buff *skb, int type, u32 pid)
{
struct nlattr *na, *ret;
@@ -370,29 +366,6 @@ static struct taskstats *mk_reply(struct sk_buff *skb, int type, u32 pid)
? TASKSTATS_TYPE_AGGR_PID
: TASKSTATS_TYPE_AGGR_TGID;

- /*
- * The taskstats structure is internally aligned on 8 byte
- * boundaries but the layout of the aggregrate reply, with
- * two NLA headers and the pid (each 4 bytes), actually
- * force the entire structure to be unaligned. This causes
- * the kernel to issue unaligned access warnings on some
- * architectures like ia64. Unfortunately, some software out there
- * doesn't properly unroll the NLA packet and assumes that the start
- * of the taskstats structure will always be 20 bytes from the start
- * of the netlink payload. Aligning the start of the taskstats
- * structure breaks this software, which we don't want. So, for now
- * the alignment only happens on architectures that require it
- * and those users will have to update to fixed versions of those
- * packages. Space is reserved in the packet only when needed.
- * This ifdef should be removed in several years e.g. 2012 once
- * we can be confident that fixed versions are installed on most
- * systems. We add the padding before the aggregate since the
- * aggregate is already a defined type.
- */
-#ifdef TASKSTATS_NEEDS_PADDING
- if (nla_put(skb, TASKSTATS_TYPE_NULL, 0, NULL) < 0)
- goto err;
-#endif
na = nla_nest_start(skb, aggr);
if (!na)
goto err;
@@ -401,7 +374,8 @@ static struct taskstats *mk_reply(struct sk_buff *skb, int type, u32 pid)
nla_nest_cancel(skb, na);
goto err;
}
- ret = nla_reserve(skb, TASKSTATS_TYPE_STATS, sizeof(struct taskstats));
+ ret = nla_reserve_64bit(skb, TASKSTATS_TYPE_STATS,
+ sizeof(struct taskstats), TASKSTATS_TYPE_NULL);
if (!ret) {
nla_nest_cancel(skb, na);
goto err;
@@ -500,10 +474,9 @@ static size_t taskstats_packet_size(void)
size_t size;

size = nla_total_size(sizeof(u32)) +
- nla_total_size(sizeof(struct taskstats)) + nla_total_size(0);
-#ifdef TASKSTATS_NEEDS_PADDING
- size += nla_total_size(0); /* Padding for alignment */
-#endif
+ nla_total_size_64bit(sizeof(struct taskstats)) +
+ nla_total_size(0);
+
return size;
}

--
2.8.1

2016-04-22 15:42:04

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 6/9] libnl: nla_put_msecs(): align on a 64-bit area

nla_data() is now aligned on a 64-bit area.

Signed-off-by: Nicolas Dichtel <[email protected]>
---
include/net/netlink.h | 11 +++++++----
include/uapi/linux/l2tp.h | 1 +
include/uapi/linux/neighbour.h | 2 ++
include/uapi/linux/tcp_metrics.h | 1 +
net/core/neighbour.c | 19 ++++++++++---------
net/ipv4/tcp_metrics.c | 6 ++++--
net/l2tp/l2tp_netlink.c | 3 ++-
7 files changed, 27 insertions(+), 16 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 074215a59d19..113b483b6ee8 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -106,7 +106,8 @@
* padattr) add s64 attribute to skb
* nla_put_string(skb, type, str) add string attribute to skb
* nla_put_flag(skb, type) add flag attribute to skb
- * nla_put_msecs(skb, type, jiffies) add msecs attribute to skb
+ * nla_put_msecs(skb, type, jiffies,
+ * padattr) add msecs attribute to skb
* nla_put_in_addr(skb, type, addr) add IPv4 address attribute to skb
* nla_put_in6_addr(skb, type, addr) add IPv6 address attribute to skb
*
@@ -965,16 +966,18 @@ static inline int nla_put_flag(struct sk_buff *skb, int attrtype)
}

/**
- * nla_put_msecs - Add a msecs netlink attribute to a socket buffer
+ * nla_put_msecs - Add a msecs netlink attribute to a skb and align it
* @skb: socket buffer to add attribute to
* @attrtype: attribute type
* @njiffies: number of jiffies to convert to msecs
+ * @padattr: attribute type for the padding
*/
static inline int nla_put_msecs(struct sk_buff *skb, int attrtype,
- unsigned long njiffies)
+ unsigned long njiffies, int padattr)
{
u64 tmp = jiffies_to_msecs(njiffies);
- return nla_put(skb, attrtype, sizeof(u64), &tmp);
+
+ return nla_put_64bit(skb, attrtype, sizeof(u64), &tmp, padattr);
}

/**
diff --git a/include/uapi/linux/l2tp.h b/include/uapi/linux/l2tp.h
index 347ef22a964e..3386a99e0397 100644
--- a/include/uapi/linux/l2tp.h
+++ b/include/uapi/linux/l2tp.h
@@ -126,6 +126,7 @@ enum {
L2TP_ATTR_IP6_DADDR, /* struct in6_addr */
L2TP_ATTR_UDP_ZERO_CSUM6_TX, /* u8 */
L2TP_ATTR_UDP_ZERO_CSUM6_RX, /* u8 */
+ L2TP_ATTR_PAD,
__L2TP_ATTR_MAX,
};

diff --git a/include/uapi/linux/neighbour.h b/include/uapi/linux/neighbour.h
index 788655bfa0f3..bd99a8d80f36 100644
--- a/include/uapi/linux/neighbour.h
+++ b/include/uapi/linux/neighbour.h
@@ -128,6 +128,7 @@ enum {
NDTPA_LOCKTIME, /* u64, msecs */
NDTPA_QUEUE_LENBYTES, /* u32 */
NDTPA_MCAST_REPROBES, /* u32 */
+ NDTPA_PAD,
__NDTPA_MAX
};
#define NDTPA_MAX (__NDTPA_MAX - 1)
@@ -160,6 +161,7 @@ enum {
NDTA_PARMS, /* nested TLV NDTPA_* */
NDTA_STATS, /* struct ndt_stats, read-only */
NDTA_GC_INTERVAL, /* u64, msecs */
+ NDTA_PAD,
__NDTA_MAX
};
#define NDTA_MAX (__NDTA_MAX - 1)
diff --git a/include/uapi/linux/tcp_metrics.h b/include/uapi/linux/tcp_metrics.h
index 93533926035c..80ad90d0cfc2 100644
--- a/include/uapi/linux/tcp_metrics.h
+++ b/include/uapi/linux/tcp_metrics.h
@@ -40,6 +40,7 @@ enum {
TCP_METRICS_ATTR_FOPEN_COOKIE, /* binary */
TCP_METRICS_ATTR_SADDR_IPV4, /* u32 */
TCP_METRICS_ATTR_SADDR_IPV6, /* binary */
+ TCP_METRICS_ATTR_PAD,

__TCP_METRICS_ATTR_MAX,
};
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index f18ae91b652e..6a395d440228 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1763,21 +1763,22 @@ static int neightbl_fill_parms(struct sk_buff *skb, struct neigh_parms *parms)
NEIGH_VAR(parms, MCAST_PROBES)) ||
nla_put_u32(skb, NDTPA_MCAST_REPROBES,
NEIGH_VAR(parms, MCAST_REPROBES)) ||
- nla_put_msecs(skb, NDTPA_REACHABLE_TIME, parms->reachable_time) ||
+ nla_put_msecs(skb, NDTPA_REACHABLE_TIME, parms->reachable_time,
+ NDTPA_PAD) ||
nla_put_msecs(skb, NDTPA_BASE_REACHABLE_TIME,
- NEIGH_VAR(parms, BASE_REACHABLE_TIME)) ||
+ NEIGH_VAR(parms, BASE_REACHABLE_TIME), NDTPA_PAD) ||
nla_put_msecs(skb, NDTPA_GC_STALETIME,
- NEIGH_VAR(parms, GC_STALETIME)) ||
+ NEIGH_VAR(parms, GC_STALETIME), NDTPA_PAD) ||
nla_put_msecs(skb, NDTPA_DELAY_PROBE_TIME,
- NEIGH_VAR(parms, DELAY_PROBE_TIME)) ||
+ NEIGH_VAR(parms, DELAY_PROBE_TIME), NDTPA_PAD) ||
nla_put_msecs(skb, NDTPA_RETRANS_TIME,
- NEIGH_VAR(parms, RETRANS_TIME)) ||
+ NEIGH_VAR(parms, RETRANS_TIME), NDTPA_PAD) ||
nla_put_msecs(skb, NDTPA_ANYCAST_DELAY,
- NEIGH_VAR(parms, ANYCAST_DELAY)) ||
+ NEIGH_VAR(parms, ANYCAST_DELAY), NDTPA_PAD) ||
nla_put_msecs(skb, NDTPA_PROXY_DELAY,
- NEIGH_VAR(parms, PROXY_DELAY)) ||
+ NEIGH_VAR(parms, PROXY_DELAY), NDTPA_PAD) ||
nla_put_msecs(skb, NDTPA_LOCKTIME,
- NEIGH_VAR(parms, LOCKTIME)))
+ NEIGH_VAR(parms, LOCKTIME), NDTPA_PAD))
goto nla_put_failure;
return nla_nest_end(skb, nest);

@@ -1804,7 +1805,7 @@ static int neightbl_fill_info(struct sk_buff *skb, struct neigh_table *tbl,
ndtmsg->ndtm_pad2 = 0;

if (nla_put_string(skb, NDTA_NAME, tbl->id) ||
- nla_put_msecs(skb, NDTA_GC_INTERVAL, tbl->gc_interval) ||
+ nla_put_msecs(skb, NDTA_GC_INTERVAL, tbl->gc_interval, NDTA_PAD) ||
nla_put_u32(skb, NDTA_THRESH1, tbl->gc_thresh1) ||
nla_put_u32(skb, NDTA_THRESH2, tbl->gc_thresh2) ||
nla_put_u32(skb, NDTA_THRESH3, tbl->gc_thresh3))
diff --git a/net/ipv4/tcp_metrics.c b/net/ipv4/tcp_metrics.c
index 7b7eec439906..b617826e2477 100644
--- a/net/ipv4/tcp_metrics.c
+++ b/net/ipv4/tcp_metrics.c
@@ -800,7 +800,8 @@ static int tcp_metrics_fill_info(struct sk_buff *msg,
}

if (nla_put_msecs(msg, TCP_METRICS_ATTR_AGE,
- jiffies - tm->tcpm_stamp) < 0)
+ jiffies - tm->tcpm_stamp,
+ TCP_METRICS_ATTR_PAD) < 0)
goto nla_put_failure;
if (tm->tcpm_ts_stamp) {
if (nla_put_s32(msg, TCP_METRICS_ATTR_TW_TS_STAMP,
@@ -864,7 +865,8 @@ static int tcp_metrics_fill_info(struct sk_buff *msg,
(nla_put_u16(msg, TCP_METRICS_ATTR_FOPEN_SYN_DROPS,
tfom->syn_loss) < 0 ||
nla_put_msecs(msg, TCP_METRICS_ATTR_FOPEN_SYN_DROP_TS,
- jiffies - tfom->last_syn_loss) < 0))
+ jiffies - tfom->last_syn_loss,
+ TCP_METRICS_ATTR_PAD) < 0))
goto nla_put_failure;
if (tfom->cookie.len > 0 &&
nla_put(msg, TCP_METRICS_ATTR_FOPEN_COOKIE,
diff --git a/net/l2tp/l2tp_netlink.c b/net/l2tp/l2tp_netlink.c
index 2caaa84ce92d..24ed2e875c45 100644
--- a/net/l2tp/l2tp_netlink.c
+++ b/net/l2tp/l2tp_netlink.c
@@ -746,7 +746,8 @@ static int l2tp_nl_session_send(struct sk_buff *skb, u32 portid, u32 seq, int fl
nla_put_u8(skb, L2TP_ATTR_USING_IPSEC, 1)) ||
#endif
(session->reorder_timeout &&
- nla_put_msecs(skb, L2TP_ATTR_RECV_TIMEOUT, session->reorder_timeout)))
+ nla_put_msecs(skb, L2TP_ATTR_RECV_TIMEOUT,
+ session->reorder_timeout, L2TP_ATTR_PAD)))
goto nla_put_failure;

nest = nla_nest_start(skb, L2TP_ATTR_STATS);
--
2.8.1

2016-04-22 15:42:00

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 5/9] libnl: nla_put_s64(): align on a 64-bit area

nla_data() is now aligned on a 64-bit area.
In fact, there is no user of this function.

Signed-off-by: Nicolas Dichtel <[email protected]>
---
include/net/netlink.h | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 066a921e7cbe..074215a59d19 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -102,7 +102,8 @@
* nla_put_s8(skb, type, value) add s8 attribute to skb
* nla_put_s16(skb, type, value) add s16 attribute to skb
* nla_put_s32(skb, type, value) add s32 attribute to skb
- * nla_put_s64(skb, type, value) add s64 attribute to skb
+ * nla_put_s64(skb, type, value,
+ * padattr) add s64 attribute to skb
* nla_put_string(skb, type, str) add string attribute to skb
* nla_put_flag(skb, type) add flag attribute to skb
* nla_put_msecs(skb, type, jiffies) add msecs attribute to skb
@@ -929,14 +930,16 @@ static inline int nla_put_s32(struct sk_buff *skb, int attrtype, s32 value)
}

/**
- * nla_put_s64 - Add a s64 netlink attribute to a socket buffer
+ * nla_put_s64 - Add a s64 netlink attribute to a socket buffer and align it
* @skb: socket buffer to add attribute to
* @attrtype: attribute type
* @value: numeric value
+ * @padattr: attribute type for the padding
*/
-static inline int nla_put_s64(struct sk_buff *skb, int attrtype, s64 value)
+static inline int nla_put_s64(struct sk_buff *skb, int attrtype, s64 value,
+ int padattr)
{
- return nla_put(skb, attrtype, sizeof(s64), &value);
+ return nla_put_64bit(skb, attrtype, sizeof(s64), &value, padattr);
}

/**
--
2.8.1

2016-04-22 15:42:37

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 1/9] libnl: fix help of _64bit functions

Fix typo and describe 'padattr'.

Fixes: 089bf1a6a924 ("libnl: add more helpers to align attributes on 64-bit")
Signed-off-by: Nicolas Dichtel <[email protected]>
---
lib/nlattr.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/nlattr.c b/lib/nlattr.c
index 2b82f1e2ebc2..fce1e9afc6d9 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -359,10 +359,11 @@ EXPORT_SYMBOL(__nla_reserve);
* @skb: socket buffer to reserve room on
* @attrtype: attribute type
* @attrlen: length of attribute payload
+ * @padattr: attribute type for the padding
*
* Adds a netlink attribute header to a socket buffer and reserves
* room for the payload but does not copy it. It also ensure that this
- * attribute will be 64-bit aign.
+ * attribute will have a 64-bit aligned nla_data() area.
*
* The caller is responsible to ensure that the skb provides enough
* tailroom for the attribute header and payload.
@@ -424,10 +425,11 @@ EXPORT_SYMBOL(nla_reserve);
* @skb: socket buffer to reserve room on
* @attrtype: attribute type
* @attrlen: length of attribute payload
+ * @padattr: attribute type for the padding
*
* Adds a netlink attribute header to a socket buffer and reserves
* room for the payload but does not copy it. It also ensure that this
- * attribute will be 64-bit aign.
+ * attribute will have a 64-bit aligned nla_data() area.
*
* Returns NULL if the tailroom of the skb is insufficient to store
* the attribute header and payload.
@@ -493,6 +495,7 @@ EXPORT_SYMBOL(__nla_put);
* @attrtype: attribute type
* @attrlen: length of attribute payload
* @data: head of attribute payload
+ * @padattr: attribute type for the padding
*
* The caller is responsible to ensure that the skb provides enough
* tailroom for the attribute header and payload.
@@ -551,6 +554,7 @@ EXPORT_SYMBOL(nla_put);
* @attrtype: attribute type
* @attrlen: length of attribute payload
* @data: head of attribute payload
+ * @padattr: attribute type for the padding
*
* Returns -EMSGSIZE if the tailroom of the skb is insufficient to store
* the attribute header and payload.
--
2.8.1

2016-04-22 15:41:59

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 4/9] libnl: nla_put_net64(): align on a 64-bit area

nla_data() is now aligned on a 64-bit area.

The temporary function nla_put_be64_32bit() is removed in this patch.

Signed-off-by: Nicolas Dichtel <[email protected]>
---
include/linux/netfilter/ipset/ip_set.h | 9 ++++++---
include/net/netlink.h | 14 ++++++--------
include/uapi/linux/netfilter/ipset/ip_set.h | 1 +
3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index f48b8a664b0f..83b9a2e0d8d4 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -351,7 +351,8 @@ ip_set_put_skbinfo(struct sk_buff *skb, struct ip_set_skbinfo *skbinfo)
return ((skbinfo->skbmark || skbinfo->skbmarkmask) &&
nla_put_net64(skb, IPSET_ATTR_SKBMARK,
cpu_to_be64((u64)skbinfo->skbmark << 32 |
- skbinfo->skbmarkmask))) ||
+ skbinfo->skbmarkmask),
+ IPSET_ATTR_PAD)) ||
(skbinfo->skbprio &&
nla_put_net32(skb, IPSET_ATTR_SKBPRIO,
cpu_to_be32(skbinfo->skbprio))) ||
@@ -374,9 +375,11 @@ static inline bool
ip_set_put_counter(struct sk_buff *skb, struct ip_set_counter *counter)
{
return nla_put_net64(skb, IPSET_ATTR_BYTES,
- cpu_to_be64(ip_set_get_bytes(counter))) ||
+ cpu_to_be64(ip_set_get_bytes(counter)),
+ IPSET_ATTR_PAD) ||
nla_put_net64(skb, IPSET_ATTR_PACKETS,
- cpu_to_be64(ip_set_get_packets(counter)));
+ cpu_to_be64(ip_set_get_packets(counter)),
+ IPSET_ATTR_PAD);
}

static inline void
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 47d7d1356fa3..066a921e7cbe 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -868,20 +868,18 @@ static inline int nla_put_be64(struct sk_buff *skb, int attrtype, __be64 value,
return nla_put_64bit(skb, attrtype, sizeof(__be64), &value, padattr);
}

-static inline int nla_put_be64_32bit(struct sk_buff *skb, int attrtype,
- __be64 value)
-{
- return nla_put(skb, attrtype, sizeof(__be64), &value);
-}
/**
- * nla_put_net64 - Add 64-bit network byte order netlink attribute to a socket buffer
+ * nla_put_net64 - Add 64-bit network byte order nlattr to a skb and align it
* @skb: socket buffer to add attribute to
* @attrtype: attribute type
* @value: numeric value
+ * @padattr: attribute type for the padding
*/
-static inline int nla_put_net64(struct sk_buff *skb, int attrtype, __be64 value)
+static inline int nla_put_net64(struct sk_buff *skb, int attrtype, __be64 value,
+ int padattr)
{
- return nla_put_be64_32bit(skb, attrtype | NLA_F_NET_BYTEORDER, value);
+ return nla_put_be64(skb, attrtype | NLA_F_NET_BYTEORDER, value,
+ padattr);
}

/**
diff --git a/include/uapi/linux/netfilter/ipset/ip_set.h b/include/uapi/linux/netfilter/ipset/ip_set.h
index 63b2e34f1b60..ebb5154976de 100644
--- a/include/uapi/linux/netfilter/ipset/ip_set.h
+++ b/include/uapi/linux/netfilter/ipset/ip_set.h
@@ -118,6 +118,7 @@ enum {
IPSET_ATTR_SKBMARK,
IPSET_ATTR_SKBPRIO,
IPSET_ATTR_SKBQUEUE,
+ IPSET_ATTR_PAD,
__IPSET_ATTR_ADT_MAX,
};
#define IPSET_ATTR_ADT_MAX (__IPSET_ATTR_ADT_MAX - 1)
--
2.8.1

2016-04-22 15:43:12

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 3/9] libnl: nla_put_be64(): align on a 64-bit area

nla_data() is now aligned on a 64-bit area.

A temporary version (nla_put_be64_32bit()) is added for nla_put_net64().
This function is removed in the next patch.

Signed-off-by: Nicolas Dichtel <[email protected]>
---
include/net/netlink.h | 15 ++++++++++----
include/uapi/linux/fib_rules.h | 1 +
include/uapi/linux/lwtunnel.h | 2 ++
include/uapi/linux/netfilter/nf_tables.h | 8 ++++++++
include/uapi/linux/netfilter/nfnetlink_acct.h | 1 +
include/uapi/linux/netfilter/nfnetlink_conntrack.h | 3 +++
include/uapi/linux/openvswitch.h | 1 +
net/core/fib_rules.c | 4 ++--
net/ipv4/ip_tunnel_core.c | 10 +++++----
net/netfilter/nf_conntrack_netlink.c | 18 +++++++++-------
net/netfilter/nf_conntrack_proto_dccp.c | 4 +++-
net/netfilter/nf_tables_api.c | 24 ++++++++++++++--------
net/netfilter/nf_tables_trace.c | 5 +++--
net/netfilter/nfnetlink_acct.c | 9 +++++---
net/netfilter/nft_counter.c | 6 ++++--
net/netfilter/nft_dynset.c | 3 ++-
net/netfilter/nft_limit.c | 6 ++++--
net/openvswitch/flow_netlink.c | 5 +++--
18 files changed, 87 insertions(+), 38 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 7f6b99483ab7..47d7d1356fa3 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -856,16 +856,23 @@ static inline int nla_put_u64(struct sk_buff *skb, int attrtype, u64 value)
}

/**
- * nla_put_be64 - Add a __be64 netlink attribute to a socket buffer
+ * 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
* @value: numeric value
+ * @padattr: attribute type for the padding
*/
-static inline int nla_put_be64(struct sk_buff *skb, int attrtype, __be64 value)
+static inline int nla_put_be64(struct sk_buff *skb, int attrtype, __be64 value,
+ int padattr)
{
- return nla_put(skb, attrtype, sizeof(__be64), &value);
+ return nla_put_64bit(skb, attrtype, sizeof(__be64), &value, padattr);
}

+static inline int nla_put_be64_32bit(struct sk_buff *skb, int attrtype,
+ __be64 value)
+{
+ return nla_put(skb, attrtype, sizeof(__be64), &value);
+}
/**
* nla_put_net64 - Add 64-bit network byte order netlink attribute to a socket buffer
* @skb: socket buffer to add attribute to
@@ -874,7 +881,7 @@ static inline int nla_put_be64(struct sk_buff *skb, int attrtype, __be64 value)
*/
static inline int nla_put_net64(struct sk_buff *skb, int attrtype, __be64 value)
{
- return nla_put_be64(skb, attrtype | NLA_F_NET_BYTEORDER, value);
+ return nla_put_be64_32bit(skb, attrtype | NLA_F_NET_BYTEORDER, value);
}

/**
diff --git a/include/uapi/linux/fib_rules.h b/include/uapi/linux/fib_rules.h
index 96161b8202b5..620c8a5ddc00 100644
--- a/include/uapi/linux/fib_rules.h
+++ b/include/uapi/linux/fib_rules.h
@@ -49,6 +49,7 @@ enum {
FRA_TABLE, /* Extended table id */
FRA_FWMASK, /* mask for netfilter mark */
FRA_OIFNAME,
+ FRA_PAD,
__FRA_MAX
};

diff --git a/include/uapi/linux/lwtunnel.h b/include/uapi/linux/lwtunnel.h
index f8b01887a495..a478fe80e203 100644
--- a/include/uapi/linux/lwtunnel.h
+++ b/include/uapi/linux/lwtunnel.h
@@ -22,6 +22,7 @@ enum lwtunnel_ip_t {
LWTUNNEL_IP_TTL,
LWTUNNEL_IP_TOS,
LWTUNNEL_IP_FLAGS,
+ LWTUNNEL_IP_PAD,
__LWTUNNEL_IP_MAX,
};

@@ -35,6 +36,7 @@ enum lwtunnel_ip6_t {
LWTUNNEL_IP6_HOPLIMIT,
LWTUNNEL_IP6_TC,
LWTUNNEL_IP6_FLAGS,
+ LWTUNNEL_IP6_PAD,
__LWTUNNEL_IP6_MAX,
};

diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index eeffde196f80..660231363bb5 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -182,6 +182,7 @@ enum nft_chain_attributes {
NFTA_CHAIN_USE,
NFTA_CHAIN_TYPE,
NFTA_CHAIN_COUNTERS,
+ NFTA_CHAIN_PAD,
__NFTA_CHAIN_MAX
};
#define NFTA_CHAIN_MAX (__NFTA_CHAIN_MAX - 1)
@@ -206,6 +207,7 @@ enum nft_rule_attributes {
NFTA_RULE_COMPAT,
NFTA_RULE_POSITION,
NFTA_RULE_USERDATA,
+ NFTA_RULE_PAD,
__NFTA_RULE_MAX
};
#define NFTA_RULE_MAX (__NFTA_RULE_MAX - 1)
@@ -308,6 +310,7 @@ enum nft_set_attributes {
NFTA_SET_TIMEOUT,
NFTA_SET_GC_INTERVAL,
NFTA_SET_USERDATA,
+ NFTA_SET_PAD,
__NFTA_SET_MAX
};
#define NFTA_SET_MAX (__NFTA_SET_MAX - 1)
@@ -341,6 +344,7 @@ enum nft_set_elem_attributes {
NFTA_SET_ELEM_EXPIRATION,
NFTA_SET_ELEM_USERDATA,
NFTA_SET_ELEM_EXPR,
+ NFTA_SET_ELEM_PAD,
__NFTA_SET_ELEM_MAX
};
#define NFTA_SET_ELEM_MAX (__NFTA_SET_ELEM_MAX - 1)
@@ -584,6 +588,7 @@ enum nft_dynset_attributes {
NFTA_DYNSET_SREG_DATA,
NFTA_DYNSET_TIMEOUT,
NFTA_DYNSET_EXPR,
+ NFTA_DYNSET_PAD,
__NFTA_DYNSET_MAX,
};
#define NFTA_DYNSET_MAX (__NFTA_DYNSET_MAX - 1)
@@ -806,6 +811,7 @@ enum nft_limit_attributes {
NFTA_LIMIT_BURST,
NFTA_LIMIT_TYPE,
NFTA_LIMIT_FLAGS,
+ NFTA_LIMIT_PAD,
__NFTA_LIMIT_MAX
};
#define NFTA_LIMIT_MAX (__NFTA_LIMIT_MAX - 1)
@@ -820,6 +826,7 @@ enum nft_counter_attributes {
NFTA_COUNTER_UNSPEC,
NFTA_COUNTER_BYTES,
NFTA_COUNTER_PACKETS,
+ NFTA_COUNTER_PAD,
__NFTA_COUNTER_MAX
};
#define NFTA_COUNTER_MAX (__NFTA_COUNTER_MAX - 1)
@@ -1055,6 +1062,7 @@ enum nft_trace_attibutes {
NFTA_TRACE_MARK,
NFTA_TRACE_NFPROTO,
NFTA_TRACE_POLICY,
+ NFTA_TRACE_PAD,
__NFTA_TRACE_MAX
};
#define NFTA_TRACE_MAX (__NFTA_TRACE_MAX - 1)
diff --git a/include/uapi/linux/netfilter/nfnetlink_acct.h b/include/uapi/linux/netfilter/nfnetlink_acct.h
index f3e34dbbf966..36047ec70f37 100644
--- a/include/uapi/linux/netfilter/nfnetlink_acct.h
+++ b/include/uapi/linux/netfilter/nfnetlink_acct.h
@@ -29,6 +29,7 @@ enum nfnl_acct_type {
NFACCT_FLAGS,
NFACCT_QUOTA,
NFACCT_FILTER,
+ NFACCT_PAD,
__NFACCT_MAX
};
#define NFACCT_MAX (__NFACCT_MAX - 1)
diff --git a/include/uapi/linux/netfilter/nfnetlink_conntrack.h b/include/uapi/linux/netfilter/nfnetlink_conntrack.h
index c1a4e1441a25..9df789709abe 100644
--- a/include/uapi/linux/netfilter/nfnetlink_conntrack.h
+++ b/include/uapi/linux/netfilter/nfnetlink_conntrack.h
@@ -116,6 +116,7 @@ enum ctattr_protoinfo_dccp {
CTA_PROTOINFO_DCCP_STATE,
CTA_PROTOINFO_DCCP_ROLE,
CTA_PROTOINFO_DCCP_HANDSHAKE_SEQ,
+ CTA_PROTOINFO_DCCP_PAD,
__CTA_PROTOINFO_DCCP_MAX,
};
#define CTA_PROTOINFO_DCCP_MAX (__CTA_PROTOINFO_DCCP_MAX - 1)
@@ -135,6 +136,7 @@ enum ctattr_counters {
CTA_COUNTERS_BYTES, /* 64bit counters */
CTA_COUNTERS32_PACKETS, /* old 32bit counters, unused */
CTA_COUNTERS32_BYTES, /* old 32bit counters, unused */
+ CTA_COUNTERS_PAD,
__CTA_COUNTERS_MAX
};
#define CTA_COUNTERS_MAX (__CTA_COUNTERS_MAX - 1)
@@ -143,6 +145,7 @@ enum ctattr_tstamp {
CTA_TIMESTAMP_UNSPEC,
CTA_TIMESTAMP_START,
CTA_TIMESTAMP_STOP,
+ CTA_TIMESTAMP_PAD,
__CTA_TIMESTAMP_MAX
};
#define CTA_TIMESTAMP_MAX (__CTA_TIMESTAMP_MAX - 1)
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
index 616d04761730..0358f94af86e 100644
--- a/include/uapi/linux/openvswitch.h
+++ b/include/uapi/linux/openvswitch.h
@@ -351,6 +351,7 @@ enum ovs_tunnel_key_attr {
OVS_TUNNEL_KEY_ATTR_VXLAN_OPTS, /* Nested OVS_VXLAN_EXT_* */
OVS_TUNNEL_KEY_ATTR_IPV6_SRC, /* struct in6_addr src IPv6 address. */
OVS_TUNNEL_KEY_ATTR_IPV6_DST, /* struct in6_addr dst IPv6 address. */
+ OVS_TUNNEL_KEY_ATTR_PAD,
__OVS_TUNNEL_KEY_ATTR_MAX
};

diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c
index 365de66436ac..840acebbb80c 100644
--- a/net/core/fib_rules.c
+++ b/net/core/fib_rules.c
@@ -549,7 +549,7 @@ static inline size_t fib_rule_nlmsg_size(struct fib_rules_ops *ops,
+ nla_total_size(4) /* FRA_SUPPRESS_IFGROUP */
+ nla_total_size(4) /* FRA_FWMARK */
+ nla_total_size(4) /* FRA_FWMASK */
- + nla_total_size(8); /* FRA_TUN_ID */
+ + nla_total_size_64bit(8); /* FRA_TUN_ID */

if (ops->nlmsg_payload)
payload += ops->nlmsg_payload(rule);
@@ -607,7 +607,7 @@ static int fib_nl_fill_rule(struct sk_buff *skb, struct fib_rule *rule,
(rule->target &&
nla_put_u32(skb, FRA_GOTO, rule->target)) ||
(rule->tun_id &&
- nla_put_be64(skb, FRA_TUN_ID, rule->tun_id)))
+ nla_put_be64(skb, FRA_TUN_ID, rule->tun_id, FRA_PAD)))
goto nla_put_failure;

if (rule->suppress_ifgroup != -1) {
diff --git a/net/ipv4/ip_tunnel_core.c b/net/ipv4/ip_tunnel_core.c
index f46c5c873831..786fa7ca28e0 100644
--- a/net/ipv4/ip_tunnel_core.c
+++ b/net/ipv4/ip_tunnel_core.c
@@ -271,7 +271,8 @@ static int ip_tun_fill_encap_info(struct sk_buff *skb,
{
struct ip_tunnel_info *tun_info = lwt_tun_info(lwtstate);

- if (nla_put_be64(skb, LWTUNNEL_IP_ID, tun_info->key.tun_id) ||
+ if (nla_put_be64(skb, LWTUNNEL_IP_ID, tun_info->key.tun_id,
+ LWTUNNEL_IP_PAD) ||
nla_put_in_addr(skb, LWTUNNEL_IP_DST, tun_info->key.u.ipv4.dst) ||
nla_put_in_addr(skb, LWTUNNEL_IP_SRC, tun_info->key.u.ipv4.src) ||
nla_put_u8(skb, LWTUNNEL_IP_TOS, tun_info->key.tos) ||
@@ -284,7 +285,7 @@ static int ip_tun_fill_encap_info(struct sk_buff *skb,

static int ip_tun_encap_nlsize(struct lwtunnel_state *lwtstate)
{
- return nla_total_size(8) /* LWTUNNEL_IP_ID */
+ return nla_total_size_64bit(8) /* LWTUNNEL_IP_ID */
+ nla_total_size(4) /* LWTUNNEL_IP_DST */
+ nla_total_size(4) /* LWTUNNEL_IP_SRC */
+ nla_total_size(1) /* LWTUNNEL_IP_TOS */
@@ -366,7 +367,8 @@ static int ip6_tun_fill_encap_info(struct sk_buff *skb,
{
struct ip_tunnel_info *tun_info = lwt_tun_info(lwtstate);

- if (nla_put_be64(skb, LWTUNNEL_IP6_ID, tun_info->key.tun_id) ||
+ if (nla_put_be64(skb, LWTUNNEL_IP6_ID, tun_info->key.tun_id,
+ LWTUNNEL_IP6_PAD) ||
nla_put_in6_addr(skb, LWTUNNEL_IP6_DST, &tun_info->key.u.ipv6.dst) ||
nla_put_in6_addr(skb, LWTUNNEL_IP6_SRC, &tun_info->key.u.ipv6.src) ||
nla_put_u8(skb, LWTUNNEL_IP6_TC, tun_info->key.tos) ||
@@ -379,7 +381,7 @@ static int ip6_tun_fill_encap_info(struct sk_buff *skb,

static int ip6_tun_encap_nlsize(struct lwtunnel_state *lwtstate)
{
- return nla_total_size(8) /* LWTUNNEL_IP6_ID */
+ return nla_total_size_64bit(8) /* LWTUNNEL_IP6_ID */
+ nla_total_size(16) /* LWTUNNEL_IP6_DST */
+ nla_total_size(16) /* LWTUNNEL_IP6_SRC */
+ nla_total_size(1) /* LWTUNNEL_IP6_HOPLIMIT */
diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 355e8552fd5b..3362d65f3285 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -245,8 +245,10 @@ dump_counters(struct sk_buff *skb, struct nf_conn_acct *acct,
if (!nest_count)
goto nla_put_failure;

- if (nla_put_be64(skb, CTA_COUNTERS_PACKETS, cpu_to_be64(pkts)) ||
- nla_put_be64(skb, CTA_COUNTERS_BYTES, cpu_to_be64(bytes)))
+ if (nla_put_be64(skb, CTA_COUNTERS_PACKETS, cpu_to_be64(pkts),
+ CTA_COUNTERS_PAD) ||
+ nla_put_be64(skb, CTA_COUNTERS_BYTES, cpu_to_be64(bytes),
+ CTA_COUNTERS_PAD))
goto nla_put_failure;

nla_nest_end(skb, nest_count);
@@ -287,9 +289,11 @@ ctnetlink_dump_timestamp(struct sk_buff *skb, const struct nf_conn *ct)
if (!nest_count)
goto nla_put_failure;

- if (nla_put_be64(skb, CTA_TIMESTAMP_START, cpu_to_be64(tstamp->start)) ||
+ if (nla_put_be64(skb, CTA_TIMESTAMP_START, cpu_to_be64(tstamp->start),
+ CTA_TIMESTAMP_PAD) ||
(tstamp->stop != 0 && nla_put_be64(skb, CTA_TIMESTAMP_STOP,
- cpu_to_be64(tstamp->stop))))
+ cpu_to_be64(tstamp->stop),
+ CTA_TIMESTAMP_PAD)))
goto nla_put_failure;
nla_nest_end(skb, nest_count);

@@ -562,8 +566,8 @@ ctnetlink_acct_size(const struct nf_conn *ct)
if (!nf_ct_ext_exist(ct, NF_CT_EXT_ACCT))
return 0;
return 2 * nla_total_size(0) /* CTA_COUNTERS_ORIG|REPL */
- + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_PACKETS */
- + 2 * nla_total_size(sizeof(uint64_t)) /* CTA_COUNTERS_BYTES */
+ + 2 * nla_total_size_64bit(sizeof(uint64_t)) /* CTA_COUNTERS_PACKETS */
+ + 2 * nla_total_size_64bit(sizeof(uint64_t)) /* CTA_COUNTERS_BYTES */
;
}

@@ -590,7 +594,7 @@ ctnetlink_timestamp_size(const struct nf_conn *ct)
#ifdef CONFIG_NF_CONNTRACK_TIMESTAMP
if (!nf_ct_ext_exist(ct, NF_CT_EXT_TSTAMP))
return 0;
- return nla_total_size(0) + 2 * nla_total_size(sizeof(uint64_t));
+ return nla_total_size(0) + 2 * nla_total_size_64bit(sizeof(uint64_t));
#else
return 0;
#endif
diff --git a/net/netfilter/nf_conntrack_proto_dccp.c b/net/netfilter/nf_conntrack_proto_dccp.c
index fce1b1cca32d..399a38fd685a 100644
--- a/net/netfilter/nf_conntrack_proto_dccp.c
+++ b/net/netfilter/nf_conntrack_proto_dccp.c
@@ -645,7 +645,8 @@ static int dccp_to_nlattr(struct sk_buff *skb, struct nlattr *nla,
nla_put_u8(skb, CTA_PROTOINFO_DCCP_ROLE,
ct->proto.dccp.role[IP_CT_DIR_ORIGINAL]) ||
nla_put_be64(skb, CTA_PROTOINFO_DCCP_HANDSHAKE_SEQ,
- cpu_to_be64(ct->proto.dccp.handshake_seq)))
+ cpu_to_be64(ct->proto.dccp.handshake_seq),
+ CTA_PROTOINFO_DCCP_PAD))
goto nla_put_failure;
nla_nest_end(skb, nest_parms);
spin_unlock_bh(&ct->lock);
@@ -660,6 +661,7 @@ static const struct nla_policy dccp_nla_policy[CTA_PROTOINFO_DCCP_MAX + 1] = {
[CTA_PROTOINFO_DCCP_STATE] = { .type = NLA_U8 },
[CTA_PROTOINFO_DCCP_ROLE] = { .type = NLA_U8 },
[CTA_PROTOINFO_DCCP_HANDSHAKE_SEQ] = { .type = NLA_U64 },
+ [CTA_PROTOINFO_DCCP_PAD] = { .type = NLA_UNSPEC },
};

static int nlattr_to_dccp(struct nlattr *cda[], struct nf_conn *ct)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 2011977cd79d..7a85a9dd37ad 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -944,8 +944,10 @@ static int nft_dump_stats(struct sk_buff *skb, struct nft_stats __percpu *stats)
if (nest == NULL)
goto nla_put_failure;

- if (nla_put_be64(skb, NFTA_COUNTER_PACKETS, cpu_to_be64(total.pkts)) ||
- nla_put_be64(skb, NFTA_COUNTER_BYTES, cpu_to_be64(total.bytes)))
+ if (nla_put_be64(skb, NFTA_COUNTER_PACKETS, cpu_to_be64(total.pkts),
+ NFTA_COUNTER_PAD) ||
+ nla_put_be64(skb, NFTA_COUNTER_BYTES, cpu_to_be64(total.bytes),
+ NFTA_COUNTER_PAD))
goto nla_put_failure;

nla_nest_end(skb, nest);
@@ -975,7 +977,8 @@ static int nf_tables_fill_chain_info(struct sk_buff *skb, struct net *net,

if (nla_put_string(skb, NFTA_CHAIN_TABLE, table->name))
goto nla_put_failure;
- if (nla_put_be64(skb, NFTA_CHAIN_HANDLE, cpu_to_be64(chain->handle)))
+ if (nla_put_be64(skb, NFTA_CHAIN_HANDLE, cpu_to_be64(chain->handle),
+ NFTA_CHAIN_PAD))
goto nla_put_failure;
if (nla_put_string(skb, NFTA_CHAIN_NAME, chain->name))
goto nla_put_failure;
@@ -1803,13 +1806,15 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net,
goto nla_put_failure;
if (nla_put_string(skb, NFTA_RULE_CHAIN, chain->name))
goto nla_put_failure;
- if (nla_put_be64(skb, NFTA_RULE_HANDLE, cpu_to_be64(rule->handle)))
+ if (nla_put_be64(skb, NFTA_RULE_HANDLE, cpu_to_be64(rule->handle),
+ NFTA_RULE_PAD))
goto nla_put_failure;

if ((event != NFT_MSG_DELRULE) && (rule->list.prev != &chain->rules)) {
prule = list_entry(rule->list.prev, struct nft_rule, list);
if (nla_put_be64(skb, NFTA_RULE_POSITION,
- cpu_to_be64(prule->handle)))
+ cpu_to_be64(prule->handle),
+ NFTA_RULE_PAD))
goto nla_put_failure;
}

@@ -2473,7 +2478,8 @@ static int nf_tables_fill_set(struct sk_buff *skb, const struct nft_ctx *ctx,
}

if (set->timeout &&
- nla_put_be64(skb, NFTA_SET_TIMEOUT, cpu_to_be64(set->timeout)))
+ nla_put_be64(skb, NFTA_SET_TIMEOUT, cpu_to_be64(set->timeout),
+ NFTA_SET_PAD))
goto nla_put_failure;
if (set->gc_int &&
nla_put_be32(skb, NFTA_SET_GC_INTERVAL, htonl(set->gc_int)))
@@ -3076,7 +3082,8 @@ static int nf_tables_fill_setelem(struct sk_buff *skb,

if (nft_set_ext_exists(ext, NFT_SET_EXT_TIMEOUT) &&
nla_put_be64(skb, NFTA_SET_ELEM_TIMEOUT,
- cpu_to_be64(*nft_set_ext_timeout(ext))))
+ cpu_to_be64(*nft_set_ext_timeout(ext)),
+ NFTA_SET_ELEM_PAD))
goto nla_put_failure;

if (nft_set_ext_exists(ext, NFT_SET_EXT_EXPIRATION)) {
@@ -3089,7 +3096,8 @@ static int nf_tables_fill_setelem(struct sk_buff *skb,
expires = 0;

if (nla_put_be64(skb, NFTA_SET_ELEM_EXPIRATION,
- cpu_to_be64(jiffies_to_msecs(expires))))
+ cpu_to_be64(jiffies_to_msecs(expires)),
+ NFTA_SET_ELEM_PAD))
goto nla_put_failure;
}

diff --git a/net/netfilter/nf_tables_trace.c b/net/netfilter/nf_tables_trace.c
index e9e959f65d91..39eb1cc62e91 100644
--- a/net/netfilter/nf_tables_trace.c
+++ b/net/netfilter/nf_tables_trace.c
@@ -156,7 +156,8 @@ static int nf_trace_fill_rule_info(struct sk_buff *nlskb,
return 0;

return nla_put_be64(nlskb, NFTA_TRACE_RULE_HANDLE,
- cpu_to_be64(info->rule->handle));
+ cpu_to_be64(info->rule->handle),
+ NFTA_TRACE_PAD);
}

void nft_trace_notify(struct nft_traceinfo *info)
@@ -174,7 +175,7 @@ void nft_trace_notify(struct nft_traceinfo *info)
size = nlmsg_total_size(sizeof(struct nfgenmsg)) +
nla_total_size(NFT_TABLE_MAXNAMELEN) +
nla_total_size(NFT_CHAIN_MAXNAMELEN) +
- nla_total_size(sizeof(__be64)) + /* rule handle */
+ nla_total_size_64bit(sizeof(__be64)) + /* rule handle */
nla_total_size(sizeof(__be32)) + /* trace type */
nla_total_size(0) + /* VERDICT, nested */
nla_total_size(sizeof(u32)) + /* verdict code */
diff --git a/net/netfilter/nfnetlink_acct.c b/net/netfilter/nfnetlink_acct.c
index 4c2b4c0c4d5f..d016066a25e3 100644
--- a/net/netfilter/nfnetlink_acct.c
+++ b/net/netfilter/nfnetlink_acct.c
@@ -160,15 +160,18 @@ nfnl_acct_fill_info(struct sk_buff *skb, u32 portid, u32 seq, u32 type,
pkts = atomic64_read(&acct->pkts);
bytes = atomic64_read(&acct->bytes);
}
- if (nla_put_be64(skb, NFACCT_PKTS, cpu_to_be64(pkts)) ||
- nla_put_be64(skb, NFACCT_BYTES, cpu_to_be64(bytes)) ||
+ if (nla_put_be64(skb, NFACCT_PKTS, cpu_to_be64(pkts),
+ NFACCT_PAD) ||
+ nla_put_be64(skb, NFACCT_BYTES, cpu_to_be64(bytes),
+ NFACCT_PAD) ||
nla_put_be32(skb, NFACCT_USE, htonl(atomic_read(&acct->refcnt))))
goto nla_put_failure;
if (acct->flags & NFACCT_F_QUOTA) {
u64 *quota = (u64 *)acct->data;

if (nla_put_be32(skb, NFACCT_FLAGS, htonl(old_flags)) ||
- nla_put_be64(skb, NFACCT_QUOTA, cpu_to_be64(*quota)))
+ nla_put_be64(skb, NFACCT_QUOTA, cpu_to_be64(*quota),
+ NFACCT_PAD))
goto nla_put_failure;
}
nlmsg_end(skb, nlh);
diff --git a/net/netfilter/nft_counter.c b/net/netfilter/nft_counter.c
index c9743f78f219..77db8358ab14 100644
--- a/net/netfilter/nft_counter.c
+++ b/net/netfilter/nft_counter.c
@@ -76,8 +76,10 @@ static int nft_counter_dump(struct sk_buff *skb, const struct nft_expr *expr)

nft_counter_fetch(priv->counter, &total);

- if (nla_put_be64(skb, NFTA_COUNTER_BYTES, cpu_to_be64(total.bytes)) ||
- nla_put_be64(skb, NFTA_COUNTER_PACKETS, cpu_to_be64(total.packets)))
+ if (nla_put_be64(skb, NFTA_COUNTER_BYTES, cpu_to_be64(total.bytes),
+ NFTA_COUNTER_PAD) ||
+ nla_put_be64(skb, NFTA_COUNTER_PACKETS, cpu_to_be64(total.packets),
+ NFTA_COUNTER_PAD))
goto nla_put_failure;
return 0;

diff --git a/net/netfilter/nft_dynset.c b/net/netfilter/nft_dynset.c
index 9dec3bd1b63c..78d4914fb39c 100644
--- a/net/netfilter/nft_dynset.c
+++ b/net/netfilter/nft_dynset.c
@@ -227,7 +227,8 @@ static int nft_dynset_dump(struct sk_buff *skb, const struct nft_expr *expr)
goto nla_put_failure;
if (nla_put_string(skb, NFTA_DYNSET_SET_NAME, priv->set->name))
goto nla_put_failure;
- if (nla_put_be64(skb, NFTA_DYNSET_TIMEOUT, cpu_to_be64(priv->timeout)))
+ if (nla_put_be64(skb, NFTA_DYNSET_TIMEOUT, cpu_to_be64(priv->timeout),
+ NFTA_DYNSET_PAD))
goto nla_put_failure;
if (priv->expr && nft_expr_dump(skb, NFTA_DYNSET_EXPR, priv->expr))
goto nla_put_failure;
diff --git a/net/netfilter/nft_limit.c b/net/netfilter/nft_limit.c
index 99d18578afc6..070b98938e02 100644
--- a/net/netfilter/nft_limit.c
+++ b/net/netfilter/nft_limit.c
@@ -97,8 +97,10 @@ static int nft_limit_dump(struct sk_buff *skb, const struct nft_limit *limit,
u64 secs = div_u64(limit->nsecs, NSEC_PER_SEC);
u64 rate = limit->rate - limit->burst;

- if (nla_put_be64(skb, NFTA_LIMIT_RATE, cpu_to_be64(rate)) ||
- nla_put_be64(skb, NFTA_LIMIT_UNIT, cpu_to_be64(secs)) ||
+ if (nla_put_be64(skb, NFTA_LIMIT_RATE, cpu_to_be64(rate),
+ NFTA_LIMIT_PAD) ||
+ nla_put_be64(skb, NFTA_LIMIT_UNIT, cpu_to_be64(secs),
+ NFTA_LIMIT_PAD) ||
nla_put_be32(skb, NFTA_LIMIT_BURST, htonl(limit->burst)) ||
nla_put_be32(skb, NFTA_LIMIT_TYPE, htonl(type)) ||
nla_put_be32(skb, NFTA_LIMIT_FLAGS, htonl(flags)))
diff --git a/net/openvswitch/flow_netlink.c b/net/openvswitch/flow_netlink.c
index 689c17264221..0bb650f4f219 100644
--- a/net/openvswitch/flow_netlink.c
+++ b/net/openvswitch/flow_netlink.c
@@ -261,7 +261,7 @@ size_t ovs_tun_key_attr_size(void)
/* Whenever adding new OVS_TUNNEL_KEY_ FIELDS, we should consider
* updating this function.
*/
- return nla_total_size(8) /* OVS_TUNNEL_KEY_ATTR_ID */
+ return nla_total_size_64bit(8) /* OVS_TUNNEL_KEY_ATTR_ID */
+ nla_total_size(16) /* OVS_TUNNEL_KEY_ATTR_IPV[46]_SRC */
+ nla_total_size(16) /* OVS_TUNNEL_KEY_ATTR_IPV[46]_DST */
+ nla_total_size(1) /* OVS_TUNNEL_KEY_ATTR_TOS */
@@ -720,7 +720,8 @@ static int __ip_tun_to_nlattr(struct sk_buff *skb,
unsigned short tun_proto)
{
if (output->tun_flags & TUNNEL_KEY &&
- nla_put_be64(skb, OVS_TUNNEL_KEY_ATTR_ID, output->tun_id))
+ nla_put_be64(skb, OVS_TUNNEL_KEY_ATTR_ID, output->tun_id,
+ OVS_TUNNEL_KEY_ATTR_PAD))
return -EMSGSIZE;
switch (tun_proto) {
case AF_INET:
--
2.8.1

2016-04-22 15:43:36

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 8/9] xfrm: align nlattr properly when needed

Signed-off-by: Nicolas Dichtel <[email protected]>
---
include/uapi/linux/xfrm.h | 1 +
net/xfrm/xfrm_user.c | 10 ++++++----
2 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/include/uapi/linux/xfrm.h b/include/uapi/linux/xfrm.h
index 2cd9e608d0d1..143338978b48 100644
--- a/include/uapi/linux/xfrm.h
+++ b/include/uapi/linux/xfrm.h
@@ -302,6 +302,7 @@ enum xfrm_attr_type_t {
XFRMA_SA_EXTRA_FLAGS, /* __u32 */
XFRMA_PROTO, /* __u8 */
XFRMA_ADDRESS_FILTER, /* struct xfrm_address_filter */
+ XFRMA_PAD,
__XFRMA_MAX

#define XFRMA_MAX (__XFRMA_MAX - 1)
diff --git a/net/xfrm/xfrm_user.c b/net/xfrm/xfrm_user.c
index 2cc7af858c6f..d516845e16e3 100644
--- a/net/xfrm/xfrm_user.c
+++ b/net/xfrm/xfrm_user.c
@@ -809,7 +809,8 @@ static int copy_to_user_state_extra(struct xfrm_state *x,
goto out;
}
if (x->lastused) {
- ret = nla_put_u64(skb, XFRMA_LASTUSED, x->lastused);
+ ret = nla_put_u64_64bit(skb, XFRMA_LASTUSED, x->lastused,
+ XFRMA_PAD);
if (ret)
goto out;
}
@@ -1813,7 +1814,7 @@ static inline size_t xfrm_aevent_msgsize(struct xfrm_state *x)

return NLMSG_ALIGN(sizeof(struct xfrm_aevent_id))
+ nla_total_size(replay_size)
- + nla_total_size(sizeof(struct xfrm_lifetime_cur))
+ + nla_total_size_64bit(sizeof(struct xfrm_lifetime_cur))
+ nla_total_size(sizeof(struct xfrm_mark))
+ nla_total_size(4) /* XFRM_AE_RTHR */
+ nla_total_size(4); /* XFRM_AE_ETHR */
@@ -1848,7 +1849,8 @@ static int build_aevent(struct sk_buff *skb, struct xfrm_state *x, const struct
}
if (err)
goto out_cancel;
- err = nla_put(skb, XFRMA_LTIME_VAL, sizeof(x->curlft), &x->curlft);
+ err = nla_put_64bit(skb, XFRMA_LTIME_VAL, sizeof(x->curlft), &x->curlft,
+ XFRMA_PAD);
if (err)
goto out_cancel;

@@ -2617,7 +2619,7 @@ static inline size_t xfrm_sa_len(struct xfrm_state *x)
l += nla_total_size(sizeof(x->props.extra_flags));

/* Must count x->lastused as it may become non-zero behind our back. */
- l += nla_total_size(sizeof(u64));
+ l += nla_total_size_64bit(sizeof(u64));

return l;
}
--
2.8.1

2016-04-22 15:43:51

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 7/9] libnl: add nla_put_u64_64bit() helper

With this function, nla_data() is aligned on a 64-bit area.

Signed-off-by: Nicolas Dichtel <[email protected]>
---
include/net/netlink.h | 13 +++++++++++++
1 file changed, 13 insertions(+)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 113b483b6ee8..e589cb3dccee 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -858,6 +858,19 @@ static inline int nla_put_u64(struct sk_buff *skb, int attrtype, u64 value)
}

/**
+ * nla_put_u64_64bit - Add a u64 netlink attribute to a skb and align it
+ * @skb: socket buffer to add attribute to
+ * @attrtype: attribute type
+ * @value: numeric value
+ * @padattr: attribute type for the padding
+ */
+static inline int nla_put_u64_64bit(struct sk_buff *skb, int attrtype,
+ u64 value, int padattr)
+{
+ return nla_put_64bit(skb, attrtype, sizeof(u64), &value, padattr);
+}
+
+/**
* 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
--
2.8.1

2016-04-22 15:44:21

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next 2/9] libnl: nla_put_le64(): align on a 64-bit area

nla_data() is now aligned on a 64-bit area.

Signed-off-by: Nicolas Dichtel <[email protected]>
---
include/net/netlink.h | 8 +++++---
include/net/nl802154.h | 6 ++++++
net/ieee802154/nl802154.c | 13 ++++++++-----
3 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 6f51a8a06498..7f6b99483ab7 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -878,14 +878,16 @@ static inline int nla_put_net64(struct sk_buff *skb, int attrtype, __be64 value)
}

/**
- * nla_put_le64 - Add a __le64 netlink attribute to a socket buffer
+ * nla_put_le64 - Add a __le64 netlink attribute to a socket buffer and align it
* @skb: socket buffer to add attribute to
* @attrtype: attribute type
* @value: numeric value
+ * @padattr: attribute type for the padding
*/
-static inline int nla_put_le64(struct sk_buff *skb, int attrtype, __le64 value)
+static inline int nla_put_le64(struct sk_buff *skb, int attrtype, __le64 value,
+ int padattr)
{
- return nla_put(skb, attrtype, sizeof(__le64), &value);
+ return nla_put_64bit(skb, attrtype, sizeof(__le64), &value, padattr);
}

/**
diff --git a/include/net/nl802154.h b/include/net/nl802154.h
index 32cb3e591e07..fcab4de49951 100644
--- a/include/net/nl802154.h
+++ b/include/net/nl802154.h
@@ -138,6 +138,8 @@ enum nl802154_attrs {
NL802154_ATTR_SEC_KEY,
#endif /* CONFIG_IEEE802154_NL802154_EXPERIMENTAL */

+ NL802154_ATTR_PAD,
+
__NL802154_ATTR_AFTER_LAST,
NL802154_ATTR_MAX = __NL802154_ATTR_AFTER_LAST - 1
};
@@ -295,6 +297,7 @@ enum nl802154_dev_addr_attrs {
NL802154_DEV_ADDR_ATTR_MODE,
NL802154_DEV_ADDR_ATTR_SHORT,
NL802154_DEV_ADDR_ATTR_EXTENDED,
+ NL802154_DEV_ADDR_ATTR_PAD,

/* keep last */
__NL802154_DEV_ADDR_ATTR_AFTER_LAST,
@@ -320,6 +323,7 @@ enum nl802154_key_id_attrs {
NL802154_KEY_ID_ATTR_IMPLICIT,
NL802154_KEY_ID_ATTR_SOURCE_SHORT,
NL802154_KEY_ID_ATTR_SOURCE_EXTENDED,
+ NL802154_KEY_ID_ATTR_PAD,

/* keep last */
__NL802154_KEY_ID_ATTR_AFTER_LAST,
@@ -402,6 +406,7 @@ enum nl802154_dev {
NL802154_DEV_ATTR_EXTENDED_ADDR,
NL802154_DEV_ATTR_SECLEVEL_EXEMPT,
NL802154_DEV_ATTR_KEY_MODE,
+ NL802154_DEV_ATTR_PAD,

/* keep last */
__NL802154_DEV_ATTR_AFTER_LAST,
@@ -414,6 +419,7 @@ enum nl802154_devkey {
NL802154_DEVKEY_ATTR_FRAME_COUNTER,
NL802154_DEVKEY_ATTR_EXTENDED_ADDR,
NL802154_DEVKEY_ATTR_ID,
+ NL802154_DEVKEY_ATTR_PAD,

/* keep last */
__NL802154_DEVKEY_ATTR_AFTER_LAST,
diff --git a/net/ieee802154/nl802154.c b/net/ieee802154/nl802154.c
index 16ef0d9f566e..614072064d03 100644
--- a/net/ieee802154/nl802154.c
+++ b/net/ieee802154/nl802154.c
@@ -722,7 +722,8 @@ ieee802154_llsec_send_key_id(struct sk_buff *msg,
break;
case NL802154_DEV_ADDR_EXTENDED:
if (nla_put_le64(msg, NL802154_DEV_ADDR_ATTR_EXTENDED,
- desc->device_addr.extended_addr))
+ desc->device_addr.extended_addr,
+ NL802154_DEV_ADDR_ATTR_PAD))
return -ENOBUFS;
break;
default:
@@ -742,7 +743,8 @@ ieee802154_llsec_send_key_id(struct sk_buff *msg,
break;
case NL802154_KEY_ID_MODE_INDEX_EXTENDED:
if (nla_put_le64(msg, NL802154_KEY_ID_ATTR_SOURCE_EXTENDED,
- desc->extended_source))
+ desc->extended_source,
+ NL802154_KEY_ID_ATTR_PAD))
return -ENOBUFS;
break;
default:
@@ -819,7 +821,8 @@ nl802154_send_iface(struct sk_buff *msg, u32 portid, u32 seq, int flags,

/* address settings */
if (nla_put_le64(msg, NL802154_ATTR_EXTENDED_ADDR,
- wpan_dev->extended_addr) ||
+ wpan_dev->extended_addr,
+ NL802154_ATTR_PAD) ||
nla_put_le16(msg, NL802154_ATTR_SHORT_ADDR,
wpan_dev->short_addr) ||
nla_put_le16(msg, NL802154_ATTR_PAN_ID, wpan_dev->pan_id))
@@ -1614,7 +1617,7 @@ static int nl802154_send_device(struct sk_buff *msg, u32 cmd, u32 portid,
nla_put_le16(msg, NL802154_DEV_ATTR_SHORT_ADDR,
dev_desc->short_addr) ||
nla_put_le64(msg, NL802154_DEV_ATTR_EXTENDED_ADDR,
- dev_desc->hwaddr) ||
+ dev_desc->hwaddr, NL802154_DEV_ATTR_PAD) ||
nla_put_u8(msg, NL802154_DEV_ATTR_SECLEVEL_EXEMPT,
dev_desc->seclevel_exempt) ||
nla_put_u32(msg, NL802154_DEV_ATTR_KEY_MODE, dev_desc->key_mode))
@@ -1778,7 +1781,7 @@ static int nl802154_send_devkey(struct sk_buff *msg, u32 cmd, u32 portid,
goto nla_put_failure;

if (nla_put_le64(msg, NL802154_DEVKEY_ATTR_EXTENDED_ADDR,
- extended_addr) ||
+ extended_addr, NL802154_DEVKEY_ATTR_PAD) ||
nla_put_u32(msg, NL802154_DEVKEY_ATTR_FRAME_COUNTER,
devkey->frame_counter))
goto nla_put_failure;
--
2.8.1

2016-04-22 16:51:26

by Eric Dumazet

[permalink] [raw]
Subject: Re: [PATCH net-next 2/9] libnl: nla_put_le64(): align on a 64-bit area

On Fri, 2016-04-22 at 17:31 +0200, Nicolas Dichtel wrote:
> nla_data() is now aligned on a 64-bit area.
>
> Signed-off-by: Nicolas Dichtel <[email protected]>
> ---
> include/net/netlink.h | 8 +++++---
> include/net/nl802154.h | 6 ++++++
> net/ieee802154/nl802154.c | 13 ++++++++-----
> 3 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index 6f51a8a06498..7f6b99483ab7 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -878,14 +878,16 @@ static inline int nla_put_net64(struct sk_buff *skb, int attrtype, __be64 value)
> }
>
> /**
> - * nla_put_le64 - Add a __le64 netlink attribute to a socket buffer
> + * nla_put_le64 - Add a __le64 netlink attribute to a socket buffer and align it
> * @skb: socket buffer to add attribute to
> * @attrtype: attribute type
> * @value: numeric value
> + * @padattr: attribute type for the padding
> */
> -static inline int nla_put_le64(struct sk_buff *skb, int attrtype, __le64 value)
> +static inline int nla_put_le64(struct sk_buff *skb, int attrtype, __le64 value,
> + int padattr)
> {
> - return nla_put(skb, attrtype, sizeof(__le64), &value);
> + return nla_put_64bit(skb, attrtype, sizeof(__le64), &value, padattr);
> }
>

But _why_ is it needed ?

nla_put() has no alignment assumptions, it simply copies 8 bytes.

Seems this is going too far.



2016-04-23 17:06:39

by Alexander Aring

[permalink] [raw]
Subject: Re: [PATCH net-next 2/9] libnl: nla_put_le64(): align on a 64-bit area


Hi,

On 04/22/2016 06:51 PM, Eric Dumazet wrote:
> On Fri, 2016-04-22 at 17:31 +0200, Nicolas Dichtel wrote:
>> nla_data() is now aligned on a 64-bit area.
>>
>> Signed-off-by: Nicolas Dichtel <[email protected]>
>> ---
>> include/net/netlink.h | 8 +++++---
>> include/net/nl802154.h | 6 ++++++
>> net/ieee802154/nl802154.c | 13 ++++++++-----
>> 3 files changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/include/net/netlink.h b/include/net/netlink.h
>> index 6f51a8a06498..7f6b99483ab7 100644
>> --- a/include/net/netlink.h
>> +++ b/include/net/netlink.h
>> @@ -878,14 +878,16 @@ static inline int nla_put_net64(struct sk_buff *skb, int attrtype, __be64 value)
>> }
>>
>> /**
>> - * nla_put_le64 - Add a __le64 netlink attribute to a socket buffer
>> + * nla_put_le64 - Add a __le64 netlink attribute to a socket buffer and align it
>> * @skb: socket buffer to add attribute to
>> * @attrtype: attribute type
>> * @value: numeric value
>> + * @padattr: attribute type for the padding
>> */
>> -static inline int nla_put_le64(struct sk_buff *skb, int attrtype, __le64 value)
>> +static inline int nla_put_le64(struct sk_buff *skb, int attrtype, __le64 value,
>> + int padattr)
>> {
>> - return nla_put(skb, attrtype, sizeof(__le64), &value);
>> + return nla_put_64bit(skb, attrtype, sizeof(__le64), &value, padattr);
>> }
>>
>
> But _why_ is it needed ?
>
> nla_put() has no alignment assumptions, it simply copies 8 bytes.
>
> Seems this is going too far.
>

if this is really needed, then nla_put_u64/be64/etc need to be changed also,
or? The function nla_put_u64 is the same like nla_put_le64 but with __le64
type, so sparse will not complain about that.

- Alex

2016-04-23 17:29:05

by Alexander Aring

[permalink] [raw]
Subject: Re: [PATCH net-next 2/9] libnl: nla_put_le64(): align on a 64-bit area


Hi,

On 04/23/2016 07:05 PM, Alexander Aring wrote:
...
>
> if this is really needed, then nla_put_u64/be64/etc need to be changed also,

Okay, I found PATCH 3/9 do it for be64, but what's about u64?

- Alex

2016-04-24 00:14:09

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next 0/9] netlink: align attributes when needed (patchset #1)

From: Nicolas Dichtel <[email protected]>
Date: Fri, 22 Apr 2016 17:31:15 +0200

> This is the continuation of the work done to align netlink attributes
> when these attributes contain some 64-bit fields.
>
> David, if the third patch is too big (or maybe the series), I can split it.
> Just tell me what you prefer.

This looks excellent, series applied, thanks!

2016-04-25 07:42:12

by Nicolas Dichtel

[permalink] [raw]
Subject: Re: [PATCH net-next 2/9] libnl: nla_put_le64(): align on a 64-bit area

Le 23/04/2016 19:28, Alexander Aring a écrit :
>
> Hi,
>
> On 04/23/2016 07:05 PM, Alexander Aring wrote:
> ...
>>
>> if this is really needed, then nla_put_u64/be64/etc need to be changed also,
>
> Okay, I found PATCH 3/9 do it for be64, but what's about u64?
It's also planned. I will send several "small" series to ease integration.


Regards,
Nicolas

2016-04-27 01:15:08

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH net-next 9/9] taskstats: use the libnl API to align nlattr on 64-bit



On 23/04/16 01:31, Nicolas Dichtel wrote:
> Goal of this patch is to use the new libnl API to align netlink attribute
> when needed.
> The layout of the netlink message will be a bit different after the patch,
> because the padattr (TASKSTATS_TYPE_STATS) will be inside the nested
> attribute instead of before it.
>
> Signed-off-by: Nicolas Dichtel <[email protected]>

The layout will change/break user space -- I've not tested the patch though..

Balbir Singh.


2016-04-27 07:29:28

by Nicolas Dichtel

[permalink] [raw]
Subject: Re: [PATCH net-next 9/9] taskstats: use the libnl API to align nlattr on 64-bit

Le 27/04/2016 03:14, Balbir Singh a ?crit :
>
>
> On 23/04/16 01:31, Nicolas Dichtel wrote:
>> Goal of this patch is to use the new libnl API to align netlink attribute
>> when needed.
>> The layout of the netlink message will be a bit different after the patch,
>> because the padattr (TASKSTATS_TYPE_STATS) will be inside the nested
>> attribute instead of before it.
>>
>> Signed-off-by: Nicolas Dichtel <[email protected]>
>
> The layout will change/break user space -- I've not tested the patch though..
Sigh.

I quote David:
"All userspace components using netlink should always ignore attributes
they do not recognize in dumps.

This is one of the most basic principles of netlink"

Do you have some pointers so I can made some tests?


Regards,
Nicolas

2016-04-27 12:29:38

by Balbir Singh

[permalink] [raw]
Subject: Re: [PATCH net-next 9/9] taskstats: use the libnl API to align nlattr on 64-bit



On 27/04/16 17:29, Nicolas Dichtel wrote:
> Le 27/04/2016 03:14, Balbir Singh a ?crit :
>>
>>
>> On 23/04/16 01:31, Nicolas Dichtel wrote:
>>> Goal of this patch is to use the new libnl API to align netlink attribute
>>> when needed.
>>> The layout of the netlink message will be a bit different after the patch,
>>> because the padattr (TASKSTATS_TYPE_STATS) will be inside the nested
>>> attribute instead of before it.
>>>
>>> Signed-off-by: Nicolas Dichtel <[email protected]>
>>
>> The layout will change/break user space -- I've not tested the patch though..
> Sigh.
>
> I quote David:
> "All userspace components using netlink should always ignore attributes
> they do not recognize in dumps.
>
> This is one of the most basic principles of netlink"
>
> Do you have some pointers so I can made some tests?
>

Please try

https://www.kernel.org/doc/Documentation/accounting/getdelays.c

iotop uses it as well. My concern is ABI breakage of user space.

Balbir Singh

2016-04-27 15:46:51

by Nicolas Dichtel

[permalink] [raw]
Subject: Re: [PATCH net-next 9/9] taskstats: use the libnl API to align nlattr on 64-bit

Le 27/04/2016 14:29, Balbir Singh a ?crit :
[snip]
> Please try
>
> https://www.kernel.org/doc/Documentation/accounting/getdelays.c
A patch follows this mail to fix that.

>
> iotop uses it as well. My concern is ABI breakage of user space.
My test is ok here, I didn't see a problem.
Code review (from here http://repo.or.cz/w/iotop.git) is also ok.


Regards,
Nicolas

2016-04-27 15:48:40

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next] taskstats: fix nl parsing in accounting/getdelays.c

The type TASKSTATS_TYPE_NULL should always be ignored.

When jumping to the next attribute, only the length of the current
attribute should be added, not the length of all nested attributes.
This last bug was not visible before commit 80df554275c2, because the
kernel didn't put more than two nested attributes.

Fixes: a3baf649ca9c ("[PATCH] per-task-delay-accounting: documentation")
Fixes: 80df554275c2 ("taskstats: use the libnl API to align nlattr on 64-bit")
Signed-off-by: Nicolas Dichtel <[email protected]>
---
Documentation/accounting/getdelays.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/accounting/getdelays.c b/Documentation/accounting/getdelays.c
index 7785fb5eb93f..d3caa6748a46 100644
--- a/Documentation/accounting/getdelays.c
+++ b/Documentation/accounting/getdelays.c
@@ -512,7 +512,7 @@ int main(int argc, char *argv[])
break;
}
len2 += NLA_ALIGN(na->nla_len);
- na = (struct nlattr *) ((char *) na + len2);
+ na = (struct nlattr *) ((char *) na + NLA_ALIGN(na->nla_len));
}
break;

--
2.8.1

2016-04-27 15:50:11

by Nicolas Dichtel

[permalink] [raw]
Subject: Re: [PATCH net-next] taskstats: fix nl parsing in accounting/getdelays.c

Le 27/04/2016 17:47, Nicolas Dichtel a écrit :
> The type TASKSTATS_TYPE_NULL should always be ignored.
>
> When jumping to the next attribute, only the length of the current
> attribute should be added, not the length of all nested attributes.
> This last bug was not visible before commit 80df554275c2, because the
> kernel didn't put more than two nested attributes.
>
> Fixes: a3baf649ca9c ("[PATCH] per-task-delay-accounting: documentation")
> Fixes: 80df554275c2 ("taskstats: use the libnl API to align nlattr on 64-bit")
> Signed-off-by: Nicolas Dichtel <[email protected]>
Please, drop this version. I fatfingered my rebase.

2016-04-27 15:53:45

by Nicolas Dichtel

[permalink] [raw]
Subject: [PATCH net-next v2] taskstats: fix nl parsing in accounting/getdelays.c

The type TASKSTATS_TYPE_NULL should always be ignored.

When jumping to the next attribute, only the length of the current
attribute should be added, not the length of all nested attributes.
This last bug was not visible before commit 80df554275c2, because the
kernel didn't put more than two nested attributes.

Fixes: a3baf649ca9c ("[PATCH] per-task-delay-accounting: documentation")
Fixes: 80df554275c2 ("taskstats: use the libnl API to align nlattr on 64-bit")
Signed-off-by: Nicolas Dichtel <[email protected]>
---
Documentation/accounting/getdelays.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Documentation/accounting/getdelays.c b/Documentation/accounting/getdelays.c
index 7785fb5eb93f..b5ca536e56a8 100644
--- a/Documentation/accounting/getdelays.c
+++ b/Documentation/accounting/getdelays.c
@@ -505,6 +505,8 @@ int main(int argc, char *argv[])
if (!loop)
goto done;
break;
+ case TASKSTATS_TYPE_NULL:
+ break;
default:
fprintf(stderr, "Unknown nested"
" nla_type %d\n",
@@ -512,7 +514,8 @@ int main(int argc, char *argv[])
break;
}
len2 += NLA_ALIGN(na->nla_len);
- na = (struct nlattr *) ((char *) na + len2);
+ na = (struct nlattr *)((char *)na +
+ NLA_ALIGN(na->nla_len));
}
break;

--
2.8.1

2016-04-27 16:41:20

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next 9/9] taskstats: use the libnl API to align nlattr on 64-bit

From: Balbir Singh <[email protected]>
Date: Wed, 27 Apr 2016 22:29:22 +1000

> My concern is ABI breakage of user space.

The "ABI" is that unrecognized attributes must be silently ignored by
userspace.

2016-04-27 16:56:46

by David Miller

[permalink] [raw]
Subject: Re: [PATCH net-next v2] taskstats: fix nl parsing in accounting/getdelays.c

From: Nicolas Dichtel <[email protected]>
Date: Wed, 27 Apr 2016 17:53:08 +0200

> The type TASKSTATS_TYPE_NULL should always be ignored.
>
> When jumping to the next attribute, only the length of the current
> attribute should be added, not the length of all nested attributes.
> This last bug was not visible before commit 80df554275c2, because the
> kernel didn't put more than two nested attributes.
>
> Fixes: a3baf649ca9c ("[PATCH] per-task-delay-accounting: documentation")
> Fixes: 80df554275c2 ("taskstats: use the libnl API to align nlattr on 64-bit")
> Signed-off-by: Nicolas Dichtel <[email protected]>

Applied, thanks.