[ovs-dev] [PATCH 3/3] odp-netlink-internal.h: Use ovs_32aligned_u64.
Ben Pfaff
blp at nicira.com
Fri Jun 13 22:28:45 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. Now that the header file for Open vSwitch datapath Netlink
definitions has diverged somewhat from the upstream Linux kernel version,
it makes sense to use ovs_32aligned_u64 locally to make it harder to
accidentally access a misaligned 64-bit member.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
datapath/linux/compat/include/linux/openvswitch.h | 2 +-
include/odp-netlink-internal.h | 54 +++++++++++----------
lib/dpif-linux.c | 43 ++++++++--------
lib/netdev-linux.c | 32 ++++++------
4 files changed, 69 insertions(+), 62 deletions(-)
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index d912884..9c2d9db 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -23,7 +23,7 @@
typedef __be16 ovs_be16;
typedef __be32 ovs_be32;
-typedef __be64 ovs_be64;
+typedef __u64 ovs_32aligned_u64;
#define ODP_NETLINK_INTERNAL_OK
#include "odp-netlink-internal.h"
diff --git a/include/odp-netlink-internal.h b/include/odp-netlink-internal.h
index 754ecaf..b87282c 100644
--- a/include/odp-netlink-internal.h
+++ b/include/odp-netlink-internal.h
@@ -55,13 +55,17 @@
* This header is not self-contained. It must be "#include"d with the
* following types already defined:
*
- * - uint8_t, uint16_t, uint32_t, uint64_t: Unsigned integer types in
- * native byte order.
+ * - uint8_t, uint16_t, uint32_t: Unsigned integer types in native byte
+ * order.
*
- * - ovs_be16, ovs_be32, ovs_be64: Unsigned integer types in big-endian
- * byte order. (These will ordinarily typedefs to the corresponding
- * native types. Separate type names enable static checkers such as
- * "sparse" to report misuses.)
+ * - ovs_be16, ovs_be32: Unsigned integer types in big-endian byte order.
+ * (These will ordinarily typedefs to the corresponding native types.
+ * Separate type names enable static checkers such as "sparse" to report
+ * misuses.)
+ *
+ * - ovs_32aligned_u64: An unsigned 64-bit integer type. Data in Netlink,
+ * even 64-bit data, may only be aligned on a 32-bit boundary, so this
+ * type name emphasizes that.
*/
#ifndef ODP_NETLINK_INTERNAL_OK
@@ -137,29 +141,29 @@ enum ovs_datapath_attr {
/* All 64-bit integers within Netlink messages are 4-byte aligned only. */
struct ovs_dp_stats {
- uint64_t n_hit; /* Number of flow table matches. */
- uint64_t n_missed; /* Number of flow table misses. */
- uint64_t n_lost; /* Number of misses not sent to userspace. */
- uint64_t n_flows; /* Number of flows present */
+ ovs_32aligned_u64 n_hit; /* Number of flow table matches. */
+ ovs_32aligned_u64 n_missed; /* Number of flow table misses. */
+ ovs_32aligned_u64 n_lost; /* Number of misses not sent to userspace. */
+ ovs_32aligned_u64 n_flows; /* Number of flows present */
};
struct ovs_dp_megaflow_stats {
- uint64_t n_mask_hit; /* Number of masks used for flow lookups. */
- uint32_t n_masks; /* Number of masks for the datapath. */
- uint32_t pad0; /* Pad for future expension. */
- uint64_t pad1; /* Pad for future expension. */
- uint64_t pad2; /* Pad for future expension. */
+ ovs_32aligned_u64 n_mask_hit; /* # of masks used for flow lookups. */
+ uint32_t n_masks; /* Number of masks for the datapath. */
+ uint32_t pad0; /* Pad for future expension. */
+ ovs_32aligned_u64 pad1; /* Pad for future expension. */
+ ovs_32aligned_u64 pad2; /* Pad for future expension. */
};
struct ovs_vport_stats {
- uint64_t rx_packets; /* total packets received */
- uint64_t tx_packets; /* total packets transmitted */
- uint64_t rx_bytes; /* total bytes received */
- uint64_t tx_bytes; /* total bytes transmitted */
- uint64_t rx_errors; /* bad packets received */
- uint64_t tx_errors; /* packet transmit problems */
- uint64_t rx_dropped; /* no space in linux buffers */
- uint64_t tx_dropped; /* no space available in linux */
+ ovs_32aligned_u64 rx_packets; /* total packets received */
+ ovs_32aligned_u64 tx_packets; /* total packets transmitted */
+ ovs_32aligned_u64 rx_bytes; /* total bytes received */
+ ovs_32aligned_u64 tx_bytes; /* total bytes transmitted */
+ ovs_32aligned_u64 rx_errors; /* bad packets received */
+ ovs_32aligned_u64 tx_errors; /* packet transmit problems */
+ ovs_32aligned_u64 rx_dropped; /* no space in linux buffers */
+ ovs_32aligned_u64 tx_dropped; /* no space available in linux */
};
/* Allow last Netlink attribute to be unaligned */
@@ -317,8 +321,8 @@ enum ovs_flow_cmd {
};
struct ovs_flow_stats {
- uint64_t n_packets; /* Number of matched packets. */
- uint64_t n_bytes; /* Number of matched bytes. */
+ ovs_32aligned_u64 n_packets; /* Number of matched packets. */
+ ovs_32aligned_u64 n_bytes; /* Number of matched bytes. */
};
enum ovs_key_attr {
diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index afe9340..f179474 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;
@@ -2186,17 +2197,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;
@@ -2235,8 +2240,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
@@ -2456,8 +2459,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 840022d..5587911 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1480,14 +1480,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;
@@ -1682,14 +1682,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.7.10.4
More information about the dev
mailing list