Skip to content

Commit 374d345

Browse files
kuba-moodavem330
authored andcommitted
netlink: add variable-length / auto integers
We currently push everyone to use padding to align 64b values in netlink. Un-padded nla_put_u64() doesn't even exist any more. The story behind this possibly start with this thread: https://lore.kernel.org/netdev/20121204.130914.1457976839967676240.davem@davemloft.net/ where DaveM was concerned about the alignment of a structure containing 64b stats. If user space tries to access such struct directly: struct some_stats *stats = nla_data(attr); printf("A: %llu", stats->a); lack of alignment may become problematic for some architectures. These days we most often put every single member in a separate attribute, meaning that the code above would use a helper like nla_get_u64(), which can deal with alignment internally. Even for arches which don't have good unaligned access - access aligned to 4B should be pretty efficient. Kernel and well known libraries deal with unaligned input already. Padded 64b is quite space-inefficient (64b + pad means at worst 16B per attr vs 32b which takes 8B). It is also more typing: if (nla_put_u64_pad(rsp, NETDEV_A_SOMETHING_SOMETHING, value, NETDEV_A_SOMETHING_PAD)) Create a new attribute type which will use 32 bits at netlink level if value is small enough (probably most of the time?), and (4B-aligned) 64 bits otherwise. Kernel API is just: if (nla_put_uint(rsp, NETDEV_A_SOMETHING_SOMETHING, value)) Calling this new type "just" sint / uint with no specific size will hopefully also make people more comfortable with using it. Currently telling people "don't use u8, you may need the bits, and netlink will round up to 4B, anyway" is the #1 comment we give to newcomers. In terms of netlink layout it looks like this: 0 4 8 12 16 32b: [nlattr][ u32 ] 64b: [ pad ][nlattr][ u64 ] uint(32) [nlattr][ u32 ] uint(64) [nlattr][ u64 ] Signed-off-by: Jakub Kicinski <kuba@kernel.org> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent e1b347c commit 374d345

File tree

5 files changed

+121
-7
lines changed

5 files changed

+121
-7
lines changed

Documentation/userspace-api/netlink/specs.rst

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -403,10 +403,21 @@ This section describes the attribute types supported by the ``genetlink``
403403
compatibility level. Refer to documentation of different levels for additional
404404
attribute types.
405405

406-
Scalar integer types
406+
Common integer types
407407
--------------------
408408

409-
Fixed-width integer types:
409+
``sint`` and ``uint`` represent signed and unsigned 64 bit integers.
410+
If the value can fit on 32 bits only 32 bits are carried in netlink
411+
messages, otherwise full 64 bits are carried. Note that the payload
412+
is only aligned to 4B, so the full 64 bit value may be unaligned!
413+
414+
Common integer types should be preferred over fix-width types in majority
415+
of cases.
416+
417+
Fix-width integer types
418+
-----------------------
419+
420+
Fixed-width integer types include:
410421
``u8``, ``u16``, ``u32``, ``u64``, ``s8``, ``s16``, ``s32``, ``s64``.
411422

412423
Note that types smaller than 32 bit should be avoided as using them
@@ -416,6 +427,9 @@ See :ref:`pad_type` for padding of 64 bit attributes.
416427
The payload of the attribute is the integer in host order unless ``byte-order``
417428
specifies otherwise.
418429

430+
64 bit values are usually aligned by the kernel but it is recommended
431+
that the user space is able to deal with unaligned values.
432+
419433
.. _pad_type:
420434

421435
pad

include/net/netlink.h

Lines changed: 67 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,8 @@
128128
* nla_len(nla) length of attribute payload
129129
*
130130
* Attribute Payload Access for Basic Types:
131+
* nla_get_uint(nla) get payload for a uint attribute
132+
* nla_get_sint(nla) get payload for a sint attribute
131133
* nla_get_u8(nla) get payload for a u8 attribute
132134
* nla_get_u16(nla) get payload for a u16 attribute
133135
* nla_get_u32(nla) get payload for a u32 attribute
@@ -183,6 +185,8 @@ enum {
183185
NLA_REJECT,
184186
NLA_BE16,
185187
NLA_BE32,
188+
NLA_SINT,
189+
NLA_UINT,
186190
__NLA_TYPE_MAX,
187191
};
188192

@@ -229,6 +233,7 @@ enum nla_policy_validation {
229233
* nested header (or empty); len field is used if
230234
* nested_policy is also used, for the max attr
231235
* number in the nested policy.
236+
* NLA_SINT, NLA_UINT,
232237
* NLA_U8, NLA_U16,
233238
* NLA_U32, NLA_U64,
234239
* NLA_S8, NLA_S16,
@@ -260,12 +265,14 @@ enum nla_policy_validation {
260265
* while an array has the nested attributes at another
261266
* level down and the attribute types directly in the
262267
* nesting don't matter.
268+
* NLA_UINT,
263269
* NLA_U8,
264270
* NLA_U16,
265271
* NLA_U32,
266272
* NLA_U64,
267273
* NLA_BE16,
268274
* NLA_BE32,
275+
* NLA_SINT,
269276
* NLA_S8,
270277
* NLA_S16,
271278
* NLA_S32,
@@ -280,6 +287,7 @@ enum nla_policy_validation {
280287
* or NLA_POLICY_FULL_RANGE_SIGNED() macros instead.
281288
* Use the NLA_POLICY_MIN(), NLA_POLICY_MAX() and
282289
* NLA_POLICY_RANGE() macros.
290+
* NLA_UINT,
283291
* NLA_U8,
284292
* NLA_U16,
285293
* NLA_U32,
@@ -288,6 +296,7 @@ enum nla_policy_validation {
288296
* to a struct netlink_range_validation that indicates
289297
* the min/max values.
290298
* Use NLA_POLICY_FULL_RANGE().
299+
* NLA_SINT,
291300
* NLA_S8,
292301
* NLA_S16,
293302
* NLA_S32,
@@ -377,9 +386,11 @@ struct nla_policy {
377386

378387
#define __NLA_IS_UINT_TYPE(tp) \
379388
(tp == NLA_U8 || tp == NLA_U16 || tp == NLA_U32 || \
380-
tp == NLA_U64 || tp == NLA_BE16 || tp == NLA_BE32)
389+
tp == NLA_U64 || tp == NLA_UINT || \
390+
tp == NLA_BE16 || tp == NLA_BE32)
381391
#define __NLA_IS_SINT_TYPE(tp) \
382-
(tp == NLA_S8 || tp == NLA_S16 || tp == NLA_S32 || tp == NLA_S64)
392+
(tp == NLA_S8 || tp == NLA_S16 || tp == NLA_S32 || tp == NLA_S64 || \
393+
tp == NLA_SINT)
383394

384395
#define __NLA_ENSURE(condition) BUILD_BUG_ON_ZERO(!(condition))
385396
#define NLA_ENSURE_UINT_TYPE(tp) \
@@ -1357,6 +1368,22 @@ static inline int nla_put_u32(struct sk_buff *skb, int attrtype, u32 value)
13571368
return nla_put(skb, attrtype, sizeof(u32), &tmp);
13581369
}
13591370

1371+
/**
1372+
* nla_put_uint - Add a variable-size unsigned int to a socket buffer
1373+
* @skb: socket buffer to add attribute to
1374+
* @attrtype: attribute type
1375+
* @value: numeric value
1376+
*/
1377+
static inline int nla_put_uint(struct sk_buff *skb, int attrtype, u64 value)
1378+
{
1379+
u64 tmp64 = value;
1380+
u32 tmp32 = value;
1381+
1382+
if (tmp64 == tmp32)
1383+
return nla_put_u32(skb, attrtype, tmp32);
1384+
return nla_put(skb, attrtype, sizeof(u64), &tmp64);
1385+
}
1386+
13601387
/**
13611388
* nla_put_be32 - Add a __be32 netlink attribute to a socket buffer
13621389
* @skb: socket buffer to add attribute to
@@ -1511,6 +1538,22 @@ static inline int nla_put_s64(struct sk_buff *skb, int attrtype, s64 value,
15111538
return nla_put_64bit(skb, attrtype, sizeof(s64), &tmp, padattr);
15121539
}
15131540

1541+
/**
1542+
* nla_put_sint - Add a variable-size signed int to a socket buffer
1543+
* @skb: socket buffer to add attribute to
1544+
* @attrtype: attribute type
1545+
* @value: numeric value
1546+
*/
1547+
static inline int nla_put_sint(struct sk_buff *skb, int attrtype, s64 value)
1548+
{
1549+
s64 tmp64 = value;
1550+
s32 tmp32 = value;
1551+
1552+
if (tmp64 == tmp32)
1553+
return nla_put_s32(skb, attrtype, tmp32);
1554+
return nla_put(skb, attrtype, sizeof(s64), &tmp64);
1555+
}
1556+
15141557
/**
15151558
* nla_put_string - Add a string netlink attribute to a socket buffer
15161559
* @skb: socket buffer to add attribute to
@@ -1667,6 +1710,17 @@ static inline u64 nla_get_u64(const struct nlattr *nla)
16671710
return tmp;
16681711
}
16691712

1713+
/**
1714+
* nla_get_uint - return payload of uint attribute
1715+
* @nla: uint netlink attribute
1716+
*/
1717+
static inline u64 nla_get_uint(const struct nlattr *nla)
1718+
{
1719+
if (nla_len(nla) == sizeof(u32))
1720+
return nla_get_u32(nla);
1721+
return nla_get_u64(nla);
1722+
}
1723+
16701724
/**
16711725
* nla_get_be64 - return payload of __be64 attribute
16721726
* @nla: __be64 netlink attribute
@@ -1729,6 +1783,17 @@ static inline s64 nla_get_s64(const struct nlattr *nla)
17291783
return tmp;
17301784
}
17311785

1786+
/**
1787+
* nla_get_sint - return payload of uint attribute
1788+
* @nla: uint netlink attribute
1789+
*/
1790+
static inline s64 nla_get_sint(const struct nlattr *nla)
1791+
{
1792+
if (nla_len(nla) == sizeof(s32))
1793+
return nla_get_s32(nla);
1794+
return nla_get_s64(nla);
1795+
}
1796+
17321797
/**
17331798
* nla_get_flag - return payload of flag attribute
17341799
* @nla: flag netlink attribute

include/uapi/linux/netlink.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,6 +298,8 @@ struct nla_bitfield32 {
298298
* entry has attributes again, the policy for those inner ones
299299
* and the corresponding maxtype may be specified.
300300
* @NL_ATTR_TYPE_BITFIELD32: &struct nla_bitfield32 attribute
301+
* @NL_ATTR_TYPE_SINT: 32-bit or 64-bit signed attribute, aligned to 4B
302+
* @NL_ATTR_TYPE_UINT: 32-bit or 64-bit unsigned attribute, aligned to 4B
301303
*/
302304
enum netlink_attribute_type {
303305
NL_ATTR_TYPE_INVALID,
@@ -322,6 +324,9 @@ enum netlink_attribute_type {
322324
NL_ATTR_TYPE_NESTED_ARRAY,
323325

324326
NL_ATTR_TYPE_BITFIELD32,
327+
328+
NL_ATTR_TYPE_SINT,
329+
NL_ATTR_TYPE_UINT,
325330
};
326331

327332
/**

lib/nlattr.c

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ void nla_get_range_unsigned(const struct nla_policy *pt,
134134
range->max = U32_MAX;
135135
break;
136136
case NLA_U64:
137+
case NLA_UINT:
137138
case NLA_MSECS:
138139
range->max = U64_MAX;
139140
break;
@@ -183,6 +184,9 @@ static int nla_validate_range_unsigned(const struct nla_policy *pt,
183184
case NLA_U64:
184185
value = nla_get_u64(nla);
185186
break;
187+
case NLA_UINT:
188+
value = nla_get_uint(nla);
189+
break;
186190
case NLA_MSECS:
187191
value = nla_get_u64(nla);
188192
break;
@@ -248,6 +252,7 @@ void nla_get_range_signed(const struct nla_policy *pt,
248252
range->max = S32_MAX;
249253
break;
250254
case NLA_S64:
255+
case NLA_SINT:
251256
range->min = S64_MIN;
252257
range->max = S64_MAX;
253258
break;
@@ -295,6 +300,9 @@ static int nla_validate_int_range_signed(const struct nla_policy *pt,
295300
case NLA_S64:
296301
value = nla_get_s64(nla);
297302
break;
303+
case NLA_SINT:
304+
value = nla_get_sint(nla);
305+
break;
298306
default:
299307
return -EINVAL;
300308
}
@@ -320,6 +328,7 @@ static int nla_validate_int_range(const struct nla_policy *pt,
320328
case NLA_U16:
321329
case NLA_U32:
322330
case NLA_U64:
331+
case NLA_UINT:
323332
case NLA_MSECS:
324333
case NLA_BINARY:
325334
case NLA_BE16:
@@ -329,6 +338,7 @@ static int nla_validate_int_range(const struct nla_policy *pt,
329338
case NLA_S16:
330339
case NLA_S32:
331340
case NLA_S64:
341+
case NLA_SINT:
332342
return nla_validate_int_range_signed(pt, nla, extack);
333343
default:
334344
WARN_ON(1);
@@ -355,6 +365,9 @@ static int nla_validate_mask(const struct nla_policy *pt,
355365
case NLA_U64:
356366
value = nla_get_u64(nla);
357367
break;
368+
case NLA_UINT:
369+
value = nla_get_uint(nla);
370+
break;
358371
case NLA_BE16:
359372
value = ntohs(nla_get_be16(nla));
360373
break;
@@ -433,6 +446,15 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
433446
goto out_err;
434447
break;
435448

449+
case NLA_SINT:
450+
case NLA_UINT:
451+
if (attrlen != sizeof(u32) && attrlen != sizeof(u64)) {
452+
NL_SET_ERR_MSG_ATTR_POL(extack, nla, pt,
453+
"invalid attribute length");
454+
return -EINVAL;
455+
}
456+
break;
457+
436458
case NLA_BITFIELD32:
437459
if (attrlen != sizeof(struct nla_bitfield32))
438460
goto out_err;

net/netlink/policy.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,8 @@ int netlink_policy_dump_attr_size_estimate(const struct nla_policy *pt)
230230
case NLA_S16:
231231
case NLA_S32:
232232
case NLA_S64:
233+
case NLA_SINT:
234+
case NLA_UINT:
233235
/* maximum is common, u64 min/max with padding */
234236
return common +
235237
2 * (nla_attr_size(0) + nla_attr_size(sizeof(u64)));
@@ -288,6 +290,7 @@ __netlink_policy_dump_write_attr(struct netlink_policy_dump_state *state,
288290
case NLA_U16:
289291
case NLA_U32:
290292
case NLA_U64:
293+
case NLA_UINT:
291294
case NLA_MSECS: {
292295
struct netlink_range_validation range;
293296

@@ -297,8 +300,10 @@ __netlink_policy_dump_write_attr(struct netlink_policy_dump_state *state,
297300
type = NL_ATTR_TYPE_U16;
298301
else if (pt->type == NLA_U32)
299302
type = NL_ATTR_TYPE_U32;
300-
else
303+
else if (pt->type == NLA_U64)
301304
type = NL_ATTR_TYPE_U64;
305+
else
306+
type = NL_ATTR_TYPE_UINT;
302307

303308
if (pt->validation_type == NLA_VALIDATE_MASK) {
304309
if (nla_put_u64_64bit(skb, NL_POLICY_TYPE_ATTR_MASK,
@@ -320,7 +325,8 @@ __netlink_policy_dump_write_attr(struct netlink_policy_dump_state *state,
320325
case NLA_S8:
321326
case NLA_S16:
322327
case NLA_S32:
323-
case NLA_S64: {
328+
case NLA_S64:
329+
case NLA_SINT: {
324330
struct netlink_range_validation_signed range;
325331

326332
if (pt->type == NLA_S8)
@@ -329,8 +335,10 @@ __netlink_policy_dump_write_attr(struct netlink_policy_dump_state *state,
329335
type = NL_ATTR_TYPE_S16;
330336
else if (pt->type == NLA_S32)
331337
type = NL_ATTR_TYPE_S32;
332-
else
338+
else if (pt->type == NLA_S64)
333339
type = NL_ATTR_TYPE_S64;
340+
else
341+
type = NL_ATTR_TYPE_SINT;
334342

335343
nla_get_range_signed(pt, &range);
336344

0 commit comments

Comments
 (0)