Patch implements link layer state awareness (aka carrier detection) in netifd; 
an
interface will only go to the setup state (and send a setup event to the 
protocol 
handlers) when it's enabled and the link state is enabled. Vice versa an 
interface 
will go the down state when either it's disabled or the link goes down; in this 
case 
a teardown will be send to the protocol handlers.
It solves the issue that a DHCPv4/v6 client remains active on a WAN link which 
loses
its link connectivity; also a DHCPv4/v6 client will only be started on a
WAN link when the link layer is up and the interface is enabled.
Patch also fixes some issues with handling of netlink messages as the original 
implementation was edge triggered.
Testing has been done with PPP, IPoE, tunnel, static interfaces.

Signed-off-by: Hans Dedecker <dedec...@gmail.com>
---
 alias.c        |   16 +++++--
 device.c       |   29 ++++++++++++-
 device.h       |    4 ++
 interface.c    |  130 +++++++++++++++++++++++++++++++++++++++++++-------------
 interface.h    |    2 +
 macvlan.c      |    6 +++
 proto-shell.c  |    1 +
 system-linux.c |  117 +++++++++++++++++++++++++++++++++++++++++---------
 vlan.c         |   13 ++++-
 9 files changed, 258 insertions(+), 60 deletions(-)

diff --git a/alias.c b/alias.c
index 4e0a6be..cef125f 100644
--- a/alias.c
+++ b/alias.c
@@ -51,7 +51,7 @@ static void alias_set_device(struct alias_device *alias, 
struct device *dev)
        device_remove_user(&alias->dep);
        alias->dev.hidden = !dev;
        if (dev) {
-               alias->dev.ifindex = dev->ifindex;
+               device_set_ifindex(&alias->dev, dev->ifindex);
                strcpy(alias->dev.ifname, dev->ifname);
                device_broadcast_event(&alias->dev, DEV_EVENT_UPDATE_IFNAME);
                device_add_user(&alias->dep, dev);
@@ -83,14 +83,22 @@ alias_device_set_state(struct device *dev, bool state)
 static void alias_device_cb(struct device_user *dep, enum device_event ev)
 {
        struct alias_device *alias;
-       bool present = false;
+       bool new_state = false;
 
        alias = container_of(dep, struct alias_device, dep);
        switch (ev) {
        case DEV_EVENT_ADD:
-               present = true;
+               new_state = true;
        case DEV_EVENT_REMOVE:
-               device_set_present(&alias->dev, present);
+               device_set_present(&alias->dev, new_state);
+               break;
+       case DEV_EVENT_LINK_UP:
+               new_state = true;
+       case DEV_EVENT_LINK_DOWN:
+               device_set_link(&alias->dev, new_state);
+               break;
+       case DEV_EVENT_UPDATE_IFINDEX:
+               device_set_ifindex(&alias->dev, dep->dev->ifindex);
                break;
        default:
                device_broadcast_event(&alias->dev, ev);
diff --git a/device.c b/device.c
index 56fc3f7..f8834f0 100644
--- a/device.c
+++ b/device.c
@@ -207,7 +207,7 @@ int device_claim(struct device_user *dep)
                return 0;
 
        dep->claimed = true;
-       D(DEVICE, "Claim %s %s, new refcount: %d\n", dev->type->name, 
dev->ifname, dev->active + 1);
+       D(DEVICE, "Claim %s %s, new active count: %d\n", dev->type->name, 
dev->ifname, dev->active + 1);
        if (++dev->active != 1)
                return 0;
 
@@ -233,7 +233,7 @@ void device_release(struct device_user *dep)
 
        dep->claimed = false;
        dev->active--;
-       D(DEVICE, "Release %s %s, new refcount: %d\n", dev->type->name, 
dev->ifname, dev->active);
+       D(DEVICE, "Release %s %s, new active count: %d\n", dev->type->name, 
dev->ifname, dev->active);
        assert(dev->active >= 0);
 
        if (dev->active)
@@ -390,6 +390,26 @@ void device_set_present(struct device *dev, bool state)
        device_refresh_present(dev);
 }
 
+void device_set_link(struct device *dev, bool state)
+{
+       if (dev->link_active == state)
+               return;
+
+       netifd_log_message(L_NOTICE, "%s '%s' link is %s\n", dev->type->name, 
dev->ifname, state ? "up" : "down" );
+
+       dev->link_active = state;
+       device_broadcast_event(dev, state ? DEV_EVENT_LINK_UP : 
DEV_EVENT_LINK_DOWN);
+}
+
+void device_set_ifindex(struct device *dev, int ifindex)
+{
+       if (dev->ifindex == ifindex)
+               return;
+
+       dev->ifindex = ifindex;
+       device_broadcast_event(dev, DEV_EVENT_UPDATE_IFINDEX);
+}
+
 static int device_refcount(struct device *dev)
 {
        struct list_head *list;
@@ -431,6 +451,9 @@ void device_add_user(struct device_user *dep, struct device 
*dev)
                dep->cb(dep, DEV_EVENT_ADD);
                if (dev->active)
                        dep->cb(dep, DEV_EVENT_UP);
+
+               if (dev->link_active)
+                       dep->cb(dep, DEV_EVENT_LINK_UP);
        }
 }
 
@@ -663,6 +686,8 @@ device_dump_status(struct blob_buf *b, struct device *dev)
                return;
 
        blobmsg_add_u8(b, "up", !!dev->active);
+       blobmsg_add_u8(b, "carrier", !!dev->link_active);
+
        if (dev->type->dump_info)
                dev->type->dump_info(dev, b);
        else
diff --git a/device.h b/device.h
index d63ffe3..dd57927 100644
--- a/device.h
+++ b/device.h
@@ -68,6 +68,7 @@ enum device_event {
        DEV_EVENT_REMOVE,
 
        DEV_EVENT_UPDATE_IFNAME,
+       DEV_EVENT_UPDATE_IFINDEX,
 
        DEV_EVENT_SETUP,
        DEV_EVENT_TEARDOWN,
@@ -122,6 +123,7 @@ struct device {
        bool sys_present;
        bool present;
        int active;
+       bool link_active;
 
        bool external;
        bool disabled;
@@ -178,6 +180,8 @@ void device_remove_user(struct device_user *dep);
 void device_broadcast_event(struct device *dev, enum device_event ev);
 
 void device_set_present(struct device *dev, bool state);
+void device_set_link(struct device *dev, bool state);
+void device_set_ifindex(struct device *dev, int ifindex);
 void device_refresh_present(struct device *dev);
 int device_claim(struct device_user *dep);
 void device_release(struct device_user *dep);
diff --git a/interface.c b/interface.c
index 9c208a2..4b381a4 100644
--- a/interface.c
+++ b/interface.c
@@ -200,6 +200,9 @@ mark_interface_down(struct interface *iface)
 {
        enum interface_state state = iface->state;
 
+       if (state == IFS_DOWN)
+               return;
+
        iface->state = IFS_DOWN;
        if (state == IFS_UP)
                interface_event(iface, IFEV_DOWN);
@@ -212,42 +215,112 @@ mark_interface_down(struct interface *iface)
 void
 __interface_set_down(struct interface *iface, bool force)
 {
-       if (iface->state == IFS_DOWN ||
-               iface->state == IFS_TEARDOWN)
+       switch (iface->state) {
+       case IFS_UP:
+               interface_event(iface, IFEV_DOWN);
+       case IFS_SETUP:
+               iface->state = IFS_TEARDOWN;
+               interface_proto_event(iface->proto, PROTO_CMD_TEARDOWN, force);
+               if (force)
+                       interface_flush_state(iface);
+
+               if (iface->dynamic)
+                       vlist_delete(&interfaces, &iface->node);
+               break;
+
+       case IFS_DOWN:
+               if (iface->main_dev.dev)
+                       device_release(&iface->main_dev);
+       case IFS_TEARDOWN:
+       default:
+               break;
+       }
+}
+
+static int
+__interface_set_up(struct interface *iface)
+{
+       int ret;
+
+       netifd_log_message(L_NOTICE, "Interface '%s' is setting up now\n", 
iface->name);
+
+       iface->state = IFS_SETUP;
+       ret = interface_proto_event(iface->proto, PROTO_CMD_SETUP, false);
+       if (ret)
+               mark_interface_down(iface);
+
+       return ret;
+}
+
+static void
+interface_check_state(struct interface *iface)
+{
+       switch (iface->state) {
+       case IFS_UP:
+               if (!iface->enabled || !iface->link_state) {
+                       mark_interface_down(iface);
+                       interface_proto_event(iface->proto, PROTO_CMD_TEARDOWN, 
false);
+               }
+               break;
+       case IFS_DOWN:
+               if (iface->enabled && iface->link_state && !config_init)
+                       __interface_set_up(iface);
+               break;
+       default:
+               break;
+       }
+}
+
+static void
+interface_set_enabled(struct interface *iface, bool new_state)
+{
+       if (iface->enabled == new_state)
                return;
 
-       if (iface->state == IFS_UP)
-               interface_event(iface, IFEV_DOWN);
-       iface->state = IFS_TEARDOWN;
-       interface_proto_event(iface->proto, PROTO_CMD_TEARDOWN, force);
-       if (force)
-               interface_flush_state(iface);
+       netifd_log_message(L_NOTICE, "Interface '%s' is %s\n", iface->name, 
new_state ? "enabled" : "disabled");
+       iface->enabled = new_state;
+       interface_check_state(iface);
+}
 
-       if (iface->dynamic)
-               vlist_delete(&interfaces, &iface->node);
+static void
+interface_set_link_state(struct interface *iface, bool new_state)
+{
+       if (iface->link_state == new_state)
+               return;
+
+       netifd_log_message(L_NOTICE, "Interface '%s' has link connectivity 
%s\n", iface->name, new_state ? "" : "loss");
+       iface->link_state = new_state;
+       interface_check_state(iface);
 }
 
 static void
 interface_cb(struct device_user *dep, enum device_event ev)
 {
        struct interface *iface;
-       bool new_state;
+       bool new_state = false;
 
        iface = container_of(dep, struct interface, main_dev);
        switch (ev) {
        case DEV_EVENT_ADD:
                new_state = true;
-               break;
        case DEV_EVENT_REMOVE:
-               new_state = false;
+               interface_set_available(iface, new_state);
+               if (!new_state && dep->dev->external)
+                       interface_set_main_dev(iface, NULL);
+               break;
+       case DEV_EVENT_UP:
+               new_state = true;
+       case DEV_EVENT_DOWN:
+               interface_set_enabled(iface, new_state);
+               break;
+       case DEV_EVENT_LINK_UP:
+               new_state = true;
+        case DEV_EVENT_LINK_DOWN:
+               interface_set_link_state(iface, new_state);
                break;
        default:
-               return;
+               break;
        }
-
-       interface_set_available(iface, new_state);
-       if (!new_state && dep->dev->external)
-               interface_set_main_dev(iface, NULL);
 }
 
 void
@@ -684,7 +757,7 @@ interface_set_l3_dev(struct interface *iface, struct device 
*dev)
 void
 interface_set_main_dev(struct interface *iface, struct device *dev)
 {
-       bool set_l3 = (iface->main_dev.dev == iface->l3_dev.dev);
+       bool set_l3 = (!dev || iface->main_dev.dev == iface->l3_dev.dev);
        bool claimed = iface->l3_dev.claimed;
 
        if (iface->main_dev.dev == dev)
@@ -694,8 +767,10 @@ interface_set_main_dev(struct interface *iface, struct 
device *dev)
                interface_set_l3_dev(iface, dev);
 
        device_add_user(&iface->main_dev, dev);
-       if (!dev)
+       if (!dev) {
+               interface_set_link_state(iface, false);
                return;
+       }
 
        if (claimed)
                device_claim(&iface->l3_dev);
@@ -794,18 +869,13 @@ interface_set_up(struct interface *iface)
 
        if (iface->main_dev.dev) {
                ret = device_claim(&iface->main_dev);
-               if (ret)
-                       return ret;
-       }
-
-       iface->state = IFS_SETUP;
-       ret = interface_proto_event(iface->proto, PROTO_CMD_SETUP, false);
-       if (ret) {
-               mark_interface_down(iface);
-               return ret;
+               if (!ret)
+                       interface_check_state(iface);
        }
+       else
+               ret = __interface_set_up(iface);
 
-       return 0;
+       return ret;
 }
 
 int
diff --git a/interface.h b/interface.h
index 4b7de59..1004bdd 100644
--- a/interface.h
+++ b/interface.h
@@ -95,6 +95,8 @@ struct interface {
        bool autostart;
        bool config_autostart;
        bool device_config;
+       bool enabled;
+       bool link_state;
        bool dynamic;
 
        time_t start_time;
diff --git a/macvlan.c b/macvlan.c
index 28567dc..9c03cd8 100644
--- a/macvlan.c
+++ b/macvlan.c
@@ -69,6 +69,12 @@ macvlan_base_cb(struct device_user *dev, enum device_event 
ev)
        case DEV_EVENT_REMOVE:
                device_set_present(&mvdev->dev, false);
                break;
+       case DEV_EVENT_LINK_UP:
+               device_set_link(&mvdev->dev, true);
+               break;
+       case DEV_EVENT_LINK_DOWN:
+               device_set_link(&mvdev->dev, false);
+               break;
        default:
                return;
        }
diff --git a/proto-shell.c b/proto-shell.c
index 6bbfe10..27fe265 100644
--- a/proto-shell.c
+++ b/proto-shell.c
@@ -158,6 +158,7 @@ proto_shell_handler(struct interface_proto_state *proto,
                action = "setup";
                state->last_error = -1;
                proto_shell_clear_host_dep(state);
+               state->sm = S_SETUP;
        } else {
                if (state->sm == S_TEARDOWN)
                        return 0;
diff --git a/system-linux.c b/system-linux.c
index dae98b0..714729f 100644
--- a/system-linux.c
+++ b/system-linux.c
@@ -58,7 +58,6 @@
 struct event_socket {
        struct uloop_fd uloop;
        struct nl_sock *sock;
-       struct nl_cb *cb;
 };
 
 static int sock_ioctl = -1;
@@ -73,7 +72,7 @@ static void
 handler_nl_event(struct uloop_fd *u, unsigned int events)
 {
        struct event_socket *ev = container_of(u, struct event_socket, uloop);
-       nl_recvmsgs(ev->sock, ev->cb);
+       nl_recvmsgs_default(ev->sock);
 }
 
 static struct nl_sock *
@@ -104,7 +103,9 @@ create_raw_event_socket(struct event_socket *ev, int 
protocol, int groups,
 
        ev->uloop.fd = nl_socket_get_fd(ev->sock);
        ev->uloop.cb = cb;
-       uloop_fd_add(&ev->uloop, ULOOP_READ | ULOOP_EDGE_TRIGGER);
+       if (uloop_fd_add(&ev->uloop, ULOOP_READ))
+               return false;
+
        return true;
 }
 
@@ -112,14 +113,20 @@ static bool
 create_event_socket(struct event_socket *ev, int protocol,
                    int (*cb)(struct nl_msg *msg, void *arg))
 {
-       // Prepare socket for link events
-       ev->cb = nl_cb_alloc(NL_CB_DEFAULT);
-       if (!ev->cb)
+       if (!create_raw_event_socket(ev, protocol, 0, handler_nl_event))
                return false;
 
-       nl_cb_set(ev->cb, NL_CB_VALID, NL_CB_CUSTOM, cb, NULL);
+       // Install the valid custom callback handler
+       nl_socket_modify_cb(ev->sock, NL_CB_VALID, NL_CB_CUSTOM, cb, NULL);
 
-       return create_raw_event_socket(ev, protocol, 0, handler_nl_event);
+       // Disable sequence number checking on event sockets
+       nl_socket_disable_seq_check(ev->sock);
+
+       // Increase rx buffer size to 65K on event sockets
+       if (nl_socket_set_buffer_size(ev->sock, 65535, 0))
+               return false;
+
+       return true;
 }
 
 int system_init(void)
@@ -128,7 +135,7 @@ int system_init(void)
        static struct event_socket hotplug_event;
 
        sock_ioctl = socket(AF_LOCAL, SOCK_DGRAM, 0);
-       fcntl(sock_ioctl, F_SETFD, fcntl(sock_ioctl, F_GETFD) | FD_CLOEXEC);
+       system_fd_set_cloexec(sock_ioctl);
 
        // Prepare socket for routing / address control
        sock_rtnl = create_socket(NETLINK_ROUTE, 0);
@@ -171,6 +178,10 @@ static void system_set_disable_ipv6(struct device *dev, 
const char *val)
        system_set_dev_sysctl("/proc/sys/net/ipv6/conf/%s/disable_ipv6", 
dev->ifname, val);
 }
 
+#ifndef IFF_LOWER_UP
+#define IFF_LOWER_UP   0x10000
+#endif
+
 // Evaluate netlink messages
 static int cb_rtnl_event(struct nl_msg *msg, void *arg)
 {
@@ -178,19 +189,19 @@ static int cb_rtnl_event(struct nl_msg *msg, void *arg)
        struct ifinfomsg *ifi = NLMSG_DATA(nh);
        struct nlattr *nla[__IFLA_MAX];
 
-       if (nh->nlmsg_type != RTM_DELLINK && nh->nlmsg_type != RTM_NEWLINK)
+       if (nh->nlmsg_type != RTM_NEWLINK)
                goto out;
 
        nlmsg_parse(nh, sizeof(*ifi), nla, __IFLA_MAX - 1, NULL);
        if (!nla[IFLA_IFNAME])
                goto out;
 
-       struct device *dev = device_get(RTA_DATA(nla[IFLA_IFNAME]), false);
+       struct device *dev = device_get(nla_data(nla[IFLA_IFNAME]), false);
        if (!dev)
                goto out;
 
-       dev->ifindex = ifi->ifi_index;
-       /* TODO: parse link status */
+       device_set_ifindex(dev, ifi->ifi_index);
+       device_set_link(dev, ifi->ifi_flags & IFF_LOWER_UP ? true : false);
 
 out:
        return 0;
@@ -537,7 +548,7 @@ void system_if_clear_state(struct device *dev)
        if (dev->external)
                return;
 
-       dev->ifindex = system_if_resolve(dev);
+       device_set_ifindex(dev, system_if_resolve(dev));
        if (!dev->ifindex)
                return;
 
@@ -781,7 +792,7 @@ int system_if_up(struct device *dev)
 {
        system_if_get_settings(dev, &dev->orig_settings);
        system_if_apply_settings(dev, &dev->settings);
-       dev->ifindex = system_if_resolve(dev);
+       device_set_ifindex(dev, system_if_resolve(dev));
        return system_if_flags(dev->ifname, IFF_UP, 0);
 }
 
@@ -793,10 +804,78 @@ int system_if_down(struct device *dev)
        return ret;
 }
 
+struct if_check_data {
+       struct device *dev;
+       int pending;
+       int ret;
+};
+
+static int cb_if_check_valid(struct nl_msg *msg, void *arg)
+{
+       struct nlmsghdr *nh = nlmsg_hdr(msg);
+       struct ifinfomsg *ifi = NLMSG_DATA(nh);
+       struct if_check_data *chk = (struct if_check_data *)arg;
+
+       if (nh->nlmsg_type != RTM_NEWLINK)
+               return NL_SKIP;
+
+       device_set_present(chk->dev, ifi->ifi_index > 0 ? true : false);
+       device_set_link(chk->dev, ifi->ifi_flags & IFF_LOWER_UP ? true : false);
+
+       return NL_OK;
+}
+
+static int cb_if_check_ack(struct nl_msg *msg, void *arg)
+{
+       struct if_check_data *chk = (struct if_check_data *)arg;
+       chk->pending = 0;
+       return NL_STOP;
+}
+
+static int cb_if_check_error(struct sockaddr_nl *nla, struct nlmsgerr *err, 
void *arg)
+{
+       struct if_check_data *chk = (struct if_check_data *)arg;
+
+       device_set_present(chk->dev, false);
+       device_set_link(chk->dev, false);
+       chk->pending = err->error;
+
+       return NL_STOP;
+}
+
 int system_if_check(struct device *dev)
 {
-       device_set_present(dev, (system_if_resolve(dev) > 0));
-       return 0;
+       struct nl_cb *cb = nl_cb_alloc(NL_CB_DEFAULT);
+       struct nl_msg *msg;
+       struct ifinfomsg ifi = {
+               .ifi_family = AF_UNSPEC,
+               .ifi_index = 0,
+       };
+       struct if_check_data chk = {
+               .dev = dev,
+               .pending = 1,
+       };
+       int ret = 1;
+
+       msg = nlmsg_alloc_simple(RTM_GETLINK, 0);
+       if (!msg || nlmsg_append(msg, &ifi, sizeof(ifi), 0) ||
+           nla_put_string(msg, IFLA_IFNAME, dev->ifname))
+               goto out;
+
+       nl_cb_set(cb, NL_CB_VALID, NL_CB_CUSTOM, cb_if_check_valid, &chk);
+       nl_cb_set(cb, NL_CB_ACK, NL_CB_CUSTOM, cb_if_check_ack, &chk);
+       nl_cb_err(cb, NL_CB_CUSTOM, cb_if_check_error, &chk);
+
+       nl_send_auto_complete(sock_rtnl, msg);
+       while (chk.pending > 0)
+               nl_recvmsgs(sock_rtnl, cb);
+
+       nlmsg_free(msg);
+       ret = chk.pending;
+
+out:
+       nl_cb_put(cb);
+       return ret;
 }
 
 struct device *
@@ -914,14 +993,10 @@ system_if_dump_info(struct device *dev, struct blob_buf 
*b)
        char buf[64], *s;
        void *c;
        int dir_fd;
-       uint64_t val = 0;
 
        snprintf(buf, sizeof(buf), "/sys/class/net/%s", dev->ifname);
        dir_fd = open(buf, O_DIRECTORY);
 
-       if (read_uint64_file(dir_fd, "carrier", &val))
-               blobmsg_add_u8(b, "link", !!val);
-
        memset(&ecmd, 0, sizeof(ecmd));
        memset(&ifr, 0, sizeof(ifr));
        strcpy(ifr.ifr_name, dev->ifname);
diff --git a/vlan.c b/vlan.c
index f70420a..9201979 100644
--- a/vlan.c
+++ b/vlan.c
@@ -70,14 +70,19 @@ static void vlan_dev_set_name(struct vlan_device *vldev, 
struct device *dev)
 static void vlan_dev_cb(struct device_user *dep, enum device_event ev)
 {
        struct vlan_device *vldev;
+       bool new_state = false;
 
        vldev = container_of(dep, struct vlan_device, dep);
        switch(ev) {
        case DEV_EVENT_ADD:
-               device_set_present(&vldev->dev, true);
-               break;
+               new_state = true;
        case DEV_EVENT_REMOVE:
-               device_set_present(&vldev->dev, false);
+               device_set_present(&vldev->dev, new_state);
+               break;
+       case DEV_EVENT_LINK_UP:
+               new_state = true;
+       case DEV_EVENT_LINK_DOWN:
+               device_set_link(&vldev->dev, new_state);
                break;
        case DEV_EVENT_UPDATE_IFNAME:
                vlan_dev_set_name(vldev, dep->dev);
@@ -113,6 +118,8 @@ static struct device *get_vlan_device(struct device *dev, 
int id, bool create)
        if (!create)
                return NULL;
 
+       D(DEVICE, "Create vlan device '%s.%d'\n", dev->ifname, id);
+
        vldev = calloc(1, sizeof(*vldev));
 
        vldev->id = id;
-- 
1.7.1
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Reply via email to