[ovs-dev] [PATCH] ofproto-dpif: Let the dpif report when a port is a duplicate.

Ben Pfaff blp at ovn.org
Thu Jun 21 22:53:53 UTC 2018


The port_add() function checks whether the port about to be added to the
dpif is already present and adds it only if it is not.  This duplicates a
check also present (and necessary) in each dpif and races with it as well.
When a dpif has a large number of ports, the check can be expensive (it is
not efficiently implemented).  It would be nice to made the check cheaper,
but it also seems reasonable to do as done in this patch and just let the
dpif report the duplication.

Reported-by: Haifeng Lin <haifeng.lin at huawei.com>
Signed-off-by: Ben Pfaff <blp at ovn.org>
---
 lib/dpif.c             | 9 +++++++--
 ofproto/ofproto-dpif.c | 7 +++----
 2 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/lib/dpif.c b/lib/dpif.c
index f6a7f6a72e18..d78330bef3b8 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -591,8 +591,13 @@ dpif_port_add(struct dpif *dpif, struct netdev *netdev, odp_port_t *port_nop)
             netdev_ports_insert(netdev, dpif->dpif_class, &dpif_port);
         }
     } else {
-        VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port: %s",
-                     dpif_name(dpif), netdev_name, ovs_strerror(error));
+        if (error != EEXIST) {
+            VLOG_WARN_RL(&error_rl, "%s: failed to add %s as port: %s",
+                         dpif_name(dpif), netdev_name, ovs_strerror(error));
+        } else {
+            /* It's fairly common for upper layers to try to add a duplicate
+             * port, and they know how to handle it properly. */
+        }
         port_no = ODPP_NONE;
     }
     if (port_nop) {
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index ca4582cd5064..771be2fcc88a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3683,11 +3683,10 @@ port_add(struct ofproto *ofproto_, struct netdev *netdev)
     }
 
     dp_port_name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf);
-    if (!dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
-        odp_port_t port_no = ODPP_NONE;
-        int error;
 
-        error = dpif_port_add(ofproto->backer->dpif, netdev, &port_no);
+    odp_port_t port_no = ODPP_NONE;
+    int error = dpif_port_add(ofproto->backer->dpif, netdev, &port_no);
+    if (error != EEXIST) {
         if (error) {
             return error;
         }
-- 
2.16.1



More information about the dev mailing list