On 18 Oct 2018, at 18:13, Sriharsha Basavapatna via dev wrote:
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.
Was there a specific reason to go with pps and not bytes per second?
Asking as larger packets might need more copying around vs a lot of
small packets need more flow processing.
Signed-off-by: Sriharsha Basavapatna
<sriharsha.basavapa...@broadcom.com>
Co-authored-by: Venkat Duvvuru <venkatkumar.duvv...@broadcom.com>
Signed-off-by: Venkat Duvvuru <venkatkumar.duvv...@broadcom.com>
Reviewed-by: Sathya Perla <sathya.pe...@broadcom.com>
Reviewed-by: Simon Horman <simon.hor...@netronome.com>
Reviewed-by: Ben Pfaff <b...@ovn.org>
---
lib/dpif-provider.h | 1 +
ofproto/ofproto-dpif-upcall.c | 51
+++++++++++++++++++++++++++++++++++
2 files changed, 52 insertions(+)
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..a372d6252 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,13 @@ 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
*/
+ 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 +1676,10 @@ 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->flow_packets = ukey->flow_backlog_packets = 0;
+
For clarity I think we need to set the flow_pps_rate to 0 also.
ukey->key_recirc_id = key_recirc_id;
recirc_refs_init(&ukey->recircs);
if (xout) {
@@ -2442,6 +2455,39 @@ reval_op_init(struct ukey_op *op, enum
reval_result result,
}
}
+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;
It does not make sense to me why the flow_backlog_packets get added
here, and also to the flow_packets? See also below.
+}
+
+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;
+ }
+
+ 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;
In the help the flow_packets is described as "#pkts seen in interval"
why do we need to add flow_backlog_packets? They are from the previous
mode? See also comment above.
+ ukey->flow_time = udpif->dpif->current_ms;
+}
+
static void
revalidate(struct revalidator *revalidator)
{
@@ -2550,6 +2596,11 @@ revalidate(struct revalidator *revalidator)
}
ukey->dump_seq = dump_seq;
+ if (netdev_is_offload_rebalance_policy_enabled() &&
+ 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,
--
2.18.0.rc1.1.g6f333ff
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
