On Tue, Oct 11, 2011 at 11:08:26AM -0700, Jesse Gross wrote: > On Wed, Oct 5, 2011 at 11:27 AM, Ben Pfaff <[email protected]> wrote: > > --- > > ??lib/dpif-linux.c | ?? 10 +++++----- > > ??1 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c > > index 4ddd464..79e84d6 100644 > > --- a/lib/dpif-linux.c > > +++ b/lib/dpif-linux.c > > @@ -102,9 +102,9 @@ struct dpif_linux_flow { > > > > ?? ?? /* Attributes. > > ?? ?? ??* > > - ?? ?? * The 'stats' and 'used' members point to 64-bit data that might > > only be > > - ?? ?? * aligned on 32-bit boundaries, so get_unaligned_u64() should be > > used to > > - ?? ?? * access their values. > > + ?? ?? * The 'stats' member points to 64-bit data that might only be > > aligned on > > + ?? ?? * 32-bit boundaries, so get_unaligned_u64() should be used to > > access its > > + ?? ?? * values. > > This looks good. However, when reviewing this I noticed that we seem > to access vport stats in an unsafe manner since they are also 64-bit > stats aligned on 32-bit boundaries.
Oops. Here's a fix. --8<--------------------------cut here-------------------------->8-- From: Ben Pfaff <[email protected]> Date: Tue, 11 Oct 2011 12:24:41 -0700 Subject: [PATCH] dpif-linux: Avoid unaligned accesses to vport stats sent by the datapath. Reported-by: Jesse Gross <[email protected]> --- lib/dpif-linux.h | 6 +++++- lib/netdev-vport.c | 33 ++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/dpif-linux.h b/lib/dpif-linux.h index c72ea88..a9f8bfe 100644 --- a/lib/dpif-linux.h +++ b/lib/dpif-linux.h @@ -32,7 +32,11 @@ struct dpif_linux_vport { uint32_t port_no; /* UINT32_MAX if unknown. */ enum ovs_vport_type type; - /* Attributes. */ + /* Attributes. + * + * The 'stats' member points to 64-bit data that might only be aligned on + * 32-bit boundaries, so use get_unaligned_u64() to access its values. + */ const char *name; /* OVS_VPORT_ATTR_NAME. */ const uint32_t *upcall_pid; /* OVS_VPORT_ATTR_UPCALL_PID. */ const struct ovs_vport_stats *stats; /* OVS_VPORT_ATTR_STATS. */ diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 06ec8fb..70cdf29 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -44,6 +44,7 @@ #include "route-table.h" #include "shash.h" #include "socket-util.h" +#include "unaligned.h" #include "vlog.h" VLOG_DEFINE_THIS_MODULE(netdev_vport); @@ -361,22 +362,21 @@ netdev_vport_get_etheraddr(const struct netdev *netdev, return error; } -#define COPY_OVS_STATS \ - dst->rx_packets = src->rx_packets; \ - dst->tx_packets = src->tx_packets; \ - dst->rx_bytes = src->rx_bytes; \ - dst->tx_bytes = src->tx_bytes; \ - dst->rx_errors = src->rx_errors; \ - dst->tx_errors = src->tx_errors; \ - dst->rx_dropped = src->rx_dropped; \ - dst->tx_dropped = src->tx_dropped; - -/* Copies 'src' into 'dst', performing format conversion in the process. */ +/* Copies 'src' into 'dst', performing format conversion in the process. + * + * 'src' is allowed to be misaligned. */ static void netdev_stats_from_ovs_vport_stats(struct netdev_stats *dst, const struct ovs_vport_stats *src) { - COPY_OVS_STATS + 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->multicast = 0; dst->collisions = 0; dst->rx_length_errors = 0; @@ -397,7 +397,14 @@ static void netdev_stats_to_ovs_vport_stats(struct ovs_vport_stats *dst, const struct netdev_stats *src) { - COPY_OVS_STATS + dst->rx_packets = src->rx_packets; + dst->tx_packets = src->tx_packets; + dst->rx_bytes = src->rx_bytes; + dst->tx_bytes = src->tx_bytes; + dst->rx_errors = src->rx_errors; + dst->tx_errors = src->tx_errors; + dst->rx_dropped = src->rx_dropped; + dst->tx_dropped = src->tx_dropped; } int -- 1.7.4.4 _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
