| Message ID | 20180629032315.7906-2-lynxis@fe80.eu |
|---|---|
| State | Accepted |
| Delegated to: | John Crispin |
| Headers | show |
| Series | [OpenWrt-Devel,1/2] Introduce new interface event "create" (IFEV_CREATE) | expand |
On Fri, Jun 29, 2018 at 5:23 AM Alexander Couzens <lynxis@fe80.eu> wrote: > > Previous netifd would only apply `ip rule`s while config phase. > If the iprule is depending on an interface (iif or oif), the rule > will fail if the interface is not up. > > Allow iprules to track interfaces and their devices by using > the interface events. > > Fixes: FS#1571 > --- > iprule.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++------- > iprule.h | 9 +++ > 2 files changed, 167 insertions(+), 21 deletions(-) > > diff --git a/iprule.c b/iprule.c > index 7cf7422f4168..740965431b4c 100644 > --- a/iprule.c > +++ b/iprule.c > @@ -2,6 +2,7 @@ > * netifd - network interface daemon > * Copyright (C) 2012 Felix Fietkau <nbd@openwrt.org> > * Copyright (C) 2013 Jo-Philipp Wich <jow@openwrt.org> > + * Copyright (C) 2018 Alexander Couzens <lynxis@fe80.eu> > * > * This program is free software; you can redistribute it and/or modify > * it under the terms of the GNU General Public License version 2 > @@ -12,6 +13,7 @@ > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > * GNU General Public License for more details. > */ > +#include <assert.h> > #include <string.h> > #include <stdlib.h> > #include <stdio.h> > @@ -66,6 +68,16 @@ const struct uci_blob_param_list rule_attr_list = { > .params = rule_attr, > }; > > +/* interface based rules are dynamic. */ > +static bool rule_ready(struct iprule *rule) { > + if (rule->flags & IPRULE_OUT && rule->out_dev == NULL) > + return false; > + > + if (rule->flags & IPRULE_IN && rule->in_dev == NULL) > + return false; > + > + return true; > +} > > static bool > iprule_parse_mark(const char *mark, struct iprule *rule) > @@ -97,13 +109,105 @@ iprule_parse_mark(const char *mark, struct iprule *rule) > return true; > } > > +/* called on interface changes of the incoming interface */ > +static void rule_in_cb( > + struct interface_user *dep, > + struct interface *iface, > + enum interface_event ev) > +{ > + struct iprule *rule = container_of(dep, struct iprule, in_iface_user); > + > + switch (ev) { > + case IFEV_UP: > + if (!iface->l3_dev.dev) > + break; > + memcpy(rule->in_dev, iface->l3_dev.dev->ifname, sizeof(rule->in_dev)); > + if (rule_ready(rule)) > + system_add_iprule(rule); > + break; > + case IFEV_DOWN: > + case IFEV_UP_FAILED: > + case IFEV_FREE: > + if (rule_ready(rule)) > + system_del_iprule(rule); > + rule->in_dev[0] = 0; > + break; > + default: > + break; > + } > +} > + > +/* called on interface changes of the outgoing interface */ > +static void rule_out_cb( > + struct interface_user *dep, > + struct interface *iface, > + enum interface_event ev) > +{ > + struct iprule *rule = container_of(dep, struct iprule, out_iface_user); > + > + switch (ev) { > + case IFEV_UP: > + if (!iface->l3_dev.dev) > + break; > + memcpy(rule->out_dev, iface->l3_dev.dev->ifname, sizeof(rule->out_dev)); > + if (rule_ready(rule)) > + system_add_iprule(rule); > + break; > + case IFEV_DOWN: > + case IFEV_UP_FAILED: > + case IFEV_FREE: > + if (rule_ready(rule)) > + system_del_iprule(rule); > + rule->out_dev[0] = 0; > + break; > + default: > + break; > + } > +} > + > +/* called on all interface events */ > +static void generic_interface_cb( > + struct interface_user *dep, > + struct interface *iface, > + enum interface_event ev) > +{ > + struct iprule *rule; > + > + if (ev != IFEV_CREATE) > + return; > + > + /* add new interfaces to rules */ > + vlist_for_each_element(&iprules, rule, node) { > + if (rule_ready(rule)) > + continue; > + > + if (!strcmp(rule->out_iface, iface->name)) { > + assert(!rule->out_dev); > + memcpy(rule->out_dev, iface->l3_dev.dev->ifname, sizeof(rule->out_dev)); > + interface_add_user(&rule->out_iface_user, iface); > + } > + > + if (!strcmp(rule->in_iface, iface->name)) { > + assert(!rule->in_dev); > + memcpy(rule->in_dev, iface->l3_dev.dev->ifname, sizeof(rule->in_dev)); > + interface_add_user(&rule->in_iface_user, iface); > + } > + > + if (rule_ready(rule)) > + system_add_iprule(rule); Is the rule_ready check and consequent call to system_add_iprule still required ? Above calls to interface_add_user install the callbacks; if the interface is up rule_in_cb and/or rule_out_cb is immediately invoked by interface_add_user which installs the ip rule if the rule is ready. Hans > + } > +} > + > +struct interface_user generic_listener = { > + .cb = generic_interface_cb > +}; > + > void > iprule_add(struct blob_attr *attr, bool v6) > { > - struct interface *iif = NULL, *oif = NULL; > struct blob_attr *tb[__RULE_MAX], *cur; > - struct interface *iface; > struct iprule *rule; > + char *iface_name; > int af = v6 ? AF_INET6 : AF_INET; > > blobmsg_parse(rule_attr, __RULE_MAX, tb, blobmsg_data(attr), blobmsg_data_len(attr)); > @@ -119,26 +223,16 @@ iprule_add(struct blob_attr *attr, bool v6) > rule->invert = blobmsg_get_bool(cur); > > if ((cur = tb[RULE_INTERFACE_IN]) != NULL) { > - iif = vlist_find(&interfaces, blobmsg_data(cur), iface, node); > - > - if (!iif || !iif->l3_dev.dev) { > - DPRINTF("Failed to resolve device of network: %s\n", (char *) blobmsg_data(cur)); > - goto error; > - } > - > - memcpy(rule->in_dev, iif->l3_dev.dev->ifname, sizeof(rule->in_dev)); > + iface_name = calloc(1, strlen(blobmsg_data(cur)) + 1); > + rule->in_iface = strcpy(iface_name, blobmsg_data(cur)); > + rule->in_iface_user.cb = &rule_in_cb; > rule->flags |= IPRULE_IN; > } > > if ((cur = tb[RULE_INTERFACE_OUT]) != NULL) { > - oif = vlist_find(&interfaces, blobmsg_data(cur), iface, node); > - > - if (!oif || !oif->l3_dev.dev) { > - DPRINTF("Failed to resolve device of network: %s\n", (char *) blobmsg_data(cur)); > - goto error; > - } > - > - memcpy(rule->out_dev, oif->l3_dev.dev->ifname, sizeof(rule->out_dev)); > + iface_name = calloc(1, strlen(blobmsg_data(cur)) + 1); > + rule->out_iface = strcpy(iface_name, blobmsg_data(cur)); > + rule->out_iface_user.cb = &rule_out_cb; > rule->flags |= IPRULE_OUT; > } > > @@ -238,6 +332,31 @@ rule_cmp(const void *k1, const void *k2, void *ptr) > return memcmp(k1, k2, sizeof(struct iprule)-offsetof(struct iprule, flags)); > } > > +static void deregister_interfaces(struct iprule *rule) > +{ > + if (rule->flags & IPRULE_IN && rule->in_iface_user.iface) > + interface_remove_user(&rule->in_iface_user); > + > + if (rule->flags & IPRULE_OUT && rule->out_iface_user.iface) > + interface_remove_user(&rule->out_iface_user); > +} > + > +static void register_interfaces(struct iprule *rule) > +{ > + struct interface *iface, *tmp; > + > + if (rule->flags & IPRULE_IN) { > + tmp = vlist_find(&interfaces, rule->in_iface, iface, node); > + if (tmp) > + interface_add_user(&rule->in_iface_user, tmp); > + } > + if (rule->flags & IPRULE_OUT) { > + tmp = vlist_find(&interfaces, rule->out_iface, iface, node); > + if (tmp) > + interface_add_user(&rule->out_iface_user, tmp); > + } > +} > + > static void > iprule_update_rule(struct vlist_tree *tree, > struct vlist_node *node_new, struct vlist_node *node_old) > @@ -248,16 +367,34 @@ iprule_update_rule(struct vlist_tree *tree, > rule_new = container_of(node_new, struct iprule, node); > > if (node_old) { > - system_del_iprule(rule_old); > + if (rule_ready(rule_old)) > + system_del_iprule(rule_old); > + > + if (rule_old->flags & (IPRULE_IN | IPRULE_OUT)) > + deregister_interfaces(rule_old); > + > + if (rule_old->in_iface) > + free(rule_old->in_iface); > + > + if (rule_old->out_iface) > + free(rule_old->out_iface); > + > free(rule_old); > } > > - if (node_new) > - system_add_iprule(rule_new); > + if (node_new) { > + /* interface based rules calls system_add_iprule over the event cb */ > + if (rule_new->flags & (IPRULE_IN | IPRULE_OUT)) { > + register_interfaces(rule_new); > + } else { > + system_add_iprule(rule_new); > + } > + } > } > > static void __init > iprule_init_list(void) > { > vlist_init(&iprules, rule_cmp, iprule_update_rule); > + interface_add_user(&generic_listener, NULL); > } > diff --git a/iprule.h b/iprule.h > index b723bdb05d7d..f05c3c93b4a1 100644 > --- a/iprule.h > +++ b/iprule.h > @@ -74,6 +74,15 @@ struct iprule { > > bool invert; > > + /* uci interface name */ > + char *in_iface; > + char *out_iface; > + > + /* to receive interface events */ > + struct interface_user in_iface_user; > + struct interface_user out_iface_user; > + > + /* device name */ > char in_dev[IFNAMSIZ + 1]; > char out_dev[IFNAMSIZ + 1]; > > -- > 2.18.0 >
On Fri, 29 Jun 2018 16:23:02 +0200 Hans Dedecker <dedeckeh@gmail.com> wrote: > On Fri, Jun 29, 2018 at 5:23 AM Alexander Couzens <lynxis@fe80.eu> > wrote: [...] > Is the rule_ready check and consequent call to system_add_iprule still > required ? Above calls to interface_add_user install the callbacks; if > the interface is up rule_in_cb and/or rule_out_cb is immediately > invoked by interface_add_user which installs the ip rule if the rule > is ready. Indeed, it's not needed.
diff --git a/iprule.c b/iprule.c index 7cf7422f4168..740965431b4c 100644 --- a/iprule.c +++ b/iprule.c @@ -2,6 +2,7 @@ * netifd - network interface daemon * Copyright (C) 2012 Felix Fietkau <nbd@openwrt.org> * Copyright (C) 2013 Jo-Philipp Wich <jow@openwrt.org> + * Copyright (C) 2018 Alexander Couzens <lynxis@fe80.eu> * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License version 2 @@ -12,6 +13,7 @@ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the * GNU General Public License for more details. */ +#include <assert.h> #include <string.h> #include <stdlib.h> #include <stdio.h> @@ -66,6 +68,16 @@ const struct uci_blob_param_list rule_attr_list = { .params = rule_attr, }; +/* interface based rules are dynamic. */ +static bool rule_ready(struct iprule *rule) { + if (rule->flags & IPRULE_OUT && rule->out_dev == NULL) + return false; + + if (rule->flags & IPRULE_IN && rule->in_dev == NULL) + return false; + + return true; +} static bool iprule_parse_mark(const char *mark, struct iprule *rule) @@ -97,13 +109,105 @@ iprule_parse_mark(const char *mark, struct iprule *rule) return true; } +/* called on interface changes of the incoming interface */ +static void rule_in_cb( + struct interface_user *dep, + struct interface *iface, + enum interface_event ev) +{ + struct iprule *rule = container_of(dep, struct iprule, in_iface_user); + + switch (ev) { + case IFEV_UP: + if (!iface->l3_dev.dev) + break; + memcpy(rule->in_dev, iface->l3_dev.dev->ifname, sizeof(rule->in_dev)); + if (rule_ready(rule)) + system_add_iprule(rule); + break; + case IFEV_DOWN: + case IFEV_UP_FAILED: + case IFEV_FREE: + if (rule_ready(rule)) + system_del_iprule(rule); + rule->in_dev[0] = 0; + break; + default: + break; + } +} + +/* called on interface changes of the outgoing interface */ +static void rule_out_cb( + struct interface_user *dep, + struct interface *iface, + enum interface_event ev) +{ + struct iprule *rule = container_of(dep, struct iprule, out_iface_user); + + switch (ev) { + case IFEV_UP: + if (!iface->l3_dev.dev) + break; + memcpy(rule->out_dev, iface->l3_dev.dev->ifname, sizeof(rule->out_dev)); + if (rule_ready(rule)) + system_add_iprule(rule); + break; + case IFEV_DOWN: + case IFEV_UP_FAILED: + case IFEV_FREE: + if (rule_ready(rule)) + system_del_iprule(rule); + rule->out_dev[0] = 0; + break; + default: + break; + } +} + +/* called on all interface events */ +static void generic_interface_cb( + struct interface_user *dep, + struct interface *iface, + enum interface_event ev) +{ + struct iprule *rule; + + if (ev != IFEV_CREATE) + return; + + /* add new interfaces to rules */ + vlist_for_each_element(&iprules, rule, node) { + if (rule_ready(rule)) + continue; + + if (!strcmp(rule->out_iface, iface->name)) { + assert(!rule->out_dev); + memcpy(rule->out_dev, iface->l3_dev.dev->ifname, sizeof(rule->out_dev)); + interface_add_user(&rule->out_iface_user, iface); + } + + if (!strcmp(rule->in_iface, iface->name)) { + assert(!rule->in_dev); + memcpy(rule->in_dev, iface->l3_dev.dev->ifname, sizeof(rule->in_dev)); + interface_add_user(&rule->in_iface_user, iface); + } + + if (rule_ready(rule)) + system_add_iprule(rule); + } +} + +struct interface_user generic_listener = { + .cb = generic_interface_cb +}; + void iprule_add(struct blob_attr *attr, bool v6) { - struct interface *iif = NULL, *oif = NULL; struct blob_attr *tb[__RULE_MAX], *cur; - struct interface *iface; struct iprule *rule; + char *iface_name; int af = v6 ? AF_INET6 : AF_INET; blobmsg_parse(rule_attr, __RULE_MAX, tb, blobmsg_data(attr), blobmsg_data_len(attr)); @@ -119,26 +223,16 @@ iprule_add(struct blob_attr *attr, bool v6) rule->invert = blobmsg_get_bool(cur); if ((cur = tb[RULE_INTERFACE_IN]) != NULL) { - iif = vlist_find(&interfaces, blobmsg_data(cur), iface, node); - - if (!iif || !iif->l3_dev.dev) { - DPRINTF("Failed to resolve device of network: %s\n", (char *) blobmsg_data(cur)); - goto error; - } - - memcpy(rule->in_dev, iif->l3_dev.dev->ifname, sizeof(rule->in_dev)); + iface_name = calloc(1, strlen(blobmsg_data(cur)) + 1); + rule->in_iface = strcpy(iface_name, blobmsg_data(cur)); + rule->in_iface_user.cb = &rule_in_cb; rule->flags |= IPRULE_IN; } if ((cur = tb[RULE_INTERFACE_OUT]) != NULL) { - oif = vlist_find(&interfaces, blobmsg_data(cur), iface, node); - - if (!oif || !oif->l3_dev.dev) { - DPRINTF("Failed to resolve device of network: %s\n", (char *) blobmsg_data(cur)); - goto error; - } - - memcpy(rule->out_dev, oif->l3_dev.dev->ifname, sizeof(rule->out_dev)); + iface_name = calloc(1, strlen(blobmsg_data(cur)) + 1); + rule->out_iface = strcpy(iface_name, blobmsg_data(cur)); + rule->out_iface_user.cb = &rule_out_cb; rule->flags |= IPRULE_OUT; } @@ -238,6 +332,31 @@ rule_cmp(const void *k1, const void *k2, void *ptr) return memcmp(k1, k2, sizeof(struct iprule)-offsetof(struct iprule, flags)); } +static void deregister_interfaces(struct iprule *rule) +{ + if (rule->flags & IPRULE_IN && rule->in_iface_user.iface) + interface_remove_user(&rule->in_iface_user); + + if (rule->flags & IPRULE_OUT && rule->out_iface_user.iface) + interface_remove_user(&rule->out_iface_user); +} + +static void register_interfaces(struct iprule *rule) +{ + struct interface *iface, *tmp; + + if (rule->flags & IPRULE_IN) { + tmp = vlist_find(&interfaces, rule->in_iface, iface, node); + if (tmp) + interface_add_user(&rule->in_iface_user, tmp); + } + if (rule->flags & IPRULE_OUT) { + tmp = vlist_find(&interfaces, rule->out_iface, iface, node); + if (tmp) + interface_add_user(&rule->out_iface_user, tmp); + } +} + static void iprule_update_rule(struct vlist_tree *tree, struct vlist_node *node_new, struct vlist_node *node_old) @@ -248,16 +367,34 @@ iprule_update_rule(struct vlist_tree *tree, rule_new = container_of(node_new, struct iprule, node); if (node_old) { - system_del_iprule(rule_old); + if (rule_ready(rule_old)) + system_del_iprule(rule_old); + + if (rule_old->flags & (IPRULE_IN | IPRULE_OUT)) + deregister_interfaces(rule_old); + + if (rule_old->in_iface) + free(rule_old->in_iface); + + if (rule_old->out_iface) + free(rule_old->out_iface); + free(rule_old); } - if (node_new) - system_add_iprule(rule_new); + if (node_new) { + /* interface based rules calls system_add_iprule over the event cb */ + if (rule_new->flags & (IPRULE_IN | IPRULE_OUT)) { + register_interfaces(rule_new); + } else { + system_add_iprule(rule_new); + } + } } static void __init iprule_init_list(void) { vlist_init(&iprules, rule_cmp, iprule_update_rule); + interface_add_user(&generic_listener, NULL); } diff --git a/iprule.h b/iprule.h index b723bdb05d7d..f05c3c93b4a1 100644 --- a/iprule.h +++ b/iprule.h @@ -74,6 +74,15 @@ struct iprule { bool invert; + /* uci interface name */ + char *in_iface; + char *out_iface; + + /* to receive interface events */ + struct interface_user in_iface_user; + struct interface_user out_iface_user; + + /* device name */ char in_dev[IFNAMSIZ + 1]; char out_dev[IFNAMSIZ + 1];