From: Jakub Kicinski <jakub.kicinski@netronome.com>
To: David Miller <davem@davemloft.net>
Cc: netdev@vger.kernel.org
Subject: Re: [PATCH v4 net-next 01/15] nfp: correct RX buffer length calculation
Date: Tue, 5 Apr 2016 18:24:56 +0100 [thread overview]
Message-ID: <20160405182456.1dfbb3e7@jkicinski-Precision-T1700> (raw)
In-Reply-To: <20160405.113917.846241936652070162.davem@davemloft.net>
On Tue, 05 Apr 2016 11:39:17 -0400 (EDT), David Miller wrote:
> From: Jakub Kicinski <jakub.kicinski@netronome.com>
> Date: Fri, 1 Apr 2016 22:06:37 +0100
>
> > When calculating the RX buffer length we need to account for
> > up to 2 VLAN tags and up to 8 MPLS labels. Rounding up to 1k
> > is an relic of a distant past and can be removed. While at
> > it also remove trivial print statement.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
>
> I disagree with the MPLS aspect of this change.
>
> VLAN is special, in that when the hardware supports VLAN properly, the
> VLAN header doesn't eat into the MTU and is sort of "transparent".
>
> But MPLS doesn't work that way.
>
> MPLS is in the main frame and takes up MTU space.
Makes sense. RFC3032 counts MPLS label stack as Frame Payload.
> Therefore I see no reason to increase the buffer length by 8 * MPLS
> which is just a rediculous amount of wasted space.
>
> I'm not applying this without at least some more explanations about
> why exactly you need to account for these values in the commit message.
It's just what FW guys asked me for. I'll try to find out what their
reasoning was.
I have a patch queued up which gets rid of unconditionally reserving
64B for firmware prepend, I guess that made me too excited to pay
attention to the fact the accounting for MPLS is indeed questionable...
next prev parent reply other threads:[~2016-04-05 17:25 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-01 21:06 [PATCH v4 net-next 00/15] MTU/buffer reconfig changes Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 01/15] nfp: correct RX buffer length calculation Jakub Kicinski
2016-04-05 15:39 ` David Miller
2016-04-05 17:24 ` Jakub Kicinski [this message]
2016-04-01 21:06 ` [PATCH v4 net-next 02/15] nfp: move link state interrupt request/free calls Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 03/15] nfp: break up nfp_net_{alloc|free}_rings Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 04/15] nfp: make *x_ring_init do all the init Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 05/15] nfp: allocate ring SW structs dynamically Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 06/15] nfp: cleanup tx ring flush and rename to reset Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 07/15] nfp: reorganize initial filling of RX rings Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 08/15] nfp: preallocate RX buffers early in .ndo_open Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 09/15] nfp: move filling ring information to FW config Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 10/15] nfp: slice .ndo_open() and .ndo_stop() up Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 11/15] nfp: sync ring state during FW reconfiguration Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 12/15] nfp: propagate list buffer size in struct rx_ring Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 13/15] nfp: convert .ndo_change_mtu() to prepare/commit paradigm Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 14/15] nfp: pass ring count as function parameter Jakub Kicinski
2016-04-01 21:06 ` [PATCH v4 net-next 15/15] nfp: allow ring size reconfiguration at runtime Jakub Kicinski
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=20160405182456.1dfbb3e7@jkicinski-Precision-T1700 \
--to=jakub.kicinski@netronome.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
/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).