All of lore.kernel.org
 help / color / mirror / Atom feed
From: Florian Fainelli <f.fainelli@gmail.com>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>,
	netdev@vger.kernel.org, Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>
Subject: Re: [RFC PATCH net] net: dsa: flush switchdev workqueue before tearing down CPU/DSA ports
Date: Sun, 12 Sep 2021 19:06:25 -0700	[thread overview]
Message-ID: <763e2236-31f9-8947-22d1-cf0b48d8a81a@gmail.com> (raw)
In-Reply-To: <20210912163341.zlhsgq3uvkro3bem@skbuf>



On 9/12/2021 9:33 AM, Vladimir Oltean wrote:
> On Sun, Sep 12, 2021 at 09:24:53AM -0700, Florian Fainelli wrote:
>>
>>
>> On 9/12/2021 9:19 AM, Vladimir Oltean wrote:
>>> On Sun, Sep 12, 2021 at 09:13:36AM -0700, Florian Fainelli wrote:
>>>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>>>>
>>>> Did you post this as a RFC for a particular reason, or just to give
>>>> reviewers some time?
>>>
>>> Both.
>>>
>>> In principle there's nothing wrong with what this patch does, only
>>> perhaps maybe something with what it doesn't do.
>>>
>>> We keep saying that a network interface should be ready to pass traffic
>>> as soon as it's registered, but that "walk dst->ports linearly when
>>> calling dsa_port_setup" might not really live up to that promise.
>>
>> That promise most definitively existed back when Lennert wrote this code and
>> we had an array of ports and the switch drivers brought up their port in
>> their ->setup() method, nowadays, not so sure anymore because of the
>> .port_enable() as much as the list.
>>
>> This is making me wonder whether the occasional messages I am seeing on
>> system suspend from __dev_queue_xmit: Virtual device %s asks to queue
>> packet! might have something to do with that and/or the inappropriate
>> ordering between suspending the switch and the DSA master.
> 
> Sorry, I have never tested the suspend/resume code path, mostly because
> I don't know what would the easiest way be to wake up my systems from
> suspend. If you could give me some pointers there I would be glad to
> look into it.

If your systems support suspend/resume just do:

echo mem > /sys/power/state
or
echo standby > /sys/power/state

if they don't, then maybe a x86 VM with dsa_loop may precipitate the 
problem, but since it uses DSA_TAG_PROTO_NONE, I doubt it, we would need 
to pass traffic on the DSA devices for this warning to show up.
-- 
Florian

  reply	other threads:[~2021-09-13  2:06 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-12 16:00 [RFC PATCH net] net: dsa: flush switchdev workqueue before tearing down CPU/DSA ports Vladimir Oltean
2021-09-12 16:13 ` Florian Fainelli
2021-09-12 16:19   ` Vladimir Oltean
2021-09-12 16:24     ` Florian Fainelli
2021-09-12 16:33       ` Vladimir Oltean
2021-09-13  2:06         ` Florian Fainelli [this message]
2021-09-13  2:12           ` Vladimir Oltean
2021-09-13  2:20             ` Florian Fainelli
2021-09-13 11:31               ` Vladimir Oltean

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=763e2236-31f9-8947-22d1-cf0b48d8a81a@gmail.com \
    --to=f.fainelli@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.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.