This patch fixes the bug described in the issue #1096 and makes
both if_nametoindex() and if_indextoname() work correctly on OSv.
The implementation of both functions comes from musl 1.1.24 AS IS
and this patch removes the old sources from the libc/network folder which
seem to have come from an older version of musl. However the musl
implementation opens a socket using the AF_UNIX family which is not supported
by OSv. As Charles Myers suggested and authored relevant patch of his own
(which is part of ipv6 branch), changing AF_UNIX to AF_INET provides
necessary workaround but is not enough to make these functions
work correctly.
The if_nametoindex() and if_indextoname() call ioctl() with commands
SIOCGIFINDEX and SIOCGIFNAME respectively. However OSv has a bug in the
FreeBSD/Linux translation logic of SIOCGIFINDEX and does not support
SIOCGIFNAME at all. So this patch fixes the first issue which actually
comes from a different commit on ipv6 branch and was authored by Spirent
developers. Secondly, it also provides support of SIOCGIFNAME. Please
note new field ifru_ifindex added to the l_ifreq struct.
Finally, this patch also adds a unit test borrowed from the Android project
and adapted to run using boot unit test framework.
Fixes #1096
Co-authored-by: Charles Myers <charles.my...@spirent.com>
Co-authored-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
Signed-off-by: Waldemar Kozaczuk <jwkozac...@gmail.com>
---
Makefile | 6 +-
bsd/sys/compat/linux/linux.h | 1 +
bsd/sys/compat/linux/linux_ioctl.cc | 33 ++++++++++-
include/osv/ioctl.h | 1 -
libc/network/README.md | 14 +----
libc/network/__socket.h | 5 ++
libc/network/if_indextoname.c | 18 ------
libc/network/if_nametoindex.c | 19 ------
modules/tests/Makefile | 3 +-
tests/tst-net_if_test.cc | 89 +++++++++++++++++++++++++++++
10 files changed, 133 insertions(+), 56 deletions(-)
create mode 100644 libc/network/__socket.h
delete mode 100644 libc/network/if_indextoname.c
delete mode 100644 libc/network/if_nametoindex.c
create mode 100644 tests/tst-net_if_test.cc
diff --git a/Makefile b/Makefile
index ad70580c..8f95e1fb 100644
--- a/Makefile
+++ b/Makefile
@@ -1495,8 +1495,10 @@ musl += network/inet_aton.o
musl += network/inet_pton.o
musl += network/inet_ntop.o
musl += network/proto.o
-libc += network/if_indextoname.o
-libc += network/if_nametoindex.o
+musl += network/if_indextoname.o
+$(out)/musl/src/network/if_indextoname.o: CFLAGS += --include
libc/syscall_to_function.h --include libc/network/__socket.h
+musl += network/if_nametoindex.o
+$(out)/musl/src/network/if_nametoindex.o: CFLAGS += --include
libc/syscall_to_function.h --include libc/network/__socket.h
musl += network/gai_strerror.o
musl += network/h_errno.o
musl += network/getservbyname_r.o
diff --git a/bsd/sys/compat/linux/linux.h b/bsd/sys/compat/linux/linux.h
index 1e6116aa..c406580a 100644
--- a/bsd/sys/compat/linux/linux.h
+++ b/bsd/sys/compat/linux/linux.h
@@ -144,6 +144,7 @@ struct l_ifreq {
struct l_ifmap ifru_map;
char ifru_slave[LINUX_IFNAMSIZ];
l_uintptr_t ifru_data;
+ int ifru_ifindex;
} ifr_ifru;
} __packed;
diff --git a/bsd/sys/compat/linux/linux_ioctl.cc
b/bsd/sys/compat/linux/linux_ioctl.cc
index 43a50bb5..43a081f2 100644
--- a/bsd/sys/compat/linux/linux_ioctl.cc
+++ b/bsd/sys/compat/linux/linux_ioctl.cc
@@ -199,7 +199,6 @@ linux_gifhwaddr(struct ifnet *ifp, struct l_ifreq *ifr)
return (ENOENT);
}
-
/*
* Fix the interface address field in bsd_ifreq. The bsd stack expects a
* length/family byte members, while linux and everyone else use a short family
@@ -222,6 +221,16 @@ linux_to_bsd_ifreq(struct bsd_ifreq *ifr_p)
ifr_p->ifr_addr.sa_len = 16 ;
}
+/*
+ * FreeBSD ifru_index is short but Linux is an int so need to clear extra bits.
+ */
+static inline void
+bsd_to_linux_ifreq_ifindex(struct bsd_ifreq *ifr_p)
+{
+ void *ptr = &ifr_p->ifr_index;
+ *(int *)(ptr) = ifr_p->ifr_index;
+}
+
/*
* Socket related ioctls
*/
@@ -241,8 +250,8 @@ linux_ioctl_socket(socket_file *fp, u_long cmd, void *data)
switch (cmd) {
case SIOCSIFADDR:
case SIOCSIFNETMASK:
- case SIOCSIFDSTADDR:
- case SIOCSIFBRDADDR:
+ case SIOCSIFDSTADDR:
+ case SIOCSIFBRDADDR:
if ((ifp = ifunit_ref((char *)data)) == NULL)
return (EINVAL);
linux_to_bsd_ifreq((struct bsd_ifreq *)data) ;
@@ -251,11 +260,29 @@ linux_ioctl_socket(socket_file *fp, u_long cmd, void
*data)
case SIOCGIFMTU:
case SIOCSIFMTU:
+ if ((ifp = ifunit_ref((char *)data)) == NULL)
+ return (EINVAL);
+ error = fp->bsd_ioctl(cmd, data);
+ break;
+
case SIOCGIFINDEX:
if ((ifp = ifunit_ref((char *)data)) == NULL)
return (EINVAL);
error = fp->bsd_ioctl(cmd, data);
+ bsd_to_linux_ifreq_ifindex((struct bsd_ifreq *)data);
+ break;
+
+ case SIOCGIFNAME:
+ {
+ struct l_ifreq *linux_ifreq = (struct l_ifreq *)data;
+ auto index = linux_ifreq->ifr_ifru.ifru_ifindex;
+ if ((ifp = ifnet_byindex_ref(index)) == NULL)
+ return (EINVAL);
+ if (!linux_ifreq->ifr_name)
+ return (EINVAL);
+ strncpy(linux_ifreq->ifr_name, ifp->if_xname, LINUX_IFNAMSIZ);
break;
+ }
case SIOCGIFADDR:
case SIOCGIFDSTADDR:
diff --git a/include/osv/ioctl.h b/include/osv/ioctl.h
index bbabfdc1..86d3f3a5 100644
--- a/include/osv/ioctl.h
+++ b/include/osv/ioctl.h
@@ -21,7 +21,6 @@
#undef SIOCADDRT
#undef SIOCDELRT
#undef SIOCRTMSG
-#undef SIOCGIFNAME
#undef SIOCSIFLINK
#undef SIOCGIFMEM
#undef SIOCSIFMEM
diff --git a/libc/network/README.md b/libc/network/README.md
index 1b5bd69d..be3dd5cb 100644
--- a/libc/network/README.md
+++ b/libc/network/README.md
@@ -1,17 +1,7 @@
-Possibly upgrade files in `libc/network` folder when merging ipv6 branch into
master.
+Some notes worth considering when upgrading to newer version of musl
-* `freeaddrinfo.o` - symlink to musl 0.9.12, in 1.1.24 more than LOCK, mess
with changes to ai_buf struct (affects `getaddrinfo.c`_
+* `freeaddrinfo.o` - symlink to musl 0.9.12, in 1.1.24 more than LOCK, mess
with the changes related to the `ai_buf` struct (affects `getaddrinfo.c`)
* `getaddrinfo.c` - originates from musl but some changes were made on OSv side
* `gethostbyname_r.c` - originates from musl and is identical except for extra
`gethostbyname()` that can probably be moved to a separate file
-* `getifaddrs.c` - originates from musl but significantly changed on OSv in
commits #41fc1499ed9cdf4f876135a723e5b0fa98453161 and
#e9f12c3f35aa22db1c947690e5049b52b6246381
* `getnameinfo.c` - originates from musl but makes a change to first read
`/etc/services`; look at #2c7f0a58b98abdf12d17c4ac71e334db4512b72f
* `__ipparse.c` - includes `__dns.hh` instead of `__dns.h`; removed from 1.1.24
-
-These changed between 0.9.12, in 1.1.24 and these changes seem to have been at
least partially addressed in ipv6 branch
-* `if_indextoname.c` - `socket(AF_UNIX,..)` vs `socket(AF_INET,..)` and
`syscall(close`
-* `if_nameindex.c` - ...
-* `if_nametoindex.c` - ... AND `strncpy` vs `strlcpy`
-
-Ideally can be addressed without copying into osv tree by using socket and
syscall macro overrides. The strncpy vs strlcpy situation is more tricky.
-
-30 files are from musl 'as is' (`grep -P '^musl \+= network' Makefile`)
diff --git a/libc/network/__socket.h b/libc/network/__socket.h
new file mode 100644
index 00000000..b92e8d17
--- /dev/null
+++ b/libc/network/__socket.h
@@ -0,0 +1,5 @@
+#include <sys/socket.h>
+//This header is included by musl src/network/if_indextoname.c and
src/network/if_nametoindex.c
+//to force that socket is opened using AF_INET family as AF_UNIX is not
supported
+#undef AF_UNIX
+#define AF_UNIX AF_INET
diff --git a/libc/network/if_indextoname.c b/libc/network/if_indextoname.c
deleted file mode 100644
index e7f526d2..00000000
--- a/libc/network/if_indextoname.c
+++ /dev/null
@@ -1,18 +0,0 @@
-#define _GNU_SOURCE
-#include <net/if.h>
-#include <sys/socket.h>
-#include <sys/ioctl.h>
-#include <string.h>
-#include "unistd.h"
-
-char *if_indextoname(unsigned index, char *name)
-{
- struct ifreq ifr;
- int fd, r;
-
- if ((fd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0) return 0;
- ifr.ifr_ifindex = index;
- r = ioctl(fd, SIOCGIFNAME, &ifr);
- close(fd);
- return r < 0 ? 0 : strncpy(name, ifr.ifr_name, IF_NAMESIZE);
-}
diff --git a/libc/network/if_nametoindex.c b/libc/network/if_nametoindex.c
deleted file mode 100644
index bdaad478..00000000
--- a/libc/network/if_nametoindex.c
+++ /dev/null
@@ -1,19 +0,0 @@
-#define _GNU_SOURCE
-#include <net/if.h>
-#include <sys/socket.h>
-#include <sys/ioctl.h>
-#include <string.h>
-#include <unistd.h>
-#include "syscall.h"
-
-unsigned if_nametoindex(const char *name)
-{
- struct ifreq ifr;
- int fd, r;
-
- if ((fd = socket(AF_UNIX, SOCK_DGRAM, 0)) < 0) return -1;
- strlcpy(ifr.ifr_name, name, sizeof ifr.ifr_name);
- r = ioctl(fd, SIOCGIFINDEX, &ifr);
- close(fd);
- return r < 0 ? r : ifr.ifr_ifindex;
-}
diff --git a/modules/tests/Makefile b/modules/tests/Makefile
index f79da870..33af8303 100644
--- a/modules/tests/Makefile
+++ b/modules/tests/Makefile
@@ -198,7 +198,8 @@ common-boost-tests := tst-vfs.so tst-libc-locking.so
misc-fs-stress.so \
tst-bsd-tcp1-zsndrcv.so tst-async.so tst-rcu-list.so tst-tcp-listen.so \
tst-poll.so tst-bitset-iter.so tst-timer-set.so tst-clock.so \
tst-rcu-hashtable.so tst-unordered-ring-mpsc.so \
- tst-seek.so tst-ctype.so tst-wctype.so tst-string.so tst-time.so
tst-dax.so
+ tst-seek.so tst-ctype.so tst-wctype.so tst-string.so tst-time.so
tst-dax.so \
+ tst-net_if_test.so
boost-tests := $(common-boost-tests)
diff --git a/tests/tst-net_if_test.cc b/tests/tst-net_if_test.cc
new file mode 100644
index 00000000..e038d8a4
--- /dev/null
+++ b/tests/tst-net_if_test.cc
@@ -0,0 +1,89 @@
+/*
+ * Copyright (C) 2016 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+#include <net/if.h>
+#include <errno.h>
+#include <ifaddrs.h>
+//
+// gcc tests/tst-time.cc -lstdc++ -lboost_unit_test_framework
-lboost_filesystem -o /tmp/a
+//#define BOOST_TEST_DYN_LINK //ONLY FOR LINUX
+#define BOOST_TEST_MODULE tst-net_if_test
+
+#include <boost/test/unit_test.hpp>
+namespace utf = boost::unit_test;
+
+#include <set>
+
+#define TEST(MODULE_NAME,TEST_NAME)
BOOST_AUTO_TEST_CASE(MODULE_NAME##TEST_NAME)
+
+#define EXPECT_EQ(EXP1,EXP2) BOOST_CHECK_EQUAL(EXP1,EXP2)
+#define EXPECT_STREQ(EXP1,EXP2) BOOST_CHECK_EQUAL(EXP1,EXP2)
+
+#define ASSERT_TRUE(EXP) BOOST_REQUIRE(EXP)
+#define ASSERT_EQ(EXP1,EXP2) BOOST_CHECK_EQUAL(EXP1,EXP2)
+#define ASSERT_STREQ(EXP1,EXP2) BOOST_CHECK_EQUAL(EXP1,EXP2)
+#define ASSERT_NE(EXP1,EXP2) BOOST_REQUIRE((EXP1) != (EXP2))
+#define ASSERT_GE(EXP1,EXP2) BOOST_REQUIRE((EXP1) >= (EXP2))
+#define ASSERT_LT(EXP1,EXP2) BOOST_REQUIRE((EXP1) < (EXP2))
+#define ASSERT_LE(EXP1,EXP2) BOOST_REQUIRE((EXP1) <= (EXP2))
+
+#ifdef __OSV__
+#define LOOPBACK_NAME "lo0"
+#else
+#define LOOPBACK_NAME "lo"
+#endif
+
+TEST(net_if, if_nametoindex_if_indextoname) {
+ unsigned index;
+ index = if_nametoindex(LOOPBACK_NAME);
+ ASSERT_NE(index, 0U);
+ char buf[IF_NAMESIZE] = {};
+ char* name = if_indextoname(index, buf);
+ ASSERT_STREQ(LOOPBACK_NAME, name);
+}
+TEST(net_if, if_nametoindex_fail) {
+ unsigned index = if_nametoindex("this-interface-does-not-exist");
+ ASSERT_EQ(0U, index);
+}
+TEST(net_if, if_nameindex) {
+ struct if_nameindex* list = if_nameindex();
+ ASSERT_TRUE(list != nullptr);
+ ASSERT_TRUE(list->if_index != 0);
+ std::set<std::string> if_nameindex_names;
+ char buf[IF_NAMESIZE] = {};
+ bool saw_lo = false;
+ for (struct if_nameindex* it = list; it->if_index != 0; ++it) {
+ fprintf(stderr, "\t%d\t%s\n", it->if_index, it->if_name);
+ if_nameindex_names.insert(it->if_name);
+ EXPECT_EQ(it->if_index, if_nametoindex(it->if_name));
+ EXPECT_STREQ(it->if_name, if_indextoname(it->if_index, buf));
+ if (strcmp(it->if_name, LOOPBACK_NAME) == 0) saw_lo = true;
+ }
+ ASSERT_TRUE(saw_lo);
+ if_freenameindex(list);
+ std::set<std::string> getifaddrs_names;
+ ifaddrs* ifa;
+ ASSERT_EQ(0, getifaddrs(&ifa));
+ for (ifaddrs* it = ifa; it != nullptr; it = it->ifa_next) {
+ getifaddrs_names.insert(it->ifa_name);
+ }
+ freeifaddrs(ifa);
+ ASSERT_TRUE(getifaddrs_names == if_nameindex_names);
+}
+TEST(net_if, if_freenameindex_nullptr) {
+#if defined(__BIONIC__)
+ if_freenameindex(nullptr);
+#endif
+}
--
2.35.1
--
You received this message because you are subscribed to the Google Groups "OSv
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to osv-dev+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/osv-dev/20220607203618.322232-1-jwkozaczuk%40gmail.com.
