[PATCH netifd] config: support bridge designed UCI section

[Hans Dedecker]

On Tue, May 11, 2021 at 7:14 PM Rafał Miłecki <zajec5 at gmail.com> wrote:
>
> From: Rafał Miłecki <rafal at milecki.pl>
>
> Network layer 2 devices should have their own UCI section types. They
> differ so much that each device type requires a custom handling anyway.
> Currently there is "type" option used to distinguish them while UCI
> supports different section types right for that purpose. This change
> will result in cleaner UCI and UI code.
>
> Example UCI section:
>
> config bridge
>         option name 'foo'
>         list ports 'lan1'
>         list ports 'lan2'
>
> While introducing this new bridge section a new option was added for
> storing bridge port names: "ports". It's clearer than previously used
> "ifname". A simple validation code is present to make sure "ports" is
> used and contains a list of ports.
>
> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
Acked-by: Hans Dedecker <dedeckeh at gmail.com>
> ---
>  bridge.c | 14 +++++++++-----
>  config.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+), 5 deletions(-)
>
> diff --git a/bridge.c b/bridge.c
> index 099dfe4..ce49a74 100644
> --- a/bridge.c
> +++ b/bridge.c
> @@ -23,7 +23,8 @@
>  #include "system.h"
>
>  enum {
> -       BRIDGE_ATTR_IFNAME,
> +       BRIDGE_ATTR_PORTS,
> +       BRIDGE_ATTR_IFNAME, /* Deprecated */
>         BRIDGE_ATTR_STP,
>         BRIDGE_ATTR_FORWARD_DELAY,
>         BRIDGE_ATTR_PRIORITY,
> @@ -44,6 +45,7 @@ enum {
>  };
>
>  static const struct blobmsg_policy bridge_attrs[__BRIDGE_ATTR_MAX] = {
> +       [BRIDGE_ATTR_PORTS] = { "ports", BLOBMSG_TYPE_ARRAY },
>         [BRIDGE_ATTR_IFNAME] = { "ifname", BLOBMSG_TYPE_ARRAY },
>         [BRIDGE_ATTR_STP] = { "stp", BLOBMSG_TYPE_BOOL },
>         [BRIDGE_ATTR_FORWARD_DELAY] = { "forward_delay", BLOBMSG_TYPE_INT32 },
> @@ -104,7 +106,7 @@ struct bridge_state {
>
>         struct blob_attr *config_data;
>         struct bridge_config config;
> -       struct blob_attr *ifnames;
> +       struct blob_attr *ports;
>         bool active;
>         bool force_active;
>         bool has_vlans;
> @@ -853,8 +855,8 @@ bridge_config_init(struct device *dev)
>
>         bst->n_failed = 0;
>         vlist_update(&bst->members);
> -       if (bst->ifnames) {
> -               blobmsg_for_each_attr(cur, bst->ifnames, rem) {
> +       if (bst->ports) {
> +               blobmsg_for_each_attr(cur, bst->ports, rem) {
>                         bridge_add_member(bst, blobmsg_data(cur));
>                 }
>         }
> @@ -970,7 +972,9 @@ bridge_reload(struct device *dev, struct blob_attr *attr)
>         if (tb_dev[DEV_ATTR_MACADDR])
>                 bst->primary_port = NULL;
>
> -       bst->ifnames = tb_br[BRIDGE_ATTR_IFNAME];
> +       bst->ports = tb_br[BRIDGE_ATTR_PORTS];
> +       if (!bst->ports)
> +               bst->ports = tb_br[BRIDGE_ATTR_IFNAME];
>         device_init_settings(dev, tb_dev);
>         bridge_apply_settings(bst, tb_br);
>
> diff --git a/config.c b/config.c
> index fa7cbe4..4cc5a61 100644
> --- a/config.c
> +++ b/config.c
> @@ -223,6 +223,57 @@ config_parse_rule(struct uci_section *s, bool v6)
>         iprule_add(blob_data(b.head), v6);
>  }
>
> +/**
> + * config_init_bridges - create bridges for new syntax UCI sections
> + *
> + * The new syntax uses dedicated UCI "bridge" sections for describing bridges.
> + * They use "ports" list instead of "ifname" for specifying bridge ports.
> + */
> +static void config_init_bridges()
> +{
> +       struct uci_element *e;
> +
> +       uci_foreach_element(&uci_network->sections, e) {
> +               struct uci_section *s = uci_to_section(e);
> +               struct device_type *devtype;
> +               struct uci_option *o;
> +               struct device *dev;
> +               const char *name;
> +
> +               if (strcmp(s->type, "bridge"))
> +                       continue;
> +
> +               name = uci_lookup_option_string(uci_ctx, s, "name");
> +               if (!name)
> +                       continue;
> +
> +               devtype = device_type_get("bridge");
> +               if (!devtype)
> +                       continue;
> +
> +               config_fixup_bridge_vlan_filtering(s, name);
> +               o = uci_lookup_option(uci_ctx, s, "ifname");
> +               if (o) {
> +                       netifd_log_message(L_WARNING, "Unsupported \"ifname\" bridge option\n");
> +                       continue;
> +               }
> +               o = uci_lookup_option(uci_ctx, s, "ports");
> +               if (o && o->type != UCI_TYPE_LIST) {
> +                       netifd_log_message(L_WARNING, "Invalid \"ports\" option format\n");
> +                       continue;
> +               }
> +
> +               blob_buf_init(&b, 0);
> +               uci_to_blob(&b, s, devtype->config_params);
> +
> +               dev = device_create(name, devtype, b.head);
> +               if (!dev)
> +                       continue;
> +
> +               dev->default_config = false;
> +       }
> +}
> +
>  static void
>  config_init_devices(bool bridge)
>  {
> @@ -737,6 +788,7 @@ config_init_all(void)
>         device_lock();
>
>         device_reset_config();
> +       config_init_bridges();
>         config_init_devices(true);
>         config_init_vlans();
>         config_init_devices(false);
> --
> 2.26.2
>