netdev - RE: [EXT] mvpp2: incorrect max mtu?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Sun, 23 May 2021 10:54:06 +0000
From:   Stefan Chulski <stefanc@...vell.com>
To:     Russell King <linux@...linux.org.uk>
CC:     Marcin Wojtas <mw@...ihalf.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [EXT] mvpp2: incorrect max mtu?

> > -----Original Message-----
> > From: Russell King <linux@...linux.org.uk>
> > Sent: Tuesday, May 18, 2021 12:42 PM
> > To: Stefan Chulski <stefanc@...vell.com>
> > Cc: Marcin Wojtas <mw@...ihalf.com>; netdev@...r.kernel.org
> > Subject: Re: [EXT] mvpp2: incorrect max mtu?
> >
> > On Tue, May 18, 2021 at 06:09:12AM +0000, Stefan Chulski wrote:
> > > Look like PPv2 tried scatter frame since it was larger than Jumbo
> > > buffer size
> > and it drained buffer pool(Buffers never released).
> > > Received packet should be less than value set in
> > MVPP2_POOL_BUF_SIZE_REG for long pool.
> >
> > So this must mean that setting dev->max_mtu is incorrect.
> >
> > From what I can see, the value programmed into that register would be
> > MVPP2_BM_JUMBO_PKT_SIZE which I believe is 9888. This is currently the
> > same value that dev->max_mtu is set to, but max_mtu is the data
> > payload size in the ethernet frame, which doesn't include the hardware
> > ethernet header.
> >
> > So, should max_mtu be set to 14 bytes less? Or should it be set to
> > 9856? Less
> > 14 bytes? Or what?
> 
> Yes, look like dev->max_mtuis incorrect. It should be 9856,
> MVPP2_POOL_BUF_SIZE_REG,  Pool Buffer Size is in 32 units.
> But before changing it I prefer to test different sizes. Currently, I don't have a
> connected board.
> I will connect board and perform some tests. JUMBO_FRAME_SIZE is the size
> of the buffer, but packet offset should be taken into account.
> 
> > It is really confusing that we have these definitions that state e.g.
> > that JUMBO_FRAME_SIZE is 10432 but the frame size comment says 9856.
> > It's not clear why it's different like that - why the additional 576 octets.
> >
> > All of this could do with some explanation in the driver - would it be
> > possible to add some kind of documentation, or at least make the
> > definitions around packet and frame size more understandable please?
> >
> > Thanks.
> 
> Yes, definition or comments should be improved.
> 
> Regards,
> Stefan.

We probably should split this to 3 issues.
1. BM drain issue. I can post patch witch should fix it. So after lowering the MTU you won't have issue.
2. Regarding MTU max, here we actually talking about max MRU not MTU.
MRU set by mvpp2_xlg_max_rx_size_set/mvpp2_gmac_max_rx_size_set port->pkt_size - MVPP2_MH_SIZE(2 Bytes).
port->pkt_size = MTU +  MVPP2_MH_SIZE(2 Bytes) + VLAN tag (4 Bytes) + ETH_HLEN(14 Bytes) + ETH_FCS_LEN(4 Bytes) and from some reason aligned to cache_line_size().
So if we set MTU = 9888, we actually get MRU 9918.
3. Fix all max MTU comments in code.

Regards,
Stefan.