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

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

On Thu, Nov 12, 2026 at 11:51:52AM +0800, Wenbo Sun wrote:
> Hi Hangbin,
> 
> On Wed, Nov 11, 2026 at 09:02:17PM +0800, Hangbin Liu wrote:
> > If ipvlan_set_port_mode() failed, the unregister_netdevice(dev) will be
> > called. Does port->flags still need to restore?
> 
> Thank you very much for taking the time to review this patch and for
> pointing out the question about the commit description.
> 
> You are completely right that when creating the first ipvlan device on a
> lower device, unregister_netdevice(dev) drops the last reference to the
> port in ipvlan_uninit() -> ipvlan_port_put(), destroying the port so no
> flag restoration is needed.
> 
> The issue happens when one or more ipvlan interfaces already exist on the
> same lower device:
> 
> 1. Because struct ipvl_port is shared across all ipvlan interfaces on a
>    lower device, ipvlan_init() takes an additional reference via
>    refcount_inc(&port->count) when creating a new ipvlan interface.
> 2. If ipvlan_link_new() writes port->flags before calling
>    ipvlan_set_port_mode(), and ipvlan_set_port_mode() fails (for
>    instance, requesting L3S mode when CONFIG_IPVLAN_L3S is disabled),
>    unregister_netdevice(dev) only decrements port->count back down to at
>    least 1. Since ipvlan_port_destroy() is not called, the shared port
>    remains active for the existing ipvlan interfaces while still carrying
>    the newly written port->flags.
> 
> Rather than saving and restoring port->flags on the error path, moving
> the port->flags assignment after ipvlan_set_port_mode() succeeds avoids
> modifying it on failure altogether, as there are no remaining failure
> paths afterward in ipvlan_link_new() (matching ipvlan_nl_changelink()).

Thanks for the explanation. The change makes sense to me now.

Regards
Hangbin