Re: [Qemu-devel] [PATCH v10 1/3] linux-user: Add support for setsockopt() options IPV6_<ADD|DROP>_MEMBERSHIP

[Laurent Vivier]

On 07/06/2019 12:35, Aleksandar Markovic wrote:
> From: Neng Chen <address@hidden>
> 
> Add support for options IPV6_ADD_MEMBERSHIP and IPV6_DROP_MEMPEMBERSHIP
> of the syscall setsockopt(). These options control membership in
> multicast groups. Their argument is a pointer to a struct ipv6_mreq,
> which is in turn defined in IP v6 header netinet/in.h as:
> 
>  struct ipv6_mreq {
>      /* IPv6 multicast address of group */
>      struct  in6_addr  ipv6mr_multiaddr;
>      /* local IPv6 address of interface */
>      int     ipv6mr_interface;
>  };
> 
> ...whereas its definition in kernel's include/uapi/linux/in6.h is:
> 
>  #if __UAPI_DEF_IPV6_MREQ
>  struct ipv6_mreq {
>      /* IPv6 multicast address of group */
>          struct  in6_addr ipv6mr_multiaddr;
>      /* local IPv6 address of interface */
>      int     ipv6mr_ifindex;
>  };
>  #endif
> 
> The first field of ipv6_mreq has the same name ("ipv6mr_multiaddr")
> and type ("in6_addr") in both cases. Moreover, the in6_addr structure
> consists of fields that are always big-endian (on host of any endian),
> therefore the ipv6_mreq's field ipv6mr_multiaddr doesn't need any
> endian conversion.
> 
> The second field of ipv6_mreq may, however, depending on the build
> environment, have different names. This is the reason why the lines
> "#if __UAPI_DEF_IPV6_MREQ" and "#if defined(__UAPI_DEF_IPV6_MREQ)"
> are used in this patch - to establish the right choice for the field
> name. Also, endian conversion is needed for this field, since it is
> of type "int".
> 
> Signed-off-by: Neng Chen <address@hidden>
> Signed-off-by: Aleksandar Markovic <address@hidden>
> ---
>  linux-user/syscall.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 5e29e67..dde6889 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -1921,6 +1921,33 @@ static abi_long do_setsockopt(int sockfd, int level, 
> int optname,
>                                         &pki, sizeof(pki)));
>              break;
>          }
> +        case IPV6_ADD_MEMBERSHIP:
> +        case IPV6_DROP_MEMBERSHIP:
> +        {
> +            struct ipv6_mreq ipv6mreq;
> +
> +            if (optlen < sizeof(ipv6mreq)) {
> +                return -TARGET_EINVAL;
> +            }
> +
> +            if (copy_from_user(&ipv6mreq, optval_addr, sizeof(ipv6mreq))) {
> +                return -TARGET_EFAULT;
> +            }
> +
> +#if defined(__UAPI_DEF_IPV6_MREQ)
> +#if __UAPI_DEF_IPV6_MREQ
> +            ipv6mreq.ipv6mr_ifindex = tswap32(ipv6mreq.ipv6mr_ifindex);
> +#else
> +            ipv6mreq.ipv6mr_interface = tswap32(ipv6mreq.ipv6mr_interface);
> +#endif /* __UAPI_DEF_IVP6_MREQ */
> +#else
> +            ipv6mreq.ipv6mr_interface = tswap32(ipv6mreq.ipv6mr_interface);
> +#endif /* defined (__UAPI_DEF_IPV6_MREQ) */
> +
> +            ret = get_errno(setsockopt(sockfd, level, optname,
> +                                       &ipv6mreq, sizeof(ipv6mreq)));
> +            break;
> +        }
>          default:
>              goto unimplemented;
>          }
> 

It becomes complicated...

I think the first version of your patch using only ipv6mr_interface is
the correct one:

- POSIX defines ipv6mr_interface [1]

- __UAPI_DEF_IVP6_MREQ appears in kernel headers with v3.12

  cfd280c91253 net: sync some IP headers with glibc

- without __UAPI_DEF_IVP6_MREQ kernel defines ipv6mr_ifindex

and in cfd280c91253 it is explained:

   "If you include the kernel headers first you get those,
    and if you include the glibc headers first you get those,
    and the following patch arranges a coordination and
    synchronization between the two."

So before 3.12, a program can't include both netinet/in.h and linux/in6.h.

In linux-user/syscall.c, we only include netinet/in.h (glibc) and not
linux/in6.h (kernel-headers), so ipv6mr_interface is the one to use.

I found debian/wheezy (2013) is the most recent debian distro without
the definition of __UAPI_DEF_IPV6_MREQ, but it doesn't have gcc-4.8 and
thus QEMU can't be built.

Thanks,
Laurent

[1]
http://pubs.opengroup.org/onlinepubs/009695399/basedefs/netinet/in.h.html