[ovs-dev] [v9 01/12] dpif-netdev: Add command line and function pointer for miniflow extract

Van Haaren, Harry harry.van.haaren at intel.com
Mon Jul 12 14:02:29 UTC 2021


> -----Original Message-----
> From: Ilya Maximets <i.maximets at ovn.org>
> Sent: Monday, July 12, 2021 2:25 PM
> To: Amber, Kumar <kumar.amber at intel.com>; ovs-dev at openvswitch.org
> Cc: fbl at sysclose.org; echaudro at redhat.com; 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: [v9 01/12] dpif-netdev: Add command line and function pointer for
> miniflow extract
> 
> On 7/12/21 7:51 AM, kumar Amber wrote:
> > From: Kumar Amber <kumar.amber at intel.com>
> >
> > This patch introduces the MFEX function pointers which allows
> > the user to switch between different miniflow extract implementations
> > which are provided by the OVS based on optimized ISA CPU.
> >
> > The user can query for the available minflow extract variants available
> > for that CPU by following commands:
> >
> > $ovs-appctl dpif-netdev/miniflow-parser-get
> >
> > Similarly an user can set the miniflow implementation by the following
> > command :
> >
> > $ ovs-appctl dpif-netdev/miniflow-parser-set name
> >
> > This allows for more performance and flexibility to the user to choose
> > the miniflow implementation according to the needs.
> >
> > Signed-off-by: Kumar Amber <kumar.amber at intel.com>
> > Co-authored-by: Harry van Haaren <harry.van.haaren at intel.com>
> > Signed-off-by: Harry van Haaren <harry.van.haaren at intel.com>
> >
> > ---
> > v9:
> > - fix review comments from Flavio
> > v7:
> > - fix review comments(Eelco, Flavio)
> > v5:
> > - fix review comments(Ian, Flavio, Eelco)
> > - add enum to hold mfex indexes
> > - add new get and set implemenatations
> > - add Atomic set and get
> > ---
> 
> ovsrobot has issues with reporting the status right now, but this
> patch fails the build in GHA:
>   https://github.com/ovsrobot/ovs/actions/runs/1021787643

Thanks for linking on results.

I've spot-checked a bunch of the failing builds, and found 2 fixable code issues.
A few of the CI run's I can't find/explain the error, but I don't know of a good
way to "jump to the error" line, am I missing a trick, or is scrolling the whole
compiler output and checking errors the best method?

ISSUES:
#1 : OVS Requires Mutex issue (Linux clang test dpdk build)
1291../../lib/dpif-netdev-private-extract.h:87:53: error: use of undeclared identifier 'dp_netdev_mutex'; did you mean 'dp_netdev_input'? 
1292 size_t pmd_list_size) OVS_REQUIRES(dp_netdev_mutex);

#2 : Unused Argument (As from mailing list review comment too, linux gcc dpdk --enable-shared)
2353lib/dpif-netdev.c:1079:63: error: unused parameter ‘argc’ [-Werror=unused-parameter] 
2354 dpif_miniflow_extract_impl_set(struct unixctl_conn *conn, int argc,

#3 : Distcheck directory not valid? (linux gcc test 3.16 build. I cannot explain this?)
make: *** [distcheck] Error 1 
4490Makefile:5298: recipe for target 'distcheck' failed 
4491+ cat '*/_build/sub/tests/testsuite.log' 
4492cat: '*/_build/sub/tests/testsuite.log': No such file or directory 
4493Error: Process completed with exit code 1.

SOLUTIONS:
#1, likely to forward-decl the "dp_netdev_mutex" to make it available
in the extract header file, and remove the "static" keyword so its no longer limited
to the dpif-netdev.c compilation unit.

#2 is a simple OVS_UNUSED as Eelco suggested during review.

#3, I'm not sure where the DistCheck issue arise from, it seems to be missing directories
during the test run? Input appreciated, as pushing & hoping tends to be a tiresome
and long process.


> Best regards, Ilya Maximets.

Regards, -Harry


More information about the dev mailing list