On 4/3/26 1:07 PM, Mikhail Dmitrichenko wrote:
> From 81def5ff65881ce6cb4ccdd23a34da8a2870568f Mon Sep 17 00:00:00 2001
> From: Mikhail Dmitrichenko <[email protected]>
> Date: Fri, 3 Apr 2026 14:19:13 +0300
> Subject: [PATCH] netdev-dpdk: Fix possible memory leak when configuring
>  rx-steering
> 
> VLOG_WARN_BUF() allocates a new string with xasprintf() every time it is
> called and overwrites *errp without freeing the previous value. This can
> lead to a memory leak if multiple warnings are emitted or if a later
> hard error in netdev_dpdk_set_config() also writes
> to errp.
> 
> The three cases in dpdk_set_rx_steer_config() are not fatal errors:
> - unsupported rx-steering value
> - rss+lacp on non-ethernet port
> - rss+lacp together with hw-offload
> 
> In all cases program simply log a warning and fall back to default RSS
> steering. Configuration continues normally (err remains 0 and execution
> flow do not goto out).
> 
> Therefore change them to plain VLOG_WARN(). As a result the 'errp'
> parameter becomes unused and is removed from the function signature and
> the call site in netdev_dpdk_set_config().
> 
> This makes the code cleaner and consistent with the rest of netdev-dpdk.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 

Hi Mikhail,

Thank you for the fix. Indeed, we don't need to set errp for these cases
and it could cause a potential leak in the event of multiple config issues.

It looks like there is some wrapping in the email on the @@'s which
confused patchwork [0] and I wasn't able to apply the patch as sent.

Would you be able to send a v2 without wrapping ? (Also, if you are
re-sending there is a couple of minor style items below)

If you have problems sending in correct format, let me know and I can
fix it up locally.

thanks,
Kevin.

[0]
https://patchwork.ozlabs.org/project/openvswitch/patch/CAJBeyFHrv+xMV0qivRC=aazrzqgks6dox0kzjrhzaofdpta...@mail.gmail.com/

> Fixes: fc06ea ("netdev-dpdk: Add custom rx-steering configuration.")

We typically use longer hash
fc06ea9a1883 ("netdev-dpdk: Add custom rx-steering configuration.")

> Signed-off-by: Mikhail Dmitrichenko <[email protected]>
> ---
>  lib/netdev-dpdk.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index 54959ff0d..9fbb204ef 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2251,7 +2251,7 @@ dpdk_process_queue_size(struct netdev *netdev,
> const struct smap *args,
> 

^^^ e.g. line wrapping here and break is confusing patchwork/git am.

>  static void
>  dpdk_set_rx_steer_config(struct netdev *netdev, struct netdev_dpdk *dev,
> -                         const struct smap *args, char **errp)
> +                         const struct smap *args)
>  {
>      const char *arg = smap_get_def(args, "rx-steering", "rss");
>      uint64_t flags = 0;
> @@ -2259,22 +2259,22 @@ dpdk_set_rx_steer_config(struct netdev
> *netdev, struct netdev_dpdk *dev,
>      if (!strcmp(arg, "rss+lacp")) {
>          flags = DPDK_RX_STEER_LACP;
>      } else if (strcmp(arg, "rss")) {
> -        VLOG_WARN_BUF(errp, "%s: options:rx-steering "
> -                      "unsupported parameter value '%s'",
> -                      netdev_get_name(netdev), arg);
> +        VLOG_WARN("%s: options:rx-steering "
> +                  "unsupported parameter value '%s'",

We don't need to wrap the string anymore

> +                  netdev_get_name(netdev), arg);
>      }
> 
>      if (flags && dev->type != DPDK_DEV_ETH) {
> -        VLOG_WARN_BUF(errp, "%s: options:rx-steering "
> -                      "is only supported on ethernet ports",
> -                      netdev_get_name(netdev));
> +        VLOG_WARN("%s: options:rx-steering "
> +                  "is only supported on ethernet ports",
> +                  netdev_get_name(netdev));
>          flags = 0;
>      }
> 
>      if (flags && dpif_offload_enabled()) {
> -        VLOG_WARN_BUF(errp, "%s: options:rx-steering "
> -                      "is incompatible with hw-offload",
> -                      netdev_get_name(netdev));
> +        VLOG_WARN("%s: options:rx-steering "
> +                  "is incompatible with hw-offload",

We don't need to wrap the string anymore

> +                  netdev_get_name(netdev));
>          flags = 0;
>      }
> 
> @@ -2304,7 +2304,7 @@ netdev_dpdk_set_config(struct netdev *netdev,
> const struct smap *args,
>      ovs_mutex_lock(&dpdk_mutex);
>      ovs_mutex_lock(&dev->mutex);
> 
> -    dpdk_set_rx_steer_config(netdev, dev, args, errp);
> +    dpdk_set_rx_steer_config(netdev, dev, args);
> 
>      dpdk_set_rxq_config(dev, args);
> 

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

Reply via email to