[ovs-dev] [next 12/35] Convert remaining network-byte-order "uint<N>_t"s into "ovs_be<N>"s.
Ethan Jackson
ethan at nicira.com
Mon May 2 18:48:58 UTC 2011
Wow, thanks for doing this.
Whats our long term plan with the ovs_be types? Do we intended to
include some sort of static analysis to catch byte order errors? Or
is it purely for documentation purposes?
Patch looks good.
Ethan
On Tue, Apr 26, 2011 at 09:24, Ben Pfaff <blp at nicira.com> wrote:
> I looked at almost every uint<N>_t in the tree to determine whether it was
> really in network byte order, and converted the ones that were.
>
> The only remaining ones, modulo my mistakes, are in openflow.h. I'm not
> sure whether we should convert those, because there might be some value
> in remaining close to upstream for this header.
> ---
> lib/byte-order.h | 41 +++++++++++++++++++++--------------------
> lib/dhcp.h | 14 +++++++-------
> lib/netdev-linux.c | 4 ++--
> lib/netdev-provider.h | 3 ++-
> lib/netdev.c | 2 +-
> lib/netdev.h | 3 ++-
> lib/ofp-print.c | 2 +-
> lib/ofp-util.c | 5 ++---
> lib/ofp-util.h | 2 +-
> lib/rconn.c | 8 ++++----
> lib/rconn.h | 9 +++++----
> lib/socket-util.c | 6 +++---
> lib/socket-util.h | 3 ++-
> lib/vconn.h | 4 ++--
> ofproto/netflow.c | 40 ++++++++++++++++++++--------------------
> 15 files changed, 75 insertions(+), 71 deletions(-)
>
> diff --git a/lib/byte-order.h b/lib/byte-order.h
> index e05a71f..d2bc8db 100644
> --- a/lib/byte-order.h
> +++ b/lib/byte-order.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2010 Nicira Networks.
> + * Copyright (c) 2008, 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.
> @@ -19,15 +19,16 @@
> #include <arpa/inet.h>
> #include <sys/types.h>
> #include <inttypes.h>
> +#include "openvswitch/types.h"
>
> -static inline uint64_t
> +static inline ovs_be64
> htonll(uint64_t n)
> {
> return htonl(1) == 1 ? n : ((uint64_t) htonl(n) << 32) | htonl(n >> 32);
> }
>
> static inline uint64_t
> -ntohll(uint64_t n)
> +ntohll(ovs_be64 n)
> {
> return htonl(1) == 1 ? n : ((uint64_t) ntohl(n) << 32) | ntohl(n >> 32);
> }
> @@ -36,27 +37,27 @@ ntohll(uint64_t n)
> * where function calls are not allowed, such as case labels. They should not
> * be used elsewhere because all of them evaluate their argument many times. */
> #ifdef WORDS_BIGENDIAN
> -#define CONSTANT_HTONS(VALUE) ((uint16_t) (VALUE))
> -#define CONSTANT_HTONL(VALUE) ((uint32_t) (VALUE))
> -#define CONSTANT_HTONLL(VALUE) ((uint64_t) (VALUE))
> +#define CONSTANT_HTONS(VALUE) ((ovs_be16) (VALUE))
> +#define CONSTANT_HTONL(VALUE) ((ovs_be32) (VALUE))
> +#define CONSTANT_HTONLL(VALUE) ((ovs_be64) (VALUE))
> #else
> #define CONSTANT_HTONS(VALUE) \
> - (((((uint16_t) (VALUE)) & 0xff00) >> 8) | \
> - ((((uint16_t) (VALUE)) & 0x00ff) << 8))
> + (((((ovs_be16) (VALUE)) & 0xff00) >> 8) | \
> + ((((ovs_be16) (VALUE)) & 0x00ff) << 8))
> #define CONSTANT_HTONL(VALUE) \
> - (((((uint32_t) (VALUE)) & 0x000000ff) << 24) | \
> - ((((uint32_t) (VALUE)) & 0x0000ff00) << 8) | \
> - ((((uint32_t) (VALUE)) & 0x00ff0000) >> 8) | \
> - ((((uint32_t) (VALUE)) & 0xff000000) >> 24))
> + (((((ovs_be32) (VALUE)) & 0x000000ff) << 24) | \
> + ((((ovs_be32) (VALUE)) & 0x0000ff00) << 8) | \
> + ((((ovs_be32) (VALUE)) & 0x00ff0000) >> 8) | \
> + ((((ovs_be32) (VALUE)) & 0xff000000) >> 24))
> #define CONSTANT_HTONLL(VALUE) \
> - (((((uint64_t) (VALUE)) & UINT64_C(0x00000000000000ff)) << 56) | \
> - ((((uint64_t) (VALUE)) & UINT64_C(0x000000000000ff00)) << 40) | \
> - ((((uint64_t) (VALUE)) & UINT64_C(0x0000000000ff0000)) << 24) | \
> - ((((uint64_t) (VALUE)) & UINT64_C(0x00000000ff000000)) << 8) | \
> - ((((uint64_t) (VALUE)) & UINT64_C(0x000000ff00000000)) >> 8) | \
> - ((((uint64_t) (VALUE)) & UINT64_C(0x0000ff0000000000)) >> 24) | \
> - ((((uint64_t) (VALUE)) & UINT64_C(0x00ff000000000000)) >> 40) | \
> - ((((uint64_t) (VALUE)) & UINT64_C(0xff00000000000000)) >> 56))
> + (((((ovs_be64) (VALUE)) & UINT64_C(0x00000000000000ff)) << 56) | \
> + ((((ovs_be64) (VALUE)) & UINT64_C(0x000000000000ff00)) << 40) | \
> + ((((ovs_be64) (VALUE)) & UINT64_C(0x0000000000ff0000)) << 24) | \
> + ((((ovs_be64) (VALUE)) & UINT64_C(0x00000000ff000000)) << 8) | \
> + ((((ovs_be64) (VALUE)) & UINT64_C(0x000000ff00000000)) >> 8) | \
> + ((((ovs_be64) (VALUE)) & UINT64_C(0x0000ff0000000000)) >> 24) | \
> + ((((ovs_be64) (VALUE)) & UINT64_C(0x00ff000000000000)) >> 40) | \
> + ((((ovs_be64) (VALUE)) & UINT64_C(0xff00000000000000)) >> 56))
> #endif
>
> #endif /* byte-order.h */
> diff --git a/lib/dhcp.h b/lib/dhcp.h
> index b368226..8858cd5 100644
> --- a/lib/dhcp.h
> +++ b/lib/dhcp.h
> @@ -31,13 +31,13 @@ struct dhcp_header {
> uint8_t htype; /* ARP_HRD_ETHERNET (typically). */
> uint8_t hlen; /* ETH_ADDR_LEN (typically). */
> uint8_t hops; /* Hop count; set to 0 by client. */
> - uint32_t xid; /* Transaction ID. */
> - uint16_t secs; /* Since client started address acquisition. */
> - uint16_t flags; /* DHCP_FLAGS_*. */
> - uint32_t ciaddr; /* Client IP, if it has a lease for one. */
> - uint32_t yiaddr; /* Client ("your") IP address. */
> - uint32_t siaddr; /* Next server IP address. */
> - uint32_t giaddr; /* Relay agent IP address. */
> + ovs_be32 xid; /* Transaction ID. */
> + ovs_be16 secs; /* Since client started address acquisition. */
> + ovs_be16 flags; /* DHCP_FLAGS_*. */
> + ovs_be32 ciaddr; /* Client IP, if it has a lease for one. */
> + ovs_be32 yiaddr; /* Client ("your") IP address. */
> + ovs_be32 siaddr; /* Next server IP address. */
> + ovs_be32 giaddr; /* Relay agent IP address. */
> uint8_t chaddr[16]; /* Client hardware address. */
> char sname[64]; /* Optional server host name. */
> char file[128]; /* Boot file name. */
> diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
> index e2af659..6b1124f 100644
> --- a/lib/netdev-linux.c
> +++ b/lib/netdev-linux.c
> @@ -2014,7 +2014,7 @@ netdev_linux_get_next_hop(const struct in_addr *host, struct in_addr *next_hop,
> while (fgets(line, sizeof line, stream)) {
> if (++ln >= 2) {
> char iface[17];
> - uint32_t dest, gateway, mask;
> + ovs_be32 dest, gateway, mask;
> int refcnt, metric, mtu;
> unsigned int flags, use, window, irtt;
>
> @@ -2081,7 +2081,7 @@ netdev_linux_get_status(const struct netdev *netdev, struct shash *sh)
> * ENXIO indicates that there is not ARP table entry for 'ip' on 'netdev'. */
> static int
> netdev_linux_arp_lookup(const struct netdev *netdev,
> - uint32_t ip, uint8_t mac[ETH_ADDR_LEN])
> + ovs_be32 ip, uint8_t mac[ETH_ADDR_LEN])
> {
> struct arpreq r;
> struct sockaddr_in sin;
> diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h
> index b095779..6887e7f 100644
> --- a/lib/netdev-provider.h
> +++ b/lib/netdev-provider.h
> @@ -544,7 +544,8 @@ struct netdev_class {
> *
> * This function may be set to null if it would always return EOPNOTSUPP
> * anyhow. */
> - int (*arp_lookup)(const struct netdev *netdev, uint32_t ip, uint8_t mac[6]);
> + int (*arp_lookup)(const struct netdev *netdev, ovs_be32 ip,
> + uint8_t mac[6]);
>
> /* Retrieves the current set of flags on 'netdev' into '*old_flags'.
> * Then, turns off the flags that are set to 1 in 'off' and turns on the
> diff --git a/lib/netdev.c b/lib/netdev.c
> index bf7ff6a..771db5a 100644
> --- a/lib/netdev.c
> +++ b/lib/netdev.c
> @@ -846,7 +846,7 @@ netdev_turn_flags_off(struct netdev *netdev, enum netdev_flags flags,
> * ENXIO indicates that there is no ARP table entry for 'ip' on 'netdev'. */
> int
> netdev_arp_lookup(const struct netdev *netdev,
> - uint32_t ip, uint8_t mac[ETH_ADDR_LEN])
> + ovs_be32 ip, uint8_t mac[ETH_ADDR_LEN])
> {
> int error = (netdev_get_dev(netdev)->netdev_class->arp_lookup
> ? netdev_get_dev(netdev)->netdev_class->arp_lookup(netdev,
> diff --git a/lib/netdev.h b/lib/netdev.h
> index e6fadb0..30bcf5e 100644
> --- a/lib/netdev.h
> +++ b/lib/netdev.h
> @@ -20,6 +20,7 @@
> #include <stdbool.h>
> #include <stddef.h>
> #include <stdint.h>
> +#include "openvswitch/types.h"
>
> #ifdef __cplusplus
> extern "C" {
> @@ -146,7 +147,7 @@ int netdev_add_router(struct netdev *, struct in_addr router);
> int netdev_get_next_hop(const struct netdev *, const struct in_addr *host,
> struct in_addr *next_hop, char **);
> int netdev_get_status(const struct netdev *, struct shash *sh);
> -int netdev_arp_lookup(const struct netdev *, uint32_t ip, uint8_t mac[6]);
> +int netdev_arp_lookup(const struct netdev *, ovs_be32 ip, uint8_t mac[6]);
>
> int netdev_get_flags(const struct netdev *, enum netdev_flags *);
> int netdev_set_flags(struct netdev *, enum netdev_flags, bool permanent);
> diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> index 30f6d37..d63ff7c 100644
> --- a/lib/ofp-print.c
> +++ b/lib/ofp-print.c
> @@ -722,7 +722,7 @@ static void print_wild(struct ds *string, const char *leader, int is_wild,
> }
>
> static void
> -print_ip_netmask(struct ds *string, const char *leader, uint32_t ip,
> +print_ip_netmask(struct ds *string, const char *leader, ovs_be32 ip,
> uint32_t wild_bits, int verbosity)
> {
> if (wild_bits >= 32 && verbosity < 2) {
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 4ee09eb..e500bf5 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -2115,10 +2115,9 @@ validate_actions(const union ofp_action *actions, size_t n_actions,
> return 0;
> }
>
> -/* Returns true if 'action' outputs to 'port' (which must be in network byte
> - * order), false otherwise. */
> +/* Returns true if 'action' outputs to 'port', false otherwise. */
> bool
> -action_outputs_to_port(const union ofp_action *action, uint16_t port)
> +action_outputs_to_port(const union ofp_action *action, ovs_be16 port)
> {
> switch (ntohs(action->type)) {
> case OFPAT_OUTPUT:
> diff --git a/lib/ofp-util.h b/lib/ofp-util.h
> index f9ff5ff..46216a9 100644
> --- a/lib/ofp-util.h
> +++ b/lib/ofp-util.h
> @@ -271,7 +271,7 @@ const union ofp_action *actions_next(struct actions_iterator *);
>
> int validate_actions(const union ofp_action *, size_t n_actions,
> const struct flow *, int max_ports);
> -bool action_outputs_to_port(const union ofp_action *, uint16_t port);
> +bool action_outputs_to_port(const union ofp_action *, ovs_be16 port);
>
> int ofputil_pull_actions(struct ofpbuf *, unsigned int actions_len,
> union ofp_action **, size_t *);
> diff --git a/lib/rconn.c b/lib/rconn.c
> index 0e18ab4..7d0f4ce 100644
> --- a/lib/rconn.c
> +++ b/lib/rconn.c
> @@ -700,7 +700,7 @@ rconn_failure_duration(const struct rconn *rconn)
>
> /* Returns the IP address of the peer, or 0 if the peer's IP address is not
> * known. */
> -uint32_t
> +ovs_be32
> rconn_get_remote_ip(const struct rconn *rconn)
> {
> return rconn->remote_ip;
> @@ -708,7 +708,7 @@ rconn_get_remote_ip(const struct rconn *rconn)
>
> /* Returns the transport port of the peer, or 0 if the peer's port is not
> * known. */
> -uint16_t
> +ovs_be16
> rconn_get_remote_port(const struct rconn *rconn)
> {
> return rconn->remote_port;
> @@ -717,7 +717,7 @@ rconn_get_remote_port(const struct rconn *rconn)
> /* Returns the IP address used to connect to the peer, or 0 if the
> * connection is not an IP-based protocol or if its IP address is not
> * known. */
> -uint32_t
> +ovs_be32
> rconn_get_local_ip(const struct rconn *rconn)
> {
> return rconn->local_ip;
> @@ -725,7 +725,7 @@ rconn_get_local_ip(const struct rconn *rconn)
>
> /* Returns the transport port used to connect to the peer, or 0 if the
> * connection does not contain a port or if the port is not known. */
> -uint16_t
> +ovs_be16
> rconn_get_local_port(const struct rconn *rconn)
> {
> return rconn->vconn ? vconn_get_local_port(rconn->vconn) : 0;
> diff --git a/lib/rconn.h b/lib/rconn.h
> index 7bc2af8..a6c2fa7 100644
> --- a/lib/rconn.h
> +++ b/lib/rconn.h
> @@ -20,6 +20,7 @@
> #include <stdbool.h>
> #include <stdint.h>
> #include <time.h>
> +#include "openvswitch/types.h"
>
> /* A wrapper around vconn that provides queuing and optionally reliability.
> *
> @@ -71,10 +72,10 @@ bool rconn_is_connected(const struct rconn *);
> bool rconn_is_admitted(const struct rconn *);
> int rconn_failure_duration(const struct rconn *);
>
> -uint32_t rconn_get_remote_ip(const struct rconn *);
> -uint16_t rconn_get_remote_port(const struct rconn *);
> -uint32_t rconn_get_local_ip(const struct rconn *);
> -uint16_t rconn_get_local_port(const struct rconn *);
> +ovs_be32 rconn_get_remote_ip(const struct rconn *);
> +ovs_be16 rconn_get_remote_port(const struct rconn *);
> +ovs_be32 rconn_get_local_ip(const struct rconn *);
> +ovs_be16 rconn_get_local_port(const struct rconn *);
>
> const char *rconn_get_state(const struct rconn *);
> unsigned int rconn_get_attempted_connections(const struct rconn *);
> diff --git a/lib/socket-util.c b/lib/socket-util.c
> index 24e8f81..249b6f0 100644
> --- a/lib/socket-util.c
> +++ b/lib/socket-util.c
> @@ -420,10 +420,10 @@ get_unix_name_len(socklen_t sun_len)
> : 0);
> }
>
> -uint32_t
> -guess_netmask(uint32_t ip)
> +ovs_be32
> +guess_netmask(ovs_be32 ip_)
> {
> - ip = ntohl(ip);
> + uint32_t ip = ntohl(ip_);
> return ((ip >> 31) == 0 ? htonl(0xff000000) /* Class A */
> : (ip >> 30) == 2 ? htonl(0xffff0000) /* Class B */
> : (ip >> 29) == 6 ? htonl(0xffffff00) /* Class C */
> diff --git a/lib/socket-util.h b/lib/socket-util.h
> index 8c5af39..56a978b 100644
> --- a/lib/socket-util.h
> +++ b/lib/socket-util.h
> @@ -20,6 +20,7 @@
> #include <sys/types.h>
> #include <netinet/in.h>
> #include <stdbool.h>
> +#include "openvswitch/types.h"
>
> int set_nonblocking(int fd);
> int get_max_fds(void);
> @@ -32,7 +33,7 @@ void drain_fd(int fd, size_t n_packets);
> int make_unix_socket(int style, bool nonblock, bool passcred,
> const char *bind_path, const char *connect_path);
> int get_unix_name_len(socklen_t sun_len);
> -uint32_t guess_netmask(uint32_t ip);
> +ovs_be32 guess_netmask(ovs_be32 ip);
> int get_null_fd(void);
>
> bool inet_parse_active(const char *target, uint16_t default_port,
> diff --git a/lib/vconn.h b/lib/vconn.h
> index 8e321b2..357fc4e 100644
> --- a/lib/vconn.h
> +++ b/lib/vconn.h
> @@ -1,5 +1,5 @@
> /*
> - * Copyright (c) 2008, 2009, 2010 Nicira Networks.
> + * Copyright (c) 2008, 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.
> @@ -47,7 +47,7 @@ ovs_be16 vconn_get_local_port(const struct vconn *);
> int vconn_connect(struct vconn *);
> int vconn_recv(struct vconn *, struct ofpbuf **);
> int vconn_send(struct vconn *, struct ofpbuf *);
> -int vconn_recv_xid(struct vconn *, uint32_t xid, struct ofpbuf **);
> +int vconn_recv_xid(struct vconn *, ovs_be32 xid, struct ofpbuf **);
> int vconn_transact(struct vconn *, struct ofpbuf *, struct ofpbuf **);
> int vconn_transact_noreply(struct vconn *, struct ofpbuf *, struct ofpbuf **);
> int vconn_transact_multiple_noreply(struct vconn *, struct list *requests,
> diff --git a/ofproto/netflow.c b/ofproto/netflow.c
> index 2d69db8..c237ef2 100644
> --- a/ofproto/netflow.c
> +++ b/ofproto/netflow.c
> @@ -41,17 +41,17 @@ VLOG_DEFINE_THIS_MODULE(netflow);
> * We only send a single record per NetFlow message.
> */
> struct netflow_v5_header {
> - uint16_t version; /* NetFlow version is 5. */
> - uint16_t count; /* Number of records in this message. */
> - uint32_t sysuptime; /* System uptime in milliseconds. */
> - uint32_t unix_secs; /* Number of seconds since Unix epoch. */
> - uint32_t unix_nsecs; /* Number of residual nanoseconds
> + ovs_be16 version; /* NetFlow version is 5. */
> + ovs_be16 count; /* Number of records in this message. */
> + ovs_be32 sysuptime; /* System uptime in milliseconds. */
> + ovs_be32 unix_secs; /* Number of seconds since Unix epoch. */
> + ovs_be32 unix_nsecs; /* Number of residual nanoseconds
> after epoch seconds. */
> - uint32_t flow_seq; /* Number of flows since sending
> + ovs_be32 flow_seq; /* Number of flows since sending
> messages began. */
> uint8_t engine_type; /* Engine type. */
> uint8_t engine_id; /* Engine id. */
> - uint16_t sampling_interval; /* Set to zero. */
> + ovs_be16 sampling_interval; /* Set to zero. */
> };
> BUILD_ASSERT_DECL(sizeof(struct netflow_v5_header) == 24);
>
> @@ -59,29 +59,29 @@ BUILD_ASSERT_DECL(sizeof(struct netflow_v5_header) == 24);
> * NetFlow v5 header.
> */
> struct netflow_v5_record {
> - uint32_t src_addr; /* Source IP address. */
> - uint32_t dst_addr; /* Destination IP address. */
> - uint32_t nexthop; /* IP address of next hop. Set to 0. */
> - uint16_t input; /* Input interface index. */
> - uint16_t output; /* Output interface index. */
> - uint32_t packet_count; /* Number of packets. */
> - uint32_t byte_count; /* Number of bytes. */
> - uint32_t init_time; /* Value of sysuptime on first packet. */
> - uint32_t used_time; /* Value of sysuptime on last packet. */
> + ovs_be32 src_addr; /* Source IP address. */
> + ovs_be32 dst_addr; /* Destination IP address. */
> + ovs_be32 nexthop; /* IP address of next hop. Set to 0. */
> + ovs_be16 input; /* Input interface index. */
> + ovs_be16 output; /* Output interface index. */
> + ovs_be32 packet_count; /* Number of packets. */
> + ovs_be32 byte_count; /* Number of bytes. */
> + ovs_be32 init_time; /* Value of sysuptime on first packet. */
> + ovs_be32 used_time; /* Value of sysuptime on last packet. */
>
> /* The 'src_port' and 'dst_port' identify the source and destination
> * port, respectively, for TCP and UDP. For ICMP, the high-order
> * byte identifies the type and low-order byte identifies the code
> * in the 'dst_port' field. */
> - uint16_t src_port;
> - uint16_t dst_port;
> + ovs_be16 src_port;
> + ovs_be16 dst_port;
>
> uint8_t pad1;
> uint8_t tcp_flags; /* Union of seen TCP flags. */
> uint8_t ip_proto; /* IP protocol. */
> uint8_t ip_tos; /* IP TOS value. */
> - uint16_t src_as; /* Source AS ID. Set to 0. */
> - uint16_t dst_as; /* Destination AS ID. Set to 0. */
> + ovs_be16 src_as; /* Source AS ID. Set to 0. */
> + ovs_be16 dst_as; /* Destination AS ID. Set to 0. */
> uint8_t src_mask; /* Source mask bits. Set to 0. */
> uint8_t dst_mask; /* Destination mask bits. Set to 0. */
> uint8_t pad[2];
> --
> 1.7.4.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>
More information about the dev
mailing list