[ovs-dev] [v11 06/11] dpif-netdev: Add packet count and core id paramters for study

Eelco Chaudron echaudro at redhat.com
Wed Jul 14 13:08:11 UTC 2021



On 14 Jul 2021, at 14:25, Van Haaren, Harry wrote:

>> -----Original Message-----
>> From: Eelco Chaudron <echaudro at redhat.com>
>> Sent: Wednesday, July 14, 2021 12:57 PM
>> To: Amber, Kumar <kumar.amber at intel.com>
>> Cc: ovs-dev at openvswitch.org; fbl at sysclose.org; i.maximets at ovn.org; Van Haaren,
>> Harry <harry.van.haaren at intel.com>; Ferriter, Cian <cian.ferriter at intel.com>;
>> Stokes, Ian <ian.stokes at intel.com>
>> Subject: Re: [v11 06/11] dpif-netdev: Add packet count and core id paramters for
>> study
>>
>>
>>
>> On 14 Jul 2021, at 13:33, Amber, Kumar wrote:
>>
>>> Hi Eelco,
>>>
>>>> -----Original Message-----
>>>> From: Eelco Chaudron <echaudro at redhat.com>
>>>> Sent: Wednesday, July 14, 2021 4:21 PM
>>>> To: Amber, Kumar <kumar.amber at intel.com>
>>>> Cc: ovs-dev at openvswitch.org; fbl at sysclose.org; i.maximets at ovn.org; Van
>>>> Haaren, Harry <harry.van.haaren at intel.com>; Ferriter, Cian
>>>> <cian.ferriter at intel.com>; Stokes, Ian <ian.stokes at intel.com>
>>>> Subject: Re: [v11 06/11] dpif-netdev: Add packet count and core id paramters
>>>> for study
>>>>
>>>>
>>>>
>>>> On 14 Jul 2021, at 12:30, Eelco Chaudron wrote:
>>>>
>>>>> On 14 Jul 2021, at 4:02, kumar Amber wrote:
>>>>>
>>>>>> From: Kumar Amber <kumar.amber at intel.com>
>>>>>>
>>>>>> This commit introduces additional command line paramter for mfex
>>>>>> study function. If user provides additional packet out it is used in
>>>>>> study to compare minimum packets which must be processed else a
>>>>>> default value is choosen.
>>>>>> Also introduces a third paramter for choosing a particular pmd core.
>>>>>>
>>>>>> $ ovs-appctl dpif-netdev/miniflow-parser-set study 500 3
>>>>>>
>>>>>> Signed-off-by: Kumar Amber <kumar.amber at intel.com>
>>>>
>>>> One additional comment, please add some (negative) test cases for the
>>>> command line options, so we know your changes work. Rather than me having
>>>> to do this manually every revision.
>>>>
>>>
>>> Yes we did think about that but we cannot as whenever a command fails the ovs
>> flags it as error in server that auto-mantically fails the test.
>>>
>>> Ex:
>>>> 2021-07-14T11:16:30.194Z|00082|unixctl|DBG|received request dpif-
>> netdev/miniflow-parser-set["-pmd","0","scalar"], id=0
>>>> 2021-07-14T11:16:30.194Z|00083|dpif_netdev|ERR|Error: Miniflow parser not
>> changed, PMD thread 0 not in use, pass a valid pmd thread ID.
>>>> 2021-07-14T11:16:30.194Z|00084|unixctl|DBG|replying with error, id=0: "Error:
>> Miniflow parser not changed, PMD thread 0 not in use, pass a valid pmd thread ID.
>>>> "
>>> 8. system-dpdk.at:291: 8. OVS-DPDK - MFEX Commands (system-dpdk.at:291):
>> FAILED (system-dpdk.at:308)
>>>
>>> And Hence we cannot add the command test-case.
>>
>> Not sure how you added it, but here is a quick try that works for me:
>>
>> [wsfd-netdev64:~/..._v20.11.1/ovs_github]$ git diff
>> diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
>> index 96072e646..5f039dec5 100644
>> --- a/tests/system-dpdk.at
>> +++ b/tests/system-dpdk.at
>> @@ -285,3 +285,26 @@ dnl Clean up
>>  AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr])
>>  AT_CLEANUP
>>  dnl --------------------------------------------------------------------------
>> +
>> +dnl --------------------------------------------------------------------------
>> +AT_SETUP([OVS-DPDK - MFEX Configuration])
>> +AT_KEYWORDS([dpdk])
>> +OVS_DPDK_START()
>> +
>> +dnl Add userspace bridge and attach it to OVS
>> +AT_CHECK([ovs-vsctl add-br br0 -- set bridge br0 datapath_type=netdev])
>> +AT_CHECK([ovs-vsctl add-port br0 p1 -- set Interface p1 type=dpdk options:dpdk-
>> devargs=net_pcap1,rx_pcap=$srcdir/pcap/mfex_test.pcap,infinite_rx=1], [],
>> [stdout], [stderr])
>> +AT_CHECK([ovs-vsctl show], [], [stdout])
>> +
>> +
>> +AT_CHECK([ovs-appctl dpif-netdev/miniflow-parser-set scalar 1], [2], [], [dnl
>> +The study_pkt_cnt option is not valid for the scalar implementation.
>> +ovs-appctl: ovs-vswitchd: server returned an error
>> +])
>> +
>> +dnl Clean up
>> +AT_CHECK([ovs-vsctl del-port br0 p1], [], [stdout], [stderr])
>> +AT_CLEANUP
>> +dnl --------------------------------------------------------------------------
>
> Hi All,
>
> I think the above point was that if the ovs-vswitchd does a VLOG_ERR, (as it does when
> bad arguments like the above test are hit), then the unit-test automatically fails.
> Its not about adding the test code, but about how the Unit test infra handles VLOG_ERR output.

The above test ran fine in my environment. Maybe other checks generate other logs that cause problem.

> As a result, we cannot have unit-tests that actually hit errors it seems, as they cause
> the unit-test to report a failure, instead of "negative testing" and passing the unit test.


You can overwrite this log checking to exclude specific errors see check_logs() and OVS_VSWITCHD_STOP.

> I'm not very familiar with the wider AT_* based unit testing, but that's my understanding
> of the infrastructure?


>>> <Snip>
>>>
>>> Br
>>> Amber
>
> Regards, -Harry



More information about the dev mailing list