All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Miller <davem@davemloft.net>
To: ecree@solarflare.com
Cc: linux-net-drivers@solarflare.com, netdev@vger.kernel.org
Subject: Re: [PATCH net 2/2] sfc: limit ARFS workitems in flight per channel
Date: Fri, 13 Apr 2018 10:45:24 -0400 (EDT)	[thread overview]
Message-ID: <20180413.104524.2300594456767773807.davem@davemloft.net> (raw)
In-Reply-To: <97aad935-d5fb-713e-fd0f-d84bbd733a8f@solarflare.com>

From: Edward Cree <ecree@solarflare.com>
Date: Fri, 13 Apr 2018 13:36:28 +0100

> It turns out this may all be moot anyway: I figured out why I was seeing
>  ARFS storms and it wasn't the configuration issue I originally blamed.
> My current ndo_rx_flow_steer() implementation, efx_filter_rfs(), returns
>  0 for success, but the caller expects a filter ID to be returned (which
>  we can't give it because we don't know what the filter ID will be until
>  we start mucking around in the software state that's now protected by a
>  sleepable lock).
> As a result, when we call rps_may_expire_flow(), and pass it the _actual_
>  filter ID, this doesn't match the one set_rps_cpu() recorded, so the
>  function returns true and we immediately expire the filter.  Then the
>  next packet to come along isn't steered, so ARFS asks us to insert a
>  steering filter again.
> As a quick fix I've simply tried making the rps_may_expire_flow() calls
>  also pass a filter ID of 0, which prevents the ARFS storms.  This is
>  safe; it may cause us to delay expiring a filter when flow_ids collide,
>  but that can happen anyway with other drivers' implementations (e.g.
>  mlx4 and mlx5 can potentially reuse filter IDs) so I presume it is OK.
> I'll post a v2 with that fix in place of this Patch #2 shortly, then try
>  to follow up with a counter-generated ID (similar to what mlx have).

I understand the constraints you are working under, but do realize
that the real root of the problems is that you are implementing what
is defined clearly as a synchronous operation as asynchronous.

  reply	other threads:[~2018-04-13 14:45 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-12 14:00 [PATCH net 0/2] sfc: couple of ARFS fixes Edward Cree
2018-04-12 14:02 ` [PATCH net 1/2] sfc: insert ARFS filters with replace_equal=true Edward Cree
2018-04-12 14:02 ` [PATCH net 2/2] sfc: limit ARFS workitems in flight per channel Edward Cree
2018-04-12 15:11   ` David Miller
2018-04-12 15:24     ` Edward Cree
2018-04-12 15:33       ` David Miller
2018-04-13 12:36         ` Edward Cree
2018-04-13 14:45           ` David Miller [this message]
2018-04-13 14:52           ` Edward Cree
2018-04-13 15:03             ` David Miller
2018-04-13 15:59               ` Edward Cree
2018-04-13 16:14                 ` David Miller
2018-04-13 16:24                   ` Edward Cree

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=20180413.104524.2300594456767773807.davem@davemloft.net \
    --to=davem@davemloft.net \
    --cc=ecree@solarflare.com \
    --cc=linux-net-drivers@solarflare.com \
    --cc=netdev@vger.kernel.org \
    /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.