On Thu, Mar 9, 2023 at 1:11 PM Ilya Maximets <[email protected]> wrote:

> On 3/6/23 14:37, 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.
> >
> > Signed-off-by: Ales Musil <[email protected]>
>
> This needs a Fixes tag.
>
> > ---
> > v2: Add test case.
> > ---
> >  lib/dpctl.c             | 17 ++++++++---------
> >  tests/system-traffic.at |  2 ++
> >  2 files changed, 10 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;
> > -    }
> > -
> >      error = opt_dpif_open(argc, argv, dpctl_p, 5, &dpif);
> >      if (error) {
> >          dpctl_error(dpctl_p, error, "Cannot open dpif");
> > diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> > index 380372430..c6ec61dba 100644
> > --- a/tests/system-traffic.at
> > +++ b/tests/system-traffic.at
> > @@ -2360,8 +2360,10 @@
> priority=100,in_port=2,icmp,action=ct(zone=5,commit),1
> >  ])
> >
> >  AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
> > +dp=$(ovs-appctl dpctl/dump-dps)
> >
> >  m4_foreach([FLUSH_CMD], [[ovs-appctl dpctl/flush-conntrack],
> > +                         [ovs-appctl dpctl/flush-conntrack $dp],
> >                           [ovs-ofctl ct-flush br0]], [
> >  AS_BOX([Testing with FLUSH_CMD])
> >
>
> It looks like we don't have tests for a case where FLUSH_CMD is called
> without any arguments while all of them are optional.  Could you, please,
> add one?
>
> Best regards, Ilya Maximets.
>
>
Both added in v3.

Thanks,
Ales

-- 

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

Reply via email to