On Thu, Apr 27, 2017 at 3:42 PM, Jarno Rajahalme <ja...@ovn.org> 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 <az...@ovn.org> wrote:
>>
>> When slowpath meter is configured, add meter action when translate
>> sample action.
>>
>> Signed-off-by: Andy Zhou <az...@ovn.org>
>> ---
>> 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 = &ctx->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
