Re: [PATCH net v2] ipvlan: update port flags only after ipvlan_set_port_mode() succeeds

[Hangbin Liu <hangbin.liu@linux.dev>]

On Thu, Nov 12, 2026 at 02:48:21PM +0800, Wenbo Sun wrote:
> In ipvlan_link_new(), port->flags is updated before calling
> ipvlan_set_port_mode(). If ipvlan_set_port_mode() fails (for example,
> when switching to L3S mode without CONFIG_IPVLAN_L3S), error unwinding
> calls unregister_netdevice(dev) via unlink_netdev.
> 
> While unregister_netdevice(dev) destroys the port when creating the first
> ipvlan device on a lower interface, struct ipvl_port is shared across all
> ipvlan interfaces on the same lower device (tracked via port->count). If
> another ipvlan interface already exists, unregister_netdevice(dev) only
> drops the extra reference and leaves the shared port alive with the
> updated port->flags, unintentionally altering the flags of existing
> ipvlan interfaces.
> 
> Defer updating port->flags until after ipvlan_set_port_mode() succeeds,
> after which ipvlan_link_new() has no remaining failure paths. This avoids
> modifying port->flags on failure without needing to save and restore it,
> matching ipvlan_nl_changelink().
> 
> This issue was identified with the assistance of an LLM-based code
> review tool, confirmed in a User-Mode Linux (UML) environment by creating
> a second ipvlan interface that fails in ipvlan_set_port_mode(), and
> verified in UML that the existing interface's port flags remain
> unchanged upon failure.
> 
> Fixes: a190d04db937 ("ipvlan: introduce 'private' attribute for all existing modes.")
> Signed-off-by: Wenbo Sun <sunwenbo@cmss.chinamobile.com>
> ---
> v2:
>   - Clarify in the commit message that when creating a second or
>     subsequent ipvlan interface on an already-active port,
>     unregister_netdevice() only drops the extra reference rather than
>     destroying the shared port, and explain why deferring the update
>     avoids needing to save and restore port->flags (Hangbin Liu).
>   - Soften and clarify the wording in the commit message.
> v1: https://lore.kernel.org/netdev/20261111064655.233265-1-sunwenbo@cmss.chinamobile.com/
> 
>  drivers/net/ipvlan/ipvlan_main.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index f29864db662a..5732b167c5b3 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -678,12 +678,6 @@ int ipvlan_link_new(struct net_device *dev, struct rtnl_newlink_params *params,
>  	if (err)
>  		goto remove_ida;
>  
> -	/* Flags are per port and latest update overrides. User has
> -	 * to be consistent in setting it just like the mode attribute.
> -	 */
> -	if (data && data[IFLA_IPVLAN_FLAGS])
> -		port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]);
> -
>  	if (data && data[IFLA_IPVLAN_MODE])
>  		mode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
>  
> @@ -691,6 +685,12 @@ int ipvlan_link_new(struct net_device *dev, struct rtnl_newlink_params *params,
>  	if (err)
>  		goto unlink_netdev;
>  
> +	/* Flags are per port and latest update overrides. User has
> +	 * to be consistent in setting it just like the mode attribute.
> +	 */
> +	if (data && data[IFLA_IPVLAN_FLAGS])
> +		port->flags = nla_get_u16(data[IFLA_IPVLAN_FLAGS]);
> +
>  	mutex_lock(&port->pnodes_lock);
>  	list_add_tail_rcu(&ipvlan->pnode, &port->ipvlans);
>  	mutex_unlock(&port->pnodes_lock);
> -- 
> 2.33.0
> 

Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>