[ovs-dev] [PATCH] ofp-actions: Correct execution of encap/decap actions in action set

[Yang, Yi Y]

Jan, sorry for late reply, I have been waiting for Sangfor guy's confirmation.

With this patch https://patchwork.ozlabs.org/patch/895405/, they won't have any issue any more. So now it can work well, thank you so much.

-----Original Message-----
From: Jan Scheurich [mailto:jan.scheurich at ericsson.com] 
Sent: Monday, April 9, 2018 3:12 PM
To: Yang, Yi Y <yi.y.yang at intel.com>; dev at openvswitch.org
Subject: RE: [PATCH] ofp-actions: Correct execution of encap/decap actions in action set

Hi Yi,

The assertion failure is indeed caused by the incorrect implementation of double encap() and should be fixed by the patch you mention (which is merged to master by now).

Prior to the below fix this happened with every encap(nsh) in an group bucket. 

I can't say why it still happens periodically every few minutes in your test. You'd need to carefully analyze a crash dump to try to understand the packet processing history that leads to a double encap() or perhaps decap().

It is definitely worth trying whether the problem is already resolved on the latest master.

BR, Jan

> -----Original Message-----
> From: Yang, Yi Y [mailto:yi.y.yang at intel.com]
> Sent: Sunday, 08 April, 2018 10:27
> To: Jan Scheurich <jan.scheurich at ericsson.com>; dev at openvswitch.org
> Subject: RE: [PATCH] ofp-actions: Correct execution of encap/decap 
> actions in action set
> 
> Hi, Jan
> 
> Sangfor guy tried this one, he still encountered assert issue after 
> ovs ran for about 20 minutes, moreover it appeared periodically. I'm not sure if https://patchwork.ozlabs.org/patch/895405/ is helpful for this issue. Do you think what the root cause is?
> 
> -----Original Message-----
> From: Jan Scheurich [mailto:jan.scheurich at ericsson.com]
> Sent: Monday, March 26, 2018 3:36 PM
> To: dev at openvswitch.org
> Cc: Yang, Yi Y <yi.y.yang at intel.com>; Jan Scheurich 
> <jan.scheurich at ericsson.com>
> Subject: [PATCH] ofp-actions: Correct execution of encap/decap actions 
> in action set
> 
> The actions encap, decap and dec_nsh_ttl were wrongly flagged as 
> set_field actions in ofpact_is_set_or_move_action(). This caused them 
> to be executed twice in the action set or a group bucket, once 
> explicitly in
> ofpacts_execute_action_set() and once again as part of the list of set_field or move actions.
> 
> Fixes: f839892a ("OF support and translation of generic encap and 
> decap")
> Fixes: 491e05c2 ("nsh: add dec_nsh_ttl action")
> 
> Signed-off-by: Jan Scheurich <jan.scheurich at ericsson.com>
> 
> ---
> 
> The fix should be backported to OVS 2.9 and OVS 2.8 (without the case for OFPACT_DEC_NSH_TTL introduced in 2.9).
> 
> 
>  lib/ofp-actions.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 
> db85716..87797bc 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -6985,9 +6985,6 @@ ofpact_is_set_or_move_action(const struct ofpact *a)
>      case OFPACT_SET_TUNNEL:
>      case OFPACT_SET_VLAN_PCP:
>      case OFPACT_SET_VLAN_VID:
> -    case OFPACT_ENCAP:
> -    case OFPACT_DECAP:
> -    case OFPACT_DEC_NSH_TTL:
>          return true;
>      case OFPACT_BUNDLE:
>      case OFPACT_CLEAR_ACTIONS:
> @@ -7025,6 +7022,9 @@ ofpact_is_set_or_move_action(const struct ofpact *a)
>      case OFPACT_WRITE_METADATA:
>      case OFPACT_DEBUG_RECIRC:
>      case OFPACT_DEBUG_SLOW:
> +    case OFPACT_ENCAP:
> +    case OFPACT_DECAP:
> +    case OFPACT_DEC_NSH_TTL:
>          return false;
>      default:
>          OVS_NOT_REACHED();
> --
> 1.9.1