[ovs-dev] [PATCH] netdev-offload-dpdk: initialize s_tnl dynamic string

[Gaëtan Rivet]

On Fri, Aug 13, 2021, at 08:14, Sriharsha Basavapatna via dev wrote:
> The 's_tnl' member in flow_patterns and flow_actions should be
> to set to DS_EMPTY_INITIALIZER, to be consistent with dynamic string
> initializations.
> 
> Also, there's a potential memory leak of flow_patterns->s_tnl.
> Fix this by destroying s_tnl in free_flow_patterns().
> 
> Fixes: 507d20e77bfe ("netdev-offload-dpdk: Support vports flows offload.")
> Fixes: be56e063d028 ("netdev-offload-dpdk: Support tunnel pop action.")
> Signed-off-by: Sriharsha Basavapatna <sriharsha.basavapatna at broadcom.com>

Hi Harsha,

Thanks for the fix. I have an optional remark below.
However, I'm not sure it's worth sending a v2 for it, so
whether it's integrated as-is or modified is fine.

Acked-by: Gaetan Rivet <grive at u256.net>

> ---
>  lib/netdev-offload-dpdk.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
> index f6706ee0c..aaf2f9a0b 100644
> --- a/lib/netdev-offload-dpdk.c
> +++ b/lib/netdev-offload-dpdk.c
> @@ -791,6 +791,7 @@ free_flow_patterns(struct flow_patterns *patterns)
>      free(patterns->items);
>      patterns->items = NULL;
>      patterns->cnt = 0;
> +    ds_destroy(&patterns->s_tnl);
>  }
>  
>  static void
> @@ -1324,7 +1325,11 @@ netdev_offload_dpdk_mark_rss(struct 
> flow_patterns *patterns,
>                               struct netdev *netdev,
>                               uint32_t flow_mark)
>  {
> -    struct flow_actions actions = { .actions = NULL, .cnt = 0 };
> +    struct flow_actions actions = {
> +        .actions = NULL,
> +        .cnt = 0,
> +        .s_tnl = DS_EMPTY_INITIALIZER

If the initializer is multi-line, I prefer to always use a terminating comma.
It reduces the diff size when the structure is modified.

-- 
Gaetan Rivet