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]> --- v2: - Renamed variables - Changed redundant legs in check_pkt_len tests --- ofproto/ofproto-dpif-xlate.c | 26 ++++++++++++++++++++++---- tests/ofproto-dpif.at | 18 +++++++++++++----- tests/system-offloads-traffic.at | 11 ++++++----- 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 4e7d6fb40..38da914e8 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -6872,7 +6872,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 offset_gt_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); @@ -6883,7 +6883,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, offset_gt_attr); xretain_base_flow_restore(ctx, retained_state); xretain_flow_restore(ctx, retained_state); @@ -6897,7 +6897,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 offset_lte_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); @@ -6908,9 +6908,27 @@ 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, offset_lte_attr); + size_t lte_len = ctx->odp_actions->size - offset_lte_attr - NLA_HDRLEN; + size_t gt_len = offset_lte_attr - offset_gt_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 (gt_len == lte_len) { + if (memcmp(ofpbuf_at(ctx->odp_actions, offset_gt_attr + NLA_HDRLEN, + gt_len), + ofpbuf_at(ctx->odp_actions, offset_lte_attr + NLA_HDRLEN, + lte_len), + lte_len) == 0) { + memmove((uint8_t *) ctx->odp_actions->data + offset, + (uint8_t *) ctx->odp_actions->data + offset_gt_attr + + NLA_HDRLEN, + lte_len); + ofpbuf_truncate(ctx->odp_actions, lte_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 efb36e058..6fc0b10d6 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -13597,13 +13597,21 @@ Datapath actions: check_pkt_len(size=200,gt(2),le(drop)) ovs-ofctl del-flows br0 AT_DATA([flows.txt], [dnl -table=0,in_port=1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]] +table=0,in_port=1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,1) +table=1,icmp,reg0=0x1/0x1 actions=4 ]) 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: check_pkt_len(size=200,gt(4),le(drop)) +]) + +dnl chk_pkt_len optimization. +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=17,tos=1,ttl=128,frag=no),udp()'], [0], [stdout]) +AT_CHECK([tail -1 stdout], [0], [dnl +Datapath actions: drop ]) ovs-ofctl del-flows br0 @@ -13649,7 +13657,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 @@ -13683,14 +13691,14 @@ table=0,in_port=1 actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_N table=1,in_port=1,reg1=0x1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,4) table=1,in_port=1,reg1=0x2 actions=output:2 table=1,in_port=1,reg1=0x3 actions=output:4 -table=4,in_port=1 actions=output:3 +table=4,in_port=1,reg0=0x1 actions=output:3 ]) 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: check_pkt_len(size=200,gt(3),le(drop)),2,4 ]) OVS_VSWITCHD_STOP diff --git a/tests/system-offloads-traffic.at b/tests/system-offloads-traffic.at index cfcc5bd6c..c75b056d8 100644 --- a/tests/system-offloads-traffic.at +++ b/tests/system-offloads-traffic.at @@ -449,7 +449,7 @@ table=0,in_port=2 actions=output:1 table=0,in_port=1 actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_NX_REG1[[]],resubmit(,1) table=1,in_port=1,reg1=0x1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,4) table=4,in_port=1,reg0=0x1 actions=output:2 -table=4,in_port=1,reg0=0x0 actions=output:2 +table=4,in_port=1,reg0=0x0 actions=output:2,3 ]) AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt]) @@ -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:check_pkt_len(size=200,gt(3),le(3,4)) in_port(3),eth(),eth_type(0x0800),ipv4(frag=no), packets:19, bytes:11614, used:0.001s, actions:output ]) @@ -621,7 +621,7 @@ table=0,in_port=2 actions=output:1 table=0,in_port=1 actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_NX_REG1[[]],resubmit(,1) table=1,in_port=1,reg1=0x1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,4) table=1,in_port=1,reg1=0x2 actions=output:2 -table=4,in_port=1,reg0=0x1 actions=output:br0 +table=4,in_port=1,reg0=0x1 actions=output:br0,3 table=4,in_port=1,reg0=0x0 actions=output:br0 ]) AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt]) @@ -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([check_pkt_len(size=200,gt(1,4),le(1)),3]) AT_CHECK([ovs-appctl revalidator/wait], [0]) @@ -674,13 +674,14 @@ table=0,in_port=2 actions=output:1 table=0,in_port=1 actions=load:0x1->NXM_NX_REG1[[]],resubmit(,1),load:0x2->NXM_NX_REG1[[]],resubmit(,1) table=1,in_port=1,reg1=0x1 actions=check_pkt_larger(200)->NXM_NX_REG0[[0]],resubmit(,4) table=1,in_port=1,reg1=0x2 actions=output:2 +table=4,in_port=1,reg0=0x1 actions=output:br0 ]) AT_CHECK([ovs-ofctl --protocols=OpenFlow10 add-flows br0 flows.txt]) 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([check_pkt_len(size=200,gt(1),le(drop)),3]) AT_CHECK([ovs-appctl revalidator/wait], [0]) -- 2.55.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
