From: David Miller <davem@davemloft.net>
To: geert@linux-m68k.org
Cc: hkallweit1@gmail.com, f.fainelli@gmail.com, andrew@lunn.ch,
kuba@kernel.org, gaku.inami.xh@renesas.com,
yoshihiro.shimoda.uh@renesas.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Revert "net: linkwatch: add check for netdevice being present to linkwatch_do_dev"
Date: Fri, 11 Sep 2020 17:44:00 -0700 (PDT) [thread overview]
Message-ID: <20200911.174400.306709791543819081.davem@davemloft.net> (raw)
In-Reply-To: <CAMuHMdUd4VtpOGr26KAmF22N32obNqQzq3tbcPxLJ7mxUtSyrg@mail.gmail.com>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Fri, 11 Sep 2020 08:32:55 +0200
> Hi David,
>
> On Thu, Sep 10, 2020 at 9:20 PM David Miller <davem@davemloft.net> wrote:
>> From: Geert Uytterhoeven <geert+renesas@glider.be>
>> Date: Tue, 1 Sep 2020 17:02:37 +0200
>>
>> > This reverts commit 124eee3f6955f7aa19b9e6ff5c9b6d37cb3d1e2c.
>> >
>> > Inami-san reported that this commit breaks bridge support in a Xen
>> > environment, and that reverting it fixes this.
>> >
>> > During system resume, bridge ports are no longer enabled, as that relies
>> > on the receipt of the NETDEV_CHANGE notification. This notification is
>> > not sent, as netdev_state_change() is no longer called.
>> >
>> > Note that the condition this commit intended to fix never existed
>> > upstream, as the patch triggering it and referenced in the commit was
>> > never applied upstream. Hence I can confirm s2ram on r8a73a4/ape6evm
>> > and sh73a0/kzm9g works fine before/after this revert.
>> >
>> > Reported-by Gaku Inami <gaku.inami.xh@renesas.com>
>> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>>
>> Maybe you cannot reproduce it, but the problem is there and it still
>> looks very real to me.
>>
>> netdev_state_change() does two things:
>>
>> 1) Emit the NETDEV_CHANGE notification
>>
>> 2) Emit an rtmsg_ifinfo() netlink message, which in turn tries to access
>> the device statistics via ->ndo_get_stats*().
>>
>> It is absolutely wrong to do #2 when netif_device_present() is false.
>>
>> So I cannot apply this patch as-is, sorry.
>
> Thanks a lot for looking into this!
>
> But doing #1 is still safe? That is the part that calls into the bridge
> code. So would moving the netif_device_present() check from
> linkwatch_do_dev() to netdev_state_change(), to prevent doing #2, be
> acceptable?
I have a better question. Why is a software device like the bridge,
that wants to effectively exist and still receive netdev event
notifications, marking itself as not present?
That's seems like the real bug here.
next prev parent reply other threads:[~2020-09-12 0:44 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-01 15:02 Geert Uytterhoeven
2020-09-03 21:58 ` David Miller
2020-09-10 19:20 ` David Miller
2020-09-11 6:32 ` Geert Uytterhoeven
2020-09-12 0:44 ` David Miller [this message]
2020-09-12 12:33 ` Geert Uytterhoeven
2020-09-13 1:34 ` David Miller
2020-09-14 7:40 ` Geert Uytterhoeven
2020-09-18 12:35 ` Nikolay Aleksandrov
2020-09-18 21:47 ` David Miller
2020-09-18 17:58 ` Saeed Mahameed
2020-09-18 22:10 ` David Miller
2020-09-23 11:49 ` Heiner Kallweit
2020-09-23 18:35 ` Saeed Mahameed
2020-09-23 19:58 ` Heiner Kallweit
2020-09-23 20:15 ` David Miller
2020-09-23 20:44 ` Heiner Kallweit
2020-09-23 22:42 ` Saeed Mahameed
2020-09-24 0:21 ` David Miller
2020-09-24 0:23 ` David Miller
2020-09-24 5:49 ` Saeed Mahameed
2020-09-24 16:03 ` Jakub Kicinski
2020-09-24 18:16 ` Saeed Mahameed
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=20200911.174400.306709791543819081.davem@davemloft.net \
--to=davem@davemloft.net \
--cc=andrew@lunn.ch \
--cc=f.fainelli@gmail.com \
--cc=gaku.inami.xh@renesas.com \
--cc=geert@linux-m68k.org \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=yoshihiro.shimoda.uh@renesas.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®