On Fri, Mar 10, 2023 at 10:36:38AM +0100, Ales Musil wrote:
> 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?

Yes, thanks.

Perhaps this could be a separate patch?
Or this information added to the patch description?
Or perhaps it is obvious to everyone except me :)
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to