[ovs-dev] [PATCH] packets: Change IP_ARGS interface to take an ovs_be32 instead of a pointer.
Ben Pfaff
blp at nicira.com
Fri Nov 9 01:12:28 UTC 2012
An ovs_be32 is a more obvious way to represent an IP address than a
pointer to one. It is also more type-safe, especially since "sparse" is
able to check that the argument is in network byte order.
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
lib/netdev-linux.c | 2 +-
lib/netdev-vport.c | 4 ++--
lib/odp-util.c | 10 +++++-----
lib/ofp-actions.c | 4 ++--
lib/ofp-print.c | 2 +-
lib/packets.c | 4 ++--
lib/packets.h | 14 +++++---------
lib/socket-util.c | 2 +-
lib/stream-ssl.c | 4 ++--
lib/stream-tcp.c | 4 ++--
ofproto/in-band.c | 6 +++---
tests/test-netflow.c | 4 ++--
vswitchd/bridge.c | 4 ++--
13 files changed, 30 insertions(+), 34 deletions(-)
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 0460c06..35b4562 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -2327,7 +2327,7 @@ netdev_linux_arp_lookup(const struct netdev *netdev,
memcpy(mac, r.arp_ha.sa_data, ETH_ADDR_LEN);
} else if (retval != ENXIO) {
VLOG_WARN_RL(&rl, "%s: could not look up ARP entry for "IP_FMT": %s",
- netdev_get_name(netdev), IP_ARGS(&ip), strerror(retval));
+ netdev_get_name(netdev), IP_ARGS(ip), strerror(retval));
}
return retval;
}
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c
index 5171171..da3f733 100644
--- a/lib/netdev-vport.c
+++ b/lib/netdev-vport.c
@@ -790,11 +790,11 @@ unparse_tunnel_config(const char *name OVS_UNUSED, const char *type OVS_UNUSED,
daddr = nl_attr_get_be32(a[OVS_TUNNEL_ATTR_DST_IPV4]);
- smap_add_format(args, "remote_ip", IP_FMT, IP_ARGS(&daddr));
+ smap_add_format(args, "remote_ip", IP_FMT, IP_ARGS(daddr));
if (a[OVS_TUNNEL_ATTR_SRC_IPV4]) {
ovs_be32 saddr = nl_attr_get_be32(a[OVS_TUNNEL_ATTR_SRC_IPV4]);
- smap_add_format(args, "local_ip", IP_FMT, IP_ARGS(&saddr));
+ smap_add_format(args, "local_ip", IP_FMT, IP_ARGS(saddr));
}
if (!a[OVS_TUNNEL_ATTR_IN_KEY] && !a[OVS_TUNNEL_ATTR_OUT_KEY]) {
diff --git a/lib/odp-util.c b/lib/odp-util.c
index 08823e2..978fd9a 100644
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -706,8 +706,8 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
ds_put_format(ds, "(tun_id=0x%"PRIx64",flags=0x%"PRIx32
",src="IP_FMT",dst="IP_FMT",tos=0x%"PRIx8",ttl=%"PRIu8")",
ntohll(ipv4_tun_key->tun_id), ipv4_tun_key->tun_flags,
- IP_ARGS(&ipv4_tun_key->ipv4_src),
- IP_ARGS(&ipv4_tun_key->ipv4_dst),
+ IP_ARGS(ipv4_tun_key->ipv4_src),
+ IP_ARGS(ipv4_tun_key->ipv4_dst),
ipv4_tun_key->ipv4_tos, ipv4_tun_key->ipv4_ttl);
break;
@@ -737,8 +737,8 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
ipv4_key = nl_attr_get(a);
ds_put_format(ds, "(src="IP_FMT",dst="IP_FMT",proto=%"PRIu8
",tos=%#"PRIx8",ttl=%"PRIu8",frag=%s)",
- IP_ARGS(&ipv4_key->ipv4_src),
- IP_ARGS(&ipv4_key->ipv4_dst),
+ IP_ARGS(ipv4_key->ipv4_src),
+ IP_ARGS(ipv4_key->ipv4_dst),
ipv4_key->ipv4_proto, ipv4_key->ipv4_tos,
ipv4_key->ipv4_ttl,
ovs_frag_type_to_string(ipv4_key->ipv4_frag));
@@ -789,7 +789,7 @@ format_odp_key_attr(const struct nlattr *a, struct ds *ds)
arp_key = nl_attr_get(a);
ds_put_format(ds, "(sip="IP_FMT",tip="IP_FMT",op=%"PRIu16","
"sha="ETH_ADDR_FMT",tha="ETH_ADDR_FMT")",
- IP_ARGS(&arp_key->arp_sip), IP_ARGS(&arp_key->arp_tip),
+ IP_ARGS(arp_key->arp_sip), IP_ARGS(arp_key->arp_tip),
ntohs(arp_key->arp_op), ETH_ADDR_ARGS(arp_key->arp_sha),
ETH_ADDR_ARGS(arp_key->arp_tha));
break;
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 170e796..d192507 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -1930,12 +1930,12 @@ ofpact_format(const struct ofpact *a, struct ds *s)
case OFPACT_SET_IPV4_SRC:
ds_put_format(s, "mod_nw_src:"IP_FMT,
- IP_ARGS(&ofpact_get_SET_IPV4_SRC(a)->ipv4));
+ IP_ARGS(ofpact_get_SET_IPV4_SRC(a)->ipv4));
break;
case OFPACT_SET_IPV4_DST:
ds_put_format(s, "mod_nw_dst:"IP_FMT,
- IP_ARGS(&ofpact_get_SET_IPV4_DST(a)->ipv4));
+ IP_ARGS(ofpact_get_SET_IPV4_DST(a)->ipv4));
break;
case OFPACT_SET_IPV4_DSCP:
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index c1b50e2..60f0a41 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -571,7 +571,7 @@ print_ip_netmask(struct ds *string, const char *leader, ovs_be32 ip,
}
ds_put_cstr(string, leader);
if (wild_bits < 32) {
- ds_put_format(string, IP_FMT, IP_ARGS(&ip));
+ ds_put_format(string, IP_FMT, IP_ARGS(ip));
if (wild_bits) {
ds_put_format(string, "/%d", 32 - wild_bits);
}
diff --git a/lib/packets.c b/lib/packets.c
index 16f4fe6..8850fa0 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -272,12 +272,12 @@ ip_count_cidr_bits(ovs_be32 netmask)
void
ip_format_masked(ovs_be32 ip, ovs_be32 mask, struct ds *s)
{
- ds_put_format(s, IP_FMT, IP_ARGS(&ip));
+ ds_put_format(s, IP_FMT, IP_ARGS(ip));
if (mask != htonl(UINT32_MAX)) {
if (ip_is_cidr(mask)) {
ds_put_format(s, "/%d", ip_count_cidr_bits(mask));
} else {
- ds_put_format(s, "/"IP_FMT, IP_ARGS(&mask));
+ ds_put_format(s, "/"IP_FMT, IP_ARGS(mask));
}
}
}
diff --git a/lib/packets.h b/lib/packets.h
index e550be0..72d1843 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -272,16 +272,12 @@ struct vlan_eth_header {
} __attribute__((packed));
BUILD_ASSERT_DECL(VLAN_ETH_HEADER_LEN == sizeof(struct vlan_eth_header));
-/* The "(void) (ip)[0]" below has no effect on the value, since it's the first
- * argument of a comma expression, but it makes sure that 'ip' is a pointer.
- * This is useful since a common mistake is to pass an integer instead of a
- * pointer to IP_ARGS. */
-#define IP_FMT "%"PRIu8".%"PRIu8".%"PRIu8".%"PRIu8
+#define IP_FMT "%"PRIu32".%"PRIu32".%"PRIu32".%"PRIu32
#define IP_ARGS(ip) \
- ((void) (ip)[0], ((uint8_t *) ip)[0]), \
- ((uint8_t *) ip)[1], \
- ((uint8_t *) ip)[2], \
- ((uint8_t *) ip)[3]
+ ntohl(ip) >> 24, \
+ (ntohl(ip) >> 16) & 0xff, \
+ (ntohl(ip) >> 8) & 0xff, \
+ ntohl(ip) & 0xff
/* Example:
*
diff --git a/lib/socket-util.c b/lib/socket-util.c
index 4edf956..583bd0a 100644
--- a/lib/socket-util.c
+++ b/lib/socket-util.c
@@ -945,7 +945,7 @@ describe_sockaddr(struct ds *string, int fd,
memcpy(&sin, &ss, sizeof sin);
ds_put_format(string, IP_FMT":%"PRIu16,
- IP_ARGS(&sin.sin_addr.s_addr), ntohs(sin.sin_port));
+ IP_ARGS(sin.sin_addr.s_addr), ntohs(sin.sin_port));
} else if (ss.ss_family == AF_UNIX) {
struct sockaddr_un sun;
const char *null;
diff --git a/lib/stream-ssl.c b/lib/stream-ssl.c
index 0ca5b18..184b3ff 100644
--- a/lib/stream-ssl.c
+++ b/lib/stream-ssl.c
@@ -805,7 +805,7 @@ pssl_open(const char *name OVS_UNUSED, char *suffix, struct pstream **pstreamp,
return -fd;
}
sprintf(bound_name, "pssl:%"PRIu16":"IP_FMT,
- ntohs(sin.sin_port), IP_ARGS(&sin.sin_addr.s_addr));
+ ntohs(sin.sin_port), IP_ARGS(sin.sin_addr.s_addr));
pssl = xmalloc(sizeof *pssl);
pstream_init(&pssl->pstream, &pssl_pstream_class, bound_name);
@@ -847,7 +847,7 @@ pssl_accept(struct pstream *pstream, struct stream **new_streamp)
return error;
}
- sprintf(name, "ssl:"IP_FMT, IP_ARGS(&sin.sin_addr));
+ sprintf(name, "ssl:"IP_FMT, IP_ARGS(sin.sin_addr.s_addr));
if (sin.sin_port != htons(OFP_SSL_PORT)) {
sprintf(strchr(name, '\0'), ":%"PRIu16, ntohs(sin.sin_port));
}
diff --git a/lib/stream-tcp.c b/lib/stream-tcp.c
index 05601e1..0384c42 100644
--- a/lib/stream-tcp.c
+++ b/lib/stream-tcp.c
@@ -116,7 +116,7 @@ ptcp_open(const char *name OVS_UNUSED, char *suffix, struct pstream **pstreamp,
}
sprintf(bound_name, "ptcp:%"PRIu16":"IP_FMT,
- ntohs(sin.sin_port), IP_ARGS(&sin.sin_addr.s_addr));
+ ntohs(sin.sin_port), IP_ARGS(sin.sin_addr.s_addr));
return new_fd_pstream(bound_name, fd, ptcp_accept, set_dscp, NULL,
pstreamp);
}
@@ -129,7 +129,7 @@ ptcp_accept(int fd, const struct sockaddr *sa, size_t sa_len,
char name[128];
if (sa_len == sizeof(struct sockaddr_in) && sin->sin_family == AF_INET) {
- sprintf(name, "tcp:"IP_FMT, IP_ARGS(&sin->sin_addr));
+ sprintf(name, "tcp:"IP_FMT, IP_ARGS(sin->sin_addr.s_addr));
sprintf(strchr(name, '\0'), ":%"PRIu16, ntohs(sin->sin_port));
} else {
strcpy(name, "tcp");
diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index 0e4754c..81b330d 100644
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -120,7 +120,7 @@ refresh_remote(struct in_band *ib, struct in_band_remote *r)
&next_hop_inaddr, &next_hop_dev);
if (retval) {
VLOG_WARN("cannot find route for controller ("IP_FMT"): %s",
- IP_ARGS(&r->remote_addr.sin_addr), strerror(retval));
+ IP_ARGS(r->remote_addr.sin_addr.s_addr), strerror(retval));
return 1;
}
if (!next_hop_inaddr.s_addr) {
@@ -137,7 +137,7 @@ refresh_remote(struct in_band *ib, struct in_band_remote *r)
if (retval) {
VLOG_WARN_RL(&rl, "cannot open netdev %s (next hop "
"to controller "IP_FMT"): %s",
- next_hop_dev, IP_ARGS(&r->remote_addr.sin_addr),
+ next_hop_dev, IP_ARGS(r->remote_addr.sin_addr.s_addr),
strerror(retval));
free(next_hop_dev);
return 1;
@@ -150,7 +150,7 @@ refresh_remote(struct in_band *ib, struct in_band_remote *r)
r->remote_mac);
if (retval) {
VLOG_DBG_RL(&rl, "cannot look up remote MAC address ("IP_FMT"): %s",
- IP_ARGS(&next_hop_inaddr.s_addr), strerror(retval));
+ IP_ARGS(next_hop_inaddr.s_addr), strerror(retval));
}
/* If we don't have a MAC address, then refresh quickly, since we probably
diff --git a/tests/test-netflow.c b/tests/test-netflow.c
index c37eeaf..5147a20 100644
--- a/tests/test-netflow.c
+++ b/tests/test-netflow.c
@@ -75,7 +75,7 @@ print_netflow(struct ofpbuf *buf)
}
printf("seq %"PRIu32": "IP_FMT" > "IP_FMT, ntohl(hdr->flow_seq),
- IP_ARGS(&rec->src_addr), IP_ARGS(&rec->dst_addr));
+ IP_ARGS(rec->src_addr), IP_ARGS(rec->dst_addr));
printf(", if %"PRIu16" > %"PRIu16,
ntohs(rec->input), ntohs(rec->output));
@@ -137,7 +137,7 @@ print_netflow(struct ofpbuf *buf)
ntohl(rec->init_time), ntohl(rec->used_time));
if (rec->nexthop != htonl(0)) {
- printf(", nexthop "IP_FMT, IP_ARGS(&rec->nexthop));
+ printf(", nexthop "IP_FMT, IP_ARGS(rec->nexthop));
}
if (rec->src_as != htons(0) || rec->dst_as != htons(0)) {
printf(", AS %"PRIu16" > %"PRIu16,
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 8f544a9..0dd16c1 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -2691,7 +2691,7 @@ bridge_configure_local_iface_netdev(struct bridge *br,
}
if (!netdev_set_in4(netdev, ip, mask)) {
VLOG_INFO("bridge %s: configured IP address "IP_FMT", netmask "IP_FMT,
- br->name, IP_ARGS(&ip.s_addr), IP_ARGS(&mask.s_addr));
+ br->name, IP_ARGS(ip.s_addr), IP_ARGS(mask.s_addr));
}
/* Configure the default gateway. */
@@ -2700,7 +2700,7 @@ bridge_configure_local_iface_netdev(struct bridge *br,
&& gateway.s_addr) {
if (!netdev_add_router(netdev, gateway)) {
VLOG_INFO("bridge %s: configured gateway "IP_FMT,
- br->name, IP_ARGS(&gateway.s_addr));
+ br->name, IP_ARGS(gateway.s_addr));
}
}
}
--
1.7.2.5
More information about the dev
mailing list