All of lore.kernel.org
 help / color / mirror / Atom feed
From: Edward Cree <ecree@solarflare.com>
To: David Miller <davem@davemloft.net>
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 13:36:28 +0100	[thread overview]
Message-ID: <97aad935-d5fb-713e-fd0f-d84bbd733a8f@solarflare.com> (raw)
In-Reply-To: <20180412.113300.475644883265871487.davem@davemloft.net>

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

-Ed

  reply	other threads:[~2018-04-13 12:36 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 [this message]
2018-04-13 14:45           ` David Miller
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=97aad935-d5fb-713e-fd0f-d84bbd733a8f@solarflare.com \
    --to=ecree@solarflare.com \
    --cc=davem@davemloft.net \
    --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.