2025-03-04, 01:33:50 +0100, Antonio Quartulli wrote:
> int ovpn_nl_key_new_doit(struct sk_buff *skb, struct genl_info *info)
> {
...
> + pkr.slot = nla_get_u8(attrs[OVPN_A_KEYCONF_SLOT]);
> + pkr.key.key_id = nla_get_u16(attrs[OVPN_A_KEYCONF_KEY_ID]);
> + pkr.key.cipher_alg = nla_get_u16(attrs[OVPN_A_KEYCONF_CIPHER_ALG]);
[...]
> +static int ovpn_nl_send_key(struct sk_buff *skb, const struct genl_info *info,
> + u32 peer_id, enum ovpn_key_slot slot,
> + const struct ovpn_key_config *keyconf)
> +{
...
> + if (nla_put_u32(skb, OVPN_A_KEYCONF_SLOT, slot) ||
> + nla_put_u32(skb, OVPN_A_KEYCONF_KEY_ID, keyconf->key_id) ||
> + nla_put_u32(skb, OVPN_A_KEYCONF_CIPHER_ALG, keyconf->cipher_alg))
That's a bit inconsistent. nla_put_u32 matches the generated policy,
but the nla_get_u{8,16} don't (and nla_get_u16 also doesn't match "u8
key_id" it's getting stored into).
[also kind of curious that the policy/spec uses U32 with max values of 1/2/7]
> int ovpn_nl_key_get_doit(struct sk_buff *skb, struct genl_info *info)
> {
...
> + slot = nla_get_u32(attrs[OVPN_A_KEYCONF_SLOT]);
> int ovpn_nl_key_del_doit(struct sk_buff *skb, struct genl_info *info)
> {
...
> + slot = nla_get_u8(attrs[OVPN_A_KEYCONF_SLOT]);
A few more inconsistencies:
* OVPN_A_IFNAME is defined in the uapi but never used (I guess
leftover from genl link creation)
* OVPN_A_PEER_DEL_REASON (u32 vs u8)
drivers/net/ovpn/netlink-gen.c:52: [OVPN_A_PEER_DEL_REASON] = NLA_POLICY_MAX(NLA_U32, 4),
drivers/net/ovpn/netlink.c:1131: if (nla_put_u8(msg, OVPN_A_PEER_DEL_REASON, peer->delete_reason))
* OVPN_A_PEER_LINK_TX_PACKETS/OVPN_A_PEER_LINK_RX_PACKETS (uint vs u32):
drivers/net/ovpn/netlink-gen.c-57- [OVPN_A_PEER_LINK_RX_BYTES] = { .type = NLA_UINT, },
drivers/net/ovpn/netlink-gen.c-58- [OVPN_A_PEER_LINK_TX_BYTES] = { .type = NLA_UINT, },
drivers/net/ovpn/netlink-gen.c-59- [OVPN_A_PEER_LINK_RX_PACKETS] = { .type = NLA_U32, },
drivers/net/ovpn/netlink-gen.c:60: [OVPN_A_PEER_LINK_TX_PACKETS] = { .type = NLA_U32, },
--
drivers/net/ovpn/netlink.c-618- /* link RX stats */
drivers/net/ovpn/netlink.c-619- nla_put_uint(skb, OVPN_A_PEER_LINK_RX_BYTES,
drivers/net/ovpn/netlink.c-620- atomic64_read(&peer->link_stats.rx.bytes)) ||
drivers/net/ovpn/netlink.c-621- nla_put_uint(skb, OVPN_A_PEER_LINK_RX_PACKETS,
drivers/net/ovpn/netlink.c-622- atomic64_read(&peer->link_stats.rx.packets)) ||
drivers/net/ovpn/netlink.c-623- /* link TX stats */
drivers/net/ovpn/netlink.c-624- nla_put_uint(skb, OVPN_A_PEER_LINK_TX_BYTES,
drivers/net/ovpn/netlink.c-625- atomic64_read(&peer->link_stats.tx.bytes)) ||
drivers/net/ovpn/netlink.c:626: nla_put_uint(skb, OVPN_A_PEER_LINK_TX_PACKETS,
drivers/net/ovpn/netlink.c-627- atomic64_read(&peer->link_stats.tx.packets)))
I guess all the stats should be UINT.
--
Sabrina
