From 60aa5c555d5d8a35f9632bc8f06b689d450e3636 Mon Sep 17 00:00:00 2001 From: Abhishek Dosi Date: Mon, 4 May 2020 10:22:03 -0700 Subject: [PATCH 1/3] Changes for LLDP for Multi NPU Platoforms:- a) Enable LLDP for Host namespace for Management Port b) Make sure Management IP is avaliable in per asic namespace needed for LLDP Chassis configuration c) Make sure chassis mac-address is correct in per asic namespace d) Do not run lldp on eth0 of per asic namespace and avoid chassis configuration for same e) Use Linux hostname instead from Device Metadata for lldp chassis configuration since in multi-npu platforms device metadata hostname will be differnt Signed-off-by: Abhishek Dosi --- dockers/docker-lldp-sv2/Dockerfile.j2 | 5 +++-- dockers/docker-lldp-sv2/docker-lldp-init.sh | 5 +++++ dockers/docker-lldp-sv2/lldpd.conf.j2 | 6 ++++-- dockers/docker-lldp-sv2/start.sh | 5 +++++ .../{supervisord.conf => supervisord.conf.j2} | 4 ++++ files/build_templates/lldp.service.j2 | 1 + src/sonic-config-engine/tests/sample_output/lldpd.conf | 2 +- 7 files changed, 23 insertions(+), 5 deletions(-) create mode 100755 dockers/docker-lldp-sv2/docker-lldp-init.sh rename dockers/docker-lldp-sv2/{supervisord.conf => supervisord.conf.j2} (87%) create mode 120000 files/build_templates/lldp.service.j2 diff --git a/dockers/docker-lldp-sv2/Dockerfile.j2 b/dockers/docker-lldp-sv2/Dockerfile.j2 index 6a720514ef9b..cb2a322b34af 100644 --- a/dockers/docker-lldp-sv2/Dockerfile.j2 +++ b/dockers/docker-lldp-sv2/Dockerfile.j2 @@ -35,12 +35,13 @@ RUN apt-get purge -y python-pip && \ /python-wheels \ ~/.cache +COPY ["docker-lldp-init.sh", "/usr/local/bin/"] COPY ["start.sh", "/usr/bin/"] -COPY ["supervisord.conf", "/etc/supervisor/conf.d/"] +COPY ["supervisord.conf.j2", "/usr/share/sonic/templates/"] COPY ["lldpd.conf.j2", "/usr/share/sonic/templates/"] COPY ["lldpd", "/etc/default/"] COPY ["lldpmgrd", "/usr/bin/"] COPY ["files/supervisor-proc-exit-listener", "/usr/bin"] COPY ["critical_processes", "/etc/supervisor"] -ENTRYPOINT ["/usr/bin/supervisord"] +ENTRYPOINT ["/usr/local/bin/docker-lldp-init.sh"] diff --git a/dockers/docker-lldp-sv2/docker-lldp-init.sh b/dockers/docker-lldp-sv2/docker-lldp-init.sh new file mode 100755 index 000000000000..ae507e8f506a --- /dev/null +++ b/dockers/docker-lldp-sv2/docker-lldp-init.sh @@ -0,0 +1,5 @@ +#!/usr/bin/env bash +#Generate supervisord.conf based on device metadata +mkdir -p /etc/supervisor/conf.d/ +sonic-cfggen -d -t /usr/share/sonic/templates/supervisord.conf.j2 > /etc/supervisor/conf.d/supervisord.conf +exec /usr/bin/supervisord diff --git a/dockers/docker-lldp-sv2/lldpd.conf.j2 b/dockers/docker-lldp-sv2/lldpd.conf.j2 index d5b6dba8700c..1b1cc1459a95 100644 --- a/dockers/docker-lldp-sv2/lldpd.conf.j2 +++ b/dockers/docker-lldp-sv2/lldpd.conf.j2 @@ -2,11 +2,13 @@ {# If MGMT port alias is available, use it for port ID subtype, otherwise use port name #} {% set mgmt_port_name = MGMT_INTERFACE.keys()[0][0] %} {% set ipv4 = MGMT_INTERFACE.keys()[0][1].split('/')[0] %} +configure system ip management pattern {{ ipv4 }} +{% if DEVICE_METADATA['localhost']['sub_role'] is not defined or DEVICE_METADATA['localhost']['sub_role']|length == 0 %} {% if MGMT_PORT and MGMT_PORT[mgmt_port_name] and MGMT_PORT[mgmt_port_name].alias %} configure ports eth0 lldp portidsubtype local {{ MGMT_PORT[mgmt_port_name].alias }} {% else %} configure ports eth0 lldp portidsubtype local {{ mgmt_port_name }} {% endif %} -configure system ip management pattern {{ ipv4 }} {% endif %} -configure system hostname {{ DEVICE_METADATA['localhost']['hostname'] }} +{% endif %} +configure system hostname ${hostname} diff --git a/dockers/docker-lldp-sv2/start.sh b/dockers/docker-lldp-sv2/start.sh index 76666e77aca2..9787e09b92d1 100755 --- a/dockers/docker-lldp-sv2/start.sh +++ b/dockers/docker-lldp-sv2/start.sh @@ -2,6 +2,11 @@ sonic-cfggen -d -t /usr/share/sonic/templates/lldpd.conf.j2 > /etc/lldpd.conf +# update the hostname. Not using device metadata as in multi-npu platform each namespace +# has its own device metadata hostname. Using the hostname set by docker using host enviroment + +sed -e "s/\${hostname}/$(hostname)/" -i /etc/lldpd.conf + mkdir -p /var/sonic echo "# Config files managed by sonic-config-engine" > /var/sonic/config_status diff --git a/dockers/docker-lldp-sv2/supervisord.conf b/dockers/docker-lldp-sv2/supervisord.conf.j2 similarity index 87% rename from dockers/docker-lldp-sv2/supervisord.conf rename to dockers/docker-lldp-sv2/supervisord.conf.j2 index 73ff52f4420e..beae3aa9425e 100644 --- a/dockers/docker-lldp-sv2/supervisord.conf +++ b/dockers/docker-lldp-sv2/supervisord.conf.j2 @@ -31,7 +31,11 @@ stderr_logfile=syslog # - `-dd` means to stay in foreground, log warnings to console # - `-ddd` means to stay in foreground, log warnings and info to console # - `-dddd` means to stay in foreground, log all to console +{% if DEVICE_METADATA['localhost']['sub_role'] is defined and DEVICE_METADATA['localhost']['sub_role']|length %} +command=/usr/sbin/lldpd -d -I Ethernet* -C Ethernet* +{% else %} command=/usr/sbin/lldpd -d -I Ethernet*,eth0 -C eth0 +{% endif %} priority=3 autostart=false autorestart=false diff --git a/files/build_templates/lldp.service.j2 b/files/build_templates/lldp.service.j2 new file mode 120000 index 000000000000..1adb318b9154 --- /dev/null +++ b/files/build_templates/lldp.service.j2 @@ -0,0 +1 @@ +per_namespace/lldp.service.j2 \ No newline at end of file diff --git a/src/sonic-config-engine/tests/sample_output/lldpd.conf b/src/sonic-config-engine/tests/sample_output/lldpd.conf index d28ec8418362..05d7bc0c1038 100644 --- a/src/sonic-config-engine/tests/sample_output/lldpd.conf +++ b/src/sonic-config-engine/tests/sample_output/lldpd.conf @@ -1,3 +1,3 @@ configure ports eth0 lldp portidsubtype local eth0 configure system ip management pattern 10.0.0.100 -configure system hostname switch-t0 +configure system hostname ${hostname} From f1bc3ebf697df8276061a1ad41f6351e112a2275 Mon Sep 17 00:00:00 2001 From: Abhishek Dosi Date: Wed, 6 May 2020 00:06:35 -0700 Subject: [PATCH 2/3] Address Review Comment with following changes: a) Use Device Metadata hostname even in per namespace conatiner. updated minigraph parsing for same to have hostname as system hostname and add new key for asic name b) Minigraph changes to have MGMT_INTERFACE Key in per asic/namespace config also as needed for LLDP for setting chassis management IP. Signed-off-by: Abhishek Dosi --- dockers/docker-lldp-sv2/lldpd.conf.j2 | 6 ++--- dockers/docker-lldp-sv2/start.sh | 5 ---- src/sonic-config-engine/minigraph.py | 24 ++++++++++++------- .../tests/sample_output/lldpd.conf | 2 +- .../tests/test_multinpu_cfggen.py | 5 ++-- 5 files changed, 22 insertions(+), 20 deletions(-) diff --git a/dockers/docker-lldp-sv2/lldpd.conf.j2 b/dockers/docker-lldp-sv2/lldpd.conf.j2 index 1b1cc1459a95..d5b6dba8700c 100644 --- a/dockers/docker-lldp-sv2/lldpd.conf.j2 +++ b/dockers/docker-lldp-sv2/lldpd.conf.j2 @@ -2,13 +2,11 @@ {# If MGMT port alias is available, use it for port ID subtype, otherwise use port name #} {% set mgmt_port_name = MGMT_INTERFACE.keys()[0][0] %} {% set ipv4 = MGMT_INTERFACE.keys()[0][1].split('/')[0] %} -configure system ip management pattern {{ ipv4 }} -{% if DEVICE_METADATA['localhost']['sub_role'] is not defined or DEVICE_METADATA['localhost']['sub_role']|length == 0 %} {% if MGMT_PORT and MGMT_PORT[mgmt_port_name] and MGMT_PORT[mgmt_port_name].alias %} configure ports eth0 lldp portidsubtype local {{ MGMT_PORT[mgmt_port_name].alias }} {% else %} configure ports eth0 lldp portidsubtype local {{ mgmt_port_name }} {% endif %} +configure system ip management pattern {{ ipv4 }} {% endif %} -{% endif %} -configure system hostname ${hostname} +configure system hostname {{ DEVICE_METADATA['localhost']['hostname'] }} diff --git a/dockers/docker-lldp-sv2/start.sh b/dockers/docker-lldp-sv2/start.sh index 9787e09b92d1..76666e77aca2 100755 --- a/dockers/docker-lldp-sv2/start.sh +++ b/dockers/docker-lldp-sv2/start.sh @@ -2,11 +2,6 @@ sonic-cfggen -d -t /usr/share/sonic/templates/lldpd.conf.j2 > /etc/lldpd.conf -# update the hostname. Not using device metadata as in multi-npu platform each namespace -# has its own device metadata hostname. Using the hostname set by docker using host enviroment - -sed -e "s/\${hostname}/$(hostname)/" -i /etc/lldpd.conf - mkdir -p /var/sonic echo "# Config files managed by sonic-config-engine" > /var/sonic/config_status diff --git a/src/sonic-config-engine/minigraph.py b/src/sonic-config-engine/minigraph.py index 8ff4944c5d13..7190135e8316 100644 --- a/src/sonic-config-engine/minigraph.py +++ b/src/sonic-config-engine/minigraph.py @@ -248,14 +248,24 @@ def parse_asic_png(png, asic_name, hostname): return (neighbors, devices, port_speeds) def parse_dpg(dpg, hname): + aclintfs = None + mgmtintfs = None for child in dpg: - """In Multi-NPU platforms the acl intfs are defined only for the host not for individual asic. + """ + In Multi-NPU platforms the acl intfs are defined only for the host not for individual asic. There is just one aclintf node in the minigraph - Get the aclintfs node first. + Get the aclintfs node first. """ - if child.find(str(QName(ns, "AclInterfaces"))) is not None: + if aclintfs is None and child.find(str(QName(ns, "AclInterfaces"))) is not None: aclintfs = child.find(str(QName(ns, "AclInterfaces"))) - + """ + In Multi-NPU platforms the mgmt intfs are defined only for the host not for individual asic + There is just one mgmtintf node in the minigraph + Get the mgmtintfs node first. We need mgmt intf to get mgmt ip in per asic dockers. + """ + if mgmtintfs is None and child.find(str(QName(ns, "ManagementIPInterfaces"))) is not None: + mgmtintfs = child.find(str(QName(ns, "ManagementIPInterfaces"))) + hostname = child.find(str(QName(ns, "Hostname"))) if hostname.text.lower() != hname.lower(): continue @@ -291,7 +301,6 @@ def parse_dpg(dpg, hname): mvrf_en_flag = mv.find(str(QName(ns, "mgmtVrfEnabled"))).text mvrf["vrf_global"] = {"mgmtVrfEnabled": mvrf_en_flag} - mgmtintfs = child.find(str(QName(ns, "ManagementIPInterfaces"))) mgmt_intf = {} for mgmtintf in mgmtintfs.findall(str(QName(ns1, "ManagementIPInterface"))): intfname = mgmtintf.find(str(QName(ns, "AttachTo"))).text @@ -767,10 +776,8 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None): if asic_name is None: current_device = [devices[key] for key in devices if key.lower() == hostname.lower()][0] - name = hostname else: current_device = [devices[key] for key in devices if key.lower() == asic_name.lower()][0] - name = asic_name results = {} results['DEVICE_METADATA'] = {'localhost': { @@ -778,7 +785,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None): 'deployment_id': deployment_id, 'region': region, 'docker_routing_config_mode': docker_routing_config_mode, - 'hostname': name, + 'hostname': hostname, 'hwsku': hwsku, 'type': current_device['type'] } @@ -788,6 +795,7 @@ def parse_xml(filename, platform=None, port_config_file=None, asic_name=None): if sub_role is not None: current_device['sub_role'] = sub_role results['DEVICE_METADATA']['localhost']['sub_role'] = sub_role + results['DEVICE_METADATA']['localhost']['asic_name'] = asic_name results['BGP_NEIGHBOR'] = bgp_sessions results['BGP_MONITORS'] = bgp_monitors results['BGP_PEER_RANGE'] = bgp_peers_with_range diff --git a/src/sonic-config-engine/tests/sample_output/lldpd.conf b/src/sonic-config-engine/tests/sample_output/lldpd.conf index 05d7bc0c1038..d28ec8418362 100644 --- a/src/sonic-config-engine/tests/sample_output/lldpd.conf +++ b/src/sonic-config-engine/tests/sample_output/lldpd.conf @@ -1,3 +1,3 @@ configure ports eth0 lldp portidsubtype local eth0 configure system ip management pattern 10.0.0.100 -configure system hostname ${hostname} +configure system hostname switch-t0 diff --git a/src/sonic-config-engine/tests/test_multinpu_cfggen.py b/src/sonic-config-engine/tests/test_multinpu_cfggen.py index 6facae0451db..4aefbb78fc61 100644 --- a/src/sonic-config-engine/tests/test_multinpu_cfggen.py +++ b/src/sonic-config-engine/tests/test_multinpu_cfggen.py @@ -116,7 +116,7 @@ def test_mgmt_port(self): self.assertDictEqual(output, {'eth0': {'alias': 'eth0', 'admin_status': 'up'}}) for asic in range(NUM_ASIC): output = json.loads(self.run_script_for_asic(argument, asic, self.port_config[asic])) - self.assertDictEqual(output, {}) + self.assertDictEqual(output, {'eth0': {'alias': 'eth0', 'admin_status': 'up'}}) def test_frontend_asic_portchannels(self): argument = "-m {} -p {} -n asic0 --var-json \"PORTCHANNEL\"".format(self.sample_graph, self.port_config[0]) @@ -213,7 +213,8 @@ def test_device_asic_metadata(self): for asic in range(NUM_ASIC): output = json.loads(self.run_script_for_asic(argument, asic,self.port_config[asic])) asic_name = "asic{}".format(asic) - self.assertEqual(output['localhost']['hostname'], asic_name) + self.assertEqual(output['localhost']['hostname'], 'multi_npu_platform_01') + self.assertEqual(output['localhost']['asic_name'], asic_name) self.assertEqual(output['localhost']['type'], 'Asic') if asic == 0 or asic == 1: self.assertEqual(output['localhost']['sub_role'], 'FrontEnd') From 358b2e49bb0a74a8ef1ef7d2794e2056ae94184b Mon Sep 17 00:00:00 2001 From: Abhishek Dosi Date: Sat, 9 May 2020 22:51:29 -0700 Subject: [PATCH 3/3] Address Review Comments --- dockers/docker-lldp-sv2/Dockerfile.j2 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dockers/docker-lldp-sv2/Dockerfile.j2 b/dockers/docker-lldp-sv2/Dockerfile.j2 index cb2a322b34af..af2b0373c373 100644 --- a/dockers/docker-lldp-sv2/Dockerfile.j2 +++ b/dockers/docker-lldp-sv2/Dockerfile.j2 @@ -35,7 +35,7 @@ RUN apt-get purge -y python-pip && \ /python-wheels \ ~/.cache -COPY ["docker-lldp-init.sh", "/usr/local/bin/"] +COPY ["docker-lldp-init.sh", "/usr/bin/"] COPY ["start.sh", "/usr/bin/"] COPY ["supervisord.conf.j2", "/usr/share/sonic/templates/"] COPY ["lldpd.conf.j2", "/usr/share/sonic/templates/"] @@ -44,4 +44,4 @@ COPY ["lldpmgrd", "/usr/bin/"] COPY ["files/supervisor-proc-exit-listener", "/usr/bin"] COPY ["critical_processes", "/etc/supervisor"] -ENTRYPOINT ["/usr/local/bin/docker-lldp-init.sh"] +ENTRYPOINT ["/usr/bin/docker-lldp-init.sh"]