[ovs-dev] [PATCH] dpif: Simplify the "listen mask" concept.
Ben Pfaff
blp at nicira.com
Wed Nov 30 23:08:33 UTC 2011
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
---
lib/dpif-linux.c | 52 ++++++++++++-----------------------------------
lib/dpif-netdev.c | 21 ++----------------
lib/dpif-provider.h | 20 ++++-------------
lib/dpif.c | 46 +++++++-----------------------------------
lib/dpif.h | 3 +-
ofproto/ofproto-dpif.c | 4 +--
6 files changed, 32 insertions(+), 114 deletions(-)
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 1f6c2c0..292dd1e 100644
--- 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;
@@ -1039,7 +1019,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;
@@ -1118,7 +1097,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;
}
@@ -1163,9 +1142,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;
}
@@ -1184,7 +1161,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;
}
@@ -1197,7 +1174,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;
}
@@ -1235,8 +1212,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 eb10134..c004704 100644
--- 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;
@@ -893,18 +891,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;
}
@@ -919,14 +907,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;
}
}
@@ -1354,8 +1340,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 429cc9d..916b461 100644
--- 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 9cbf877..ca875b6 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1032,54 +1032,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 0ff2389..f2a9c48 100644
--- 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 14b5447..722ca9a 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -625,9 +625,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);
--
1.7.2.5
More information about the dev
mailing list