dpctl: Fix flush-conntrack with datapath as argument.

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.

Also add couple of test cases to ensure that everything works as
expected.

Fixes: a9ae73b916ba ("ofp, dpif: Allow CT flush based on partial match.")
Signed-off-by: Ales Musil <amusil@redhat.com>
Reviewed-by: Roi Dayan <roid@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

3 files changed, 55 insertions, 3 deletions

diff --git a/lib/dpctl.c b/lib/dpctl.c
index d12d9b8a5..970373389 100644
--- a/lib/dpctl.c
+++ b/lib/dpctl.c
@@ -1713,10 +1713,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;
@@ -1744,7 +1750,7 @@ dpctl_flush_conntrack(int argc, const char *argv[],
     }
 
     /* Report error if there is more than one unparsed argument. */
-    if (args > 1) {
+    if (args > 0) {
         ds_put_cstr(&ds, "invalid arguments");
         error = EINVAL;
         goto error;
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index bff1d43a0..6678911b4 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])
 
@@ -2504,8 +2506,48 @@ udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.
 AT_CHECK([FLUSH_CMD zone=5 '' 'ct_nw_src=10.1.1.1'])
 
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+
+dnl Test UDP from port 1 and 2, flush without arguments
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"])
+
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1" | sort], [0], [dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
+udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),zone=5
 ])
 
+AT_CHECK([FLUSH_CMD])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "10\.1\.1\.1"], [1])
+])
+
+dnl Test flush with invalid arguments
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=invalid 'ct_nw_src=10.1.1.1' 'ct_nw_dst=10.1.1.1'], [2], [ignore], [stderr])
+AT_CHECK([grep -q "failed to parse zone" stderr])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1 'ct_nw_src=10.1.1.1,invalid=invalid' 'ct_nw_dst=10.1.1.1'], [2], [ignore], [stderr])
+AT_CHECK([grep -q "invalid conntrack tuple field: invalid" stderr])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1 'ct_nw_src=invalid' 'ct_nw_dst=10.1.1.1'], [2], [ignore], [stderr])
+AT_CHECK([grep -q "failed to parse field ct_nw_src" stderr])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack zone=1 'ct_nw_src=10.1.1.1' 'ct_nw_dst=10.1.1.1' invalid], [2], [ignore], [stderr])
+AT_CHECK([grep -q "invalid arguments" stderr])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack $dp zone=1 'ct_nw_src=10.1.1.1' 'ct_nw_dst=10.1.1.1' invalid], [2], [ignore], [stderr])
+AT_CHECK([grep -q "command takes at most 4 arguments" stderr])
+
+AT_CHECK([ovs-appctl dpctl/flush-conntrack $dp 'ct_nw_src=10.1.1.1' 'ct_nw_dst=10.1.1.1' invalid], [2], [ignore], [stderr])
+AT_CHECK([grep -q "invalid arguments" stderr])
+
+AT_CHECK([ovs-ofctl ct-flush br0 zone=1 'ct_nw_src=10.1.1.1' 'ct_nw_dst=10.1.1.1' invalid], [1], [ignore], [stderr])
+AT_CHECK([grep -q "command takes at most 4 arguments" stderr])
+
+AT_CHECK([ovs-ofctl ct-flush br0 'ct_nw_src=10.1.1.1' 'ct_nw_dst=10.1.1.1' invalid], [1], [ignore], [stderr])
+AT_CHECK([grep -q "Invalid arguments" stderr])
+
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index eabec18a3..3ce4e82ec 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -3089,6 +3089,10 @@ ofctl_ct_flush(struct ovs_cmdl_context *ctx)
         args--;
     }
 
+    if (args > 0) {
+        ovs_fatal(0, "Invalid arguments");
+    }
+
     open_vconn(ctx->argv[1], &vconn);
     enum ofp_version version = vconn_get_version(vconn);
     struct ofpbuf *msg = ofp_ct_match_encode(&match, pzone, version);