netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Robert Beckett <bob.beckett@collabora.com>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	netdev@vger.kernel.org, Vivien Didelot <vivien.didelot@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	bob.beckett@gmail.com
Subject: Re: [PATCH 1/7] net/dsa: configure autoneg for CPU port
Date: Thu, 12 Sep 2019 11:14:39 +0100	[thread overview]
Message-ID: <8d63d4dbd9d075b5c238fd8933673b95b2fa96e9.camel@collabora.com> (raw)
In-Reply-To: <20190911225252.GA5710@lunn.ch>

On Thu, 2019-09-12 at 00:52 +0200, Andrew Lunn wrote:
> > It is not just for broadcast storm protection. The original issue
> > that
> > made me look in to all of this turned out to be rx descritor ring
> > buffer exhaustion due to the CPU not being able to keep up with
> > packet
> > reception.
> 
> Pause frames does not really solve this problem. The switch will at
> some point fill its buffers, and start throwing packets away. Or it
> needs to send pause packets it its peers. And then your whole switch
> throughput goes down. Packets will always get thrown away, so you
> need
> QoS in your network to give the network hints about which frames is
> should throw away first.
> 

Indeed. This is the understanding I was working with.
This patch series enables pause frames, output queue prriority and
strict scheduling to egress the high priority queues first.
This means that when the switch starts dropping frames, it drops from
the lowest priority as the highest ones are delivered at line speed
without issue.

> ..
> 
> > Fundamentally, with a phy to phy CPU connection, the CPU MAC may
> > well
> > wish to enable pause frames for various reasons, so we should
> > strive to
> > handle that I think.
> 
> It actually has nothing to do with PHY to PHY connections. You can
> use
> pause frames with direct MAC to MAC connections. PHY auto-negotiation
> is one way to indicate both ends support it, but there are also other
> ways. e.g.
> 
> ethtool -A|--pause devname [autoneg on|off] [rx on|off] [tx on|off]
> 
> on the SoC you could do
> 
> ethtool --pause eth0 autoneg off rx on tx on
> 
> to force the SoC to send and process pause frames. Ideally i would
> prefer a solution like this, since it is not a change of behaviour
> for
> everybody else.

Good point, well made.
The reason for using autoneg in this series was due to having no netdev
to run ethtool against for the CPU port.
If we go down the route of creating a netdev for the CPU port, then we
could indeed force pause frames at both ends.

However, given that the phy on the marvell switch is capable of autoneg
, is it not reasonable to setup the advertisement and let autoneg take
care of it if using phy to phy connection?

> 
>    Andrew


  reply	other threads:[~2019-09-12 10:14 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-10 15:41 [PATCH 0/7] net: dsa: mv88e6xxx: features to handle network storms Robert Beckett
2019-09-10 15:41 ` [PATCH 1/7] net/dsa: configure autoneg for CPU port Robert Beckett
2019-09-10 16:14   ` Vivien Didelot
2019-09-10 16:56   ` Florian Fainelli
2019-09-10 18:26   ` Andrew Lunn
2019-09-10 18:29     ` Florian Fainelli
2019-09-11  9:16       ` Robert Beckett
2019-09-11  9:54         ` Robert Beckett
2019-09-11 22:52         ` Andrew Lunn
2019-09-12 10:14           ` Robert Beckett [this message]
2019-09-12 10:43             ` Andrew Lunn
2019-09-11 11:43   ` kbuild test robot
2019-09-14  7:16   ` kbuild test robot
2019-09-10 15:41 ` [PATCH 2/7] net: dsa: mv88e6xxx: add ability to set default queue priorities per port Robert Beckett
2019-09-10 16:43   ` Vivien Didelot
2019-09-10 15:41 ` [PATCH 3/7] dt-bindings: " Robert Beckett
2019-09-10 16:42   ` Florian Fainelli
2019-09-10 16:49     ` Vivien Didelot
2019-09-10 20:46       ` Vladimir Oltean
2019-09-10 15:41 ` [PATCH 4/7] net: dsa: mv88e6xxx: add ability to set queue scheduling Robert Beckett
2019-09-10 17:18   ` Vivien Didelot
2019-09-10 15:41 ` [PATCH 5/7] dt-bindings: " Robert Beckett
2019-09-10 15:41 ` [PATCH 6/7] net: dsa: mv88e6xxx: add egress rate limiting Robert Beckett
2019-09-10 17:13   ` Vivien Didelot
2019-09-11 12:26   ` kbuild test robot
2019-09-10 15:41 ` [PATCH 7/7] dt-bindings: " Robert Beckett
2019-09-10 16:49 ` [PATCH 0/7] net: dsa: mv88e6xxx: features to handle network storms Florian Fainelli
2019-09-11  9:43   ` Robert Beckett
2019-09-11 11:21   ` Ido Schimmel
2019-09-11 11:49     ` Robert Beckett
2019-09-11 22:58       ` Andrew Lunn
2019-09-12  9:05         ` Ido Schimmel
2019-09-12  9:03       ` Ido Schimmel
2019-09-12  9:21         ` Andrew Lunn
2019-09-12 16:25         ` Florian Fainelli
2019-09-12 16:46           ` Robert Beckett
2019-09-12 17:41             ` Florian Fainelli
2019-09-13 12:47               ` Robert Beckett
2019-09-10 17:19 ` Vivien Didelot
2019-09-11  9:46   ` Robert Beckett
2019-09-11 15:31     ` Vivien Didelot
2019-09-11 23:01     ` Andrew Lunn

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=8d63d4dbd9d075b5c238fd8933673b95b2fa96e9.camel@collabora.com \
    --to=bob.beckett@collabora.com \
    --cc=andrew@lunn.ch \
    --cc=bob.beckett@gmail.com \
    --cc=davem@davemloft.net \
    --cc=f.fainelli@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=vivien.didelot@gmail.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;
as well as URLs for NNTP newsgroup(s).