From: Yohei Kojima <yk@y-koj.net>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Jakub Kicinski <kuba@kernel.org>,
Andrew Lunn <andrew+netdev@lunn.ch>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Paolo Abeni <pabeni@redhat.com>, Breno Leitao <leitao@debian.org>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH net 1/5] net: netdevsim: fix inconsistent carrier state after link/unlink
Date: Tue, 30 Dec 2025 04:56:16 +0900 [thread overview]
Message-ID: <aVLc4J8SQYLPWdZZ@y-koj.net> (raw)
In-Reply-To: <e8180dc5-fc23-4044-bd67-92fc3eebdaa0@lunn.ch>
On Mon, Dec 29, 2025 at 07:39:29PM +0100, Andrew Lunn wrote:
> On Tue, Dec 30, 2025 at 03:32:34AM +0900, yk@y-koj.net wrote:
> > From: Yohei Kojima <yk@y-koj.net>
> >
> > This patch fixes the edge case behavior on ifup/ifdown and
> > linking/unlinking two netdevsim interfaces:
> >
> > 1. unlink two interfaces netdevsim1 and netdevsim2
> > 2. ifdown netdevsim1
> > 3. ifup netdevsim1
> > 4. link two interfaces netdevsim1 and netdevsim2
> > 5. (Now two interfaces are linked in terms of netdevsim peer, but
> > carrier state of the two interfaces remains DOWN.)
> >
> > This inconsistent behavior is caused by the current implementation,
> > which only cares about the "link, then ifup" order, not "ifup, then
> > link" order. This patch fixes the inconsistency by calling
> > netif_carrier_on() when two netdevsim interfaces are linked.
> >
> > This patch solves buggy behavior on NetworkManager-based systems which
> > causes the netdevsim test to fail with the following error:
> >
> > # timeout set to 600
> > # selftests: drivers/net/netdevsim: peer.sh
> > # 2025/12/25 00:54:03 socat[9115] W address is opened in read-write mode but only supports read-only
> > # 2025/12/25 00:56:17 socat[9115] W connect(7, AF=2 192.168.1.1:1234, 16): Connection timed out
> > # 2025/12/25 00:56:17 socat[9115] E TCP:192.168.1.1:1234: Connection timed out
> > # expected 3 bytes, got 0
> > # 2025/12/25 00:56:17 socat[9109] W exiting on signal 15
> > not ok 13 selftests: drivers/net/netdevsim: peer.sh # exit=1
> >
> > This patch also fixes timeout on TCP Fast Open (TFO) test because the
> > test also depends on netdevsim.
> >
> > Fixes: 1a8fed52f7be ("netdevsim: set the carrier when the device goes up")
>
> Stable rules say:
>
> It must either fix a real bug that bothers people or just add a device ID.
Thank you for the quick reply. I don't intend for this patch to be
backported to the stable tree. My understanding was that bugfix patches
to the net tree should have Fixes: tag for historical tracking.
>
> netdevsim is not a real device. Do its bugs actually bother people?
This patch fixes a real bug that is seen when a developer tries to test
TFO or netdevsim tests on NetworkManager-enabled systems: it causes
false positives in kselftests on such systems.
> Should this patch have a Fixes: tag?
The patch 1a8fed52f7be ("netdevsim: set the carrier when the device goes
up"), which does a similar change, has Fixes: tag. Since this patch fixes
the corner-case behavior which was missed in the previous fix, this
patch should have Fixes: tag for consistency.
>
> Andrew
Thank you,
Yohei Kojima
next prev parent reply other threads:[~2025-12-29 19:56 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-29 18:32 [PATCH net 0/5] net: netdevsim: fix inconsistent carrier state after link/unlink yk
2025-12-29 18:32 ` [PATCH net 1/5] " yk
2025-12-29 18:39 ` Andrew Lunn
2025-12-29 19:56 ` Yohei Kojima [this message]
2025-12-30 11:02 ` Andrew Lunn
2025-12-30 16:20 ` Yohei Kojima
2025-12-30 18:38 ` Andrew Lunn
2025-12-30 18:44 ` Yohei Kojima
2025-12-29 18:32 ` [PATCH net 2/5] selftests: netdevsim: test that linking already-connected devices fails yk
2025-12-29 18:32 ` [PATCH net 3/5] selftests: netdevsim: add carrier state consistency test yk
2025-12-29 18:32 ` [PATCH net 4/5] selftests: net: improve error handling in TFO test yk
2025-12-29 18:32 ` [PATCH net 5/5] selftests: net: report SKIP if TFO test processes timed out yk
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aVLc4J8SQYLPWdZZ@y-koj.net \
--to=yk@y-koj.net \
--cc=andrew+netdev@lunn.ch \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=leitao@debian.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.