[ovs-dev] [PATCH v2 09/10] tc: Move tunnel_key unset action before output ports

Marcelo Ricardo Leitner mleitner at redhat.com
Wed Nov 27 15:33:54 UTC 2019


On Wed, Nov 27, 2019 at 02:55:15PM +0200, Roi Dayan wrote:
> From: Paul Blakey <paulb at mellanox.com>
> 
> Since OvS datapath gets packets already decapsulated from tunnel devices,
> it doesn't explicitly decapsulate them. So in a recirculation setup,
> the tunnel matching continues in the recirculation as the tunnel
> metadata still exists on the SKB.
> 
> Tunnel key unset action unsets this metadata. Some drivers might rely
> on this explicit tunnel key unset to know when to decapsulate the packet
> instead of the device type. So instead of removing it completly,
> we move it near the output actions.

Hm, you said 'move', but the patch is only 'adding' stuff.

Did you intend to:

diff --git a/lib/tc.c b/lib/tc.c
index 1541629a58e8..00d7f2f2239d 100644
--- a/lib/tc.c
+++ b/lib/tc.c
@@ -2375,13 +2375,6 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
     {
         int error;
 
-        if (flower->tunnel) {
-            act_offset = nl_msg_start_nested(request, act_index++);
-            nl_msg_put_act_tunnel_key_release(request);
-            nl_msg_put_act_flags(request);
-            nl_msg_end_nested(request, act_offset);
-        }
-
         action = flower->actions;
         for (i = 0; i < flower->action_count; i++, action++) {
             switch (action->type) {

?

> 
> This way, we also keep SKB metadata through recirculation, and for
> non-recirculation rules, the resulting tc rules should remain the same.
> 
> Signed-off-by: Paul Blakey <paulb at mellanox.com>
> ---
>  lib/tc.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/tc.c b/lib/tc.c
> index 83ee0f6473aa..1541629a58e8 100644
> --- a/lib/tc.c
> +++ b/lib/tc.c
> @@ -2364,12 +2364,12 @@ nl_msg_put_flower_rewrite_pedits(struct ofpbuf *request,
>  static int
>  nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
>  {
> +    bool ingress, released = false;
>      size_t offset;
>      size_t act_offset;
>      uint16_t act_index = 1;
>      struct tc_action *action;
>      int i, ifindex = 0;
> -    bool ingress;
>  
>      offset = nl_msg_start_nested(request, TCA_FLOWER_ACT);
>      {
> @@ -2458,6 +2458,13 @@ nl_msg_put_flower_acts(struct ofpbuf *request, struct tc_flower *flower)
>              }
>              break;
>              case TC_ACT_OUTPUT: {
> +                if (!released && flower->tunnel) {
> +                    act_offset = nl_msg_start_nested(request, act_index++);
> +                    nl_msg_put_act_tunnel_key_release(request);
> +                    nl_msg_end_nested(request, act_offset);
> +                    released = true;
> +                }
> +
>                  ingress = action->out.ingress;
>                  ifindex = action->out.ifindex_out;
>                  if (ifindex < 1) {
> -- 
> 2.8.4
> 



More information about the dev mailing list