All of lore.kernel.org
 help / color / mirror / Atom feed
From: Aaron Conole <aconole@redhat.com>
To: Ilya Maximets <i.maximets@ovn.org>
Cc: netdev@vger.kernel.org,  Eelco Chaudron <echaudro@redhat.com>,
	 "David S. Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>, Simon Horman <horms@kernel.org>,
	 Shuah Khan <shuah@kernel.org>,  Yuan Tan <tanyuan98@outlook.com>,
	 Yang Yang <n05ec@lzu.edu.cn>,
	dev@openvswitch.org,  linux-kernel@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [PATCH net v2 0/2] openvswitch: fix self-deadlock on release of tunnel vports
Date: Mon, 04 May 2026 16:24:30 -0400	[thread overview]
Message-ID: <f7t8q9yylc1.fsf@redhat.com> (raw)
In-Reply-To: <d9a9fcf7-07e2-4e26-afde-f1bec6108c40@ovn.org> (Ilya Maximets's message of "Mon, 4 May 2026 13:43:53 +0200")

Ilya Maximets <i.maximets@ovn.org> writes:

> On 5/1/26 1:38 AM, Ilya Maximets wrote:
>> Two patches - the fix for the actual bug and the selftest that reproduces it.
>> 
>> I missed the self-deadlock in the original patch that introduced the issue,
>> because testing required code modification in the ovs-vswitchd to force it to
>> use legacy tunnel ports.  I thought I made the change correctly, but apparently
>> something went wrong and the tests were run with the standard LWT infra instead.
>> The selftest added in this patch set will at least prevent this kind of mistakes
>> in the future.
>> 
>> I mentioned, however, that these tunnel vports are legacy and not actually used
>> by ovs-vswitchd.  RTM_NEWLINK + COLLECT_METADATA is used in conjunction with the
>> standard OVS_VPORT_TYPE_NETDEV instead since 2017.  The code to use the legacy
>> tunnels still exists in ovs-vswitchd however, but only as a fallback for older
>> kernels and we're planning to remove it in the next release.  I'll be sending an
>> RFC to remove support for these legacy tunnel types from the kernel, as they
>> serve no real purpose today and only increase the uAPI surface for CVEs, but
>> we need to fix the known bugs for stable versions.
>> 
>> 
>> Version 2:
>>   - Added Ack from Eelco to the first patch (not to the second as it
>>     changed a little).
>>   - Removed now unused import socket in the dpctl.py [pylint/ruff].
>> 
>>   - Regarding comments from both Sashiko instances on the selftest patch:
>> 
>>     * The background process is not waited for / not killed.
>>       If it hangs it will not be killable anyway, so it's not a problem.
>
> Both sashiko instances still flag this.  Looks like the cover letter is not
> included in the prompt.
>
> If someone thinks I should add the suggested kill on exit, I can, but it will
> not be effective in case the process hangs.

One option is to put a comment in the test itself documenting this kind
of behavior.  At least, then the model might not flag it.  I don't feel
strongly about that, however.

> Best regards, Ilya Maximets.


  reply	other threads:[~2026-05-04 20:24 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-30 23:38 [PATCH net v2 0/2] openvswitch: fix self-deadlock on release of tunnel vports Ilya Maximets
2026-04-30 23:38 ` [PATCH net v2 1/2] openvswitch: vport: fix self-deadlock on release of tunnel ports Ilya Maximets
2026-05-04 15:57   ` Aaron Conole
2026-04-30 23:38 ` [PATCH net v2 2/2] selftests: openvswitch: add tests for tunnel vport refcounting Ilya Maximets
2026-05-01  8:56   ` Eelco Chaudron
2026-05-04 15:57   ` Aaron Conole
2026-05-05 13:25   ` Paolo Abeni
2026-05-05 13:28     ` Ilya Maximets
2026-05-04 11:43 ` [PATCH net v2 0/2] openvswitch: fix self-deadlock on release of tunnel vports Ilya Maximets
2026-05-04 20:24   ` Aaron Conole [this message]
2026-05-05 13:30 ` patchwork-bot+netdevbpf

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=f7t8q9yylc1.fsf@redhat.com \
    --to=aconole@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dev@openvswitch.org \
    --cc=echaudro@redhat.com \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=i.maximets@ovn.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=n05ec@lzu.edu.cn \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    --cc=tanyuan98@outlook.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.