[ovs-dev] [PATCH] lib/flow: Always zero init before use.
Jarno Rajahalme
jrajahalme at nicira.com
Wed Oct 9 20:47:50 UTC 2013
Use the offset of the last member in struct flow instead of the
struct size to help catch changes in the declaration.
Add flow_random_hash_fields() used for testing in places where
struct flow was used without zero initialization before.
With these changes we do not need to keep updating explicit padding
in struct flow as fields are added or deleted.
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
lib/flow.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
lib/flow.h | 28 ++++++++++++++++++----------
tests/test-bundle.c | 4 +---
tests/test-multipath.c | 8 +++++---
4 files changed, 70 insertions(+), 17 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 0678c6f..9a36417 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -35,6 +35,7 @@
#include "ofpbuf.h"
#include "openflow/openflow.h"
#include "packets.h"
+#include "random.h"
#include "unaligned.h"
COVERAGE_DEFINE(flow_extract);
@@ -603,7 +604,11 @@ flow_wildcards_init_catchall(struct flow_wildcards *wc)
}
/* Initializes 'wc' as an exact-match set of wildcards; that is, 'wc' does not
- * wildcard any bits or fields. */
+ * wildcard any bits or fields.
+ * Note that also bits that are always zeroes (padding, extra bits in fields
+ * like IPv6 flow label), and fields that are mutually exclusive (e.g., IPv4
+ * and IPv6 addresses) are included.
+ */
void
flow_wildcards_init_exact(struct flow_wildcards *wc)
{
@@ -799,6 +804,46 @@ flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis)
return jhash_bytes(&fields, sizeof fields, basis);
}
+/* Initialize a flow with random fields that matter for nx_hash_fields. */
+void
+flow_random_hash_fields(struct flow *flow)
+{
+ uint16_t rnd = random_uint16();
+
+ /* Initialize to all zeros. */
+ memset(flow, 0, sizeof *flow);
+
+ eth_addr_random(flow->dl_src);
+ eth_addr_random(flow->dl_dst);
+
+ flow->vlan_tci = (OVS_FORCE ovs_be16) random_uint16() & VLAN_VID_MASK;
+
+ /* Make most of the random flows IPv4, some IPv6, and rest random. */
+ flow->dl_type = rnd < 0x8000 ? htons(ETH_TYPE_IP) :
+ rnd < 0xc000 ? htons(ETH_TYPE_IPV6) : (OVS_FORCE ovs_be16)rnd;
+
+ if (dl_type_is_ip_any(flow->dl_type)) {
+ if (flow->dl_type == htons(ETH_TYPE_IP)) {
+ flow->nw_src = (OVS_FORCE ovs_be32)random_uint32();
+ flow->nw_dst = (OVS_FORCE ovs_be32)random_uint32();
+ } else {
+ random_bytes(&flow->ipv6_src, sizeof flow->ipv6_src);
+ random_bytes(&flow->ipv6_dst, sizeof flow->ipv6_dst);
+ }
+ /* Make most of IP flows TCP, some UDP or SCTP, and rest random. */
+ rnd = random_uint16();
+ flow->nw_proto = rnd < 0x8000 ? IPPROTO_TCP :
+ rnd < 0xc000 ? IPPROTO_UDP :
+ rnd < 0xd000 ? IPPROTO_SCTP : (uint8_t)rnd;
+ if (flow->nw_proto == IPPROTO_TCP ||
+ flow->nw_proto == IPPROTO_UDP ||
+ flow->nw_proto == IPPROTO_SCTP) {
+ flow->tp_src = (OVS_FORCE ovs_be16)random_uint16();
+ flow->tp_dst = (OVS_FORCE ovs_be16)random_uint16();
+ }
+ }
+}
+
/* Masks the fields in 'wc' that are used by the flow hash 'fields'. */
void
flow_mask_hash_fields(const struct flow *flow, struct flow_wildcards *wc,
diff --git a/lib/flow.h b/lib/flow.h
index 4bd1504..026f109 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -78,13 +78,17 @@ union flow_in_port {
};
/*
-* A flow in the network.
-*
-* The meaning of 'in_port' is context-dependent. In most cases, it is a
-* 16-bit OpenFlow 1.0 port number. In the software datapath interface (dpif)
-* layer and its implementations (e.g. dpif-linux, dpif-netdev), it is instead
-* a 32-bit datapath port number.
-*/
+ * A flow in the network.
+ *
+ * Must be initialized to all zeros to make any compiler-induced padding
+ * zeroed. Helps also in keeping unused fields (such as mutually exclusive
+ * IPv4 and IPv6 addresses) zeroed out.
+ *
+ * The meaning of 'in_port' is context-dependent. In most cases, it is a
+ * 16-bit OpenFlow 1.0 port number. In the software datapath interface (dpif)
+ * layer and its implementations (e.g. dpif-linux, dpif-netdev), it is instead
+ * a 32-bit datapath port number.
+ */
struct flow {
struct flow_tnl tunnel; /* Encapsulating tunnel parameters. */
ovs_be64 metadata; /* OpenFlow Metadata. */
@@ -110,15 +114,17 @@ struct flow {
uint8_t arp_sha[6]; /* ARP/ND source hardware address. */
uint8_t arp_tha[6]; /* ARP/ND target hardware address. */
uint8_t nw_ttl; /* IP TTL/Hop Limit. */
- uint8_t nw_frag; /* FLOW_FRAG_* flags. */
+ uint8_t nw_frag; /* FLOW_FRAG_* flags. Keep last for the
+ BUILD_ASSERT_DECL below */
};
BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
#define FLOW_U32S (sizeof(struct flow) / 4)
/* Remember to update FLOW_WC_SEQ when changing 'struct flow'. */
-BUILD_ASSERT_DECL(sizeof(struct flow) == sizeof(struct flow_tnl) + 152 &&
- FLOW_WC_SEQ == 21);
+BUILD_ASSERT_DECL(offsetof(struct flow, nw_frag) + 1
+ == sizeof(struct flow_tnl) + 152
+ && FLOW_WC_SEQ == 21);
/* Represents the metadata fields of struct flow. */
struct flow_metadata {
@@ -262,6 +268,8 @@ bool flow_wildcards_equal(const struct flow_wildcards *,
const struct flow_wildcards *);
uint32_t flow_hash_symmetric_l4(const struct flow *flow, uint32_t basis);
+/* Initialize a flow with random fields that matter for nx_hash_fields. */
+void flow_random_hash_fields(struct flow *);
void flow_mask_hash_fields(const struct flow *, struct flow_wildcards *,
enum nx_hash_fields);
uint32_t flow_hash_fields(const struct flow *, enum nx_hash_fields,
diff --git a/tests/test-bundle.c b/tests/test-bundle.c
index 1bb2b0b..5e5ef52 100644
--- a/tests/test-bundle.c
+++ b/tests/test-bundle.c
@@ -23,7 +23,6 @@
#include "flow.h"
#include "ofp-actions.h"
#include "ofpbuf.h"
-#include "random.h"
#include "util.h"
@@ -116,7 +115,6 @@ main(int argc, char *argv[])
int old_active;
set_program_name(argv[0]);
- random_init();
if (argc != 2) {
ovs_fatal(0, "usage: %s bundle_action", program_name);
@@ -140,7 +138,7 @@ main(int argc, char *argv[])
/* Generate flows. */
flows = xmalloc(N_FLOWS * sizeof *flows);
for (i = 0; i < N_FLOWS; i++) {
- random_bytes(&flows[i], sizeof flows[i]);
+ flow_random_hash_fields(&flows[i]);
flows[i].regs[0] = ofp_to_u16(OFPP_NONE);
}
diff --git a/tests/test-multipath.c b/tests/test-multipath.c
index 4ba3692..bbfd684 100644
--- a/tests/test-multipath.c
+++ b/tests/test-multipath.c
@@ -26,7 +26,6 @@
#include "flow.h"
#include "ofp-actions.h"
-#include "random.h"
#include "util.h"
int
@@ -39,7 +38,6 @@ main(int argc, char *argv[])
int n;
set_program_name(argv[0]);
- random_init();
if (argc != 2) {
ovs_fatal(0, "usage: %s multipath_action", program_name);
@@ -65,7 +63,11 @@ main(int argc, char *argv[])
struct flow_wildcards wc;
struct flow flow;
- random_bytes(&flow, sizeof flow);
+ /* The generated flows will have random bits where zeroes would
+ * normally be required (padding, extra bits in fields like IPv6
+ * flow label, but this test will never use those bits, so it does
+ * not matter. */
+ flow_random_hash_fields(&flow);
mp.max_link = n - 1;
multipath_execute(&mp, &flow, &wc);
--
1.7.10.4
More information about the dev
mailing list