dpif: Simplify the "listen mask" concept.

At one point in the past, there were three separate queues between the
kernel module and OVS userspace, each of which corresponded to a Netlink
socket (or, before that, to a character device).  It made sense to allow
each of these to be enabled or disabled separately, hence the "listen mask"
concept in the dpif layer.

These days, the concept is much less clear-cut.  Queuing is no longer on
the basis of different classes of packets but instead striped across a
collection of sockets based on input port.  It doesn't really make sense
to enable receiving packets on the basis of the kind of packet anymore.
Accordingly, this commit simplifies the "listen_mask" to just a bool that
either enables or disables receiving packets.

It could be useful to enable or disable receiving packets on a per-vport
basis, but the rest of the code isn't ready to make use of that so this
commit doesn't generalize this much.

Based on this discussion on ovs-dev:
http://openvswitch.org/pipermail/dev/2011-October/012044.html

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 317274e398deaaf285574cf7a2b23e1cb4e404ca..741a6f0e04fb7599c7400f4efdf61cd6d370f9db 100644 (file)
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -140,7 +140,6 @@ struct dpif_linux {
     /* Upcall messages. */
     struct nl_sock *upcall_socks[N_UPCALL_SOCKS];
     uint32_t ready_mask;        /* 1-bit for each sock with unread messages. */
-    unsigned int listen_mask;   /* Mask of DPIF_UC_* bits. */
     int epoll_fd;               /* epoll fd that includes the upcall socks. */
 
     /* Change notification. */
@@ -171,9 +170,7 @@ static int dpif_linux_init(void);
 static void open_dpif(const struct dpif_linux_dp *, struct dpif **);
 static bool dpif_linux_nln_parse(struct ofpbuf *, void *);
 static void dpif_linux_port_changed(const void *vport, void *dpif);
-static uint32_t dpif_linux_port_get_pid__(const struct dpif *,
-                                          uint16_t port_no,
-                                          enum dpif_upcall_type);
+static uint32_t dpif_linux_port_get_pid(const struct dpif *, uint16_t port_no);
 
 static void dpif_linux_vport_to_ofpbuf(const struct dpif_linux_vport *,
                                        struct ofpbuf *);
@@ -404,8 +401,7 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
         uint32_t upcall_pid;
 
         request.port_no = dpif_linux_pop_port(dpif);
-        upcall_pid = dpif_linux_port_get_pid__(dpif_, request.port_no,
-                                               DPIF_UC_MISS);
+        upcall_pid = dpif_linux_port_get_pid(dpif_, request.port_no);
         request.upcall_pid = &upcall_pid;
         error = dpif_linux_vport_transact(&request, &reply, &buf);
 
@@ -488,12 +484,11 @@ dpif_linux_get_max_ports(const struct dpif *dpif OVS_UNUSED)
 }
 
 static uint32_t
-dpif_linux_port_get_pid__(const struct dpif *dpif_, uint16_t port_no,
-                          enum dpif_upcall_type upcall_type)
+dpif_linux_port_get_pid(const struct dpif *dpif_, uint16_t port_no)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
 
-    if (!(dpif->listen_mask & (1u << upcall_type))) {
+    if (dpif->epoll_fd < 0) {
         return 0;
     } else {
         int idx = port_no & (N_UPCALL_SOCKS - 1);
@@ -501,12 +496,6 @@ dpif_linux_port_get_pid__(const struct dpif *dpif_, uint16_t port_no,
     }
 }
 
-static uint32_t
-dpif_linux_port_get_pid(const struct dpif *dpif, uint16_t port_no)
-{
-    return dpif_linux_port_get_pid__(dpif, port_no, DPIF_UC_ACTION);
-}
-
 static int
 dpif_linux_flow_flush(struct dpif *dpif_)
 {
@@ -959,14 +948,6 @@ dpif_linux_operate(struct dpif *dpif_, union dpif_op **ops, size_t n_ops)
     free(txns);
 }
 
-static int
-dpif_linux_recv_get_mask(const struct dpif *dpif_, int *listen_mask)
-{
-    struct dpif_linux *dpif = dpif_linux_cast(dpif_);
-    *listen_mask = dpif->listen_mask;
-    return 0;
-}
-
 static void
 set_upcall_pids(struct dpif *dpif_)
 {
@@ -976,8 +957,7 @@ set_upcall_pids(struct dpif *dpif_)
     int error;
 
     DPIF_PORT_FOR_EACH (&port, &port_dump, &dpif->dpif) {
-        uint32_t upcall_pid = dpif_linux_port_get_pid__(dpif_, port.port_no,
-                                                        DPIF_UC_MISS);
+        uint32_t upcall_pid = dpif_linux_port_get_pid(dpif_, port.port_no);
         struct dpif_linux_vport vport_request;
 
         dpif_linux_vport_init(&vport_request);
@@ -998,17 +978,17 @@ set_upcall_pids(struct dpif *dpif_)
 }
 
 static int
-dpif_linux_recv_set_mask(struct dpif *dpif_, int listen_mask)
+dpif_linux_recv_set(struct dpif *dpif_, bool enable)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
 
-    if (listen_mask == dpif->listen_mask) {
+    if ((dpif->epoll_fd >= 0) == enable) {
         return 0;
     }
 
-    if (!listen_mask) {
+    if (!enable) {
         destroy_upcall_socks(dpif);
-    } else if (!dpif->listen_mask) {
+    } else {
         int i;
         int error;
 
@@ -1040,7 +1020,6 @@ dpif_linux_recv_set_mask(struct dpif *dpif_, int listen_mask)
         dpif->ready_mask = 0;
     }
 
-    dpif->listen_mask = listen_mask;
     set_upcall_pids(dpif_);
 
     return 0;
@@ -1119,7 +1098,7 @@ dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall)
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
     int read_tries = 0;
 
-    if (!dpif->listen_mask) {
+    if (dpif->epoll_fd < 0) {
        return EAGAIN;
     }
 
@@ -1164,9 +1143,7 @@ dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall)
             }
 
             error = parse_odp_packet(buf, upcall, &dp_ifindex);
-            if (!error
-                && dp_ifindex == dpif->dp_ifindex
-                && dpif->listen_mask & (1u << upcall->type)) {
+            if (!error && dp_ifindex == dpif->dp_ifindex) {
                 return 0;
             }
 
@@ -1185,7 +1162,7 @@ dpif_linux_recv_wait(struct dpif *dpif_)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
 
-    if (!dpif->listen_mask) {
+    if (dpif->epoll_fd < 0) {
        return;
     }
 
@@ -1198,7 +1175,7 @@ dpif_linux_recv_purge(struct dpif *dpif_)
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
     int i;
 
-    if (!dpif->listen_mask) {
+    if (dpif->epoll_fd < 0) {
        return;
     }
 
@@ -1236,8 +1213,7 @@ const struct dpif_class dpif_linux_class = {
     dpif_linux_flow_dump_done,
     dpif_linux_execute,
     dpif_linux_operate,
-    dpif_linux_recv_get_mask,
-    dpif_linux_recv_set_mask,
+    dpif_linux_recv_set,
     dpif_linux_queue_to_priority,
     dpif_linux_recv,
     dpif_linux_recv_wait,

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 6a411fe95febe067159f56d9791239a50014f35a..e2fb816bd6126cb443c3aaaa35b466980298ffd2 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -123,7 +123,6 @@ struct dp_netdev_flow {
 struct dpif_netdev {
     struct dpif dpif;
     struct dp_netdev *dp;
-    int listen_mask;
     unsigned int dp_serial;
 };
 
@@ -178,7 +177,6 @@ create_dpif_netdev(struct dp_netdev *dp)
     dpif = xmalloc(sizeof *dpif);
     dpif_init(&dpif->dpif, dp->class, dp->name, netflow_id >> 8, netflow_id);
     dpif->dp = dp;
-    dpif->listen_mask = 0;
     dpif->dp_serial = dp->serial;
 
     return &dpif->dpif;
@@ -927,18 +925,8 @@ dpif_netdev_execute(struct dpif *dpif,
 }
 
 static int
-dpif_netdev_recv_get_mask(const struct dpif *dpif, int *listen_mask)
+dpif_netdev_recv_set(struct dpif *dpif OVS_UNUSED, bool enable OVS_UNUSED)
 {
-    struct dpif_netdev *dpif_netdev = dpif_netdev_cast(dpif);
-    *listen_mask = dpif_netdev->listen_mask;
-    return 0;
-}
-
-static int
-dpif_netdev_recv_set_mask(struct dpif *dpif, int listen_mask)
-{
-    struct dpif_netdev *dpif_netdev = dpif_netdev_cast(dpif);
-    dpif_netdev->listen_mask = listen_mask;
     return 0;
 }
 
@@ -953,14 +941,12 @@ dpif_netdev_queue_to_priority(const struct dpif *dpif OVS_UNUSED,
 static struct dp_netdev_queue *
 find_nonempty_queue(struct dpif *dpif)
 {
-    struct dpif_netdev *dpif_netdev = dpif_netdev_cast(dpif);
     struct dp_netdev *dp = get_dp_netdev(dpif);
-    int mask = dpif_netdev->listen_mask;
     int i;
 
     for (i = 0; i < N_QUEUES; i++) {
         struct dp_netdev_queue *q = &dp->queues[i];
-        if (q->head != q->tail && mask & (1u << i)) {
+        if (q->head != q->tail) {
             return q;
         }
     }
@@ -1300,8 +1286,7 @@ const struct dpif_class dpif_netdev_class = {
     dpif_netdev_flow_dump_done,
     dpif_netdev_execute,
     NULL,                       /* operate */
-    dpif_netdev_recv_get_mask,
-    dpif_netdev_recv_set_mask,
+    dpif_netdev_recv_set,
     dpif_netdev_queue_to_priority,
     dpif_netdev_recv,
     dpif_netdev_recv_wait,

diff --git a/lib/dpif-provider.h b/lib/dpif-provider.h
index 429cc9d73e279318ee29974397cf031dc3a2ef8c..916b4612ae18b83ee66a030fbc642d9e45fa0bec 100644 (file)
--- a/lib/dpif-provider.h
+++ b/lib/dpif-provider.h
@@ -301,21 +301,11 @@ struct dpif_class {
      * 'dpif' can perform operations in batch faster than individually. */
     void (*operate)(struct dpif *dpif, union dpif_op **ops, size_t n_ops);
 
-    /* Retrieves 'dpif''s "listen mask" into '*listen_mask'.  A 1-bit of value
-     * 2**X set in '*listen_mask' indicates that 'dpif' will receive messages
-     * of the type (from "enum dpif_upcall_type") with value X when its 'recv'
-     * function is called. */
-    int (*recv_get_mask)(const struct dpif *dpif, int *listen_mask);
-
-    /* Sets 'dpif''s "listen mask" to 'listen_mask'.  A 1-bit of value 2**X set
-     * in '*listen_mask' requests that 'dpif' will receive messages of the type
-     * (from "enum dpif_upcall_type") with value X when its 'recv' function is
-     * called.
-     *
-     * Turning DPIF_UC_ACTION off and then back on is allowed to change Netlink
+    /* Enables or disables receiving packets with dpif_recv() for 'dpif'.
+     * Turning packet receive off and then back on is allowed to change Netlink
      * PID assignments (see ->port_get_pid()).  The client is responsible for
      * updating flows as necessary if it does this. */
-    int (*recv_set_mask)(struct dpif *dpif, int listen_mask);
+    int (*recv_set)(struct dpif *dpif, bool enable);
 
     /* Translates OpenFlow queue ID 'queue_id' (in host byte order) into a
      * priority value used for setting packet priority. */
@@ -323,8 +313,8 @@ struct dpif_class {
                              uint32_t *priority);
 
     /* Polls for an upcall from 'dpif'.  If successful, stores the upcall into
-     * '*upcall'.  Only upcalls of the types selected with the set_listen_mask
-     * member function should be received.
+     * '*upcall'.  Should only be called if 'recv_set' has been used to enable
+     * receiving packets from 'dpif'.
      *
      * The caller takes ownership of the data that 'upcall' points to.
      * 'upcall->key' and 'upcall->actions' (if nonnull) point into data owned

diff --git a/lib/dpif.c b/lib/dpif.c
index ecafac1ac2a68ad0a181e8c580dcc2643f4b2b24..4edf54e83075a03c0fd1cdb62ccd974da260cbfa 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1033,54 +1033,24 @@ dpif_upcall_type_to_string(enum dpif_upcall_type type)
     }
 }
 
-static bool OVS_UNUSED
-is_valid_listen_mask(int listen_mask)
-{
-    return !(listen_mask & ~((1u << DPIF_UC_MISS) |
-                             (1u << DPIF_UC_ACTION)));
-}
-
-/* Retrieves 'dpif''s "listen mask" into '*listen_mask'.  A 1-bit of value 2**X
- * set in '*listen_mask' indicates that dpif_recv() will receive messages of
- * the type (from "enum dpif_upcall_type") with value X.  Returns 0 if
- * successful, otherwise a positive errno value. */
-int
-dpif_recv_get_mask(const struct dpif *dpif, int *listen_mask)
-{
-    int error = dpif->dpif_class->recv_get_mask(dpif, listen_mask);
-    if (error) {
-        *listen_mask = 0;
-    }
-    assert(is_valid_listen_mask(*listen_mask));
-    log_operation(dpif, "recv_get_mask", error);
-    return error;
-}
-
-/* Sets 'dpif''s "listen mask" to 'listen_mask'.  A 1-bit of value 2**X set in
- * '*listen_mask' requests that dpif_recv() will receive messages of the type
- * (from "enum dpif_upcall_type") with value X.  Returns 0 if successful,
- * otherwise a positive errno value.
+/* Enables or disables receiving packets with dpif_recv() on 'dpif'.  Returns 0
+ * if successful, otherwise a positive errno value.
  *
- * Turning DPIF_UC_ACTION off and then back on may change the Netlink PID
+ * Turning packet receive off and then back on may change the Netlink PID
  * assignments returned by dpif_port_get_pid().  If the client does this, it
  * must update all of the flows that have OVS_ACTION_ATTR_USERSPACE actions
  * using the new PID assignment. */
 int
-dpif_recv_set_mask(struct dpif *dpif, int listen_mask)
+dpif_recv_set(struct dpif *dpif, bool enable)
 {
-    int error;
-
-    assert(is_valid_listen_mask(listen_mask));
-
-    error = dpif->dpif_class->recv_set_mask(dpif, listen_mask);
-    log_operation(dpif, "recv_set_mask", error);
+    int error = dpif->dpif_class->recv_set(dpif, enable);
+    log_operation(dpif, "recv_set", error);
     return error;
 }
 
 /* Polls for an upcall from 'dpif'.  If successful, stores the upcall into
- * '*upcall'.  Only upcalls of the types selected with dpif_recv_set_mask()
- * member function will ordinarily be received (but if a message type is
- * enabled and then later disabled, some stragglers might pop up).
+ * '*upcall'.  Should only be called if dpif_recv_set() has been used to enable
+ * receiving packets on 'dpif'.
  *
  * The caller takes ownership of the data that 'upcall' points to.
  * 'upcall->key' and 'upcall->actions' (if nonnull) point into data owned by

diff --git a/lib/dpif.h b/lib/dpif.h
index 0ff2389ddee8513bcef42669912f3eea68871e2a..f2a9c48991063779f60b1318b209124db23c8252 100644 (file)
--- a/lib/dpif.h
+++ b/lib/dpif.h
@@ -245,8 +245,7 @@ struct dpif_upcall {
     uint64_t userdata;          /* Argument to OVS_ACTION_ATTR_USERSPACE. */
 };
 
-int dpif_recv_get_mask(const struct dpif *, int *listen_mask);
-int dpif_recv_set_mask(struct dpif *, int listen_mask);
+int dpif_recv_set(struct dpif *, bool enable);
 int dpif_recv(struct dpif *, struct dpif_upcall *);
 void dpif_recv_purge(struct dpif *);
 void dpif_recv_wait(struct dpif *);

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 15532e02ce98975c678e4d0ed2cb6f09d52dce5f..6ecf71b8c04507d1090ba0821a714cc43b89e06b 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -638,9 +638,7 @@ construct(struct ofproto *ofproto_, int *n_tablesp)
     dpif_flow_flush(ofproto->dpif);
     dpif_recv_purge(ofproto->dpif);
 
-    error = dpif_recv_set_mask(ofproto->dpif,
-                               ((1u << DPIF_UC_MISS) |
-                                (1u << DPIF_UC_ACTION)));
+    error = dpif_recv_set(ofproto->dpif, true);
     if (error) {
         VLOG_ERR("failed to listen on datapath %s: %s", name, strerror(error));
         dpif_close(ofproto->dpif);