On 9/21/26 6:01 PM, Mike Pattrick via dev wrote: > 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); Can this break the last_observe_offset that would corrupt the actions later or write out of bounds?
I also wonder if this optimization belongs to xlate_tweak_odp_actions() as that's the centralized place for applying this type of optimizations. Though it will for sure be more expensive there... Best regards, Ilya Maximets. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
