public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Petr Oros <poros@redhat.com>
To: Jacob Keller <jacob.e.keller@intel.com>, Simon Horman <horms@kernel.org>
Cc: netdev@vger.kernel.org, Tony Nguyen <anthony.l.nguyen@intel.com>,
	Przemek Kitszel <przemyslaw.kitszel@intel.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Mitch Williams <mitch.a.williams@intel.com>,
	Aaron Brown <aaron.f.brown@intel.com>,
	Przemyslaw Patynowski <przemyslawx.patynowski@intel.com>,
	Jedrzej Jagielski <jedrzej.jagielski@intel.com>,
	intel-wired-lan@lists.osuosl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH iwl-net v2 0/4] iavf: fix VLAN filter state machine races
Date: Tue, 28 Apr 2026 12:53:50 +0200	[thread overview]
Message-ID: <c90116ad-4b32-4a55-970c-fd8518fccf62@redhat.com> (raw)
In-Reply-To: <b2d56951-4776-4d99-bb4a-69ef7da0a502@intel.com>


On 4/23/26 22:48, Jacob Keller wrote:
> On 4/21/2026 2:02 AM, Simon Horman wrote:
>> On Fri, Apr 17, 2026 at 04:29:41PM +0200, Petr Oros wrote:
>>> The iavf VLAN filter state machine has several design issues that lead
>>> to race conditions between userspace add/del calls and the watchdog
>>> task's virtchnl processing.  Filters can get lost or leak HW resources,
>>> especially during interface down/up cycles and namespace moves.
>> ...
>>
>> Hi Petr,
>>
>> Sashiko has a bit to say about this patch.
>> I'd appreciate it if you could look over that.
>>
>> In particular, the feedback on patches 2 and 3 may warrant
>> some updates to this patchset, while I think 4 is more
>> in the realm of possible future work.
> @Petr,
>
> Could you please review the Sashiko reports and clarify whether a new
> version will be needed?
>
> The original series posted as a net-next was Tested-by, and it would be
> good to get this moving, but I don't want to queue it up for sending
> until certain it won't simply get rejected due to these unresolved comments.
>
> Thanks,
> Jake
>
Hi Jake,

The Sashiko review identified seven concerns across the four patches.

Five of them describe sub millisecond race windows. Rapid del and re add
of a VLAN in IAVF_VLAN_ADDING state. Pending IAVF_VLAN_ADD lost across
down and up before the watchdog ships the request. REMOVING combined with
user re add and user re del state confusion. The reset path resurrecting
filters that are in REMOVE or REMOVING state. Phantom ACTIVE after the PF
rejects an ADD whose user side del raced through.

The remaining two are deterministic pre existing V1 bugs unrelated to
this series. The V1 ADD_VLAN error path has never called 
iavf_vlan_add_reject().
The V2 path got it in 968996c070ef ("iavf: Fix VLAN_V2 addition/rejection")
and V1 was missed. These manifest whenever the PF rejects an ADD on i40e
for example a port VLAN conflict or an untrusted cap reached, and they
belong in a separate fix.

The five race window findings require tight syscall sequencing via ip batch
or sysfs FLR concurrent with del to reach. These patterns do not match how
NetworkManager, systemd-networkd, libvirt or cloud-init configure VLANs.
Those tools add VLANs once on VF setup and do not issue rapid del and
re add or trigger FLR mid operation. The current version keeps the state
machine minimal. Closing these windows requires per filter flag tracking
that adds complexity disproportionate to the user visible benefit on real
workloads.

Two larger problems are worth addressing in follow up work.
The first is num_vlan_filters accounting on V2 under high churn.
Post series, filters in REMOVING state count against
iavf_get_max_vlans_allowed until the PF confirms the deletion.
This can cause a transient EIO on rapid del then add when at the cap.
Pre series this was avoided by immediate kfree. The trade off here is
correctness (no HW resource leak on PF reject) at the cost of a transient
userspace error.

The second is the i40e silent ADD reject. The i40e PF rejects over cap or
untrusted VF VLAN ADDs by returning VIRTCHNL_STATUS_SUCCESS, so iavf cannot
surface the failure to userspace. ip link add ... type vlan reports success
while no filter exists in HW. V2 on ice avoids this via the client side cap.
Closing this gap requires PF and driver ABI coordination.

The series has been tested across documented user workflows on both
ice and i40e PFs in trusted and untrusted modes. The tested scenarios
include interface up and down cycles, namespace migration, VF reset,
VLAN add and remove sequences, parallel VLAN operations across two VFs,
traffic verification via ping under spoofcheck, port VLAN, and multi
VLAN configurations. The workflow scenarios pass on the patched kernel.

The small number of test failures observed were test framework artifacts
(missing IP configuration on probe interfaces, settle time too short for
PF round trip drainage, V1 PF reject classification) and not kernel 
regressions.

Regards,

Petr


      reply	other threads:[~2026-04-28 10:53 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-17 14:29 [PATCH iwl-net v2 0/4] iavf: fix VLAN filter state machine races Petr Oros
2026-04-17 14:29 ` [PATCH iwl-net v2 1/4] iavf: rename IAVF_VLAN_IS_NEW to IAVF_VLAN_ADDING Petr Oros
2026-04-17 14:29 ` [PATCH iwl-net v2 2/4] iavf: stop removing VLAN filters from PF on interface down Petr Oros
2026-04-17 14:29 ` [PATCH iwl-net v2 3/4] iavf: wait for PF confirmation before removing VLAN filters Petr Oros
2026-04-17 14:29 ` [PATCH iwl-net v2 4/4] iavf: add VIRTCHNL_OP_ADD_VLAN to success completion handler Petr Oros
2026-04-17 15:22 ` [PATCH iwl-net v2 0/4] iavf: fix VLAN filter state machine races Przemek Kitszel
2026-04-21  9:02 ` Simon Horman
2026-04-23 20:48   ` Jacob Keller
2026-04-28 10:53     ` Petr Oros [this message]

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=c90116ad-4b32-4a55-970c-fd8518fccf62@redhat.com \
    --to=poros@redhat.com \
    --cc=aaron.f.brown@intel.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=anthony.l.nguyen@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=intel-wired-lan@lists.osuosl.org \
    --cc=jacob.e.keller@intel.com \
    --cc=jedrzej.jagielski@intel.com \
    --cc=jesse.brandeburg@intel.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mitch.a.williams@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=przemyslaw.kitszel@intel.com \
    --cc=przemyslawx.patynowski@intel.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox