On Fri, Mar 10, 2023 at 9:58 AM Simon Horman <[email protected]>
wrote:
> On Thu, Mar 09, 2023 at 03:00:40PM +0100, Ales Musil wrote:
> > Specifying datapath with "dpctl/flush-conntrack" didn't
> > work as expected and caused error:
> > ovs-dpctl: field system@ovs-system missing value (Invalid argument)
> >
> > To prevent that check if we have datapath as first argument
> > and use it accordingly.
> >
> > Fixes: a9ae73b916ba ("ofp, dpif: Allow CT flush based on partial match.")
> > Signed-off-by: Ales Musil <[email protected]>
> > ---
> > v2: Add test case.
> > v3: Add test case also for flush-conntrack without arguments.
> > ---
> > lib/dpctl.c | 17 ++++++++---------
> > tests/system-traffic.at | 16 ++++++++++++++++
> > 2 files changed, 24 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/dpctl.c b/lib/dpctl.c
> > index c501a0cd7..a7a4d8ee8 100644
> > --- a/lib/dpctl.c
> > +++ b/lib/dpctl.c
> > @@ -1717,10 +1717,16 @@ dpctl_flush_conntrack(int argc, const char
> *argv[],
> > uint16_t zone, *pzone = NULL;
> > int error;
> > int args = argc - 1;
> > + int zone_pos = 1;
> > +
> > + if (dp_arg_exists(argc, argv)) {
> > + args--;
> > + zone_pos = 2;
> > + }
> >
> > /* Parse zone. */
> > - if (args && !strncmp(argv[1], "zone=", 5)) {
> > - if (!ovs_scan(argv[1], "zone=%"SCNu16, &zone)) {
> > + if (args && !strncmp(argv[zone_pos], "zone=", 5)) {
> > + if (!ovs_scan(argv[zone_pos], "zone=%"SCNu16, &zone)) {
> > ds_put_cstr(&ds, "failed to parse zone");
> > error = EINVAL;
> > goto error;
> > @@ -1747,13 +1753,6 @@ dpctl_flush_conntrack(int argc, const char
> *argv[],
> > args--;
> > }
> >
> > - /* Report error if there is more than one unparsed argument. */
> > - if (args > 1) {
> > - ds_put_cstr(&ds, "invalid arguments");
> > - error = EINVAL;
> > - goto error;
> > - }
> > -
>
> Hi Ales,
>
> I'm a little unclear on the motivation for the above hunk -
> what if any rule regarding unparsed arguments should be enforced here?
>
Hi Simon,
it actually shouldn't happen that there is something left unprased,
because unknown arguments will be reported by "ofp_ct_tuple_parse".
Does that make sense?
Thanks,
Ales
>
> > error = opt_dpif_open(argc, argv, dpctl_p, 5, &dpif);
> > if (error) {
> > dpctl_error(dpctl_p, error, "Cannot open dpif");
>
> ...
>
>
--
Ales Musil
Senior Software Engineer - OVN Core
Red Hat EMEA <https://www.redhat.com>
[email protected] IM: amusil
<https://red.ht/sig>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
