Hi Tao,

Thank you for looking at my patch.
Could you let me know where you saw this error so I can look into it?
I didn't see any failed tests.

Thanks,
Rosemarie

On Wed, Apr 13, 2022 at 10:44 PM Tao Liu <[email protected]> wrote:
>
> On Wed, Apr 13, 2022 at 05:30:19PM -0400, Rosemarie O'Riorden wrote:
> > When OVS sees a tunnel push with a nested list next, it will not
> > clone the packet, as a clone is not needed. However, a clone action will
> > still be created with the tunnel push encapulated inside. There is no
> > need to create the clone action in this case, as extra parsing will need
> > to be performed, which is less efficient.
> >
> > Signed-off-by: Rosemarie O'Riorden <[email protected]>
> > ---
> > v2:
> >  - Fixes tests that are affected by this change
> >
> >  ofproto/ofproto-dpif-xlate.c  | 15 ++++--
> >  tests/nsh.at                  |  6 +--
> >  tests/packet-type-aware.at    | 24 ++++-----
> >  tests/tunnel-push-pop-ipv6.at | 20 ++++----
> >  tests/tunnel-push-pop.at      | 95 +++++++++++++++++++++++++----------
> >  5 files changed, 103 insertions(+), 57 deletions(-)
> >
> > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
> > index 9bd0b0a38..319bb6db5 100644
> > --- a/ofproto/ofproto-dpif-xlate.c
> > +++ b/ofproto/ofproto-dpif-xlate.c
> > @@ -3720,7 +3720,10 @@ native_tunnel_output(struct xlate_ctx *ctx, const 
> > struct xport *xport,
> >      size_t clone_ofs = 0;
> >      size_t push_action_size;
> >
> > -    clone_ofs = nl_msg_start_nested(ctx->odp_actions, 
> > OVS_ACTION_ATTR_CLONE);
> > +    if (!is_last_action) {
> > +        clone_ofs = nl_msg_start_nested(ctx->odp_actions,
> > +                                OVS_ACTION_ATTR_CLONE);
> > +    }
>
> Hi, this breaks tunnel encap offload, we should also consider it in :
>
>     parse_flow_actions
>     parse_clone_actions
>
> >      odp_put_tnl_push_action(ctx->odp_actions, &tnl_push_data);
> >      push_action_size = ctx->odp_actions->size;
> >
> > @@ -3763,10 +3766,12 @@ native_tunnel_output(struct xlate_ctx *ctx, const 
> > struct xport *xport,
> >          }
> >          xlate_cache_steal_entries(backup_xcache, ctx->xin->xcache);
> >
> > -        if (ctx->odp_actions->size > push_action_size) {
> > -            nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
> > -        } else {
> > -            nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
> > +        if (!is_last_action) {
> > +            if (ctx->odp_actions->size > push_action_size) {
> > +                nl_msg_end_non_empty_nested(ctx->odp_actions, clone_ofs);
> > +            } else {
> > +                nl_msg_cancel_nested(ctx->odp_actions, clone_ofs);
> > +            }
> >          }
> >
> >          /* Restore context status. */
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to