[ovs-dev] [tunnel 07/11] netdev: New function netdev_get_dpif_port().

Ethan Jackson ethan at nicira.com
Tue Jan 29 03:09:19 UTC 2013


> I don't see a problem with this except for a layering violation: the
> generic "netdev" layer now has a function and a function pointer in it
> that is specific to the dpif code.  I can see two ways to deal with
> this: either move it out of the generic "netdev" layer into some other
> place, or to add conspicuous comments on ->get_dpif_port() and
> netdev_get_dpif_port() that explain the context and why this
> particular function might not make any sense (and you should just
> leave it NULL) if you're implementing something that isn't a
> dpif-based switch.

That's fair, I had went back and forth several times on which way was better,
but in the end I think you're right and it's better to pull it into the vport
layer.  Below is a new version of the patch.

Also note that netdev_vport_get_dpif_port()'s implementation changes to the
following in the "lib: Switch to flow based tunneling." patch.  I can resend
that if you'd like.

const char *
netdev_vport_get_dpif_port(const struct netdev *netdev)
{
    const struct netdev_dev *dev = netdev_get_dev(netdev);
    const struct netdev_class *class = netdev_dev_get_class(dev);
    const char *dpif_port;

    dpif_port = (is_vport_class(class)
                 ? vport_class_cast(class)->dpif_port
                 : NULL);
    return dpif_port ? dpif_port : netdev_get_name(netdev);
}

---
 lib/dpif-linux.c   |    4 ++--
 lib/dpif-netdev.c  |   18 ++++++++++--------
 lib/netdev-vport.c |    6 ++++++
 lib/netdev-vport.h |    2 ++
 4 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 6863e08..3dc94c2 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -426,7 +426,7 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
                     uint32_t *port_nop)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
-    const char *name = netdev_get_name(netdev);
+    const char *name = netdev_vport_get_dpif_port(netdev);
     const char *type = netdev_get_type(netdev);
     struct dpif_linux_vport request, reply;
     const struct ofpbuf *options;
diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 2cf2265..038dfdc 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -40,6 +40,7 @@
 #include "hmap.h"
 #include "list.h"
 #include "netdev.h"
+#include "netdev-vport.h"
 #include "netlink.h"
 #include "odp-util.h"
 #include "ofp-print.h"
@@ -437,11 +438,11 @@ dpif_netdev_port_add(struct dpif *dpif, struct netdev *netdev,
         }
         port_no = *port_nop;
     } else {
-        port_no = choose_port(dp, netdev_get_name(netdev));
+        port_no = choose_port(dp, netdev_vport_get_dpif_port(netdev));
     }
     if (port_no >= 0) {
         *port_nop = port_no;
-        return do_add_port(dp, netdev_get_name(netdev),
+        return do_add_port(dp, netdev_vport_get_dpif_port(netdev),
                            netdev_get_type(netdev), port_no);
     }
     return EFBIG;
@@ -480,7 +481,7 @@ get_port_by_name(struct dp_netdev *dp,
     struct dp_netdev_port *port;
 
     LIST_FOR_EACH (port, node, &dp->port_list) {
-        if (!strcmp(netdev_get_name(port->netdev), devname)) {
+        if (!strcmp(netdev_vport_get_dpif_port(port->netdev), devname)) {
             *portp = port;
             return 0;
         }
@@ -504,7 +505,7 @@ do_del_port(struct dp_netdev *dp, uint32_t port_no)
     dp->ports[port->port_no] = NULL;
     dp->serial++;
 
-    name = xstrdup(netdev_get_name(port->netdev));
+    name = xstrdup(netdev_vport_get_dpif_port(port->netdev));
     netdev_close(port->netdev);
     free(port->type);
 
@@ -518,7 +519,7 @@ static void
 answer_port_query(const struct dp_netdev_port *port,
                   struct dpif_port *dpif_port)
 {
-    dpif_port->name = xstrdup(netdev_get_name(port->netdev));
+    dpif_port->name = xstrdup(netdev_vport_get_dpif_port(port->netdev));
     dpif_port->type = xstrdup(port->type);
     dpif_port->port_no = port->port_no;
 }
@@ -609,7 +610,7 @@ dpif_netdev_port_dump_next(const struct dpif *dpif, void *state_,
         struct dp_netdev_port *port = dp->ports[port_no];
         if (port) {
             free(state->name);
-            state->name = xstrdup(netdev_get_name(port->netdev));
+            state->name = xstrdup(netdev_vport_get_dpif_port(port->netdev));
             dpif_port->name = state->name;
             dpif_port->type = port->type;
             dpif_port->port_no = port->port_no;
@@ -1068,7 +1069,8 @@ dpif_netdev_run(struct dpif *dpif)
         } else if (error != EAGAIN && error != EOPNOTSUPP) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
             VLOG_ERR_RL(&rl, "error receiving data from %s: %s",
-                        netdev_get_name(port->netdev), strerror(error));
+                        netdev_vport_get_dpif_port(port->netdev),
+                        strerror(error));
         }
     }
     ofpbuf_uninit(&packet);
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 94f1110..95cffd2 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -151,6 +151,12 @@ netdev_vport_is_patch(const struct netdev *netdev)
     return class->get_config == get_patch_config; 
 }
 
+const char *
+netdev_vport_get_dpif_port(const struct netdev *netdev)
+{
+    return netdev_get_name(netdev);
+}
+
 static uint32_t
 get_u32_or_zero(const struct nlattr *a)
 {
diff --git a/lib/netdev-vport.h b/lib/netdev-vport.h
index b372a74..8c81d7a 100644
--- a/lib/netdev-vport.h
+++ b/lib/netdev-vport.h
@@ -41,4 +41,6 @@ void netdev_vport_patch_inc_rx(const struct netdev *,
 void netdev_vport_patch_inc_tx(const struct netdev *,
                                const struct dpif_flow_stats *);
 
+const char *netdev_vport_get_dpif_port(const struct netdev *);
+
 #endif /* netdev-vport.h */
-- 
1.7.9.5




More information about the dev mailing list