public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>,
	netdev@vger.kernel.org, Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	"Rafael J. Wysocki" <rafael@kernel.org>,
	Kevin Hilman <khilman@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [RFC PATCH net-next 00/10] Use robust notifiers in DSA
Date: Fri, 19 Aug 2022 01:28:50 +0300	[thread overview]
Message-ID: <20220818222850.mskqhmzpvz2ooamv@skbuf> (raw)
In-Reply-To: <Yv6z5HTyenpJ+pex@lunn.ch>

On Thu, Aug 18, 2022 at 11:49:24PM +0200, Andrew Lunn wrote:
> I would split it into two classes of errors:
> 
> Bus transactions fail. This very likely means the hardware design is
> bad, connectors are loose, etc. There is not much we can do about
> this, bad things are going to happen no what.
> 
> We have consumed all of some sort of resource. Out of memory, the ATU
> is full, too many LAGs, etc. We try to roll back in order to get out
> of this resource problem.
> 
> So i would say -EIO, -ETIMEDOUT, we don't care about too
> much. -ENOMEM, -ENOBUF, -EOPNOTSUPP or whatever, we should try to do a
> robust rollback.
> 
> The original design of switchdev was two phase:
> 
> 1) Allocate whatever resources are needed, can fail
> 2) Put those resources into use, must not fail
> 
> At some point that all got thrown away.

So you think that rollback at the cross-chip notifier layer is a new
problem we need to tackle, because we don't have enough transactional
layering in the code?

In case you don't remember how that utopia dug itself into a hole in practice:
nobody (not even DSA) used the switchdev transactional item queue (which
passed allocated resources between the prepare and the commit phase)
from its introduction in 2015 until it was deleted in 2019, and then
drivers were left unable to reclaim the memory they allocated during
preparation, if the code path never came to the commit stage. There was
nothing left to do except to throw it away.

To discover whether the ATU is full, you either need to reserve space
for static entries beforehand, which is inefficient, or just try to add
what you want and see if you could. Which inevitably leads to encouraging
the strategy of doing the work in the preparation phase and nothing in
the commit phase.

"Too many X" where the resource limitation is known beforehand is about
the only case where a prepare/commit phase could avoid useless rollback.
It's also a case which could also be solved by being upfront about the
limitation to your higher layer, then it would not even try at all.
And do note that "less useless rollback" is different than "code gives
more guarantees that system will remain in a known state".

Sadly reality is much more dynamic than "allocate" -> can fail / "use" ->
must not fail. I think when the model fails to describe reality, you
change the model, not reality.

  reply	other threads:[~2022-08-18 22:29 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-18 15:49 [RFC PATCH net-next 00/10] Use robust notifiers in DSA Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 01/10] notifier: allow robust variants to take a different void *v argument on rollback Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 02/10] net: dsa: introduce and use robust form of dsa_tree_notify() Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 03/10] net: dsa: introduce and use robust form of dsa_broadcast() Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 04/10] net: dsa: introduce and use robust form of dsa_port_notify() Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 05/10] Revert "net: dsa: felix: suppress non-changes to the tagging protocol" Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 06/10] net: dsa: convert switch.c functions to return void if they can Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 07/10] net: dsa: remove "breaking chain" comment from dsa_switch_event Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 08/10] net: dsa: introduce a robust form of MTU cross-chip notifiers Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 09/10] net: dsa: make dsa_tree_notify() and derivatives return void Vladimir Oltean
2022-08-18 15:49 ` [RFC PATCH net-next 10/10] net: dsa: make _del variants of functions " Vladimir Oltean
2022-08-18 21:49 ` [RFC PATCH net-next 00/10] Use robust notifiers in DSA Andrew Lunn
2022-08-18 22:28   ` Vladimir Oltean [this message]
2022-08-18 22:35     ` Andrew Lunn
2022-08-19  0:13       ` 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=20220818222850.mskqhmzpvz2ooamv@skbuf \
    --to=olteanv@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=khilman@kernel.org \
    --cc=kuba@kernel.org \
    --cc=len.brown@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=pavel@ucw.cz \
    --cc=rafael@kernel.org \
    --cc=ulf.hansson@linaro.org \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox