From: Leon Romanovsky <leon@kernel.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Vladimir Oltean <olteanv@gmail.com>,
"David S . Miller" <davem@davemloft.net>,
Andrew Lunn <andrew@lunn.ch>,
bridge@lists.linux-foundation.org,
Eric Dumazet <edumazet@google.com>,
Florian Fainelli <f.fainelli@gmail.com>,
netdev@vger.kernel.org, Nikolay Aleksandrov <razor@blackwall.org>,
Paolo Abeni <pabeni@redhat.com>, Roopa Prabhu <roopa@nvidia.com>
Subject: Re: [PATCH net-next] netlink: provide an ability to set default extack message
Date: Fri, 27 Jan 2023 07:26:13 +0200 [thread overview]
Message-ID: <Y9NgdXk3NLtjG3Mj@unreal> (raw)
In-Reply-To: <20230126143723.7593ce0b@kernel.org>
On Thu, Jan 26, 2023 at 02:37:23PM -0800, Jakub Kicinski wrote:
> On Fri, 27 Jan 2023 00:32:13 +0200 Vladimir Oltean wrote:
> > On Thu, Jan 26, 2023 at 09:15:03PM +0200, Leon Romanovsky wrote:
> > > From: Leon Romanovsky <leonro@nvidia.com>
> > >
> > > In netdev common pattern, xxtack pointer is forwarded to the drivers
> > ~~~~~~
> > extack
> >
> > > to be filled with error message. However, the caller can easily
> > > overwrite the filled message.
> > >
> > > Instead of adding multiple "if (!extack->_msg)" checks before any
> > > NL_SET_ERR_MSG() call, which appears after call to the driver, let's
> > > add this check to common code.
> > >
> > > [1] https://lore.kernel.org/all/Y9Irgrgf3uxOjwUm@unreal
> > > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> > > ---
> >
> > I would somewhat prefer not doing this, and instead introducing a new
> > NL_SET_ERR_MSG_WEAK() of sorts.
>
> That'd be my preference too, FWIW. It's only the offload cases which
> need this sort of fallback.
Of course not, almost any error unwind path which sets extack will need it.
See devlink as an example, but I'm confident that same issue exists in other
places too.
You are suggesting API which is very easy to do wrong.
So I prefer to stay with my proposal if it is possible.
Thanks
next prev parent reply other threads:[~2023-01-27 5:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-26 19:15 [PATCH net-next] netlink: provide an ability to set default extack message Leon Romanovsky
2023-01-26 22:32 ` Vladimir Oltean
2023-01-26 22:37 ` Jakub Kicinski
2023-01-26 22:44 ` Vladimir Oltean
2023-01-26 23:39 ` Jakub Kicinski
2023-01-27 5:26 ` Leon Romanovsky [this message]
2023-01-27 7:26 ` Jakub Kicinski
2023-01-29 11:43 ` Leon Romanovsky
2023-01-27 5:22 ` Leon Romanovsky
2023-01-27 9:22 ` Vladimir Oltean
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y9NgdXk3NLtjG3Mj@unreal \
--to=leon@kernel.org \
--cc=andrew@lunn.ch \
--cc=bridge@lists.linux-foundation.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=razor@blackwall.org \
--cc=roopa@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).