[PATCH netifd] bridge: rename "ifname" attribute to "ports"
Paul Oranje
por at oranjevos.nl
Sat May 15 02:09:20 PDT 2021
Good morning !
Thanks for trying to improve on the model set in the netifd UCI, but ...
Regards, Paul
Op 14 mei 2021, om 15:27 heeft Rafał Miłecki <zajec5 at gmail.com> het volgende geschreven:
>
> From: Rafał Miłecki <rafal at milecki.pl>
>
> Bridge aggregates multiple ports so use a more accurate name ("ports").
Confusing that a logical network "interface" references something physical like a "port".
One would expect that at least a level is modelled in between and that a bridge in a "interface" config section can bridge several devices (like plain devices/L2 bridges/VLANs/tunnel devices/...).
Assuming I fail to understand the model, what am I missing ?
> For backward compatibility add a temporary config translation.
>
> Config example:
>
> config interface 'lan'
> option type 'bridge'
> list ports 'lan1'
> list ports 'lan2'
>
> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
> ---
> bridge.c | 16 ++++++++--------
> config.c | 23 ++++++++++++++++++++++-
> 2 files changed, 30 insertions(+), 9 deletions(-)
>
> diff --git a/bridge.c b/bridge.c
> index 099dfe4..a1f3992 100644
> --- a/bridge.c
> +++ b/bridge.c
> @@ -23,7 +23,7 @@
> #include "system.h"
>
> enum {
> - BRIDGE_ATTR_IFNAME,
> + BRIDGE_ATTR_PORTS,
> BRIDGE_ATTR_STP,
> BRIDGE_ATTR_FORWARD_DELAY,
> BRIDGE_ATTR_PRIORITY,
> @@ -44,7 +44,7 @@ enum {
> };
>
> static const struct blobmsg_policy bridge_attrs[__BRIDGE_ATTR_MAX] = {
> - [BRIDGE_ATTR_IFNAME] = { "ifname", BLOBMSG_TYPE_ARRAY },
> + [BRIDGE_ATTR_PORTS] = { "ports", BLOBMSG_TYPE_ARRAY },
> [BRIDGE_ATTR_STP] = { "stp", BLOBMSG_TYPE_BOOL },
> [BRIDGE_ATTR_FORWARD_DELAY] = { "forward_delay", BLOBMSG_TYPE_INT32 },
> [BRIDGE_ATTR_PRIORITY] = { "priority", BLOBMSG_TYPE_INT32 },
> @@ -64,7 +64,7 @@ static const struct blobmsg_policy bridge_attrs[__BRIDGE_ATTR_MAX] = {
> };
>
> static const struct uci_blob_param_info bridge_attr_info[__BRIDGE_ATTR_MAX] = {
> - [BRIDGE_ATTR_IFNAME] = { .type = BLOBMSG_TYPE_STRING },
> + [BRIDGE_ATTR_PORTS] = { .type = BLOBMSG_TYPE_STRING },
> };
>
> static const struct uci_blob_param_list bridge_attr_list = {
> @@ -104,7 +104,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 +853,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 +970,7 @@ 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];
> device_init_settings(dev, tb_dev);
> bridge_apply_settings(bst, tb_br);
>
> @@ -991,7 +991,7 @@ bridge_reload(struct device *dev, struct blob_attr *attr)
>
> diff = 0;
> uci_blob_diff(tb_br, otb_br, &bridge_attr_list, &diff);
> - if (diff & ~(1 << BRIDGE_ATTR_IFNAME))
> + if (diff & ~(1 << BRIDGE_ATTR_PORTS))
> ret = DEV_CONFIG_RESTART;
>
> bridge_config_init(dev);
> diff --git a/config.c b/config.c
> index fa7cbe4..1f4560f 100644
> --- a/config.c
> +++ b/config.c
> @@ -95,6 +95,24 @@ config_fixup_bridge_var(struct uci_section *s, const char *name, const char *val
> uci_set(uci_ctx, &ptr);
> }
>
> +/**
> + * config_fixup_bridge_ports - translate deprecated configs
> + *
> + * Old configs used "ifname" option for specifying bridge ports. For backward
> + * compatibility translate it into the new "ports" option.
> + */
> +static void config_fixup_bridge_ports(struct uci_section *s)
> +{
> + const char *ifname;
> +
> + if (uci_lookup_option(uci_ctx, s, "ports"))
> + return;
> +
> + ifname = uci_lookup_option_string(uci_ctx, s, "ifname");
> + if (ifname)
> + config_fixup_bridge_var(s, "ports", ifname);
> +}
> +
> static void
> config_fixup_bridge_vlan_filtering(struct uci_section *s, const char *name)
> {
> @@ -117,6 +135,7 @@ config_parse_bridge_interface(struct uci_section *s, struct device_type *devtype
> sprintf(name, "%s-%s", devtype->name_prefix, s->e.name);
> blobmsg_add_string(&b, "name", name);
>
> + config_fixup_bridge_ports(s);
> config_fixup_bridge_vlan_filtering(s, name);
> uci_to_blob(&b, s, devtype->config_params);
> if (!device_create(name, devtype, b.head)) {
> @@ -254,8 +273,10 @@ config_init_devices(bool bridge)
> if (!params)
> params = simple_device_type.config_params;
>
> - if (devtype && devtype->bridge_capability)
> + if (devtype && devtype->bridge_capability) {
> + config_fixup_bridge_ports(s);
> config_fixup_bridge_vlan_filtering(s, name);
> + }
>
> blob_buf_init(&b, 0);
> uci_to_blob(&b, s, params);
> --
> 2.26.2
>
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel at lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
More information about the openwrt-devel
mailing list