diff mbox series

[ovs-dev] utilities: Correct deletion reason in flow_reval_monitor.py.

Message ID 1b2e8192cbbea9164e45c2c17607ecabbe9807cf.1714644699.git.echaudro@redhat.com
State Superseded
Delegated to: Eelco Chaudron
Headers show
Series [ovs-dev] utilities: Correct deletion reason in flow_reval_monitor.py. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

Eelco Chaudron May 2, 2024, 10:11 a.m. UTC
The flow_reval_monitor.py script incorrectly reported the reasons for
FDR_PURGE and FDR_TOO_EXPENSIVE, as their descriptions were swapped.
This patch rectifies the order.

Fixes: 86b9e653ef22 ("revalidator: Add a USDT probe during flow deletion with purge reason.")
Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
---
 utilities/usdt-scripts/flow_reval_monitor.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Adrian Moreno May 3, 2024, 7:36 a.m. UTC | #1
On 5/2/24 12:11 PM, Eelco Chaudron wrote:
> The flow_reval_monitor.py script incorrectly reported the reasons for
> FDR_PURGE and FDR_TOO_EXPENSIVE, as their descriptions were swapped.
> This patch rectifies the order.
> 
> Fixes: 86b9e653ef22 ("revalidator: Add a USDT probe during flow deletion with purge reason.")
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>   utilities/usdt-scripts/flow_reval_monitor.py | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utilities/usdt-scripts/flow_reval_monitor.py b/utilities/usdt-scripts/flow_reval_monitor.py
> index 534ba8fa2..117f5bc27 100755
> --- a/utilities/usdt-scripts/flow_reval_monitor.py
> +++ b/utilities/usdt-scripts/flow_reval_monitor.py
> @@ -262,8 +262,8 @@ FdrReasonStrings = [
>       "Kill all flows condition detected",
>       "Mask too wide - need narrower match",
>       "No matching ofproto rules",
> -    "Too expensive to revalidate",
>       "Purged with user action",
> +    "Too expensive to revalidate",
>       "Flow state inconsistent after updates",
>       "Flow translation error",
>   ]

Reviewed-by: Adrian Moreno <amorenoz@redhat.com>
Ilya Maximets May 3, 2024, 11:22 a.m. UTC | #2
On 5/2/24 12:11, Eelco Chaudron wrote:
> The flow_reval_monitor.py script incorrectly reported the reasons for
> FDR_PURGE and FDR_TOO_EXPENSIVE, as their descriptions were swapped.
> This patch rectifies the order.
> 
> Fixes: 86b9e653ef22 ("revalidator: Add a USDT probe during flow deletion with purge reason.")
> Signed-off-by: Eelco Chaudron <echaudro@redhat.com>
> ---
>  utilities/usdt-scripts/flow_reval_monitor.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/utilities/usdt-scripts/flow_reval_monitor.py b/utilities/usdt-scripts/flow_reval_monitor.py
> index 534ba8fa2..117f5bc27 100755
> --- a/utilities/usdt-scripts/flow_reval_monitor.py
> +++ b/utilities/usdt-scripts/flow_reval_monitor.py
> @@ -262,8 +262,8 @@ FdrReasonStrings = [
>      "Kill all flows condition detected",
>      "Mask too wide - need narrower match",
>      "No matching ofproto rules",
> -    "Too expensive to revalidate",
>      "Purged with user action",
> +    "Too expensive to revalidate",
>      "Flow state inconsistent after updates",
>      "Flow translation error",
>  ]

Hi, Eelco.  Thanks for the fix!

Did you consider changing this array to a dictionary?  This may help
avoiding such issues in the future.

A few other general notes:

We may consider adding a comment to the C definition of the enum that
python version should be kept in sync.

Comments in the .c file and the descriptions here are fairly different
as well.  That may be a little confusing.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/utilities/usdt-scripts/flow_reval_monitor.py b/utilities/usdt-scripts/flow_reval_monitor.py
index 534ba8fa2..117f5bc27 100755
--- a/utilities/usdt-scripts/flow_reval_monitor.py
+++ b/utilities/usdt-scripts/flow_reval_monitor.py
@@ -262,8 +262,8 @@  FdrReasonStrings = [
     "Kill all flows condition detected",
     "Mask too wide - need narrower match",
     "No matching ofproto rules",
-    "Too expensive to revalidate",
     "Purged with user action",
+    "Too expensive to revalidate",
     "Flow state inconsistent after updates",
     "Flow translation error",
 ]