pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ggsn/+/38480?usp=email ) Change subject: ggsn: use libosmocore tundev API to create apn tun device ...................................................................... ggsn: use libosmocore tundev API to create apn tun device This way we can start dropping old osmo-ggsn specific API, avoiding duplication of code. Moreover, the osmo-ggsn code is using older ioctl APIs, which are discouraged nowadays in favour of netlink, which osmo_tundev/osmo_netdev from libosmocore is used. Even better, we win for free non-blocking write behavior in the tundev when switching to the new API, since it already has its own internal wqueue. While doing this, BSD code is dropped since anyway it's not been maintained for a long time. If needed, the BSD support can be added to libosmocore osmo_tundev/osmo_netdev API. This is a first step (already working). Follow-up commits will replace the APIs to set up routes and addresses, and later on osmo-ggsn will win support to set MTU on the interface. Furthermore, this will allow easily adding netns support to osmo-ggsn later on if ever needed. Depends: libosmocore.git Change-Id Ia8a7e7ec6d96c7aebc80528236a0e0d035e7f38d Change-Id: I4d99ba147ac0f3b414d2efef0068b6b8d6cf0014 --- M TODO-RELEASE M ggsn/ggsn.c M ggsn/ggsn.h M lib/tun.c M lib/tun.h M sgsnemu/sgsnemu.c 6 files changed, 90 insertions(+), 148 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved osmith: Looks good to me, but someone else must approve diff --git a/TODO-RELEASE b/TODO-RELEASE index 905c234..194885d 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -9,4 +9,5 @@ #library what description / commit summary line libgtp append new field dir_tun_flags in struct pdp_t (older users not using the field should be fine since struct pdp_t is allocated internally) libgtp ABI break new field cb_create_context_ind in struct gsn_t -libgtp new API gtp_set_cb_update_context_ind(), gtp_update_context_resp() \ No newline at end of file +libgtp new API gtp_set_cb_update_context_ind(), gtp_update_context_resp() +libosmocore >1.10.0 osmo_tundev_get_fd() \ No newline at end of file diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c index 19b0132..7967f42 100644 --- a/ggsn/ggsn.c +++ b/ggsn/ggsn.c @@ -62,7 +62,6 @@ LLIST_HEAD(g_ggsn_list); -static int ggsn_tun_fd_cb(struct osmo_fd *fd, unsigned int what); static int cb_tun_ind(struct tun_t *tun, void *pack, unsigned len); void ggsn_close_one_pdp(struct pdp_t *pdp) @@ -150,7 +149,6 @@ if (apn->cfg.gtpu_mode == APN_GTPU_MODE_TUN) { /* release tun device */ LOGPAPN(LOGL_INFO, apn, "Closing TUN device %s\n", apn->tun.tun->devname); - osmo_fd_unregister(&apn->tun.fd); } tun_free(apn->tun.tun); apn->tun.tun = NULL; @@ -234,10 +232,6 @@ } LOGPAPN(LOGL_INFO, apn, "Opened TUN device %s\n", apn->tun.tun->devname); - /* Register with libosmocore */ - osmo_fd_setup(&apn->tun.fd, apn->tun.tun->fd, OSMO_FD_READ, ggsn_tun_fd_cb, apn, 0); - osmo_fd_register(&apn->tun.fd); - /* Set TUN library callback */ tun_set_cb_ind(apn->tun.tun, cb_tun_ind); break; @@ -799,16 +793,6 @@ return tun_encaps((struct tun_t *)pdp->ipif, pack, len); } -/* callback for tun device osmocom select loop integration */ -static int ggsn_tun_fd_cb(struct osmo_fd *fd, unsigned int what) -{ - struct apn_ctx *apn = fd->data; - - OSMO_ASSERT(what & OSMO_FD_READ); - - return tun_decaps(apn->tun.tun); -} - /* callback for libgtp osmocom select loop integration */ static int ggsn_gtp_fd_cb(struct osmo_fd *fd, unsigned int what) { diff --git a/ggsn/ggsn.h b/ggsn/ggsn.h index 7073575..bd953a4 100644 --- a/ggsn/ggsn.h +++ b/ggsn/ggsn.h @@ -97,7 +97,6 @@ char *ipdown_script; } cfg; struct tun_t *tun; - struct osmo_fd fd; } tun; /* ipv6 link-local address */ diff --git a/lib/tun.c b/lib/tun.c index cb66fef..acc3927 100644 --- a/lib/tun.c +++ b/lib/tun.c @@ -41,20 +41,9 @@ #include <net/route.h> #include <net/if.h> -#if defined(__linux__) #include <linux/if_tun.h> -#elif defined (__FreeBSD__) -#include <net/if_tun.h> -#include <net/if_var.h> -#include <netinet/in_var.h> - -#elif defined (__APPLE__) -#include <net/if.h> - -#else -#error "Unknown platform!" -#endif +#include <osmocom/core/msgb.h> #include "tun.h" #include "syserr.h" @@ -155,121 +144,93 @@ } } +static int tun_tundev_data_ind_cb(struct osmo_tundev *tundev, struct msgb *msg) +{ + struct tun_t *tun = osmo_tundev_get_priv_data(tundev); + int rc = 0; + if (tun->cb_ind) + rc = tun->cb_ind(tun, msgb_data(msg), msgb_length(msg)); + msgb_free(msg); + return rc; +} + int tun_new(struct tun_t **tun, const char *dev_name, bool use_kernel, int fd0, int fd1u) { + struct tun_t *t; + int rc; -#if defined(__linux__) - struct ifreq ifr; - -#elif defined(__FreeBSD__) || defined (__APPLE__) - char devname[IFNAMSIZ + 5]; /* "/dev/" + ifname */ - int devnum; - struct ifaliasreq areq; - int fd; -#endif - - if (!(*tun = calloc(1, sizeof(struct tun_t)))) { - SYS_ERR(DTUN, LOGL_ERROR, errno, "calloc() failed"); + t = talloc_zero(NULL, struct tun_t); + if (!t) { + SYS_ERR(DTUN, LOGL_ERROR, errno, "talloc_zero() failed"); return EOF; } + *tun = t; - (*tun)->cb_ind = NULL; - (*tun)->addrs = 0; - (*tun)->routes = 0; + t->cb_ind = NULL; + t->addrs = 0; + t->routes = 0; + t->fd = -1; -#if defined(__linux__) if (!use_kernel) { - /* Open the actual tun device */ - if (((*tun)->fd = open("/dev/net/tun", O_RDWR)) < 0) { - SYS_ERR(DTUN, LOGL_ERROR, errno, "open() failed"); + osmo_strlcpy(t->devname, dev_name, IFNAMSIZ); + t->devname[IFNAMSIZ - 1] = 0; + + t->tundev = osmo_tundev_alloc(t, dev_name); + if (!t->tundev) goto err_free; - } + osmo_tundev_set_priv_data(t->tundev, t); + osmo_tundev_set_data_ind_cb(t->tundev, tun_tundev_data_ind_cb); + rc = osmo_tundev_set_dev_name(t->tundev, dev_name); + if (rc < 0) + goto err_free_tundev; - /* Set device flags. For some weird reason this is also the method - used to obtain the network interface name */ - memset(&ifr, 0, sizeof(ifr)); - if (dev_name) - strcpy(ifr.ifr_name, dev_name); - ifr.ifr_flags = IFF_TUN | IFF_NO_PI; /* Tun device, no packet info */ - if (ioctl((*tun)->fd, TUNSETIFF, (void *)&ifr) < 0) { - SYS_ERR(DTUN, LOGL_ERROR, errno, "ioctl() failed"); - goto err_close; - } - - strncpy((*tun)->devname, ifr.ifr_name, IFNAMSIZ); - (*tun)->devname[IFNAMSIZ - 1] = 0; + /* Open the actual tun device */ + rc = osmo_tundev_open(t->tundev); + if (rc < 0) + goto err_free; + t->fd = osmo_tundev_get_fd(t->tundev); + t->netdev = osmo_tundev_get_netdev(t->tundev); /* Disable checksums */ - if (ioctl((*tun)->fd, TUNSETNOCSUM, 1) < 0) { - SYS_ERR(DTUN, LOGL_NOTICE, errno, "could not disable checksum on %s", (*tun)->devname); + if (ioctl(t->fd, TUNSETNOCSUM, 1) < 0) { + SYS_ERR(DTUN, LOGL_NOTICE, errno, "could not disable checksum on %s", t->devname); } + + LOGP(DTUN, LOGL_NOTICE, "tun %s configured\n", t->devname); return 0; +err_free_tundev: + osmo_tundev_free(t->tundev); +err_free: + talloc_free(t); + *tun = NULL; + return -1; + } else { - strncpy((*tun)->devname, dev_name, IFNAMSIZ); - (*tun)->devname[IFNAMSIZ - 1] = 0; - (*tun)->fd = -1; + osmo_strlcpy(t->devname, dev_name, IFNAMSIZ); + t->devname[IFNAMSIZ - 1] = 0; if (gtp_kernel_create(-1, dev_name, fd0, fd1u) < 0) { LOGP(DTUN, LOGL_ERROR, "cannot create GTP tunnel device: %s\n", strerror(errno)); return -1; } + t->netdev = osmo_netdev_alloc(t, dev_name); + if (!t->netdev) + goto err_kernel_create; + rc = osmo_netdev_set_ifindex(t->netdev, if_nametoindex(dev_name)); + if (rc < 0) + goto err_netdev_free; + rc = osmo_netdev_register(t->netdev); + if (rc < 0) + goto err_netdev_free; LOGP(DTUN, LOGL_NOTICE, "GTP kernel configured\n"); return 0; - } - -#elif defined(__FreeBSD__) || defined (__APPLE__) - - if (use_kernel) { - LOGP(DTUN, LOGL_ERROR, "No kernel GTP-U support in FreeBSD!\n"); - return -1; - } - - /* Find suitable device */ - for (devnum = 0; devnum < 255; devnum++) { /* TODO 255 */ - snprintf(devname, sizeof(devname), "/dev/tun%d", devnum); - if (((*tun)->fd = open(devname, O_RDWR)) >= 0) - break; - if (errno != EBUSY) - break; - } - if ((*tun)->fd < 0) { - SYS_ERR(DTUN, LOGL_ERROR, errno, - "Can't find tunnel device"); - goto err_free; - } - - snprintf((*tun)->devname, sizeof((*tun)->devname), "tun%d", devnum); - (*tun)->devname[sizeof((*tun)->devname)-1] = 0; - - /* The tun device we found might have "old" IP addresses allocated */ - /* We need to delete those. This problem is not present on Linux */ - - memset(&areq, 0, sizeof(areq)); - - /* Set up interface name */ - strncpy(areq.ifra_name, (*tun)->devname, IFNAMSIZ); - areq.ifra_name[IFNAMSIZ - 1] = 0; /* Make sure to terminate */ - - /* Create a channel to the NET kernel. */ - if ((fd = socket(AF_INET, SOCK_DGRAM, 0)) < 0) { - SYS_ERR(DTUN, LOGL_ERROR, errno, "socket() failed"); - goto err_close; - } - - /* Delete any IP addresses until SIOCDIFADDR fails */ - while (ioctl(fd, SIOCDIFADDR, (void *)&areq) != -1) ; - - close(fd); - return 0; -#endif - -err_close: - close((*tun)->fd); -err_free: - free(*tun); - *tun = NULL; +err_netdev_free: + osmo_netdev_free(t->netdev); +err_kernel_create: + gtp_kernel_stop(t->devname); return -1; + } } int tun_free(struct tun_t *tun) @@ -279,17 +240,23 @@ netdev_delroute4(&tun->dstaddr.v4, &tun->addr.v4, &tun->netmask); } - if (tun->fd >= 0) { - if (close(tun->fd)) { + if (tun->tundev) { + if (osmo_tundev_close(tun->tundev) < 0) { SYS_ERR(DTUN, LOGL_ERROR, errno, "close() failed"); } + osmo_tundev_free(tun->tundev); + tun->tundev = NULL; + /* netdev is owned by tundev: */ + tun->netdev = NULL; + } else { + /* netdev was allocated directly, free it: */ + osmo_netdev_free(tun->netdev); + tun->netdev = NULL; } gtp_kernel_stop(tun->devname); - /* TODO: For solaris we need to unlink streams */ - - free(tun); + talloc_free(tun); return 0; } @@ -300,30 +267,16 @@ return 0; } -int tun_decaps(struct tun_t *this) -{ - unsigned char buffer[PACKET_MAX]; - int status; - - if ((status = read(this->fd, buffer, sizeof(buffer))) <= 0) { - SYS_ERR(DTUN, LOGL_ERROR, errno, "read() failed"); - return -1; - } - - if (this->cb_ind) - return this->cb_ind(this, buffer, status); - - return 0; -} - int tun_encaps(struct tun_t *tun, void *pack, unsigned len) { + struct msgb *msg = msgb_alloc(PACKET_MAX, "tun_tx"); int rc; - rc = write(tun->fd, pack, len); + + OSMO_ASSERT(msg); + memcpy(msgb_put(msg, len), pack, len); + rc = osmo_tundev_send(tun->tundev, msg); if (rc < 0) { SYS_ERR(DTUN, LOGL_ERROR, errno, "TUN(%s): write() failed", tun->devname); - } else if (rc < len) { - LOGTUN(LOGL_ERROR, tun, "short write() %d < %u\n", rc, len); } return rc; } diff --git a/lib/tun.h b/lib/tun.h index 36a4f7e..8a3a2d4 100644 --- a/lib/tun.h +++ b/lib/tun.h @@ -16,6 +16,9 @@ #include <stdbool.h> #include <net/if.h> +#include <osmocom/core/netdev.h> +#include <osmocom/core/tun.h> + #include "../lib/in46_addr.h" #define PACKET_MAX 8196 /* Maximum packet size we receive */ @@ -30,6 +33,8 @@ *************************************************************/ struct tun_t { + struct osmo_tundev *tundev; + struct osmo_netdev *netdev; int fd; /* File descriptor to tun interface */ struct in46_addr addr; struct in46_addr dstaddr; diff --git a/sgsnemu/sgsnemu.c b/sgsnemu/sgsnemu.c index 152b05b..d6d87b7 100644 --- a/sgsnemu/sgsnemu.c +++ b/sgsnemu/sgsnemu.c @@ -23,6 +23,7 @@ #include <osmocom/core/application.h> #include <osmocom/core/msgb.h> +#include <osmocom/core/select.h> #include <ctype.h> #include <netdb.h> @@ -2186,9 +2187,8 @@ if (!signal_received) { - if ((tun) && FD_ISSET(tun->fd, &fds) && tun_decaps(tun) < 0) { - SYS_ERR(DSGSN, LOGL_ERROR, 0, - "TUN decaps failed"); + if ((tun) && FD_ISSET(tun->fd, &fds)) { + osmo_select_main(1); } if (FD_ISSET(gsn->fd0, &fds)) -- To view, visit https://gerrit.osmocom.org/c/osmo-ggsn/+/38480?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-ggsn Gerrit-Branch: master Gerrit-Change-Id: I4d99ba147ac0f3b414d2efef0068b6b8d6cf0014 Gerrit-Change-Number: 38480 Gerrit-PatchSet: 3 Gerrit-Owner: pespin &lt;pespin(a)sysmocom.de&gt; Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge &lt;laforge(a)osmocom.org&gt; Gerrit-Reviewer: osmith &lt;osmith(a)sysmocom.de&gt; Gerrit-Reviewer: pespin &lt;pespin(a)sysmocom.de&gt;