> > I noticed this patch introduced a memory leak since event->meter is > never freed. > > However, I can understand how this happened because currently everything > in ovnact_controller_event is being leaked. > ovnact_controller_event_free() should be calling free_gen_options(), but > it's not. And with this patch, it should now also free event->meter. > > To avoid convoluting the purpose of each patch, I think the easiest way > to handle this is to submit a v2, where the first patch fixes the > existing memory leak. Then patch 2 can be this one, with the added > freeing of event->meter. And the current patch 2 of this changeset can > become patch 3.
Hi Mark, ack, I agree. I will post v2 soon. Thanks. Regards, Lorenzo > > The memory leak fix will need to be backported to OVS 2.12 as well. > > On 8/1/19 5:56 AM, Lorenzo Bianconi wrote: > > Introduce meter support to trigger_event action in order to not > > overload pinctrl thread under heavy load > > > > Signed-off-by: Lorenzo Bianconi <[email protected]> > > --- > > include/ovn/actions.h | 1 + > > lib/actions.c | 42 ++++++++++++++++++++++++++++++++++++------ > > 2 files changed, 37 insertions(+), 6 deletions(-) > > > > diff --git a/include/ovn/actions.h b/include/ovn/actions.h > > index 63d3907d8..1f2ac2eb2 100644 > > --- a/include/ovn/actions.h > > +++ b/include/ovn/actions.h > > @@ -326,6 +326,7 @@ struct ovnact_controller_event { > > int event_type; /* controller event type */ > > struct ovnact_gen_option *options; > > size_t n_options; > > + char *meter; > > }; > > > > /* Internal use by the helpers below. */ > > diff --git a/lib/actions.c b/lib/actions.c > > index 4eacc44ed..2e321d82e 100644 > > --- a/lib/actions.c > > +++ b/lib/actions.c > > @@ -1273,6 +1273,9 @@ 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, ", "); > > @@ -1420,10 +1423,21 @@ encode_TRIGGER_EVENT(const struct > > ovnact_controller_event *event, > > const struct ovnact_encode_params *ep OVS_UNUSED, > > 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, > > - NX_CTLR_NO_METER, ofpacts); > > + meter_id, ofpacts); > > ovs_be32 ofs = htonl(event->event_type); > > ofpbuf_put(ofpacts, &ofs, sizeof ofs); > > > > @@ -1738,11 +1752,27 @@ parse_trigger_event(struct action_context *ctx, > > sizeof *event->options); > > } > > > > - 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 (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)); > > + } > > if (ctx->lexer->error) { > > return; > > } > > > > _______________________________________________ > dev mailing list > [email protected] > https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
