All of lore.kernel.org
 help / color / mirror / Atom feed
From: Scott Feldman <sfeldma@gmail.com>
To: David Miller <davem@davemloft.net>
Cc: "Andy Gospodarek" <gospo@cumulusnetworks.com>,
	"Roopa Prabhu" <roopa@cumulusnetworks.com>,
	"Fastabend, John R" <john.r.fastabend@intel.com>,
	"john fastabend" <john.fastabend@gmail.com>,
	"Jiří Pírko" <jiri@resnulli.us>, Netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH net v2] switchdev: don't abort hardware ipv4 fib offload on failure to program fib entry in hardware
Date: Wed, 20 May 2015 22:46:54 -0700	[thread overview]
Message-ID: <CAE4R7bAB2+Oy92w9RpLKL5LUq7gP1mBazBK9y4ZQ4b0k8t2WhQ@mail.gmail.com> (raw)
In-Reply-To: <20150519.162845.955021778058631119.davem@davemloft.net>

On Tue, May 19, 2015 at 1:28 PM, David Miller <davem@davemloft.net> wrote:
> From: Andy Gospodarek <gospo@cumulusnetworks.com>
> Date: Tue, 19 May 2015 15:47:32 -0400
>
>> Are you actually saying that if users complain loudly enough about
>> the current behavior (not the change Roopa has proposed) that you
>> would be open to considering a change the current behavior?
>
> I am saying that we have a contract with users not to break existing
> behavior.  Full stop.

After rehearing David's argument, we should probably explore option d)
which is a refinement on the fib_offload_disable mechanism we have
today.  fib_offload_disable is global for all routes.  Once we hit a
HW install problem, the global flag is set and all routes fallback to
SW.  We did this because we can't allow the failed route to exist in
SW and not in HW because it could mess up LPM searches (HW could hit
on a lesser prefix even when SW has the true LPM, because HW gets
first shot at match).  The refinement on fib_offload_disable is this:
make it per-related-prefix rather than global, and on a HW install
problem, set the flag for the related-prefix and uninstall only those
routes from HW.  Related-prefix (is there a correct term for this?)
are routes to the same dst addr but with different prefix lengths.  I
haven't parsed the fib_trie structure to see how routes are organized,
but I suspect since it's optimized for lookup the related-prefix
tracking is already there and we can build on that.

Option d) requires no application changes.  It requires no additional
understanding or input from the user.  Kernel fallback happens
transparently.  In the case where the HW install failure was due to
out-of-resource in HW, there may be some oscillation as
related-prefixes are removed from HW, freeing up a little space, only
to be filled as new routes come in, and so on.  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 others, when resources are limited.

I think the parts we need are:

1) A new fib_offload_disable bit for related-prefixes.
2) On switchdev fib offload, look up if route is marked as
fib_offload_disabled based on it's related-prefix membership
3) A notification mechanism from driver to indicate a related-prefix
is fib_offload_disabled.

Feasible?

  parent reply	other threads:[~2015-05-21  5:46 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 [this message]
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
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=CAE4R7bAB2+Oy92w9RpLKL5LUq7gP1mBazBK9y4ZQ4b0k8t2WhQ@mail.gmail.com \
    --to=sfeldma@gmail.com \
    --cc=davem@davemloft.net \
    --cc=gospo@cumulusnetworks.com \
    --cc=jiri@resnulli.us \
    --cc=john.fastabend@gmail.com \
    --cc=john.r.fastabend@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=roopa@cumulusnetworks.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.