[ovs-dev] [PATCH ovn 4/5] ovn-northd: Extend metering to Controller-Events
Mark Gray
mark.d.gray at redhat.com
Fri May 21 14:16:57 UTC 2021
On 29/04/2021 18:04, Lorenzo Bianconi wrote:
> From: Dumitru Ceara <dceara at redhat.com>
>
> Co-authored-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
> Signed-off-by: Dumitru Ceara <dceara at redhat.com>
> ---
> include/ovn/actions.h | 1 -
> lib/actions.c | 50 ++++++++-----------------------------------
> northd/ovn-northd.c | 18 ++++++++--------
> tests/ovn.at | 4 ++--
> 4 files changed, 20 insertions(+), 53 deletions(-)
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index ab03df12c..b2f2f57c6 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -393,7 +393,6 @@ struct ovnact_controller_event {
> int event_type; /* controller event type */
> struct ovnact_gen_option *options;
> size_t n_options;
> - char *meter;
> };
>
> /* OVNACT_BIND_VPORT. */
> diff --git a/lib/actions.c b/lib/actions.c
> index 155b4a45a..f0291afef 100644
> --- a/lib/actions.c
> +++ b/lib/actions.c
> @@ -1613,9 +1613,6 @@ format_TRIGGER_EVENT(const struct ovnact_controller_event *event,
> {
> ds_put_format(s, "trigger_event(event = \"%s\"",
> event_to_string(event->event_type));
> - if (event->meter) {
> - ds_put_format(s, ", meter = \"%s\"", event->meter);
> - }
> for (const struct ovnact_gen_option *o = event->options;
> o < &event->options[event->n_options]; o++) {
> ds_put_cstr(s, ", ");
> @@ -1790,24 +1787,11 @@ encode_event_empty_lb_backends_opts(struct ofpbuf *ofpacts,
>
> static void
> encode_TRIGGER_EVENT(const struct ovnact_controller_event *event,
> - const struct ovnact_encode_params *ep OVS_UNUSED,
> + const struct ovnact_encode_params *ep,
> struct ofpbuf *ofpacts)
> {
> - uint32_t meter_id = NX_CTLR_NO_METER;
> - size_t oc_offset;
> -
> - if (event->meter) {
> - meter_id = ovn_extend_table_assign_id(ep->meter_table, event->meter,
> - ep->lflow_uuid);
> - if (meter_id == EXT_TABLE_ID_INVALID) {
> - VLOG_WARN("Unable to assign id for trigger meter: %s",
> - event->meter);
> - return;
> - }
> - }
> -
> - oc_offset = encode_start_controller_op(ACTION_OPCODE_EVENT, false,
> - meter_id, ofpacts);
> + size_t oc_offset = encode_start_controller_op(ACTION_OPCODE_EVENT, false,
> + ep->ctrl_meter_id, ofpacts);
> ovs_be32 ofs = htonl(event->event_type);
> ofpbuf_put(ofpacts, &ofs, sizeof ofs);
>
> @@ -2341,27 +2325,12 @@ parse_trigger_event(struct action_context *ctx,
> sizeof *event->options);
> }
>
> - if (lexer_match_id(ctx->lexer, "meter")) {
> - if (!lexer_force_match(ctx->lexer, LEX_T_EQUALS)) {
> - return;
> - }
> - /* If multiple meters are given, use the most recent. */
> - if (ctx->lexer->token.type == LEX_T_STRING &&
> - strlen(ctx->lexer->token.s)) {
> - free(event->meter);
> - event->meter = xstrdup(ctx->lexer->token.s);
> - } else if (ctx->lexer->token.type != LEX_T_STRING) {
> - lexer_syntax_error(ctx->lexer, "expecting string");
> - return;
> - }
> - lexer_get(ctx->lexer);
> - } else {
> - struct ovnact_gen_option *o = &event->options[event->n_options++];
> - memset(o, 0, sizeof *o);
> - parse_gen_opt(ctx, o,
> - &ctx->pp->controller_event_opts->event_opts[event_type],
> - event_to_string(event_type));
> - }
> + struct ovnact_gen_option *o = &event->options[event->n_options++];
> + memset(o, 0, sizeof *o);
> + parse_gen_opt(ctx, o,
> + &ctx->pp->controller_event_opts->event_opts[event_type],
> + event_to_string(event_type));
> +
> if (ctx->lexer->error) {
> return;
> }
> @@ -2382,7 +2351,6 @@ static void
> ovnact_controller_event_free(struct ovnact_controller_event *event)
> {
> free_gen_options(event->options, event->n_options);
> - free(event->meter);
> }
>
> static void
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 93b431f4c..a3eb8f646 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -5083,11 +5083,7 @@ build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows,
>
> bool ipv4 = IN6_IS_ADDR_V4MAPPED(&lb_vip->vip);
> struct ds match = DS_EMPTY_INITIALIZER;
> - char *meter = "", *action;
> -
> - if (meter_groups && shash_find(meter_groups, "event-elb")) {
> - meter = "event-elb";
> - }
> + char *action;
>
> ds_put_format(&match, "ip%s.dst == %s && %s",
> ipv4 ? "4": "6", lb_vip->vip_str, lb->protocol);
> @@ -5101,14 +5097,18 @@ build_empty_lb_event_flow(struct ovn_datapath *od, struct hmap *lflows,
> }
>
> action = xasprintf("trigger_event(event = \"%s\", "
> - "meter = \"%s\", vip = \"%s\", "
> + "vip = \"%s\", "
> "protocol = \"%s\", "
> "load_balancer = \"" UUID_FMT "\");",
> event_to_string(OVN_EVENT_EMPTY_LB_BACKENDS),
> - meter, vip, lb->protocol,
> + vip, lb->protocol,
> UUID_ARGS(&lb->header_.uuid));
> - ovn_lflow_add_with_hint(lflows, od, pl, 130, ds_cstr(&match), action,
> - &lb->header_);
> +
> + const struct nbrec_copp *copp = (od->nbr ? od->nbr->copp : od->nbs->copp);
> + ovn_lflow_add_with_hint__(lflows, od, pl, 130, ds_cstr(&match), action,
> + copp_meter_get(COPP_EVENT_ELB, copp,
> + meter_groups),
> + &lb->header_);
> ds_destroy(&match);
> if (lb_vip->vip_port) {
> free(vip);
> diff --git a/tests/ovn.at b/tests/ovn.at
> index ae1b472e4..aff520f3a 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1661,8 +1661,7 @@ trigger_event(event = "empty_lb_backends", vip = "10.0.0.1:80", protocol = "tcp"
> encodes as controller(userdata=00.00.00.0f.00.00.00.00.00.00.00.00.00.01.00.0b.31.30.2e.30.2e.30.2e.31.3a.38.30.00.02.00.03.74.63.70.00.03.00.24.31.32.33.34.35.36.37.38.2d.61.62.63.64.2d.39.38.37.36.2d.66.65.64.63.2d.31.31.31.31.39.66.38.65.37.64.36.63)
>
> trigger_event(event = "empty_lb_backends", meter="event-elb" vip = "10.0.0.1:80", protocol = "tcp", load_balancer = "12345678-abcd-9876-fedc-11119f8e7d6c");
> - formats as trigger_event(event = "empty_lb_backends", meter = "event-elb", vip = "10.0.0.1:80", protocol = "tcp", load_balancer = "12345678-abcd-9876-fedc-11119f8e7d6c");
> - encodes as controller(userdata=00.00.00.0f.00.00.00.00.00.00.00.00.00.01.00.0b.31.30.2e.30.2e.30.2e.31.3a.38.30.00.02.00.03.74.63.70.00.03.00.24.31.32.33.34.35.36.37.38.2d.61.62.63.64.2d.39.38.37.36.2d.66.65.64.63.2d.31.31.31.31.39.66.38.65.37.64.36.63,meter_id=5)
> + Syntax error at `meter' expecting empty_lb_backends option name.
>
> # Testing invalid vip results in extra error messages from socket-util.c
> trigger_event(event = "empty_lb_backends", vip = "10.0.0.1:80", protocol = "aarp", load_balancer = "12345678-abcd-9876-fedc-11119f8e7d6c");
> @@ -17629,6 +17628,7 @@ ovn-nbctl ls-lb-add sw0 lb2
> uuid_lb2=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb2)
>
> ovn-nbctl --wait=hv meter-add event-elb drop 100 pktps 10
> +ovn-nbctl --wait=hv ls-copp-add sw0 event-elb event-elb
>
> OVN_POPULATE_ARP
> wait_for_ports_up
>
Should this not be merged into the previous patch?
More information about the dev
mailing list