netdev - Re: [net 09/15] net/mlx5e: Forbid devlink reload if IPSec rules are offloaded

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZV3O7dwQMLlNFZp3@nanopsycho>
Date: Wed, 22 Nov 2023 10:50:37 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Leon Romanovsky <leon@...nel.org>
Cc: Saeed Mahameed <saeed@...nel.org>,
	"David S. Miller" <davem@...emloft.net>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Eric Dumazet <edumazet@...gle.com>,
	Saeed Mahameed <saeedm@...dia.com>, netdev@...r.kernel.org,
	Tariq Toukan <tariqt@...dia.com>, Jianbo Liu <jianbol@...dia.com>
Subject: Re: [net 09/15] net/mlx5e: Forbid devlink reload if IPSec rules are
 offloaded

Wed, Nov 22, 2023 at 10:35:46AM CET, leon@...nel.org wrote:
>On Wed, Nov 22, 2023 at 10:13:45AM +0100, Jiri Pirko wrote:
>> Wed, Nov 22, 2023 at 02:47:58AM CET, saeed@...nel.org wrote:
>> >From: Jianbo Liu <jianbol@...dia.com>
>> >
>> >When devlink reload, mlx5 IPSec module can't be safely cleaned up if
>> >there is any IPSec rule offloaded, so forbid it in this condition.
>> >
>> >Fixes: edd8b295f9e2 ("Merge branch 'mlx5-ipsec-packet-offload-support-in-eswitch-mode'")
>> >Signed-off-by: Jianbo Liu <jianbol@...dia.com>
>> >Signed-off-by: Leon Romanovsky <leonro@...dia.com>
>> >Signed-off-by: Saeed Mahameed <saeedm@...dia.com>
>> >---
>> > drivers/net/ethernet/mellanox/mlx5/core/devlink.c |  5 +++++
>> > drivers/net/ethernet/mellanox/mlx5/core/eswitch.h |  2 ++
>> > .../mellanox/mlx5/core/eswitch_offloads.c         | 15 +++++++++++++++
>> > 3 files changed, 22 insertions(+)
>> >
>> >diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>> >index 3e064234f6fe..8925e87a3ed5 100644
>> >--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>> >+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
>> >@@ -157,6 +157,11 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
>> > 		return -EOPNOTSUPP;
>> > 	}
>> > 
>> >+	if (mlx5_eswitch_mode_is_blocked(dev)) {
>> >+		NL_SET_ERR_MSG_MOD(extack, "reload is unsupported if IPSec rules are configured");
>> 
>> That sounds a bit odd to me to be honest. Is pci device unbind forbidden
>> if ipsec rules are present too? This should be gracefully handled
>> instead of forbid.
>
>unbind is handled differently because that operation will call to
>unregister netdevice event which will clean everything.

But in reload, the netdevice is also unregistered. Same flow, isn't it?

>
>devlink reload behaves differently from unbind.

I don't see why. Forget about the driver implementation for now. From
the perspective of the user, what's the difference between these flows:
1) unbind->netdevremoval
2) reload->netdevremoval

Both should be working and do necessary cleanups.


>
>Thanks