| Message ID | 20240712151416.992033-8-dceara@redhat.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Mark Michelson |
| Headers | show |
| Series | Add ACL Sampling using per-flow IPFIX. | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | success | apply and check: success |
| ovsrobot/github-robot-_Build_and_Test | success | github build: passed |
| ovsrobot/github-robot-_ovn-kubernetes | fail | github build: failed |
On Fri, Jul 12, 2024 at 5:15 PM Dumitru Ceara <dceara@redhat.com> wrote: > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > --- > Hi Dumitru, I have a couple of small comments down below. NEWS | 3 +++ > northd/debug.c | 12 +++++++----- > northd/debug.h | 3 ++- > northd/en-global-config.c | 31 +++++++++++++++++++++++-------- > northd/inc-proc-northd.c | 1 + > tests/ovn-northd.at | 27 ++++++++++++++++++++++++++- > 6 files changed, 62 insertions(+), 15 deletions(-) > > diff --git a/NEWS b/NEWS > index 3e392ff08b..fcf182bc02 100644 > --- a/NEWS > +++ b/NEWS > @@ -38,6 +38,9 @@ Post v24.03.0 > ability to disable "VXLAN mode" to extend available tunnel IDs space > for > datapaths from 4095 to 16711680. For more details see man ovn-nb(5) > for > mentioned option. > + - The NB_Global.debug_drop_domain_id configured value is now overridden > by > + the ID associated with the Sampling_App record created for drop > sampling > + (Sampling_App.name configured to be "drop-sampling"). > Should we also deprecate the old global option once this drops? I think it would make sense. > OVN v24.03.0 - 01 Mar 2024 > -------------------------- > diff --git a/northd/debug.c b/northd/debug.c > index 59da5d4f66..457993b7cf 100644 > --- a/northd/debug.c > +++ b/northd/debug.c > @@ -3,6 +3,7 @@ > #include <string.h> > > #include "debug.h" > +#include "en-sampling-app.h" > > #include "openvswitch/dynamic-string.h" > #include "openvswitch/vlog.h" > @@ -26,16 +27,17 @@ debug_enabled(void) > } > > void > -init_debug_config(const struct nbrec_nb_global *nb) > +init_debug_config(const struct nbrec_nb_global *nb, > + uint8_t drop_domain_id_override) > { > - > const struct smap *options = &nb->options; > uint32_t collector_set_id = smap_get_uint(options, > "debug_drop_collector_set", > 0); > - uint32_t observation_domain_id = smap_get_uint(options, > - "debug_drop_domain_id", > - 0); > + uint32_t observation_domain_id = > + drop_domain_id_override != SAMPLING_APP_ID_NONE > + ? drop_domain_id_override > + : smap_get_uint(options, "debug_drop_domain_id", 0); > > if (collector_set_id != config.collector_set_id || > observation_domain_id != config.observation_domain_id || > diff --git a/northd/debug.h b/northd/debug.h > index c1a5e5aadb..a0b535253a 100644 > --- a/northd/debug.h > +++ b/northd/debug.h > @@ -21,7 +21,8 @@ > #include "lib/ovn-nb-idl.h" > #include "openvswitch/dynamic-string.h" > > -void init_debug_config(const struct nbrec_nb_global *nb); > +void init_debug_config(const struct nbrec_nb_global *nb, > + uint8_t drop_domain_id_override); > void destroy_debug_config(void); > > const char *debug_drop_action(void); > diff --git a/northd/en-global-config.c b/northd/en-global-config.c > index 5b71ede1f2..c683c8fae5 100644 > --- a/northd/en-global-config.c > +++ b/northd/en-global-config.c > @@ -24,6 +24,7 @@ > /* OVN includes */ > #include "debug.h" > #include "en-global-config.h" > +#include "en-sampling-app.h" > #include "include/ovn/features.h" > #include "ipam.h" > #include "lib/ovn-nb-idl.h" > @@ -42,8 +43,10 @@ static bool chassis_features_changed(const struct > chassis_features *, > static bool config_out_of_sync(const struct smap *config, > const struct smap *saved_config, > const char *key, bool must_be_present); > -static bool check_nb_options_out_of_sync(const struct nbrec_nb_global *, > - struct ed_type_global_config *); > +static bool check_nb_options_out_of_sync( > + const struct nbrec_nb_global *, > + struct ed_type_global_config *, > + const struct sampling_app_table *); > static void update_sb_config_options_to_sbrec(struct > ed_type_global_config *, > const struct > sbrec_sb_global *); > > @@ -72,6 +75,9 @@ en_global_config_run(struct engine_node *node , void > *data) > EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); > const struct sbrec_chassis_table *sbrec_chassis_table = > EN_OVSDB_GET(engine_get_input("SB_chassis", node)); > + const struct ed_type_sampling_app_data *sampling_app_data = > + engine_get_input_data("sampling_app", node); > + const struct sampling_app_table *sampling_apps = > &sampling_app_data->apps; > > struct ed_type_global_config *config_data = data; > > @@ -145,7 +151,8 @@ en_global_config_run(struct engine_node *node , void > *data) > build_chassis_features(sbrec_chassis_table, > &config_data->features); > } > > - init_debug_config(nb); > + init_debug_config(nb, sampling_app_get_id(sampling_apps, > + SAMPLING_APP_DROP_DEBUG)); > > const struct sbrec_sb_global *sb = > sbrec_sb_global_table_first(sb_global_table); > @@ -186,6 +193,9 @@ global_config_nb_global_handler(struct engine_node > *node, void *data) > EN_OVSDB_GET(engine_get_input("NB_nb_global", node)); > const struct sbrec_sb_global_table *sb_global_table = > EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); > + const struct ed_type_sampling_app_data *sampling_app_data = > + engine_get_input_data("sampling_app", node); > + const struct sampling_app_table *sampling_apps = > &sampling_app_data->apps; > > const struct nbrec_nb_global *nb = > nbrec_nb_global_table_first(nb_global_table); > @@ -248,7 +258,7 @@ global_config_nb_global_handler(struct engine_node > *node, void *data) > return false; > } > > - if (check_nb_options_out_of_sync(nb, config_data)) { > + if (check_nb_options_out_of_sync(nb, config_data, sampling_apps)) { > config_data->tracked_data.nb_options_changed = true; > } > > @@ -461,8 +471,10 @@ config_out_of_sync(const struct smap *config, const > struct smap *saved_config, > } > > static bool > -check_nb_options_out_of_sync(const struct nbrec_nb_global *nb, > - struct ed_type_global_config *config_data) > +check_nb_options_out_of_sync( > + const struct nbrec_nb_global *nb, > + struct ed_type_global_config *config_data, > + const struct sampling_app_table *sampling_apps) > { > if (config_out_of_sync(&nb->options, &config_data->nb_options, > "mac_binding_removal_limit", false)) { > @@ -496,13 +508,16 @@ check_nb_options_out_of_sync(const struct > nbrec_nb_global *nb, > > if (config_out_of_sync(&nb->options, &config_data->nb_options, > "debug_drop_domain_id", false)) { > - init_debug_config(nb); > + init_debug_config(nb, sampling_app_get_id(sampling_apps, > + > SAMPLING_APP_DROP_DEBUG)); > + > return true; > } > > if (config_out_of_sync(&nb->options, &config_data->nb_options, > "debug_drop_collector_set", false)) { > - init_debug_config(nb); > + init_debug_config(nb, sampling_app_get_id(sampling_apps, > + > SAMPLING_APP_DROP_DEBUG)); > return true; > } > > diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c > index 5d89670c29..95bedc5cd0 100644 > --- a/northd/inc-proc-northd.c > +++ b/northd/inc-proc-northd.c > @@ -181,6 +181,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, > global_config_sb_global_handler); > engine_add_input(&en_global_config, &en_sb_chassis, > global_config_sb_chassis_handler); > + engine_add_input(&en_global_config, &en_sampling_app, NULL); > > engine_add_input(&en_northd, &en_nb_mirror, NULL); > engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL); > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at > index f2f11c005c..3fb883225a 100644 > --- a/tests/ovn-northd.at > +++ b/tests/ovn-northd.at > @@ -12459,13 +12459,38 @@ check as northd ovn-appctl -t ovn-northd > inc-engine/clear-stats > ovn-nbctl create Sampling_App name="acl-new-traffic-sampling" id="42" > check_row_count nb:Sampling_App 1 > check_engine_stats sampling_app recompute nocompute > -check_engine_stats northd norecompute nocompute > +check_engine_stats northd recomput nocompute > nit: s/recomput/recompute/ > check_engine_stats lflow recompute nocompute > +check_engine_stats global_config recompute nocompute > CHECK_NO_CHANGE_AFTER_RECOMPUTE > > AT_CLEANUP > ]) > > +OVN_FOR_EACH_NORTHD_NO_HV([ > +AT_SETUP([Sampling_App override debug_drop_domain_id]) > + > +ovn_start > + > +check ovn-nbctl -- set NB_Global . options:debug_drop_collector_set="123" > \ > + -- set NB_Global . options:debug_drop_domain_id="1" \ > + -- ls-add ls > +check ovn-nbctl --wait=sb sync > + > +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | > ovn_strip_lflows], [0], [dnl > + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == > "none"), > action=(sample(probability=65535,collector_set=123,obs_domain=1,obs_point=$cookie); > /* drop */) > +]) > + > +ovn-nbctl --wait=sb create Sampling_App name="drop-sampling" id="42" > +check_row_count nb:Sampling_App 1 > + > +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | > ovn_strip_lflows], [0], [dnl > + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == > "none"), > action=(sample(probability=65535,collector_set=123,obs_domain=42,obs_point=$cookie); > /* drop */) > +]) > + > +AT_CLEANUP > +]) > + > OVN_FOR_EACH_NORTHD_NO_HV([ > AT_SETUP([NAT with match]) > ovn_start > -- > 2.44.0 > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > > Thanks, Ales
I have no additions other than Ales's, so with those fixed, Acked-by: Mark Michelson <mmichels@redhat.com> On 7/15/24 05:22, Ales Musil wrote: > On Fri, Jul 12, 2024 at 5:15 PM Dumitru Ceara <dceara@redhat.com> wrote: > >> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >> --- >> > > > Hi Dumitru, > > I have a couple of small comments down below. > > NEWS | 3 +++ >> northd/debug.c | 12 +++++++----- >> northd/debug.h | 3 ++- >> northd/en-global-config.c | 31 +++++++++++++++++++++++-------- >> northd/inc-proc-northd.c | 1 + >> tests/ovn-northd.at | 27 ++++++++++++++++++++++++++- >> 6 files changed, 62 insertions(+), 15 deletions(-) >> >> diff --git a/NEWS b/NEWS >> index 3e392ff08b..fcf182bc02 100644 >> --- a/NEWS >> +++ b/NEWS >> @@ -38,6 +38,9 @@ Post v24.03.0 >> ability to disable "VXLAN mode" to extend available tunnel IDs space >> for >> datapaths from 4095 to 16711680. For more details see man ovn-nb(5) >> for >> mentioned option. >> + - The NB_Global.debug_drop_domain_id configured value is now overridden >> by >> + the ID associated with the Sampling_App record created for drop >> sampling >> + (Sampling_App.name configured to be "drop-sampling"). >> > > Should we also deprecate the old global option once this drops? I think it > would make sense. > > >> OVN v24.03.0 - 01 Mar 2024 >> -------------------------- >> diff --git a/northd/debug.c b/northd/debug.c >> index 59da5d4f66..457993b7cf 100644 >> --- a/northd/debug.c >> +++ b/northd/debug.c >> @@ -3,6 +3,7 @@ >> #include <string.h> >> >> #include "debug.h" >> +#include "en-sampling-app.h" >> >> #include "openvswitch/dynamic-string.h" >> #include "openvswitch/vlog.h" >> @@ -26,16 +27,17 @@ debug_enabled(void) >> } >> >> void >> -init_debug_config(const struct nbrec_nb_global *nb) >> +init_debug_config(const struct nbrec_nb_global *nb, >> + uint8_t drop_domain_id_override) >> { >> - >> const struct smap *options = &nb->options; >> uint32_t collector_set_id = smap_get_uint(options, >> "debug_drop_collector_set", >> 0); >> - uint32_t observation_domain_id = smap_get_uint(options, >> - "debug_drop_domain_id", >> - 0); >> + uint32_t observation_domain_id = >> + drop_domain_id_override != SAMPLING_APP_ID_NONE >> + ? drop_domain_id_override >> + : smap_get_uint(options, "debug_drop_domain_id", 0); >> >> if (collector_set_id != config.collector_set_id || >> observation_domain_id != config.observation_domain_id || >> diff --git a/northd/debug.h b/northd/debug.h >> index c1a5e5aadb..a0b535253a 100644 >> --- a/northd/debug.h >> +++ b/northd/debug.h >> @@ -21,7 +21,8 @@ >> #include "lib/ovn-nb-idl.h" >> #include "openvswitch/dynamic-string.h" >> >> -void init_debug_config(const struct nbrec_nb_global *nb); >> +void init_debug_config(const struct nbrec_nb_global *nb, >> + uint8_t drop_domain_id_override); >> void destroy_debug_config(void); >> >> const char *debug_drop_action(void); >> diff --git a/northd/en-global-config.c b/northd/en-global-config.c >> index 5b71ede1f2..c683c8fae5 100644 >> --- a/northd/en-global-config.c >> +++ b/northd/en-global-config.c >> @@ -24,6 +24,7 @@ >> /* OVN includes */ >> #include "debug.h" >> #include "en-global-config.h" >> +#include "en-sampling-app.h" >> #include "include/ovn/features.h" >> #include "ipam.h" >> #include "lib/ovn-nb-idl.h" >> @@ -42,8 +43,10 @@ static bool chassis_features_changed(const struct >> chassis_features *, >> static bool config_out_of_sync(const struct smap *config, >> const struct smap *saved_config, >> const char *key, bool must_be_present); >> -static bool check_nb_options_out_of_sync(const struct nbrec_nb_global *, >> - struct ed_type_global_config *); >> +static bool check_nb_options_out_of_sync( >> + const struct nbrec_nb_global *, >> + struct ed_type_global_config *, >> + const struct sampling_app_table *); >> static void update_sb_config_options_to_sbrec(struct >> ed_type_global_config *, >> const struct >> sbrec_sb_global *); >> >> @@ -72,6 +75,9 @@ en_global_config_run(struct engine_node *node , void >> *data) >> EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); >> const struct sbrec_chassis_table *sbrec_chassis_table = >> EN_OVSDB_GET(engine_get_input("SB_chassis", node)); >> + const struct ed_type_sampling_app_data *sampling_app_data = >> + engine_get_input_data("sampling_app", node); >> + const struct sampling_app_table *sampling_apps = >> &sampling_app_data->apps; >> >> struct ed_type_global_config *config_data = data; >> >> @@ -145,7 +151,8 @@ en_global_config_run(struct engine_node *node , void >> *data) >> build_chassis_features(sbrec_chassis_table, >> &config_data->features); >> } >> >> - init_debug_config(nb); >> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >> + SAMPLING_APP_DROP_DEBUG)); >> >> const struct sbrec_sb_global *sb = >> sbrec_sb_global_table_first(sb_global_table); >> @@ -186,6 +193,9 @@ global_config_nb_global_handler(struct engine_node >> *node, void *data) >> EN_OVSDB_GET(engine_get_input("NB_nb_global", node)); >> const struct sbrec_sb_global_table *sb_global_table = >> EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); >> + const struct ed_type_sampling_app_data *sampling_app_data = >> + engine_get_input_data("sampling_app", node); >> + const struct sampling_app_table *sampling_apps = >> &sampling_app_data->apps; >> >> const struct nbrec_nb_global *nb = >> nbrec_nb_global_table_first(nb_global_table); >> @@ -248,7 +258,7 @@ global_config_nb_global_handler(struct engine_node >> *node, void *data) >> return false; >> } >> >> - if (check_nb_options_out_of_sync(nb, config_data)) { >> + if (check_nb_options_out_of_sync(nb, config_data, sampling_apps)) { >> config_data->tracked_data.nb_options_changed = true; >> } >> >> @@ -461,8 +471,10 @@ config_out_of_sync(const struct smap *config, const >> struct smap *saved_config, >> } >> >> static bool >> -check_nb_options_out_of_sync(const struct nbrec_nb_global *nb, >> - struct ed_type_global_config *config_data) >> +check_nb_options_out_of_sync( >> + const struct nbrec_nb_global *nb, >> + struct ed_type_global_config *config_data, >> + const struct sampling_app_table *sampling_apps) >> { >> if (config_out_of_sync(&nb->options, &config_data->nb_options, >> "mac_binding_removal_limit", false)) { >> @@ -496,13 +508,16 @@ check_nb_options_out_of_sync(const struct >> nbrec_nb_global *nb, >> >> if (config_out_of_sync(&nb->options, &config_data->nb_options, >> "debug_drop_domain_id", false)) { >> - init_debug_config(nb); >> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >> + >> SAMPLING_APP_DROP_DEBUG)); >> + >> return true; >> } >> >> if (config_out_of_sync(&nb->options, &config_data->nb_options, >> "debug_drop_collector_set", false)) { >> - init_debug_config(nb); >> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >> + >> SAMPLING_APP_DROP_DEBUG)); >> return true; >> } >> >> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c >> index 5d89670c29..95bedc5cd0 100644 >> --- a/northd/inc-proc-northd.c >> +++ b/northd/inc-proc-northd.c >> @@ -181,6 +181,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, >> global_config_sb_global_handler); >> engine_add_input(&en_global_config, &en_sb_chassis, >> global_config_sb_chassis_handler); >> + engine_add_input(&en_global_config, &en_sampling_app, NULL); >> >> engine_add_input(&en_northd, &en_nb_mirror, NULL); >> engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL); >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index f2f11c005c..3fb883225a 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -12459,13 +12459,38 @@ check as northd ovn-appctl -t ovn-northd >> inc-engine/clear-stats >> ovn-nbctl create Sampling_App name="acl-new-traffic-sampling" id="42" >> check_row_count nb:Sampling_App 1 >> check_engine_stats sampling_app recompute nocompute >> -check_engine_stats northd norecompute nocompute >> +check_engine_stats northd recomput nocompute >> > > nit: s/recomput/recompute/ > > >> check_engine_stats lflow recompute nocompute >> +check_engine_stats global_config recompute nocompute >> CHECK_NO_CHANGE_AFTER_RECOMPUTE >> >> AT_CLEANUP >> ]) >> >> +OVN_FOR_EACH_NORTHD_NO_HV([ >> +AT_SETUP([Sampling_App override debug_drop_domain_id]) >> + >> +ovn_start >> + >> +check ovn-nbctl -- set NB_Global . options:debug_drop_collector_set="123" >> \ >> + -- set NB_Global . options:debug_drop_domain_id="1" \ >> + -- ls-add ls >> +check ovn-nbctl --wait=sb sync >> + >> +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | >> ovn_strip_lflows], [0], [dnl >> + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == >> "none"), >> action=(sample(probability=65535,collector_set=123,obs_domain=1,obs_point=$cookie); >> /* drop */) >> +]) >> + >> +ovn-nbctl --wait=sb create Sampling_App name="drop-sampling" id="42" >> +check_row_count nb:Sampling_App 1 >> + >> +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | >> ovn_strip_lflows], [0], [dnl >> + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == >> "none"), >> action=(sample(probability=65535,collector_set=123,obs_domain=42,obs_point=$cookie); >> /* drop */) >> +]) >> + >> +AT_CLEANUP >> +]) >> + >> OVN_FOR_EACH_NORTHD_NO_HV([ >> AT_SETUP([NAT with match]) >> ovn_start >> -- >> 2.44.0 >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> > Thanks, > Ales >
On 7/23/24 15:18, Mark Michelson wrote: > I have no additions other than Ales's, so with those fixed, > > Acked-by: Mark Michelson <mmichels@redhat.com> > Whoops, this was meant to be my reply on patch 6, not patch 7. I'll move the reply to patch 6, and you can ignore this reply for now. It's possible that once I actually review patch 7 this will also be my opinion. Please hold... > On 7/15/24 05:22, Ales Musil wrote: >> On Fri, Jul 12, 2024 at 5:15 PM Dumitru Ceara <dceara@redhat.com> wrote: >> >>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >>> --- >>> >> >> >> Hi Dumitru, >> >> I have a couple of small comments down below. >> >> NEWS | 3 +++ >>> northd/debug.c | 12 +++++++----- >>> northd/debug.h | 3 ++- >>> northd/en-global-config.c | 31 +++++++++++++++++++++++-------- >>> northd/inc-proc-northd.c | 1 + >>> tests/ovn-northd.at | 27 ++++++++++++++++++++++++++- >>> 6 files changed, 62 insertions(+), 15 deletions(-) >>> >>> diff --git a/NEWS b/NEWS >>> index 3e392ff08b..fcf182bc02 100644 >>> --- a/NEWS >>> +++ b/NEWS >>> @@ -38,6 +38,9 @@ Post v24.03.0 >>> ability to disable "VXLAN mode" to extend available tunnel IDs >>> space >>> for >>> datapaths from 4095 to 16711680. For more details see man >>> ovn-nb(5) >>> for >>> mentioned option. >>> + - The NB_Global.debug_drop_domain_id configured value is now >>> overridden >>> by >>> + the ID associated with the Sampling_App record created for drop >>> sampling >>> + (Sampling_App.name configured to be "drop-sampling"). >>> >> >> Should we also deprecate the old global option once this drops? I >> think it >> would make sense. >> >> >>> OVN v24.03.0 - 01 Mar 2024 >>> -------------------------- >>> diff --git a/northd/debug.c b/northd/debug.c >>> index 59da5d4f66..457993b7cf 100644 >>> --- a/northd/debug.c >>> +++ b/northd/debug.c >>> @@ -3,6 +3,7 @@ >>> #include <string.h> >>> >>> #include "debug.h" >>> +#include "en-sampling-app.h" >>> >>> #include "openvswitch/dynamic-string.h" >>> #include "openvswitch/vlog.h" >>> @@ -26,16 +27,17 @@ debug_enabled(void) >>> } >>> >>> void >>> -init_debug_config(const struct nbrec_nb_global *nb) >>> +init_debug_config(const struct nbrec_nb_global *nb, >>> + uint8_t drop_domain_id_override) >>> { >>> - >>> const struct smap *options = &nb->options; >>> uint32_t collector_set_id = smap_get_uint(options, >>> >>> "debug_drop_collector_set", >>> 0); >>> - uint32_t observation_domain_id = smap_get_uint(options, >>> - >>> "debug_drop_domain_id", >>> - 0); >>> + uint32_t observation_domain_id = >>> + drop_domain_id_override != SAMPLING_APP_ID_NONE >>> + ? drop_domain_id_override >>> + : smap_get_uint(options, "debug_drop_domain_id", 0); >>> >>> if (collector_set_id != config.collector_set_id || >>> observation_domain_id != config.observation_domain_id || >>> diff --git a/northd/debug.h b/northd/debug.h >>> index c1a5e5aadb..a0b535253a 100644 >>> --- a/northd/debug.h >>> +++ b/northd/debug.h >>> @@ -21,7 +21,8 @@ >>> #include "lib/ovn-nb-idl.h" >>> #include "openvswitch/dynamic-string.h" >>> >>> -void init_debug_config(const struct nbrec_nb_global *nb); >>> +void init_debug_config(const struct nbrec_nb_global *nb, >>> + uint8_t drop_domain_id_override); >>> void destroy_debug_config(void); >>> >>> const char *debug_drop_action(void); >>> diff --git a/northd/en-global-config.c b/northd/en-global-config.c >>> index 5b71ede1f2..c683c8fae5 100644 >>> --- a/northd/en-global-config.c >>> +++ b/northd/en-global-config.c >>> @@ -24,6 +24,7 @@ >>> /* OVN includes */ >>> #include "debug.h" >>> #include "en-global-config.h" >>> +#include "en-sampling-app.h" >>> #include "include/ovn/features.h" >>> #include "ipam.h" >>> #include "lib/ovn-nb-idl.h" >>> @@ -42,8 +43,10 @@ static bool chassis_features_changed(const struct >>> chassis_features *, >>> static bool config_out_of_sync(const struct smap *config, >>> const struct smap *saved_config, >>> const char *key, bool must_be_present); >>> -static bool check_nb_options_out_of_sync(const struct >>> nbrec_nb_global *, >>> - struct >>> ed_type_global_config *); >>> +static bool check_nb_options_out_of_sync( >>> + const struct nbrec_nb_global *, >>> + struct ed_type_global_config *, >>> + const struct sampling_app_table *); >>> static void update_sb_config_options_to_sbrec(struct >>> ed_type_global_config *, >>> const struct >>> sbrec_sb_global *); >>> >>> @@ -72,6 +75,9 @@ en_global_config_run(struct engine_node *node , void >>> *data) >>> EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); >>> const struct sbrec_chassis_table *sbrec_chassis_table = >>> EN_OVSDB_GET(engine_get_input("SB_chassis", node)); >>> + const struct ed_type_sampling_app_data *sampling_app_data = >>> + engine_get_input_data("sampling_app", node); >>> + const struct sampling_app_table *sampling_apps = >>> &sampling_app_data->apps; >>> >>> struct ed_type_global_config *config_data = data; >>> >>> @@ -145,7 +151,8 @@ en_global_config_run(struct engine_node *node , void >>> *data) >>> build_chassis_features(sbrec_chassis_table, >>> &config_data->features); >>> } >>> >>> - init_debug_config(nb); >>> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >>> + >>> SAMPLING_APP_DROP_DEBUG)); >>> >>> const struct sbrec_sb_global *sb = >>> sbrec_sb_global_table_first(sb_global_table); >>> @@ -186,6 +193,9 @@ global_config_nb_global_handler(struct engine_node >>> *node, void *data) >>> EN_OVSDB_GET(engine_get_input("NB_nb_global", node)); >>> const struct sbrec_sb_global_table *sb_global_table = >>> EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); >>> + const struct ed_type_sampling_app_data *sampling_app_data = >>> + engine_get_input_data("sampling_app", node); >>> + const struct sampling_app_table *sampling_apps = >>> &sampling_app_data->apps; >>> >>> const struct nbrec_nb_global *nb = >>> nbrec_nb_global_table_first(nb_global_table); >>> @@ -248,7 +258,7 @@ global_config_nb_global_handler(struct engine_node >>> *node, void *data) >>> return false; >>> } >>> >>> - if (check_nb_options_out_of_sync(nb, config_data)) { >>> + if (check_nb_options_out_of_sync(nb, config_data, sampling_apps)) { >>> config_data->tracked_data.nb_options_changed = true; >>> } >>> >>> @@ -461,8 +471,10 @@ config_out_of_sync(const struct smap *config, const >>> struct smap *saved_config, >>> } >>> >>> static bool >>> -check_nb_options_out_of_sync(const struct nbrec_nb_global *nb, >>> - struct ed_type_global_config *config_data) >>> +check_nb_options_out_of_sync( >>> + const struct nbrec_nb_global *nb, >>> + struct ed_type_global_config *config_data, >>> + const struct sampling_app_table *sampling_apps) >>> { >>> if (config_out_of_sync(&nb->options, &config_data->nb_options, >>> "mac_binding_removal_limit", false)) { >>> @@ -496,13 +508,16 @@ check_nb_options_out_of_sync(const struct >>> nbrec_nb_global *nb, >>> >>> if (config_out_of_sync(&nb->options, &config_data->nb_options, >>> "debug_drop_domain_id", false)) { >>> - init_debug_config(nb); >>> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >>> + >>> SAMPLING_APP_DROP_DEBUG)); >>> + >>> return true; >>> } >>> >>> if (config_out_of_sync(&nb->options, &config_data->nb_options, >>> "debug_drop_collector_set", false)) { >>> - init_debug_config(nb); >>> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >>> + >>> SAMPLING_APP_DROP_DEBUG)); >>> return true; >>> } >>> >>> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c >>> index 5d89670c29..95bedc5cd0 100644 >>> --- a/northd/inc-proc-northd.c >>> +++ b/northd/inc-proc-northd.c >>> @@ -181,6 +181,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, >>> global_config_sb_global_handler); >>> engine_add_input(&en_global_config, &en_sb_chassis, >>> global_config_sb_chassis_handler); >>> + engine_add_input(&en_global_config, &en_sampling_app, NULL); >>> >>> engine_add_input(&en_northd, &en_nb_mirror, NULL); >>> engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL); >>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >>> index f2f11c005c..3fb883225a 100644 >>> --- a/tests/ovn-northd.at >>> +++ b/tests/ovn-northd.at >>> @@ -12459,13 +12459,38 @@ check as northd ovn-appctl -t ovn-northd >>> inc-engine/clear-stats >>> ovn-nbctl create Sampling_App name="acl-new-traffic-sampling" id="42" >>> check_row_count nb:Sampling_App 1 >>> check_engine_stats sampling_app recompute nocompute >>> -check_engine_stats northd norecompute nocompute >>> +check_engine_stats northd recomput nocompute >>> >> >> nit: s/recomput/recompute/ >> >> >>> check_engine_stats lflow recompute nocompute >>> +check_engine_stats global_config recompute nocompute >>> CHECK_NO_CHANGE_AFTER_RECOMPUTE >>> >>> AT_CLEANUP >>> ]) >>> >>> +OVN_FOR_EACH_NORTHD_NO_HV([ >>> +AT_SETUP([Sampling_App override debug_drop_domain_id]) >>> + >>> +ovn_start >>> + >>> +check ovn-nbctl -- set NB_Global . >>> options:debug_drop_collector_set="123" >>> \ >>> + -- set NB_Global . options:debug_drop_domain_id="1" \ >>> + -- ls-add ls >>> +check ovn-nbctl --wait=sb sync >>> + >>> +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | >>> ovn_strip_lflows], [0], [dnl >>> + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == >>> "none"), >>> action=(sample(probability=65535,collector_set=123,obs_domain=1,obs_point=$cookie); >>> /* drop */) >>> +]) >>> + >>> +ovn-nbctl --wait=sb create Sampling_App name="drop-sampling" id="42" >>> +check_row_count nb:Sampling_App 1 >>> + >>> +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | >>> ovn_strip_lflows], [0], [dnl >>> + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == >>> "none"), >>> action=(sample(probability=65535,collector_set=123,obs_domain=42,obs_point=$cookie); >>> /* drop */) >>> +]) >>> + >>> +AT_CLEANUP >>> +]) >>> + >>> OVN_FOR_EACH_NORTHD_NO_HV([ >>> AT_SETUP([NAT with match]) >>> ovn_start >>> -- >>> 2.44.0 >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>> >>> >> Thanks, >> Ales >> >
On 7/23/24 15:20, Mark Michelson wrote: > On 7/23/24 15:18, Mark Michelson wrote: >> I have no additions other than Ales's, so with those fixed, >> >> Acked-by: Mark Michelson <mmichels@redhat.com> >> > > Whoops, this was meant to be my reply on patch 6, not patch 7. I'll move > the reply to patch 6, and you can ignore this reply for now. It's > possible that once I actually review patch 7 this will also be my > opinion. Please hold... > As it turns out, I still have the same opinion as I originally replied when I thought I was replying to patch 6. Acked-by: Mark Michelson <mmichels@redhat.com> (after addressing Ales's comments) >> On 7/15/24 05:22, Ales Musil wrote: >>> On Fri, Jul 12, 2024 at 5:15 PM Dumitru Ceara <dceara@redhat.com> wrote: >>> >>>> Signed-off-by: Dumitru Ceara <dceara@redhat.com> >>>> --- >>>> >>> >>> >>> Hi Dumitru, >>> >>> I have a couple of small comments down below. >>> >>> NEWS | 3 +++ >>>> northd/debug.c | 12 +++++++----- >>>> northd/debug.h | 3 ++- >>>> northd/en-global-config.c | 31 +++++++++++++++++++++++-------- >>>> northd/inc-proc-northd.c | 1 + >>>> tests/ovn-northd.at | 27 ++++++++++++++++++++++++++- >>>> 6 files changed, 62 insertions(+), 15 deletions(-) >>>> >>>> diff --git a/NEWS b/NEWS >>>> index 3e392ff08b..fcf182bc02 100644 >>>> --- a/NEWS >>>> +++ b/NEWS >>>> @@ -38,6 +38,9 @@ Post v24.03.0 >>>> ability to disable "VXLAN mode" to extend available tunnel IDs >>>> space >>>> for >>>> datapaths from 4095 to 16711680. For more details see man >>>> ovn-nb(5) >>>> for >>>> mentioned option. >>>> + - The NB_Global.debug_drop_domain_id configured value is now >>>> overridden >>>> by >>>> + the ID associated with the Sampling_App record created for drop >>>> sampling >>>> + (Sampling_App.name configured to be "drop-sampling"). >>>> >>> >>> Should we also deprecate the old global option once this drops? I >>> think it >>> would make sense. >>> >>> >>>> OVN v24.03.0 - 01 Mar 2024 >>>> -------------------------- >>>> diff --git a/northd/debug.c b/northd/debug.c >>>> index 59da5d4f66..457993b7cf 100644 >>>> --- a/northd/debug.c >>>> +++ b/northd/debug.c >>>> @@ -3,6 +3,7 @@ >>>> #include <string.h> >>>> >>>> #include "debug.h" >>>> +#include "en-sampling-app.h" >>>> >>>> #include "openvswitch/dynamic-string.h" >>>> #include "openvswitch/vlog.h" >>>> @@ -26,16 +27,17 @@ debug_enabled(void) >>>> } >>>> >>>> void >>>> -init_debug_config(const struct nbrec_nb_global *nb) >>>> +init_debug_config(const struct nbrec_nb_global *nb, >>>> + uint8_t drop_domain_id_override) >>>> { >>>> - >>>> const struct smap *options = &nb->options; >>>> uint32_t collector_set_id = smap_get_uint(options, >>>> "debug_drop_collector_set", >>>> 0); >>>> - uint32_t observation_domain_id = smap_get_uint(options, >>>> - "debug_drop_domain_id", >>>> - 0); >>>> + uint32_t observation_domain_id = >>>> + drop_domain_id_override != SAMPLING_APP_ID_NONE >>>> + ? drop_domain_id_override >>>> + : smap_get_uint(options, "debug_drop_domain_id", 0); >>>> >>>> if (collector_set_id != config.collector_set_id || >>>> observation_domain_id != config.observation_domain_id || >>>> diff --git a/northd/debug.h b/northd/debug.h >>>> index c1a5e5aadb..a0b535253a 100644 >>>> --- a/northd/debug.h >>>> +++ b/northd/debug.h >>>> @@ -21,7 +21,8 @@ >>>> #include "lib/ovn-nb-idl.h" >>>> #include "openvswitch/dynamic-string.h" >>>> >>>> -void init_debug_config(const struct nbrec_nb_global *nb); >>>> +void init_debug_config(const struct nbrec_nb_global *nb, >>>> + uint8_t drop_domain_id_override); >>>> void destroy_debug_config(void); >>>> >>>> const char *debug_drop_action(void); >>>> diff --git a/northd/en-global-config.c b/northd/en-global-config.c >>>> index 5b71ede1f2..c683c8fae5 100644 >>>> --- a/northd/en-global-config.c >>>> +++ b/northd/en-global-config.c >>>> @@ -24,6 +24,7 @@ >>>> /* OVN includes */ >>>> #include "debug.h" >>>> #include "en-global-config.h" >>>> +#include "en-sampling-app.h" >>>> #include "include/ovn/features.h" >>>> #include "ipam.h" >>>> #include "lib/ovn-nb-idl.h" >>>> @@ -42,8 +43,10 @@ static bool chassis_features_changed(const struct >>>> chassis_features *, >>>> static bool config_out_of_sync(const struct smap *config, >>>> const struct smap *saved_config, >>>> const char *key, bool >>>> must_be_present); >>>> -static bool check_nb_options_out_of_sync(const struct >>>> nbrec_nb_global *, >>>> - struct >>>> ed_type_global_config *); >>>> +static bool check_nb_options_out_of_sync( >>>> + const struct nbrec_nb_global *, >>>> + struct ed_type_global_config *, >>>> + const struct sampling_app_table *); >>>> static void update_sb_config_options_to_sbrec(struct >>>> ed_type_global_config *, >>>> const struct >>>> sbrec_sb_global *); >>>> >>>> @@ -72,6 +75,9 @@ en_global_config_run(struct engine_node *node , void >>>> *data) >>>> EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); >>>> const struct sbrec_chassis_table *sbrec_chassis_table = >>>> EN_OVSDB_GET(engine_get_input("SB_chassis", node)); >>>> + const struct ed_type_sampling_app_data *sampling_app_data = >>>> + engine_get_input_data("sampling_app", node); >>>> + const struct sampling_app_table *sampling_apps = >>>> &sampling_app_data->apps; >>>> >>>> struct ed_type_global_config *config_data = data; >>>> >>>> @@ -145,7 +151,8 @@ en_global_config_run(struct engine_node *node , >>>> void >>>> *data) >>>> build_chassis_features(sbrec_chassis_table, >>>> &config_data->features); >>>> } >>>> >>>> - init_debug_config(nb); >>>> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >>>> + SAMPLING_APP_DROP_DEBUG)); >>>> >>>> const struct sbrec_sb_global *sb = >>>> sbrec_sb_global_table_first(sb_global_table); >>>> @@ -186,6 +193,9 @@ global_config_nb_global_handler(struct engine_node >>>> *node, void *data) >>>> EN_OVSDB_GET(engine_get_input("NB_nb_global", node)); >>>> const struct sbrec_sb_global_table *sb_global_table = >>>> EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); >>>> + const struct ed_type_sampling_app_data *sampling_app_data = >>>> + engine_get_input_data("sampling_app", node); >>>> + const struct sampling_app_table *sampling_apps = >>>> &sampling_app_data->apps; >>>> >>>> const struct nbrec_nb_global *nb = >>>> nbrec_nb_global_table_first(nb_global_table); >>>> @@ -248,7 +258,7 @@ global_config_nb_global_handler(struct engine_node >>>> *node, void *data) >>>> return false; >>>> } >>>> >>>> - if (check_nb_options_out_of_sync(nb, config_data)) { >>>> + if (check_nb_options_out_of_sync(nb, config_data, >>>> sampling_apps)) { >>>> config_data->tracked_data.nb_options_changed = true; >>>> } >>>> >>>> @@ -461,8 +471,10 @@ config_out_of_sync(const struct smap *config, >>>> const >>>> struct smap *saved_config, >>>> } >>>> >>>> static bool >>>> -check_nb_options_out_of_sync(const struct nbrec_nb_global *nb, >>>> - struct ed_type_global_config >>>> *config_data) >>>> +check_nb_options_out_of_sync( >>>> + const struct nbrec_nb_global *nb, >>>> + struct ed_type_global_config *config_data, >>>> + const struct sampling_app_table *sampling_apps) >>>> { >>>> if (config_out_of_sync(&nb->options, &config_data->nb_options, >>>> "mac_binding_removal_limit", false)) { >>>> @@ -496,13 +508,16 @@ check_nb_options_out_of_sync(const struct >>>> nbrec_nb_global *nb, >>>> >>>> if (config_out_of_sync(&nb->options, &config_data->nb_options, >>>> "debug_drop_domain_id", false)) { >>>> - init_debug_config(nb); >>>> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >>>> + >>>> SAMPLING_APP_DROP_DEBUG)); >>>> + >>>> return true; >>>> } >>>> >>>> if (config_out_of_sync(&nb->options, &config_data->nb_options, >>>> "debug_drop_collector_set", false)) { >>>> - init_debug_config(nb); >>>> + init_debug_config(nb, sampling_app_get_id(sampling_apps, >>>> + >>>> SAMPLING_APP_DROP_DEBUG)); >>>> return true; >>>> } >>>> >>>> diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c >>>> index 5d89670c29..95bedc5cd0 100644 >>>> --- a/northd/inc-proc-northd.c >>>> +++ b/northd/inc-proc-northd.c >>>> @@ -181,6 +181,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop >>>> *nb, >>>> global_config_sb_global_handler); >>>> engine_add_input(&en_global_config, &en_sb_chassis, >>>> global_config_sb_chassis_handler); >>>> + engine_add_input(&en_global_config, &en_sampling_app, NULL); >>>> >>>> engine_add_input(&en_northd, &en_nb_mirror, NULL); >>>> engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL); >>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >>>> index f2f11c005c..3fb883225a 100644 >>>> --- a/tests/ovn-northd.at >>>> +++ b/tests/ovn-northd.at >>>> @@ -12459,13 +12459,38 @@ check as northd ovn-appctl -t ovn-northd >>>> inc-engine/clear-stats >>>> ovn-nbctl create Sampling_App name="acl-new-traffic-sampling" id="42" >>>> check_row_count nb:Sampling_App 1 >>>> check_engine_stats sampling_app recompute nocompute >>>> -check_engine_stats northd norecompute nocompute >>>> +check_engine_stats northd recomput nocompute >>>> >>> >>> nit: s/recomput/recompute/ >>> >>> >>>> check_engine_stats lflow recompute nocompute >>>> +check_engine_stats global_config recompute nocompute >>>> CHECK_NO_CHANGE_AFTER_RECOMPUTE >>>> >>>> AT_CLEANUP >>>> ]) >>>> >>>> +OVN_FOR_EACH_NORTHD_NO_HV([ >>>> +AT_SETUP([Sampling_App override debug_drop_domain_id]) >>>> + >>>> +ovn_start >>>> + >>>> +check ovn-nbctl -- set NB_Global . >>>> options:debug_drop_collector_set="123" >>>> \ >>>> + -- set NB_Global . options:debug_drop_domain_id="1" \ >>>> + -- ls-add ls >>>> +check ovn-nbctl --wait=sb sync >>>> + >>>> +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | >>>> ovn_strip_lflows], [0], [dnl >>>> + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == >>>> "none"), >>>> action=(sample(probability=65535,collector_set=123,obs_domain=1,obs_point=$cookie); >>>> /* drop */) >>>> +]) >>>> + >>>> +ovn-nbctl --wait=sb create Sampling_App name="drop-sampling" id="42" >>>> +check_row_count nb:Sampling_App 1 >>>> + >>>> +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | >>>> ovn_strip_lflows], [0], [dnl >>>> + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == >>>> "none"), >>>> action=(sample(probability=65535,collector_set=123,obs_domain=42,obs_point=$cookie); >>>> /* drop */) >>>> +]) >>>> + >>>> +AT_CLEANUP >>>> +]) >>>> + >>>> OVN_FOR_EACH_NORTHD_NO_HV([ >>>> AT_SETUP([NAT with match]) >>>> ovn_start >>>> -- >>>> 2.44.0 >>>> >>>> _______________________________________________ >>>> dev mailing list >>>> dev@openvswitch.org >>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >>>> >>>> >>> Thanks, >>> Ales >>> >> >
diff --git a/NEWS b/NEWS index 3e392ff08b..fcf182bc02 100644 --- a/NEWS +++ b/NEWS @@ -38,6 +38,9 @@ Post v24.03.0 ability to disable "VXLAN mode" to extend available tunnel IDs space for datapaths from 4095 to 16711680. For more details see man ovn-nb(5) for mentioned option. + - The NB_Global.debug_drop_domain_id configured value is now overridden by + the ID associated with the Sampling_App record created for drop sampling + (Sampling_App.name configured to be "drop-sampling"). OVN v24.03.0 - 01 Mar 2024 -------------------------- diff --git a/northd/debug.c b/northd/debug.c index 59da5d4f66..457993b7cf 100644 --- a/northd/debug.c +++ b/northd/debug.c @@ -3,6 +3,7 @@ #include <string.h> #include "debug.h" +#include "en-sampling-app.h" #include "openvswitch/dynamic-string.h" #include "openvswitch/vlog.h" @@ -26,16 +27,17 @@ debug_enabled(void) } void -init_debug_config(const struct nbrec_nb_global *nb) +init_debug_config(const struct nbrec_nb_global *nb, + uint8_t drop_domain_id_override) { - const struct smap *options = &nb->options; uint32_t collector_set_id = smap_get_uint(options, "debug_drop_collector_set", 0); - uint32_t observation_domain_id = smap_get_uint(options, - "debug_drop_domain_id", - 0); + uint32_t observation_domain_id = + drop_domain_id_override != SAMPLING_APP_ID_NONE + ? drop_domain_id_override + : smap_get_uint(options, "debug_drop_domain_id", 0); if (collector_set_id != config.collector_set_id || observation_domain_id != config.observation_domain_id || diff --git a/northd/debug.h b/northd/debug.h index c1a5e5aadb..a0b535253a 100644 --- a/northd/debug.h +++ b/northd/debug.h @@ -21,7 +21,8 @@ #include "lib/ovn-nb-idl.h" #include "openvswitch/dynamic-string.h" -void init_debug_config(const struct nbrec_nb_global *nb); +void init_debug_config(const struct nbrec_nb_global *nb, + uint8_t drop_domain_id_override); void destroy_debug_config(void); const char *debug_drop_action(void); diff --git a/northd/en-global-config.c b/northd/en-global-config.c index 5b71ede1f2..c683c8fae5 100644 --- a/northd/en-global-config.c +++ b/northd/en-global-config.c @@ -24,6 +24,7 @@ /* OVN includes */ #include "debug.h" #include "en-global-config.h" +#include "en-sampling-app.h" #include "include/ovn/features.h" #include "ipam.h" #include "lib/ovn-nb-idl.h" @@ -42,8 +43,10 @@ static bool chassis_features_changed(const struct chassis_features *, static bool config_out_of_sync(const struct smap *config, const struct smap *saved_config, const char *key, bool must_be_present); -static bool check_nb_options_out_of_sync(const struct nbrec_nb_global *, - struct ed_type_global_config *); +static bool check_nb_options_out_of_sync( + const struct nbrec_nb_global *, + struct ed_type_global_config *, + const struct sampling_app_table *); static void update_sb_config_options_to_sbrec(struct ed_type_global_config *, const struct sbrec_sb_global *); @@ -72,6 +75,9 @@ en_global_config_run(struct engine_node *node , void *data) EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); const struct sbrec_chassis_table *sbrec_chassis_table = EN_OVSDB_GET(engine_get_input("SB_chassis", node)); + const struct ed_type_sampling_app_data *sampling_app_data = + engine_get_input_data("sampling_app", node); + const struct sampling_app_table *sampling_apps = &sampling_app_data->apps; struct ed_type_global_config *config_data = data; @@ -145,7 +151,8 @@ en_global_config_run(struct engine_node *node , void *data) build_chassis_features(sbrec_chassis_table, &config_data->features); } - init_debug_config(nb); + init_debug_config(nb, sampling_app_get_id(sampling_apps, + SAMPLING_APP_DROP_DEBUG)); const struct sbrec_sb_global *sb = sbrec_sb_global_table_first(sb_global_table); @@ -186,6 +193,9 @@ global_config_nb_global_handler(struct engine_node *node, void *data) EN_OVSDB_GET(engine_get_input("NB_nb_global", node)); const struct sbrec_sb_global_table *sb_global_table = EN_OVSDB_GET(engine_get_input("SB_sb_global", node)); + const struct ed_type_sampling_app_data *sampling_app_data = + engine_get_input_data("sampling_app", node); + const struct sampling_app_table *sampling_apps = &sampling_app_data->apps; const struct nbrec_nb_global *nb = nbrec_nb_global_table_first(nb_global_table); @@ -248,7 +258,7 @@ global_config_nb_global_handler(struct engine_node *node, void *data) return false; } - if (check_nb_options_out_of_sync(nb, config_data)) { + if (check_nb_options_out_of_sync(nb, config_data, sampling_apps)) { config_data->tracked_data.nb_options_changed = true; } @@ -461,8 +471,10 @@ config_out_of_sync(const struct smap *config, const struct smap *saved_config, } static bool -check_nb_options_out_of_sync(const struct nbrec_nb_global *nb, - struct ed_type_global_config *config_data) +check_nb_options_out_of_sync( + const struct nbrec_nb_global *nb, + struct ed_type_global_config *config_data, + const struct sampling_app_table *sampling_apps) { if (config_out_of_sync(&nb->options, &config_data->nb_options, "mac_binding_removal_limit", false)) { @@ -496,13 +508,16 @@ check_nb_options_out_of_sync(const struct nbrec_nb_global *nb, if (config_out_of_sync(&nb->options, &config_data->nb_options, "debug_drop_domain_id", false)) { - init_debug_config(nb); + init_debug_config(nb, sampling_app_get_id(sampling_apps, + SAMPLING_APP_DROP_DEBUG)); + return true; } if (config_out_of_sync(&nb->options, &config_data->nb_options, "debug_drop_collector_set", false)) { - init_debug_config(nb); + init_debug_config(nb, sampling_app_get_id(sampling_apps, + SAMPLING_APP_DROP_DEBUG)); return true; } diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 5d89670c29..95bedc5cd0 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -181,6 +181,7 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, global_config_sb_global_handler); engine_add_input(&en_global_config, &en_sb_chassis, global_config_sb_chassis_handler); + engine_add_input(&en_global_config, &en_sampling_app, NULL); engine_add_input(&en_northd, &en_nb_mirror, NULL); engine_add_input(&en_northd, &en_nb_static_mac_binding, NULL); diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index f2f11c005c..3fb883225a 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -12459,13 +12459,38 @@ check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats ovn-nbctl create Sampling_App name="acl-new-traffic-sampling" id="42" check_row_count nb:Sampling_App 1 check_engine_stats sampling_app recompute nocompute -check_engine_stats northd norecompute nocompute +check_engine_stats northd recomput nocompute check_engine_stats lflow recompute nocompute +check_engine_stats global_config recompute nocompute CHECK_NO_CHANGE_AFTER_RECOMPUTE AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD_NO_HV([ +AT_SETUP([Sampling_App override debug_drop_domain_id]) + +ovn_start + +check ovn-nbctl -- set NB_Global . options:debug_drop_collector_set="123" \ + -- set NB_Global . options:debug_drop_domain_id="1" \ + -- ls-add ls +check ovn-nbctl --wait=sb sync + +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | ovn_strip_lflows], [0], [dnl + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == "none"), action=(sample(probability=65535,collector_set=123,obs_domain=1,obs_point=$cookie); /* drop */) +]) + +ovn-nbctl --wait=sb create Sampling_App name="drop-sampling" id="42" +check_row_count nb:Sampling_App 1 + +AT_CHECK([ovn-sbctl lflow-list | grep ls_in_l2_unknown.*sample | ovn_strip_lflows], [0], [dnl + table=??(ls_in_l2_unknown ), priority=50 , match=(outport == "none"), action=(sample(probability=65535,collector_set=123,obs_domain=42,obs_point=$cookie); /* drop */) +]) + +AT_CLEANUP +]) + OVN_FOR_EACH_NORTHD_NO_HV([ AT_SETUP([NAT with match]) ovn_start
Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- NEWS | 3 +++ northd/debug.c | 12 +++++++----- northd/debug.h | 3 ++- northd/en-global-config.c | 31 +++++++++++++++++++++++-------- northd/inc-proc-northd.c | 1 + tests/ovn-northd.at | 27 ++++++++++++++++++++++++++- 6 files changed, 62 insertions(+), 15 deletions(-)