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. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev