On 05/01/2017 03:54, Joe Stringer wrote:
On 25 December 2016 at 03:39, Paul Blakey <pa...@mellanox.com> wrote:
To use netdev flow offloading api, dpifs needs to iterate over
added ports. This addition inserts the added dpif ports in a hash map,
The map will also be used to translate dpif ports to netdevs.

Signed-off-by: Paul Blakey <pa...@mellanox.com>
Reviewed-by: Roi Dayan <r...@mellanox.com>
---
  lib/dpif.c   |  25 +++++++++++
  lib/netdev.c | 133 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
  lib/netdev.h |  14 +++++++
  3 files changed, 172 insertions(+)

diff --git a/lib/dpif.c b/lib/dpif.c
index 53958c5..c67ea92 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -352,7 +352,22 @@ do_open(const char *name, const char *type, bool create, 
struct dpif **dpifp)
      error = registered_class->dpif_class->open(registered_class->dpif_class,
                                                 name, create, &dpif);
      if (!error) {
+        struct dpif_port_dump port_dump;
+        struct dpif_port dpif_port;
+
          ovs_assert(dpif->dpif_class == registered_class->dpif_class);
+
+        DPIF_PORT_FOR_EACH(&dpif_port, &port_dump, dpif) {
+            struct netdev *netdev;
+            int err = netdev_open(dpif_port.name, dpif_port.type, &netdev);
+
+            if (!err) {
+                netdev_hmap_port_add(netdev, dpif->dpif_class, &dpif_port);
+                netdev_close(netdev);
+            } else {
+                VLOG_WARN("could not open netdev %s type %s", name, type);
+            }
+        }
      } else {
          dp_class_unref(registered_class);
      }
@@ -545,6 +560,14 @@ dpif_port_add(struct dpif *dpif, struct netdev *netdev, 
odp_port_t *port_nop)
      if (!error) {
          VLOG_DBG_RL(&dpmsg_rl, "%s: added %s as port %"PRIu32,
                      dpif_name(dpif), netdev_name, port_no);
+
+        /* temp dpif_port, will be cloned in netdev_hmap_port_add */
+        struct dpif_port dpif_port;
+
+        dpif_port.type = CONST_CAST(char *, netdev_get_type(netdev));
+        dpif_port.name = CONST_CAST(char *, netdev_name);
+        dpif_port.port_no = port_no;
+        netdev_hmap_port_add(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));
@@ -569,6 +592,8 @@ dpif_port_del(struct dpif *dpif, odp_port_t port_no)
      if (!error) {
          VLOG_DBG_RL(&dpmsg_rl, "%s: port_del(%"PRIu32")",
                      dpif_name(dpif), port_no);
+
+        netdev_hmap_port_del(port_no, dpif->dpif_class->type);
      } else {
          log_operation(dpif, "port_del", error);
      }
diff --git a/lib/netdev.c b/lib/netdev.c
index b289166..2630802 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -2080,6 +2080,139 @@ netdev_init_flow_api(struct netdev *netdev)
              : EOPNOTSUPP);
  }

+static struct hmap port_to_netdev = HMAP_INITIALIZER(&port_to_netdev);
+static struct hmap ifindex_to_port = HMAP_INITIALIZER(&ifindex_to_port);
+
+struct port_to_netdev_data {
+    struct hmap_node node;
+    struct netdev *netdev;
+    struct dpif_port dpif_port;
+    const void *obj;
+};
+
+struct ifindex_to_port_data {
+    struct hmap_node node;
+    int ifindex;
+    odp_port_t port;
+};
+
+int
+netdev_hmap_port_add(struct netdev *netdev, const void *obj,
+                     struct dpif_port *dpif_port)
+{
+    size_t hash = hash_int(dpif_port->port_no, hash_pointer(obj, 0));
+    struct port_to_netdev_data *data = xzalloc(sizeof *data);
+    struct ifindex_to_port_data *ifidx = xzalloc(sizeof *ifidx);
+
+    netdev_hmap_port_del(dpif_port->port_no, obj);
+
+    data->netdev = netdev_ref(netdev);
+    data->obj = obj;
+    dpif_port_clone(&data->dpif_port, dpif_port);
+
+    ifidx->ifindex = netdev_get_ifindex(netdev);
+    ifidx->port = dpif_port->port_no;
+
+    hmap_insert(&port_to_netdev, &data->node, hash);
+    hmap_insert(&ifindex_to_port, &ifidx->node, ifidx->ifindex);
+
+    return 0;
+}
+
+struct netdev *
+netdev_hmap_port_get(odp_port_t port_no, const void *obj)
+{
+    size_t hash = hash_int(port_no, hash_pointer(obj, 0));
+    struct port_to_netdev_data *data;
+
+    HMAP_FOR_EACH_WITH_HASH(data, node, hash, &port_to_netdev) {
+            if (data->obj == obj && data->dpif_port.port_no == port_no) {
+                break;
+            }
+    }
+
+    if (data) {
+        netdev_ref(data->netdev);
+        return data->netdev;
+    }
+
+    return 0;
+}
+
+odp_port_t
+netdev_hmap_port_get_byifidx(int ifindex)
netdev_hmap_get_port_by_ifindex?

+{
+    struct ifindex_to_port_data *data;
+
+    HMAP_FOR_EACH_WITH_HASH(data, node, ifindex, &ifindex_to_port) {
+            if (data->ifindex == ifindex) {
+                return data->port;
+            }
+    }
+
+    return 0;
+}
+
+int
+netdev_hmap_port_del(odp_port_t port_no, const void *obj)
+{
+    size_t hash = hash_int(port_no, hash_pointer(obj, 0));
+    struct port_to_netdev_data *data;
+
+    HMAP_FOR_EACH_WITH_HASH(data, node, hash, &port_to_netdev) {
+            if (data->obj == obj && data->dpif_port.port_no == port_no) {
+                break;
+            }
+    }
+
+    if (data) {
+        dpif_port_destroy(&data->dpif_port);
+        netdev_close(data->netdev); /* unref and possibly close */
+        hmap_remove(&port_to_netdev, &data->node);
+        free(data);
+        return 0;
+    }
+
+    return ENOENT;
+}
+
+int
+netdev_hmap_port_get_list(const void *obj, struct ovs_list *list)
+{
+    struct port_to_netdev_data *data;
+    int count = 0;
+
+    ovs_list_init(list);
+
+    HMAP_FOR_EACH(data, node, &port_to_netdev) {
+            if (data->obj == obj) {
+                struct netdev_list_element *element = xzalloc(sizeof *element);
+
+                element->netdev = netdev_ref(data->netdev);
+                element->port_no = data->dpif_port.port_no;
+                ovs_list_insert(list, &element->node);
+                count++;
+            }
+    }
+
+    return count;
+}
+
+void
+netdev_port_list_del(struct ovs_list *list)
+{
+    struct netdev_list_element *element;
+
+    if (!list) {
+        return;
+    }
+
+    LIST_FOR_EACH_POP(element, node, list) {
+        netdev_close(element->netdev);
+        free(element);
+    }
+}
+
It seems that all of the users of these list assemble/delete functions do:

netdev_hmap_port_get_list(...)
<iterate list to get what they actually want>
netdev_port_list_del(...)

Perhaps the functions that call these should actually live in
lib/netdev.c, and just get exactly what they want directly from the
hmap instead of allocating list elements, iterating the netdev list,
and freeing the elements (in addition to the iteration they actually
want to do).
Doesn't that break encapsulation a bit?
This is used to iterate over the ports/netdevs and:
    1) flush their offloaded rules
    2) start a dump for offloaded rules
That will introduce all that is required for dumping/flushing to lib/netdev.
I suggest we use a higher order function instead, that will save the allocations and freeing.

/* Netdev dumping. */
struct netdev_list_element {
    struct ovs_list node;
    struct netdev *netdev;
    odp_port_t port_no;
};

/* return true to stop iterating, element is next on the list */
typedef bool (*portIterator)(netdev_list_element *element);

/* pseudo code: */

void netdev_hmap_port_for_each(const void *obj, portIterator func) {
        struct netdev_list_element;
        for each in netdev hmap {
                fill element with next
                if (func(element)) break;
        }
}


  bool netdev_flow_api_enabled = false;
Different patch, but should this be static?
Yes, thanks.
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to