On Tue, Nov 28, 2023 at 3:01 PM Ilya Maximets <i.maxim...@ovn.org> wrote:
> On 11/2/23 13:00, Ales Musil wrote: > > Make sure that if any zone limit was set via DB > > all zones are forced to be set there also. This > > is done by tracking which datapath has zone limit > > protection and it is reflected in the dpctl command. > > > > If the datapath is protected the dpctl command will > > return permission error. > > > > Signed-off-by: Ales Musil <amu...@redhat.com> > > --- > > v6: Rebase on top of current master. > > Address comments from Ilya: > > - Drop the log message about protection. > > - Make the dpctl error message more user-friendly. > > - Do not ignore error messages in the system-test. > > v5: Rebase on top of current master. > > Address comments from Ilya: > > - Add more user friendly error message to the dpctl. > > - Fix style related problems. > > v4: Rebase on top of current master. > > Make the protection datapath wide. > > --- > > lib/ct-dpif.c | 25 +++++++++++++++++++++ > > lib/ct-dpif.h | 2 ++ > > lib/dpctl.c | 14 ++++++++++++ > > ofproto/ofproto-dpif.c | 13 +++++++++++ > > ofproto/ofproto-provider.h | 5 +++++ > > ofproto/ofproto.c | 11 +++++++++ > > ofproto/ofproto.h | 2 ++ > > tests/ofproto-macros.at | 4 ++-- > > tests/system-traffic.at | 46 ++++++++++++++++++++++++++++++++++++++ > > vswitchd/bridge.c | 7 ++++++ > > 10 files changed, 127 insertions(+), 2 deletions(-) > > > > diff --git a/lib/ct-dpif.c b/lib/ct-dpif.c > > index 2ee045164..5115c886b 100644 > > --- a/lib/ct-dpif.c > > +++ b/lib/ct-dpif.c > > @@ -23,6 +23,7 @@ > > #include "openvswitch/ofp-ct.h" > > #include "openvswitch/ofp-parse.h" > > #include "openvswitch/vlog.h" > > +#include "sset.h" > > > > VLOG_DEFINE_THIS_MODULE(ct_dpif); > > > > @@ -32,6 +33,10 @@ struct flags { > > const char *name; > > }; > > > > +/* Protection for CT zone limit per datapath. */ > > +static struct sset ct_limit_protection = > > + SSET_INITIALIZER(&ct_limit_protection); > > + > > static void ct_dpif_format_counters(struct ds *, > > const struct ct_dpif_counters *); > > static void ct_dpif_format_timestamp(struct ds *, > > @@ -1064,3 +1069,23 @@ ct_dpif_get_features(struct dpif *dpif, enum > ct_features *features) > > ? dpif->dpif_class->ct_get_features(dpif, features) > > : EOPNOTSUPP); > > } > > + > > +void > > +ct_dpif_set_zone_limit_protection(struct dpif *dpif, bool protected) > > +{ > > + if (sset_contains(&ct_limit_protection, dpif->full_name) == > protected) { > > + return; > > + } > > + > > + if (protected) { > > + sset_add(&ct_limit_protection, dpif->full_name); > > + } else { > > + sset_find_and_delete(&ct_limit_protection, dpif->full_name); > > + } > > +} > > + > > +bool > > +ct_dpif_is_zone_limit_protected(struct dpif *dpif) > > +{ > > + return sset_contains(&ct_limit_protection, dpif->full_name); > > +} > > diff --git a/lib/ct-dpif.h b/lib/ct-dpif.h > > index c8a7c155e..c3786d5ae 100644 > > --- a/lib/ct-dpif.h > > +++ b/lib/ct-dpif.h > > @@ -350,5 +350,7 @@ int ct_dpif_get_timeout_policy_name(struct dpif > *dpif, uint32_t tp_id, > > uint16_t dl_type, uint8_t nw_proto, > > char **tp_name, bool *is_generic); > > int ct_dpif_get_features(struct dpif *dpif, enum ct_features *features); > > +void ct_dpif_set_zone_limit_protection(struct dpif *, bool protected); > > +bool ct_dpif_is_zone_limit_protected(struct dpif *); > > > > #endif /* CT_DPIF_H */ > > diff --git a/lib/dpctl.c b/lib/dpctl.c > > index a8c654747..2a1aac5e5 100644 > > --- a/lib/dpctl.c > > +++ b/lib/dpctl.c > > @@ -2234,6 +2234,13 @@ dpctl_ct_set_limits(int argc, const char *argv[], > > ct_dpif_push_zone_limit(&zone_limits, zone, limit, 0); > > } > > > > + if (ct_dpif_is_zone_limit_protected(dpif)) { > > + ds_put_cstr(&ds, "the zone limits are set via database, " > > + "use 'ovs-vsctl set-zone-limit <...>' > instead."); > > + error = EPERM; > > + goto error; > > + } > > + > > error = ct_dpif_set_limits(dpif, &zone_limits); > > if (!error) { > > ct_dpif_free_zone_limits(&zone_limits); > > @@ -2310,6 +2317,13 @@ dpctl_ct_del_limits(int argc, const char *argv[], > > } > > } > > > > + if (ct_dpif_is_zone_limit_protected(dpif)) { > > + ds_put_cstr(&ds, "the zone limits are set via database, " > > + "use 'ovs-vsctl del-zone-limit <...>' > instead."); > > + error = EPERM; > > + goto error; > > + } > > + > > error = ct_dpif_del_limits(dpif, &zone_limits); > > if (!error) { > > goto out; > > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > > index 6a931a806..4cc8e2807 100644 > > --- a/ofproto/ofproto-dpif.c > > +++ b/ofproto/ofproto-dpif.c > > @@ -5663,6 +5663,18 @@ ct_zone_limits_commit(struct dpif_backer *backer) > > } > > } > > > > +static void > > +ct_zone_limit_protection_update(const char *datapath_type, bool > protected) > > +{ > > + struct dpif_backer *backer = shash_find_data(&all_dpif_backers, > > + datapath_type); > > + if (!backer) { > > + return; > > + } > > + > > + ct_dpif_set_zone_limit_protection(backer->dpif, protected); > > +} > > + > > static void > > get_datapath_cap(const char *datapath_type, struct smap *cap) > > { > > @@ -6953,4 +6965,5 @@ const struct ofproto_class ofproto_dpif_class = { > > ct_set_zone_timeout_policy, > > ct_del_zone_timeout_policy, > > ct_zone_limit_update, > > + ct_zone_limit_protection_update, > > }; > > diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h > > index face0b574..83c509fcf 100644 > > --- a/ofproto/ofproto-provider.h > > +++ b/ofproto/ofproto-provider.h > > @@ -1929,6 +1929,11 @@ struct ofproto_class { > > * within proper range (0 - UINT32_MAX). */ > > void (*ct_zone_limit_update)(const char *dp_type, int32_t zone, > > int64_t *limit); > > + > > + /* Sets the CT zone limit protection to "protected" for the > specified > > + * datapath type. */ > > + void (*ct_zone_limit_protection_update)(const char *dp_type, > > + bool protected); > > }; > > > > extern const struct ofproto_class ofproto_dpif_class; > > diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c > > index 649add089..122a06f30 100644 > > --- a/ofproto/ofproto.c > > +++ b/ofproto/ofproto.c > > @@ -1038,6 +1038,17 @@ ofproto_ct_zone_limit_update(const char > *datapath_type, int32_t zone_id, > > } > > } > > > > +void > > +ofproto_ct_zone_limit_protection_update(const char *datapath_type, > > + bool protected) > > +{ > > + datapath_type = ofproto_normalize_type(datapath_type); > > + const struct ofproto_class *class = > ofproto_class_find__(datapath_type); > > + > > + if (class && class->ct_zone_limit_protection_update) { > > + class->ct_zone_limit_protection_update(datapath_type, > protected); > > + } > > +} > > > > /* Spanning Tree Protocol (STP) configuration. */ > > > > diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h > > index 7ce6a65e1..1c07df275 100644 > > --- a/ofproto/ofproto.h > > +++ b/ofproto/ofproto.h > > @@ -386,6 +386,8 @@ void ofproto_ct_del_zone_timeout_policy(const char > *datapath_type, > > uint16_t zone); > > void ofproto_ct_zone_limit_update(const char *datapath_type, int32_t > zone_id, > > int64_t *limit); > > +void ofproto_ct_zone_limit_protection_update(const char *datapath_type, > > + bool protected); > > void ofproto_get_datapath_cap(const char *datapath_type, > > struct smap *dp_cap); > > > > diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at > > index d2e6ac768..09d21f916 100644 > > --- a/tests/ofproto-macros.at > > +++ b/tests/ofproto-macros.at > > @@ -171,14 +171,14 @@ m4_define([_OVS_VSWITCHD_START], > > AT_CHECK([[sed < stderr ' > > /vlog|INFO|opened log file/d > > /ovsdb_server|INFO|ovsdb-server (Open vSwitch)/d']]) > > - AT_CAPTURE_FILE([ovsdb-server.log]) > > + #AT_CAPTURE_FILE([ovsdb-server.log]) > > > > dnl Initialize database. > > AT_CHECK([ovs-vsctl --no-wait init $2]) > > > > dnl Start ovs-vswitchd. > > AT_CHECK([ovs-vswitchd $1 --detach --no-chdir --pidfile --log-file > -vvconn -vofproto_dpif -vunixctl], [0], [], [stderr]) > > - AT_CAPTURE_FILE([ovs-vswitchd.log]) > > + #AT_CAPTURE_FILE([ovs-vswitchd.log]) > > on_exit "kill_ovs_vswitchd `cat ovs-vswitchd.pid`" > > AT_CHECK([[sed < stderr ' > > /ovs_numa|INFO|Discovered /d > > > This chunk looks like a testing artifact. > > Best regards, Ilya Maximets. > > Yes, sorry about that fixed in v7. Thanks, Ales -- Ales Musil Senior Software Engineer - OVN Core Red Hat EMEA <https://www.redhat.com> amu...@redhat.com <https://red.ht/sig> _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev