Re: [PATCH v2 4/4] linux-user: handle netlink flag NLA_F_NESTED

[Mathis MARION]

On 20/02/2023 09:58, Mathis Marion wrote:
> From: Mathis Marion <mathis.marion@silabs.com>
>
> Newer kernel versions require this flag to be present contrary to older
> ones. Depending on the libnl version it is added or not.
>
> Typically when using rtnl_link_inet6_set_addr_gen_mode, the netlink
> packet generated may contain the following attribute:
>
> with libnl 3.4
>
>    {nla_len=16, nla_type=IFLA_AF_SPEC},
>    [
>      {nla_len=12, nla_type=AF_INET6},
>      [{nla_len=5, nla_type=IFLA_INET6_ADDR_GEN_MODE}, IN6_ADDR_GEN_MODE_NONE]
>    ]
>
> with libnl 3.7
>
>    {nla_len=16, nla_type=NLA_F_NESTED|IFLA_AF_SPEC},
>    [
>      {nla_len=12, nla_type=NLA_F_NESTED|AF_INET6},
>      [{nla_len=5, nla_type=IFLA_INET6_ADDR_GEN_MODE}, IN6_ADDR_GEN_MODE_NONE]]
>    ]
> > Signed-off-by: Mathis Marion <mathis.marion@silabs.com>
> ---
>   linux-user/fd-trans.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/fd-trans.c b/linux-user/fd-trans.c
> index 4852a75d9d..1fb8ef9d4e 100644
> --- a/linux-user/fd-trans.c
> +++ b/linux-user/fd-trans.c
> @@ -1359,7 +1359,7 @@ static abi_long target_to_host_for_each_rtattr(struct 
> rtattr *rtattr,
>   
>   static abi_long target_to_host_data_spec_nlattr(struct nlattr *nlattr)
>   {
> -    switch (nlattr->nla_type) {
> +    switch (nlattr->nla_type & ~NLA_F_NESTED) {

After looking a bit more into it, linux/netlink.h defines:

/*
 * nla_type (16 bits)
 * +---+---+-------------------------------+
 * | N | O | Attribute Type                |
 * +---+---+-------------------------------+
 * N := Carries nested attributes
 * O := Payload stored in network byte order
 *
 * Note: The N and O flag are mutually exclusive.
 */
#define NLA_F_NESTED            (1 << 15)
#define NLA_F_NET_BYTEORDER     (1 << 14)
#define NLA_TYPE_MASK           ~(NLA_F_NESTED | NLA_F_NET_BYTEORDER)

So maybe I should use NLA_TYPE_MASK.

However since NLA_F_NET_BYTEORDER is not supported it may make more
sense to return an error.

>       case AF_INET6:
>           return target_to_host_for_each_nlattr(NLA_DATA(nlattr), 
> nlattr->nla_len,
>                                                 
> target_to_host_data_inet6_nlattr);
> @@ -1375,7 +1375,7 @@ static abi_long target_to_host_data_link_rtattr(struct 
> rtattr *rtattr)
>   {
>       uint32_t *u32;
>   
> -    switch (rtattr->rta_type) {
> +    switch (rtattr->rta_type & ~NLA_F_NESTED) {
>       /* uint32_t */
>       case QEMU_IFLA_MTU:
>       case QEMU_IFLA_TXQLEN: