[net] openvswitch: Allow deferred action fifo to expand during run time

* [net] openvswitch: Allow deferred action fifo to expand during run time
@ 2016-03-18  4:32 Andy Zhou
       [not found] ` <1458275533-2896-1-git-send-email-azhou-LZ6Gd1LRuIk@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Andy Zhou @ 2016-03-18  4:32 UTC (permalink / raw)
  To: dev-yBygre7rU0SM8Zsap4Y0gw, davem-fT/PcQaiUtIeIZ0/mPfg9Q
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA

Current openvswitch implementation allows up to 10 recirculation actions
for each packet. This limit was sufficient for most use cases in the
past, but with more new features, such as supporting connection
tracking, and testing in larger scale network environment,
This limit may be too restrictive.

An obvious design choice is to increase the hard coded limit
from 10 to a larger number.  However, there are two draw backs to this
approach. First, it is not obvious what the upper limit should be.
Second, the fifos are allocated per CPU core. Statically allocated
large buffers will be over kill for most packet execution, and will
increase the module's memory footprint unnecessarily for some
smaller scale applications.

This patch makes a different design choice that accommodates packets
that require more than 10 recirculation actions without significantly
increase module's memory footprint.  kmalloc() is used to expand
deferred action fifo at run time whenever a packet's action contains
more than 10 recirculation actions. The dynamically allocated fifos are
freed as soon as the packet's actions execution are completed.

Reported-and-tested-by: Ramu Ramamurthy <sramamur@linux.vnet.ibm.com>
Reported-at: http://openvswitch.org/pipermail/dev/2016-March/067672.html
Signed-off-by: Andy Zhou <azhou@ovn.org>
---
 net/openvswitch/actions.c | 45 ++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 40 insertions(+), 5 deletions(-)

diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
index 2d59df5..eb068c7 100644
--- a/net/openvswitch/actions.c
+++ b/net/openvswitch/actions.c
@@ -70,12 +70,14 @@ struct ovs_frag_data {
 
 static DEFINE_PER_CPU(struct ovs_frag_data, ovs_frag_data_storage);
 
-#define DEFERRED_ACTION_FIFO_SIZE 10
+#define DEFERRED_ACTION_FIFO_INIT_SIZE 10
 struct action_fifo {
 	int head;
 	int tail;
 	/* Deferred action fifo queue storage. */
-	struct deferred_action fifo[DEFERRED_ACTION_FIFO_SIZE];
+	struct deferred_action *fifo;
+	size_t size;     /* Current size of the fifo in bytes */
+	struct deferred_action initial_fifo[DEFERRED_ACTION_FIFO_INIT_SIZE];
 };
 
 static struct action_fifo __percpu *action_fifos;
@@ -85,6 +87,16 @@ static void action_fifo_init(struct action_fifo *fifo)
 {
 	fifo->head = 0;
 	fifo->tail = 0;
+	fifo->fifo = fifo->initial_fifo;
+	fifo->size = sizeof(*fifo->fifo) * DEFERRED_ACTION_FIFO_INIT_SIZE;
+}
+
+static void action_fifo_clear(struct action_fifo *fifo)
+{
+	if (fifo->fifo != fifo->initial_fifo)
+		kfree(fifo->fifo);
+
+	action_fifo_init(fifo);
 }
 
 static bool action_fifo_is_empty(const struct action_fifo *fifo)
@@ -102,8 +114,22 @@ static struct deferred_action *action_fifo_get(struct action_fifo *fifo)
 
 static struct deferred_action *action_fifo_put(struct action_fifo *fifo)
 {
-	if (fifo->head >= DEFERRED_ACTION_FIFO_SIZE - 1)
-		return NULL;
+	if (fifo->head >= fifo->size / sizeof(*fifo->fifo) - 1) {
+		/* out of fifo buffer space, try to allocate a new fifo
+		 * buffer. */
+		struct deferred_action *new_fifo;
+		int new_size = fifo->size * 2;
+
+		new_fifo = kmalloc(new_size, GFP_ATOMIC);
+		if (new_fifo) {
+			memcpy(new_fifo, fifo->fifo, fifo->size);
+			if (fifo->fifo != fifo->initial_fifo)
+				kfree(fifo->fifo);
+			fifo->fifo = new_fifo;
+			fifo->size = new_size;
+		} else
+			return NULL;
+	}
 
 	return &fifo->fifo[fifo->head++];
 }
@@ -1152,7 +1178,7 @@ static void process_deferred_actions(struct datapath *dp)
 	} while (!action_fifo_is_empty(fifo));
 
 	/* Reset FIFO for the next packet.  */
-	action_fifo_init(fifo);
+	action_fifo_clear(fifo);
 }
 
 /* Execute a list of actions against 'skb'. */
@@ -1185,10 +1211,19 @@ out:
 
 int action_fifos_init(void)
 {
+	int i;
+
 	action_fifos = alloc_percpu(struct action_fifo);
 	if (!action_fifos)
 		return -ENOMEM;
 
+	/* Use pre allocated fifo. */
+	for_each_possible_cpu(i) {
+		struct action_fifo *action_fifo;
+
+		action_fifo = per_cpu_ptr(action_fifos, i);
+		action_fifo_init(action_fifo);
+	}
 	return 0;
 }
 
-- 
1.9.1

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

^ permalink raw reply related	[flat|nested] 3+ messages in thread