Leon Romanovsky
2023-Jan-27 05:26 UTC
[Bridge] [PATCH net-next] netlink: provide an ability to set default extack message
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 at 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 at unreal > > > Signed-off-by: Leon Romanovsky <leonro at 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
Jakub Kicinski
2023-Jan-27 07:26 UTC
[Bridge] [PATCH net-next] netlink: provide an ability to set default extack message
On Fri, 27 Jan 2023 07:26:13 +0200 Leon Romanovsky wrote:> > 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.I guess we can come up with scenarios where the new behavior would be useful. But the fact is - your patch changes 4 places...> See devlink as an exampleI don't know what part of devlink you mean at a quick scroll.
Apparently Analagous Threads
- [Bridge] [PATCH net-next] netlink: provide an ability to set default extack message
- [Bridge] [PATCH net-next] netlink: provide an ability to set default extack message
- [Bridge] [PATCH net-next] netlink: provide an ability to set default extack message
- [Bridge] [PATCH net-next] netlink: provide an ability to set default extack message
- [Bridge] [PATCH net-next v2] netlink: provide an ability to set default extack message