Re: [PATCH net-next] net/core: Allow live renaming when an interface is up

[Jakub Kicinski <kuba@kernel.org>]

On Thu,  3 Nov 2022 16:58:47 -0700 Andy Ren wrote:
> @@ -1691,7 +1690,6 @@ enum netdev_priv_flags {
>  	IFF_FAILOVER			= 1<<27,
>  	IFF_FAILOVER_SLAVE		= 1<<28,
>  	IFF_L3MDEV_RX_HANDLER		= 1<<29,
> -	IFF_LIVE_RENAME_OK		= 1<<30,

As Stephen says the hole should be somehow noted.
Comment saying what it was, or just a comment saying there 
is a hole that can be reused.

>  	IFF_TX_SKB_NO_LINEAR		= BIT_ULL(31),
>  	IFF_CHANGE_PROTO_DOWN		= BIT_ULL(32),
>  };
> @@ -1726,7 +1724,6 @@ enum netdev_priv_flags {
>  #define IFF_FAILOVER			IFF_FAILOVER
>  #define IFF_FAILOVER_SLAVE		IFF_FAILOVER_SLAVE
>  #define IFF_L3MDEV_RX_HANDLER		IFF_L3MDEV_RX_HANDLER
> -#define IFF_LIVE_RENAME_OK		IFF_LIVE_RENAME_OK
>  #define IFF_TX_SKB_NO_LINEAR		IFF_TX_SKB_NO_LINEAR
>  
>  /* Specifies the type of the struct net_device::ml_priv pointer */
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 2e4f1c97b59e..a2d650ae15d7 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1163,22 +1163,6 @@ int dev_change_name(struct net_device *dev, const char *newname)
>  
>  	net = dev_net(dev);
>  
> -	/* Some auto-enslaved devices e.g. failover slaves are
> -	 * special, as userspace might rename the device after
> -	 * the interface had been brought up and running since
> -	 * the point kernel initiated auto-enslavement. Allow
> -	 * live name change even when these slave devices are
> -	 * up and running.
> -	 *
> -	 * Typically, users of these auto-enslaving devices
> -	 * don't actually care about slave name change, as
> -	 * they are supposed to operate on master interface
> -	 * directly.
> -	 */
> -	if (dev->flags & IFF_UP &&
> -	    likely(!(dev->priv_flags & IFF_LIVE_RENAME_OK)))
> -		return -EBUSY;
> -

Let's leave a hint for potential triage and add something extra to the
netdev_info() print a few lines down in case the interface is renamed
while UP. Perhaps:

	netdev_info(dev, "renamed from %s%s\n", oldname,
		    dev->flags & IFF_UP ? " (while UP)" : "");

or some such.