[OpenWrt-Devel] [PATCH][netifd] iprule: rework interface based rules to handle dynamic interfaces
Hans Dedecker
dedeckeh at gmail.com
Mon Jun 18 17:18:32 EDT 2018
On Mon, Jun 18, 2018 at 5:10 PM Alexander Couzens <lynxis at 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 at openwrt.org>
> * Copyright (C) 2013 Jo-Philipp Wich <jow at openwrt.org>
> + * Copyright (C) 2018 Alexander Couzens <lynxis at 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 at lists.openwrt.org
> https://lists.openwrt.org/listinfo/openwrt-devel
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list