netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: Scott Feldman <sfeldma@gmail.com>
Cc: Jiri Pirko <jiri@resnulli.us>, David Miller <davem@davemloft.net>,
	Roopa Prabhu <roopa@cumulusnetworks.com>,
	Netdev <netdev@vger.kernel.org>,
	Andy Gospodarek <andy@greyhouse.net>
Subject: Re: [PATCH net v2] switchdev: don't abort hardware ipv4 fib offload on failure to program fib entry in hardware
Date: Thu, 28 May 2015 09:10:29 -0700	[thread overview]
Message-ID: <55673DF5.7060401@gmail.com> (raw)
In-Reply-To: <CAE4R7bB-LazvjPYZmPRE8e-mq_vML+jCBswMkOq-QeV-8ad41A@mail.gmail.com>

On 05/28/2015 08:40 AM, Scott Feldman wrote:
> On Thu, May 28, 2015 at 2:42 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Mon, May 18, 2015 at 10:19:16PM CEST, davem@davemloft.net wrote:
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>> Date: Sun, 17 May 2015 16:42:05 -0700
>>>
>>>> On most systems where you can offload routes to hardware,
>>>> doing routing in software is not an option (the cpu limitations
>>>> make routing impossible in software).
>>>
>>> You absolutely do not get to determine this policy, none of us
>>> do.
>>>
>>> What matters is that by default the damn switch device being there
>>> is %100 transparent to the user.
>>>
>>> And the way to achieve that default is to do software routes as
>>> a fallback.
>>>
>>> I am not going to entertain changes of this nature which fail
>>> route loading by default just because we've exceeded a device's
>>> HW capacity to offload.
>>>
>>> I thought I was _really_ clear about this at netdev 0.1
>>
>> I certainly agree that by default, transparency 1:1 sw:hw mapping is
>> what we need for fib. The current code is a good start!
>>
>> I see couple of issues regarding switchdev_fib_ipv4_abort:
>> 1) If user adds and entry, switchdev_fib_ipv4_add fails, abort is
>>     executed -> and, error returned. I would expect that route entry should
>>     be added in this case. The next attempt of adding the same entry will
>>     be successful.
>>     The current behaviour breaks the transparency you are reffering to.
>> 2) When switchdev_fib_ipv4_abort happens to be executed, the offload is
>>     disabled for good (until reboot). That is certainly not nice, alhough
>>     I understand that is the easiest solution for now.
>>
>> I believe that we all agree that the 1:1 transparency, although it is a
>> default, may not be optimal for real-life usage. HW resources are
>> limited and user does not know them. The danger of hitting _abort and
>> screwing-up the whole system is huge, unacceptable.
>>
>> So here, there are couple of more or less simple things that I suggest to
>> do in order to move a little bit forward:
>> 1) Introduce system-wide option to switch _abort to just plain fail.
>>     When HW does not have capacity, do not flush and fallback to sw, but
>>     rather just fail to add the entry. This would not break anything.
>>     Userspace has to be prepared that entry add could fail.
>> 2) Introduce a way to propagate resources to userspace. Driver knows about
>>     resources used/available/potentially_available. Switchdev infra could
>>     be extended in order to propagate the info to the user.
>> 3) Introduce couple of flags for entry add that would alter the default
>>     behaviour. Something like:
>>          NLM_F_SKIP_KERNEL
>>          NLM_F_SKIP_OFFLOAD
>>     Again, this does not break the current users. On the other hand, this
>>     gives new users a leverage to instruct kernel where the entry should
>>     be added to (or not added to).
>>
>> Any thoughts? Objections?
>
> I don't like these.  Breaks transparency and forces the user in a
> position of having to know hardware failures modes (unique to each
> hardware device).  I presented an option d) which avoids this issues;
> was it not understood?
>

Hi Scott,

I understood your proposal. One caveat I had is in response to this,

"Actually, now that I think of it, the device/driver could decide which
related-prefix to evict from HW, if driver/device wanted to have a
sense of which routes are more important to offload than other"

hardware/driver/device shouldn't have a sense of which routes are more
important than others. I think this is where the NLM_F_* flags come in.
If userspace _wants_ to push policy into the kernel about what is
important it can. If it doesn't we get a sensible heuristic that does
a reasonable job offloading rules transparently. This is how we did
L2 and I think that seems to work fairly well. At least for me but,
always interested to hear other use cases though.

Also I guess I'm not seeing the multitude of hardware failure modes. I
see two either the hardware doesn't support the operation or it is out
of resources. Both can be learned if the hardware exports a model of its
capabilities and resources.

Thanks,
John

-- 
John Fastabend         Intel Corporation

  reply	other threads:[~2015-05-28 16:10 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-17 23:42 [PATCH net v2] switchdev: don't abort hardware ipv4 fib offload on failure to program fib entry in hardware Roopa Prabhu
2015-05-18  5:11 ` Scott Feldman
2015-05-18 20:19 ` David Miller
2015-05-19  0:21   ` John Fastabend
2015-05-19  3:48     ` David Miller
2015-05-19  5:58       ` roopa
2015-05-19 16:34         ` David Miller
2015-05-19 17:01           ` Jiri Pirko
2015-05-19 19:47           ` Andy Gospodarek
2015-05-19 20:28             ` David Miller
2015-05-20 14:37               ` Andy Gospodarek
2015-05-21  5:46               ` Scott Feldman
2015-05-21 15:37                 ` roopa
2015-05-29  7:50                 ` Jiri Pirko
2015-05-29 15:39                   ` Scott Feldman
2015-05-30  9:00                     ` Jiri Pirko
2015-05-31  4:19                       ` John Fastabend
2015-05-31  6:34                         ` Scott Feldman
2015-05-31  7:34                       ` Scott Feldman
2015-05-19  5:57   ` roopa
2015-05-28  9:42   ` Jiri Pirko
2015-05-28 15:35     ` John Fastabend
2015-05-29  7:42       ` Jiri Pirko
2015-05-28 15:40     ` Scott Feldman
2015-05-28 16:10       ` John Fastabend [this message]
2015-05-29  5:37         ` roopa
2015-05-28 22:35       ` Andy Gospodarek
2015-05-29  5:51         ` roopa
2015-05-29  7:50       ` Jiri Pirko
2015-05-29  5:31     ` roopa
2015-05-29 15:12     ` Scott Feldman
2015-05-29 15:37       ` Jiri Pirko
  -- strict thread matches above, loose matches on Subject: below --
2015-05-17  3:46 Roopa Prabhu
2015-05-17 23:41 ` roopa

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=55673DF5.7060401@gmail.com \
    --to=john.fastabend@gmail.com \
    --cc=andy@greyhouse.net \
    --cc=davem@davemloft.net \
    --cc=jiri@resnulli.us \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@cumulusnetworks.com \
    --cc=sfeldma@gmail.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 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).