[ovs-dev] [PATCH] tun-metadata: Always set option present when copying data.
Jesse Gross
jesse at nicira.com
Fri Aug 14 02:59:29 UTC 2015
Whenever we write into a tunnel option field, we also need to mark
it as significant. If we don't, then the data will later be ignored.
We currently do this in every case except for flow metadata. This causes
us to not correctly serialize the tunnel metadata for Packet Ins to the
controller.
Rather than separately writing the data and marking the options as present,
it is better to combine the two steps to ensure that one can never be
done without the other.
Signed-off-by: Jesse Gross <jesse at nicira.com>
---
lib/tun-metadata.c | 27 ++++++++++++++-------------
tests/tunnel-push-pop.at | 16 +++++++++++++---
2 files changed, 27 insertions(+), 16 deletions(-)
diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
index 216d5e4..acaacff 100644
--- a/lib/tun-metadata.c
+++ b/lib/tun-metadata.c
@@ -60,7 +60,8 @@ static enum ofperr tun_metadata_add_entry(struct tun_table *map, uint8_t idx,
static void tun_metadata_del_entry(struct tun_table *map, uint8_t idx)
OVS_REQUIRES(tab_mutex);
static void memcpy_to_metadata(struct tun_metadata *dst, const void *src,
- const struct tun_metadata_loc *);
+ const struct tun_metadata_loc *,
+ unsigned int idx);
static void memcpy_from_metadata(void *dst, const struct tun_metadata *src,
const struct tun_metadata_loc *);
@@ -273,10 +274,8 @@ tun_metadata_write(struct flow_tnl *tnl,
}
loc = &map->entries[idx].loc;
-
- ULLONG_SET1(tnl->metadata.present.map, idx);
memcpy_to_metadata(&tnl->metadata,
- value->tun_metadata + mf->n_bytes - loc->len, loc);
+ value->tun_metadata + mf->n_bytes - loc->len, loc, idx);
}
static const struct tun_metadata_loc *
@@ -355,8 +354,8 @@ tun_metadata_set_match(const struct mf_field *mf, const union mf_value *value,
mask->tun_metadata[data_offset + i];
}
}
- ULLONG_SET1(match->flow.tunnel.metadata.present.map, idx);
- memcpy_to_metadata(&match->flow.tunnel.metadata, data.tun_metadata, loc);
+ memcpy_to_metadata(&match->flow.tunnel.metadata, data.tun_metadata,
+ loc, idx);
if (!value) {
memset(data.tun_metadata, 0, loc->len);
@@ -365,8 +364,8 @@ tun_metadata_set_match(const struct mf_field *mf, const union mf_value *value,
} else {
memcpy(data.tun_metadata, mask->tun_metadata + data_offset, loc->len);
}
- ULLONG_SET1(match->wc.masks.tunnel.metadata.present.map, idx);
- memcpy_to_metadata(&match->wc.masks.tunnel.metadata, data.tun_metadata, loc);
+ memcpy_to_metadata(&match->wc.masks.tunnel.metadata, data.tun_metadata,
+ loc, idx);
}
static bool
@@ -427,11 +426,11 @@ tun_metadata_get_fmd(const struct flow_tnl *tnl, struct match *flow_metadata)
memcpy_from_metadata(opts.tun_metadata, &flow.metadata, old_loc);
memcpy_to_metadata(&flow_metadata->flow.tunnel.metadata,
- opts.tun_metadata, new_loc);
+ opts.tun_metadata, new_loc, i);
memset(opts.tun_metadata, 0xff, old_loc->len);
memcpy_to_metadata(&flow_metadata->wc.masks.tunnel.metadata,
- opts.tun_metadata, new_loc);
+ opts.tun_metadata, new_loc, i);
}
}
@@ -456,7 +455,7 @@ tun_meta_find_key(const struct hmap *hmap, uint32_t key)
static void
memcpy_to_metadata(struct tun_metadata *dst, const void *src,
- const struct tun_metadata_loc *loc)
+ const struct tun_metadata_loc *loc, unsigned int idx)
{
const struct tun_metadata_loc_chain *chain = &loc->c;
int addr = 0;
@@ -467,6 +466,8 @@ memcpy_to_metadata(struct tun_metadata *dst, const void *src,
addr += chain->len;
chain = chain->next;
}
+
+ ULLONG_SET1(dst->present.map, idx);
}
static void
@@ -654,8 +655,8 @@ tun_metadata_from_geneve__(const struct tun_metadata *flow_metadata,
flow_opt->type));
if (entry) {
if (entry->loc.len == flow_opt->length * 4) {
- memcpy_to_metadata(metadata, opt + 1, &entry->loc);
- ULLONG_SET1(metadata->present.map, entry - map->entries);
+ memcpy_to_metadata(metadata, opt + 1, &entry->loc,
+ entry - map->entries);
} else {
return EINVAL;
}
diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index 0f1724a..a34af3f 100644
--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -123,16 +123,26 @@ AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 3'], [0], [dnl
])
dnl Check decapsulation of Geneve packet with options
+AT_CAPTURE_FILE([ofctl_monitor.log])
+AT_CHECK([ovs-ofctl monitor int-br 65534 --detach --no-chdir --pidfile 2> ofctl_monitor.log])
+
AT_CHECK([ovs-ofctl del-flows int-br])
-AT_CHECK([ovs-ofctl add-flow int-br "tun_metadata0=0xa/0xf,actions=5"])
+AT_CHECK([ovs-ofctl add-flow int-br "tun_metadata0=0xa/0xf,actions=5,controller"])
AT_CHECK([ovs-appctl netdev-dummy/receive p0 '001b213cac30001b213cab64080045000096794640004011ba630101025c01010258308817c1008200000400655800007b00ffff80010000000affff00010000000bfe71d883724fbeb6f4e1494a080045000054ba200000400184861e0000011e00000200004227e75400030af3195500000000f265010000000000101112131415161718191a1b1c1d1e1f202122232425262728292a2b2c2d2e2f3031323334353637'])
-ovs-appctl time/warp 1000
+OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 2])
+OVS_APP_EXIT_AND_WAIT(ovs-ofctl)
+
+AT_CHECK([cat ofctl_monitor.log], [0], [dnl
+NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=98 tun_id=0x7b,tun_src=1.1.2.92,tun_dst=1.1.2.88,tun_metadata0=0xa,in_port=5 (via action) data_len=98 (unbuffered)
+icmp,vlan_tci=0x0000,dl_src=be:b6:f4:e1:49:4a,dl_dst=fe:71:d8:83:72:4f,nw_src=30.0.0.1,nw_dst=30.0.0.2,nw_tos=0,nw_ecn=0,nw_ttl=64,icmp_type=0,icmp_code=0 icmp_csum:4227
+])
+
AT_CHECK([ovs-ofctl dump-ports int-br | grep 'port 5'], [0], [dnl
port 5: rx pkts=1, bytes=98, drop=0, errs=0, frame=0, over=0, crc=0
])
AT_CHECK([ovs-appctl dpif/dump-flows int-br], [0], [dnl
-tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,ttl=64,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),skb_mark(0),recirc_id(0),in_port(6081),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:drop
+tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,ttl=64,geneve({class=0xffff,type=0x80,len=4,0xa/0xf}{class=0xffff,type=0,len=4}),flags(-df-csum+key)),skb_mark(0),recirc_id(0),in_port(6081),eth_type(0x0800),ipv4(frag=no), packets:0, bytes:0, used:never, actions:userspace(pid=0,slow_path(controller))
])
OVS_VSWITCHD_STOP
--
2.1.4
More information about the dev
mailing list