Message-ID: <CAE4R7bB-LazvjPYZmPRE8e-mq_vML+jCBswMkOq-QeV-8ad41A@mail.gmail.com>
Date:	Thu, 28 May 2015 08:40:11 -0700
From:	Scott Feldman <sfeldma@...il.com>
To:	Jiri Pirko <jiri@...nulli.us>
Cc:	David Miller <davem@...emloft.net>,
	Roopa Prabhu <roopa@...ulusnetworks.com>,
	john fastabend <john.fastabend@...il.com>,
	Netdev <netdev@...r.kernel.org>,
	Andy Gospodarek <andy@...yhouse.net>
Subject: Re: [PATCH net v2] switchdev: don't abort hardware ipv4 fib offload
 on failure to program fib entry in hardware

On Thu, May 28, 2015 at 2:42 AM, Jiri Pirko <jiri@...nulli.us> wrote:
> Mon, May 18, 2015 at 10:19:16PM CEST, davem@...emloft.net wrote:
>>From: Roopa Prabhu <roopa@...ulusnetworks.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?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Powered by blists - more mailing lists