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

[Leon Romanovsky <leon@kernel.org>]

On Wed, Nov 29, 2023 at 03:51:14PM +0100, Jiri Pirko wrote:
> Tue, Nov 28, 2023 at 05:08:49PM CET, leon@kernel.org wrote:
> >On Tue, Nov 28, 2023 at 01:02:46PM +0100, Jiri Pirko wrote:
> >> Wed, Nov 22, 2023 at 12:28:32PM CET, leon@kernel.org wrote:
> >> >On Wed, Nov 22, 2023 at 10:50:37AM +0100, Jiri Pirko wrote:
> >> >> Wed, Nov 22, 2023 at 10:35:46AM CET, leon@kernel.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@kernel.org wrote:
> >> >> >> >From: Jianbo Liu <jianbol@nvidia.com>
> >> 
> >> [...]
> >> 
> >> 
> >> >while we are in eswitch. It is skipped in lines 1556-1558:
> >> >
> >> >  1548 static void
> >> >  1549 mlx5e_vport_rep_unload(struct mlx5_eswitch_rep *rep)
> >> >  1550 {
> >> >  1551         struct mlx5e_rep_priv *rpriv = mlx5e_rep_to_rep_priv(rep);
> >> >  1552         struct net_device *netdev = rpriv->netdev;
> >> >  1553         struct mlx5e_priv *priv = netdev_priv(netdev);
> >> >  1554         void *ppriv = priv->ppriv;
> >> >  1555
> >> >  1556         if (rep->vport == MLX5_VPORT_UPLINK) {
> >> >  1557                 mlx5e_vport_uplink_rep_unload(rpriv);
> >> >  1558                 goto free_ppriv;
> >> >  1559         }
> >> 
> >> Uplink netdev is created and destroyed by a different code:
> >> mlx5e_probe()
> >> mlx5e_remove()
> >> 
> >> According to my testing. The uplink netdev is properly removed and
> >> re-added during reload-reinit. What am I missing?
> >
> >You are missing internal profile switch from eswitch to legacy,
> >when you perform driver unload.
> >
> >Feel free to contact me or Jianbo offline if you need more mlx5 specific
> >details.
> 
> Got it. But that switch can happend independently of devlink reload
> reinit. 

Right, devlink reload was relatively easy "to close" and users would see
the reason for it.


> Also, I think it cause more issues than just abandoned ipsec rules.

Yes, it is.

> 
> 
> >
> >Thanks
> >
> >> 
> >> 
> >> 
> >> >  1560
> >> >  1561         unregister_netdev(netdev);
> >> >  1562         mlx5e_rep_vnic_reporter_destroy(priv);
> >> >  1563         mlx5e_detach_netdev(priv);
> >> >  1564         priv->profile->cleanup(priv);
> >> >  1565         mlx5e_destroy_netdev(priv);
> >> >  1566 free_ppriv:
> >> >  1567         kvfree(ppriv); /* mlx5e_rep_priv */
> >> >  1568 }
> >> >
> >> 
> >> [...]
> >> 
> >>