* [patch net-next] devlink: don't allow to change net namespace for FW_ACTIVATE reload action
@ 2023-02-10 11:58 Jiri Pirko
2023-02-10 13:15 ` Simon Horman
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Pirko @ 2023-02-10 11:58 UTC (permalink / raw)
To: netdev; +Cc: davem, kuba, pabeni, edumazet, jacob.e.keller, moshe,
simon.horman
From: Jiri Pirko <jiri@nvidia.com>
The change on network namespace only makes sense during re-init reload
action. For FW activation it is not applicable. So check if user passed
an ATTR indicating network namespace change request and forbid it.
Fixes: ccdf07219da6 ("devlink: Add reload action option to devlink reload command")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
Sending to net-next as this is not actually fixing any real bug,
it just adds a forgotten check.
---
net/devlink/dev.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 78d824eda5ec..a6a2bcded723 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -474,6 +474,11 @@ int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
if (info->attrs[DEVLINK_ATTR_NETNS_PID] ||
info->attrs[DEVLINK_ATTR_NETNS_FD] ||
info->attrs[DEVLINK_ATTR_NETNS_ID]) {
+ if (action != DEVLINK_RELOAD_ACTION_DRIVER_REINIT) {
+ NL_SET_ERR_MSG_MOD(info->extack,
+ "Changing namespace is only supported for reinit action");
+ return -EOPNOTSUPP;
+ }
dest_net = devlink_netns_get(skb, info);
if (IS_ERR(dest_net))
return PTR_ERR(dest_net);
--
2.39.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [patch net-next] devlink: don't allow to change net namespace for FW_ACTIVATE reload action
2023-02-10 11:58 [patch net-next] devlink: don't allow to change net namespace for FW_ACTIVATE reload action Jiri Pirko
@ 2023-02-10 13:15 ` Simon Horman
2023-02-10 19:43 ` Jacob Keller
0 siblings, 1 reply; 5+ messages in thread
From: Simon Horman @ 2023-02-10 13:15 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, kuba, pabeni, edumazet, jacob.e.keller, moshe
On Fri, Feb 10, 2023 at 12:58:27PM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> The change on network namespace only makes sense during re-init reload
> action. For FW activation it is not applicable. So check if user passed
> an ATTR indicating network namespace change request and forbid it.
>
> Fixes: ccdf07219da6 ("devlink: Add reload action option to devlink reload command")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> Sending to net-next as this is not actually fixing any real bug,
> it just adds a forgotten check.
> ---
> net/devlink/dev.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/net/devlink/dev.c b/net/devlink/dev.c
> index 78d824eda5ec..a6a2bcded723 100644
> --- a/net/devlink/dev.c
> +++ b/net/devlink/dev.c
> @@ -474,6 +474,11 @@ int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
> if (info->attrs[DEVLINK_ATTR_NETNS_PID] ||
> info->attrs[DEVLINK_ATTR_NETNS_FD] ||
> info->attrs[DEVLINK_ATTR_NETNS_ID]) {
> + if (action != DEVLINK_RELOAD_ACTION_DRIVER_REINIT) {
> + NL_SET_ERR_MSG_MOD(info->extack,
> + "Changing namespace is only supported for reinit action");
> + return -EOPNOTSUPP;
> + }
Is this also applicable in the case where the requested ns (dest_net)
is the same as the current ns, which I think means that the ns
is not changed?
> dest_net = devlink_netns_get(skb, info);
> if (IS_ERR(dest_net))
> return PTR_ERR(dest_net);
> --
> 2.39.0
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch net-next] devlink: don't allow to change net namespace for FW_ACTIVATE reload action
2023-02-10 13:15 ` Simon Horman
@ 2023-02-10 19:43 ` Jacob Keller
2023-02-11 13:23 ` Simon Horman
0 siblings, 1 reply; 5+ messages in thread
From: Jacob Keller @ 2023-02-10 19:43 UTC (permalink / raw)
To: Simon Horman, Jiri Pirko; +Cc: netdev, davem, kuba, pabeni, edumazet, moshe
On 2/10/2023 5:15 AM, Simon Horman wrote:
> On Fri, Feb 10, 2023 at 12:58:27PM +0100, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>>
>> The change on network namespace only makes sense during re-init reload
>> action. For FW activation it is not applicable. So check if user passed
>> an ATTR indicating network namespace change request and forbid it.
>>
>> Fixes: ccdf07219da6 ("devlink: Add reload action option to devlink reload command")
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> ---
>> Sending to net-next as this is not actually fixing any real bug,
>> it just adds a forgotten check.
>> ---
>> net/devlink/dev.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/net/devlink/dev.c b/net/devlink/dev.c
>> index 78d824eda5ec..a6a2bcded723 100644
>> --- a/net/devlink/dev.c
>> +++ b/net/devlink/dev.c
>> @@ -474,6 +474,11 @@ int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
>> if (info->attrs[DEVLINK_ATTR_NETNS_PID] ||
>> info->attrs[DEVLINK_ATTR_NETNS_FD] ||
>> info->attrs[DEVLINK_ATTR_NETNS_ID]) {
>> + if (action != DEVLINK_RELOAD_ACTION_DRIVER_REINIT) {
>> + NL_SET_ERR_MSG_MOD(info->extack,
>> + "Changing namespace is only supported for reinit action");
>> + return -EOPNOTSUPP;
>> + }
>
> Is this also applicable in the case where the requested ns (dest_net)
> is the same as the current ns, which I think means that the ns
> is not changed?
>
In that case wouldn't userspace simply not add the attribute though?
Thanks,
Jake
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch net-next] devlink: don't allow to change net namespace for FW_ACTIVATE reload action
2023-02-10 19:43 ` Jacob Keller
@ 2023-02-11 13:23 ` Simon Horman
2023-02-13 10:45 ` Jiri Pirko
0 siblings, 1 reply; 5+ messages in thread
From: Simon Horman @ 2023-02-11 13:23 UTC (permalink / raw)
To: Jacob Keller; +Cc: Jiri Pirko, netdev, davem, kuba, pabeni, edumazet, moshe
On Fri, Feb 10, 2023 at 11:43:04AM -0800, Jacob Keller wrote:
>
>
> On 2/10/2023 5:15 AM, Simon Horman wrote:
> > On Fri, Feb 10, 2023 at 12:58:27PM +0100, Jiri Pirko wrote:
> >> From: Jiri Pirko <jiri@nvidia.com>
> >>
> >> The change on network namespace only makes sense during re-init reload
> >> action. For FW activation it is not applicable. So check if user passed
> >> an ATTR indicating network namespace change request and forbid it.
> >>
> >> Fixes: ccdf07219da6 ("devlink: Add reload action option to devlink reload command")
> >> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> >> ---
> >> Sending to net-next as this is not actually fixing any real bug,
> >> it just adds a forgotten check.
> >> ---
> >> net/devlink/dev.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/net/devlink/dev.c b/net/devlink/dev.c
> >> index 78d824eda5ec..a6a2bcded723 100644
> >> --- a/net/devlink/dev.c
> >> +++ b/net/devlink/dev.c
> >> @@ -474,6 +474,11 @@ int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
> >> if (info->attrs[DEVLINK_ATTR_NETNS_PID] ||
> >> info->attrs[DEVLINK_ATTR_NETNS_FD] ||
> >> info->attrs[DEVLINK_ATTR_NETNS_ID]) {
> >> + if (action != DEVLINK_RELOAD_ACTION_DRIVER_REINIT) {
> >> + NL_SET_ERR_MSG_MOD(info->extack,
> >> + "Changing namespace is only supported for reinit action");
> >> + return -EOPNOTSUPP;
> >> + }
> >
> > Is this also applicable in the case where the requested ns (dest_net)
> > is the same as the current ns, which I think means that the ns
> > is not changed?
> >
>
> In that case wouldn't userspace simply not add the attribute though?
Yes, that may be the case.
But my question is about the correct behaviour if user space doesn't do that.
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [patch net-next] devlink: don't allow to change net namespace for FW_ACTIVATE reload action
2023-02-11 13:23 ` Simon Horman
@ 2023-02-13 10:45 ` Jiri Pirko
0 siblings, 0 replies; 5+ messages in thread
From: Jiri Pirko @ 2023-02-13 10:45 UTC (permalink / raw)
To: Simon Horman; +Cc: Jacob Keller, netdev, davem, kuba, pabeni, edumazet, moshe
Sat, Feb 11, 2023 at 02:23:28PM CET, simon.horman@corigine.com wrote:
>On Fri, Feb 10, 2023 at 11:43:04AM -0800, Jacob Keller wrote:
>>
>>
>> On 2/10/2023 5:15 AM, Simon Horman wrote:
>> > On Fri, Feb 10, 2023 at 12:58:27PM +0100, Jiri Pirko wrote:
>> >> From: Jiri Pirko <jiri@nvidia.com>
>> >>
>> >> The change on network namespace only makes sense during re-init reload
>> >> action. For FW activation it is not applicable. So check if user passed
>> >> an ATTR indicating network namespace change request and forbid it.
>> >>
>> >> Fixes: ccdf07219da6 ("devlink: Add reload action option to devlink reload command")
>> >> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> >> ---
>> >> Sending to net-next as this is not actually fixing any real bug,
>> >> it just adds a forgotten check.
>> >> ---
>> >> net/devlink/dev.c | 5 +++++
>> >> 1 file changed, 5 insertions(+)
>> >>
>> >> diff --git a/net/devlink/dev.c b/net/devlink/dev.c
>> >> index 78d824eda5ec..a6a2bcded723 100644
>> >> --- a/net/devlink/dev.c
>> >> +++ b/net/devlink/dev.c
>> >> @@ -474,6 +474,11 @@ int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
>> >> if (info->attrs[DEVLINK_ATTR_NETNS_PID] ||
>> >> info->attrs[DEVLINK_ATTR_NETNS_FD] ||
>> >> info->attrs[DEVLINK_ATTR_NETNS_ID]) {
>> >> + if (action != DEVLINK_RELOAD_ACTION_DRIVER_REINIT) {
>> >> + NL_SET_ERR_MSG_MOD(info->extack,
>> >> + "Changing namespace is only supported for reinit action");
>> >> + return -EOPNOTSUPP;
>> >> + }
>> >
>> > Is this also applicable in the case where the requested ns (dest_net)
>> > is the same as the current ns, which I think means that the ns
>> > is not changed?
>> >
>>
>> In that case wouldn't userspace simply not add the attribute though?
>
>Yes, that may be the case.
>But my question is about the correct behaviour if user space doesn't do that.
Okay. Will send v2.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-13 10:47 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-10 11:58 [patch net-next] devlink: don't allow to change net namespace for FW_ACTIVATE reload action Jiri Pirko
2023-02-10 13:15 ` Simon Horman
2023-02-10 19:43 ` Jacob Keller
2023-02-11 13:23 ` Simon Horman
2023-02-13 10:45 ` Jiri Pirko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox