diff mbox series

[ovs-dev,v4,ovn,1/2] OVN: add meter support to trigger_event action

Message ID 1e8d936ad66a1bdeabaa37b640c06c20fcb0fa9f.1566993890.git.lorenzo.bianconi@redhat.com
State Superseded
Headers show
Series Add rate limiting support for controller_events | expand

Commit Message

Lorenzo Bianconi Aug. 28, 2019, 12:13 p.m. UTC
Introduce meter support to trigger_event action in order to not
overload pinctrl thread under heavy load

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
 include/ovn/actions.h |  1 +
 lib/actions.c         | 43 +++++++++++++++++++++++++++++++++++++------
 ovn-sb.xml            |  5 ++++-
 tests/ovn.at          |  4 ++++
 4 files changed, 46 insertions(+), 7 deletions(-)

Comments

Mark Michelson Aug. 29, 2019, 8:31 p.m. UTC | #1
Acked-by: Mark Michelson <mmichels@redhat.com>

On 8/28/19 8:13 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 <lorenzo.bianconi@redhat.com>
> ---
>   include/ovn/actions.h |  1 +
>   lib/actions.c         | 43 +++++++++++++++++++++++++++++++++++++------
>   ovn-sb.xml            |  5 ++++-
>   tests/ovn.at          |  4 ++++
>   4 files changed, 46 insertions(+), 7 deletions(-)
> 
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 0ca06537c..145f27f25 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -327,6 +327,7 @@ 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 08c589ab3..6a5907e1b 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;
>           }
> @@ -1763,6 +1793,7 @@ 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/ovn-sb.xml b/ovn-sb.xml
> index 02691bbd3..477e7bc7a 100644
> --- a/ovn-sb.xml
> +++ b/ovn-sb.xml
> @@ -1996,7 +1996,9 @@ tcp.flags = RST;
>             <p>
>               This action is used to allow ovs-vswitchd to report CMS related
>               events writing them in <ref table="Controller_Event"/> table.
> -            Supported event:
> +            It is possible to associate a meter to a each event in order to
> +            not overload pinctrl thread under heavy load; each meter is
> +            identified though a defined naming convention. Supported events:
>             </p>
>   
>             <ul>
> @@ -2007,6 +2009,7 @@ tcp.flags = RST;
>                   no configured backend destinations. For this event, the event
>                   info includes the load balancer VIP, the load balancer UUID,
>                   and the transport protocol.
> +                Associated meter: <code>event-elb</code>
>                 </p>
>               </li>
>             </ul>
> diff --git a/tests/ovn.at b/tests/ovn.at
> index 86078c400..014566d50 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -1353,6 +1353,10 @@ tcp_reset { };
>   trigger_event(event = "empty_lb_backends", 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)
>   
> +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)
> +
>   # 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 = "sctp", load_balancer = "12345678-abcd-9876-fedc-11119f8e7d6c");
>       Load balancer protocol 'sctp' is not 'tcp' or 'udp'
>
diff mbox series

Patch

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 0ca06537c..145f27f25 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -327,6 +327,7 @@  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 08c589ab3..6a5907e1b 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;
         }
@@ -1763,6 +1793,7 @@  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/ovn-sb.xml b/ovn-sb.xml
index 02691bbd3..477e7bc7a 100644
--- a/ovn-sb.xml
+++ b/ovn-sb.xml
@@ -1996,7 +1996,9 @@  tcp.flags = RST;
           <p>
             This action is used to allow ovs-vswitchd to report CMS related
             events writing them in <ref table="Controller_Event"/> table.
-            Supported event:
+            It is possible to associate a meter to a each event in order to
+            not overload pinctrl thread under heavy load; each meter is
+            identified though a defined naming convention. Supported events:
           </p>
 
           <ul>
@@ -2007,6 +2009,7 @@  tcp.flags = RST;
                 no configured backend destinations. For this event, the event
                 info includes the load balancer VIP, the load balancer UUID,
                 and the transport protocol.
+                Associated meter: <code>event-elb</code>
               </p>
             </li>
           </ul>
diff --git a/tests/ovn.at b/tests/ovn.at
index 86078c400..014566d50 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -1353,6 +1353,10 @@  tcp_reset { };
 trigger_event(event = "empty_lb_backends", 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)
 
+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)
+
 # 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 = "sctp", load_balancer = "12345678-abcd-9876-fedc-11119f8e7d6c");
     Load balancer protocol 'sctp' is not 'tcp' or 'udp'