netdev - Re: [PATCH net 1/3] ipvlan: fix NETDEV_UP/NETDEV

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: <Z-6IbvorOVx6hpxM@fedora>
Date: Thu, 3 Apr 2025 13:09:02 +0000
From: Hangbin Liu <liuhangbin@...il.com>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: netdev@...r.kernel.org, Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>, Shuah Khan <shuah@...nel.org>,
	Xiao Liang <shaw.leon@...il.com>,
	Kuniyuki Iwashima <kuniyu@...zon.com>,
	Alexander Lobakin <aleksander.lobakin@...el.com>,
	Stanislav Fomichev <sdf@...ichev.me>,
	Venkat Venkatsubra <venkat.x.venkatsubra@...cle.com>,
	Etienne Champetier <champetier.etienne@...il.com>,
	Nikolay Aleksandrov <razor@...ckwall.org>,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH net 1/3] ipvlan: fix NETDEV_UP/NETDEV_DOWN event handling

Hi Sabrina,
On Thu, Apr 03, 2025 at 12:28:54PM +0200, Sabrina Dubroca wrote:
> Hello Hangbin,
> 
> 2025-04-03, 08:58:55 +0000, Hangbin Liu wrote:
> > When setting the lower-layer link up/down, the ipvlan device synchronizes
> > its state via netif_stacked_transfer_operstate(), which only checks the
> > carrier state. However, setting the link down does not necessarily change
> > the carrier state for virtual interfaces like bonding. This causes the
> > ipvlan state to become out of sync with the lower-layer link state.
> > 
> > If the lower link and ipvlan are in the same namespace, this issue is
> > hidden because ip link show checks the link state in IFLA_LINK and has
> > a m_flag to control the state, displaying M-DOWN in the flags. However,
> > if the ipvlan and the lower link are in different namespaces, this
> > information is not available, and the ipvlan link state remains unchanged.
> 
> Is the issue with the actual behavior (sending/receiving packets,
> etc), or just in how it's displayed by iproute?

The upper link in netns up while lower link down will cause the traffic break
in the pod.

> 
> > For example:
> > 
> >   1. Add an ipvlan over bond0.
> >   2. Move the ipvlan to a separate namespace and bring it up.
> >   3. Set bond0 link down.
> >   4. The ipvlan remains up.
> > 
> > This issue affects containers and pods, causing them to display an
> > incorrect link state for ipvlan. Fix this by explicitly changing the
> > IFF_UP flag, similar to how VLAN handles it.
> 
> I'm not sure this change of behavior can be done anymore. And I'm not
> convinced vlan's behavior is better (commit 5e7565930524 ("vlan:
> support "loose binding" to the underlying network device") describes
> why it's not always wanted). IMO it makes sense to have admin state
> separate from link state.

Thanks for the comments, that's also what I am worried. I have send
a question email[1] 2 months ago but not reply yet. So I post this
patch and welcome any feedback.

[1]https://lore.kernel.org/netdev/Z67lt5v6vrltiRyG@fedora/
> 
> If you want a consistent behavior, the admin should also not be
> allowed to set the link UP again while its lower device is not, like
> VLAN does:
> 
> static int vlan_dev_open(struct net_device *dev)
> {
> 	struct vlan_dev_priv *vlan = vlan_dev_priv(dev);
> 	struct net_device *real_dev = vlan->real_dev;
> 	int err;
> 
> 	if (!(real_dev->flags & IFF_UP) &&
> 	    !(vlan->flags & VLAN_FLAG_LOOSE_BINDING))
> 		return -ENETDOWN;
> 
> 
> (but that would almost certainly break someone's scripts)

Yes, so let's wait for others feedback first.

Thanks
Hangbin