-----Original Message-----
From: Jan Tluka [mailto:jtluka@redhat.com]
Sent: Monday, October 31, 2016 6:24 PM
To: Yotam Gigi <yotamg(a)mellanox.com>
Cc: lnst-developers(a)lists.fedorahosted.org; Elad Raz <eladr(a)mellanox.com>; Ido
Schimmel <idosch(a)mellanox.com>; Nogah Frankel <nogahf(a)mellanox.com>; Jiri
Pirko <jiri(a)mellanox.com>
Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
Mon, Oct 31, 2016 at 05:04:07PM CET, yotamg(a)mellanox.com wrote:
>>-----Original Message-----
>>From: Jan Tluka [mailto:jtluka@redhat.com]
>>Sent: Monday, October 31, 2016 5:13 PM
>>To: Yotam Gigi <yotamg(a)mellanox.com>
>>Cc: lnst-developers(a)lists.fedorahosted.org; Elad Raz <eladr(a)mellanox.com>;
Ido
>>Schimmel <idosch(a)mellanox.com>; Nogah Frankel <nogahf(a)mellanox.com>;
Jiri
>>Pirko <jiri(a)mellanox.com>
>>Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
>>
>>Sun, Oct 23, 2016 at 08:45:04AM CEST, yotamg(a)mellanox.com wrote:
>>>
>>>
>>>>-----Original Message-----
>>>>From: Jan Tluka [mailto:jtluka@redhat.com]
>>>>Sent: Thursday, October 20, 2016 5:24 PM
>>>>To: Yotam Gigi <yotamg(a)mellanox.com>
>>>>Cc: lnst-developers(a)lists.fedorahosted.org; Elad Raz
<eladr(a)mellanox.com>;
>>Ido
>>>>Schimmel <idosch(a)mellanox.com>; Nogah Frankel
<nogahf(a)mellanox.com>;
>>>>jpirko(a)mellanox.com
>>>>Subject: Re: [PATCH 4/5] Slave: Add basic support for tc.
>>>>
>>>>Mon, Aug 15, 2016 at 10:05:53AM CEST, yotamg(a)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(a)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