netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bill Fink <billfink@mindspring.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: Eric Dumazet <eric.dumazet@gmail.com>,
	Thomas Graf <tgraf@suug.ch>,
	Chris Friesen <chris.friesen@genband.com>,
	Vimal <j.vimal@gmail.com>,
	netdev@vger.kernel.org, shemminger <shemminger@vyatta.com>
Subject: Re: [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit)
Date: Thu, 14 Mar 2013 00:08:13 -0400	[thread overview]
Message-ID: <20130314000813.b47a11de.billfink@mindspring.com> (raw)
In-Reply-To: <20130313083400.2329e982@nehalam.linuxnetplumber.net>

On Wed, 13 Mar 2013, Stephen Hemminger wrote:

> On Wed, 13 Mar 2013 11:29:50 -0400
> Bill Fink <billfink@mindspring.com> wrote:
> 
> > On Wed, 13 Mar 2013, Eric Dumazet wrote:
> > 
> > > On Wed, 2013-03-13 at 02:01 -0400, Bill Fink wrote:
> > > 
> > > > The last time this was discussed appears to be (on 2011-03-28):
> > > > 
> > > > 	http://marc.info/?l=linux-netdev&m=130128741907282&w=2
> > > > 
> > > > where Maciej Żenczykowski argued that creating a new 64-bit
> > > > Netlink attribute for this would be much more complex than for
> > > > the IFLA_STATS64 support.  There was no reply.
> > > > 
> > > > Providing a new multiplier/shift parameter would be a simple
> > > > way to extend support for higher rates, and would not break
> > > > existing user space that doesn't require the higher rates.
> > > > I imagine the user would not explicitly specify the multiplier/
> > > > shift parameter, but would just normally specify the desired
> > > > rate, and a newer tc would figure out what multiplier/shift
> > > > to use if a high enough rate demanded it.  To maintain user
> > > > space compatibility, the kernel should report back the same
> > > > rate and multiplier/shift it was given, and the newer tc would
> > > > convert it back to the user's originally specified rate.  Older
> > > > user space that was fine with the ~34 Gbps rate limitation would
> > > > always have the default multiplier of 1 or shift of 0 bits, and
> > > > would see the exact same unmultiplied/unshifted rate it always
> > > > did.
> > > 
> > > We already said no to such a hack. Maybe its not clear enough ?
> > > 
> > > netlink allows us to a proper way, and Thomas Graf explained how we
> > > expect the thing to be done.
> > > 
> > > Yes, this is not a one liner patch, its a bit more of work, and its how
> > > it will be done when someone does the job.
> > 
> > I've no problem with that since it is a cleaner solution, but
> > one that requires significantly more work.  I was only arguing
> > that the multiplier/shift approach was also a workable solution
> > and should be simpler to implement.  But since there appears to
> > be developer consensus that it's not a desired method, I'm fine
> > with going along with that expert opinion.
> > 
> > 						-Bill
> 
> As others have said the multiplier shift approach is a not a workable
> solution because it is likely to cause too many compatibility surprises.
> Older kernels would ignore the multiplier and therefore not give the users
> the effective rate they wanted.

Hopefully they would get an error saying that the rate was not
supported by the running kernel, from a failure of trying to
set the multiplier/shift.  You can't get new features from an
old kernel.  But anything working today should still work since
if your rate is less than or equal to ~34 Gbps, your multiplier
would be 1 (or shift of 0 bits), and thus the effective rate is
unmodified from what the user specified (and thus no need to even
use the new interface).

Today if you specify a rate greater than ~34 Gbps, you don't get
what you expected, since from what I understand the value just
gets silently truncated so 40 Gbps results in 5.64 Gbps.  See:

	http://marc.info/?l=linux-netdev&m=130103727012841&w=2

So I don't think anything would get broken that isn't already
broken (or just currently impossible to do).

But I've already said that I'm fine with a more proper solution.
I just hope someone will implement it before another 2 years
passes, as I suspect 100G NICs will become available in that
timeframe.

					-Bill

      parent reply	other threads:[~2013-03-14  4:08 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-10  3:20 [PATCH] Rate should be u64 to avoid integer overflow at high speeds (>= ~35Gbit) Vimalkumar
2013-03-10  4:03 ` Eric Dumazet
2013-03-10  4:53   ` Vimal
2013-03-10  5:05     ` Eric Dumazet
2013-03-10  5:49       ` Bill Fink
2013-03-10  5:54         ` Eric Dumazet
2013-03-12 14:29           ` Chris Friesen
2013-03-12 15:42             ` Thomas Graf
2013-03-12 15:44               ` Eric Dumazet
2013-03-12 15:53               ` Chris Friesen
2013-03-12 15:56                 ` Chris Friesen
2013-03-13  6:01               ` Bill Fink
2013-03-13  6:13                 ` Eric Dumazet
2013-03-13 15:29                   ` Bill Fink
2013-03-13 15:34                     ` Stephen Hemminger
2013-03-13 16:57                       ` Chris Friesen
2013-03-14  4:08                       ` Bill Fink [this message]

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=20130314000813.b47a11de.billfink@mindspring.com \
    --to=billfink@mindspring.com \
    --cc=chris.friesen@genband.com \
    --cc=eric.dumazet@gmail.com \
    --cc=j.vimal@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=shemminger@vyatta.com \
    --cc=stephen@networkplumber.org \
    --cc=tgraf@suug.ch \
    /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).