diff options
author | Ben Pfaff <blp@nicira.com> | 2013-08-15 10:47:39 -0700 |
---|---|---|
committer | Ben Pfaff <blp@nicira.com> | 2013-08-27 22:06:02 -0700 |
commit | 7c457c334539957702a5f09f6fd893975bcef568 (patch) | |
tree | 01554efbd844a23b122d3c64b5c38028f324316e /lib/packets.h | |
parent | a69553be4250a8a40d603d8f3123ec532d5fb390 (diff) |
packets: Do not assume that IPv4, TCP, or ARP headers are 32-bit aligned.
Ethernet headers are 14 bytes long, so when the beginning of such a header
is 32-bit aligned, the following data is misaligned. The usual trick to
fix that is to start the Ethernet header on an odd-numbered 16-bit
boundary. That trick works OK for Open vSwitch, but there are two
problems:
- OVS doesn't use that trick everywhere. Maybe it should, but it's
difficult to make sure that it does consistently because the CPUs
most commonly used with OVS don't care about misalignment, so we
only find problems when porting.
- Some protocols (GRE, VXLAN) don't use that trick, so in such a case
one can properly align the inner or outer L3/L4/L7 but not both. (OVS
userspace doesn't directly deal with such protocols yet, so this is
just future-proofing.)
- OpenFlow uses the alignment trick in a few places but not all of them.
This commit starts the adoption of what I hope will be a more robust way
to avoid misalignment problems and the resulting bus errors on RISC
architectures. Instead of trying to ensure that 32-bit quantities are
always aligned, we always read them as if they were misaligned. To ensure
that they are read this way, we change their types from 32-bit types to
pairs of 16-bit types. (I don't know of any protocols that offset the
next header by an odd number of bytes, so a 16-bit alignment assumption
seems OK.)
The same would be necessary for 64-bit types in protocol headers, but we
don't yet have any protocol definitions with 64-bit types.
IPv6 protocol headers need the same treatment, but for those we rely on
structs provided by system headers, so I'll leave them for an upcoming
patch.
Signed-off-by: Ben Pfaff <blp@nicira.com>
Diffstat (limited to 'lib/packets.h')
-rw-r--r-- | lib/packets.h | 17 |
1 files changed, 8 insertions, 9 deletions
diff --git a/lib/packets.h b/lib/packets.h index 2108beff..32f3b2da 100644 --- a/lib/packets.h +++ b/lib/packets.h @@ -446,8 +446,8 @@ struct ip_header { uint8_t ip_ttl; uint8_t ip_proto; ovs_be16 ip_csum; - ovs_be32 ip_src; - ovs_be32 ip_dst; + ovs_16aligned_be32 ip_src; + ovs_16aligned_be32 ip_dst; }; BUILD_ASSERT_DECL(IP_HEADER_LEN == sizeof(struct ip_header)); @@ -465,7 +465,7 @@ struct icmp_header { ovs_be16 empty; ovs_be16 mtu; } frag; - ovs_be32 gateway; + ovs_16aligned_be32 gateway; } icmp_fields; uint8_t icmp_data[0]; }; @@ -504,8 +504,8 @@ BUILD_ASSERT_DECL(UDP_HEADER_LEN == sizeof(struct udp_header)); struct tcp_header { ovs_be16 tcp_src; ovs_be16 tcp_dst; - ovs_be32 tcp_seq; - ovs_be32 tcp_ack; + ovs_16aligned_be32 tcp_seq; + ovs_16aligned_be32 tcp_ack; ovs_be16 tcp_ctl; ovs_be16 tcp_winsz; ovs_be16 tcp_csum; @@ -520,7 +520,6 @@ BUILD_ASSERT_DECL(TCP_HEADER_LEN == sizeof(struct tcp_header)); #define ARP_OP_RARP 3 #define ARP_ETH_HEADER_LEN 28 -OVS_PACKED( struct arp_eth_header { /* Generic members. */ ovs_be16 ar_hrd; /* Hardware type. */ @@ -531,10 +530,10 @@ struct arp_eth_header { /* Ethernet+IPv4 specific members. */ uint8_t ar_sha[ETH_ADDR_LEN]; /* Sender hardware address. */ - ovs_be32 ar_spa; /* Sender protocol address. */ + ovs_16aligned_be32 ar_spa; /* Sender protocol address. */ uint8_t ar_tha[ETH_ADDR_LEN]; /* Target hardware address. */ - ovs_be32 ar_tpa; /* Target protocol address. */ -}); + ovs_16aligned_be32 ar_tpa; /* Target protocol address. */ +}; BUILD_ASSERT_DECL(ARP_ETH_HEADER_LEN == sizeof(struct arp_eth_header)); /* The IPv6 flow label is in the lower 20 bits of the first 32-bit word. */ |