netdev - Re: [PATCH net v2] switchdev: don't abort hardware ipv4 fib offload on failure to program fib entry in hardware

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 29 May 2015 08:12:35 -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.

This breaks 1:1 transparency.  A route now fails to install and the
user is scratching his/her head as to why it failed.  It used to work
when there was no switch offload.  It works with switch offload on
this other device.  So it must be a failure due to switch offload on
this device.  But why this route?  I just installed 20 IPv4 routes and
10 IPv6 routes.  Why did this 11th IPv6 route fail to install?  See,
now user needs to learn about details of that particular device's
limits to understand failure.  When they move their application to
another device, they need to re-learn failure modes.

> 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).

I don't think we want an NLM_F_SKIP_KERNEL option and only have the
route installed on the device.  We want offload to be an acceleration
of the kernel's FIB, not a bypass.

SKIP_OFFLOAD can mess up LPM if the user is not really really careful.

> Any thoughts? Objections?
>
> Thanks!
>
> Jiri
--
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