From: Radhika Mahankali Date: Fri, 6 Dec 2019 19:25:48 +0000 (-0800) Subject: interfaces: fix for limitation of 10 VLANs for LLDP .1q feature X-Git-Tag: 1.0.5~17^2~2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=626ea3610fa365ccfece217085c84b7d699a700b;p=thirdparty%2Flldpd.git interfaces: fix for limitation of 10 VLANs for LLDP .1q feature Max 10 VLAN ids are supported on a port for .1q feature Root Cause: All the VLANs learnt from netlink is dropped after 10 VLANs due to the static array allocation of only 10 VLAN ids in the interface structure. Beyond 10 VLAN membership for a port are ignored and error message gets printed causing flooding of messages when hundreds of VLANs are configured. Fix: Changed the static VLAN id array to VLAN id bitmap. With the bitmap all 4k VLANs can be stored and learnt from netlink messages. - Added a message to indicate when the LLDP packet is not sent out because its too big. This will be helpful for user when too many VLANs are configured and LLDP packets are not sent out. Limitation: Even though the VLAN learning from netlink messages has been alleviated, due to the LLDP message size around 380 VLANs can be advertised in the packet. This number can vary based on the number of other TLVs being advertised by LLDP. vlan info shows interface_name.vlan-id which makes it look like sub interface and causes confusion Root Cause: Vlan name is sent as part of the .1q TLVs. But, the vlan name format was . which makes it look like a sub-interface. Fix: The vlan name cannot be removed from the vlan-info display since it is sent/received as part of the .1q TLV in LLDP packet. But, changed the vlan name format to vlan- to avoid the confusion. (cherry picked from commit 38db598121f5ce615f98d6cdaf41d5360c40dc3c) Conflicts: src/daemon/interfaces-linux.c src/daemon/lldpd.h --- diff --git a/NEWS b/NEWS index 9c414cf6..2f2756b0 100644 --- a/NEWS +++ b/NEWS @@ -7,6 +7,8 @@ lldpd (1.0.5) + Add support for VLAN-aware bridges for Linux (no range support). + Add support for 802.3BT (no SNMP support). + Add support for millisecond-grained tx-interval (Jean-Pierre Tosoni). + + Use generic names for VLAN names, instead of interface names (eg + vlan100 instead of eth1.100). * Fix: + Don't clear chassis TLV on shutdown LLDPDU. + Don't require/display powerpairs for Dot3 power when device type diff --git a/src/daemon/interfaces-bsd.c b/src/daemon/interfaces-bsd.c index 8ebd1712..ee35fc26 100644 --- a/src/daemon/interfaces-bsd.c +++ b/src/daemon/interfaces-bsd.c @@ -307,7 +307,7 @@ ifbsd_check_vlan(struct lldpd *cfg, "%s is VLAN %d of %s", vlan->name, vreq.vlr_tag, lower->name); vlan->lower = lower; - vlan->vlanids[0] = vreq.vlr_tag; + bitmap_set(vlan->vlan_bmap, vreq.vlr_tag); vlan->type |= IFACE_VLAN_T; } diff --git a/src/daemon/interfaces-linux.c b/src/daemon/interfaces-linux.c index 1e3aaa5c..56931bf6 100644 --- a/src/daemon/interfaces-linux.c +++ b/src/daemon/interfaces-linux.c @@ -222,7 +222,7 @@ iflinux_is_vlan(struct lldpd *cfg, } iface->lower = lower; - iface->vlanids[0] = ifv.u.VID; + bitmap_set(iface->vlan_bmap, ifv.u.VID); return 1; } #endif diff --git a/src/daemon/interfaces.c b/src/daemon/interfaces.c index e936e766..80324566 100644 --- a/src/daemon/interfaces.c +++ b/src/daemon/interfaces.c @@ -212,6 +212,7 @@ iface_append_vlan(struct lldpd *cfg, struct lldpd_port *port; struct lldpd_vlan *v; char *name = NULL; + uint16_t vlan_id; if (hardware == NULL) { log_debug("interfaces", @@ -221,39 +222,38 @@ iface_append_vlan(struct lldpd *cfg, } port = &hardware->h_lport; - for (int i = 0; - i < sizeof(vlan->vlanids)/sizeof(vlan->vlanids[0]) && vlan->vlanids[i] != 0; - i++) { - /* Compute name */ - if (vlan->vlanids[1] == 0) { - /* Only one VLAN. */ - if ((name = strdup(vlan->name)) == NULL) - return; - } else { - if (asprintf(&name, "%s.%d", vlan->name, vlan->vlanids[i]) == -1) + for (int i = 0; (i < VLAN_BITMAP_LEN); i++) { + if (vlan->vlan_bmap[i] == 0) + continue; + for (unsigned bit = 0; bit < 32; bit++) { + uint32_t mask = 1L << bit; + if (!(vlan->vlan_bmap[i] & mask)) + continue; + vlan_id = (i * 32) + bit; + if (asprintf(&name, "vlan%d", vlan_id) == -1) return; - } - /* Check if the VLAN is already here. */ - TAILQ_FOREACH(v, &port->p_vlans, v_entries) - if (strncmp(name, v->v_name, IFNAMSIZ) == 0) { - free(name); - return; - } + /* Check if the VLAN is already here. */ + TAILQ_FOREACH(v, &port->p_vlans, v_entries) + if (strncmp(name, v->v_name, IFNAMSIZ) == 0) { + free(name); + return; + } - if ((v = (struct lldpd_vlan *) - calloc(1, sizeof(struct lldpd_vlan))) == NULL) { - free(name); - return; + if ((v = (struct lldpd_vlan *) + calloc(1, sizeof(struct lldpd_vlan))) == NULL) { + free(name); + return; + } + v->v_name = name; + v->v_vid = vlan_id; + if (vlan->pvid) + port->p_pvid = vlan->pvid; + log_debug("interfaces", "append VLAN %s for %s", + v->v_name, + hardware->h_ifname); + TAILQ_INSERT_TAIL(&port->p_vlans, v, v_entries); } - v->v_name = name; - v->v_vid = vlan->vlanids[i]; - if (vlan->pvid) - port->p_pvid = vlan->pvid; - log_debug("interfaces", "append VLAN %s for %s", - v->v_name, - hardware->h_ifname); - TAILQ_INSERT_TAIL(&port->p_vlans, v, v_entries); } } @@ -322,7 +322,7 @@ interfaces_helper_vlan(struct lldpd *cfg, struct interfaces_device *iface; TAILQ_FOREACH(iface, interfaces, next) { - if (!(iface->type & IFACE_VLAN_T) && iface->vlanids[0] == 0) + if (!(iface->type & IFACE_VLAN_T) && is_bitmap_empty(iface->vlan_bmap)) continue; /* We need to find the physical interfaces of this diff --git a/src/daemon/lldpd.h b/src/daemon/lldpd.h index 13b8225b..7d73a0c9 100644 --- a/src/daemon/lldpd.h +++ b/src/daemon/lldpd.h @@ -303,6 +303,11 @@ void interfaces_update(struct lldpd *); #define IFACE_BOND_T (1 << 2) /* Bond interface */ #define IFACE_VLAN_T (1 << 3) /* VLAN interface */ #define IFACE_WIRELESS_T (1 << 4) /* Wireless interface */ +#define IFACE_BRIDGE_PORT_T (1 << 5) /* Bridge Port */ +#define IFACE_BOND_SLAVE_T (1 << 6) /* Bond slave */ + +#define MAX_VLAN 4096 +#define VLAN_BITMAP_LEN (MAX_VLAN / 32) struct interfaces_device { TAILQ_ENTRY(interfaces_device) next; int ignore; /* Ignore this interface */ @@ -314,7 +319,7 @@ struct interfaces_device { int flags; /* Flags (IFF_*) */ int mtu; /* MTU */ int type; /* Type (see IFACE_*_T) */ - int vlanids[10]; /* If a VLAN, what are the VLAN ID? */ + uint32_t vlan_bmap[VLAN_BITMAP_LEN]; /* If a VLAN, what are the VLAN ID? */ int pvid; /* If a VLAN, what is the default VLAN? */ struct interfaces_device *lower; /* Lower interface (for a VLAN for example) */ struct interfaces_device *upper; /* Upper interface (for a bridge or a bond) */ @@ -384,6 +389,9 @@ void netlink_cleanup(struct lldpd *); struct lldpd_netlink; #endif +void bitmap_set(uint32_t *bmap, uint16_t vlan_id); +int is_bitmap_empty(uint32_t *bmap); + #ifndef HOST_OS_LINUX int ifbpf_phys_init(struct lldpd *, struct lldpd_hardware *); #endif diff --git a/src/daemon/netlink.c b/src/daemon/netlink.c index f017ffe3..a9a7d54e 100644 --- a/src/daemon/netlink.c +++ b/src/daemon/netlink.c @@ -45,6 +45,53 @@ struct lldpd_netlink { struct interfaces_address_list *addresses; }; +/* + * Set vlan id in the bitmap + */ +void +bitmap_set(uint32_t *bmap, uint16_t vlan_id) +{ + if (vlan_id < MAX_VLAN) + bmap[vlan_id / 32] |= (((uint32_t) 1) << (vlan_id % 32)); +} + +/* + * Checks if the bitmap is empty + */ +int +is_bitmap_empty(uint32_t *bmap) +{ + int i; + + for (i = 0; i < VLAN_BITMAP_LEN; i++) { + if (bmap[i] != 0) + return 0; + } + + return 1; +} + +/* + * Calculate the number of bits set in the bitmap to get total + * number of VLANs + */ +static int +num_bits_set(uint32_t *bmap) +{ + int num = 0; + int i, bit; + + for (i = 0; (i < VLAN_BITMAP_LEN); i++) { + if (!bmap[i]) + continue; + for (bit = 0; bit < 32; bit++) { + if (bmap[i] & (1 << bit)) + num++; + } + } + + return num; +} /** * Set netlink socket buffer size. @@ -207,6 +254,7 @@ netlink_parse_linkinfo(struct interfaces_device *iff, struct rtattr *rta, int le { struct rtattr *link_info_attrs[IFLA_INFO_MAX+1] = {}; char *kind = NULL; + uint16_t vlan_id; netlink_parse_rtattr(link_info_attrs, IFLA_INFO_MAX, rta, len); @@ -240,9 +288,10 @@ netlink_parse_linkinfo(struct interfaces_device *iff, struct rtattr *rta, int le RTA_PAYLOAD(link_info_attrs[IFLA_INFO_DATA])); if (vlan_link_info_data_attrs[IFLA_VLAN_ID]) { - iff->vlanids[0] = *(uint16_t *)RTA_DATA(vlan_link_info_data_attrs[IFLA_VLAN_ID]); + vlan_id = *(uint16_t *)RTA_DATA(vlan_link_info_data_attrs[IFLA_VLAN_ID]); + bitmap_set(iff->vlan_bmap, vlan_id); log_debug("netlink", "VLAN ID for interface %s is %d", - iff->name, iff->vlanids[0]); + iff->name, vlan_id); } } @@ -259,7 +308,6 @@ netlink_parse_linkinfo(struct interfaces_device *iff, struct rtattr *rta, int le static void netlink_parse_afspec(struct interfaces_device *iff, struct rtattr *rta, int len) { - int i = 0; while (RTA_OK(rta, len)) { struct bridge_vlan_info *vinfo; switch (rta->rta_type) { @@ -267,12 +315,8 @@ netlink_parse_afspec(struct interfaces_device *iff, struct rtattr *rta, int len) vinfo = RTA_DATA(rta); log_debug("netlink", "found VLAN %d on interface %s", vinfo->vid, iff->name ? iff->name : "(unknown)"); - if (i >= sizeof(iff->vlanids)/sizeof(iff->vlanids[0])) { - log_info("netlink", "too many VLANs on interface %s", - iff->name ? iff->name : "(unknown)"); - break; - } - iff->vlanids[i++] = vinfo->vid; + + bitmap_set(iff->vlan_bmap, vinfo->vid); if (vinfo->flags & (BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED)) iff->pvid = vinfo->vid; break; @@ -284,10 +328,11 @@ netlink_parse_afspec(struct interfaces_device *iff, struct rtattr *rta, int len) rta = RTA_NEXT(rta, len); } /* All enbridged interfaces will have VLAN 1 by default, ignore it */ - if (iff->vlanids[0] == 1 && iff->vlanids[1] == 0 && iff->pvid == 1) { + if (iff->vlan_bmap[0] == 1 && (num_bits_set(iff->vlan_bmap) == 1) + && iff->pvid == 1) { log_debug("netlink", "found only default VLAN 1 on interface %s, removing", iff->name ? iff->name : "(unknown)"); - iff->vlanids[0] = iff->pvid = 0; + iff->vlan_bmap[0] = iff->pvid = 0; } } @@ -486,8 +531,10 @@ netlink_merge(struct interfaces_device *old, struct interfaces_device *new) new->mtu = old->mtu; if (new->type == 0) new->type = old->type; - if (new->vlanids[0] == 0 && new->type == IFACE_VLAN_T) - new->vlanids[0] = old->vlanids[0]; + + if (is_bitmap_empty(new->vlan_bmap) && new->type == IFACE_VLAN_T) + memcpy((void *)new->vlan_bmap, (void *)old->vlan_bmap, + sizeof(uint32_t) * VLAN_BITMAP_LEN); /* It's not possible for lower link to change */ new->lower_idx = old->lower_idx; diff --git a/src/daemon/protocols/lldp.c b/src/daemon/protocols/lldp.c index a6705038..a5d93528 100644 --- a/src/daemon/protocols/lldp.c +++ b/src/daemon/protocols/lldp.c @@ -518,6 +518,7 @@ end: return 0; toobig: + log_info("lldp", "Cannot send LLDP packet for %s, Too big message", p_id); free(packet); return E2BIG; } diff --git a/tests/integration/test_interfaces.py b/tests/integration/test_interfaces.py index db73c391..29d7ad4f 100644 --- a/tests/integration/test_interfaces.py +++ b/tests/integration/test_interfaces.py @@ -83,7 +83,7 @@ def test_vlan_aware_bridge_with_vlan(lldpd1, lldpd, lldpcli, namespaces, links, out = lldpcli("-f", "keyvalue", "show", "neighbors", "details") assert out['lldp.eth0.port.descr'] == 'eth1' assert out['lldp.eth0.vlan'] == \ - ['eth1.100', 'eth1.200', 'eth1.300'] + ['vlan100', 'vlan200', 'vlan300'] assert out['lldp.eth0.vlan.vlan-id'] == \ ['100', '200', '300'] assert out['lldp.eth0.vlan.pvid'] == \ @@ -217,10 +217,7 @@ def test_remove_vlan(lldpd1, lldpd, lldpcli, namespaces, links, kind): with namespaces(1): out = lldpcli("-f", "keyvalue", "show", "neighbors", "details") assert out['lldp.eth0.port.descr'] == 'eth1' - if kind != 'vlan-aware-bridge': - assert out['lldp.eth0.vlan'] == ['vlan400', 'vlan500'] - else: - assert out['lldp.eth0.vlan'] == ['eth1.400', 'eth1.500'] + assert out['lldp.eth0.vlan'] == ['vlan400', 'vlan500'] assert out['lldp.eth0.vlan.vlan-id'] == ['400', '500'] assert out['lldp.eth0.vlan.pvid'] == ['no', 'no']