The decision to add or not the explicit drop action is currently based
on whether the resulting dp action list is empty or not.
This is OK for most of the cases but if per-flow IPFIX/sFlow sampling
is enabled on the bridge, it doesn't work as expected.
Fix this by taking into account the size of these sampling actions.
Fixes: a13a0209750c ("userspace: Improved packet drop statistics.")
Cc: [email protected]
Signed-off-by: Adrian Moreno <[email protected]>
---
ofproto/ofproto-dpif-xlate.c | 5 ++--
tests/drop-stats.at | 44 ++++++++++++++++++++++++++++++++++++
tests/ofproto-dpif.at | 4 ++--
3 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 094fe5d72..323a58cbf 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -8153,6 +8153,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
uint64_t action_set_stub[1024 / 8];
uint64_t frozen_actions_stub[1024 / 8];
uint64_t actions_stub[256 / 8];
+ size_t sample_actions_len = 0;
struct ofpbuf scratch_actions = OFPBUF_STUB_INITIALIZER(actions_stub);
struct xlate_ctx ctx = {
.xin = xin,
@@ -8412,7 +8413,7 @@ xlate_actions(struct xlate_in *xin, struct xlate_out
*xout)
user_cookie_offset = compose_sflow_action(&ctx);
compose_ipfix_action(&ctx, ODPP_NONE);
}
- size_t sample_actions_len = ctx.odp_actions->size;
+ sample_actions_len = ctx.odp_actions->size;
bool ecn_drop = !tnl_process_ecn(flow);
if (!ecn_drop
@@ -8575,7 +8576,7 @@ exit:
}
/* Install drop action if datapath supports explicit drop action. */
- if (xin->odp_actions && !xin->odp_actions->size &&
+ if (xin->odp_actions && xin->odp_actions->size == sample_actions_len &&
ovs_explicit_drop_action_supported(ctx.xbridge->ofproto)) {
put_drop_action(xin->odp_actions, ctx.error);
}
diff --git a/tests/drop-stats.at b/tests/drop-stats.at
index 44c5cc35b..31782ac52 100644
--- a/tests/drop-stats.at
+++ b/tests/drop-stats.at
@@ -192,6 +192,50 @@ ovs-appctl coverage/read-counter
drop_action_too_many_mpls_labels
OVS_VSWITCHD_STOP(["/|WARN|/d"])
AT_CLEANUP
+AT_SETUP([drop-stats - sampling])
+
+OVS_VSWITCHD_START([dnl
+ set bridge br0 datapath_type=dummy \
+ protocols=OpenFlow10,OpenFlow13,OpenFlow14,OpenFlow15 -- \
+ add-port br0 p1 -- set Interface p1 type=dummy ofport_request=1])
+
+AT_DATA([flows.txt], [dnl
+table=0,in_port=1,actions=drop
+])
+
+AT_CHECK([
+ ovs-ofctl del-flows br0
+ ovs-ofctl -Oopenflow13 add-flows br0 flows.txt
+ ovs-ofctl -Oopenflow13 dump-flows br0 | ofctl_strip | sort | grep actions
], [0], [dnl
+ in_port=1 actions=drop
+])
+
+AT_CHECK([ovs-vsctl -- set bridge br0 ipfix=@fix -- \
+ --id=@fix create ipfix targets=\"127.0.0.1:4739\" \
+ sampling=1],
+ [0], [ignore])
+
+AT_CHECK([
+ ovs-appctl netdev-dummy/receive p1
'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'
+ ovs-appctl netdev-dummy/receive p1
'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'
+ ovs-appctl netdev-dummy/receive p1
'in_port(1),packet_type(ns=0,id=0),eth(src=3a:6d:d2:09:9c:ab,dst=1e:2c:e9:2a:66:9e),ipv4(src=192.168.10.10,dst=192.168.10.30,proto=1,tos=0,ttl=64,frag=no),icmp(type=8,code=0)'
+], [0], [ignore])
+
+AT_CHECK([ovs-appctl dpctl/dump-flows | sed
's/used:[[0-9]].[[0-9]]*s/used:0.0/' | sort], [0], [flow-dump from the main
thread:
+recirc_id(0),in_port(1),packet_type(ns=0,id=0),eth_type(0x0800),ipv4(frag=no),
packets:2, bytes:212, used:0.0,
actions:userspace(pid=0,ipfix(output_port=4294967295)),drop
+])
+
+ovs-appctl time/warp 5000
+
+AT_CHECK([
+ovs-appctl coverage/read-counter drop_action_of_pipeline
+], [0], [dnl
+3
+])
+
+OVS_VSWITCHD_STOP(["/sending to collector failed/d"])
+
+AT_CLEANUP
AT_SETUP([drop-stats - sampling action])
OVS_VSWITCHD_START([dnl
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 701b8a851..ba8f3b69c 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -8082,7 +8082,7 @@ for i in `seq 1 3`; do
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 the main thread:
-packets:2, bytes:68, used:0.001s,
actions:userspace(pid=0,ipfix(output_port=4294967295))
+packets:2, bytes:68, used:0.001s,
actions:userspace(pid=0,ipfix(output_port=4294967295)),drop
])
AT_CHECK([ovs-appctl revalidator/purge])
@@ -8118,7 +8118,7 @@ for i in `seq 1 3`; do
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 the main thread:
-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:sample(sample=100.0%,actions(meter(0),userspace(pid=0,ipfix(output_port=4294967295)))),drop
])
dnl Remove the IPFIX configuration.
--
2.45.2
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
