Currently it's possible for both legs of a check_pkt_len to result in the same actions. In the most extreme example of this, multiple check_pkt_len actions could be chained together, all resulting in a drop. Packet's transiting a check_pkt_len action can get cloned even if the legs don't need, resulting in potentially unneeded memory activity.
This patch checks if both legs of the action are identical, and replaces the entire action with one of the legs if they are. Signed-off-by: Mike Pattrick <[email protected]> --- ofproto/ofproto-dpif-xlate.c | 25 +++++++++++++++++++++---- tests/ofproto-dpif.at | 6 +++--- tests/system-offloads-traffic.at | 6 +++--- 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 764dbd662..410bcd7cd 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -6853,7 +6853,7 @@ xlate_check_pkt_larger(struct xlate_ctx *ctx, OVS_ACTION_ATTR_CHECK_PKT_LEN); nl_msg_put_u16(ctx->odp_actions, OVS_CHECK_PKT_LEN_ATTR_PKT_LEN, check_pkt_larger->pkt_len); - size_t offset_attr = nl_msg_start_nested( + size_t gt_offset_attr = nl_msg_start_nested( ctx->odp_actions, OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_GREATER); value->u8_val = 1; mf_write_subfield_flow(&check_pkt_larger->dst, value, &ctx->xin->flow); @@ -6864,7 +6864,7 @@ xlate_check_pkt_larger(struct xlate_ctx *ctx, if (ctx->freezing) { finish_freezing(ctx); } - nl_msg_end_nested(ctx->odp_actions, offset_attr); + nl_msg_end_nested(ctx->odp_actions, gt_offset_attr); xretain_base_flow_restore(ctx, retained_state); xretain_flow_restore(ctx, retained_state); @@ -6878,7 +6878,7 @@ xlate_check_pkt_larger(struct xlate_ctx *ctx, bool old_exit = ctx->exit; ctx->exit = false; - offset_attr = nl_msg_start_nested( + size_t lte_offset_attr = nl_msg_start_nested( ctx->odp_actions, OVS_CHECK_PKT_LEN_ATTR_ACTIONS_IF_LESS_EQUAL); value->u8_val = 0; mf_write_subfield_flow(&check_pkt_larger->dst, value, &ctx->xin->flow); @@ -6889,9 +6889,26 @@ xlate_check_pkt_larger(struct xlate_ctx *ctx, if (ctx->freezing) { finish_freezing(ctx); } - nl_msg_end_nested(ctx->odp_actions, offset_attr); + nl_msg_end_nested(ctx->odp_actions, lte_offset_attr); + size_t act_len = ctx->odp_actions->size - lte_offset_attr - NLA_HDRLEN; nl_msg_end_nested(ctx->odp_actions, offset); + /* If the two legs are the identical length and content, replace this + * check_pkt_len action with one of the legs. */ + if (lte_offset_attr - gt_offset_attr - NLA_HDRLEN == act_len) { + if (memcmp(ofpbuf_at(ctx->odp_actions, gt_offset_attr + NLA_HDRLEN, + act_len), + ofpbuf_at(ctx->odp_actions, lte_offset_attr + NLA_HDRLEN, + act_len), + act_len) == 0) { + memmove((uint8_t *) ctx->odp_actions->data + offset, + (uint8_t *) ctx->odp_actions->data + gt_offset_attr + + NLA_HDRLEN, + act_len); + ofpbuf_truncate(ctx->odp_actions, act_len + offset); + } + } + ctx->was_mpls = old_was_mpls; ctx->conntracked = old_conntracked; ctx->exit = old_exit; diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index ee6ac873d..569900f8c 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -13568,7 +13568,7 @@ table=0,in_port=1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]] AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: check_pkt_len(size=200,gt(drop),le(drop)) +Datapath actions: drop ]) ovs-ofctl del-flows br0 @@ -13614,7 +13614,7 @@ ovs-ofctl dump-flows br0 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([tail -1 stdout], [0], [dnl -Datapath actions: check_pkt_len(size=200,gt(set(ipv4(src=192.168.3.3)),check_pkt_len(size=200,gt(3),le(3))),le(set(ipv4(src=192.168.3.4)),check_pkt_len(size=200,gt(4),le(4)))) +Datapath actions: check_pkt_len(size=200,gt(set(ipv4(src=192.168.3.3)),3),le(set(ipv4(src=192.168.3.4)),4)) ]) ovs-ofctl del-flows br0 @@ -13655,7 +13655,7 @@ AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt]) AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.10.10.2,dst=10.10.10.1,proto=1,tos=1,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout]) AT_CHECK([cat stdout | grep Datapath -B1], [0], [dnl Megaflow: recirc_id=0,eth,ip,in_port=1,nw_frag=no -Datapath actions: check_pkt_len(size=200,gt(3),le(3)),2,4 +Datapath actions: 3,2,4 ]) OVS_VSWITCHD_STOP diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index a51bba5f9..05e8409f4 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -463,7 +463,7 @@ NS_CHECK_EXEC([at_ns1], [ping -q -c 10 -i 0.1 -W 2 -s 1024 10.1.1.2 | FORMAT_PIN ], [], [ovs-appctl dpctl/dump-flows; ovs-ofctl dump-flows br0]) AT_CHECK([ovs-appctl dpctl/dump-flows | grep "eth_type(0x0800)" | DUMP_CLEAN_SORTED | sed 's/bytes:11348/bytes:11614/'], [0], [dnl -in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:19, bytes:11614, used:0.001s, actions:check_pkt_len(size=200,gt(3),le(3)) +in_port(2),eth(),eth_type(0x0800),ipv4(frag=no), packets:19, bytes:11614, used:0.001s, actions:output in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:19, bytes:11614, used:0.001s, actions:output ]) @@ -629,7 +629,7 @@ sleep 1 NS_CHECK_EXEC([at_ns1], [ping -q -c 10 -i 0.1 -W 2 -s 64 10.1.1.2 | FORMAT_PING], [0], [dnl 10 packets transmitted, 10 received, 0% packet loss, time 0ms ], [], [ovs-appctl dpctl/dump-flows; ovs-ofctl dump-flows br0]) -OVS_CHECK_ACTIONS([check_pkt_len(size=200,gt(1),le(1)),3]) +OVS_CHECK_ACTIONS([1,3]) AT_CHECK([ovs-appctl revalidator/wait], [0]) @@ -680,7 +680,7 @@ sleep 1 NS_CHECK_EXEC([at_ns1], [ping -q -c 10 -i 0.1 -W 2 -s 64 10.1.1.2 | FORMAT_PING], [0], [dnl 10 packets transmitted, 10 received, 0% packet loss, time 0ms ], [], [ovs-appctl dpctl/dump-flows; ovs-ofctl dump-flows br0]) -OVS_CHECK_ACTIONS([check_pkt_len(size=200,gt(drop),le(drop)),3]) +OVS_CHECK_ACTIONS([3]) AT_CHECK([ovs-appctl revalidator/wait], [0]) -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
