laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31069 ) Change subject: gprs_ns2_fr: use osmo_netdev to monitor and operate network device ...................................................................... gprs_ns2_fr: use osmo_netdev to monitor and operate network device As a result libosmogb doesn't depend directly on libmnl anymore, but through libosmocore. Change-Id: Ib0e499e09c50135a5c4a361332d6120f660a1a45 --- M src/core/netdev.c M src/gb/Makefile.am M src/gb/gprs_ns2_fr.c M src/gb/gprs_ns2_internal.h M tests/Makefile.am 5 files changed, 65 insertions(+), 141 deletions(-) Approvals: laforge: Looks good to me, approved daniel: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/core/netdev.c b/src/core/netdev.c index 28eafbd..e244ed1 100644 --- a/src/core/netdev.c +++ b/src/core/netdev.c @@ -470,6 +470,29 @@ return MNL_CB_OK; } +/* Trigger an initial dump of the iface to get link information */ +static int netdev_mnl_request_initial_dump(struct osmo_mnl *omnl, unsigned int if_index) +{ + char buf[MNL_SOCKET_BUFFER_SIZE]; + struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf); + struct ifinfomsg *ifm; + + nlh->nlmsg_type = RTM_GETLINK; + nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK; + nlh->nlmsg_seq = time(NULL); + + ifm = mnl_nlmsg_put_extra_header(nlh, sizeof(*ifm)); + ifm->ifi_family = AF_UNSPEC; + ifm->ifi_index = if_index; + + if (mnl_socket_sendto(omnl->mnls, nlh, nlh->nlmsg_len) < 0) { + LOGP(DLGLOBAL, LOGL_ERROR, "mnl_socket_sendto\n"); + return -EIO; + } + + return 0; +} + static int netdev_mnl_set_ifupdown(struct osmo_mnl *omnl, unsigned int if_index, const char *dev_name, bool up) { @@ -669,6 +692,10 @@ } osmo_talloc_replace_string(netdev, &netdev->dev_name, ifnamebuf); +#if ENABLE_LIBMNL + rc = netdev_mnl_request_initial_dump(netdev->netns_ctx->omnl, netdev->ifindex); +#endif + NETDEV_NETNS_EXIT(netdev, &switch_state, "register"); netdev->registered = true; diff --git a/src/gb/Makefile.am b/src/gb/Makefile.am index 1a26939..5bda374 100644 --- a/src/gb/Makefile.am +++ b/src/gb/Makefile.am @@ -6,7 +6,6 @@ AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include -I$(top_builddir) AM_CFLAGS = -Wall -fno-strict-aliasing \ $(TALLOC_CFLAGS) \ - $(LIBMNL_CFLAGS) \ $(NULL) # FIXME: this should eventually go into a milenage/Makefile.am diff --git a/src/gb/gprs_ns2_fr.c b/src/gb/gprs_ns2_fr.c index f85b1e4..d651601 100644 --- a/src/gb/gprs_ns2_fr.c +++ b/src/gb/gprs_ns2_fr.c @@ -55,13 +55,10 @@ #include <osmocom/core/timer.h> #include <osmocom/core/talloc.h> #include <osmocom/gprs/gprs_ns2.h> +#include <osmocom/core/netdev.h> #include <osmocom/gprs/protocol/gsm_08_16.h> #include <osmocom/gprs/protocol/gsm_08_18.h> -#ifdef ENABLE_LIBMNL -#include <osmocom/core/mnl.h> -#endif - #include "config.h" #include "common_vty.h" #include "gprs_ns2_internal.h" @@ -92,6 +89,7 @@ }; struct priv_bind { + struct osmo_netdev *netdev; char netif[IFNAMSIZ]; struct osmo_fr_link *link; int ifindex; @@ -172,6 +170,7 @@ } msgb_free(priv->backlog.lmi_msg); + osmo_netdev_free(priv->netdev); osmo_fr_link_free(priv->link); osmo_fd_close(&priv->backlog.ofd); talloc_free(priv); @@ -515,60 +514,14 @@ return fd; } -#ifdef ENABLE_LIBMNL - -#include <osmocom/core/mnl.h> -#include <linux/if_link.h> -#include <linux/rtnetlink.h> - -#ifndef ARPHRD_FRAD -#define ARPHRD_FRAD 770 -#endif - -/* validate the netlink attributes */ -static int data_attr_cb(const struct nlattr *attr, void *data) +static int gprs_n2_fr_ifupdown_ind_cb(struct osmo_netdev *netdev, bool if_running) { - const struct nlattr **tb = data; - int type = mnl_attr_get_type(attr); - - if (mnl_attr_type_valid(attr, IFLA_MAX) < 0) - return MNL_CB_OK; - - switch (type) { - case IFLA_MTU: - if (mnl_attr_validate(attr, MNL_TYPE_U32) < 0) - return MNL_CB_ERROR; - break; - case IFLA_IFNAME: - if (mnl_attr_validate(attr, MNL_TYPE_STRING) < 0) - return MNL_CB_ERROR; - break; - } - tb[type] = attr; - return MNL_CB_OK; -} - -/* find the bind for the netdev (if any) */ -static struct gprs_ns2_vc_bind *bind4netdev(struct gprs_ns2_inst *nsi, const char *ifname) -{ - struct gprs_ns2_vc_bind *bind; - - llist_for_each_entry(bind, &nsi->binding, list) { - struct priv_bind *bpriv = bind->priv; - if (!strcmp(bpriv->netif, ifname)) - return bind; - } - - return NULL; -} - -static void link_state_change(struct gprs_ns2_vc_bind *bind, bool if_running) -{ + struct gprs_ns2_vc_bind *bind = osmo_netdev_get_priv_data(netdev); struct priv_bind *bpriv = bind->priv; struct msgb *msg, *msg2; if (bpriv->if_running == if_running) - return; + return 0; LOGBIND(bind, LOGL_NOTICE, "FR net-device '%s': Physical link state changed: %s\n", bpriv->netif, if_running ? "UP" : "DOWN"); @@ -589,93 +542,36 @@ } bpriv->if_running = if_running; + return 0; } -static void mtu_change(struct gprs_ns2_vc_bind *bind, uint32_t mtu) +static int gprs_n2_fr_mtu_chg_cb(struct osmo_netdev *netdev, uint32_t new_mtu) { + struct gprs_ns2_vc_bind *bind = osmo_netdev_get_priv_data(netdev); struct priv_bind *bpriv = bind->priv; struct gprs_ns2_nse *nse; /* 2 byte DLCI header */ - if (mtu <= 2) - return; - mtu -= 2; + if (new_mtu <= 2) + return 0; + new_mtu -= 2; - if (mtu == bind->mtu) - return; + if (new_mtu == bind->mtu) + return 0; LOGBIND(bind, LOGL_INFO, "MTU changed from %d to %d.\n", - bind->mtu + 2, mtu + 2); + bind->mtu + 2, new_mtu + 2); - bind->mtu = mtu; + bind->mtu = new_mtu; if (!bpriv->if_running) - return; + return 0; llist_for_each_entry(nse, &bind->nsi->nse, list) { ns2_nse_update_mtu(nse); } + return 0; } -/* handle a single netlink message received via libmnl */ -static int linkmon_mnl_cb(const struct nlmsghdr *nlh, void *data) -{ - struct osmo_mnl *omnl = data; - struct gprs_ns2_vc_bind *bind; - struct nlattr *tb[IFLA_MAX+1] = {}; - struct ifinfomsg *ifm = mnl_nlmsg_get_payload(nlh); - struct gprs_ns2_inst *nsi; - const char *ifname; - bool if_running; - - OSMO_ASSERT(omnl); - OSMO_ASSERT(ifm); - - nsi = omnl->priv; - - if (ifm->ifi_type != ARPHRD_FRAD) - return MNL_CB_OK; - - mnl_attr_parse(nlh, sizeof(*ifm), data_attr_cb, tb); - - if (!tb[IFLA_IFNAME]) - return MNL_CB_OK; - ifname = mnl_attr_get_str(tb[IFLA_IFNAME]); - if_running = !!(ifm->ifi_flags & IFF_RUNNING); - - bind = bind4netdev(nsi, ifname); - if (!bind) - return MNL_CB_OK; - - if (tb[IFLA_MTU]) { - mtu_change(bind, mnl_attr_get_u32(tb[IFLA_MTU])); - } - - link_state_change(bind, if_running); - - return MNL_CB_OK; -} - -/* trigger one initial dump of all link information */ -static void linkmon_initial_dump(struct osmo_mnl *omnl) -{ - char buf[MNL_SOCKET_BUFFER_SIZE]; - struct nlmsghdr *nlh = mnl_nlmsg_put_header(buf); - struct rtgenmsg *rt; - - nlh->nlmsg_type = RTM_GETLINK; - nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP; - nlh->nlmsg_seq = time(NULL); - rt = mnl_nlmsg_put_extra_header(nlh, sizeof(struct rtgenmsg)); - rt->rtgen_family = AF_PACKET; - - if (mnl_socket_sendto(omnl->mnls, nlh, nlh->nlmsg_len) < 0) { - LOGP(DLNS, LOGL_ERROR, "linkmon: Cannot send rtnetlink message: %s\n", strerror(errno)); - } - - /* the response[s] will be handled just like the events */ -} -#endif /* LIBMNL */ - static int set_ifupdown(const char *netif, bool up) { int sock, rc; @@ -854,16 +750,31 @@ goto err_fr; } + priv->netdev = osmo_netdev_alloc(bind, name); + if (!priv->netdev) { + rc = -ENOENT; + goto err_fr; + } + osmo_netdev_set_priv_data(priv->netdev, bind); + osmo_netdev_set_ifupdown_ind_cb(priv->netdev, gprs_n2_fr_ifupdown_ind_cb); + osmo_netdev_set_mtu_chg_cb(priv->netdev, gprs_n2_fr_mtu_chg_cb); + rc = osmo_netdev_set_ifindex(priv->netdev, priv->ifindex); + if (rc < 0) + goto err_free_netdev; + rc = osmo_netdev_register(priv->netdev); + if (rc < 0) + goto err_free_netdev; + /* set protocol frame relay and lmi */ rc = setup_device(priv->netif, bind); if(rc < 0) { LOGBIND(bind, LOGL_ERROR, "Failed to setup the interface %s for frame relay and lmi\n", netif); - goto err_fr; + goto err_free_netdev; } rc = open_socket(priv->ifindex, bind); if (rc < 0) - goto err_fr; + goto err_free_netdev; priv->backlog.retry_us = 2500; /* start with some non-zero value; this corrsponds to 496 bytes */ osmo_timer_setup(&priv->backlog.timer, fr_backlog_timer_cb, bind); osmo_fd_setup(&priv->backlog.ofd, rc, OSMO_FD_READ, fr_netif_ofd_cb, bind, 0); @@ -871,16 +782,6 @@ if (rc < 0) goto err_fd; -#ifdef ENABLE_LIBMNL - if (!nsi->linkmon_mnl) - nsi->linkmon_mnl = osmo_mnl_init(nsi, NETLINK_ROUTE, RTMGRP_LINK, linkmon_mnl_cb, nsi); - - /* we get a new full dump after every bind. which is a bit excessive. But that's just once - * at start-up, so we can get away with it */ - if (nsi->linkmon_mnl) - linkmon_initial_dump(nsi->linkmon_mnl); -#endif - if (result) *result = bind; @@ -888,6 +789,9 @@ err_fd: close(priv->backlog.ofd.fd); +err_free_netdev: + osmo_netdev_free(priv->netdev); + priv->netdev = NULL; err_fr: osmo_fr_link_free(fr_link); priv->link = NULL; diff --git a/src/gb/gprs_ns2_internal.h b/src/gb/gprs_ns2_internal.h index 93aaa99..4f33221 100644 --- a/src/gb/gprs_ns2_internal.h +++ b/src/gb/gprs_ns2_internal.h @@ -163,9 +163,6 @@ /*! workaround for rate counter until rate counter accepts char str as index */ uint32_t nsvc_rate_ctr_idx; uint32_t bind_rate_ctr_idx; - - /*! libmnl netlink socket for link state monitoring */ - struct osmo_mnl *linkmon_mnl; }; diff --git a/tests/Makefile.am b/tests/Makefile.am index a28aa7f..c637c16 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -211,9 +211,6 @@ $(top_builddir)/src/gsm/libosmogsm.la \ $(top_builddir)/src/core/libosmocore.la \ $(top_builddir)/src/gb/libosmogb-test.la -if ENABLE_LIBMNL -gb_gprs_ns2_test_LDADD += $(LIBMNL_LIBS) -endif logging_logging_test_SOURCES = logging/logging_test.c -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31069 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ib0e499e09c50135a5c4a361332d6120f660a1a45 Gerrit-Change-Number: 31069 Gerrit-PatchSet: 6 Gerrit-Owner: pespin <pespin@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> Gerrit-Reviewer: laforge <laforge@osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis@fe80.eu> Gerrit-MessageType: merged