On 10/18/22 17:59, Adrian Moreno wrote: > Introduce a new table called Sample where per-flow IPFIX configuration > can be specified. > Also, reference rows from such table from the ACL table to enable the > configuration of ACL sampling. If enabled, northd will add a sample > action to each ACL-related logical flow. > > Signed-off-by: Adrian Moreno <[email protected]> > --- > northd/northd.c | 31 ++++++++++++++++++++++++++++++- > ovn-nb.ovsschema | 23 ++++++++++++++++++++++- > ovn-nb.xml | 31 +++++++++++++++++++++++++++++++ > 3 files changed, 83 insertions(+), 2 deletions(-) > > diff --git a/northd/northd.c b/northd/northd.c > index 61d474840..3e09e3a0f 100644 > --- a/northd/northd.c > +++ b/northd/northd.c > @@ -6194,6 +6194,27 @@ build_acl_log(struct ds *actions, const struct > nbrec_acl *acl, > ds_put_cstr(actions, "); "); > } > > +static void > +build_acl_sample(struct ds *actions, const struct nbrec_acl *acl) > +{ > + if (!acl->sample) { > + return; > + } > + ds_put_format(actions, "sample(probability=%"PRId16"," > + "collector_set=%d," > + "obs_domain=%hd,", > + (uint16_t) acl->sample->probability, > + (uint32_t) acl->sample->collector_set_id, > + (uint8_t) acl->sample->obs_domain_id); > + > + if (acl->sample->obs_point_id) { > + ds_put_format(actions, "obs_point=%"PRId32");", > + (uint32_t) *acl->sample->obs_point_id); > + } else { > + ds_put_cstr(actions, "obs_point=$cookie);"); > + } > +} > + > static void > build_reject_acl_rules(struct ovn_datapath *od, struct hmap *lflows, > enum ovn_stage stage, struct nbrec_acl *acl, > @@ -6260,6 +6281,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > if (!strcmp(acl->action, "allow-stateless")) { > ds_clear(actions); > build_acl_log(actions, acl, meter_groups); > + build_acl_sample(actions, acl); > ds_put_cstr(actions, "next;"); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > @@ -6275,6 +6297,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > if (!has_stateful) { > ds_clear(actions); > build_acl_log(actions, acl, meter_groups); > + build_acl_sample(actions, acl); > ds_put_cstr(actions, "next;"); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > @@ -6304,6 +6327,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > REG_LABEL" = %"PRId64"; ", acl->label); > } > build_acl_log(actions, acl, meter_groups); > + build_acl_sample(actions, acl); > ds_put_cstr(actions, "next;"); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > @@ -6329,6 +6353,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > REG_LABEL" = %"PRId64"; ", acl->label); > } > build_acl_log(actions, acl, meter_groups); > + build_acl_sample(actions, acl); > ds_put_cstr(actions, "next;"); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > @@ -6349,7 +6374,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > */ > bool log_related = smap_get_bool(&acl->options, "log-related", > false); > - if (acl->log && acl->label && log_related) { > + if ((acl->log || acl->sample) && acl->label && log_related) { > /* Related/reply flows need to be set on the opposite > pipeline > * from where the ACL itself is set. > */ > @@ -6365,6 +6390,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > use_ct_inv_match ? " && !ct.inv" : "", > ct_blocked_match, acl->label); > build_acl_log(actions, acl, meter_groups); > + build_acl_sample(actions, acl); > ds_put_cstr(actions, "next;"); > ovn_lflow_add_with_hint(lflows, od, log_related_stage, > UINT16_MAX - 2, > @@ -6402,6 +6428,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > } else { > ds_put_format(match, " && (%s)", acl->match); > build_acl_log(actions, acl, meter_groups); > + build_acl_sample(actions, acl); > ds_put_cstr(actions, debug_implicit_drop_action()); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > @@ -6430,6 +6457,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > } else { > ds_put_format(match, " && (%s)", acl->match); > build_acl_log(actions, acl, meter_groups); > + build_acl_sample(actions, acl); > ds_put_cstr(actions, debug_implicit_drop_action()); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > @@ -6447,6 +6475,7 @@ consider_acl(struct hmap *lflows, struct ovn_datapath > *od, > actions, &acl->header_, meter_groups); > } else { > build_acl_log(actions, acl, meter_groups); > + build_acl_sample(actions, acl); > ds_put_cstr(actions, debug_implicit_drop_action()); > ovn_lflow_add_with_hint(lflows, od, stage, > acl->priority + OVN_ACL_PRI_OFFSET, > diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema > index 174364c8b..6178b532e 100644 > --- a/ovn-nb.ovsschema > +++ b/ovn-nb.ovsschema > @@ -1,7 +1,7 @@ > { > "name": "OVN_Northbound", > "version": "6.3.0", > - "cksum": "4042813038 31869", > + "cksum": "3795038812 33116", > "tables": { > "NB_Global": { > "columns": { > @@ -30,6 +30,23 @@ > "ipsec": {"type": "boolean"}}, > "maxRows": 1, > "isRoot": true}, > + "Sample": { > + "columns": { > + "probability": {"type": {"key": {"type": "integer", > + "minInteger": 0, > + "maxInteger": 65535}}}, > + "collector_set_id": {"type": {"key": {"type": "integer", > + "minInteger": 0, > + "maxInteger": > 4294967295}}}, > + "obs_domain_id": {"type": {"key": {"type": "integer", > + "minInteger": 0, > + "maxInteger": 255}}}, > + "obs_point_id": {"type": {"key": {"type": "integer", > + "minInteger": 0, > + "maxInteger": 4294967295}, > + "min": 0, "max":1}} > + }
This seems to match 1:1 the arguments of the OVS SAMPLE action. That's fine but do we expect more arguments to be added in the future? Please add a column that can be used as index and also define a unique index. That will save us from trouble like "spurious duplicates" [0] when the DB is in RAFT. It also makes it easier for the CMS to track what it installed or not. An external_ids column might also make sense. [0] https://github.com/openvswitch/ovs/commit/04e5adfedd2a2e4ceac62136671057542a7cf875 > + }, Nit: By default "isRoot" is false. Don't we want this as root table? > "Copp": { > "columns": { > "name": {"type": "string"}, > @@ -267,6 +284,10 @@ > "label": {"type": {"key": {"type": "integer", > "minInteger": 0, > "maxInteger": 4294967295}}}, > + "sample": {"type": {"key": {"type": "uuid", > + "refTable": "Sample", > + "refType": "strong"}, > + "min": 0, "max": 1}}, > "options": { > "type": {"key": "string", > "value": "string", > diff --git a/ovn-nb.xml b/ovn-nb.xml > index 9581aef7c..e818fd8d1 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -387,6 +387,31 @@ > > </table> > > + <table name="Sample" title="Sample"> > + <p> > + This table describes an IPFIX Sampling Point. Entries in other tables > + might be associated with Sample entries to indicate how the sample > + should be generated. > + > + For an example, see <ref table="ACL"/>. > + </p> > + <column name="probability"> > + Sampling probability. It must be an integer number between 0 and 65535. > + </column> > + <column name="collector_set_id"> > + The 32-bit integer identifier of the set of of collectors to send > + packets to. See Flow_Sample_Collector_Set Table in ovs-vswitchd's > + database schema. > + </column> > + <column name="obs_domain_id"> > + The 8 most significant bits of the Observation Domain ID that will be > + added to evvery IPFIX sample. The 24 LSB will be the datapath key. > + </column> > + <column name="obs_point_id"> > + If set, it'll be use as Observation Point ID in every IPFIX sample. > + Otherwise the Logical Flow's coockie will be used. > + </column> > + </table> > <table name="Copp" title="Control plane protection"> > <p> > This table is used to define control plane protection policies, i.e., > @@ -2191,6 +2216,12 @@ > </column> > </group> > > + <column name="sample"> > + <p> > + The entry in the <ref table="Sample"/> table to use for sampling. > + </p> > + </column> > + > <group title="Common Columns"> > <column name="options"> > This column provides general key/value settings. The supported _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
