Message ID | 1492199182-4479-5-git-send-email-azhou@ovn.org |
---|---|
State | Superseded |
Headers | show |
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 <azhou@ovn.org> wrote: > > When slowpath meter is configured, add meter action when translate > sample action. > > Signed-off-by: Andy Zhou <azhou@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. > 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 > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Thu, Apr 27, 2017 at 3:42 PM, Jarno Rajahalme <jarno@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 <azhou@ovn.org> wrote: >> >> When slowpath meter is configured, add meter action when translate >> sample action. >> >> Signed-off-by: Andy Zhou <azhou@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 >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
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)))) +]) + dnl Remove the IPFIX configuration. AT_CHECK([ovs-vsctl clear bridge br0 ipfix]) AT_CHECK([ovs-appctl revalidator/purge])
When slowpath meter is configured, add meter action when translate sample action. Signed-off-by: Andy Zhou <azhou@ovn.org> --- ofproto/ofproto-dpif-xlate.c | 9 +++++++++ tests/ofproto-dpif.at | 14 ++++++++++++++ 2 files changed, 23 insertions(+)