[ovs-dev] [loss-report 3/4] dpif-linux: Slightly refactor internal data structures.

Ben Pfaff blp at nicira.com
Fri May 25 21:36:36 UTC 2012


An initial attempt also replaced the 'uint32_t ready_mask' in struct
dpif_linux by a 'bool ready' in each struct dpif_channel, but I wasn't
happy with the result (the ready_mask bitmap works out really well) and so
I dropped that part.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/dpif-linux.c |   67 +++++++++++++++++++++++++++++-------------------------
 1 files changed, 36 insertions(+), 31 deletions(-)

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index 9d84edd..4de44a8 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -61,10 +61,10 @@
 VLOG_DEFINE_THIS_MODULE(dpif_linux);
 enum { MAX_PORTS = USHRT_MAX };
 
-enum { N_UPCALL_SOCKS = 17 };
-BUILD_ASSERT_DECL(IS_POW2(N_UPCALL_SOCKS - 1));
-BUILD_ASSERT_DECL(N_UPCALL_SOCKS > 1);
-BUILD_ASSERT_DECL(N_UPCALL_SOCKS <= 32); /* We use a 32-bit word as a mask. */
+enum { N_CHANNELS = 17 };
+BUILD_ASSERT_DECL(IS_POW2(N_CHANNELS - 1));
+BUILD_ASSERT_DECL(N_CHANNELS > 1);
+BUILD_ASSERT_DECL(N_CHANNELS <= 32); /* We use a 32-bit word as a mask. */
 
 /* This ethtool flag was introduced in Linux 2.6.24, so it might be
  * missing if we have old headers. */
@@ -130,15 +130,19 @@ static int dpif_linux_flow_transact(struct dpif_linux_flow *request,
 static void dpif_linux_flow_get_stats(const struct dpif_linux_flow *,
                                       struct dpif_flow_stats *);
 
+struct dpif_channel {
+    struct nl_sock *sock;
+};
+
 /* Datapath interface for the openvswitch Linux kernel module. */
 struct dpif_linux {
     struct dpif dpif;
     int dp_ifindex;
 
     /* Upcall messages. */
-    struct nl_sock *upcall_socks[N_UPCALL_SOCKS];
+    struct dpif_channel channels[N_CHANNELS];
     uint32_t ready_mask;        /* 1-bit for each sock with unread messages. */
-    int epoll_fd;               /* epoll fd that includes the upcall socks. */
+    int epoll_fd;               /* epoll fd that includes channel socks. */
 
     /* Change notification. */
     struct sset changed_ports;  /* Ports that have changed. */
@@ -256,17 +260,17 @@ open_dpif(const struct dpif_linux_dp *dp, struct dpif **dpifp)
 }
 
 static void
-destroy_upcall_socks(struct dpif_linux *dpif)
+destroy_channels(struct dpif_linux *dpif)
 {
-    int i;
+    struct dpif_channel *ch;
 
     if (dpif->epoll_fd >= 0) {
         close(dpif->epoll_fd);
         dpif->epoll_fd = -1;
     }
-    for (i = 0; i < N_UPCALL_SOCKS; i++) {
-        nl_sock_destroy(dpif->upcall_socks[i]);
-        dpif->upcall_socks[i] = NULL;
+    for (ch = dpif->channels; ch < &dpif->channels[N_CHANNELS]; ch++) {
+        nl_sock_destroy(ch->sock);
+        ch->sock = NULL;
     }
 }
 
@@ -276,7 +280,7 @@ dpif_linux_close(struct dpif *dpif_)
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
 
     nln_notifier_destroy(dpif->port_notifier);
-    destroy_upcall_socks(dpif);
+    destroy_channels(dpif);
     sset_destroy(&dpif->changed_ports);
     free(dpif);
 }
@@ -465,9 +469,9 @@ dpif_linux_port_get_pid(const struct dpif *dpif_, uint16_t port_no)
         int idx;
 
         idx = (port_no != UINT16_MAX
-               ? 1 + (port_no & (N_UPCALL_SOCKS - 2))
+               ? 1 + (port_no & (N_CHANNELS - 2))
                : 0);
-        return nl_sock_pid(dpif->upcall_socks[idx]);
+        return nl_sock_pid(dpif->channels[idx].sock);
     }
 }
 
@@ -989,32 +993,33 @@ dpif_linux_recv_set(struct dpif *dpif_, bool enable)
     }
 
     if (!enable) {
-        destroy_upcall_socks(dpif);
+        destroy_channels(dpif);
     } else {
-        int i;
+        struct dpif_channel *ch;
         int error;
 
-        dpif->epoll_fd = epoll_create(N_UPCALL_SOCKS);
+        dpif->epoll_fd = epoll_create(N_CHANNELS);
         if (dpif->epoll_fd < 0) {
             return errno;
         }
 
-        for (i = 0; i < N_UPCALL_SOCKS; i++) {
+        for (ch = dpif->channels; ch < &dpif->channels[N_CHANNELS]; ch++) {
+            int indx = ch - dpif->channels;
             struct epoll_event event;
 
-            error = nl_sock_create(NETLINK_GENERIC, &dpif->upcall_socks[i]);
+            error = nl_sock_create(NETLINK_GENERIC, &ch->sock);
             if (error) {
-                destroy_upcall_socks(dpif);
+                destroy_channels(dpif);
                 return error;
             }
 
             memset(&event, 0, sizeof event);
             event.events = EPOLLIN;
-            event.data.u32 = i;
-            if (epoll_ctl(dpif->epoll_fd, EPOLL_CTL_ADD,
-                          nl_sock_fd(dpif->upcall_socks[i]), &event) < 0) {
+            event.data.u32 = indx;
+            if (epoll_ctl(dpif->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(ch->sock),
+                          &event) < 0) {
                 error = errno;
-                destroy_upcall_socks(dpif);
+                destroy_channels(dpif);
                 return error;
             }
         }
@@ -1106,12 +1111,12 @@ dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall,
     }
 
     if (!dpif->ready_mask) {
-        struct epoll_event events[N_UPCALL_SOCKS];
+        struct epoll_event events[N_CHANNELS];
         int retval;
         int i;
 
         do {
-            retval = epoll_wait(dpif->epoll_fd, events, N_UPCALL_SOCKS, 0);
+            retval = epoll_wait(dpif->epoll_fd, events, N_CHANNELS, 0);
         } while (retval < 0 && errno == EINTR);
         if (retval < 0) {
             static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
@@ -1125,7 +1130,7 @@ dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall,
 
     while (dpif->ready_mask) {
         int indx = ffs(dpif->ready_mask) - 1;
-        struct nl_sock *upcall_sock = dpif->upcall_socks[indx];
+        struct dpif_channel *ch = &dpif->channels[indx];
 
         dpif->ready_mask &= ~(1u << indx);
 
@@ -1137,7 +1142,7 @@ dpif_linux_recv(struct dpif *dpif_, struct dpif_upcall *upcall,
                 return EAGAIN;
             }
 
-            error = nl_sock_recv(upcall_sock, buf, false);
+            error = nl_sock_recv(ch->sock, buf, false);
             if (error) {
                 if (error == ENOBUFS) {
                     /* ENOBUFS typically means that we've received so many
@@ -1183,14 +1188,14 @@ static void
 dpif_linux_recv_purge(struct dpif *dpif_)
 {
     struct dpif_linux *dpif = dpif_linux_cast(dpif_);
-    int i;
+    struct dpif_channel *ch;
 
     if (dpif->epoll_fd < 0) {
        return;
     }
 
-    for (i = 0; i < N_UPCALL_SOCKS; i++) {
-        nl_sock_drain(dpif->upcall_socks[i]);
+    for (ch = dpif->channels; ch < &dpif->channels[N_CHANNELS]; ch++) {
+        nl_sock_drain(ch->sock);
     }
 }
 
-- 
1.7.2.5




More information about the dev mailing list