Re: [ovs-dev] [action upcall meters 4/5] ofproto: Meter sample action when configured.

2017-04-28 Thread Andy Zhou 
On Thu, Apr 27, 2017 at 3:42 PM, Jarno Rajahalme  wrote:
> This breaks the test "ofproto-dpif - Bridge IPFIX sanity check” (currently 
> test #1128), which appears to be the tests case that is being modified.
Oops. As you have noticed with patch 5, I messed up in splitting those patches.
>
> More comments below.
>
>> On Apr 14, 2017, at 12:46 PM, Andy Zhou  wrote:
>>
>> When slowpath meter is configured, add meter action when translate
>> sample action.
>>
>> Signed-off-by: Andy Zhou 
>> ---
>> ofproto/ofproto-dpif-xlate.c |  9 +
>> tests/ofproto-dpif.at| 14 ++
>> 2 files changed, 23 insertions(+)
>>
>> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>> index a24aef9a43a1..52e0d3f1b0bb 100644
>> --- a/ofproto/ofproto-dpif-xlate.c
>> +++ b/ofproto/ofproto-dpif-xlate.c
>> @@ -2861,6 +2861,15 @@ compose_sample_action(struct xlate_ctx *ctx,
>>  OVS_SAMPLE_ATTR_ACTIONS);
>> }
>>
>> +/* If the slow path meter is configured by the controller,
>> + * Insert a meter action before the user space action.   */
>> +struct ofproto *ofproto = >xin->ofproto->up;
>> +uint32_t meter_id = ofproto->slowpath_meter_id;
>> +
>> +if (meter_id != UINT32_MAX) {
>> +nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER, meter_id);
>> +}
>> +
>> odp_port_t odp_port = ofp_port_to_odp_port(
>> ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
>> uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port,
>> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>> index 0c2ea384b422..3c3037b16548 100644
>> --- a/tests/ofproto-dpif.at
>> +++ b/tests/ofproto-dpif.at
>> @@ -6491,6 +6491,20 @@ flow-dump from non-dpdk interfaces:
>> packets:2, bytes:68, used:0.001s, 
>> actions:userspace(pid=0,ipfix(output_port=4294967295))
>> ])
>>
>> +AT_CHECK([ovs-appctl revalidator/purge])
>> +dnl
>> +dnl Add a slowpath meter. The userspace action should be metered.
>> +AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=slowpath pktps burst 
>> stats bands=type=drop rate=3 burst_size=1'])
>> +
>> +dnl Send some packets that should be sampled and metered.
>> +for i in `seq 1 3`; do
>> +AT_CHECK([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(0x0800)'])
>> +done
>> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/.*\(packets:\)/\1/' | sed 
>> 's/used:[[0-9]].[[0-9]]*s/used:0.001s/'], [0], [dnl
>> +flow-dump from non-dpdk interfaces:
>> +packets:2, bytes:68, used:0.001s, 
>> actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,ipfix(output_port=4294967295
>> +])
>> +
>
> This is the test failure:
>
> -packets:2, bytes:68, used:0.001s, 
> actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,ipfix(output_port=4294967295
> +packets:2, bytes:68, used:0.001s, 
> actions:meter(0),userspace(pid=0,ipfix(output_port=4294967295))
>
> Applied to current master the sample envelope is not being inserted when 
> probability is 100%. However, when using a meter the sample envelope is 
> needed in all cases, as if the meter drops the packet, we still need to 
> continue execution if there are further actions after the sample action.
>
I agree, the test is correct, the logic is correct in patch 5, but not
here. Will fix.
>
>> dnl Remove the IPFIX configuration.
>> AT_CHECK([ovs-vsctl clear bridge br0 ipfix])
>> AT_CHECK([ovs-appctl revalidator/purge])
>> --
>> 1.8.3.1
>>
>> ___
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [action upcall meters 4/5] ofproto: Meter sample action when configured.

2017-04-27 Thread Jarno Rajahalme 
This breaks the test "ofproto-dpif - Bridge IPFIX sanity check” (currently test 
#1128), which appears to be the tests case that is being modified.

More comments below.

> On Apr 14, 2017, at 12:46 PM, Andy Zhou  wrote:
> 
> When slowpath meter is configured, add meter action when translate
> sample action.
> 
> Signed-off-by: Andy Zhou 
> ---
> ofproto/ofproto-dpif-xlate.c |  9 +
> tests/ofproto-dpif.at| 14 ++
> 2 files changed, 23 insertions(+)
> 
> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> index a24aef9a43a1..52e0d3f1b0bb 100644
> --- a/ofproto/ofproto-dpif-xlate.c
> +++ b/ofproto/ofproto-dpif-xlate.c
> @@ -2861,6 +2861,15 @@ compose_sample_action(struct xlate_ctx *ctx,
>  OVS_SAMPLE_ATTR_ACTIONS);
> }
> 
> +/* If the slow path meter is configured by the controller,
> + * Insert a meter action before the user space action.   */
> +struct ofproto *ofproto = >xin->ofproto->up;
> +uint32_t meter_id = ofproto->slowpath_meter_id;
> +
> +if (meter_id != UINT32_MAX) {
> +nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER, meter_id);
> +}
> +
> odp_port_t odp_port = ofp_port_to_odp_port(
> ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
> uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port,
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 0c2ea384b422..3c3037b16548 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -6491,6 +6491,20 @@ flow-dump from non-dpdk interfaces:
> packets:2, bytes:68, used:0.001s, 
> actions:userspace(pid=0,ipfix(output_port=4294967295))
> ])
> 
> +AT_CHECK([ovs-appctl revalidator/purge])
> +dnl
> +dnl Add a slowpath meter. The userspace action should be metered.
> +AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=slowpath pktps burst 
> stats bands=type=drop rate=3 burst_size=1'])
> +
> +dnl Send some packets that should be sampled and metered.
> +for i in `seq 1 3`; do
> +AT_CHECK([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(0x0800)'])
> +done
> +AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/.*\(packets:\)/\1/' | sed 
> 's/used:[[0-9]].[[0-9]]*s/used:0.001s/'], [0], [dnl
> +flow-dump from non-dpdk interfaces:
> +packets:2, bytes:68, used:0.001s, 
> actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,ipfix(output_port=4294967295
> +])
> +

This is the test failure:

-packets:2, bytes:68, used:0.001s, 
actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,ipfix(output_port=4294967295
+packets:2, bytes:68, used:0.001s, 
actions:meter(0),userspace(pid=0,ipfix(output_port=4294967295))

Applied to current master the sample envelope is not being inserted when 
probability is 100%. However, when using a meter the sample envelope is needed 
in all cases, as if the meter drops the packet, we still need to continue 
execution if there are further actions after the sample action.


> dnl Remove the IPFIX configuration.
> AT_CHECK([ovs-vsctl clear bridge br0 ipfix])
> AT_CHECK([ovs-appctl revalidator/purge])
> -- 
> 1.8.3.1
> 
> ___
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [action upcall meters 4/5] ofproto: Meter sample action when configured.

2017-04-14 Thread Andy Zhou 
When slowpath meter is configured, add meter action when translate
sample action.

Signed-off-by: Andy Zhou 
---
 ofproto/ofproto-dpif-xlate.c |  9 +
 tests/ofproto-dpif.at| 14 ++
 2 files changed, 23 insertions(+)

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a24aef9a43a1..52e0d3f1b0bb 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2861,6 +2861,15 @@ compose_sample_action(struct xlate_ctx *ctx,
  OVS_SAMPLE_ATTR_ACTIONS);
 }
 
+/* If the slow path meter is configured by the controller,
+ * Insert a meter action before the user space action.   */
+struct ofproto *ofproto = >xin->ofproto->up;
+uint32_t meter_id = ofproto->slowpath_meter_id;
+
+if (meter_id != UINT32_MAX) {
+nl_msg_put_u32(ctx->odp_actions, OVS_ACTION_ATTR_METER, meter_id);
+}
+
 odp_port_t odp_port = ofp_port_to_odp_port(
 ctx->xbridge, ctx->xin->flow.in_port.ofp_port);
 uint32_t pid = dpif_port_get_pid(ctx->xbridge->dpif, odp_port,
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 0c2ea384b422..3c3037b16548 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -6491,6 +6491,20 @@ flow-dump from non-dpdk interfaces:
 packets:2, bytes:68, used:0.001s, 
actions:userspace(pid=0,ipfix(output_port=4294967295))
 ])
 
+AT_CHECK([ovs-appctl revalidator/purge])
+dnl
+dnl Add a slowpath meter. The userspace action should be metered.
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br0 'meter=slowpath pktps burst 
stats bands=type=drop rate=3 burst_size=1'])
+
+dnl Send some packets that should be sampled and metered.
+for i in `seq 1 3`; do
+AT_CHECK([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(0x0800)'])
+done
+AT_CHECK([ovs-appctl dpctl/dump-flows | sed 's/.*\(packets:\)/\1/' | sed 
's/used:[[0-9]].[[0-9]]*s/used:0.001s/'], [0], [dnl
+flow-dump from non-dpdk interfaces:
+packets:2, bytes:68, used:0.001s, 
actions:sample(sample=100.0%,actions(meter(0),userspace(pid=0,ipfix(output_port=4294967295
+])
+
 dnl Remove the IPFIX configuration.
 AT_CHECK([ovs-vsctl clear bridge br0 ipfix])
 AT_CHECK([ovs-appctl revalidator/purge])
-- 
1.8.3.1

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev