On 10 August 2015 at 18:46, Ethan J. Jackson <[email protected]> wrote:
> exit:
> - if (ok) {
> + if (result != UKEY_DELETE) {
> ukey->reval_seq = reval_seq;
> }
> - if (netflow && !ok) {
> + if (netflow && result != UKEY_DELETE) {
> netflow_flow_clear(netflow, &flow);
> }
I think netflow should be cleared only if we delete.
> + if (purge) {
> + result = UKEY_DELETE;
> + } else if (ukey->dump_seq == dump_seq
> + || ukey->reval_seq == reval_seq) {
> + result = UKEY_KEEP;
Indentation in elseif.
> - } else if (!flow_exists) {
> - ovs_mutex_lock(&umap->mutex);
> - ukey_delete(umap, ukey);
> - ovs_mutex_unlock(&umap->mutex);
> + } else if (result == UKEY_MODIFY) {
> + ofpbuf_delete(ukey->actions);
> + ukey->actions = ofpbuf_clone(&odp_actions);
> + modify_op_init(&ops[n_ops++], ukey);
Per-batch push_ukey_ops() occurs for UKEY_DELETE case, but not
UKEY_MODIFY. this may allow n_ops to exceed REVALIDATE_MAX_BATCH if a
bunch of modify operations all occur at once.
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 58c426b..121f84d 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -6985,3 +6985,43 @@ recirc_id=0,ip,in_port=1,dl_vlan=10,nw_frag=no,
> actions: <del>
> ])
> OVS_VSWITCHD_STOP
> AT_CLEANUP
> +
> +# Tests in place modification of installed datapath flows.
> +AT_SETUP([ofproto-dpif - in place modification])
> +OVS_VSWITCHD_START(
> + [add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1])
> +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg])
> +
> +AT_CHECK([ovs-ofctl del-flows br0])
> +AT_CHECK([ovs-ofctl add-flow br0
> in_port=1,actions=mod_vlan_vid:3,output:local])
> +
> +ovs-appctl vlog/set PATTERN:ANY:'%c|%p|%m'
> +
> +ovs-appctl time/stop
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1
> 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x1234)'
> +done
> +
> +AT_CHECK([ovs-appctl dpif/dump-flows br0 | STRIP_UFID | STRIP_USED | sort],
> [0], [dnl
> +recirc_id(0),in_port(1),eth_type(0x1234), packets:2, bytes:120, used:0.0s,
> actions:push_vlan(vid=3,pcp=0),100
> +])
> +
> +AT_CHECK([ovs-ofctl add-flow br0
> priority=60000,in_port=1,actions=mod_vlan_vid:4,output:local])
> +
> +ovs-appctl time/warp 500
> +ovs-appctl time/warp 500
> +
> +for i in 1 2 3; do
> + ovs-appctl netdev-dummy/receive p1
> 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x1234)'
> +done
> +
> +AT_CHECK([ovs-appctl dpif/dump-flows br0 | STRIP_UFID | STRIP_USED | sort],
> [0], [dnl
> +recirc_id(0),in_port(1),eth_type(0x1234), packets:5, bytes:300, used:0.0s,
> actions:push_vlan(vid=4,pcp=0),100
> +])
> +
> +AT_CHECK([cat ovs-vswitchd.log | grep 'modify' | STRIP_UFID ], [0], [dnl
> +dpif|DBG|dummy@ovs-dummy: put[[modify]]
> skb_priority(0/0),skb_mark(0/0),recirc_id(0),dp_hash(0/0),in_port(1),eth(src=50:54:00:00:00:09/00:00:00:00:00:00,dst=50:54:00:00:00:0a/00:00:00:00:00:00),eth_type(0x1234),
> actions:push_vlan(vid=4,pcp=0),100
> +])
> +OVS_VSWITCHD_STOP
> +AT_CLEANUP
Have you tried running this test on travis or a similar environment
under high load? I wonder if it's prone to race conditions, because
the test script doesn't do any waiting to ensure that the packets are
handled before it checks the logs.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
