On Tue, Nov 28, 2023 at 2:47 PM Ilya Maximets <[email protected]> wrote:
> On 11/2/23 13:00, Ales Musil wrote: > > Add optional argument to dpctl ct-del-limits called > > "default", which allows to remove the default limit > > making it effectively system default. > > > > Signed-off-by: Ales Musil <[email protected]> > > --- > > v6: Rebase on top of current master. > > Address comments from Ilya: > > - Adjust the log message so it doesn't report anything for default > zone. > > v5: Rebase on top of current master. > > Address comments from Ilya: > > - Correct the NEWS entry. > > - Fix style related problems. > > --- > > NEWS | 3 +++ > > lib/conntrack.c | 13 +++++++------ > > lib/dpctl.c | 21 +++++++++++++++------ > > tests/system-traffic.at | 26 ++++++++++++++++++++++++++ > > 4 files changed, 51 insertions(+), 12 deletions(-) > > > > diff --git a/NEWS b/NEWS > > index 6b45492f1..7bc27b687 100644 > > --- a/NEWS > > +++ b/NEWS > > @@ -6,6 +6,9 @@ Post-v3.2.0 > > from older version is supported but it may trigger more leader > elections > > during the process, and error logs complaining unrecognized > fields may > > be observed on old nodes. > > + - ovs-appctl: > > + * Added support removal of default CT zone limit, e.g. > > Added support for ... > > > + "ovs-appctl dpctl/ct-del-limits default". > > > > > > v3.2.0 - 17 Aug 2023 > > diff --git a/lib/conntrack.c b/lib/conntrack.c > > index 31f00a127..b533dd3df 100644 > > --- a/lib/conntrack.c > > +++ b/lib/conntrack.c > > @@ -404,13 +404,14 @@ zone_limit_delete(struct conntrack *ct, int32_t > zone) > > struct zone_limit *zl = zone_limit_lookup_protected(ct, zone); > > if (zl) { > > zone_limit_clean(ct, zl); > > - ovs_mutex_unlock(&ct->ct_lock); > > - VLOG_INFO("Deleted zone limit for zone %d", zone); > > - } else { > > - ovs_mutex_unlock(&ct->ct_lock); > > - VLOG_INFO("Attempted delete of non-existent zone limit: zone > %d", > > - zone); > > } > > + > > + if (zone != DEFAULT_ZONE) { > > + VLOG_INFO(zl ? "Deleted zone limit for zone %d" : "Attempted > delete" > > + " of non-existent zone limit: zone %d", zone); > > Nit: Might be better to not split the string. E.g.: > > VLOG_INFO(zl ? "Deleted zone limit for zone %d" > : "Attempted delete of non-existent zone limit: zone > %d", > zone); > > > + } > > + > > + ovs_mutex_unlock(&ct->ct_lock); > > return 0; > > } > > > > diff --git a/lib/dpctl.c b/lib/dpctl.c > > index 76f21a530..a8c654747 100644 > > --- a/lib/dpctl.c > > +++ b/lib/dpctl.c > > @@ -2291,14 +2291,23 @@ dpctl_ct_del_limits(int argc, const char *argv[], > > int i = dp_arg_exists(argc, argv) ? 2 : 1; > > struct ovs_list zone_limits = OVS_LIST_INITIALIZER(&zone_limits); > > > > - error = opt_dpif_open(argc, argv, dpctl_p, 3, &dpif); > > + error = opt_dpif_open(argc, argv, dpctl_p, 4, &dpif); > > if (error) { > > return error; > > } > > > > - error = parse_ct_limit_zones(argv[i], &zone_limits, &ds); > > - if (error) { > > - goto error; > > + /* Parse default limit. */ > > + if (!strcmp(argv[i], "default")) { > > + ct_dpif_push_zone_limit(&zone_limits, > OVS_ZONE_LIMIT_DEFAULT_ZONE, > > + 0, 0); > > + i++; > > + } > > + > > + if (argc > i) { > > + error = parse_ct_limit_zones(argv[i], &zone_limits, &ds); > > + if (error) { > > + goto error; > > + } > > } > > > > error = ct_dpif_del_limits(dpif, &zone_limits); > > @@ -3031,8 +3040,8 @@ static const struct dpctl_command all_commands[] = > { > > { "ct-get-tcp-seq-chk", "[dp]", 0, 1, dpctl_ct_get_tcp_seq_chk, > DP_RO }, > > { "ct-set-limits", "[dp] [default=L] [zone=N,limit=L]...", 1, > INT_MAX, > > dpctl_ct_set_limits, DP_RO }, > > - { "ct-del-limits", "[dp] zone=N1[,N2]...", 1, 2, > dpctl_ct_del_limits, > > - DP_RO }, > > + { "ct-del-limits", "[dp] [default] [zone=N1[,N2]...]", 1, 3, > > + dpctl_ct_del_limits, DP_RO }, > > { "ct-get-limits", "[dp] [zone=N1[,N2]...]", 0, 2, > dpctl_ct_get_limits, > > DP_RO }, > > { "ct-get-sweep-interval", "[dp]", 0, 1, dpctl_ct_get_sweep, DP_RO > }, > > diff --git a/tests/system-traffic.at b/tests/system-traffic.at > > index 7ea450202..b6c8d7faf 100644 > > --- a/tests/system-traffic.at > > +++ b/tests/system-traffic.at > > @@ -5195,6 +5195,32 @@ > udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=3),reply=(src=10.1.1.4,dst=10. > > > udp,orig=(src=10.1.1.3,dst=10.1.1.4,sport=1,dport=4),reply=(src=10.1.1.4,dst=10.1.1.3,sport=4,dport=1),zone=3 > > ]) > > > > +dnl Test ct-del-limits for default zone. > > + > > +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15 zone=4,limit=4]) > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl > > +default limit=15 > > +zone=4,limit=4,count=0 > > +]) > > + > > +AT_CHECK([ovs-appctl dpctl/ct-del-limits default]) > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl > > +default limit=0 > > +zone=4,limit=4,count=0 > > +]) > > + > > +AT_CHECK([ovs-appctl dpctl/ct-set-limits default=15]) > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl > > +default limit=15 > > +zone=4,limit=4,count=0 > > +]) > > + > > +AT_CHECK([ovs-appctl dpctl/ct-del-limits default zone=4]) > > +AT_CHECK([ovs-appctl dpctl/ct-get-limits zone=4], [0], [dnl > > +default limit=0 > > +zone=4,limit=0,count=0 > > +]) > > + > > OVS_TRAFFIC_VSWITCHD_STOP(["dnl > > /could not create datapath/d > > /(Cannot allocate memory) on packet/d"]) > > Thank you for the review, everything should be addressed in v7. Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> [email protected] <https://red.ht/sig> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
