diff mbox series

[ovs-dev,v5,2/3] revalidator: Gather packets-per-second rate of flows

Message ID 20180912142706.19969-3-sriharsha.basavapatna@broadcom.com
State Superseded
Headers show
Series Support dynamic rebalancing of offloaded flows | expand

Commit Message

Li,Rongqing via dev Sept. 12, 2018, 2:27 p.m. UTC 
This is the second patch in the patch-set to support dynamic rebalancing
of offloaded flows.

The packets-per-second (pps) rate for each flow is computed in the context
of revalidator threads when the flow stats are retrieved. The pps-rate is
computed only after a flow is revalidated and is not scheduled for
deletion. The parameters used to compute pps and the pps itself are saved
in udpif_key since they need to be persisted across iterations of
rebalancing.

Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Co-authored-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Reviewed-by: Sathya Perla <sathya.perla@broadcom.com>
---
 lib/dpif-provider.h           |   1 +
 ofproto/ofproto-dpif-upcall.c | 129 ++++++++++++++++++++++++++++++++++
 2 files changed, 130 insertions(+)

Comments

0-day Robot Sept. 12, 2018, 2:58 p.m. UTC | #1 
Bleep bloop.  Greetings Sriharsha Basavapatna via dev, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author Sriharsha Basavapatna via dev <ovs-dev@openvswitch.org> needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors or committers: Sriharsha Basavapatna <sriharsha.basavapatna@broadcom.com>
Lines checked: 208, Warnings: 1, Errors: 1


build:
mv -f ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-mirror.Tpo ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-mirror.Plo
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror   -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-monitor.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-monitor.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto-dpif-monitor.lo `test -f 'ofproto/ofproto-dpif-monitor.c' || echo './'`ofproto/ofproto-dpif-monitor.c
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-monitor.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-monitor.Tpo -c ofproto/ofproto-dpif-monitor.c -o ofproto/ofproto_libofproto_la-ofproto-dpif-monitor.o
mv -f ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-monitor.Tpo ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-monitor.Plo
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror   -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-rid.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-rid.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto-dpif-rid.lo `test -f 'ofproto/ofproto-dpif-rid.c' || echo './'`ofproto/ofproto-dpif-rid.c
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-rid.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-rid.Tpo -c ofproto/ofproto-dpif-rid.c -o ofproto/ofproto_libofproto_la-ofproto-dpif-rid.o
mv -f ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-rid.Tpo ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-rid.Plo
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror   -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-sflow.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-sflow.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto-dpif-sflow.lo `test -f 'ofproto/ofproto-dpif-sflow.c' || echo './'`ofproto/ofproto-dpif-sflow.c
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-sflow.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-sflow.Tpo -c ofproto/ofproto-dpif-sflow.c -o ofproto/ofproto_libofproto_la-ofproto-dpif-sflow.o
mv -f ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-sflow.Tpo ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-sflow.Plo
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror   -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-trace.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-trace.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto-dpif-trace.lo `test -f 'ofproto/ofproto-dpif-trace.c' || echo './'`ofproto/ofproto-dpif-trace.c
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-trace.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-trace.Tpo -c ofproto/ofproto-dpif-trace.c -o ofproto/ofproto_libofproto_la-ofproto-dpif-trace.o
mv -f ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-trace.Tpo ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-trace.Plo
/bin/sh ./libtool  --tag=CC   --mode=compile gcc -std=gnu99 -DHAVE_CONFIG_H -I.    -I ./include -I ./include -I ./lib -I ./lib    -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror   -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-upcall.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-upcall.Tpo -c -o ofproto/ofproto_libofproto_la-ofproto-dpif-upcall.lo `test -f 'ofproto/ofproto-dpif-upcall.c' || echo './'`ofproto/ofproto-dpif-upcall.c
libtool: compile:  gcc -std=gnu99 -DHAVE_CONFIG_H -I. -I ./include -I ./include -I ./lib -I ./lib -Wstrict-prototypes -Wall -Wextra -Wno-sign-compare -Wpointer-arith -Wformat -Wformat-security -Wswitch-enum -Wunused-parameter -Wbad-function-cast -Wcast-align -Wstrict-prototypes -Wold-style-definition -Wmissing-prototypes -Wmissing-field-initializers -fno-strict-aliasing -Wshadow -Wno-null-pointer-arithmetic -Werror -Werror -g -O2 -MT ofproto/ofproto_libofproto_la-ofproto-dpif-upcall.lo -MD -MP -MF ofproto/.deps/ofproto_libofproto_la-ofproto-dpif-upcall.Tpo -c ofproto/ofproto-dpif-upcall.c -o ofproto/ofproto_libofproto_la-ofproto-dpif-upcall.o
ofproto/ofproto-dpif-upcall.c:2481:1: error: 'ukey_to_flow_netdevs' defined but not used [-Werror=unused-function]
 ukey_to_flow_netdevs(struct udpif *udpif, struct udpif_key *ukey)
 ^
cc1: error: unrecognized command line option "-Wno-null-pointer-arithmetic" [-Werror]
cc1: all warnings being treated as errors
make[2]: *** [ofproto/ofproto_libofproto_la-ofproto-dpif-upcall.lo] Error 1
make[2]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/var/lib/jenkins/jobs/upstream_build_from_pw/workspace'
make: *** [all] Error 2


Please check this out.  If you feel there has been an error, please email aconole@bytheb.org

Thanks,
0-day Robot
diff mbox series

Patch

diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 873b6e3d4..7a71f5c0a 100644
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -39,6 +39,7 @@  struct dpif {
     char *full_name;
     uint8_t netflow_engine_type;
     uint8_t netflow_engine_id;
+    long long int current_ms;
 };
 
 void dpif_init(struct dpif *, const struct dpif_class *, const char *name,
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 62222079f..ef64b9c48 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -42,6 +42,8 @@ 
 #include "tunnel.h"
 #include "unixctl.h"
 #include "openvswitch/vlog.h"
+#include "lib/dpif-provider.h"
+#include "lib/netdev-provider.h"
 
 #define MAX_QUEUE_LENGTH 512
 #define UPCALL_MAX_BATCH 64
@@ -304,6 +306,15 @@  struct udpif_key {
 
     uint32_t key_recirc_id;   /* Non-zero if reference is held by the ukey. */
     struct recirc_refs recircs;  /* Action recirc IDs with references held. */
+
+#define OFFL_REBAL_INTVL_MSEC  3000	/* dynamic offload rebalance freq */
+    struct netdev *in_netdev;		/* in_odp_port's netdev */
+    struct netdev *out_netdev;		/* out_odp_port's netdev */
+    bool offloaded;			/* True if flow is offloaded */
+    uint64_t flow_pps_rate;		/* Packets-Per-Second rate */
+    long long int flow_time;		/* last pps update time */
+    uint64_t flow_packets;		/* #pkts seen in interval */
+    uint64_t flow_backlog_packets;	/* prev-mode #pkts (offl or kernel) */
 };
 
 /* Datapath operation with optional ukey attached. */
@@ -1667,6 +1678,11 @@  ukey_create__(const struct nlattr *key, size_t key_len,
     ukey->stats.used = used;
     ukey->xcache = NULL;
 
+    ukey->offloaded = false;
+    ukey->flow_time = 0;
+    ukey->in_netdev = ukey->out_netdev = NULL;
+    ukey->flow_packets = ukey->flow_backlog_packets = 0;
+
     ukey->key_recirc_id = key_recirc_id;
     recirc_refs_init(&ukey->recircs);
     if (xout) {
@@ -2442,6 +2458,115 @@  reval_op_init(struct ukey_op *op, enum reval_result result,
     }
 }
 
+static void
+ukey_netdev_unref(struct udpif_key *ukey)
+{
+    if (ukey->in_netdev) {
+        netdev_close(ukey->in_netdev);
+        ukey->in_netdev = NULL;
+    }
+    if (ukey->out_netdev) {
+        netdev_close(ukey->out_netdev);
+        ukey->out_netdev = NULL;
+    }
+}
+
+/*
+ * Given a udpif_key, get its input and output ports (netdevs) by parsing
+ * the flow keys and actions. The flow may not contain flow attributes if
+ * it is a terse dump; read its attributes from the ukey and then parse
+ * the flow to get the port info. Save them in udpif_key.
+ */
+static void
+ukey_to_flow_netdevs(struct udpif *udpif, struct udpif_key *ukey)
+{
+    const struct dpif *dpif = udpif->dpif;
+    const struct dpif_class *dpif_class = dpif->dpif_class;
+    const struct nlattr *actions;
+    const struct nlattr *a;
+    const struct nlattr *k;
+    size_t actions_len;
+    unsigned int left;
+
+    /* Remove existing references to netdevs */
+    ukey_netdev_unref(ukey);
+
+    /* Read actions from ukey */
+    ukey_get_actions(ukey, &actions, &actions_len);
+
+    /* Capture the output port and get a reference to its netdev; we are
+     * only interested that the flow has an output port, so we just save the
+     * first port if there are multiple output actions associated with it.
+     */
+    NL_ATTR_FOR_EACH (a, left, actions, actions_len) {
+        enum ovs_action_attr type = nl_attr_type(a);
+        if (type == OVS_ACTION_ATTR_OUTPUT) {
+            ukey->out_netdev = netdev_ports_get(nl_attr_get_odp_port(a),
+                                                dpif_class);
+            break;
+        }
+    }
+
+    /* Now find the input port and get a reference to its netdev */
+    NL_ATTR_FOR_EACH (k, left, ukey->key, ukey->key_len) {
+        enum ovs_key_attr type = nl_attr_type(k);
+
+        if (type == OVS_KEY_ATTR_IN_PORT) {
+            ukey->in_netdev = netdev_ports_get(nl_attr_get_odp_port(k),
+                                               dpif_class);
+        } else if (type == OVS_KEY_ATTR_TUNNEL) {
+            struct flow_tnl tnl;
+            enum odp_key_fitness res;
+
+            if (ukey->in_netdev) {
+                netdev_close(ukey->in_netdev);
+                ukey->in_netdev = NULL;
+            }
+            res = odp_tun_key_from_attr(k, &tnl);
+            if (res != ODP_FIT_ERROR) {
+                ukey->in_netdev = flow_get_tunnel_netdev(&tnl);
+                break;
+            }
+        }
+    }
+}
+
+static uint64_t
+udpif_flow_packet_delta(struct udpif_key *ukey, const struct dpif_flow *f)
+{
+    return f->stats.n_packets + ukey->flow_backlog_packets -
+                ukey->flow_packets;
+}
+
+static long long int
+udpif_flow_time_delta(struct udpif *udpif, struct udpif_key *ukey)
+{
+    return (udpif->dpif->current_ms - ukey->flow_time) / 1000;
+}
+
+/* Gather pps-rate for the given dpif_flow and save it in its ukey */
+static void
+udpif_update_flow_pps(struct udpif *udpif, struct udpif_key *ukey,
+                      const struct dpif_flow *f)
+{
+    uint64_t pps;
+
+    /* Update pps-rate only when we are close to rebalance interval */
+    if (udpif->dpif->current_ms - ukey->flow_time < OFFL_REBAL_INTVL_MSEC) {
+        return;
+    }
+
+    ovs_assert(f->stats.n_packets + ukey->flow_backlog_packets >=
+               ukey->flow_packets);
+
+    ukey->offloaded = f->attrs.offloaded;
+    pps = udpif_flow_packet_delta(ukey, f) /
+                    udpif_flow_time_delta(udpif, ukey);
+    ukey->flow_pps_rate = pps;
+    ukey->flow_packets = ukey->flow_backlog_packets + f->stats.n_packets;
+    ukey->flow_time = udpif->dpif->current_ms;
+}
+
 static void
 revalidate(struct revalidator *revalidator)
 {
@@ -2550,6 +2675,10 @@  revalidate(struct revalidator *revalidator)
             }
             ukey->dump_seq = dump_seq;
 
+            if (result != UKEY_DELETE) {
+                udpif_update_flow_pps(udpif, ukey, f);
+            }
+
             if (result != UKEY_KEEP) {
                 /* Takes ownership of 'recircs'. */
                 reval_op_init(&ops[n_ops++], result, udpif, ukey, &recircs,