pespin has submitted this change. (
https://gerrit.osmocom.org/c/osmo-ggsn/+/38524?usp=email )
Change subject: Refactor tun_t fields and alloc APIs
......................................................................
Refactor tun_t fields and alloc APIs
The previous state of code made allocation code more complex for no good
reason; use usual alloc() type function instead, splitting between the 2
types of tun_t implementations available to make it easier to
understand.
Move the several tun-iface specific mode fields to a substruct to make
it clear those are only really containing useful information when the
tun_t is created in tun-iface mode.
Change-Id: Ic71f91c62cd5bd48c6d35534eaac2091e4c69735
---
M ggsn/ggsn.c
M lib/tun.c
M lib/tun.h
M sgsnemu/sgsnemu.c
4 files changed, 109 insertions(+), 84 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
osmith: Looks good to me, but someone else must approve
fixeria: Looks good to me, but someone else must approve
diff --git a/ggsn/ggsn.c b/ggsn/ggsn.c
index 062e542..329b15f 100644
--- a/ggsn/ggsn.c
+++ b/ggsn/ggsn.c
@@ -226,7 +226,8 @@
switch (apn->cfg.gtpu_mode) {
case APN_GTPU_MODE_TUN:
LOGPAPN(LOGL_INFO, apn, "Opening TUN device %s\n",
apn->tun.cfg.dev_name);
- if (tun_new(&apn->tun.tun, apn->tun.cfg.dev_name, false, -1,
-1)) {
+ apn->tun.tun = tun_alloc_tundev(apn->tun.cfg.dev_name);
+ if (!apn->tun.tun) {
LOGPAPN(LOGL_ERROR, apn, "Failed to configure tun
device\n");
return -1;
}
@@ -246,7 +247,8 @@
return 0;
}
/* use GTP kernel module for data packet encapsulation */
- if (tun_new(&apn->tun.tun, apn->tun.cfg.dev_name, true,
gsn->fd0, gsn->fd1u)) {
+ apn->tun.tun = tun_alloc_gtpdev(apn->tun.cfg.dev_name,
gsn->fd0, gsn->fd1u);
+ if (!apn->tun.tun) {
LOGPAPN(LOGL_ERROR, apn, "Failed to configure Kernel
GTP device\n");
return -1;
}
diff --git a/lib/tun.c b/lib/tun.c
index 3cce3a5..89fe292 100644
--- a/lib/tun.c
+++ b/lib/tun.c
@@ -93,102 +93,118 @@
return rc;
}
-int tun_new(struct tun_t **tun, const char *dev_name, bool use_kernel, int
fd0, int fd1u)
+static struct tun_t *tun_alloc_common(const char *devname)
{
- struct tun_t *t;
+ struct tun_t *tun;
+
+ tun = talloc_zero(NULL, struct tun_t);
+ if (!tun) {
+ LOGP(DTUN, LOGL_ERROR, "tun_alloc_common() failed\n");
+ return NULL;
+ }
+
+ tun->cb_ind = NULL;
+ tun->addrs = 0;
+ tun->tundev.fd = -1;
+
+ osmo_strlcpy(tun->devname, devname, IFNAMSIZ);
+ tun->devname[IFNAMSIZ - 1] = 0;
+
+ return tun;
+}
+
+struct tun_t *tun_alloc_tundev(const char *devname)
+{
+ struct tun_t *tun;
int rc;
- t = talloc_zero(NULL, struct tun_t);
- if (!t) {
- SYS_ERR(DTUN, LOGL_ERROR, errno, "talloc_zero() failed");
- return EOF;
+ tun = tun_alloc_common(devname);
+ if (!tun)
+ return NULL;
+
+ tun->tundev.tundev = osmo_tundev_alloc(tun, tun->devname);
+ if (!tun->tundev.tundev)
+ goto err_free;
+ osmo_tundev_set_priv_data(tun->tundev.tundev, tun);
+ osmo_tundev_set_data_ind_cb(tun->tundev.tundev, tun_tundev_data_ind_cb);
+ rc = osmo_tundev_set_dev_name(tun->tundev.tundev, tun->devname);
+ if (rc < 0)
+ goto err_free_tundev;
+
+ /* Open the actual tun device */
+ rc = osmo_tundev_open(tun->tundev.tundev);
+ if (rc < 0)
+ goto err_free_tundev;
+ tun->tundev.fd = osmo_tundev_get_fd(tun->tundev.tundev);
+ tun->netdev = osmo_tundev_get_netdev(tun->tundev.tundev);
+
+ /* Disable checksums */
+ if (ioctl(tun->tundev.fd, TUNSETNOCSUM, 1) < 0) {
+ SYS_ERR(DTUN, LOGL_NOTICE, errno, "could not disable checksum
on %s", tun->devname);
}
- *tun = t;
- t->cb_ind = NULL;
- t->addrs = 0;
- t->fd = -1;
+ LOGP(DTUN, LOGL_NOTICE, "tun %s configured\n", tun->devname);
+ return tun;
- if (!use_kernel) {
- 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;
-
- /* 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(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);
+ osmo_tundev_free(tun->tundev.tundev);
err_free:
- talloc_free(t);
- *tun = NULL;
- return -1;
+ talloc_free(tun);
+ return NULL;
+}
- } else {
- osmo_strlcpy(t->devname, dev_name, IFNAMSIZ);
- t->devname[IFNAMSIZ - 1] = 0;
+struct tun_t *tun_alloc_gtpdev(const char *devname, int fd0, int fd1u)
+{
+ struct tun_t *tun;
+ int rc;
- 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;
-err_netdev_free:
- osmo_netdev_free(t->netdev);
-err_kernel_create:
- gtp_kernel_stop(t->devname);
- return -1;
+ tun = tun_alloc_common(devname);
+ if (!tun)
+ return NULL;
+
+ if (gtp_kernel_create(-1, tun->devname, fd0, fd1u) < 0) {
+ LOGP(DTUN, LOGL_ERROR, "cannot create GTP tunnel device: %s\n",
+ strerror(errno));
+ goto err_free;
}
+ tun->netdev = osmo_netdev_alloc(tun, tun->devname);
+ if (!tun->netdev)
+ goto err_kernel_create;
+ rc = osmo_netdev_set_ifindex(tun->netdev, if_nametoindex(tun->devname));
+ if (rc < 0)
+ goto err_netdev_free;
+ rc = osmo_netdev_register(tun->netdev);
+ if (rc < 0)
+ goto err_netdev_free;
+ LOGP(DTUN, LOGL_NOTICE, "GTP kernel configured\n");
+ return tun;
+
+err_netdev_free:
+ osmo_netdev_free(tun->netdev);
+err_kernel_create:
+ gtp_kernel_stop(tun->devname);
+err_free:
+ talloc_free(tun);
+ return NULL;
}
int tun_free(struct tun_t *tun)
{
- if (tun->tundev) {
- if (osmo_tundev_close(tun->tundev) < 0) {
- SYS_ERR(DTUN, LOGL_ERROR, errno, "close() failed");
+ if (tun->tundev.tundev) {
+ if (osmo_tundev_close(tun->tundev.tundev) < 0) {
+ SYS_ERR(DTUN, LOGL_ERROR, errno, "osmo_tundev_close()
failed");
}
- osmo_tundev_free(tun->tundev);
- tun->tundev = NULL;
+ osmo_tundev_free(tun->tundev.tundev);
+ tun->tundev.tundev = NULL;
/* netdev is owned by tundev: */
tun->netdev = NULL;
} else {
+ gtp_kernel_stop(tun->devname);
/* netdev was allocated directly, free it: */
osmo_netdev_free(tun->netdev);
tun->netdev = NULL;
}
- gtp_kernel_stop(tun->devname);
-
talloc_free(tun);
return 0;
}
@@ -205,7 +221,7 @@
struct msgb *msg;
int rc;
- if (!tun->tundev) {
+ if (!tun->tundev.tundev) {
LOGTUN(LOGL_ERROR, tun,
"Injecting decapsulated packet not supported in kernel
gtp mode: %s\n",
osmo_hexdump(pack, len));
@@ -215,7 +231,7 @@
msg = msgb_alloc(PACKET_MAX, "tun_tx");
OSMO_ASSERT(msg);
memcpy(msgb_put(msg, len), pack, len);
- rc = osmo_tundev_send(tun->tundev, msg);
+ rc = osmo_tundev_send(tun->tundev.tundev, msg);
if (rc < 0) {
SYS_ERR(DTUN, LOGL_ERROR, errno, "TUN(%s): write() failed",
tun->devname);
}
diff --git a/lib/tun.h b/lib/tun.h
index 4d41eb8..eb1bee8 100644
--- a/lib/tun.h
+++ b/lib/tun.h
@@ -33,9 +33,9 @@
*************************************************************/
struct tun_t {
- struct osmo_tundev *tundev;
+ /* In tun device mode: operates on the tun interface, owned by
tun.tundev below.
+ * In gtp kernel mode: operates on the gtp device, allocated explicitly
*/
struct osmo_netdev *netdev;
- int fd; /* File descriptor to tun interface */
struct in46_addr addr;
struct in_addr netmask;
int addrs; /* Number of allocated IP addresses */
@@ -43,9 +43,15 @@
int (*cb_ind) (struct tun_t * tun, void *pack, unsigned len);
/* to be used by libgtp callers/users (to attach their own private
state) */
void *priv;
+ /* Fields only in use when using the tun device mode: */
+ struct {
+ struct osmo_tundev *tundev; /* Manages the tun interface; NULL
on gtp kernel mode */
+ int fd; /* File descriptor to tun interface; -1 on gtp kernel
mode */
+ } tundev;
};
-extern int tun_new(struct tun_t **tun, const char *dev_name, bool use_kernel,
int fd0, int fd1u);
+extern struct tun_t *tun_alloc_tundev(const char *devname);
+extern struct tun_t *tun_alloc_gtpdev(const char *devname, int fd0, int fd1u);
extern int tun_free(struct tun_t *tun);
extern int tun_inject_pkt(struct tun_t *tun, void *pack, unsigned len);
diff --git a/sgsnemu/sgsnemu.c b/sgsnemu/sgsnemu.c
index d195fab..afa7339 100644
--- a/sgsnemu/sgsnemu.c
+++ b/sgsnemu/sgsnemu.c
@@ -1905,7 +1905,8 @@
if (options.createif) {
printf("Setting up interface\n");
/* Create a tunnel interface */
- if (tun_new((struct tun_t **)&tun, options.tun_dev_name, false,
-1, -1)) {
+ tun = tun_alloc_tundev(options.tun_dev_name);
+ if (!tun) {
SYS_ERR(DSGSN, LOGL_ERROR, 0,
"Failed to create tun");
exit(1);
@@ -1925,8 +1926,8 @@
#endif
tun_set_cb_ind(tun, cb_tun_ind);
- if (tun->fd > maxfd)
- maxfd = tun->fd;
+ if (tun->tundev.fd > maxfd)
+ maxfd = tun->tundev.fd;
if (proc_ipv6_conf_write(tun->devname, "accept_ra", "0") < 0) {
SYS_ERR(DSGSN, LOGL_ERROR, 0,
@@ -2159,7 +2160,7 @@
FD_ZERO(&fds);
if (tun)
- FD_SET(tun->fd, &fds);
+ FD_SET(tun->tundev.fd, &fds);
FD_SET(gsn->fd0, &fds);
FD_SET(gsn->fd1c, &fds);
FD_SET(gsn->fd1u, &fds);
@@ -2187,7 +2188,7 @@
if (!signal_received) {
- if ((tun) && FD_ISSET(tun->fd, &fds)) {
+ if ((tun) && FD_ISSET(tun->tundev.fd, &fds)) {
osmo_select_main(1);
}
--
To view, visit https://gerrit.osmocom.org/c/osmo-ggsn/+/38524?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: Ic71f91c62cd5bd48c6d35534eaac2091e4c69735
Gerrit-Change-Number: 38524
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: osmith <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>
