All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kameron Carr <kameroncarr@linux.microsoft.com>
To: Jakub Kicinski <kuba@kernel.org>, haiyangz@microsoft.com
Cc: davem@davemloft.net, netdev@vger.kernel.org, edumazet@google.com,
	pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org,
	wei.liu@kernel.org, decui@microsoft.com,
	linux-hyperv@vger.kernel.org, hawk@kernel.org, andriin@fb.com,
	stable+noautosel@kernel.org
Subject: Re: [PATCH net-next 1/5] hv_netvsc: fix the program refcount when the VF refuses XDP
Date: Tue, 6 Oct 2026 12:05:15 -0700	[thread overview]
Message-ID: <2188ea84-8fb3-4170-a4b7-01dcab459ceb@linux.microsoft.com> (raw)
In-Reply-To: <20261001014131.310771-2-kuba@kernel.org>

On 9/30/2026 6:41 PM, Jakub Kicinski wrote:
> ndo_bpf() takes over the reference it is passed only on success, the
> caller puts it back itself on failure. netvsc_xdp_set() takes that one
> plus num_chn - 1 more for its channels, and the rollback after the VF
> refuses the program clears the channels again, putting all num_chn of
> them. dev_xdp_install() then puts the one it passed in a second time,
> and the program can be freed while the fd which loaded it still points
> at it.
> 
> The VF refuses a program netvsc has already committed to when the
> program is single-buffer and the VF has header-data split enabled, when
> the VF has a memory provider bound, or when the VF's driver has
> conditions of its own.
> 
> Reported by Sashiko during core rework. Unverified and untested.
> 
> Cc: stable+noautosel@kernel.org # untested fix to unlikely driver error path
> Fixes: 184367dce4f7 ("hv_netvsc: Fix XDP refcnt for synthetic and VF NICs")
> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  drivers/net/hyperv/netvsc_bpf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/net/hyperv/netvsc_bpf.c b/drivers/net/hyperv/netvsc_bpf.c
> index 1dd3755d9e6d..731bb7721fe2 100644
> --- a/drivers/net/hyperv/netvsc_bpf.c
> +++ b/drivers/net/hyperv/netvsc_bpf.c
> @@ -216,6 +216,13 @@ int netvsc_bpf(struct net_device *dev, struct netdev_bpf *bpf)
>  			netdev_err(dev, "vf_setxdp failed:%d\n", ret);
>  			NL_SET_ERR_MSG_MOD(extack, "vf_setxdp failed");
>  
> +			/* Since we haven't completed the installation
> +			 * of bpf->prog the reference core implicitly
> +			 * transfers to us on success isn't ours.
> +			 * Take a reference to balance the accounting.
> +			 */
> +			if (bpf->prog)
> +				bpf_prog_inc(bpf->prog);
>  			netvsc_xdp_set(dev, NULL, extack, nvdev);
>  		}
>  

The refcount part looks right to me.

Not new, but this rollback clears the channels rather than restoring
them. If the VF refuses a replace (P1 -> P2), or refuses the NULL on
detach (memory provider bound, mana failing to re-create its queues),
the core keeps P1 in xdp_state and the VF keeps running P1, but the
synthetic path is left with no program at all.

Could we save a reference to the old program before netvsc_xdp_set() and
restore it here instead of setting NULL? In patch 3/5
netvsc_unregister_vf() decides the take-back based on the channels, so
this matters there too.

-- 
Regards,
Kameron

  reply	other threads:[~2026-10-06 19:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-01  1:41 [PATCH net-next 0/5] hv_netvsc: make XDP propagation act more like bonding Jakub Kicinski
2026-10-01  1:41 ` [PATCH net-next 1/5] hv_netvsc: fix the program refcount when the VF refuses XDP Jakub Kicinski
2026-10-06 19:05   ` Kameron Carr [this message]
2026-10-01  1:41 ` [PATCH net-next 2/5] hv_netvsc: hold the VF's instance lock when installing XDP on it Jakub Kicinski
2026-10-01  1:41 ` [PATCH net-next 3/5] hv_netvsc: treat the VF's XDP program the way bonding treats a slave's Jakub Kicinski
2026-10-04 16:02   ` netdev-bot+sashiko
2026-10-01  1:41 ` [PATCH net-next 4/5] hv_netvsc: move a new VF to netvsc's netns from a work item Jakub Kicinski
2026-10-04 16:02   ` netdev-bot+sashiko
2026-10-01  1:41 ` [PATCH net-next 5/5] hv_netvsc: let the core take XDP off a netvsc device that is going away Jakub Kicinski
2026-10-04 16:02   ` netdev-bot+sashiko
2026-10-02 19:38 ` [EXTERNAL] [PATCH net-next 0/5] hv_netvsc: make XDP propagation act more like bonding Haiyang Zhang
2026-10-06 20:59   ` Kameron Carr
2026-10-07  2:00     ` Jakub Kicinski
2026-10-08 21:22     ` Erni Sri Satya Vennela

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=2188ea84-8fb3-4170-a4b7-01dcab459ceb@linux.microsoft.com \
    --to=kameroncarr@linux.microsoft.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=andriin@fb.com \
    --cc=davem@davemloft.net \
    --cc=decui@microsoft.com \
    --cc=edumazet@google.com \
    --cc=haiyangz@microsoft.com \
    --cc=hawk@kernel.org \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-hyperv@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable+noautosel@kernel.org \
    --cc=wei.liu@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.