Message-ID: <CAE4R7bCDG614esO1UByPYoX47+zzjr=Ed6LNPbqTKG7Nvnsm3A@mail.gmail.com>
Date:	Sun, 3 May 2015 10:45:16 -0700
From:	Scott Feldman <sfeldma@...il.com>
To:	Roopa Prabhu <roopa@...ulusnetworks.com>
Cc:	"David S. Miller" <davem@...emloft.net>,
	john fastabend <john.fastabend@...il.com>,
	Jiří Pírko <jiri@...nulli.us>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH net-next] switchdev: don't abort hardware ipv4 fib
 offload on failure to program fib entry in hardware

On Sat, May 2, 2015 at 9:24 AM,  <roopa@...ulusnetworks.com> wrote:
> From: Roopa Prabhu <roopa@...ulusnetworks.com>
>
> This basically removes the calls to netdev_switch_fib_ipv4_abort when
> there is an error in programming fib entry in hardware.
>
> On most systems where you can offload routes to hardware,
> doing routing in software is not an option (the cpu's are not powerful
> to route in software).
>
> I understand that this was added to keep the first fib offload support
> simple. This RFC patch is to start a discussion around why the fib
> abort will not work for most hardware and to discuss alternate options
> for default behaviour.

Thanks for revisiting this topic.  Agreed, current arrangement isn't
practical for real offload devices with limited CPU support.

> Available options:
> a) Dont abort hardware offload and return appropriate error to the user on
> fib offload failure by default (this patch)
> b) make the behaviour in a) conditional on a global flag/sysctl (a per fib
> entry flag will not work because by default user/routing-daemons dont care
> if they are hardware offloaded)
> c) for the users/routing-daemons interested in controlling the hardware
> offload behaviour there is always the per fib entry flag RTF_F_EXTERNAL
> (and special error codes -EOFFLOAD could perhaps indicate that the
> hardware offload failed)
>
> Considering the characteristics of the systems that support fib offloads,
> a) above seems to be the right default.

Going with option a) puts a burden on the user: on failure, the user
may or may not understand why the operation failed.  Or worse, the
routing protocol may not make the failure obvious to the user, so the
user is unaware or unsure why routes aren't getting installed.  With
the current solution, routes are still installed even if there was a
failure to offload, so the user doesn't care (yes, whimpy CPU would
make the user care :).

Anyway, I'd prefer to see an option c) -like solution where we force
user to explicitly indicate route offload (by setting
RTNH_F_EXTERNAL).  The default would be no offload.  The apps
(iproute2 + routing protocols) would need to be updated to support
setting this new flag.  This way, the user participates at the app
level in setting the flag and if there is a failure, the user would be
aware and can react.  (switchdev driver can return meaningful err
codes, such as ENOSPC for hw route tables full, for example).

With c), the user must be aware that offloaded routes take priority
over non-offloaded routes.  It's as if there is a new ip rule -1
that's higher priority than the kernel's ip rule 0.  ip rule -1 says
look up in offload device first.

> PS: This patch currently removes use of the netdev_switch_fib_ipv4_abort
> function but not the function itself. This will be fixed if we converge
> on the default behaviour.

I'd prefer a full cleanup, including removal of
ipv4.fib_offload_disabled, rather than leaving orphaned code.
--
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