Thu, Sep 15, 2016 at 12:57:31PM CEST, idosch(a)idosch.org wrote:
On Thu, Sep 15, 2016 at 12:28:00PM +0200, Jan Tluka wrote:
> Thu, Sep 15, 2016 at 12:08:41PM CEST, idosch(a)idosch.org wrote:
> >On Thu, Sep 15, 2016 at 10:51:18AM +0200, Jan Tluka wrote:
> >> Thu, Sep 15, 2016 at 09:42:21AM CEST, idosch(a)idosch.org wrote:
> >> >From: Ido Schimmel <idosch(a)mellanox.com>
> >> >
> >> >Apparently we need to set 'vlan_id' before 'vlan_p', so
simply sort()
> >> >the options.
> >> >
> >> >Signed-off-by: Ido Schimmel <idosch(a)mellanox.com>
> >> >---
> >> > test_modules/PktgenTx.py | 1 +
> >> > 1 file changed, 1 insertion(+)
> >> >
> >> >diff --git a/test_modules/PktgenTx.py b/test_modules/PktgenTx.py
> >> >index 51fcd4c..49f50fb 100644
> >> >--- a/test_modules/PktgenTx.py
> >> >+++ b/test_modules/PktgenTx.py
> >> >@@ -78,6 +78,7 @@ class PktgenTx(TestGeneric):
> >> > ]
> >> > pktgen_options = pktget_options_merge(pktgen_options,
> >> > default_pktgen_options)
> >> >+ pktgen_options.sort() # vlan_id must come before vlan_p
> >>
> >> This is a bit weird solution IMO.
> >>
> >> Why does it have to be set this way?
> >
> >Well, when I didn't sort the options and used both 'vlan_id' and
'vlan_p'
> >I noticed that the packets aren't generated correctly. I checked the
> >code and I saw this:
> >http://lxr.free-electrons.com/source/net/core/pktgen.c#L1658
> >
> >So, if 'vlan_id' is set to 0xffff, then 'vlan_p' isn't set.
If you look
> >at how a 'pkt_dev' is initialized, you'll see that its
'vlan_id' value
> >is set to 0xffff:
> >http://lxr.free-electrons.com/source/net/core/pktgen.c#L3667
> >
> >So, AFAICS, the only way is to first set 'vlan_id' and then
'vlan_p'.
> >
>
> And when you specify vlan_id and vlan_p in "correct" order in a recipe
> does it work then?
Yes. If I sort() in TestLib (which is the wrapper we invoke from
recipes/switchev/), then the parameters are received by PktgenTx in the
correct order.
Do you prefer this to be done in TestLib instead? I don't really mind,
but I just thought this is something others can benefit from as well.
Yes that would be better. As I said, if you keep sort() there and some
other option combinations do no match that approach you will have NO way
to fix it.
After realizing this I think that this patch should not be applied
at all. It's too risky IMO.
-Jan
>>
>> >If the problem is the use of sort(), I can try something else.
>> >
>>
>> Thanks for explanation. I'm a bit concerned if this affects also other
>> "unknown" option combinations. The thing is that for you case it can
>> work but for others it may break. But I don't have anything in hand to
>> prove that.
>>
>> >Thanks for reviewing!
>> >
>> >>
>> >> -Jan
>> >>
>> >> >
>> >> > exec_cmd("modprobe pktgen")
>> >> >
>> >> >--
>> >> >2.7.4
>> >> >_______________________________________________
>> >> >LNST-developers mailing list
>> >> >lnst-developers(a)lists.fedorahosted.org
>> >>
>https://lists.fedorahosted.org/admin/lists/lnst-developers@lists.fedorahosted.org