diff mbox series

[FFmpeg-devel] avformat: enable UDP IPv6 multicast interface selection

Message ID AS8P193MB199731D2B920E976CEF231CE8D292@AS8P193MB1997.EURP193.PROD.OUTLOOK.COM
State New
Headers show
Series [FFmpeg-devel] avformat: enable UDP IPv6 multicast interface selection | expand

Checks

Context Check Description
yinshiyou/make_loongarch64 success Make finished
yinshiyou/make_fate_loongarch64 success Make fate finished
andriy/make_x86 success Make finished
andriy/make_fate_x86 success Make fate finished

Commit Message

Ignjatović, Lazar (RS) March 14, 2024, 12:04 p.m. UTC
localaddr option now properly works with IPv6 addresses. Properly
resolved interface index in places where default 0 interface index is
used (marked with TODO: within udp.c). Added SO_BINDTODEVICE for mcast
sockets that are used for reading from the network. Need for this
arises from the fact that [ffx1::*] and [ffx2::*] mcast addresses need
to have a defined interface for binding to avoid ambiguity between
multiple link-local networks on the same host. Failing to set this
option causes errors on Linux systems for interface and link-local
scopes.

For mcast addresses, bind to mcast address is attempted as before.
In case that this fails, which will happen on Windows, socket is bound
to INADDR_ANY/IN6ADDR_ANY_INIT depending on address family. Actual
interface selection is performed using udp_set_multicast_interface to
point to the desired interface for sending/listening.

Utilization of sin6_scope_id field enables usage and adequate resolving
of IPv6 addresses that utilize zone index
(e.g. fe80::1ff:fe23:4567:890a%eth2)
This is not fully supported on Windows, thus relying on this field
is not done on Windows systems.

Closes: #368

Signed-off-by: Lazar Ignjatovic <Lazar.Ignjatovic@cubic.com>
---
 configure             |  3 ++
 libavformat/ip.c      | 45 ++++++++++++++++++++++++
 libavformat/ip.h      |  6 ++++
 libavformat/network.h |  6 ++++
 libavformat/udp.c     | 80 ++++++++++++++++++++++++++++++++++++++-----
 5 files changed, 132 insertions(+), 8 deletions(-)

--
2.41.0.windows.2


This message has been marked as Public on 03/14/2024 12:04Z.

Comments

Michael Niedermayer March 14, 2024, 9:13 p.m. UTC | #1
On Thu, Mar 14, 2024 at 12:04:47PM +0000, Ignjatović, Lazar (RS) wrote:
> localaddr option now properly works with IPv6 addresses. Properly
> resolved interface index in places where default 0 interface index is
> used (marked with TODO: within udp.c). Added SO_BINDTODEVICE for mcast
> sockets that are used for reading from the network. Need for this
> arises from the fact that [ffx1::*] and [ffx2::*] mcast addresses need
> to have a defined interface for binding to avoid ambiguity between
> multiple link-local networks on the same host. Failing to set this
> option causes errors on Linux systems for interface and link-local
> scopes.
> 
> For mcast addresses, bind to mcast address is attempted as before.
> In case that this fails, which will happen on Windows, socket is bound
> to INADDR_ANY/IN6ADDR_ANY_INIT depending on address family. Actual
> interface selection is performed using udp_set_multicast_interface to
> point to the desired interface for sending/listening.
> 
> Utilization of sin6_scope_id field enables usage and adequate resolving
> of IPv6 addresses that utilize zone index
> (e.g. fe80::1ff:fe23:4567:890a%eth2)
> This is not fully supported on Windows, thus relying on this field
> is not done on Windows systems.
> 
> Closes: #368
> 
> Signed-off-by: Lazar Ignjatovic <Lazar.Ignjatovic@cubic.com>
> ---
>  configure             |  3 ++
>  libavformat/ip.c      | 45 ++++++++++++++++++++++++
>  libavformat/ip.h      |  6 ++++
>  libavformat/network.h |  6 ++++
>  libavformat/udp.c     | 80 ++++++++++++++++++++++++++++++++++++++-----
>  5 files changed, 132 insertions(+), 8 deletions(-)

breaks mingw64 build

CC	libavformat/ip.o
src/libavformat/ip.c: In function ‘ff_ip_resolve_interface_index’:
src/libavformat/ip.c:206:1: error: control reaches end of non-void function [-Werror=return-type]
 }
 ^
cc1: some warnings being treated as errors
src/ffbuild/common.mak:81: recipe for target 'libavformat/ip.o' failed
make: *** [libavformat/ip.o] Error 1

also configure produces
../configure: 6415: ../configure: network_extralibs+= -liphlpapi: not found


[...]
Ignjatović, Lazar (RS) March 18, 2024, 1:45 p.m. UTC | #2
This message has been marked as Public on 03/18/2024 13:45Z.
On Thursday, March 14, 2024 10:14 PM, Michael Niedermayer wrote:

> breaks mingw64 build
>
> CC      libavformat/ip.o
> src/libavformat/ip.c: In function ‘ff_ip_resolve_interface_index’:
> src/libavformat/ip.c:206:1: error: control reaches end of non-void function [-Werror=return-type]  }  ^
> cc1: some warnings being treated as errors
> src/ffbuild/common.mak:81: recipe for target 'libavformat/ip.o' failed
> make: *** [libavformat/ip.o] Error 1

I see the problem, when building for windows without iphlpapi.h, mentioned function has no code within.
Will account for such case.

> also configure produces
> ../configure: 6415: ../configure: network_extralibs+= -liphlpapi: not found

Problem here is that "+=" operator is a bashism, probably causing the previous error by not including iphlpapi properly.

Some questions I have:
Should I send another patch within this thread to address these comments, or should I create a V2 of the patch?
Also, if creating a V2, should the patch be sent within this thread, or should a new thread be opened?

Thank you for your time!

Sincirely,
Lazar Ignjatović
Associate Software Engineer

Cubic Defense
cubic.com
Michael Niedermayer March 19, 2024, 1:23 a.m. UTC | #3
Hi

On Mon, Mar 18, 2024 at 01:45:00PM +0000, Ignjatović, Lazar (RS)� wrote:
> 
> This message has been marked as Public on 03/18/2024 13:45Z.
> On Thursday, March 14, 2024 10:14 PM, Michael Niedermayer wrote:
> 
> > breaks mingw64 build
> >
> > CC      libavformat/ip.o
> > src/libavformat/ip.c: In function ‘ff_ip_resolve_interface_index’:
> > src/libavformat/ip.c:206:1: error: control reaches end of non-void function [-Werror=return-type]  }  ^
> > cc1: some warnings being treated as errors
> > src/ffbuild/common.mak:81: recipe for target 'libavformat/ip.o' failed
> > make: *** [libavformat/ip.o] Error 1
> 
> I see the problem, when building for windows without iphlpapi.h, mentioned function has no code within.
> Will account for such case.
> 
> > also configure produces
> > ../configure: 6415: ../configure: network_extralibs+= -liphlpapi: not found
> 
> Problem here is that "+=" operator is a bashism, probably causing the previous error by not including iphlpapi properly.
> 
> Some questions I have:
> Should I send another patch within this thread to address these comments, or should I create a V2 of the patch?
> Also, if creating a V2, should the patch be sent within this thread, or should a new thread be opened?

new patch with "V2"
it can be sent in the same or a new thread, developers are not consistent with
that.

thx

[...]
diff mbox series

Patch

diff --git a/configure b/configure
index c34bdd13f5..77f03948ce 100755
--- a/configure
+++ b/configure
@@ -2256,6 +2256,7 @@  HEADERS_LIST="
     valgrind_valgrind_h
     windows_h
     winsock2_h
+    iphlpapi_h
 "

 INTRINSICS_LIST="
@@ -6408,6 +6409,8 @@  if ! disabled network; then
         check_struct winsock2.h "struct sockaddr" sa_len
         check_type ws2tcpip.h "struct sockaddr_in6"
         check_type ws2tcpip.h "struct sockaddr_storage"
+        check_headers iphlpapi.h && network_extralibs+=" -liphlpapi" || disable iphlpapi_h
+        check_func_headers iphlpapi.h GetBestInterfaceEx
     else
         disable network
     fi
diff --git a/libavformat/ip.c b/libavformat/ip.c
index b2c7ef07e5..4f2d998c34 100644
--- a/libavformat/ip.c
+++ b/libavformat/ip.c
@@ -18,6 +18,9 @@ 
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */

+#define _DEFAULT_SOURCE
+#define _SVID_SOURCE
+
 #include <string.h>
 #include "ip.h"
 #include "libavutil/avstring.h"
@@ -159,3 +162,45 @@  void ff_ip_reset_filters(IPSourceFilters *filters)
     filters->nb_include_addrs = 0;
     filters->nb_exclude_addrs = 0;
 }
+
+unsigned int ff_ip_resolve_interface_index(struct sockaddr_storage *local_addr)
+{
+#if HAVE_WINSOCK2_H && HAVE_IPHLPAPI_H
+    DWORD retval;
+    unsigned long iface;
+
+    if (local_addr == NULL)
+        return 0;
+
+    retval = GetBestInterfaceEx((struct sockaddr*)local_addr, &iface);
+    if (retval == NO_ERROR)
+        return iface;
+
+    return 0;
+#elif !HAVE_WINSOCK2_H
+    struct ifaddrs *ifaddr, *ifa;
+
+    if (local_addr == NULL)
+        return 0;
+
+    /* Special case for link-local addresses, relevant interface is stored in sin6_scope_id */
+#if HAVE_STRUCT_SOCKADDR_IN6 && defined(IN6_IS_ADDR_LINKLOCAL)
+    if (local_addr->ss_family == AF_INET6 && IN6_IS_ADDR_LINKLOCAL(&((struct sockaddr_in6*)local_addr)->sin6_addr)) {
+        unsigned int interface;
+        interface = ((struct sockaddr_in6*)local_addr)->sin6_scope_id;
+
+        if (interface != 0)
+            return interface;
+    }
+#endif
+    if (getifaddrs(&ifaddr) == -1)
+        return 0;
+
+    for (ifa = ifaddr; ifa != NULL; ifa = ifa->ifa_next) {
+        if (ifa->ifa_addr != NULL && compare_addr((struct sockaddr_storage*)ifa->ifa_addr, local_addr) == 0)
+            return if_nametoindex(ifa->ifa_name);
+    }
+
+    return 0;
+#endif
+}
diff --git a/libavformat/ip.h b/libavformat/ip.h
index b76cdab91c..4085e96f08 100644
--- a/libavformat/ip.h
+++ b/libavformat/ip.h
@@ -69,4 +69,10 @@  int ff_ip_parse_blocks(void *log_ctx, const char *buf, IPSourceFilters *filters)
  */
 void ff_ip_reset_filters(IPSourceFilters *filters);

+/**
+ * Resolves IP address to an associated interface index
+ * @return interface index, 0 as default interface value on error
+ */
+unsigned int ff_ip_resolve_interface_index(struct sockaddr_storage *local_addr);
+
 #endif /* AVFORMAT_IP_H */
diff --git a/libavformat/network.h b/libavformat/network.h
index ca214087fc..2461b651d4 100644
--- a/libavformat/network.h
+++ b/libavformat/network.h
@@ -38,6 +38,10 @@ 
 #include <winsock2.h>
 #include <ws2tcpip.h>

+#if HAVE_IPHLPAPI_H
+#include <iphlpapi.h>
+#endif
+
 #ifndef EPROTONOSUPPORT
 #define EPROTONOSUPPORT WSAEPROTONOSUPPORT
 #endif
@@ -64,6 +68,8 @@  int ff_neterrno(void);
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <netdb.h>
+#include <net/if.h>
+#include <ifaddrs.h>

 #define ff_neterrno() AVERROR(errno)
 #endif /* HAVE_WINSOCK2_H */
diff --git a/libavformat/udp.c b/libavformat/udp.c
index d9514f5026..a603a9961e 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -220,8 +220,7 @@  static int udp_join_multicast_group(int sockfd, struct sockaddr *addr,
         struct ipv6_mreq mreq6;

         memcpy(&mreq6.ipv6mr_multiaddr, &(((struct sockaddr_in6 *)addr)->sin6_addr), sizeof(struct in6_addr));
-        //TODO: Interface index should be looked up from local_addr
-        mreq6.ipv6mr_interface = 0;
+        mreq6.ipv6mr_interface = ff_ip_resolve_interface_index((struct sockaddr_storage *)local_addr);
         if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_ADD_MEMBERSHIP, &mreq6, sizeof(mreq6)) < 0) {
             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV6_ADD_MEMBERSHIP)");
             return ff_neterrno();
@@ -231,6 +230,38 @@  static int udp_join_multicast_group(int sockfd, struct sockaddr *addr,
     return 0;
 }

+static int udp_set_multicast_interface(int sockfd, struct sockaddr *addr,
+                                    struct sockaddr *local_addr, void *logctx)
+{
+#ifdef IP_MULTICAST_IF
+    if (addr->sa_family == AF_INET) {
+        struct ip_mreq mreq;
+
+        mreq.imr_multiaddr.s_addr = ((struct sockaddr_in *)addr)->sin_addr.s_addr;
+        if (local_addr)
+            mreq.imr_interface = ((struct sockaddr_in *)local_addr)->sin_addr;
+        else
+            mreq.imr_interface.s_addr = INADDR_ANY;
+
+        if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_IF, (const void *)&mreq, sizeof(mreq)) < 0) {
+            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IP_MULTICAST_IF)");
+            return ff_neterrno();
+        }
+    }
+#endif
+#if defined(IPV6_MULTICAST_IF) && defined(IPPROTO_IPV6)
+    if (addr->sa_family == AF_INET6) {
+        unsigned int iface = ff_ip_resolve_interface_index((struct sockaddr_storage *)local_addr);
+
+        if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_IF, &iface, sizeof(unsigned int)) < 0) {
+            ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV6_MULTICAST_IF)");
+            return ff_neterrno();
+        }
+    }
+#endif
+    return 0;
+}
+
 static int udp_leave_multicast_group(int sockfd, struct sockaddr *addr,
                                      struct sockaddr *local_addr, void *logctx)
 {
@@ -254,8 +285,7 @@  static int udp_leave_multicast_group(int sockfd, struct sockaddr *addr,
         struct ipv6_mreq mreq6;

         memcpy(&mreq6.ipv6mr_multiaddr, &(((struct sockaddr_in6 *)addr)->sin6_addr), sizeof(struct in6_addr));
-        //TODO: Interface index should be looked up from local_addr
-        mreq6.ipv6mr_interface = 0;
+        mreq6.ipv6mr_interface = ff_ip_resolve_interface_index((struct sockaddr_storage *)local_addr);
         if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_DROP_MEMBERSHIP, &mreq6, sizeof(mreq6)) < 0) {
             ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV6_DROP_MEMBERSHIP)");
             return -1;
@@ -282,8 +312,7 @@  static int udp_set_multicast_sources(URLContext *h,
             struct group_source_req mreqs;
             int level = addr->sa_family == AF_INET ? IPPROTO_IP : IPPROTO_IPV6;

-            //TODO: Interface index should be looked up from local_addr
-            mreqs.gsr_interface = 0;
+            mreqs.gsr_interface = ff_ip_resolve_interface_index(local_addr);
             memcpy(&mreqs.gsr_group, addr, addr_len);
             memcpy(&mreqs.gsr_source, &sources[i], sizeof(*sources));

@@ -839,10 +868,39 @@  static int udp_open(URLContext *h, const char *uri, int flags)
      * port. This fails on windows. This makes sending to the same address
      * using sendto() fail, so only do it if we're opened in read-only mode. */
     if (s->is_multicast && (h->flags & AVIO_FLAG_READ)) {
+#if defined(SO_BINDTODEVICE) && !HAVE_WINSOCK2_H
+        {
+            struct ifreq ifr;
+            unsigned int iface;
+            iface = ff_ip_resolve_interface_index((struct sockaddr_storage *)&s->local_addr_storage);
+            memset(&ifr, 0, sizeof(ifr));
+
+            if (if_indextoname(iface, ifr.ifr_name)) {
+                if (setsockopt(udp_fd, SOL_SOCKET, SO_BINDTODEVICE, (void *)&ifr, sizeof(ifr)) < 0) {
+                    ff_log_net_error(h, AV_LOG_ERROR, "setsockopt(SO_BINDTODEVICE)");
+                }
+            }
+        }
+#endif // defined(SO_BINDTODEVICE) && !HAVE_WINSOCK2_H
         bind_ret = bind(udp_fd,(struct sockaddr *)&s->dest_addr, len);
     }
-    /* bind to the local address if not multicast or if the multicast
-     * bind failed */
+
+    /* bind to ADDR_ANY if the multicast bind failed */
+    if (s->is_multicast && bind_ret < 0) {
+        struct addrinfo *res;
+
+        if (s->dest_addr.ss_family == AF_INET)
+            res = ff_ip_resolve_host(h, "0.0.0.0", 0, SOCK_DGRAM, AF_UNSPEC, 0);
+        else if (s->dest_addr.ss_family == AF_INET6)
+            res = ff_ip_resolve_host(h, "::", 0, SOCK_DGRAM, AF_UNSPEC, 0);
+
+        if (res && res->ai_addr) {
+            bind_ret = bind(udp_fd, res->ai_addr, res->ai_addrlen);
+        }
+
+        freeaddrinfo(res);
+    }
+
     /* the bind is needed to give a port to the socket now */
     if (bind_ret < 0 && bind(udp_fd,(struct sockaddr *)&my_addr, len) < 0) {
         ff_log_net_error(h, AV_LOG_ERROR, "bind failed");
@@ -855,6 +913,12 @@  static int udp_open(URLContext *h, const char *uri, int flags)
     s->local_port = udp_port(&my_addr, len);

     if (s->is_multicast) {
+        if ((ret = udp_set_multicast_interface(udp_fd,
+                                        (struct sockaddr *)&s->dest_addr,
+                                        (struct sockaddr *)&s->local_addr_storage,
+                                        h)) < 0)
+            goto fail;
+
         if (h->flags & AVIO_FLAG_WRITE) {
             /* output */
             if ((ret = udp_set_multicast_ttl(udp_fd, s->ttl, (struct sockaddr *)&s->dest_addr, h)) < 0)