In order to collect meters since the last meter action
to the current action in the dpif_execute_helper_cb,
the next entry is simply obtained using nl_attr_next, and
not considering the wrong nested nl without end_nested.
This will cause memory access to be overflowed.
Gdb debug looks like this:
(gdb) print a
$6 = (const struct nlattr *) 0xaaaad175a69c
(gdb) n
1206 a = nl_attr_next(a);
(gdb) n
1207 } while (a != action &&
(gdb) n
1206 a = nl_attr_next(a);
(gdb) print a
$7 = (const struct nlattr *) 0xaaaad175a69c
(gdb) print *a
$8 = {nla_len = 0, nla_type = 0}
(gdb)
Fixes: 076caa2fb077 ("ofproto: Meter translation.")
Signed-off-by: Daniel Ding <danieldin...@gmail.com>
---
lib/dpif.c | 43 ++++++++++++++++--------------------------
tests/system-dpdk.at | 45 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+), 27 deletions(-)
diff --git a/lib/dpif.c b/lib/dpif.c
index d07241f1e..05dfd2919 100644
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1158,11 +1158,14 @@ dpif_flow_dump_next(struct dpif_flow_dump_thread
*thread,
return n;
}
+enum { EXECUTE_HELPER_MAX_METER = 32 };
+
struct dpif_execute_helper_aux {
struct dpif *dpif;
const struct flow *flow;
int error;
- const struct nlattr *meter_action; /* Non-NULL, if have a meter action. */
+ size_t meter_count;
+ const struct nlattr *meter_actions[EXECUTE_HELPER_MAX_METER + 1];
};
/* This is called for actions that need the context of the datapath to be
@@ -1179,9 +1182,9 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch
*packets_,
switch ((enum ovs_action_attr)type) {
case OVS_ACTION_ATTR_METER:
- /* Maintain a pointer to the first meter action seen. */
- if (!aux->meter_action) {
- aux->meter_action = action;
+ /* Records the meter actions. */
+ if (aux->meter_count < EXECUTE_HELPER_MAX_METER) {
+ aux->meter_actions[aux->meter_count++] = action;
}
break;
@@ -1197,27 +1200,15 @@ dpif_execute_helper_cb(void *aux_, struct
dp_packet_batch *packets_,
uint64_t stub[256 / 8];
struct pkt_metadata *md = &packet->md;
- if (flow_tnl_dst_is_set(&md->tunnel) || aux->meter_action) {
+ if (flow_tnl_dst_is_set(&md->tunnel) || aux->meter_count) {
ofpbuf_use_stub(&execute_actions, stub, sizeof stub);
- if (aux->meter_action) {
- const struct nlattr *a = aux->meter_action;
-
- /* XXX: This code collects meter actions since the last action
- * execution via the datapath to be executed right before the
- * current action that needs to be executed by the datapath.
- * This is only an approximation, but better than nothing.
- * Fundamentally, we should have a mechanism by which the
- * datapath could return the result of the meter action so that
- * we could execute them at the right order. */
- do {
+ if (aux->meter_count) {
+ size_t i;
+ for (i = 0; i < aux->meter_count; i++) {
+ const struct nlattr *a = aux->meter_actions[i];
ofpbuf_put(&execute_actions, a, NLA_ALIGN(a->nla_len));
- /* Find next meter action before 'action', if any. */
- do {
- a = nl_attr_next(a);
- } while (a != action &&
- nl_attr_type(a) != OVS_ACTION_ATTR_METER);
- } while (a != action);
+ }
}
/* The Linux kernel datapath throws away the tunnel information
@@ -1261,11 +1252,9 @@ dpif_execute_helper_cb(void *aux_, struct
dp_packet_batch *packets_,
dp_packet_delete(clone);
- if (flow_tnl_dst_is_set(&md->tunnel) || aux->meter_action) {
+ if (flow_tnl_dst_is_set(&md->tunnel) || aux->meter_count) {
ofpbuf_uninit(&execute_actions);
-
- /* Do not re-use the same meters for later output actions. */
- aux->meter_action = NULL;
+ aux->meter_count = 0;
}
break;
}
@@ -1303,7 +1292,7 @@ dpif_execute_helper_cb(void *aux_, struct dp_packet_batch
*packets_,
static int
dpif_execute_with_help(struct dpif *dpif, struct dpif_execute *execute)
{
- struct dpif_execute_helper_aux aux = {dpif, execute->flow, 0, NULL};
+ struct dpif_execute_helper_aux aux = {dpif, execute->flow, 0, 0, {NULL}};
struct dp_packet_batch pb;
COVERAGE_INC(dpif_execute_with_help);
diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 1c97bf777..43b41a9f2 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -863,3 +863,48 @@ AT_CHECK([ovs-vsctl del-port br10 p1], [], [stdout],
[stderr])
OVS_DPDK_STOP_VSWITCHD
AT_CLEANUP
dnl --------------------------------------------------------------------------
+
+dnl --------------------------------------------------------------------------
+dnl Setup dpif collecting meters
+AT_SETUP([OVS-DPDK - dpif collecting meters])
+AT_KEYWORDS([dpdk])
+
+OVS_DPDK_PRE_CHECK()
+OVS_DPDK_START([--no-pci])
+
+AT_CHECK([ovs-vsctl add-br br-vm -- set bridge br-vm datapath_type=netdev])
+AT_CHECK([ovs-vsctl add-br br-phy -- set bridge br-phy datapath_type=netdev])
+AT_CHECK([ovs-vsctl add-port br-vm p1 -- set Interface p1 type=vxlan
options:remote_ip=1.1.1.1 options:local_ip=1.1.1.2])
+
+dnl Active vxlan egress network
+AT_CHECK([ip link set up dev br-phy])
+AT_CHECK([ip addr add 1.1.1.2/24 dev br-phy])
+
+ADD_NAMESPACES(ns1)
+# ADD_VETH(tap1, ns1, br-phy, "1.1.1.1/24")
+
+# NOT dpdk type with net_af_xdp
+AT_CHECK([ip link add tap1 type veth peer name ovs-tap1])
+AT_CHECK([ip link set up dev ovs-tap1])
+AT_CHECK([ip link set netns ns1 dev tap1])
+
+AT_CHECK([ovs-vsctl add-port br-phy ovs-tap1])
+
+NS_CHECK_EXEC([ns1], [ip link set up dev tap1])
+NS_CHECK_EXEC([ns1], [ip addr add 1.1.1.1/24 dev tap1])
+
+AT_CHECK([ping -c 3 1.1.1.1], [0], [stdout])
+OVS_WAIT_UNTIL([test -n "`ovs-appctl tnl/arp/show | grep 1.1.1.1`"])
+
+dnl Force packet-out into collecting meters
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-meter br-vm 'meter=1 pktps
bands=type=drop rate=1'])
+AT_CHECK([ovs-ofctl -O OpenFlow13 add-flow br-vm 'table=2,arp
actions=meter:1,set_field:2->arp_op,p1,br-vm'])
+
+dnl Send a arp packet-out message
+AT_CHECK([ovs-ofctl packet-out br-vm 'in_port=controller
packet=ffffffffffff000000010102080600010800060400010000000101020101010300000000000001010102
actions=resubmit(,2)'])
+
+dnl Clean up
+OVS_DPDK_STOP_VSWITCHD
+AT_CHECK([ip link del ovs-tap1])
+AT_CLEANUP
+dnl --------------------------------------------------------------------------
--
2.43.0
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
