Merge ~andre-ruiz/maas/+git/maas-fix-intel-cna-lldp:master into maas:master

[Andre Ruiz]

LP: #1750688 Disable embedded LLDP on Intel CNA cards

Improved lldp_install script to disable embedded lldp processing on Intel CNA cards using i40e driver so that the host can receive LLDP packets on those cards.

Implemented fix for lldp on intel cna cards

- Improved lldp_install script to disable embedded lldp processing
  on intel cna cards using i40e driver so that the host can receive
  lldp packets on those cards.

fixes: lp#1750688

[Andre Ruiz (andre-ruiz)]

Please note that the large number of changes in install_lldpd.py are mostly because of re-indentation of text when moving to inside the function.

[MAAS Lander (maas-lander)]

UNIT TESTS
-b master lp:~andre-ruiz/maas/+git/maas-fix-intel-cna-lldp into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8180/console
COMMIT: 8234e85b91b214cf14dff60c965d00b28762c594

review: Needs Fixing

153391d... by Andre Ruiz on 2020-08-25

Changes to make lint happy

[MAAS Lander (maas-lander)]

UNIT TESTS
-b master lp:~andre-ruiz/maas/+git/maas-fix-intel-cna-lldp into -b master lp:~maas-committers/maas

STATUS: FAILED
LOG: http://maas-ci.internal:8080/job/maas/job/branch-tester/8181/console
COMMIT: 153391d645efbe175edbead7398e4923561a62d0

review: Needs Fixing

[Andre Ruiz (andre-ruiz)]

I'm failing to see how this test failure is connected to the changes I made in a different part of the code.

At the same time, although I also get the same failure here on my side on the first run, it goes away (finishes ok) if I re-run the test just after it failed (??).

My last full test run just passed:

Ran 17326 tests in 2508.329s
OK (skipped=22)

Also, it always passes when I run that test alone, using "LC_ALL=en_US.UTF-8 ./bin/test.region -v src/provisioningserver/refresh/tests/test_refresh.py".

So I'm not sure what to do. Suggestions?

[Adam Collard (adam-collard)]

jenkins: !test

[MAAS Lander (maas-lander)]

UNIT TESTS
-b master lp:~andre-ruiz/maas/+git/maas-fix-intel-cna-lldp into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 153391d645efbe175edbead7398e4923561a62d0

review: Approve

[Adam Collard (adam-collard)]

review: Needs Fixing

1541067... by Andre Ruiz on 2020-08-26

Fixes per review requests

- moved static string attribution to outside of try/except
- added "\n" to command string
- swapped f-string for format() everywhere
- improved test function to check what was written too

[MAAS Lander (maas-lander)]

UNIT TESTS
-b master lp:~andre-ruiz/maas/+git/maas-fix-intel-cna-lldp into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 15410675b843458eb7f744ad47558d353038dd1b

review: Approve

[Adam Collard (adam-collard)]

review: Approve

diff --git a/src/metadataserver/builtin_scripts/commissioning_scripts/install_lldpd.py b/src/metadataserver/builtin_scripts/commissioning_scripts/install_lldpd.py
index e319775..f875011 100644
--- a/src/metadataserver/builtin_scripts/commissioning_scripts/install_lldpd.py
+++ b/src/metadataserver/builtin_scripts/commissioning_scripts/install_lldpd.py
@@ -25,43 +25,45 @@
 # packages: {apt: lldpd}
 # timeout: 30
 # --- End MAAS 1.0 script metadata ---
-"""Installs and configures `lldpd` for passive capture.
 
-`config_file` refers to a shell script that is sourced by
-`lldpd`'s init script, i.e. it's Upstart config on Ubuntu.
+from codecs import open
+import os
+from subprocess import check_call
 
-It selects the following options for the `lldpd` daemon:
 
--c  Enable the support of CDP protocol to deal with Cisco routers
-    that do not speak LLDP. If repeated, CDPv1 packets will be
-    sent even when there is no CDP peer detected.
+def lldpd_install(config_file):
+    """Installs and configures `lldpd` for passive capture.
 
--f  Enable the support of FDP protocol to deal with Foundry routers
-    that do not speak LLDP. If repeated, FDP packets will be sent
-    even when there is no FDP peer detected.
+    `config_file` refers to a shell script that is sourced by
+    `lldpd`'s init script, i.e. it's Upstart config on Ubuntu.
 
--s  Enable the support of SONMP protocol to deal with Nortel
-    routers and switches that do not speak LLDP. If repeated,
-    SONMP packets will be sent even when there is no SONMP peer
-    detected.
+    It selects the following options for the `lldpd` daemon:
 
--e  Enable the support of EDP protocol to deal with Extreme
-    routers and switches that do not speak LLDP. If repeated, EDP
-    packets will be sent even when there is no EDP peer detected.
+    -c  Enable the support of CDP protocol to deal with Cisco routers
+        that do not speak LLDP. If repeated, CDPv1 packets will be
+        sent even when there is no CDP peer detected.
 
--r  Receive-only mode. With this switch, lldpd will not send any
-    frame. It will only listen to neighbors.
+    -f  Enable the support of FDP protocol to deal with Foundry routers
+        that do not speak LLDP. If repeated, FDP packets will be sent
+        even when there is no FDP peer detected.
 
-These flags are chosen so that we're able to capture information
-from a broad spectrum of equipment, but without advertising the
-node's temporary presence.
+    -s  Enable the support of SONMP protocol to deal with Nortel
+        routers and switches that do not speak LLDP. If repeated,
+        SONMP packets will be sent even when there is no SONMP peer
+        detected.
 
-"""
-from codecs import open
-from subprocess import check_call
+    -e  Enable the support of EDP protocol to deal with Extreme
+        routers and switches that do not speak LLDP. If repeated, EDP
+        packets will be sent even when there is no EDP peer detected.
 
+    -r  Receive-only mode. With this switch, lldpd will not send any
+        frame. It will only listen to neighbors.
 
-def lldpd_install(config_file):
+    These flags are chosen so that we're able to capture information
+    from a broad spectrum of equipment, but without advertising the
+    node's temporary presence.
+
+    """
     print("INFO: Configuring lldpd...")
 
     with open(config_file, "a", "ascii") as fd:
@@ -73,5 +75,32 @@ def lldpd_install(config_file):
     check_call(("systemctl", "restart", "lldpd"))
 
 
+def disable_embedded_lldp_agent_in_intel_cna_cards():
+    """Intel cards that use i40e driver have an internal lldp processor
+    on the NIC that filter out lldp packets before they can reach the
+    host.  For the linux lldp daemon to receive such packets, we have to
+    disable that feature.
+
+    """
+    addr_path = "/sys/kernel/debug/i40e"
+    if not os.path.exists(addr_path):
+        return
+    for inner_dir in os.listdir(addr_path):
+        command_path = "{}/{}/command".format(addr_path, inner_dir)
+        try:
+            with open(command_path, "w", encoding="ascii") as command_file:
+                command_file.write("lldp stop\n")
+            print(
+                "INFO: Disabled embedded lldp agent for {}".format(inner_dir)
+            )
+        except (OSError, IOError):
+            print(
+                "WARNING: Failed to disable the embedded lldp agent for {}".format(
+                    inner_dir
+                )
+            )
+
+
 if __name__ == "__main__":
+    disable_embedded_lldp_agent_in_intel_cna_cards()
     lldpd_install("/etc/default/lldpd")
diff --git a/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_lldp.py b/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_lldp.py
index 0afdd92..dda2740 100644
--- a/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_lldp.py
+++ b/src/metadataserver/builtin_scripts/commissioning_scripts/tests/test_lldp.py
@@ -44,6 +44,26 @@ class TestLLDPScripts(MAASTestCase):
             config_observed = fd.read()
         self.assertEqual(config_expected, config_observed)
 
+    def test_install_script_disables_intel_lldp(self):
+        self.patch(os.path, "exists").return_value = True
+        self.patch(os, "listdir").return_value = ["0000:1a:00.0"]
+        temp_file = self.make_file("temp", "")
+        mock_open = self.patch(install_lldpd, "open")
+        mock_open.return_value = open(temp_file, "w", encoding="ascii")
+        install_lldpd.disable_embedded_lldp_agent_in_intel_cna_cards()
+        output_expected = "lldp stop\n".encode("ascii")
+        with open(temp_file, "rb") as fd:
+            output_observed = fd.read()
+        self.assertEqual(output_expected, output_observed)
+        self.assertThat(
+            mock_open,
+            MockCalledOnceWith(
+                "/sys/kernel/debug/i40e/0000:1a:00.0/command",
+                "w",
+                encoding="ascii",
+            ),
+        )
+
     def test_capture_lldpd_script_waits_for_lldpd(self):
         reference_file = self.make_file("reference")
         time_delay = 8.98  # seconds