On Thu, Mar 9, 2023 at 3:00 PM Ales Musil <[email protected]> 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;
> - }
> -
> 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..d5723685a 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])
>
> @@ -2503,6 +2505,20 @@
> 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, partial flush by src address in reply
> direction
> +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])
> ])
>
> --
> 2.39.2
>
>
Ah I forgot to change the comment so this should be applied on top:
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index d5723685a..7115a8ef4 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -2507,7 +2507,7 @@ 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, partial flush by src address in reply
direction
+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)"])
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
