Hi Kevin! Thank you very much for the review! I sent an updated version of patch but unfortunately it created a new thread https://mail.openvswitch.org/pipermail/ovs-dev/2026-April/431643.html in mail list.. Previously, I sent my patch simply from gmail web client and maybe it caused problems with weird wrapping, sorry for that, I will never do it again. Now I configured sending via git send-email, so I hope this time everything will be better. Sorry again for such a mess.
Best regards, Mikhail Dmitrichenko On Fri, Apr 10, 2026 at 2:04 PM Kevin Traynor <[email protected]> wrote: > > 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
