pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-uecups/+/42444?usp=email )
Change subject: Use libosmocore netdev API ...................................................................... Use libosmocore netdev API Depends: libosmocore.git Change-Id I75153eb59e96a6a2505dc5b29432c76e5c21ea24 Change-Id: I8f18ef56a6e7186fed88f965fbb34aa390c3bac1 --- A TODO-RELEASE M daemon/Makefile.am M daemon/gtp_tunnel.c M daemon/internal.h D daemon/netdev.c M daemon/tun_device.c 6 files changed, 68 insertions(+), 182 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/TODO-RELEASE b/TODO-RELEASE new file mode 100644 index 0000000..8eda384 --- /dev/null +++ b/TODO-RELEASE @@ -0,0 +1,10 @@ +# When cleaning up this file: bump API version in corresponding Makefile.am and rename corresponding debian/lib*.install +# according to https://osmocom.org/projects/cellular-infrastructure/wiki/Make_a_new_release +# In short: https://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html#Updating-version-info +# LIBVERSION=c:r:a +# If the library source code has changed at all since the last update, then increment revision: c:r + 1:a. +# If any interfaces have been added, removed, or changed since the last update: c + 1:0:a. +# If any interfaces have been added since the last public release: c:r:a + 1. +# If any interfaces have been removed or changed since the last public release: c:r:0. +#library what description / commit summary line +libosmocore >1.13.1 osmo_netdev_del_addr() diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 1c680a6..89afe7a 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -37,7 +37,6 @@ osmo_uecups_daemon_SOURCES = \ cups_client.c \ utility.c \ - netdev.c \ tun_device.c \ gtp_daemon.c \ gtp_endpoint.c \ diff --git a/daemon/gtp_tunnel.c b/daemon/gtp_tunnel.c index a13b163..4c6b2c5 100644 --- a/daemon/gtp_tunnel.c +++ b/daemon/gtp_tunnel.c @@ -25,6 +25,7 @@ struct gtp_tunnel *gtp_tunnel_alloc(struct gtp_daemon *d, const struct gtp_tunnel_params *cpars) { struct gtp_tunnel *t; + int rc; t = talloc_zero(d, struct gtp_tunnel); if (!t) @@ -60,9 +61,9 @@ memcpy(&t->user_addr, &cpars->user_addr, sizeof(t->user_addr)); memcpy(&t->remote_udp, &cpars->remote_udp, sizeof(t->remote_udp)); - if (netdev_add_addr(t->tun_dev->nl, t->tun_dev->ifindex, &t->user_addr) < 0) { + if ((rc = osmo_netdev_add_addr(t->tun_dev->netdev, &t->user_addr, 32)) < 0) { LOGT(t, LOGL_ERROR, "Cannot add user addr to tun device: %s\n", - strerror(errno)); + strerror(-rc)); } /* TODO: hash table? */ @@ -134,12 +135,14 @@ /* UNLOCKED destroy of tunnel; drops references to EP + TUN */ void _gtp_tunnel_destroy(struct gtp_tunnel *t) { + int rc; + LOGT(t, LOGL_NOTICE, "Destroying\n"); /* talloc is not thread safe, all alloc/free must come from main thread */ ASSERT_MAIN_THREAD(t->d); - if (netdev_del_addr(t->tun_dev->nl, t->tun_dev->ifindex, &t->user_addr) < 0) - LOGT(t, LOGL_ERROR, "Cannot remove user address: %s\n", strerror(errno)); + if ((rc = osmo_netdev_del_addr(t->tun_dev->netdev, &t->user_addr, 32)) < 0) + LOGT(t, LOGL_ERROR, "Cannot remove user address: %s\n", strerror(-rc)); llist_del(&t->list); diff --git a/daemon/internal.h b/daemon/internal.h index 7a016fa..ae2356b 100644 --- a/daemon/internal.h +++ b/daemon/internal.h @@ -13,6 +13,7 @@ #include <osmocom/core/it_q.h> #include <osmocom/core/utils.h> #include <osmocom/core/socket.h> +#include <osmocom/core/netdev.h> #include <osmocom/netif/stream.h> @@ -36,15 +37,6 @@ DUECUPS, }; -/*********************************************************************** - * netdev / netlink - ***********************************************************************/ - -int netdev_add_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa); -int netdev_del_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa); -int netdev_set_link(struct nl_sock *nlsk, int ifindex, bool up); -int netdev_add_defaultroute(struct nl_sock *nlsk, int ifindex, uint8_t family); - /*********************************************************************** * GTP Endpoint (UDP socket) @@ -115,8 +107,7 @@ const char *netns_name; int netns_fd; - /* netlink socket in the namespace of the tun device */ - struct nl_sock *nl; + struct osmo_netdev *netdev; /* list of local addresses? or simply only have the kernel know thses? */ diff --git a/daemon/netdev.c b/daemon/netdev.c deleted file mode 100644 index 165a6bc..0000000 --- a/daemon/netdev.c +++ /dev/null @@ -1,137 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0 */ -#include <unistd.h> -#include <stdint.h> -#include <stdbool.h> -#include <stdlib.h> -#include <string.h> -#include <stdio.h> -#include <errno.h> -#include <sys/types.h> -#include <sys/socket.h> - -#include <netinet/ip.h> -#include <netinet/ip6.h> - -#include <linux/if.h> -#include <linux/if_tun.h> -#include <sys/ioctl.h> - -#include <linux/netlink.h> -#include <netlink/socket.h> -#include <netlink/route/link.h> -#include <netlink/route/addr.h> -#include <netlink/route/route.h> -#include <netlink/route/nexthop.h> - -#include <osmocom/core/utils.h> -#include <osmocom/core/socket.h> - -/*********************************************************************** - * netlink helper functions - ***********************************************************************/ - -static int _netdev_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa, bool add) -{ - struct nl_addr *local = NULL; - struct rtnl_addr *addr; - int rc; - - switch (osa->u.sa.sa_family) { - case AF_INET: - local = nl_addr_build(AF_INET, &osa->u.sin.sin_addr, 4); - break; - case AF_INET6: - local = nl_addr_build(AF_INET6, &osa->u.sin6.sin6_addr, 16); - break; - } - OSMO_ASSERT(local); - - addr = rtnl_addr_alloc(); - OSMO_ASSERT(addr); - rtnl_addr_set_ifindex(addr, ifindex); - OSMO_ASSERT(rtnl_addr_set_local(addr, local) == 0); - - if (add) - rc = rtnl_addr_add(nlsk, addr, 0); - else - rc = rtnl_addr_delete(nlsk, addr, 0); - - rtnl_addr_put(addr); - - return rc; -} - -int netdev_add_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa) -{ - return _netdev_addr(nlsk, ifindex, osa, true); -} - -int netdev_del_addr(struct nl_sock *nlsk, int ifindex, const struct osmo_sockaddr *osa) -{ - return _netdev_addr(nlsk, ifindex, osa, false); -} - -int netdev_set_link(struct nl_sock *nlsk, int ifindex, bool up) -{ - struct rtnl_link *link, *change; - int rc; - - rc = rtnl_link_get_kernel(nlsk, ifindex, NULL, &link); - if (rc < 0) - return rc; - - change = rtnl_link_alloc(); - OSMO_ASSERT(change); - - if (up) - rtnl_link_set_flags(change, IFF_UP); - else - rtnl_link_unset_flags(change, IFF_UP); - - rc = rtnl_link_change(nlsk, link, change, 0); - - rtnl_link_put(change); - rtnl_link_put(link); - - return rc; -} - -int netdev_add_defaultroute(struct nl_sock *nlsk, int ifindex, uint8_t family) -{ - struct rtnl_route *route = rtnl_route_alloc(); - struct rtnl_nexthop *nhop = rtnl_route_nh_alloc(); - struct nl_addr *dst, *gw; - uint8_t buf[16]; - int rc; - - OSMO_ASSERT(route); - OSMO_ASSERT(nhop); - - /* destination address of route: all-zero */ - memset(buf, 0, sizeof(buf)); - dst = nl_addr_build(family, buf, family == AF_INET ? 4 : 16); - OSMO_ASSERT(dst); - nl_addr_set_prefixlen(dst, 0); - - /* gateway address of route: also all-zero */ - gw = nl_addr_clone(dst); - OSMO_ASSERT(gw); - - /* nexthop for route */ - rtnl_route_nh_set_ifindex(nhop, ifindex); - rtnl_route_nh_set_gateway(nhop, gw); - - /* tie everything together in the route */ - rtnl_route_set_dst(route, dst); - rtnl_route_set_family(route, family); - rtnl_route_add_nexthop(route, nhop); - - rc = rtnl_route_add(nlsk, route, NLM_F_CREATE); - - //rtnl_route_nh_free(nhop); - nl_addr_put(gw); - nl_addr_put(dst); - rtnl_route_put(route); - - return rc; -} diff --git a/daemon/tun_device.c b/daemon/tun_device.c index 837fd9d..c12c694 100644 --- a/daemon/tun_device.c +++ b/daemon/tun_device.c @@ -16,7 +16,7 @@ #include <fcntl.h> #include <signal.h> #include <netdb.h> - +#include <net/if.h> #include <netinet/ip.h> #include <netinet/ip6.h> @@ -26,10 +26,6 @@ #include <linux/if_tun.h> #include <sys/ioctl.h> -#include <linux/netlink.h> -#include <netlink/socket.h> -#include <netlink/route/link.h> - #include <osmocom/core/linuxlist.h> #include <osmocom/core/talloc.h> #include <osmocom/core/logging.h> @@ -318,9 +314,9 @@ static struct tun_device * _tun_device_create(struct gtp_daemon *d, const char *devname, const char *netns_name) { - struct rtnl_link *link; struct tun_device *tun; struct osmo_netns_switch_state switch_state; + bool inside_netns = false; int rc; tun = talloc_zero(d, struct tun_device); @@ -349,6 +345,7 @@ tun->netns_name, strerror(errno)); goto err_close_ns; } + inside_netns = true; } tun->fd = tun_open(0, tun->devname); @@ -357,49 +354,72 @@ goto err_restore_ns; } - tun->nl = nl_socket_alloc(); - if (!tun->nl || nl_connect(tun->nl, NETLINK_ROUTE) < 0) { - LOGTUN(tun, LOGL_ERROR, "Cannot create netlink socket in namespace '%s'\n", - tun->netns_name); + /* Store interface index: + * (Note: there's a potential race condition here between creating the + * iface with a given name above and attempting to retrieve its ifindex based + * on that name. Someone (ie udev) could have the iface renamed in + * between here. It's a pity that TUNSETIFF doesn't copy back to us the + * newly allocated ifindex as it does with ifname) + */ + tun->ifindex = if_nametoindex(tun->devname); + if (tun->ifindex == 0) { + LOGTUN(tun, LOGL_ERROR, "Unable to find ifindex for dev %s\n", + tun->devname); goto err_close; } - rc = rtnl_link_get_kernel(tun->nl, 0, tun->devname, &link); - if (rc < 0) { - LOGTUN(tun, LOGL_ERROR, "Cannot get ifindex for netif after create?!?\n"); - goto err_free_nl; - } - tun->ifindex = rtnl_link_get_ifindex(link); - rtnl_link_put(link); - /* switch back to default namespace before creating new thread */ - if (tun->netns_name) + if (inside_netns) { OSMO_ASSERT(osmo_netns_switch_exit(&switch_state) == 0); + inside_netns = false; + } + + tun->netdev = osmo_netdev_alloc(tun, tun->devname); + if (tun->netns_name) { + rc = osmo_netdev_set_netns_name(tun->netdev, tun->netns_name); + if (rc < 0) + goto err_free_netdev; + } + rc = osmo_netdev_set_ifindex(tun->netdev, tun->ifindex); + if (rc < 0) + goto err_free_netdev; + + rc = osmo_netdev_register(tun->netdev); + if (rc < 0) + goto err_free_netdev; /* bring the network device up */ - rc = netdev_set_link(tun->nl, tun->ifindex, true); + rc = osmo_netdev_ifupdown(tun->netdev, true); if (rc < 0) LOGTUN(tun, LOGL_ERROR, "Cannot set interface to 'up'\n"); if (tun->netns_name) { - rc = netdev_add_defaultroute(tun->nl, tun->ifindex, AF_INET); + struct osmo_sockaddr osa; + + memset(&osa, 0, sizeof(osa)); + osa.u.sin.sin_family = AF_INET; + osa.u.sin.sin_addr.s_addr = 0; + rc = osmo_netdev_add_route(tun->netdev, &osa, 0, NULL); if (rc < 0) LOGTUN(tun, LOGL_ERROR, "Cannot add IPv4 default route " - "(rc=%d): %s\n", rc, nl_geterror(rc)); + "(rc=%d): %s\n", rc, strerror(-rc)); else LOGTUN(tun, LOGL_INFO, "Added IPv4 default route\n"); - rc = netdev_add_defaultroute(tun->nl, tun->ifindex, AF_INET6); + memset(&osa, 0, sizeof(osa)); + osa.u.sin6.sin6_family = AF_INET6; + memset(&osa.u.sin6.sin6_addr, 0, sizeof(osa.u.sin6.sin6_addr)); + rc = osmo_netdev_add_route(tun->netdev, &osa, 0, NULL); if (rc < 0) LOGTUN(tun, LOGL_ERROR, "Cannot add IPv6 default route " - "(rc=%d): %s\n", rc, nl_geterror(rc)); + "(rc=%d): %s\n", rc, strerror(-rc)); else LOGTUN(tun, LOGL_INFO, "Added IPv6 default route\n"); } if (pthread_create(&tun->thread, NULL, tun_device_thread, tun)) { LOGTUN(tun, LOGL_ERROR, "Cannot create TUN thread: %s\n", strerror(errno)); - goto err_free_nl; + goto err_free_netdev; } LOGTUN(tun, LOGL_INFO, "Created (in netns '%s')\n", tun->netns_name); @@ -407,12 +427,12 @@ return tun; -err_free_nl: - nl_socket_free(tun->nl); +err_free_netdev: + osmo_netdev_free(tun->netdev); err_close: close(tun->fd); err_restore_ns: - if (tun->netns_name) + if (inside_netns) OSMO_ASSERT(osmo_netns_switch_exit(&switch_state) == 0); err_close_ns: if (tun->netns_name) @@ -482,7 +502,7 @@ if (tun->netns_name) close(tun->netns_fd); close(tun->fd); - nl_socket_free(tun->nl); + osmo_netdev_free(tun->netdev); talloc_free(tun); } -- To view, visit https://gerrit.osmocom.org/c/osmo-uecups/+/42444?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-uecups Gerrit-Branch: master Gerrit-Change-Id: I8f18ef56a6e7186fed88f965fbb34aa390c3bac1 Gerrit-Change-Number: 42444 Gerrit-PatchSet: 4 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <[email protected]> Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Reviewer: lynxis lazus <[email protected]> Gerrit-Reviewer: osmith <[email protected]> Gerrit-Reviewer: pespin <[email protected]>
