netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: stephen@networkplumber.org
Cc: nikolay@cumulusnetworks.com, netdev@vger.kernel.org,
	roopa@cumulusnetworks.com, dsa@cumulusnetworks.com,
	jkbs@redhat.com, edumazet@google.com, pch@ordbogen.com
Subject: Re: [PATCH net-next v3] net: ipv4: add support for ECMP hash policy choice
Date: Tue, 14 Mar 2017 17:24:24 -0700 (PDT)	[thread overview]
Message-ID: <20170314.172424.1077779028779735713.davem@davemloft.net> (raw)
In-Reply-To: <20170314132506.6233b1e7@xeon-e3>

From: Stephen Hemminger <stephen@networkplumber.org>
Date: Tue, 14 Mar 2017 13:25:06 -0700

> On Tue, 14 Mar 2017 11:48:37 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
> 
>> From: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
>> Date: Tue, 14 Mar 2017 17:58:46 +0200
>> 
>> > On 14/03/17 17:55, Stephen Hemminger wrote:  
>> >> On Tue, 14 Mar 2017 17:36:15 +0200
>> >> Nikolay Aleksandrov <nikolay@cumulusnetworks.com> wrote:
>> >>   
>> >>> This patch adds support for ECMP hash policy choice via a new sysctl
>> >>> called fib_multipath_hash_policy and also adds support for L4 hashes.
>> >>> The current values for fib_multipath_hash_policy are:
>> >>>  0 - layer 3 (default)
>> >>>  1 - layer 4
>> >>> If there's an skb hash already set and it matches the chosen policy then it
>> >>> will be used instead of being calculated (currently only for L4).
>> >>> In L3 mode we always calculate the hash due to the ICMP error special
>> >>> case, the flow dissector's field consistentification should handle the
>> >>> address order thus we can remove the address reversals.
>> >>>
>> >>> Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>  
>> >> 
>> >> It is good to see ECMP come back from the grave.
>> >> Linux used to support it long ago but was abandoned after it was unstable
>> >> and removed from iproute2 in 2012.
>> >> 
>> >> The old API was through route attributes which makes more sense than
>> >> doing it with sysctl. It makes more sense to use netlink instead.
>> >> Therefore please go back and do something like the old API rather than doing it through
>> >> sysctl.
>> >>   
>> > 
>> > That's what my initial version did, but this was discussed during NetConf in Seville
>> > and it was decided that it's best to make a global sysctl, thus the change.  
>> 
>> Correct, we discussed this, and we all agreed to only have a sysctl for now.
> 
> Why? If you are going to have private discussions please post the rationale
> in public.

The idea is that we couldn't come up with an immediate use case, and if one
came up we could easily add the per-route or per-fib-table attribute.

Most people want the entire system to behave a certain way wrt. ECMP, rather
than have fine granularity.  For example, the case being discussed here is
to simply have software's behavior match that of hardware offloads.

We shouldn't add things until there is a real demonstrated and requested need.

  parent reply	other threads:[~2017-03-15  0:24 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-06 14:59 [PATCH net-next] net: ipv4: add support for ECMP hash policy choice Nikolay Aleksandrov
2017-03-06 16:24 ` David Ahern
2017-03-06 16:52   ` Nikolay Aleksandrov
2017-03-07  6:16 ` Roopa Prabhu
2017-03-07 11:01 ` [PATCH net-next v2] " Nikolay Aleksandrov
2017-03-08 12:05   ` Jakub Sitnicki
2017-03-08 12:43     ` Nikolay Aleksandrov
2017-03-08 16:00       ` Jakub Sitnicki
2017-03-13  2:23         ` David Miller
2017-03-14 15:36   ` [PATCH net-next v3] " Nikolay Aleksandrov
2017-03-14 15:55     ` Stephen Hemminger
2017-03-14 15:58       ` Nikolay Aleksandrov
2017-03-14 18:48         ` David Miller
2017-03-14 20:25           ` Stephen Hemminger
2017-03-14 21:10             ` Roopa Prabhu
2017-03-14 21:42               ` Stephen Hemminger
2017-03-14 22:38                 ` Roopa Prabhu
2017-03-14 23:27                   ` Stephen Hemminger
2017-03-14 23:45                     ` David Ahern
2017-03-15  9:17                       ` Nicolas Dichtel
2017-03-15 10:46                         ` Nikolay Aleksandrov
2017-03-15 11:18                           ` Nicolas Dichtel
2017-03-15 11:27                             ` Nikolay Aleksandrov
2017-03-15 15:01                         ` David Ahern
2017-03-15 15:20                           ` Stephen Hemminger
2017-03-15  0:24             ` David Miller [this message]
2017-03-15  2:30               ` Tom Herbert
2017-03-17  3:36                 ` David Miller
2017-03-14 18:55     ` Nikolay Aleksandrov
2017-03-15 11:32     ` Jakub Sitnicki
2017-03-15 12:10       ` Nikolay Aleksandrov
2017-03-16 13:28   ` [PATCH net-next v4] " Nikolay Aleksandrov
2017-03-16 16:41     ` Stephen Hemminger
2017-03-16 16:49       ` Nikolay Aleksandrov
2017-03-17 10:06         ` Nikolay Aleksandrov
2017-03-21 22:28     ` David Miller

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=20170314.172424.1077779028779735713.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=dsa@cumulusnetworks.com \
    --cc=edumazet@google.com \
    --cc=jkbs@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@cumulusnetworks.com \
    --cc=pch@ordbogen.com \
    --cc=roopa@cumulusnetworks.com \
    --cc=stephen@networkplumber.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).