[ovs-dev] [PATCH v2 2/2] odp-netlink.h: Use 32-bit aligned 64-bit types.

Ben Pfaff blp at nicira.com
Sat Aug 2 04:36:53 UTC 2014


Open vSwitch userspace uses special types to indicate that a particular
object may not be naturally aligned.  Netlink is one source of such
problems: in Netlink, 64-bit integers are often aligned only on 32-bit
boundaries.  This commit changes the odp-netlink.h that is transformed from
<linux/openvswitch.h> to use these types to make it harder to accidentally
access a misaligned 64-bit member.

Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 build-aux/extract-odp-netlink-h | 12 ++++++++----
 lib/dpif-linux.c                | 43 ++++++++++++++++++++++-------------------
 lib/netdev-linux.c              | 32 +++++++++++++++---------------
 3 files changed, 47 insertions(+), 40 deletions(-)

diff --git a/build-aux/extract-odp-netlink-h b/build-aux/extract-odp-netlink-h
index f3441c1..7b15225 100755
--- a/build-aux/extract-odp-netlink-h
+++ b/build-aux/extract-odp-netlink-h
@@ -11,18 +11,22 @@
 # Avoid using reserved names in header guards.
 s/_LINUX_OPENVSWITCH_H/ODP_NETLINK_H/
 
-# Transform Linux-specific __u<N> types into C99 uint<N>_t types,
-# and Linux-specific __be<N> into Open vSwitch ovs_be<N>,
+# Transform most Linux-specific __u<N> types into C99 uint<N>_t types,
+# and most Linux-specific __be<N> into Open vSwitch ovs_be<N>,
 # and use the appropriate userspace header.
 s,<linux/types\.h>,"openvswitch/types.h",
-s/__u64/uint64_t/g
 s/__u32/uint32_t/g
 s/__u16/uint16_t/g
 s/__u8/uint8_t/g
-s/__be64/ovs_be64/g
 s/__be32/ovs_be32/g
 s/__be16/ovs_be16/g
 
+# Transform 64-bit Linux-specific types into Open vSwitch specialized
+# types for 64-bit numbers that might only be aligned on a 32-bit
+# boundary.
+s/__u64/ovs_32aligned_u64/g
+s/__be64/ovs_32aligned_be64/g
+
 # Use OVS's own ETH_ADDR_LEN instead of Linux-specific ETH_ALEN.
 s,<linux/if_ether\.h>,"packets.h",
 s/ETH_ALEN/ETH_ADDR_LEN/
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index b98413d..aa01cef 100644
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -73,8 +73,8 @@ struct dpif_linux_dp {
     const char *name;                  /* OVS_DP_ATTR_NAME. */
     const uint32_t *upcall_pid;        /* OVS_DP_ATTR_UPCALL_PID. */
     uint32_t user_features;            /* OVS_DP_ATTR_USER_FEATURES */
-    struct ovs_dp_stats stats;         /* OVS_DP_ATTR_STATS. */
-    struct ovs_dp_megaflow_stats megaflow_stats;
+    const struct ovs_dp_stats *stats;  /* OVS_DP_ATTR_STATS. */
+    const struct ovs_dp_megaflow_stats *megaflow_stats;
                                        /* OVS_DP_ATTR_MEGAFLOW_STATS.*/
 };
 
@@ -554,12 +554,23 @@ dpif_linux_get_stats(const struct dpif *dpif_, struct dpif_dp_stats *stats)
 
     error = dpif_linux_dp_get(dpif_, &dp, &buf);
     if (!error) {
-        stats->n_hit    = dp.stats.n_hit;
-        stats->n_missed = dp.stats.n_missed;
-        stats->n_lost   = dp.stats.n_lost;
-        stats->n_flows  = dp.stats.n_flows;
-        stats->n_masks  = dp.megaflow_stats.n_masks;
-        stats->n_mask_hit  = dp.megaflow_stats.n_mask_hit;
+        memset(stats, 0, sizeof *stats);
+
+        if (dp.stats) {
+            stats->n_hit    = get_32aligned_u64(&dp.stats->n_hit);
+            stats->n_missed = get_32aligned_u64(&dp.stats->n_missed);
+            stats->n_lost   = get_32aligned_u64(&dp.stats->n_lost);
+            stats->n_flows  = get_32aligned_u64(&dp.stats->n_flows);
+        }
+
+        if (dp.megaflow_stats) {
+            stats->n_masks = dp.megaflow_stats->n_masks;
+            stats->n_mask_hit = get_32aligned_u64(
+                &dp.megaflow_stats->n_mask_hit);
+        } else {
+            stats->n_masks = UINT32_MAX;
+            stats->n_mask_hit = UINT64_MAX;
+        }
         ofpbuf_delete(buf);
     }
     return error;
@@ -2203,17 +2214,11 @@ dpif_linux_dp_from_ofpbuf(struct dpif_linux_dp *dp, const struct ofpbuf *buf)
     dp->dp_ifindex = ovs_header->dp_ifindex;
     dp->name = nl_attr_get_string(a[OVS_DP_ATTR_NAME]);
     if (a[OVS_DP_ATTR_STATS]) {
-        /* Can't use structure assignment because Netlink doesn't ensure
-         * sufficient alignment for 64-bit members. */
-        memcpy(&dp->stats, nl_attr_get(a[OVS_DP_ATTR_STATS]),
-               sizeof dp->stats);
+        dp->stats = nl_attr_get(a[OVS_DP_ATTR_STATS]);
     }
 
     if (a[OVS_DP_ATTR_MEGAFLOW_STATS]) {
-        /* Can't use structure assignment because Netlink doesn't ensure
-         * sufficient alignment for 64-bit members. */
-        memcpy(&dp->megaflow_stats, nl_attr_get(a[OVS_DP_ATTR_MEGAFLOW_STATS]),
-               sizeof dp->megaflow_stats);
+        dp->megaflow_stats = nl_attr_get(a[OVS_DP_ATTR_MEGAFLOW_STATS]);
     }
 
     return 0;
@@ -2252,8 +2257,6 @@ static void
 dpif_linux_dp_init(struct dpif_linux_dp *dp)
 {
     memset(dp, 0, sizeof *dp);
-    dp->megaflow_stats.n_masks = UINT32_MAX;
-    dp->megaflow_stats.n_mask_hit = UINT64_MAX;
 }
 
 static void
@@ -2473,8 +2476,8 @@ dpif_linux_flow_get_stats(const struct dpif_linux_flow *flow,
                           struct dpif_flow_stats *stats)
 {
     if (flow->stats) {
-        stats->n_packets = get_unaligned_u64(&flow->stats->n_packets);
-        stats->n_bytes = get_unaligned_u64(&flow->stats->n_bytes);
+        stats->n_packets = get_32aligned_u64(&flow->stats->n_packets);
+        stats->n_bytes = get_32aligned_u64(&flow->stats->n_bytes);
     } else {
         stats->n_packets = 0;
         stats->n_bytes = 0;
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 1780639..e50392a 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1499,14 +1499,14 @@ static void
 netdev_stats_from_ovs_vport_stats(struct netdev_stats *dst,
                                   const struct ovs_vport_stats *src)
 {
-    dst->rx_packets = get_unaligned_u64(&src->rx_packets);
-    dst->tx_packets = get_unaligned_u64(&src->tx_packets);
-    dst->rx_bytes = get_unaligned_u64(&src->rx_bytes);
-    dst->tx_bytes = get_unaligned_u64(&src->tx_bytes);
-    dst->rx_errors = get_unaligned_u64(&src->rx_errors);
-    dst->tx_errors = get_unaligned_u64(&src->tx_errors);
-    dst->rx_dropped = get_unaligned_u64(&src->rx_dropped);
-    dst->tx_dropped = get_unaligned_u64(&src->tx_dropped);
+    dst->rx_packets = get_32aligned_u64(&src->rx_packets);
+    dst->tx_packets = get_32aligned_u64(&src->tx_packets);
+    dst->rx_bytes = get_32aligned_u64(&src->rx_bytes);
+    dst->tx_bytes = get_32aligned_u64(&src->tx_bytes);
+    dst->rx_errors = get_32aligned_u64(&src->rx_errors);
+    dst->tx_errors = get_32aligned_u64(&src->tx_errors);
+    dst->rx_dropped = get_32aligned_u64(&src->rx_dropped);
+    dst->tx_dropped = get_32aligned_u64(&src->tx_dropped);
     dst->multicast = 0;
     dst->collisions = 0;
     dst->rx_length_errors = 0;
@@ -1701,14 +1701,14 @@ netdev_internal_set_stats(struct netdev *netdev,
     struct dpif_linux_vport vport;
     int err;
 
-    vport_stats.rx_packets = stats->rx_packets;
-    vport_stats.tx_packets = stats->tx_packets;
-    vport_stats.rx_bytes = stats->rx_bytes;
-    vport_stats.tx_bytes = stats->tx_bytes;
-    vport_stats.rx_errors = stats->rx_errors;
-    vport_stats.tx_errors = stats->tx_errors;
-    vport_stats.rx_dropped = stats->rx_dropped;
-    vport_stats.tx_dropped = stats->tx_dropped;
+    put_32aligned_u64(&vport_stats.rx_packets, stats->rx_packets);
+    put_32aligned_u64(&vport_stats.tx_packets, stats->tx_packets);
+    put_32aligned_u64(&vport_stats.rx_bytes, stats->rx_bytes);
+    put_32aligned_u64(&vport_stats.tx_bytes, stats->tx_bytes);
+    put_32aligned_u64(&vport_stats.rx_errors, stats->rx_errors);
+    put_32aligned_u64(&vport_stats.tx_errors, stats->tx_errors);
+    put_32aligned_u64(&vport_stats.rx_dropped, stats->rx_dropped);
+    put_32aligned_u64(&vport_stats.tx_dropped, stats->tx_dropped);
 
     dpif_linux_vport_init(&vport);
     vport.cmd = OVS_VPORT_CMD_SET;
-- 
1.9.1




More information about the dev mailing list