Message ID | 20180618150626.16571-1-lynxis@fe80.eu |
---|---|
State | Changes Requested |
Delegated to: | John Crispin |
Headers | show |
Series | [OpenWrt-Devel,netifd] iprule: rework interface based rules to handle dynamic interfaces | expand |
On Mon, Jun 18, 2018 at 5:10 PM 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 interface and their devices by hooking into > into the interface_ip_set_enabled() call. > > Fixes: FS#1571 > --- > interface-ip.c | 3 ++ > iprule.c | 104 ++++++++++++++++++++++++++++++++++++++++--------- > iprule.h | 7 ++++ > 3 files changed, 96 insertions(+), 18 deletions(-) > > diff --git a/interface-ip.c b/interface-ip.c > index 1e49fe6feac7..1260f6059ecd 100644 > --- a/interface-ip.c > +++ b/interface-ip.c > @@ -1451,6 +1451,9 @@ void interface_ip_set_enabled(struct interface_ip_settings *ip, bool enabled) > NULL, 0, 0, ip->iface, "failed_policy", true); > ip->iface->policy_rules_set = enabled; > } > + > + /* apply ip rules */ > + iprule_set_enabled(ip->iface, enabled); I prefer iprule_init_list installing an interface callback handler via interface_add_user; based on the IFEV_UP and IFEV_DOWN events the necessary logic can be triggered inside iprule.c. This would keep the layering clean inside netifd and avoids the need to export extra iprule functions > } > > void > diff --git a/iprule.c b/iprule.c > index 7cf7422f4168..79e6eca9dfb7 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 > @@ -66,6 +67,16 @@ const struct uci_blob_param_list rule_attr_list = { > .params = rule_attr, > }; > > +/* interface based rules are dynamic. */ > +static int rule_ready(struct iprule *rule) { Better to let this function return a bool Hans > + if (rule->flags & IPRULE_OUT && rule->out_dev == NULL) > + return 0; > + > + if (rule->flags & IPRULE_IN && rule->in_dev == NULL) > + return 0; > + > + return 1; > +} > > static bool > iprule_parse_mark(const char *mark, struct iprule *rule) > @@ -97,13 +108,65 @@ iprule_parse_mark(const char *mark, struct iprule *rule) > return true; > } > > +/* remove all rules from the system which depend on iface */ > +static void > +remove_iface_rules(struct interface *iface) { > + struct iprule *rule; > + > + vlist_for_each_element(&iprules, rule, node) { > + if (!(rule->flags & (IPRULE_IN | IPRULE_OUT))) > + continue; > + > + if (!strcmp(rule->in_iface, iface->name)) { > + if (rule_ready(rule)) > + system_del_iprule(rule); > + rule->in_dev[0] = 0; > + } > + > + if (!strcmp(rule->out_iface, iface->name)) { > + if (rule_ready(rule)) > + system_del_iprule(rule); > + rule->out_dev[0] = 0; > + } > + } > +} > + > +/* update rules which depend on iface. Add them to the system if they validates afterwards. */ > +static void add_iface_rule(struct interface *iface) { > + struct iprule *rule; > + > + vlist_for_each_element(&iprules, rule, node) { > + if (rule_ready(rule)) > + continue; > + > + if (!strcmp(rule->out_iface, iface->name)) > + memcpy(rule->out_dev, iface->l3_dev.dev->ifname, sizeof(rule->out_dev)); > + > + if (!strcmp(rule->in_iface, iface->name)) > + memcpy(rule->in_dev, iface->l3_dev.dev->ifname, sizeof(rule->in_dev)); > + > + if (rule_ready(rule)) > + system_add_iprule(rule); > + } > +} > + > +/* called by interface_ip_set_enabled */ > +void iprule_set_enabled(struct interface *iface, int enabled) > +{ > + if (enabled) > + add_iface_rule(iface); > + else > + remove_iface_rules(iface); > +} > + > 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 interface *iface, *tmp; > 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)); > @@ -118,28 +181,30 @@ iprule_add(struct blob_attr *attr, bool v6) > if ((cur = tb[RULE_INVERT]) != NULL) > rule->invert = blobmsg_get_bool(cur); > > + /* TODO: check if we need to validate the uci interface name or not */ > if ((cur = tb[RULE_INTERFACE_IN]) != NULL) { > - iif = vlist_find(&interfaces, blobmsg_data(cur), iface, node); > + iface_name = calloc(1, strlen(blobmsg_data(cur)) + 1); > + rule->in_iface = strcpy(iface_name, blobmsg_data(cur)); > + rule->flags |= IPRULE_IN; > > - if (!iif || !iif->l3_dev.dev) { > - DPRINTF("Failed to resolve device of network: %s\n", (char *) blobmsg_data(cur)); > - goto error; > + /* this can be filled later by the set_interface_ip call */ > + tmp = vlist_find(&interfaces, blobmsg_data(cur), iface, node); > + if (tmp && tmp->l3_dev.dev) { > + memcpy(rule->in_dev, tmp->l3_dev.dev->ifname, sizeof(rule->in_dev)); > } > - > - memcpy(rule->in_dev, iif->l3_dev.dev->ifname, sizeof(rule->in_dev)); > - rule->flags |= IPRULE_IN; > } > > + /* TODO: check if we need to validate the uci interface name or not */ > if ((cur = tb[RULE_INTERFACE_OUT]) != NULL) { > - oif = vlist_find(&interfaces, blobmsg_data(cur), iface, node); > + iface_name = calloc(1, strlen(blobmsg_data(cur)) + 1); > + rule->out_iface = strcpy(iface_name, blobmsg_data(cur)); > + rule->flags |= IPRULE_OUT; > > - if (!oif || !oif->l3_dev.dev) { > - DPRINTF("Failed to resolve device of network: %s\n", (char *) blobmsg_data(cur)); > - goto error; > + /* this can be filled later by the set_interface_ip call */ > + tmp = vlist_find(&interfaces, blobmsg_data(cur), iface, node); > + if (tmp && tmp->l3_dev.dev) { > + memcpy(rule->out_dev, tmp->l3_dev.dev->ifname, sizeof(rule->out_dev)); > } > - > - memcpy(rule->out_dev, oif->l3_dev.dev->ifname, sizeof(rule->out_dev)); > - rule->flags |= IPRULE_OUT; > } > > if ((cur = tb[RULE_SRC]) != NULL) { > @@ -248,12 +313,15 @@ iprule_update_rule(struct vlist_tree *tree, > rule_new = container_of(node_new, struct iprule, node); > > if (node_old) { > - system_del_iprule(rule_old); > + /* interface based rules must not be present at all times */ > + if (rule_ready(rule_old)) > + system_del_iprule(rule_old); > free(rule_old); > } > > - if (node_new) > + if (node_new && rule_ready(rule_new)) { > system_add_iprule(rule_new); > + } > } > > static void __init > diff --git a/iprule.h b/iprule.h > index b723bdb05d7d..c399c413efb0 100644 > --- a/iprule.h > +++ b/iprule.h > @@ -74,6 +74,11 @@ struct iprule { > > bool invert; > > + /* uci interface name */ > + const char *in_iface; > + const char *out_iface; > + > + /* device name */ > char in_dev[IFNAMSIZ + 1]; > char out_dev[IFNAMSIZ + 1]; > > @@ -102,4 +107,6 @@ void iprule_add(struct blob_attr *attr, bool v6); > void iprule_update_start(void); > void iprule_update_complete(void); > > +void iprule_set_enabled(struct interface *iface, int enabled); > + > #endif > -- > 2.17.1 > > > _______________________________________________ > openwrt-devel mailing list > openwrt-devel@lists.openwrt.org > https://lists.openwrt.org/listinfo/openwrt-devel
diff --git a/interface-ip.c b/interface-ip.c index 1e49fe6feac7..1260f6059ecd 100644 --- a/interface-ip.c +++ b/interface-ip.c @@ -1451,6 +1451,9 @@ void interface_ip_set_enabled(struct interface_ip_settings *ip, bool enabled) NULL, 0, 0, ip->iface, "failed_policy", true); ip->iface->policy_rules_set = enabled; } + + /* apply ip rules */ + iprule_set_enabled(ip->iface, enabled); } void diff --git a/iprule.c b/iprule.c index 7cf7422f4168..79e6eca9dfb7 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 @@ -66,6 +67,16 @@ const struct uci_blob_param_list rule_attr_list = { .params = rule_attr, }; +/* interface based rules are dynamic. */ +static int rule_ready(struct iprule *rule) { + if (rule->flags & IPRULE_OUT && rule->out_dev == NULL) + return 0; + + if (rule->flags & IPRULE_IN && rule->in_dev == NULL) + return 0; + + return 1; +} static bool iprule_parse_mark(const char *mark, struct iprule *rule) @@ -97,13 +108,65 @@ iprule_parse_mark(const char *mark, struct iprule *rule) return true; } +/* remove all rules from the system which depend on iface */ +static void +remove_iface_rules(struct interface *iface) { + struct iprule *rule; + + vlist_for_each_element(&iprules, rule, node) { + if (!(rule->flags & (IPRULE_IN | IPRULE_OUT))) + continue; + + if (!strcmp(rule->in_iface, iface->name)) { + if (rule_ready(rule)) + system_del_iprule(rule); + rule->in_dev[0] = 0; + } + + if (!strcmp(rule->out_iface, iface->name)) { + if (rule_ready(rule)) + system_del_iprule(rule); + rule->out_dev[0] = 0; + } + } +} + +/* update rules which depend on iface. Add them to the system if they validates afterwards. */ +static void add_iface_rule(struct interface *iface) { + struct iprule *rule; + + vlist_for_each_element(&iprules, rule, node) { + if (rule_ready(rule)) + continue; + + if (!strcmp(rule->out_iface, iface->name)) + memcpy(rule->out_dev, iface->l3_dev.dev->ifname, sizeof(rule->out_dev)); + + if (!strcmp(rule->in_iface, iface->name)) + memcpy(rule->in_dev, iface->l3_dev.dev->ifname, sizeof(rule->in_dev)); + + if (rule_ready(rule)) + system_add_iprule(rule); + } +} + +/* called by interface_ip_set_enabled */ +void iprule_set_enabled(struct interface *iface, int enabled) +{ + if (enabled) + add_iface_rule(iface); + else + remove_iface_rules(iface); +} + 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 interface *iface, *tmp; 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)); @@ -118,28 +181,30 @@ iprule_add(struct blob_attr *attr, bool v6) if ((cur = tb[RULE_INVERT]) != NULL) rule->invert = blobmsg_get_bool(cur); + /* TODO: check if we need to validate the uci interface name or not */ if ((cur = tb[RULE_INTERFACE_IN]) != NULL) { - iif = vlist_find(&interfaces, blobmsg_data(cur), iface, node); + iface_name = calloc(1, strlen(blobmsg_data(cur)) + 1); + rule->in_iface = strcpy(iface_name, blobmsg_data(cur)); + rule->flags |= IPRULE_IN; - if (!iif || !iif->l3_dev.dev) { - DPRINTF("Failed to resolve device of network: %s\n", (char *) blobmsg_data(cur)); - goto error; + /* this can be filled later by the set_interface_ip call */ + tmp = vlist_find(&interfaces, blobmsg_data(cur), iface, node); + if (tmp && tmp->l3_dev.dev) { + memcpy(rule->in_dev, tmp->l3_dev.dev->ifname, sizeof(rule->in_dev)); } - - memcpy(rule->in_dev, iif->l3_dev.dev->ifname, sizeof(rule->in_dev)); - rule->flags |= IPRULE_IN; } + /* TODO: check if we need to validate the uci interface name or not */ if ((cur = tb[RULE_INTERFACE_OUT]) != NULL) { - oif = vlist_find(&interfaces, blobmsg_data(cur), iface, node); + iface_name = calloc(1, strlen(blobmsg_data(cur)) + 1); + rule->out_iface = strcpy(iface_name, blobmsg_data(cur)); + rule->flags |= IPRULE_OUT; - if (!oif || !oif->l3_dev.dev) { - DPRINTF("Failed to resolve device of network: %s\n", (char *) blobmsg_data(cur)); - goto error; + /* this can be filled later by the set_interface_ip call */ + tmp = vlist_find(&interfaces, blobmsg_data(cur), iface, node); + if (tmp && tmp->l3_dev.dev) { + memcpy(rule->out_dev, tmp->l3_dev.dev->ifname, sizeof(rule->out_dev)); } - - memcpy(rule->out_dev, oif->l3_dev.dev->ifname, sizeof(rule->out_dev)); - rule->flags |= IPRULE_OUT; } if ((cur = tb[RULE_SRC]) != NULL) { @@ -248,12 +313,15 @@ iprule_update_rule(struct vlist_tree *tree, rule_new = container_of(node_new, struct iprule, node); if (node_old) { - system_del_iprule(rule_old); + /* interface based rules must not be present at all times */ + if (rule_ready(rule_old)) + system_del_iprule(rule_old); free(rule_old); } - if (node_new) + if (node_new && rule_ready(rule_new)) { system_add_iprule(rule_new); + } } static void __init diff --git a/iprule.h b/iprule.h index b723bdb05d7d..c399c413efb0 100644 --- a/iprule.h +++ b/iprule.h @@ -74,6 +74,11 @@ struct iprule { bool invert; + /* uci interface name */ + const char *in_iface; + const char *out_iface; + + /* device name */ char in_dev[IFNAMSIZ + 1]; char out_dev[IFNAMSIZ + 1]; @@ -102,4 +107,6 @@ void iprule_add(struct blob_attr *attr, bool v6); void iprule_update_start(void); void iprule_update_complete(void); +void iprule_set_enabled(struct interface *iface, int enabled); + #endif