All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Stefan Chulski <stefanc@marvell.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	"thomas.petazzoni@bootlin.com" <thomas.petazzoni@bootlin.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	Nadav Haklai <nadavh@marvell.com>,
	Yan Markman <ymarkman@marvell.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kuba@kernel.org" <kuba@kernel.org>,
	"mw@semihalf.com" <mw@semihalf.com>,
	"andrew@lunn.ch" <andrew@lunn.ch>
Subject: Re: [EXT] Re: [PATCH v1] net: mvpp2: divide fifo for dts-active ports only
Date: Mon, 23 Nov 2020 15:51:48 +0000	[thread overview]
Message-ID: <20201123155148.GX1551@shell.armlinux.org.uk> (raw)
In-Reply-To: <CO6PR18MB3873B4205ECAF2383F9539CCB0FC0@CO6PR18MB3873.namprd18.prod.outlook.com>

On Mon, Nov 23, 2020 at 03:44:05PM +0000, Stefan Chulski wrote:
> > > > On Mon, Nov 23, 2020 at 04:52:40PM +0200, stefanc@marvell.com wrote:
> > > > > From: Stefan Chulski <stefanc@marvell.com>
> > > > >
> > > > > Tx/Rx FIFO is a HW resource limited by total size, but shared by
> > > > > all ports of same CP110 and impacting port-performance.
> > > > > Do not divide the FIFO for ports which are not enabled in DTS, so
> > > > > active ports could have more FIFO.
> > > > >
> > > > > The active port mapping should be done in probe before FIFO-init.
> > > >
> > > > It would be nice to know what the effect is from this - is it a
> > > > small or large boost in performance?
> > >
> > > I didn't saw any significant improvement with LINUX bridge or
> > > forwarding, but this reduced PPv2 overruns drops, reduced CRC sent errors
> > with DPDK user space application.
> > > So this improved zero loss throughput. Probably with XDP we would see a
> > similar effect.
> > >
> > > > What is the effect when the ports on a CP110 are configured for 10G,
> > > > 1G, and 2.5G in that order, as is the case on the Macchiatobin board?
> > >
> > > Macchiatobin has two CP's.  CP1 has 3 ports, so the distribution of FIFO would
> > be the same as before.
> > > On CP0 which has a single port, all FIFO would be allocated for 10G port.
> > 
> > Your code allocates for CP1:
> > 
> > 32K to port 0 (the 10G port on Macchiatobin) 8K to port 1 (the 1G dedicated
> > ethernet port on Macchiatobin) 4K to port 2 (the 1G/2.5G SFP port on
> > Macchiatobin)
> > 
> > I'm questioning that allocation for port 1 and 2.
> 
> Yes, but this allocation exists also in current code.
> From HW point of view(MAC and PPv2) maximum supported speed
> in CP110: port 0 - 10G, port 1 - 2.5G, port 2 - 2.5G.
> in CP115: port 0 - 10G, port 1 - 5G, port 2 - 2.5G.
> 
> So this allocation looks correct at least for CP115.
> Problem that we cannot reallocate FIFO during runtime, after specific speed negotiation.

We could do much better. DT has a "max-speed" property for ethernet
controllers. If we have that property, then I think we should use
that to determine the initialisation time FIFO allocation.

As I say, on Macchiatobin, the allocations we end up with are just
crazy when you consider the port speeds that the hardware supports.
Maybe that should be done as a follow-on patch - but I think it
needs to be done.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2020-11-23 15:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-23 14:52 [PATCH v1] net: mvpp2: divide fifo for dts-active ports only stefanc
2020-11-23 15:10 ` Russell King - ARM Linux admin
2020-11-23 15:26   ` [EXT] " Stefan Chulski
2020-11-23 15:33     ` Russell King - ARM Linux admin
2020-11-23 15:44       ` Stefan Chulski
2020-11-23 15:51         ` Russell King - ARM Linux admin [this message]
2020-11-23 16:03           ` Stefan Chulski
2020-11-23 17:30             ` Russell King - ARM Linux admin
2020-11-23 17:48               ` Stefan Chulski
2020-11-23 15:30   ` Russell King - ARM Linux admin

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=20201123155148.GX1551@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mw@semihalf.com \
    --cc=nadavh@marvell.com \
    --cc=netdev@vger.kernel.org \
    --cc=stefanc@marvell.com \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=ymarkman@marvell.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.