| Message ID | 20220613161054.2896553-2-amorenoz@redhat.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add ovn drop debugging | 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 | success | github build: passed |
Hi Adrian, just a small comment below. On 6/13/22 12:10, Adrian Moreno wrote: > sample ovn action encodes into the OFPACT_SAMPLE ovs action. > > OVN action allows the following parameters: > > - obs_domain_id: 8-bit integer that identifies the sampling application. > This value will be combined with the datapath's tunnel_id to form the > final observation_domain_id that will be used in the OVS action. > > - obs_point_id: a 32-bit integer or the $cookie macro that will be > expanded into the first 32 bits of the lflow's UUID. > > - probability: a 16-bit integer that specifies the sampling probability. > Specifying 0 has no effect and 65535 means sampling all packets. > > Signed-off-by: Adrian Moreno <amorenoz@redhat.com> > --- > controller/lflow.c | 1 + > include/ovn/actions.h | 16 ++++++ > lib/actions.c | 119 ++++++++++++++++++++++++++++++++++++++++++ > tests/ovn.at | 9 ++++ > tests/test-ovn.c | 3 ++ > utilities/ovn-trace.c | 2 + > 6 files changed, 150 insertions(+) > > diff --git a/controller/lflow.c b/controller/lflow.c > index 934b23698..54312d2df 100644 > --- a/controller/lflow.c > +++ b/controller/lflow.c > @@ -1163,6 +1163,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, > .group_table = l_ctx_out->group_table, > .meter_table = l_ctx_out->meter_table, > .lflow_uuid = lflow->header_.uuid, > + .tunnel_key = ldp->datapath->tunnel_key, > > .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, > .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE, > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > index 1ae496960..915f9124e 100644 > --- a/include/ovn/actions.h > +++ b/include/ovn/actions.h > @@ -118,6 +118,7 @@ struct ovn_extend_table; > OVNACT(LOOKUP_FDB, ovnact_lookup_fdb) \ > OVNACT(CHECK_IN_PORT_SEC, ovnact_result) \ > OVNACT(CHECK_OUT_PORT_SEC, ovnact_result) \ > + OVNACT(SAMPLE, ovnact_sample) \ > > /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */ > enum OVS_PACKED_ENUM ovnact_type { > @@ -453,6 +454,18 @@ struct ovnact_lookup_fdb { > struct expr_field dst; /* 1-bit destination field. */ > }; > > +/* OVNACT_SAMPLE */ > +struct ovnact_sample { > + struct ovnact ovnact; > + uint16_t probability; /* probability over UINT16_MAX. */ > + uint8_t obs_domain_id; /* most significant byte of the > + observation domain id. The other 24 bits > + will come from the datapath's tunnel key. */ > + uint32_t collector_set_id; /* colector_set_id. */ > + uint32_t obs_point_id; /* observation point id. */ > + bool use_cookie; /* use cookie as obs_point_id */ > +}; > + > /* Internal use by the helpers below. */ > void ovnact_init(struct ovnact *, enum ovnact_type, size_t len); > void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len); > @@ -772,6 +785,9 @@ struct ovnact_encode_params { > /* The logical flow uuid that drove this action. */ > struct uuid lflow_uuid; > > + /* The tunnel key of the datapath. */ > + uint32_t tunnel_key; > + > /* OVN maps each logical flow table (ltable), one-to-one, onto a physical > * OpenFlow flow table (ptable). A number of parameters describe this > * mapping and data related to flow tables: > diff --git a/lib/actions.c b/lib/actions.c > index aab044306..6d1622c66 100644 > --- a/lib/actions.c > +++ b/lib/actions.c > @@ -4278,6 +4278,123 @@ encode_CHECK_OUT_PORT_SEC(const struct ovnact_result *dl, > MLF_CHECK_PORT_SEC_BIT, ofpacts); > } > > +static void > +format_SAMPLE(const struct ovnact_sample *sample, struct ds *s) > +{ > + ds_put_format(s, "sample(probability=%"PRId16, sample->probability); > + > + ds_put_format(s, ",collector_set=%"PRId32, sample->collector_set_id); > + ds_put_format(s, ",obs_domain=%"PRId8, sample->obs_domain_id); > + if (sample->use_cookie) { > + ds_put_cstr(s, ",obs_point=$cookie"); > + } else { > + ds_put_format(s, ",obs_point=%"PRId32, sample->obs_point_id); > + } > + ds_put_format(s, ");"); > +} > + > +static void > +encode_SAMPLE(const struct ovnact_sample *sample, > + const struct ovnact_encode_params *ep, > + struct ofpbuf *ofpacts) > +{ > + struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts); > + os->probability = sample->probability; > + os->collector_set_id = sample->collector_set_id; > + os->obs_domain_id = > + (sample->obs_domain_id << 24) | (ep->tunnel_key & 0xFFFFFF); > + > + if (sample->use_cookie) { > + os->obs_point_id = ep->lflow_uuid.parts[0]; > + } else { > + os->obs_point_id = sample->obs_point_id; > + } > + os->sampling_port = OFPP_NONE; > +} > + > +static void > +parse_sample_arg(struct action_context *ctx, struct ovnact_sample *sample) > +{ > + if (lexer_match_id(ctx->lexer, "probability")) { > + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { > + return; > + } > + if (ctx->lexer->token.type == LEX_T_INTEGER > + && ctx->lexer->token.format == LEX_F_DECIMAL) { > + if (!action_parse_uint16(ctx, &sample->probability, > + "probability")) { > + return; > + } > + } > + } else if (lexer_match_id(ctx->lexer, "obs_point")) { > + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { > + return; > + } > + if (ctx->lexer->token.type == LEX_T_MACRO && > + !strcmp(ctx->lexer->token.s, "cookie")) { > + sample->use_cookie = true; > + lexer_get(ctx->lexer); > + } else if (ctx->lexer->token.type == LEX_T_INTEGER > + && ctx->lexer->token.format == LEX_F_DECIMAL) { > + sample->obs_point_id = ntohll(ctx->lexer->token.value.integer); > + lexer_get(ctx->lexer); > + } else { > + lexer_syntax_error(ctx->lexer, > + "Malformed sample observation_point_id"); > + } > + } else if (lexer_match_id(ctx->lexer, "obs_domain")) { > + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { > + return; > + } > + if (ctx->lexer->token.type == LEX_T_INTEGER > + && ctx->lexer->token.format == LEX_F_DECIMAL) { > + uint32_t obs_domain = ntohll(ctx->lexer->token.value.integer); > + if (obs_domain > UINT8_MAX) { > + lexer_syntax_error(ctx->lexer, > + "Malformed sample action: obs_domain must be 8-bit long"); > + return; > + } > + sample->obs_domain_id = obs_domain; > + } > + lexer_get(ctx->lexer); > + } else if (lexer_match_id(ctx->lexer, "collector_set")) { > + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { > + return; > + } > + if (ctx->lexer->token.type == LEX_T_INTEGER > + && ctx->lexer->token.format == LEX_F_DECIMAL) { > + sample->collector_set_id = ntohll(ctx->lexer->token.value.integer); > + } > + lexer_get(ctx->lexer); > + } else { > + lexer_syntax_error(ctx->lexer, "Malformed sample action"); > + } > +} > +static void > +parse_sample(struct action_context *ctx) > +{ > + struct ovnact_sample * sample = ovnact_put_SAMPLE(ctx->ovnacts); > + > + if (lexer_match(ctx->lexer, LEX_T_LPAREN)) { > + while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) { > + parse_sample_arg(ctx, sample); > + if (ctx->lexer->error) { > + return; > + } > + lexer_match(ctx->lexer, LEX_T_COMMA); > + } > + } > + if (!sample->probability) { > + lexer_error(ctx->lexer, "probability must be greater than zero"); > + return; > + } > +} > + > +static void > +ovnact_sample_free(struct ovnact_sample *sample OVS_UNUSED) > +{ > +} > + > /* Parses an assignment or exchange or put_dhcp_opts action. */ > static void > parse_set_action(struct action_context *ctx) > @@ -4458,6 +4575,8 @@ parse_action(struct action_context *ctx) > ovnact_put_CT_SNAT_TO_VIP(ctx->ovnacts); > } else if (lexer_match_id(ctx->lexer, "put_fdb")) { > parse_put_fdb(ctx, ovnact_put_PUT_FDB(ctx->ovnacts)); > + } else if (lexer_match_id(ctx->lexer, "sample")) { > + parse_sample(ctx); > } else { > lexer_syntax_error(ctx->lexer, "expecting action"); > } > diff --git a/tests/ovn.at b/tests/ovn.at > index 59d51f3e0..7afe0040b 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -2080,6 +2080,15 @@ pop(eth.type); > push(abc); > Syntax error at `abc' expecting field name. > > +# sample > +sample(probability=100,collector_set=200,obs_domain=0,obs_point=1000); > + encodes as sample(probability=100,collector_set_id=200,obs_domain_id=11259375,obs_point_id=1000) > + > +# sample with obs_domain = 10. Final obs_domain is 0xA << 24 | 0xABCDEF. > +sample(probability=100,collector_set=200,obs_domain=10,obs_point=$cookie); > + encodes as sample(probability=100,collector_set_id=200,obs_domain_id=179031535,obs_point_id=2863311530) > + It's a good idea to add cases where arguments are missing/malformed to ensure that those cases act as expected. > + > # Miscellaneous negative tests. > ; > Syntax error at `;'. > diff --git a/tests/test-ovn.c b/tests/test-ovn.c > index a241f150d..ea319f81e 100644 > --- a/tests/test-ovn.c > +++ b/tests/test-ovn.c > @@ -1355,6 +1355,9 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) > .common_nat_ct_zone = MFF_LOG_DNAT_ZONE, > .in_port_sec_ptable = OFTABLE_CHK_IN_PORT_SEC, > .out_port_sec_ptable = OFTABLE_CHK_OUT_PORT_SEC, > + .lflow_uuid.parts = > + { 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0xdddddddd}, > + .tunnel_key = 0xabcdef, > }; > struct ofpbuf ofpacts; > ofpbuf_init(&ofpacts, 0); > diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c > index c4110de0a..d6ccc145f 100644 > --- a/utilities/ovn-trace.c > +++ b/utilities/ovn-trace.c > @@ -3224,6 +3224,8 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, > execute_check_out_port_sec(ovnact_get_CHECK_OUT_PORT_SEC(a), > dp, uflow); > break; > + case OVNACT_SAMPLE: > + break; > } > } > ofpbuf_uninit(&stack); >
On 6/16/22 22:31, Mark Michelson wrote: > Hi Adrian, just a small comment below. > > On 6/13/22 12:10, Adrian Moreno wrote: >> sample ovn action encodes into the OFPACT_SAMPLE ovs action. >> >> OVN action allows the following parameters: >> >> - obs_domain_id: 8-bit integer that identifies the sampling application. >> This value will be combined with the datapath's tunnel_id to form the >> final observation_domain_id that will be used in the OVS action. >> >> - obs_point_id: a 32-bit integer or the $cookie macro that will be >> expanded into the first 32 bits of the lflow's UUID. >> >> - probability: a 16-bit integer that specifies the sampling probability. >> Specifying 0 has no effect and 65535 means sampling all packets. >> >> Signed-off-by: Adrian Moreno <amorenoz@redhat.com> >> --- >> controller/lflow.c | 1 + >> include/ovn/actions.h | 16 ++++++ >> lib/actions.c | 119 ++++++++++++++++++++++++++++++++++++++++++ >> tests/ovn.at | 9 ++++ >> tests/test-ovn.c | 3 ++ >> utilities/ovn-trace.c | 2 + >> 6 files changed, 150 insertions(+) >> >> diff --git a/controller/lflow.c b/controller/lflow.c >> index 934b23698..54312d2df 100644 >> --- a/controller/lflow.c >> +++ b/controller/lflow.c >> @@ -1163,6 +1163,7 @@ add_matches_to_flow_table(const struct >> sbrec_logical_flow *lflow, >> .group_table = l_ctx_out->group_table, >> .meter_table = l_ctx_out->meter_table, >> .lflow_uuid = lflow->header_.uuid, >> + .tunnel_key = ldp->datapath->tunnel_key, >> .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, >> .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE, >> diff --git a/include/ovn/actions.h b/include/ovn/actions.h >> index 1ae496960..915f9124e 100644 >> --- a/include/ovn/actions.h >> +++ b/include/ovn/actions.h >> @@ -118,6 +118,7 @@ struct ovn_extend_table; >> OVNACT(LOOKUP_FDB, ovnact_lookup_fdb) \ >> OVNACT(CHECK_IN_PORT_SEC, ovnact_result) \ >> OVNACT(CHECK_OUT_PORT_SEC, ovnact_result) \ >> + OVNACT(SAMPLE, ovnact_sample) \ >> /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */ >> enum OVS_PACKED_ENUM ovnact_type { >> @@ -453,6 +454,18 @@ struct ovnact_lookup_fdb { >> struct expr_field dst; /* 1-bit destination field. */ >> }; >> +/* OVNACT_SAMPLE */ >> +struct ovnact_sample { >> + struct ovnact ovnact; >> + uint16_t probability; /* probability over UINT16_MAX. */ >> + uint8_t obs_domain_id; /* most significant byte of the >> + observation domain id. The other 24 bits >> + will come from the datapath's tunnel key. */ >> + uint32_t collector_set_id; /* colector_set_id. */ >> + uint32_t obs_point_id; /* observation point id. */ >> + bool use_cookie; /* use cookie as obs_point_id */ >> +}; >> + >> /* Internal use by the helpers below. */ >> void ovnact_init(struct ovnact *, enum ovnact_type, size_t len); >> void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len); >> @@ -772,6 +785,9 @@ struct ovnact_encode_params { >> /* The logical flow uuid that drove this action. */ >> struct uuid lflow_uuid; >> + /* The tunnel key of the datapath. */ >> + uint32_t tunnel_key; >> + >> /* OVN maps each logical flow table (ltable), one-to-one, onto a physical >> * OpenFlow flow table (ptable). A number of parameters describe this >> * mapping and data related to flow tables: >> diff --git a/lib/actions.c b/lib/actions.c >> index aab044306..6d1622c66 100644 >> --- a/lib/actions.c >> +++ b/lib/actions.c >> @@ -4278,6 +4278,123 @@ encode_CHECK_OUT_PORT_SEC(const struct ovnact_result *dl, >> MLF_CHECK_PORT_SEC_BIT, ofpacts); >> } >> +static void >> +format_SAMPLE(const struct ovnact_sample *sample, struct ds *s) >> +{ >> + ds_put_format(s, "sample(probability=%"PRId16, sample->probability); >> + >> + ds_put_format(s, ",collector_set=%"PRId32, sample->collector_set_id); >> + ds_put_format(s, ",obs_domain=%"PRId8, sample->obs_domain_id); >> + if (sample->use_cookie) { >> + ds_put_cstr(s, ",obs_point=$cookie"); >> + } else { >> + ds_put_format(s, ",obs_point=%"PRId32, sample->obs_point_id); >> + } >> + ds_put_format(s, ");"); >> +} >> + >> +static void >> +encode_SAMPLE(const struct ovnact_sample *sample, >> + const struct ovnact_encode_params *ep, >> + struct ofpbuf *ofpacts) >> +{ >> + struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts); >> + os->probability = sample->probability; >> + os->collector_set_id = sample->collector_set_id; >> + os->obs_domain_id = >> + (sample->obs_domain_id << 24) | (ep->tunnel_key & 0xFFFFFF); >> + >> + if (sample->use_cookie) { >> + os->obs_point_id = ep->lflow_uuid.parts[0]; >> + } else { >> + os->obs_point_id = sample->obs_point_id; >> + } >> + os->sampling_port = OFPP_NONE; >> +} >> + >> +static void >> +parse_sample_arg(struct action_context *ctx, struct ovnact_sample *sample) >> +{ >> + if (lexer_match_id(ctx->lexer, "probability")) { >> + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { >> + return; >> + } >> + if (ctx->lexer->token.type == LEX_T_INTEGER >> + && ctx->lexer->token.format == LEX_F_DECIMAL) { >> + if (!action_parse_uint16(ctx, &sample->probability, >> + "probability")) { >> + return; >> + } >> + } >> + } else if (lexer_match_id(ctx->lexer, "obs_point")) { >> + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { >> + return; >> + } >> + if (ctx->lexer->token.type == LEX_T_MACRO && >> + !strcmp(ctx->lexer->token.s, "cookie")) { >> + sample->use_cookie = true; >> + lexer_get(ctx->lexer); >> + } else if (ctx->lexer->token.type == LEX_T_INTEGER >> + && ctx->lexer->token.format == LEX_F_DECIMAL) { >> + sample->obs_point_id = ntohll(ctx->lexer->token.value.integer); >> + lexer_get(ctx->lexer); >> + } else { >> + lexer_syntax_error(ctx->lexer, >> + "Malformed sample observation_point_id"); >> + } >> + } else if (lexer_match_id(ctx->lexer, "obs_domain")) { >> + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { >> + return; >> + } >> + if (ctx->lexer->token.type == LEX_T_INTEGER >> + && ctx->lexer->token.format == LEX_F_DECIMAL) { >> + uint32_t obs_domain = ntohll(ctx->lexer->token.value.integer); >> + if (obs_domain > UINT8_MAX) { >> + lexer_syntax_error(ctx->lexer, >> + "Malformed sample action: obs_domain must be 8-bit long"); >> + return; >> + } >> + sample->obs_domain_id = obs_domain; >> + } >> + lexer_get(ctx->lexer); >> + } else if (lexer_match_id(ctx->lexer, "collector_set")) { >> + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { >> + return; >> + } >> + if (ctx->lexer->token.type == LEX_T_INTEGER >> + && ctx->lexer->token.format == LEX_F_DECIMAL) { >> + sample->collector_set_id = ntohll(ctx->lexer->token.value.integer); >> + } >> + lexer_get(ctx->lexer); >> + } else { >> + lexer_syntax_error(ctx->lexer, "Malformed sample action"); >> + } >> +} >> +static void >> +parse_sample(struct action_context *ctx) >> +{ >> + struct ovnact_sample * sample = ovnact_put_SAMPLE(ctx->ovnacts); >> + >> + if (lexer_match(ctx->lexer, LEX_T_LPAREN)) { >> + while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) { >> + parse_sample_arg(ctx, sample); >> + if (ctx->lexer->error) { >> + return; >> + } >> + lexer_match(ctx->lexer, LEX_T_COMMA); >> + } >> + } >> + if (!sample->probability) { >> + lexer_error(ctx->lexer, "probability must be greater than zero"); >> + return; >> + } >> +} >> + >> +static void >> +ovnact_sample_free(struct ovnact_sample *sample OVS_UNUSED) >> +{ >> +} >> + >> /* Parses an assignment or exchange or put_dhcp_opts action. */ >> static void >> parse_set_action(struct action_context *ctx) >> @@ -4458,6 +4575,8 @@ parse_action(struct action_context *ctx) >> ovnact_put_CT_SNAT_TO_VIP(ctx->ovnacts); >> } else if (lexer_match_id(ctx->lexer, "put_fdb")) { >> parse_put_fdb(ctx, ovnact_put_PUT_FDB(ctx->ovnacts)); >> + } else if (lexer_match_id(ctx->lexer, "sample")) { >> + parse_sample(ctx); >> } else { >> lexer_syntax_error(ctx->lexer, "expecting action"); >> } >> diff --git a/tests/ovn.at b/tests/ovn.at >> index 59d51f3e0..7afe0040b 100644 >> --- a/tests/ovn.at >> +++ b/tests/ovn.at >> @@ -2080,6 +2080,15 @@ pop(eth.type); >> push(abc); >> Syntax error at `abc' expecting field name. >> +# sample >> +sample(probability=100,collector_set=200,obs_domain=0,obs_point=1000); >> + encodes as >> sample(probability=100,collector_set_id=200,obs_domain_id=11259375,obs_point_id=1000) >> >> + >> +# sample with obs_domain = 10. Final obs_domain is 0xA << 24 | 0xABCDEF. >> +sample(probability=100,collector_set=200,obs_domain=10,obs_point=$cookie); >> + encodes as >> sample(probability=100,collector_set_id=200,obs_domain_id=179031535,obs_point_id=2863311530) >> >> + > > It's a good idea to add cases where arguments are missing/malformed to ensure > that those cases act as expected. > You're absolutely right. I'll send another version with more tests. >> + >> # Miscellaneous negative tests. >> ; >> Syntax error at `;'. >> diff --git a/tests/test-ovn.c b/tests/test-ovn.c >> index a241f150d..ea319f81e 100644 >> --- a/tests/test-ovn.c >> +++ b/tests/test-ovn.c >> @@ -1355,6 +1355,9 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) >> .common_nat_ct_zone = MFF_LOG_DNAT_ZONE, >> .in_port_sec_ptable = OFTABLE_CHK_IN_PORT_SEC, >> .out_port_sec_ptable = OFTABLE_CHK_OUT_PORT_SEC, >> + .lflow_uuid.parts = >> + { 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0xdddddddd}, >> + .tunnel_key = 0xabcdef, >> }; >> struct ofpbuf ofpacts; >> ofpbuf_init(&ofpacts, 0); >> diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c >> index c4110de0a..d6ccc145f 100644 >> --- a/utilities/ovn-trace.c >> +++ b/utilities/ovn-trace.c >> @@ -3224,6 +3224,8 @@ trace_actions(const struct ovnact *ovnacts, size_t >> ovnacts_len, >> execute_check_out_port_sec(ovnact_get_CHECK_OUT_PORT_SEC(a), >> dp, uflow); >> break; >> + case OVNACT_SAMPLE: >> + break; >> } >> } >> ofpbuf_uninit(&stack); >> >
diff --git a/controller/lflow.c b/controller/lflow.c index 934b23698..54312d2df 100644 --- a/controller/lflow.c +++ b/controller/lflow.c @@ -1163,6 +1163,7 @@ add_matches_to_flow_table(const struct sbrec_logical_flow *lflow, .group_table = l_ctx_out->group_table, .meter_table = l_ctx_out->meter_table, .lflow_uuid = lflow->header_.uuid, + .tunnel_key = ldp->datapath->tunnel_key, .pipeline = ingress ? OVNACT_P_INGRESS : OVNACT_P_EGRESS, .ingress_ptable = OFTABLE_LOG_INGRESS_PIPELINE, diff --git a/include/ovn/actions.h b/include/ovn/actions.h index 1ae496960..915f9124e 100644 --- a/include/ovn/actions.h +++ b/include/ovn/actions.h @@ -118,6 +118,7 @@ struct ovn_extend_table; OVNACT(LOOKUP_FDB, ovnact_lookup_fdb) \ OVNACT(CHECK_IN_PORT_SEC, ovnact_result) \ OVNACT(CHECK_OUT_PORT_SEC, ovnact_result) \ + OVNACT(SAMPLE, ovnact_sample) \ /* enum ovnact_type, with a member OVNACT_<ENUM> for each action. */ enum OVS_PACKED_ENUM ovnact_type { @@ -453,6 +454,18 @@ struct ovnact_lookup_fdb { struct expr_field dst; /* 1-bit destination field. */ }; +/* OVNACT_SAMPLE */ +struct ovnact_sample { + struct ovnact ovnact; + uint16_t probability; /* probability over UINT16_MAX. */ + uint8_t obs_domain_id; /* most significant byte of the + observation domain id. The other 24 bits + will come from the datapath's tunnel key. */ + uint32_t collector_set_id; /* colector_set_id. */ + uint32_t obs_point_id; /* observation point id. */ + bool use_cookie; /* use cookie as obs_point_id */ +}; + /* Internal use by the helpers below. */ void ovnact_init(struct ovnact *, enum ovnact_type, size_t len); void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t len); @@ -772,6 +785,9 @@ struct ovnact_encode_params { /* The logical flow uuid that drove this action. */ struct uuid lflow_uuid; + /* The tunnel key of the datapath. */ + uint32_t tunnel_key; + /* OVN maps each logical flow table (ltable), one-to-one, onto a physical * OpenFlow flow table (ptable). A number of parameters describe this * mapping and data related to flow tables: diff --git a/lib/actions.c b/lib/actions.c index aab044306..6d1622c66 100644 --- a/lib/actions.c +++ b/lib/actions.c @@ -4278,6 +4278,123 @@ encode_CHECK_OUT_PORT_SEC(const struct ovnact_result *dl, MLF_CHECK_PORT_SEC_BIT, ofpacts); } +static void +format_SAMPLE(const struct ovnact_sample *sample, struct ds *s) +{ + ds_put_format(s, "sample(probability=%"PRId16, sample->probability); + + ds_put_format(s, ",collector_set=%"PRId32, sample->collector_set_id); + ds_put_format(s, ",obs_domain=%"PRId8, sample->obs_domain_id); + if (sample->use_cookie) { + ds_put_cstr(s, ",obs_point=$cookie"); + } else { + ds_put_format(s, ",obs_point=%"PRId32, sample->obs_point_id); + } + ds_put_format(s, ");"); +} + +static void +encode_SAMPLE(const struct ovnact_sample *sample, + const struct ovnact_encode_params *ep, + struct ofpbuf *ofpacts) +{ + struct ofpact_sample *os = ofpact_put_SAMPLE(ofpacts); + os->probability = sample->probability; + os->collector_set_id = sample->collector_set_id; + os->obs_domain_id = + (sample->obs_domain_id << 24) | (ep->tunnel_key & 0xFFFFFF); + + if (sample->use_cookie) { + os->obs_point_id = ep->lflow_uuid.parts[0]; + } else { + os->obs_point_id = sample->obs_point_id; + } + os->sampling_port = OFPP_NONE; +} + +static void +parse_sample_arg(struct action_context *ctx, struct ovnact_sample *sample) +{ + if (lexer_match_id(ctx->lexer, "probability")) { + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { + return; + } + if (ctx->lexer->token.type == LEX_T_INTEGER + && ctx->lexer->token.format == LEX_F_DECIMAL) { + if (!action_parse_uint16(ctx, &sample->probability, + "probability")) { + return; + } + } + } else if (lexer_match_id(ctx->lexer, "obs_point")) { + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { + return; + } + if (ctx->lexer->token.type == LEX_T_MACRO && + !strcmp(ctx->lexer->token.s, "cookie")) { + sample->use_cookie = true; + lexer_get(ctx->lexer); + } else if (ctx->lexer->token.type == LEX_T_INTEGER + && ctx->lexer->token.format == LEX_F_DECIMAL) { + sample->obs_point_id = ntohll(ctx->lexer->token.value.integer); + lexer_get(ctx->lexer); + } else { + lexer_syntax_error(ctx->lexer, + "Malformed sample observation_point_id"); + } + } else if (lexer_match_id(ctx->lexer, "obs_domain")) { + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { + return; + } + if (ctx->lexer->token.type == LEX_T_INTEGER + && ctx->lexer->token.format == LEX_F_DECIMAL) { + uint32_t obs_domain = ntohll(ctx->lexer->token.value.integer); + if (obs_domain > UINT8_MAX) { + lexer_syntax_error(ctx->lexer, + "Malformed sample action: obs_domain must be 8-bit long"); + return; + } + sample->obs_domain_id = obs_domain; + } + lexer_get(ctx->lexer); + } else if (lexer_match_id(ctx->lexer, "collector_set")) { + if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) { + return; + } + if (ctx->lexer->token.type == LEX_T_INTEGER + && ctx->lexer->token.format == LEX_F_DECIMAL) { + sample->collector_set_id = ntohll(ctx->lexer->token.value.integer); + } + lexer_get(ctx->lexer); + } else { + lexer_syntax_error(ctx->lexer, "Malformed sample action"); + } +} +static void +parse_sample(struct action_context *ctx) +{ + struct ovnact_sample * sample = ovnact_put_SAMPLE(ctx->ovnacts); + + if (lexer_match(ctx->lexer, LEX_T_LPAREN)) { + while (!lexer_match(ctx->lexer, LEX_T_RPAREN)) { + parse_sample_arg(ctx, sample); + if (ctx->lexer->error) { + return; + } + lexer_match(ctx->lexer, LEX_T_COMMA); + } + } + if (!sample->probability) { + lexer_error(ctx->lexer, "probability must be greater than zero"); + return; + } +} + +static void +ovnact_sample_free(struct ovnact_sample *sample OVS_UNUSED) +{ +} + /* Parses an assignment or exchange or put_dhcp_opts action. */ static void parse_set_action(struct action_context *ctx) @@ -4458,6 +4575,8 @@ parse_action(struct action_context *ctx) ovnact_put_CT_SNAT_TO_VIP(ctx->ovnacts); } else if (lexer_match_id(ctx->lexer, "put_fdb")) { parse_put_fdb(ctx, ovnact_put_PUT_FDB(ctx->ovnacts)); + } else if (lexer_match_id(ctx->lexer, "sample")) { + parse_sample(ctx); } else { lexer_syntax_error(ctx->lexer, "expecting action"); } diff --git a/tests/ovn.at b/tests/ovn.at index 59d51f3e0..7afe0040b 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -2080,6 +2080,15 @@ pop(eth.type); push(abc); Syntax error at `abc' expecting field name. +# sample +sample(probability=100,collector_set=200,obs_domain=0,obs_point=1000); + encodes as sample(probability=100,collector_set_id=200,obs_domain_id=11259375,obs_point_id=1000) + +# sample with obs_domain = 10. Final obs_domain is 0xA << 24 | 0xABCDEF. +sample(probability=100,collector_set=200,obs_domain=10,obs_point=$cookie); + encodes as sample(probability=100,collector_set_id=200,obs_domain_id=179031535,obs_point_id=2863311530) + + # Miscellaneous negative tests. ; Syntax error at `;'. diff --git a/tests/test-ovn.c b/tests/test-ovn.c index a241f150d..ea319f81e 100644 --- a/tests/test-ovn.c +++ b/tests/test-ovn.c @@ -1355,6 +1355,9 @@ test_parse_actions(struct ovs_cmdl_context *ctx OVS_UNUSED) .common_nat_ct_zone = MFF_LOG_DNAT_ZONE, .in_port_sec_ptable = OFTABLE_CHK_IN_PORT_SEC, .out_port_sec_ptable = OFTABLE_CHK_OUT_PORT_SEC, + .lflow_uuid.parts = + { 0xaaaaaaaa, 0xbbbbbbbb, 0xcccccccc, 0xdddddddd}, + .tunnel_key = 0xabcdef, }; struct ofpbuf ofpacts; ofpbuf_init(&ofpacts, 0); diff --git a/utilities/ovn-trace.c b/utilities/ovn-trace.c index c4110de0a..d6ccc145f 100644 --- a/utilities/ovn-trace.c +++ b/utilities/ovn-trace.c @@ -3224,6 +3224,8 @@ trace_actions(const struct ovnact *ovnacts, size_t ovnacts_len, execute_check_out_port_sec(ovnact_get_CHECK_OUT_PORT_SEC(a), dp, uflow); break; + case OVNACT_SAMPLE: + break; } } ofpbuf_uninit(&stack);
sample ovn action encodes into the OFPACT_SAMPLE ovs action. OVN action allows the following parameters: - obs_domain_id: 8-bit integer that identifies the sampling application. This value will be combined with the datapath's tunnel_id to form the final observation_domain_id that will be used in the OVS action. - obs_point_id: a 32-bit integer or the $cookie macro that will be expanded into the first 32 bits of the lflow's UUID. - probability: a 16-bit integer that specifies the sampling probability. Specifying 0 has no effect and 65535 means sampling all packets. Signed-off-by: Adrian Moreno <amorenoz@redhat.com> --- controller/lflow.c | 1 + include/ovn/actions.h | 16 ++++++ lib/actions.c | 119 ++++++++++++++++++++++++++++++++++++++++++ tests/ovn.at | 9 ++++ tests/test-ovn.c | 3 ++ utilities/ovn-trace.c | 2 + 6 files changed, 150 insertions(+)