Hi, As a general remark it's better to use vlist_simple_tree and vlist_simple_node for the list implementation; for the reasons see below
On Sun, May 3, 2020 at 7:43 PM Pau Espin Pedrol <[email protected]> wrote: > > From: Pau Espin Pedrol <[email protected]> > > It allows setting mappings for instance this way: > """ > config device > option name 'vlan41' > option type '8021q' > option vid '41' > option ifname 'eth1' > list ingress_qos_mapping '1:2' > list ingress_qos_mapping '2:5' > list egress_qos_mapping '0:3' > """ > > Signed-off-by: Pau Espin Pedrol <[email protected]> > Tested-by: Pedro <[email protected]> > --- > system-linux.c | 22 +++++++++++++- > system.h | 8 +++++ > vlandev.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 109 insertions(+), 1 deletion(-) > > diff --git a/system-linux.c b/system-linux.c > index 62636c4..c61a5a2 100644 > --- a/system-linux.c > +++ b/system-linux.c > @@ -1401,8 +1401,10 @@ int system_vlan_del(struct device *dev) > int system_vlandev_add(struct device *vlandev, struct device *dev, struct > vlandev_config *cfg) > { > struct nl_msg *msg; > - struct nlattr *linkinfo, *data; > + struct nlattr *linkinfo, *data, *qos; > struct ifinfomsg iim = { .ifi_family = AF_UNSPEC }; > + struct vlan_qos_mapping *dep; > + struct ifla_vlan_qos_mapping nl_qos_map; > int rv; > > msg = nlmsg_alloc_simple(RTM_NEWLINK, NLM_F_REQUEST | NLM_F_CREATE | > NLM_F_EXCL); > @@ -1431,6 +1433,24 @@ int system_vlandev_add(struct device *vlandev, struct > device *dev, struct vlande > netifd_log_message(L_WARNING, "%s Your kernel is older than > linux 3.10.0, 802.1ad is not supported defaulting to 802.1q", > vlandev->type->name); > #endif > > + if (!(qos = nla_nest_start(msg, IFLA_VLAN_INGRESS_QOS))) > + goto nla_put_failure; Add an empty line after the goto statement > > + list_for_each_entry(dep, &cfg->ingress_qos_mapping_list, list) { > + nl_qos_map.from = dep->from; > + nl_qos_map.to = dep->to; > + nla_put(msg, IFLA_VLAN_QOS_MAPPING, sizeof(nl_qos_map), > &nl_qos_map); > + } > + nla_nest_end(msg, qos); > + > + if (!(qos = nla_nest_start(msg, IFLA_VLAN_EGRESS_QOS))) > + goto nla_put_failure; Add an empty line after the goto statement > > + list_for_each_entry(dep, &cfg->egress_qos_mapping_list, list) { > + nl_qos_map.from = dep->from; > + nl_qos_map.to = dep->to; > + nla_put(msg, IFLA_VLAN_QOS_MAPPING, sizeof(nl_qos_map), > &nl_qos_map); > + } > + nla_nest_end(msg, qos); > + > nla_nest_end(msg, data); > nla_nest_end(msg, linkinfo); > > diff --git a/system.h b/system.h > index b377416..e677c95 100644 > --- a/system.h > +++ b/system.h > @@ -158,9 +158,17 @@ enum vlan_proto { > VLAN_PROTO_8021AD = 0x88A8 > }; > > +struct vlan_qos_mapping { > + struct list_head list; /* entry in > vlandev_config->{e,in}ress_qos_mapping_list */ It's better to use struct vlist_simple_node iso struct list_head; see struct dns_server as example > > + uint32_t from; > + uint32_t to; > +}; > + > struct vlandev_config { > enum vlan_proto proto; > uint16_t vid; > + struct list_head ingress_qos_mapping_list; /* list of struct > vlan_qos_mapping */ Use struct vlist_simple_tree iso struct list_head > > + struct list_head egress_qos_mapping_list; /* list of struct > vlan_qos_mapping */ Use struct vlist_simple_tree iso struct list_head > > }; > > static inline int system_get_addr_family(unsigned int flags) > diff --git a/vlandev.c b/vlandev.c > index ceaeb3e..3d4d0e4 100644 > --- a/vlandev.c > +++ b/vlandev.c > @@ -13,6 +13,7 @@ > */ > > #include <string.h> > +#include <inttypes.h> > > #include "netifd.h" > #include "device.h" > @@ -22,12 +23,16 @@ > enum { > VLANDEV_ATTR_IFNAME, > VLANDEV_ATTR_VID, > + VLANDEV_ATTR_INGRESS_QOS_MAPPING, > + VLANDEV_ATTR_EGRESS_QOS_MAPPING, > __VLANDEV_ATTR_MAX > }; > > static const struct blobmsg_policy vlandev_attrs[__VLANDEV_ATTR_MAX] = { > [VLANDEV_ATTR_IFNAME] = { "ifname", BLOBMSG_TYPE_STRING }, > [VLANDEV_ATTR_VID] = { "vid", BLOBMSG_TYPE_INT32 }, > + [VLANDEV_ATTR_INGRESS_QOS_MAPPING] = { "ingress_qos_mapping", > BLOBMSG_TYPE_ARRAY }, > + [VLANDEV_ATTR_EGRESS_QOS_MAPPING] = { "egress_qos_mapping", > BLOBMSG_TYPE_ARRAY }, > }; > > static const struct uci_blob_param_list vlandev_attr_list = { > @@ -122,13 +127,41 @@ static void > vlandev_free(struct device *dev) > { > struct vlandev_device *mvdev; > + struct vlan_qos_mapping *dep, *tmp; > > mvdev = container_of(dev, struct vlandev_device, dev); > device_remove_user(&mvdev->parent); > free(mvdev->config_data); > + list_for_each_entry_safe(dep, tmp, > &mvdev->config.ingress_qos_mapping_list, list) { > + list_del(&dep->list); > + free(dep); > + } The lines above can be replaced by vlist_simple_flush_all call when using vlist_simple_tree > > + list_for_each_entry_safe(dep, tmp, > &mvdev->config.egress_qos_mapping_list, list) { > + list_del(&dep->list); > + free(dep); > + } The lines above can be replaced by vlist_simple_flush_all when using vlist_simple_tree > > free(mvdev); > } > > +static void vlandev_qos_mapping_dump(struct blob_buf *b, const char *name, > const struct list_head *qos_mapping_li) > +{ > + const struct vlan_qos_mapping *dep; > + void *a, *t; > + > + a = blobmsg_open_array(b, name); > + > + list_for_each_entry(dep, qos_mapping_li, list) { > + t = blobmsg_open_table(b, NULL); > + > + blobmsg_add_u32(b, "from", dep->from); > + blobmsg_add_u32(b, "to", dep->to); > + > + blobmsg_close_table(b, t); > + } > + > + blobmsg_close_array(b, a); > +} > + > static void > vlandev_dump_info(struct device *dev, struct blob_buf *b) > { > @@ -137,6 +170,8 @@ vlandev_dump_info(struct device *dev, struct blob_buf *b) > mvdev = container_of(dev, struct vlandev_device, dev); > blobmsg_add_string(b, "parent", mvdev->parent.dev->ifname); > system_if_dump_info(dev, b); > + vlandev_qos_mapping_dump(b, "ingress_qos_mapping", > &mvdev->config.ingress_qos_mapping_list); > + vlandev_qos_mapping_dump(b, "egress_qos_mapping", > &mvdev->config.egress_qos_mapping_list); > } > > static void > @@ -152,18 +187,60 @@ vlandev_config_init(struct device *dev) > device_add_user(&mvdev->parent, basedev); > } > > +static void vlandev_qos_mapping_list_apply(struct list_head *qos_mapping_li, > struct blob_attr *list) > +{ > + struct blob_attr *cur; > + struct vlan_qos_mapping *qos_mapping; > + int rem, rc; > + > + blobmsg_for_each_attr(cur, list, rem) { > + if (blobmsg_type(cur) != BLOBMSG_TYPE_STRING) > + continue; > + > + if (!blobmsg_check_attr(cur, false)) > + continue; > + > + qos_mapping = calloc(1, sizeof(*qos_mapping)); > + if (!qos_mapping) > + continue; Keep coding style aligned by adding a new line after the continue statement > > + INIT_LIST_HEAD(&qos_mapping->list); > + rc = sscanf(blobmsg_data(cur), "%" PRIu32 ":%" PRIu32, > &qos_mapping->from, &qos_mapping->to); > + if (rc != 2) { > + free(qos_mapping); > + continue; > + } > + list_add_tail(&qos_mapping->list, qos_mapping_li); > + } > +} > + > static void > vlandev_apply_settings(struct vlandev_device *mvdev, struct blob_attr **tb) > { > struct vlandev_config *cfg = &mvdev->config; > + struct vlan_qos_mapping *dep, *tmp; > struct blob_attr *cur; > > cfg->proto = (mvdev->dev.type == &vlan8021q_device_type) ? > VLAN_PROTO_8021Q : VLAN_PROTO_8021AD; > cfg->vid = 1; > > + list_for_each_entry_safe(dep, tmp, &cfg->ingress_qos_mapping_list, > list) { > + list_del(&dep->list); > + free(dep); > + } This can be replaced by vlist_simple_update when using vlist_simple_tree > > + list_for_each_entry_safe(dep, tmp, &cfg->egress_qos_mapping_list, > list) { > + list_del(&dep->list); > + free(dep); > + } This can be replaced by vlist_simple_update when using vlist_simple_tree > > + > if ((cur = tb[VLANDEV_ATTR_VID])) > cfg->vid = (uint16_t) blobmsg_get_u32(cur); > + > + if ((cur = tb[VLANDEV_ATTR_INGRESS_QOS_MAPPING])) > + > vlandev_qos_mapping_list_apply(&cfg->ingress_qos_mapping_list, cur); > + > + if ((cur = tb[VLANDEV_ATTR_EGRESS_QOS_MAPPING])) > + vlandev_qos_mapping_list_apply(&cfg->egress_qos_mapping_list, > cur); At the end you need to call vlist_simple_flush to flush the unused entries Thx, Hans > > } > > static enum dev_change_type > @@ -221,6 +298,9 @@ vlandev_create(const char *name, struct device_type > *devtype, > if (!mvdev) > return NULL; > > + INIT_LIST_HEAD(&mvdev->config.ingress_qos_mapping_list); > + INIT_LIST_HEAD(&mvdev->config.egress_qos_mapping_list); > + > dev = &mvdev->dev; > > if (device_init(dev, devtype, name) < 0) { > -- > 2.26.2 > _______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
