Add the SPAN port mirroring recipe for switchdev, and several lnst features that were missing and were needed for that recipe.
Among the features added: - The lnst slave now cleans the tc qdiscs and filters at the clean procedure. - The PacketAssert module supports setting promiscuos flag. - The Switchdev TestLib IcmpPing module supports setting ping parameters.
The SPAN recipe consist of two machines connected via a bridge in the switch, and pinging each other, while ingress/egress port mirrors are added and tested.
Yotam Gigi (5): Controller: Task: Add documentation for one of the flags in run func. switchdev: TestLib: Add controllable parameters to IcmpPing module. PacketAssert: Add the promiscuous optional param. Slave: Add basic support for tc. recipes: switchdev: Add the SPAN recipe.
lnst/Controller/Task.py | 2 + lnst/Slave/InterfaceManager.py | 28 +++++++++ recipes/switchdev/TestLib.py | 10 ++-- recipes/switchdev/l2-021-span.py | 119 ++++++++++++++++++++++++++++++++++++++ recipes/switchdev/l2-021-span.xml | 26 +++++++++ test_modules/PacketAssert.py | 6 +- 6 files changed, 185 insertions(+), 6 deletions(-) create mode 100644 recipes/switchdev/l2-021-span.py create mode 100644 recipes/switchdev/l2-021-span.xml
Add documentation for the flag save_output on the Task.run function.
Signed-off-by: Yotam Gigi yotamg@mellanox.com --- lnst/Controller/Task.py | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py index 6f36e86..be53498 100644 --- a/lnst/Controller/Task.py +++ b/lnst/Controller/Task.py @@ -236,6 +236,8 @@ class HostAPI(object):
:param bg: Run in background flag. :type bg: bool + :param save_output: Save the process output in result + :type save_output: bool :param expect: "pass" or "fail". :type expect: string :param timeout: A time limit in seconds.
Add the count and interval optional parameters for IcmpPing module. The change is backward-compatible, as the defalt values remain unchanged.
Signed-off-by: Yotam Gigi yotamg@mellanox.com --- recipes/switchdev/TestLib.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/recipes/switchdev/TestLib.py b/recipes/switchdev/TestLib.py index 152e76e..5b93efd 100644 --- a/recipes/switchdev/TestLib.py +++ b/recipes/switchdev/TestLib.py @@ -75,7 +75,7 @@ class TestLib: m2.run(linkneg_mod, desc=desc, netns=if2.get_netns())
def ping_simple(self, if1, if2, fail_expected=False, desc=None, - limit_rate=90): + limit_rate=90, count=100, interval=0.2): if not desc: desc = self._generate_default_desc(if1, [if2])
@@ -88,16 +88,16 @@ class TestLib: ping_mod = self._ctl.get_module("IcmpPing", options={ "addr": if2.get_ip(0), - "count": 100, - "interval": 0.2, + "count": count, + "interval": interval, "iface" : if1.get_devname(), "limit_rate": limit_rate})
ping_mod6 = self._ctl.get_module("Icmp6Ping", options={ "addr": if2.get_ip(1), - "count": 100, - "interval": 0.2, + "count": count, + "interval": interval, "iface" : if1.get_ip(1), "limit_rate": limit_rate})
The promiscuos parameter is passed to the tcpdump that is ran by the PacketAssert module. This way, the user can control whether he wants to make a promiscuos snif or not.
Signed-off-by: Yotam Gigi yotamg@mellanox.com --- test_modules/PacketAssert.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/test_modules/PacketAssert.py b/test_modules/PacketAssert.py index 49790b9..1f3c4cb 100644 --- a/test_modules/PacketAssert.py +++ b/test_modules/PacketAssert.py @@ -61,10 +61,14 @@ class PacketAssert(TestGeneric):
interface = self.get_mopt("interface") pcap_filter = self.get_opt("filter") + promiscuous = self.get_opt("promiscuous", default=False) if not pcap_filter: pcap_filter = ""
- cmd = "tcpdump -p -nn -i %s "%s"" % (interface, pcap_filter) + promiscuous_str = "" if promiscuous else "-p" + + cmd = "tcpdump %s -nn -i %s "%s"" % (promiscuous_str, interface, + pcap_filter) logging.debug("PacketAssert tcpdump command: %s" % cmd) self._cmd = cmd
Mon, Aug 15, 2016 at 10:05:52AM CEST, yotamg@mellanox.com wrote:
The promiscuos parameter is passed to the tcpdump that is ran by the PacketAssert module. This way, the user can control whether he wants to make a promiscuos snif or not.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
test_modules/PacketAssert.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/test_modules/PacketAssert.py b/test_modules/PacketAssert.py index 49790b9..1f3c4cb 100644 --- a/test_modules/PacketAssert.py +++ b/test_modules/PacketAssert.py @@ -61,10 +61,14 @@ class PacketAssert(TestGeneric):
interface = self.get_mopt("interface") pcap_filter = self.get_opt("filter")
promiscuous = self.get_opt("promiscuous", default=False)
Default should be true so we don't break existing users
if not pcap_filter: pcap_filter = ""
cmd = "tcpdump -p -nn -i %s \"%s\"" % (interface, pcap_filter)
promiscuous_str = "" if promiscuous else "-p"cmd = "tcpdump %s -nn -i %s \"%s\"" % (promiscuous_str, interface,pcap_filter) logging.debug("PacketAssert tcpdump command: %s" % cmd) self._cmd = cmd-- 2.4.11 _______________________________________________ LNST-developers mailing list lnst-developers@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahoste...
-----Original Message----- From: Jiri Pirko [mailto:jiri@resnulli.us] Sent: Monday, August 15, 2016 11:38 AM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com; Ido Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; jpirko@mellanox.com Subject: Re: [PATCH 3/5] PacketAssert: Add the promiscuous optional param.
Mon, Aug 15, 2016 at 10:05:52AM CEST, yotamg@mellanox.com wrote:
The promiscuos parameter is passed to the tcpdump that is ran by the PacketAssert module. This way, the user can control whether he wants to make a promiscuos snif or not.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
test_modules/PacketAssert.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/test_modules/PacketAssert.py b/test_modules/PacketAssert.py index 49790b9..1f3c4cb 100644 --- a/test_modules/PacketAssert.py +++ b/test_modules/PacketAssert.py @@ -61,10 +61,14 @@ class PacketAssert(TestGeneric):
interface = self.get_mopt("interface") pcap_filter = self.get_opt("filter")
promiscuous = self.get_opt("promiscuous", default=False)Default should be true so we don't break existing users
The default behavior was no promiscuous (which means, with the '-p' flag). As far as I see, the default behavior did not change due to that commit.
if not pcap_filter: pcap_filter = ""
cmd = "tcpdump -p -nn -i %s \"%s\"" % (interface, pcap_filter)
promiscuous_str = "" if promiscuous else "-p"cmd = "tcpdump %s -nn -i %s \"%s\"" % (promiscuous_str, interface,pcap_filter) logging.debug("PacketAssert tcpdump command: %s" % cmd) self._cmd = cmd-- 2.4.11 _______________________________________________ LNST-developers mailing list lnst-developers@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedora hosted.org
Add the tc filter clear and tc qdisc clear commands to the Interface clear_configuration of the InterfaceManager.
Signed-off-by: Yotam Gigi yotamg@mellanox.com --- lnst/Slave/InterfaceManager.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index dbb0771..a4d6117 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -519,6 +519,32 @@ class Device(object): self._conf = None self._conf_dict = None
+ def _clear_tc_qdisc(self): + exec_cmd("tc qdisc replace dev %s root pfifo" % self._name) + out, _ = exec_cmd("tc filter show dev %s" % self._name) + ingress_handles = re.findall("ingress (\d+):", out) + for ingress_handle in ingress_handles: + exec_cmd("tc qdisc del dev %s handle %s: ingress" % + (self._name, ingress_handle)) + out, _ = exec_cmd("tc qdisc show dev %s" % self._name) + ingress_qdiscs = re.findall("qdisc ingress (\w+):", out) + if len(ingress_qdiscs) != 0: + exec_cmd("tc qdisc del dev %s ingress" % self._name) + + def _clear_tc_filters(self): + out, _ = exec_cmd("tc filter show dev %s" % self._name) + egress_prefs = re.findall("pref (\d+) .* handle", out) + out, _ = exec_cmd("tc filter show dev %s ingress" % self._name) + ingress_prefs = re.findall("pref (\d+) .* handle", out) + + for egress_pref in egress_prefs: + exec_cmd("tc filter del dev %s pref %s" % (self._name, + egress_pref)) + + for ingress_pref in ingress_prefs: + exec_cmd("tc filter del dev %s pref %s ingress" % (self._name, + ingress_pref)) + def clear_configuration(self): if self._master["primary"]: primary_id = self._master["primary"] @@ -532,6 +558,8 @@ class Device(object): m_dev.clear_configuration()
if self._conf != None: + self._clear_tc_qdisc() + self._clear_tc_filters() self.down() self.deconfigure() self.destroy()
Mon, Aug 15, 2016 at 10:05:53AM CEST, yotamg@mellanox.com wrote:
Add the tc filter clear and tc qdisc clear commands to the Interface clear_configuration of the InterfaceManager.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
lnst/Slave/InterfaceManager.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index dbb0771..a4d6117 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -519,6 +519,32 @@ class Device(object): self._conf = None self._conf_dict = None
- def _clear_tc_qdisc(self):
exec_cmd("tc qdisc replace dev %s root pfifo" % self._name)
Hi, just noticed that this can change the original qdisc setting on the device.
The pfifo qdisc does not have to be default, e.g. on RHEL7 I see it's mq that enables multiqueue. This should be fixed so that original setting is restored.
out, _ = exec_cmd("tc filter show dev %s" % self._name)ingress_handles = re.findall("ingress (\\d+):", out)for ingress_handle in ingress_handles:exec_cmd("tc qdisc del dev %s handle %s: ingress" %(self._name, ingress_handle))out, _ = exec_cmd("tc qdisc show dev %s" % self._name)ingress_qdiscs = re.findall("qdisc ingress (\\w+):", out)if len(ingress_qdiscs) != 0:exec_cmd("tc qdisc del dev %s ingress" % self._name)- def _clear_tc_filters(self):
out, _ = exec_cmd("tc filter show dev %s" % self._name)egress_prefs = re.findall("pref (\\d+) .* handle", out)out, _ = exec_cmd("tc filter show dev %s ingress" % self._name)ingress_prefs = re.findall("pref (\\d+) .* handle", out)for egress_pref in egress_prefs:exec_cmd("tc filter del dev %s pref %s" % (self._name,egress_pref))for ingress_pref in ingress_prefs:exec_cmd("tc filter del dev %s pref %s ingress" % (self._name,ingress_pref))- def clear_configuration(self): if self._master["primary"]: primary_id = self._master["primary"]
@@ -532,6 +558,8 @@ class Device(object): m_dev.clear_configuration()
if self._conf != None:
self._clear_tc_qdisc()self._clear_tc_filters() self.down() self.deconfigure() self.destroy()-- 2.4.11 _______________________________________________ LNST-developers mailing list lnst-developers@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahoste...
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Thursday, October 20, 2016 5:24 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com; Ido Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; jpirko@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Mon, Aug 15, 2016 at 10:05:53AM CEST, yotamg@mellanox.com wrote:
Add the tc filter clear and tc qdisc clear commands to the Interface clear_configuration of the InterfaceManager.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
lnst/Slave/InterfaceManager.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index dbb0771..a4d6117 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -519,6 +519,32 @@ class Device(object): self._conf = None self._conf_dict = None
- def _clear_tc_qdisc(self):
exec_cmd("tc qdisc replace dev %s root pfifo" % self._name)Hi, just noticed that this can change the original qdisc setting on the device.
The pfifo qdisc does not have to be default, e.g. on RHEL7 I see it's mq that enables multiqueue. This should be fixed so that original setting is restored.
OK. I assumed that the default is always pfifo but seems like I was wrong. I think that the best way to fix it is to store, at the beginning of each test the interface qdisc type, and then restore it.
I hope I will get to fixing it soon.
out, _ = exec_cmd("tc filter show dev %s" % self._name)ingress_handles = re.findall("ingress (\\d+):", out)for ingress_handle in ingress_handles:exec_cmd("tc qdisc del dev %s handle %s: ingress" %(self._name, ingress_handle))out, _ = exec_cmd("tc qdisc show dev %s" % self._name)ingress_qdiscs = re.findall("qdisc ingress (\\w+):", out)if len(ingress_qdiscs) != 0:exec_cmd("tc qdisc del dev %s ingress" % self._name)- def _clear_tc_filters(self):
out, _ = exec_cmd("tc filter show dev %s" % self._name)egress_prefs = re.findall("pref (\\d+) .* handle", out)out, _ = exec_cmd("tc filter show dev %s ingress" % self._name)ingress_prefs = re.findall("pref (\\d+) .* handle", out)for egress_pref in egress_prefs:exec_cmd("tc filter del dev %s pref %s" % (self._name,egress_pref))for ingress_pref in ingress_prefs:exec_cmd("tc filter del dev %s pref %s ingress" % (self._name,ingress_pref))- def clear_configuration(self): if self._master["primary"]: primary_id = self._master["primary"]
@@ -532,6 +558,8 @@ class Device(object): m_dev.clear_configuration()
if self._conf != None:
self._clear_tc_qdisc()self._clear_tc_filters() self.down() self.deconfigure() self.destroy()-- 2.4.11 _______________________________________________ LNST-developers mailing list lnst-developers@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/lnst-
developers@lists.fedorahosted.org
Sun, Oct 23, 2016 at 08:45:04AM CEST, yotamg@mellanox.com wrote:
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Thursday, October 20, 2016 5:24 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com; Ido Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; jpirko@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Mon, Aug 15, 2016 at 10:05:53AM CEST, yotamg@mellanox.com wrote:
Add the tc filter clear and tc qdisc clear commands to the Interface clear_configuration of the InterfaceManager.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
lnst/Slave/InterfaceManager.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index dbb0771..a4d6117 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -519,6 +519,32 @@ class Device(object): self._conf = None self._conf_dict = None
- def _clear_tc_qdisc(self):
exec_cmd("tc qdisc replace dev %s root pfifo" % self._name)Hi, just noticed that this can change the original qdisc setting on the device.
The pfifo qdisc does not have to be default, e.g. on RHEL7 I see it's mq that enables multiqueue. This should be fixed so that original setting is restored.
OK. I assumed that the default is always pfifo but seems like I was wrong. I think that the best way to fix it is to store, at the beginning of each test the interface qdisc type, and then restore it.
I hope I will get to fixing it soon.
Hi, I did some preliminary testing and it seems that the fix should be as easy as following:
[root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 [root@rhel73-1 ~]# tc qdisc replace dev eth2 root pfifo [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo 8006: root refcnt 2 limit 1000p [root@rhel73-1 ~]# tc qdisc del dev eth2 root # <------------ [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Deleting root qdisc should restore the original (default) value. So it's not necessary to do 'tc replace ...'.
Checked on RHEL7, RHEL6, both virtio and bare NICs.
Could you please check if this works fine also for your switchdev testing. Just simple check. I will send a patch for this.
-Jan
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Monday, October 31, 2016 5:13 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com; Ido Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; Jiri Pirko jiri@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Sun, Oct 23, 2016 at 08:45:04AM CEST, yotamg@mellanox.com wrote:
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Thursday, October 20, 2016 5:24 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com;
Ido
Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; jpirko@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Mon, Aug 15, 2016 at 10:05:53AM CEST, yotamg@mellanox.com wrote:
Add the tc filter clear and tc qdisc clear commands to the Interface clear_configuration of the InterfaceManager.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
lnst/Slave/InterfaceManager.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index dbb0771..a4d6117 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -519,6 +519,32 @@ class Device(object): self._conf = None self._conf_dict = None
- def _clear_tc_qdisc(self):
exec_cmd("tc qdisc replace dev %s root pfifo" % self._name)Hi, just noticed that this can change the original qdisc setting on the device.
The pfifo qdisc does not have to be default, e.g. on RHEL7 I see it's mq that enables multiqueue. This should be fixed so that original setting is restored.
OK. I assumed that the default is always pfifo but seems like I was wrong. I think that the best way to fix it is to store, at the beginning of each test the interface qdisc type, and then restore it.
I hope I will get to fixing it soon.
Hi, I did some preliminary testing and it seems that the fix should be as easy as following:
[root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 [root@rhel73-1 ~]# tc qdisc replace dev eth2 root pfifo [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo 8006: root refcnt 2 limit 1000p [root@rhel73-1 ~]# tc qdisc del dev eth2 root # <------------ [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Deleting root qdisc should restore the original (default) value. So it's not necessary to do 'tc replace ...'.
Checked on RHEL7, RHEL6, both virtio and bare NICs.
Could you please check if this works fine also for your switchdev testing. Just simple check. I will send a patch for this.
No problem. Send me the patch and I will make sure it works.
-Jan
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Monday, October 31, 2016 5:13 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com; Ido Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; Jiri Pirko jiri@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Sun, Oct 23, 2016 at 08:45:04AM CEST, yotamg@mellanox.com wrote:
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Thursday, October 20, 2016 5:24 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com;
Ido
Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; jpirko@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Mon, Aug 15, 2016 at 10:05:53AM CEST, yotamg@mellanox.com wrote:
Add the tc filter clear and tc qdisc clear commands to the Interface clear_configuration of the InterfaceManager.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
lnst/Slave/InterfaceManager.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index dbb0771..a4d6117 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -519,6 +519,32 @@ class Device(object): self._conf = None self._conf_dict = None
- def _clear_tc_qdisc(self):
exec_cmd("tc qdisc replace dev %s root pfifo" % self._name)Hi, just noticed that this can change the original qdisc setting on the device.
The pfifo qdisc does not have to be default, e.g. on RHEL7 I see it's mq that enables multiqueue. This should be fixed so that original setting is restored.
OK. I assumed that the default is always pfifo but seems like I was wrong. I think that the best way to fix it is to store, at the beginning of each test the interface qdisc type, and then restore it.
I hope I will get to fixing it soon.
Hi, I did some preliminary testing and it seems that the fix should be as easy as following:
[root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 [root@rhel73-1 ~]# tc qdisc replace dev eth2 root pfifo [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo 8006: root refcnt 2 limit 1000p [root@rhel73-1 ~]# tc qdisc del dev eth2 root # <------------ [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Deleting root qdisc should restore the original (default) value. So it's not necessary to do 'tc replace ...'.
Checked on RHEL7, RHEL6, both virtio and bare NICs.
Could you please check if this works fine also for your switchdev testing. Just simple check. I will send a patch for this.
Re-reading you email got me thinking that I got you wrong.
I tested and the command indeed remove all the egress filters that I create. So it seems functional for our tests.
Thanks and sorry for not getting to fix it soon enough.
-Jan
Mon, Oct 31, 2016 at 05:04:07PM CET, yotamg@mellanox.com wrote:
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Monday, October 31, 2016 5:13 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com; Ido Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; Jiri Pirko jiri@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Sun, Oct 23, 2016 at 08:45:04AM CEST, yotamg@mellanox.com wrote:
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Thursday, October 20, 2016 5:24 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com;
Ido
Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; jpirko@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Mon, Aug 15, 2016 at 10:05:53AM CEST, yotamg@mellanox.com wrote:
Add the tc filter clear and tc qdisc clear commands to the Interface clear_configuration of the InterfaceManager.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
lnst/Slave/InterfaceManager.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index dbb0771..a4d6117 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -519,6 +519,32 @@ class Device(object): self._conf = None self._conf_dict = None
- def _clear_tc_qdisc(self):
exec_cmd("tc qdisc replace dev %s root pfifo" % self._name)Hi, just noticed that this can change the original qdisc setting on the device.
The pfifo qdisc does not have to be default, e.g. on RHEL7 I see it's mq that enables multiqueue. This should be fixed so that original setting is restored.
OK. I assumed that the default is always pfifo but seems like I was wrong. I think that the best way to fix it is to store, at the beginning of each test the interface qdisc type, and then restore it.
I hope I will get to fixing it soon.
Hi, I did some preliminary testing and it seems that the fix should be as easy as following:
[root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 [root@rhel73-1 ~]# tc qdisc replace dev eth2 root pfifo [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo 8006: root refcnt 2 limit 1000p [root@rhel73-1 ~]# tc qdisc del dev eth2 root # <------------ [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Deleting root qdisc should restore the original (default) value. So it's not necessary to do 'tc replace ...'.
Checked on RHEL7, RHEL6, both virtio and bare NICs.
Could you please check if this works fine also for your switchdev testing. Just simple check. I will send a patch for this.
Re-reading you email got me thinking that I got you wrong.
I tested and the command indeed remove all the egress filters that I create. So it seems functional for our tests.
Thanks and sorry for not getting to fix it soon enough.
Just to confirm, does it also restore the original qdisc, when you run tc qdisc show dev XYZ
-Jan
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Monday, October 31, 2016 6:24 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com; Ido Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; Jiri Pirko jiri@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Mon, Oct 31, 2016 at 05:04:07PM CET, yotamg@mellanox.com wrote:
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Monday, October 31, 2016 5:13 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com;
Ido
Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com;
Jiri
Pirko jiri@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Sun, Oct 23, 2016 at 08:45:04AM CEST, yotamg@mellanox.com wrote:
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Thursday, October 20, 2016 5:24 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com;
Ido
Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; jpirko@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Mon, Aug 15, 2016 at 10:05:53AM CEST, yotamg@mellanox.com wrote:
Add the tc filter clear and tc qdisc clear commands to the Interface clear_configuration of the InterfaceManager.
Signed-off-by: Yotam Gigi yotamg@mellanox.com
lnst/Slave/InterfaceManager.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+)
diff --git a/lnst/Slave/InterfaceManager.py
b/lnst/Slave/InterfaceManager.py
index dbb0771..a4d6117 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -519,6 +519,32 @@ class Device(object): self._conf = None self._conf_dict = None
- def _clear_tc_qdisc(self):
exec_cmd("tc qdisc replace dev %s root pfifo" % self._name)Hi, just noticed that this can change the original qdisc setting on the device.
The pfifo qdisc does not have to be default, e.g. on RHEL7 I see it's mq that enables multiqueue. This should be fixed so that original setting is restored.
OK. I assumed that the default is always pfifo but seems like I was wrong. I think that the best way to fix it is to store, at the beginning of each test the interface qdisc type, and then restore it.
I hope I will get to fixing it soon.
Hi, I did some preliminary testing and it seems that the fix should be as easy as following:
[root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1 [root@rhel73-1 ~]# tc qdisc replace dev eth2 root pfifo [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo 8006: root refcnt 2 limit 1000p [root@rhel73-1 ~]# tc qdisc del dev eth2 root # <------------ [root@rhel73-1 ~]# tc qdisc show dev eth2 qdisc pfifo_fast 0: root refcnt 2 bands 3 priomap 1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
Deleting root qdisc should restore the original (default) value. So it's not necessary to do 'tc replace ...'.
Checked on RHEL7, RHEL6, both virtio and bare NICs.
Could you please check if this works fine also for your switchdev testing. Just simple check. I will send a patch for this.
Re-reading you email got me thinking that I got you wrong.
I tested and the command indeed remove all the egress filters that I create. So it seems functional for our tests.
Thanks and sorry for not getting to fix it soon enough.
Just to confirm, does it also restore the original qdisc, when you run tc qdisc show dev XYZ
Yep, That's what I see when I test it.
This command does effect the ingress qdiscs though. The ingress qidscs clear code should be left untouched, no?
-Jan
Mon, Oct 31, 2016 at 05:32:49PM CET, yotamg@mellanox.com wrote:
-----Original Message----- From: Jan Tluka [mailto:jtluka@redhat.com] Sent: Monday, October 31, 2016 6:24 PM To: Yotam Gigi yotamg@mellanox.com Cc: lnst-developers@lists.fedorahosted.org; Elad Raz eladr@mellanox.com; Ido Schimmel idosch@mellanox.com; Nogah Frankel nogahf@mellanox.com; Jiri Pirko jiri@mellanox.com Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Just to confirm, does it also restore the original qdisc, when you run tc qdisc show dev XYZ
Yep, That's what I see when I test it.
This command does effect the ingress qdiscs though. The ingress qidscs clear code should be left untouched, no?
I thought so. Could you give more details here? E.g. output of commands.
Thanks, Jan
This recipe tests the port mirroring SPAN feature using two machines connected via a bridge in the switch, and pinging each other. The recipe creates and removes port mirroring on one of the ports and tests whether the packet gets mirrored or not.
Signed-off-by: Yotam Gigi yotamg@mellanox.com --- recipes/switchdev/l2-021-span.py | 119 ++++++++++++++++++++++++++++++++++++++ recipes/switchdev/l2-021-span.xml | 26 +++++++++ 2 files changed, 145 insertions(+) create mode 100644 recipes/switchdev/l2-021-span.py create mode 100644 recipes/switchdev/l2-021-span.xml
diff --git a/recipes/switchdev/l2-021-span.py b/recipes/switchdev/l2-021-span.py new file mode 100644 index 0000000..ca7d15e --- /dev/null +++ b/recipes/switchdev/l2-021-span.py @@ -0,0 +1,119 @@ +""" +Copyright 2016 Mellanox Technologies. All rights reserved. +Licensed under the GNU General Public License, version 2 as +published by the Free Software Foundation; see COPYING for details. +""" + +__author__ = """ +yotamg@mellanox.com (Yotam Gigi) +""" + +from lnst.Controller.Task import ctl +from TestLib import TestLib +from time import sleep +import re +import random + +class MirredPort: + def __init__(self, mirred_port): + mach = mirred_port.get_host() + devname = mirred_port.get_devname() + self.mirred_port = mirred_port + self.mach = mach + + mach.run("tc qdisc replace dev %s handle 0: root prio" % devname) + mach.run("tc qdisc add dev %s handle ffff: ingress" % devname) + + def create_mirror(self, to_port, ingress = False): + ingress_str = "ingress" if ingress else "" + qdisc_handle = "ffff" if ingress else "0" + from_dev = self.mirred_port.get_devname() + to_dev = to_port.get_devname() + + self.mach.run("tc filter add dev %s parent %s: matchall skip_sw action \ + mirred egress mirror dev %s" % (from_dev, qdisc_handle, + to_dev)) + + def get_mirrors(self, to_port, ingress = False): + ingress_str = "ingress" if ingress else "" + from_dev = self.mirred_port.get_devname() + cmd = self.mach.run("tc filter show dev %s %s" % (from_dev, + ingress_str), save_output=True) + output = cmd.get_result()["res_data"]["stdout"] + return re.findall("pref (\d+) .* handle .*\n.* device %s" % + to_port.get_devname(), output, re.M) + + def remove_mirror(self, to_port, ingress = False): + prefs = self.get_mirrors(to_port, ingress) + from_dev = self.mirred_port.get_devname() + ingress_str = "ingress" if ingress else "" + for pref in prefs: + self.mach.run("tc filter del dev %s pref %s %s" % (from_dev, pref, + ingress_str)) + +def run_packet_assert(num, main_if, from_if=None, to_if=None): + mach = main_if.get_host() + filter_str = "icmp" + filter_str += " && src %s" % from_if.get_ip() if from_if else "" + filter_str += " && dst %s" % to_if.get_ip() if to_if else "" + + packet_assert_mod = ctl.get_module("PacketAssert", options = { + "min" : num, "max" : num, + "promiscuous" : True, + "filter" : filter_str, + "interface" : main_if.get_devname()}) + return mach.run(packet_assert_mod, bg=True) + +def change_mirror_status(mirror_status, change, mirred_port, to_if): + mirror_status[change] = not mirror_status[change] + change_ingress = (change == "ingress") + + to_if.get_host().run("echo " + str(mirror_status)) + + if mirror_status[change]: + mirred_port.create_mirror(to_if, change_ingress) + else: + mirred_port.remove_mirror(to_if, change_ingress) + +def do_task(ctl, hosts, ifaces, aliases): + tl = TestLib(ctl, aliases) + m1, m2, sw = hosts + m1_if1, m2_if1, m2_if2, sw_if1, sw_if2, sw_if3 = ifaces + m1.sync_resources(modules=["PacketAssert"]) + + # configure interfaces + m1_if1.reset(ip=["192.168.101.10/24", "2002::1/64"]) + m2_if1.reset(ip=["192.168.101.11/24", "2002::2/64"]) + sw_if3.set_link_up() + m2_if2.set_link_up() + sw.create_bridge(slaves=[sw_if1, sw_if2], options={"vlan_filtering": 1}) + mirred_port = MirredPort(sw_if2) + + sleep(15) + + mirror_status = {"ingress": False, "egress": False } + for i in range(10): + change = random.choice(mirror_status.keys()) + change_mirror_status(mirror_status, change, mirred_port, sw_if3) + + in_num = 10 if mirror_status["ingress"] else 0 + out_num = 10 if mirror_status["egress"] else 0 + + in_assert_proc = run_packet_assert(in_num, m2_if2, m2_if1, m1_if1) + out_assert_proc = run_packet_assert(out_num, m2_if2, m1_if1, m2_if1) + tl.ping_simple(m1_if1, m2_if1, count=10) + in_assert_proc.intr() + out_assert_proc.intr() + + return 0 + +do_task(ctl, [ctl.get_host("machine1"), + ctl.get_host("machine2"), + ctl.get_host("switch")], + [ctl.get_host("machine1").get_interface("if1"), + ctl.get_host("machine2").get_interface("if1"), + ctl.get_host("machine2").get_interface("if2"), + ctl.get_host("switch").get_interface("if1"), + ctl.get_host("switch").get_interface("if2"), + ctl.get_host("switch").get_interface("if3")], + ctl.get_aliases()) diff --git a/recipes/switchdev/l2-021-span.xml b/recipes/switchdev/l2-021-span.xml new file mode 100644 index 0000000..097351b --- /dev/null +++ b/recipes/switchdev/l2-021-span.xml @@ -0,0 +1,26 @@ +<lnstrecipe xmlns:xi="http://www.w3.org/2003/XInclude"> + <xi:include href="default_aliases.xml" /> + <network> + <host id="machine1"> + <params/> + <interfaces> + <eth id="if1" label="A" /> + </interfaces> + </host> + <host id="machine2"> + <params/> + <interfaces> + <eth id="if1" label="B" /> + <eth id="if2" label="C" /> + </interfaces> + </host> + <host id="switch"> + <interfaces> + <eth id="if1" label="A" /> + <eth id="if2" label="B" /> + <eth id="if3" label="C" /> + </interfaces> + </host> + </network> + <task python="l2-021-span.py" /> +</lnstrecipe>
Mon, Aug 15, 2016 at 10:05:49AM CEST, yotamg@mellanox.com wrote:
Add the SPAN port mirroring recipe for switchdev, and several lnst features that were missing and were needed for that recipe.
Among the features added:
- The lnst slave now cleans the tc qdiscs and filters at the clean procedure.
- The PacketAssert module supports setting promiscuos flag.
- The Switchdev TestLib IcmpPing module supports setting ping parameters.
The SPAN recipe consist of two machines connected via a bridge in the switch, and pinging each other, while ingress/egress port mirrors are added and tested.
Applied, excluding patch 1
lnst-developers@lists.fedorahosted.org
-
Jan Tluka -
Jiri Pirko -
Yotam Gigi