diff mbox series

[ovs-dev] dpif-netdev: Allow to set max capacity of flow on netdev

Message ID 1577099854-83820-1-git-send-email-xiangxia.m.yue@gmail.com
State Superseded
Headers show
Series [ovs-dev] dpif-netdev: Allow to set max capacity of flow on netdev | expand

Commit Message

Tonghao Zhang Dec. 23, 2019, 11:17 a.m. UTC 
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

There may be too many flows (> MAX_FLOWS 65536) on
dpif-netdev at same time. For this case, we support
the ovs-appctl command to change the flow max number.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 lib/dpif-netdev.c | 51 +++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 49 insertions(+), 2 deletions(-)

Comments

Ben Pfaff Jan. 7, 2020, 10:34 p.m. UTC | #1 
On Mon, Dec 23, 2019 at 07:17:34PM +0800, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> There may be too many flows (> MAX_FLOWS 65536) on
> dpif-netdev at same time. For this case, we support
> the ovs-appctl command to change the flow max number.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Thanks for the patch.

I think that netdev_max_flows should be an atomic_uint32_t because it is
accessed from multiple threads.

Usually the replies to appctl commands are very terse, so that a
successful reply to change a value would just be empty and the reply for
getting a value would just be the raw value, and usually we don't
include a new-line at the end.

Thanks,

Ben.
Tonghao Zhang Jan. 8, 2020, 9:36 a.m. UTC | #2 
On Wed, Jan 8, 2020 at 6:34 AM Ben Pfaff <blp@ovn.org> wrote:
>
> On Mon, Dec 23, 2019 at 07:17:34PM +0800, xiangxia.m.yue@gmail.com wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > There may be too many flows (> MAX_FLOWS 65536) on
> > dpif-netdev at same time. For this case, we support
> > the ovs-appctl command to change the flow max number.
> >
> > Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> Thanks for the patch.
>
> I think that netdev_max_flows should be an atomic_uint32_t because it is
> accessed from multiple threads.
>
> Usually the replies to appctl commands are very terse, so that a
> successful reply to change a value would just be empty and the reply for
> getting a value would just be the raw value, and usually we don't
> include a new-line at the end.
v2 is sent, pls help me review this patch:
http://patchwork.ozlabs.org/patch/1219494/

Thanks.
> Thanks,
>
> Ben.
diff mbox series

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 8485b54db0d8..9d14268aa703 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -97,7 +97,6 @@  DEFINE_STATIC_PER_THREAD_DATA(uint32_t, recirc_depth, 0)
 #define DEFAULT_TX_FLUSH_INTERVAL 0
 
 /* Configuration parameters. */
-enum { MAX_FLOWS = 65536 };     /* Maximum number of flows in flow table. */
 enum { MAX_METERS = 65536 };    /* Maximum number of meters. */
 enum { MAX_BANDS = 8 };         /* Maximum number of bands / meter. */
 enum { N_METER_LOCKS = 64 };    /* Maximum number of meters. */
@@ -105,6 +104,9 @@  enum { N_METER_LOCKS = 64 };    /* Maximum number of meters. */
 /* Protects against changes to 'dp_netdevs'. */
 static struct ovs_mutex dp_netdev_mutex = OVS_MUTEX_INITIALIZER;
 
+/* Maximum number of flows in flow table. */
+static uint32_t netdev_max_flows = 65536;
+
 /* Contains all 'struct dp_netdev's. */
 static struct shash dp_netdevs OVS_GUARDED_BY(dp_netdev_mutex)
     = SHASH_INITIALIZER(&dp_netdevs);
@@ -1112,6 +1114,43 @@  pmd_info_show_perf(struct ds *reply,
     }
 }
 
+static void
+dpif_netdev_set_max_flow(struct unixctl_conn *conn,
+                         int argc OVS_UNUSED,
+                         const char *argv[],
+                         void *aux OVS_UNUSED)
+{
+    struct ds ds = DS_EMPTY_INITIALIZER;
+    uint32_t max_flows = atoi(argv[1]);
+
+    if (!max_flows) {
+        unixctl_command_reply_error(conn,
+                                    "the max flow capacity should > 0\n");
+        return;
+    }
+
+    netdev_max_flows = max_flows;
+    ds_put_format(&ds, "set netdev_max_flows to %u\n", netdev_max_flows);
+    unixctl_command_reply(conn, ds_cstr(&ds));
+
+    ds_destroy(&ds);
+}
+
+static void
+dpif_netdev_show_max_flow(struct unixctl_conn *conn,
+                          int argc OVS_UNUSED,
+                          const char *argv[] OVS_UNUSED,
+                          void *aux OVS_UNUSED)
+{
+    struct ds reply = DS_EMPTY_INITIALIZER;
+
+    ds_put_format(&reply,"pmd flow capacity: %u\n",
+                  netdev_max_flows);
+    unixctl_command_reply(conn, ds_cstr(&reply));
+
+    ds_destroy(&reply);
+}
+
 static int
 compare_poll_list(const void *a_, const void *b_)
 {
@@ -1416,6 +1455,14 @@  dpif_netdev_init(void)
                              "[-us usec] [-q qlen]",
                              0, 10, pmd_perf_log_set_cmd,
                              NULL);
+    unixctl_command_register("dpif-netdev/pmd-set-flow-capacity",
+                             "max-flow-number",
+                             1, 1, dpif_netdev_set_max_flow,
+                             NULL);
+    unixctl_command_register("dpif-netdev/pmd-show-flow-capacity",
+                             "",
+                             0, 0, dpif_netdev_show_max_flow,
+                             NULL);
     return 0;
 }
 
@@ -3345,7 +3392,7 @@  flow_put_on_pmd(struct dp_netdev_pmd_thread *pmd,
     netdev_flow = dp_netdev_pmd_lookup_flow(pmd, key, NULL);
     if (!netdev_flow) {
         if (put->flags & DPIF_FP_CREATE) {
-            if (cmap_count(&pmd->flow_table) < MAX_FLOWS) {
+            if (cmap_count(&pmd->flow_table) < netdev_max_flows) {
                 dp_netdev_flow_add(pmd, match, ufid, put->actions,
                                    put->actions_len);
                 error = 0;