[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aXBhKlAxxAkvr4/u@devvm11784.nha0.facebook.com>
Date: Tue, 20 Jan 2026 21:16:26 -0800
From: Bobby Eshleman <bobbyeshleman@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: sgarzare@...hat.com, kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
mst@...hat.com, horms@...nel.org, bobbyeshleman@...a.com,
corbet@....net, xuanzhuo@...ux.alibaba.com, haiyangz@...rosoft.com,
jasowang@...hat.com, linux-hyperv@...r.kernel.org,
pabeni@...hat.com, kys@...rosoft.com, vishnu.dasa@...adcom.com,
longli@...rosoft.com, linux-doc@...r.kernel.org,
linux-kselftest@...r.kernel.org, netdev@...r.kernel.org,
stefanha@...hat.com, berrange@...hat.com, decui@...rosoft.com,
bryan-bt.tan@...adcom.com, eperezma@...hat.com, wei.liu@...nel.org,
davem@...emloft.net, shuah@...nel.org,
bcm-kernel-feedback-list@...adcom.com, edumazet@...gle.com,
virtualization@...ts.linux.dev, sargun@...gun.me
Subject: Re: [net-next,v15,06/12] selftests/vsock: prepare vm management
helpers for namespaces
On Tue, Jan 20, 2026 at 06:54:21PM -0800, Jakub Kicinski wrote:
> In case you need to respin..
>
> This is an AI-generated review of your patch.
> For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
> ---
> selftests/vsock: prepare vm management helpers for namespaces
>
> This commit adds namespace support to VM management helpers and wrapper
> functions, requiring callers to explicitly pass "init_ns" for the init
> namespace. The commit message states "No functional changes for existing
> tests. All have been updated to pass 'init_ns' explicitly."
>
> > diff --git a/tools/testing/selftests/vsock/vmtest.sh b/tools/testing/selftests/vsock/vmtest.sh
> > index c2bdc293b94c..1d03acb62347 100755
> > --- a/tools/testing/selftests/vsock/vmtest.sh
> > +++ b/tools/testing/selftests/vsock/vmtest.sh
> > @@ -135,7 +135,18 @@ del_namespaces() {
> > }
> >
> > vm_ssh() {
> > - ssh -q -o UserKnownHostsFile=/dev/null -p ${SSH_HOST_PORT} localhost "$@"
> > + local ns_exec
> > +
> > + if [[ "${1}" == init_ns ]]; then
> > + ns_exec=""
> > + else
> > + ns_exec="ip netns exec ${1}"
> > + fi
> > +
> > + shift
> > +
> > + ${ns_exec} ssh -q -o UserKnownHostsFile=/dev/null -p "${SSH_HOST_PORT}" localhost "$@"
> > +
> > return $?
> > }
>
> The vm_ssh() function now expects a namespace as its first argument, but
> run_shared_vm_test() still calls vm_ssh without this parameter:
>
> vm_oops_cnt_before=$(vm_ssh -- dmesg | grep -c -i 'Oops')
> vm_warn_cnt_before=$(vm_ssh -- dmesg --level=warn | grep -c -i 'vsock')
> vm_oops_cnt_after=$(vm_ssh -- dmesg | grep -i 'Oops' | wc -l)
> vm_warn_cnt_after=$(vm_ssh -- dmesg --level=warn | grep -c -i 'vsock')
>
> When called as "vm_ssh -- dmesg", the "--" is interpreted as the namespace
> argument. Since "--" does not equal "init_ns", the function sets
> ns_exec="ip netns exec --" which is invalid. The shift then consumes "--"
> and the command becomes "ip netns exec -- ssh ... dmesg" which will fail.
>
> Interestingly, the v9 version of this patch on lore correctly updated these
> calls to pass "init_ns", but this change appears to have been lost between
> v9 and v15. The fix for this regression is present later in the series in
> commit 3ae984505e00 ("selftests/vsock: add vm_dmesg_{warn,oops}_count()
> helpers").
>
> [ ... ]
Got it, thank you.
Best,
Bobby
Powered by blists - more mailing lists