[ovs-dev] system-dpdk: Add testpmd clean up in MTU unit tests.

The MTU vport unit tests do not clean up testpmd after use which causes them to fail randomly.
This commit amends the MTU vport unit tests to clean up testpmd after running.

Fixes: bf47829116a8feb54fe795aa19915f6e6283af93 ("tests: Add OVS-DPDK MTU unit tests.")
Signed-off-by: Michael Phelan <michael.phelan@intel.com>
---
 tests/system-dpdk.at | 11 +++++++++++
 1 file changed, 11 insertions(+)

Hi Michael,


 The Regression looks good, and the patch looks good too.
Regression CI runs are added as Screenshots.

Acked-by: Kumar Amber <kumar.amber@intel.com>



> -----Original Message-----
> From: Phelan, Michael <michael.phelan@intel.com>
> > Fixes: bf47829116a8feb54fe795aa19915f6e6283af93 ("tests: Add OVS-
> DPDK
> > MTU unit tests.")
> > Signed-off-by: Michael Phelan <michael.phelan@intel.com>
> > ---
> >  tests/system-dpdk.at | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at index
> > 3beccda44..15f97097a 100644
> > --- a/tests/system-dpdk.at
> > +++ b/tests/system-dpdk.at
> > @@ -669,6 +669,9 @@ AT_CHECK([ovs-vsctl set Interface
> > dpdkvhostuserclient0 mtu_request=9000])  AT_CHECK([ovs-appctl
> > dpctl/show], [], [stdout])  AT_CHECK([egrep 'mtu=9000' stdout], [],
> > [stdout])
> >
> > +dnl Clean up the testpmd now
> > +pkill -f -x -9 'tail -f /dev/null'
> > +
> >  dnl Clean up
> >  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [],
> > [stdout],
> > [stderr])  OVS_VSWITCHD_STOP("m4_join([],
> [SYSTEM_DPDK_ALLOWED_LOGS],
> > [ @@ -723,6 +726,9 @@ AT_CHECK([ovs- vsctl set Interface
> > dpdkvhostuserclient0 mtu_request=2000]) AT_CHECK([ovs-appctl
> > dpctl/show], [], [stdout])  AT_CHECK([egrep 'mtu=2000' stdout], [],
> > [stdout])
> >
> > +dnl Clean up the testpmd now
> > +pkill -f -x -9 'tail -f /dev/null'
> > +
> >  dnl Clean up
> >  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [],
> > [stdout],
> > [stderr])  OVS_VSWITCHD_STOP("m4_join([],
> [SYSTEM_DPDK_ALLOWED_LOGS],
> > [ @@ -852,6 +858,9 @@ dnl Set MTU value above upper bound and check
> > for error  AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0
> > mtu_request=9711])  AT_CHECK([grep
> > "dpdkvhostuserclient0: unsupported MTU 9711" ovs-vswitchd.log], [],
> > [stdout])
> >
> > +dnl Clean up the testpmd now
> > +pkill -f -x -9 'tail -f /dev/null'
> > +
> >  dnl Clean up
> >  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [],
> > [stdout],
> > [stderr])  OVS_VSWITCHD_STOP("m4_join([],
> [SYSTEM_DPDK_ALLOWED_LOGS],
> > [ @@ -906,6 +915,8 @@ dnl Set MTU value below lower bound and check
> > for error  AT_CHECK([ovs-vsctl set Interface dpdkvhostuserclient0
> > mtu_request=67])  AT_CHECK([grep
> > "dpdkvhostuserclient0: unsupported MTU 67" ovs-vswitchd.log], [],
> > [stdout])
> >
> > +dnl Clean up the testpmd now
> > +pkill -f -x -9 'tail -f /dev/null'
> >
> >  dnl Clean up
> >  AT_CHECK([ovs-vsctl del-port br10 dpdkvhostuserclient0], [],
> > [stdout],
> > [stderr])
> > --
> > 2.25.1

Hi Mike, Amber,

Thanks for working on this. Couple of comments below,  rest looks good.


> -----Original Message-----
> From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of Amber, Kumar
> Sent: Friday, July 22, 2022 2:36 PM
> To: dev@openvswitch.org
> Cc: Ilya Maximets <i.maximets@redhat.com>
> Subject: Re: [ovs-dev] [PATCH] system-dpdk: Add testpmd clean up in MTU
> unit tests.
> 
> Hi Michael,
> 
> 
>  The Regression looks good, and the patch looks good too.
> Regression CI runs are added as Screenshots.

Amber, the screenshots are stripped away. (Mail-list strips attachments)

> 
> Acked-by: Kumar Amber <kumar.amber@intel.com>
> 
> 
> 
> > -----Original Message-----
> > From: Phelan, Michael <michael.phelan@intel.com>
> > > Fixes: bf47829116a8feb54fe795aa19915f6e6283af93 ("tests: Add OVS-
> > DPDK
> > > MTU unit tests.")

To be consistent, only the first 12 chars of the commit id are sufficient, so this becomes:
Fixes: bf47829116a8 ("tests: Add OVS-DPDK MTU unit tests.")

But I guess this can be changed when applying, so might not require another revision.
With that, 
Acked-by: Sunil Pai G <sunil.pai.g@intel.com>

You might want to add the following alias to your git config to generate the fixes string easily:
git config --global alias.fixline " log -1 --abbrev=12 --format='Fixes: %h (\"%s\")%nCc: %ae'"
and use it as: git fixline <commit_id>

> > > Signed-off-by: Michael Phelan <michael.phelan@intel.com>

Since this was reported by Ilya:
Reported-by: Ilya Maximets <i.maximets@ovn.org>

> > > ---
> > >  tests/system-dpdk.at | 11 +++++++++++
> > >  1 file changed, 11 insertions(+)
> > >

<snipped>

Thanks and regards
Sunil

On 7/20/22 18:26, Michael Phelan wrote:
> The MTU vport unit tests do not clean up testpmd after use which causes them to fail randomly.
> This commit amends the MTU vport unit tests to clean up testpmd after running.
> 
> Fixes: bf47829116a8feb54fe795aa19915f6e6283af93 ("tests: Add OVS-DPDK MTU unit tests.")
> Signed-off-by: Michael Phelan <michael.phelan@intel.com>
> ---

Thanks!  With the commit message adjustments, applied to
master and 3.0.

Robot seems to be on PTO today. :)  Hopefully, we'll start
getting green reports when it's back.

Best regards, Ilya Maximets.

Ilya Maximets <i.maximets@ovn.org> writes:

> On 7/20/22 18:26, Michael Phelan wrote:
>> The MTU vport unit tests do not clean up testpmd after use which causes them to fail randomly.
>> This commit amends the MTU vport unit tests to clean up testpmd after running.
>> 
>> Fixes: bf47829116a8feb54fe795aa19915f6e6283af93 ("tests: Add OVS-DPDK MTU unit tests.")
>> Signed-off-by: Michael Phelan <michael.phelan@intel.com>
>> ---
>
> Thanks!  With the commit message adjustments, applied to
> master and 3.0.
>
> Robot seems to be on PTO today. :)  Hopefully, we'll start
> getting green reports when it's back.

I guess it took a long vacation.  Hopefully it is all good now.

> Best regards, Ilya Maximets.