diff options
author | Jakub Kicinski <kuba@kernel.org> | 2023-10-19 00:39:20 +0300 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2023-10-20 13:43:35 +0300 |
commit | 374d345d9b5e13380c66d7042f9533a6ac6d1195 (patch) | |
tree | 547bfdcc3e1a56bf2bc7a17999857b66cdbe6794 /net/netlink | |
parent | e1b347c5f7de2c1a112c2dd6e380241135e34be2 (diff) | |
download | linux-374d345d9b5e13380c66d7042f9533a6ac6d1195.tar.xz |
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>
Diffstat (limited to 'net/netlink')
-rw-r--r-- | net/netlink/policy.c | 14 |
1 files changed, 11 insertions, 3 deletions
diff --git a/net/netlink/policy.c b/net/netlink/policy.c index e2f111edf66c..1f8909c16f14 100644 --- a/net/netlink/policy.c +++ b/net/netlink/policy.c @@ -230,6 +230,8 @@ int netlink_policy_dump_attr_size_estimate(const struct nla_policy *pt) case NLA_S16: case NLA_S32: case NLA_S64: + case NLA_SINT: + case NLA_UINT: /* maximum is common, u64 min/max with padding */ return common + 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, case NLA_U16: case NLA_U32: case NLA_U64: + case NLA_UINT: case NLA_MSECS: { struct netlink_range_validation range; @@ -297,8 +300,10 @@ __netlink_policy_dump_write_attr(struct netlink_policy_dump_state *state, type = NL_ATTR_TYPE_U16; else if (pt->type == NLA_U32) type = NL_ATTR_TYPE_U32; - else + else if (pt->type == NLA_U64) type = NL_ATTR_TYPE_U64; + else + type = NL_ATTR_TYPE_UINT; if (pt->validation_type == NLA_VALIDATE_MASK) { 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, case NLA_S8: case NLA_S16: case NLA_S32: - case NLA_S64: { + case NLA_S64: + case NLA_SINT: { struct netlink_range_validation_signed range; if (pt->type == NLA_S8) @@ -329,8 +335,10 @@ __netlink_policy_dump_write_attr(struct netlink_policy_dump_state *state, type = NL_ATTR_TYPE_S16; else if (pt->type == NLA_S32) type = NL_ATTR_TYPE_S32; - else + else if (pt->type == NLA_S64) type = NL_ATTR_TYPE_S64; + else + type = NL_ATTR_TYPE_SINT; nla_get_range_signed(pt, &range); |