On 4/10/26 3:25 PM, Mikhail Dmitrichenko wrote: > 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
New thread is actually the custom for ovs dev mailing list so you aligned with it, even if it was not deliberate :-) > 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. > No problem at all. Yes, gmail may cause that. Thanks for the new version. > 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
