From d84d4b88d26e3f37ce24f1d3eebe0d70ef264f73 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Fri, 6 May 2011 11:27:05 -0700 Subject: [PATCH] Fix incorrect byte order annotations. These are not actual bugs, just deceptive use of the wrong function or type. Found by sparse. --- lib/bond.c | 2 +- lib/dpif-netdev.c | 8 ++++---- lib/flow.c | 2 +- lib/flow.h | 2 +- lib/netdev-vport.c | 2 +- lib/ofp-print.c | 2 +- lib/packets.h | 4 ++-- lib/rconn.c | 4 ++-- ofproto/netflow.c | 2 +- tests/test-csum.c | 43 ++++++++++++++++++++++--------------------- tests/test-vconn.c | 6 +++--- 11 files changed, 39 insertions(+), 38 deletions(-) diff --git a/lib/bond.c b/lib/bond.c index 2b540523..929d6823 100644 --- a/lib/bond.c +++ b/lib/bond.c @@ -1384,7 +1384,7 @@ static unsigned int bond_hash_tcp(const struct flow *flow, uint16_t vlan, uint32_t basis) { struct flow hash_flow = *flow; - hash_flow.vlan_tci = vlan; + hash_flow.vlan_tci = htons(vlan); /* The symmetric quality of this hash function is not required, but * flow_hash_symmetric_l4 already exists, and is sufficient for our diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 5efc8697..cc898fb2 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -114,7 +114,7 @@ struct dp_netdev_flow { long long int used; /* Last used time, in monotonic msecs. */ long long int packet_count; /* Number of packets matched. */ long long int byte_count; /* Number of bytes matched. */ - uint16_t tcp_ctl; /* Bitwise-OR of seen tcp_ctl values. */ + ovs_be16 tcp_ctl; /* Bitwise-OR of seen tcp_ctl values. */ /* Actions. */ struct nlattr *actions; @@ -1182,7 +1182,7 @@ dp_netdev_set_nw_addr(struct ofpbuf *packet, const struct flow *key, struct ip_header *nh = packet->l3; ovs_be32 ip = nl_attr_get_be32(a); uint16_t type = nl_attr_type(a); - uint32_t *field; + ovs_be32 *field; field = type == ODP_ACTION_ATTR_SET_NW_SRC ? &nh->ip_src : &nh->ip_dst; if (key->nw_proto == IPPROTO_TCP && packet->l7) { @@ -1193,7 +1193,7 @@ dp_netdev_set_nw_addr(struct ofpbuf *packet, const struct flow *key, if (uh->udp_csum) { uh->udp_csum = recalc_csum32(uh->udp_csum, *field, ip); if (!uh->udp_csum) { - uh->udp_csum = 0xffff; + uh->udp_csum = htons(0xffff); } } } @@ -1226,7 +1226,7 @@ dp_netdev_set_tp_port(struct ofpbuf *packet, const struct flow *key, if (is_ip(packet, key)) { uint16_t type = nl_attr_type(a); ovs_be16 port = nl_attr_get_be16(a); - uint16_t *field; + ovs_be16 *field; if (key->nw_proto == IPPROTO_TCP && packet->l7) { struct tcp_header *th = packet->l4; diff --git a/lib/flow.c b/lib/flow.c index 32157b84..42e80abd 100644 --- a/lib/flow.c +++ b/lib/flow.c @@ -556,7 +556,7 @@ flow_wildcards_is_exact(const struct flow_wildcards *wc) } for (i = 0; i < FLOW_N_REGS; i++) { - if (wc->reg_masks[i] != htonl(UINT32_MAX)) { + if (wc->reg_masks[i] != UINT32_MAX) { return false; } } diff --git a/lib/flow.h b/lib/flow.h index 60229f58..c596f7d8 100644 --- a/lib/flow.h +++ b/lib/flow.h @@ -70,7 +70,7 @@ BUILD_ASSERT_DECL(offsetof(struct flow, nd_target) == FLOW_SIG_SIZE - 16); BUILD_ASSERT_DECL(sizeof(((struct flow *)0)->nd_target) == 16); BUILD_ASSERT_DECL(sizeof(struct flow) == FLOW_SIG_SIZE + FLOW_PAD_SIZE); -int flow_extract(struct ofpbuf *, uint64_t tun_id, uint16_t in_port, +int flow_extract(struct ofpbuf *, ovs_be64 tun_id, uint16_t in_port, struct flow *); void flow_extract_stats(const struct flow *flow, struct ofpbuf *packet, struct dpif_flow_stats *); diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index e11cb2a8..30673236 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -555,7 +555,7 @@ static const char * netdev_vport_get_tnl_iface(const struct netdev *netdev) { struct nlattr *a[ODP_TUNNEL_ATTR_MAX + 1]; - uint32_t route; + ovs_be32 route; struct netdev_dev_vport *ndv; static char name[IFNAMSIZ]; diff --git a/lib/ofp-print.c b/lib/ofp-print.c index 2790130d..25258665 100644 --- a/lib/ofp-print.c +++ b/lib/ofp-print.c @@ -123,7 +123,7 @@ ofp_print_packet_in(struct ds *string, const struct ofp_packet_in *op, data_len = len - offsetof(struct ofp_packet_in, data); ds_put_format(string, " data_len=%zu", data_len); - if (htonl(op->buffer_id) == UINT32_MAX) { + if (op->buffer_id == htonl(UINT32_MAX)) { ds_put_format(string, " (unbuffered)"); if (ntohs(op->total_len) != data_len) ds_put_format(string, " (***total_len != data_len***)"); diff --git a/lib/packets.h b/lib/packets.h index f45c3312..20065ade 100644 --- a/lib/packets.h +++ b/lib/packets.h @@ -328,8 +328,8 @@ BUILD_ASSERT_DECL(UDP_HEADER_LEN == sizeof(struct udp_header)); #define TCP_ACK 0x10 #define TCP_URG 0x20 -#define TCP_FLAGS(tcp_ctl) (htons(tcp_ctl) & 0x003f) -#define TCP_OFFSET(tcp_ctl) (htons(tcp_ctl) >> 12) +#define TCP_FLAGS(tcp_ctl) (ntohs(tcp_ctl) & 0x003f) +#define TCP_OFFSET(tcp_ctl) (ntohs(tcp_ctl) >> 12) #define TCP_HEADER_LEN 20 struct tcp_header { diff --git a/lib/rconn.c b/lib/rconn.c index 7d0f4ce6..1b69b8f6 100644 --- a/lib/rconn.c +++ b/lib/rconn.c @@ -119,8 +119,8 @@ struct rconn { * * We don't cache the local port, because that changes from one connection * attempt to the next. */ - uint32_t local_ip, remote_ip; - uint16_t remote_port; + ovs_be32 local_ip, remote_ip; + ovs_be16 remote_port; /* Messages sent or received are copied to the monitor connections. */ #define MAX_MONITORS 8 diff --git a/ofproto/netflow.c b/ofproto/netflow.c index c237ef25..381ff8ef 100644 --- a/ofproto/netflow.c +++ b/ofproto/netflow.c @@ -133,7 +133,7 @@ gen_netflow_rec(struct netflow *nf, struct netflow_flow *nf_flow, nf_rec = ofpbuf_put_zeros(&nf->packet, sizeof *nf_rec); nf_rec->src_addr = expired->flow.nw_src; nf_rec->dst_addr = expired->flow.nw_dst; - nf_rec->nexthop = htons(0); + nf_rec->nexthop = htonl(0); if (nf->add_id_to_iface) { uint16_t iface = (nf->engine_id & 0x7f) << 9; nf_rec->input = htons(iface | (expired->flow.in_port & 0x1ff)); diff --git a/tests/test-csum.c b/tests/test-csum.c index af6655e7..81f54b80 100644 --- a/tests/test-csum.c +++ b/tests/test-csum.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2010 Nicira Networks. + * Copyright (c) 2009, 2010, 2011 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -40,7 +40,7 @@ static const struct test_case test_cases[] = { /* RFC 1071 section 3. */ TEST_CASE("\x00\x01\xf2\x03" "\xf4\xf5\xf6\xf7", - (uint16_t) ~0xddf2), + 0xffff - 0xddf2 /* ~0xddf2 */), /* http://www.sbprojects.com/projects/tcpip/theory/theory14.htm */ TEST_CASE("\x45\x00\x00\x28" @@ -99,7 +99,7 @@ test_rfc1624(void) "\x66\x64\xfc\x96\x63\x97\x64\xee\x12\x53\x1d\xa9\x2d\xa9\x55\x55"; /* "...the one's complement sum of all other header octets is 0xCD7A." */ - assert(ntohs(csum(data, sizeof data - 2)) == (uint16_t) ~0xcd7a); + assert(ntohs(csum(data, sizeof data - 2)) == 0xffff - 0xcd7a); /* "...the header checksum would be: @@ -127,7 +127,8 @@ test_rfc1624(void) = ~(0x22D0 + ~0x5555 + 0x3285) = ~0xFFFF = 0x0000" */ - assert(recalc_csum16(0xdd2f, 0x5555, 0x3285) == 0x0000); + assert(recalc_csum16(htons(0xdd2f), htons(0x5555), htons(0x3285)) + == htons(0x0000)); mark('#'); } @@ -139,8 +140,8 @@ main(void) int i; for (tc = test_cases; tc < &test_cases[ARRAY_SIZE(test_cases)]; tc++) { - const uint16_t *data16 = (const uint16_t *) tc->data; - const uint32_t *data32 = (const uint32_t *) tc->data; + const ovs_be16 *data16 = (OVS_FORCE const ovs_be16 *) tc->data; + const ovs_be32 *data32 = (OVS_FORCE const ovs_be32 *) tc->data; uint32_t partial; /* Test csum(). */ @@ -150,7 +151,7 @@ main(void) /* Test csum_add16(). */ partial = 0; for (i = 0; i < tc->size / 2; i++) { - partial = csum_add16(partial, get_unaligned_u16(&data16[i])); + partial = csum_add16(partial, get_unaligned_be16(&data16[i])); } assert(ntohs(csum_finish(partial)) == tc->csum); mark('.'); @@ -158,7 +159,7 @@ main(void) /* Test csum_add32(). */ partial = 0; for (i = 0; i < tc->size / 4; i++) { - partial = csum_add32(partial, get_unaligned_u32(&data32[i])); + partial = csum_add32(partial, get_unaligned_be32(&data32[i])); } assert(ntohs(csum_finish(partial)) == tc->csum); mark('.'); @@ -167,10 +168,10 @@ main(void) partial = 0; for (i = 0; i < tc->size / 4; i++) { if (i % 2) { - partial = csum_add32(partial, get_unaligned_u32(&data32[i])); + partial = csum_add32(partial, get_unaligned_be32(&data32[i])); } else { - uint16_t u0 = get_unaligned_u16(&data16[i * 2]); - uint16_t u1 = get_unaligned_u16(&data16[i * 2 + 1]); + ovs_be16 u0 = get_unaligned_be16(&data16[i * 2]); + ovs_be16 u1 = get_unaligned_be16(&data16[i * 2 + 1]); partial = csum_add16(partial, u0); partial = csum_add16(partial, u1); } @@ -196,18 +197,18 @@ main(void) /* Test recalc_csum16(). */ for (i = 0; i < 32; i++) { - uint16_t old_u16, new_u16; - uint16_t old_csum; - uint16_t data[16]; + ovs_be16 old_u16, new_u16; + ovs_be16 old_csum; + ovs_be16 data[16]; int j, index; for (j = 0; j < ARRAY_SIZE(data); j++) { - data[j] = random_uint32(); + data[j] = (OVS_FORCE ovs_be16) random_uint32(); } old_csum = csum(data, sizeof data); index = random_range(ARRAY_SIZE(data)); old_u16 = data[index]; - new_u16 = data[index] = random_uint32(); + new_u16 = data[index] = (OVS_FORCE ovs_be16) random_uint32(); assert(csum(data, sizeof data) == recalc_csum16(old_csum, old_u16, new_u16)); mark('.'); @@ -216,18 +217,18 @@ main(void) /* Test recalc_csum32(). */ for (i = 0; i < 32; i++) { - uint32_t old_u32, new_u32; - uint16_t old_csum; - uint32_t data[16]; + ovs_be32 old_u32, new_u32; + ovs_be16 old_csum; + ovs_be32 data[16]; int j, index; for (j = 0; j < ARRAY_SIZE(data); j++) { - data[j] = random_uint32(); + data[j] = (OVS_FORCE ovs_be32) random_uint32(); } old_csum = csum(data, sizeof data); index = random_range(ARRAY_SIZE(data)); old_u32 = data[index]; - new_u32 = data[index] = random_uint32(); + new_u32 = data[index] = (OVS_FORCE ovs_be32) random_uint32(); assert(csum(data, sizeof data) == recalc_csum32(old_csum, old_u32, new_u32)); mark('.'); diff --git a/tests/test-vconn.c b/tests/test-vconn.c index 12b69ca0..2b14fa85 100644 --- a/tests/test-vconn.c +++ b/tests/test-vconn.c @@ -1,5 +1,5 @@ /* - * Copyright (c) 2009, 2010 Nicira Networks. + * Copyright (c) 2009, 2010, 2011 Nicira Networks. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -203,7 +203,7 @@ test_read_hello(int argc OVS_UNUSED, char *argv[]) if (retval == sizeof hello) { CHECK(hello.version, OFP_VERSION); CHECK(hello.type, OFPT_HELLO); - CHECK(hello.length, htons(sizeof hello)); + CHECK(ntohs(hello.length), sizeof hello); break; } else { CHECK_ERRNO(retval, -EAGAIN); @@ -269,7 +269,7 @@ test_send_hello(const char *type, const void *out, size_t out_size, if (retval == sizeof hello) { CHECK(hello.version, OFP_VERSION); CHECK(hello.type, OFPT_HELLO); - CHECK(hello.length, htons(sizeof hello)); + CHECK(ntohs(hello.length), sizeof hello); read_hello = true; } else { CHECK_ERRNO(retval, -EAGAIN); -- 2.30.2