[ovs-dev] [PATCH v4] More accurate wildcarding and fragment handling.
Jarno Rajahalme
jrajahalme at nicira.com
Thu Oct 17 21:28:20 UTC 2013
This patch gets rid of the need for having explicit padding in struct
flow as new fields are being added. flow_wildcards_init_exact(), which
used to set bits in both compiler generated and explicit padding, is
removed. match_wc_init() is now used instead, which generates the mask
based on a given flow, setting bits only in fields which make sense.
Places where random bits were placed in struct flow have been changed to
only set random bits on fields that are significant in the given context.
This avoids setting padding bits.
- lib/flow:
- Properly initialize struct flow also in places we used to zero out
padding before.
- Add flow_random_hash_fields() used for testing.
- Remove flow_wildcards_init_exact() to avoid initializing
masks where compiler generated padding has bits set.
- lib/match.c match_wc_init(): Wildcard transport layer fields for later
fragments, remove match_init_exact(), which used
flow_wildcards_init_exact().
- tests/test-flows.c: use match_wc_init() instead of match_init_exact()
- tests/flowgen.pl: generate more accurate packets and flows when
fragmenting, mark unavailable fields as wildcarded.
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
lib/flow.c | 49 ++++++++++++++++++++++++++------
lib/flow.h | 29 +++++++++++--------
lib/match.c | 13 +++------
lib/match.h | 1 -
ofproto/ofproto-dpif.c | 4 +--
tests/flowgen.pl | 73 ++++++++++++++++++++++++++----------------------
tests/test-bundle.c | 4 +--
tests/test-flows.c | 2 +-
tests/test-multipath.c | 4 +--
9 files changed, 107 insertions(+), 72 deletions(-)
diff --git a/lib/flow.c b/lib/flow.c
index 0678c6f..fedba69 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);
@@ -602,14 +603,6 @@ flow_wildcards_init_catchall(struct flow_wildcards *wc)
memset(&wc->masks, 0, sizeof wc->masks);
}
-/* Initializes 'wc' as an exact-match set of wildcards; that is, 'wc' does not
- * wildcard any bits or fields. */
-void
-flow_wildcards_init_exact(struct flow_wildcards *wc)
-{
- memset(&wc->masks, 0xff, sizeof wc->masks);
-}
-
/* Returns true if 'wc' matches every packet, false if 'wc' fixes any bits or
* fields. */
bool
@@ -799,6 +792,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..ad51496 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 {
@@ -238,7 +244,6 @@ struct flow_wildcards {
};
void flow_wildcards_init_catchall(struct flow_wildcards *);
-void flow_wildcards_init_exact(struct flow_wildcards *);
bool flow_wildcards_is_catchall(const struct flow_wildcards *);
@@ -262,6 +267,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/lib/match.c b/lib/match.c
index 93f61f9..73b1bf0 100644
--- a/lib/match.c
+++ b/lib/match.c
@@ -110,6 +110,10 @@ match_wc_init(struct match *match, const struct flow *flow)
if (flow->nw_frag) {
memset(&wc->masks.nw_frag, 0xff, sizeof wc->masks.nw_frag);
+ if (flow->nw_frag & FLOW_NW_FRAG_LATER) {
+ /* No transport layer header in later fragments. */
+ return;
+ }
}
if (flow->nw_proto == IPPROTO_ICMP ||
@@ -128,15 +132,6 @@ match_wc_init(struct match *match, const struct flow *flow)
return;
}
-/* Converts the flow in 'flow' into an exact-match match in 'match'. */
-void
-match_init_exact(struct match *match, const struct flow *flow)
-{
- match->flow = *flow;
- match->flow.skb_priority = 0;
- flow_wildcards_init_exact(&match->wc);
-}
-
/* Initializes 'match' as a "catch-all" match that matches every packet. */
void
match_init_catchall(struct match *match)
diff --git a/lib/match.h b/lib/match.h
index 48c8aa2..6e87a09 100644
--- a/lib/match.h
+++ b/lib/match.h
@@ -38,7 +38,6 @@ void match_init(struct match *,
const struct flow *, const struct flow_wildcards *);
void match_wc_init(struct match *match, const struct flow *flow);
void match_init_catchall(struct match *);
-void match_init_exact(struct match *, const struct flow *);
void match_zero_wildcarded_fields(struct match *);
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 8ef1c8c..2911ad3 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4628,9 +4628,7 @@ rule_dpif_lookup_in_table(struct ofproto_dpif *ofproto,
cls_rule = classifier_lookup(cls, &ofpc_normal_flow, wc);
} else if (frag && ofproto->up.frag_handling == OFPC_FRAG_DROP) {
cls_rule = &ofproto->drop_frags_rule->up.cr;
- if (wc) {
- flow_wildcards_init_exact(wc);
- }
+ /* Frag mask in wc already set above. */
} else {
cls_rule = classifier_lookup(cls, flow, wc);
}
diff --git a/tests/flowgen.pl b/tests/flowgen.pl
index 536fb32..cdc275e 100755
--- a/tests/flowgen.pl
+++ b/tests/flowgen.pl
@@ -109,7 +109,8 @@ sub output {
# Compose packet.
my $packet = '';
- my $wildcards = 0;
+ my $wildcards = 1 << 5 | 1 << 6 | 1 << 7 | 32 << 8 | 32 << 14 | 1 << 21;
+
$packet .= pack_ethaddr($flow{DL_DST});
$packet .= pack_ethaddr($flow{DL_SRC});
if ($flow{DL_VLAN} != 0xffff) {
@@ -139,6 +140,7 @@ sub output {
0, # checksum
0x0a00020f, # source
0xc0a80114); # dest
+ $wildcards &= ~( 1 << 5 | 63 << 8 | 63 << 14 | 1 << 21);
if ($attrs{IP_OPTIONS} eq 'yes') {
substr($ip, 0, 1) = pack('C', (4 << 4) | 8);
$ip .= pack('CCnnnCCCx',
@@ -151,6 +153,7 @@ sub output {
2,
3);
}
+
if ($attrs{IP_FRAGMENT} ne 'no') {
my (%frag_map) = ('first' => 0x2000, # more frags, ofs 0
'middle' => 0x2111, # more frags, ofs 0x888
@@ -158,39 +161,43 @@ sub output {
substr($ip, 6, 2)
= pack('n', $frag_map{$attrs{IP_FRAGMENT}});
}
-
- if ($attrs{TP_PROTO} =~ '^TCP') {
- my $tcp = pack('nnNNnnnn',
- $flow{TP_SRC}, # source port
- $flow{TP_DST}, # dest port
- 87123455, # seqno
- 712378912, # ackno
- (5 << 12) | 0x02 | 0x10, # hdrlen, SYN, ACK
- 5823, # window size
- 18923, # checksum
- 12893); # urgent pointer
- if ($attrs{TP_PROTO} eq 'TCP+options') {
- substr($tcp, 12, 2) = pack('n', (6 << 12) | 0x02 | 0x10);
- $tcp .= pack('CCn', 2, 4, 1975); # MSS option
+ if ($attrs{IP_FRAGMENT} eq 'no' || $attrs{IP_FRAGMENT} eq 'first') {
+ if ($attrs{TP_PROTO} =~ '^TCP') {
+ my $tcp = pack('nnNNnnnn',
+ $flow{TP_SRC}, # source port
+ $flow{TP_DST}, # dest port
+ 87123455, # seqno
+ 712378912, # ackno
+ (5 << 12) | 0x02 | 0x10, # hdrlen, SYN, ACK
+ 5823, # window size
+ 18923, # checksum
+ 12893); # urgent pointer
+ if ($attrs{TP_PROTO} eq 'TCP+options') {
+ substr($tcp, 12, 2) = pack('n', (6 << 12) | 0x02 | 0x10);
+ $tcp .= pack('CCn', 2, 4, 1975); # MSS option
+ }
+ $tcp .= 'payload';
+ $ip .= $tcp;
+ $wildcards &= ~(1 << 6 | 1 << 7);
+ } elsif ($attrs{TP_PROTO} eq 'UDP') {
+ my $len = 15;
+ my $udp = pack('nnnn', $flow{TP_SRC}, $flow{TP_DST}, $len, 0);
+ $udp .= chr($len) while length($udp) < $len;
+ $ip .= $udp;
+ $wildcards &= ~(1 << 6 | 1 << 7);
+ } elsif ($attrs{TP_PROTO} eq 'ICMP') {
+ $ip .= pack('CCnnn',
+ 8, # echo request
+ 0, # code
+ 0, # checksum
+ 736, # identifier
+ 931); # sequence number
+ $wildcards &= ~(1 << 6 | 1 << 7);
+ } elsif ($attrs{TP_PROTO} eq 'other') {
+ $ip .= 'other header';
+ } else {
+ die;
}
- $tcp .= 'payload';
- $ip .= $tcp;
- } elsif ($attrs{TP_PROTO} eq 'UDP') {
- my $len = 15;
- my $udp = pack('nnnn', $flow{TP_SRC}, $flow{TP_DST}, $len, 0);
- $udp .= chr($len) while length($udp) < $len;
- $ip .= $udp;
- } elsif ($attrs{TP_PROTO} eq 'ICMP') {
- $ip .= pack('CCnnn',
- 8, # echo request
- 0, # code
- 0, # checksum
- 736, # identifier
- 931); # sequence number
- } elsif ($attrs{TP_PROTO} eq 'other') {
- $ip .= 'other header';
- } else {
- die;
}
substr($ip, 2, 2) = pack('n', length($ip));
$packet .= $ip;
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-flows.c b/tests/test-flows.c
index 8308bf8..6528b07 100644
--- a/tests/test-flows.c
+++ b/tests/test-flows.c
@@ -70,7 +70,7 @@ main(int argc OVS_UNUSED, char *argv[])
in_port_.ofp_port = u16_to_ofp(1);
flow_extract(packet, 0, 0, NULL, &in_port_, &flow);
- match_init_exact(&match, &flow);
+ match_wc_init(&match, &flow);
ofputil_match_to_ofp10_match(&match, &extracted_match);
if (memcmp(&expected_match, &extracted_match, sizeof expected_match)) {
diff --git a/tests/test-multipath.c b/tests/test-multipath.c
index 4ba3692..bf879c7 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,7 @@ main(int argc, char *argv[])
struct flow_wildcards wc;
struct flow flow;
- random_bytes(&flow, sizeof flow);
+ 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