Linux-HyperV List
 help / color / mirror / Atom feed
* RE: [EXTERNAL] Re: [PATCH net] net: core: Fix the loop in default_device_exit_net()
From: Haiyang Zhang @ 2025-07-22 16:20 UTC (permalink / raw)
  To: Kuniyuki Iwashima, Jakub Kicinski
  Cc: Haiyang Zhang, linux-hyperv@vger.kernel.org,
	netdev@vger.kernel.org, KY Srinivasan, wei.liu@kernel.org,
	edumazet@google.com, pabeni@redhat.com, horms@kernel.org,
	davem@davemloft.net, sdf@fomichev.me, ahmed.zaki@intel.com,
	aleksander.lobakin@intel.com, linux-kernel@vger.kernel.org
In-Reply-To: <CAAVpQUC_sH2UDdf0e5c=iPFU5EcaB7YeN=__2j6w_h6_pe8m_g@mail.gmail.com>



> -----Original Message-----
> From: Kuniyuki Iwashima <kuniyu@google.com>
> Sent: Saturday, July 19, 2025 4:48 PM
> To: Jakub Kicinski <kuba@kernel.org>
> Cc: Haiyang Zhang <haiyangz@linux.microsoft.com>; linux-
> hyperv@vger.kernel.org; netdev@vger.kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> wei.liu@kernel.org; edumazet@google.com; pabeni@redhat.com;
> horms@kernel.org; davem@davemloft.net; sdf@fomichev.me;
> ahmed.zaki@intel.com; aleksander.lobakin@intel.com; linux-
> kernel@vger.kernel.org
> Subject: [EXTERNAL] Re: [PATCH net] net: core: Fix the loop in
> default_device_exit_net()
> 
> [You don't often get email from kuniyu@google.com. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Fri, Jul 18, 2025 at 4:37 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Fri, 18 Jul 2025 13:20:14 -0700 Haiyang Zhang wrote:
> > > The loop in default_device_exit_net() won't be able to properly detect
> the
> > > head then stop, and will hit NULL pointer, when a driver, like
> hv_netvsc,
> > > automatically moves the slave device together with the master device.
> > >
> > > To fix this, add a helper function to return the first migratable
> netdev
> > > correctly, no matter one or two devices were removed from this net's
> list
> > > in the last iteration.
> >
> > FTR I think that what the driver is trying to do is way too hacky, and
> > it should be fixed instead. But I defer to Kuniyuki for the final word,
> > maybe this change is useful for other reasons..
> 
> I agree that it should be fixed on the driver side.  I don't
> think of a good reason for the change.

Kuniyuki and Jakub:

Thanks for the reviews. I'm working on a patch that will fix the driver side.

- Haiyang


^ permalink raw reply

* RE: [EXTERNAL] Re: [PATCH net] net: core: Fix the loop in default_device_exit_net()
From: Haiyang Zhang @ 2025-07-22 16:15 UTC (permalink / raw)
  To: Eric Dumazet, Haiyang Zhang
  Cc: linux-hyperv@vger.kernel.org, netdev@vger.kernel.org,
	KY Srinivasan, wei.liu@kernel.org, kuba@kernel.org,
	pabeni@redhat.com, horms@kernel.org, davem@davemloft.net,
	sdf@fomichev.me, kuniyu@google.com, ahmed.zaki@intel.com,
	aleksander.lobakin@intel.com, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org, #@linux.microsoft.com,
	5.4+@linux.microsoft.com
In-Reply-To: <CANn89iJLnprFvLpRpJ7_br5EiyCF0xqcMM7seUVQNAfroc4Taw@mail.gmail.com>



> -----Original Message-----
> From: Eric Dumazet <edumazet@google.com>
> Sent: Tuesday, July 22, 2025 2:52 AM
> To: Haiyang Zhang <haiyangz@linux.microsoft.com>
> Cc: linux-hyperv@vger.kernel.org; netdev@vger.kernel.org; Haiyang Zhang
> <haiyangz@microsoft.com>; KY Srinivasan <kys@microsoft.com>;
> wei.liu@kernel.org; kuba@kernel.org; pabeni@redhat.com; horms@kernel.org;
> davem@davemloft.net; sdf@fomichev.me; kuniyu@google.com;
> ahmed.zaki@intel.com; aleksander.lobakin@intel.com; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org; #@linux.microsoft.com;
> 5.4+@linux.microsoft.com
> Subject: [EXTERNAL] Re: [PATCH net] net: core: Fix the loop in
> default_device_exit_net()
> 
> On Fri, Jul 18, 2025 at 1:21 PM Haiyang Zhang
> <haiyangz@linux.microsoft.com> wrote:
> >
> > From: Haiyang Zhang <haiyangz@microsoft.com>
> >
> > The loop in default_device_exit_net() won't be able to properly detect
> the
> > head then stop, and will hit NULL pointer, when a driver, like
> hv_netvsc,
> > automatically moves the slave device together with the master device.
> >
> > To fix this, add a helper function to return the first migratable netdev
> > correctly, no matter one or two devices were removed from this net's
> list
> > in the last iteration.
> >
> > Cc: stable@vger.kernel.org # 5.4+
> 
> We (network maintainers) prefer a Fixes: tag, so that we can look at
> the blamed patch, rather than trusting your '5.4' hint.
> 
> Without a Fixes tag, you are forcing each reviewer to do the
> archeology work, and possibly completely miss your point.

Thanks. I will have the Fixes tag in the new patch.

- Haiyang

^ permalink raw reply

* Re: [RFC PATCH net-next v6 00/14] virtio/vsock: support datagrams
From: Stefano Garzarella @ 2025-07-22 14:35 UTC (permalink / raw)
  To: Amery Hung
  Cc: stefanha, mst, jasowang, xuanzhuo, davem, edumazet, kuba, pabeni,
	kys, haiyangz, wei.liu, decui, bryantan, vdasa, pv-drivers,
	dan.carpenter, simon.horman, oxffffaa, kvm, virtualization,
	netdev, linux-kernel, linux-hyperv, bpf, bobby.eshleman,
	jiang.wang, amery.hung, xiyou.wangcong
In-Reply-To: <20240710212555.1617795-1-amery.hung@bytedance.com>

Hi Amery,

On Wed, Jul 10, 2024 at 09:25:41PM +0000, Amery Hung wrote:
>Hey all!
>
>This series introduces support for datagrams to virtio/vsock.

any update on v7 of this series?

Thanks,
Stefano

>
>It is a spin-off (and smaller version) of this series from the summer:
>  https://lore.kernel.org/all/cover.1660362668.git.bobby.eshleman@bytedance.com/
>
>Please note that this is an RFC and should not be merged until
>associated changes are made to the virtio specification, which will
>follow after discussion from this series.
>
>Another aside, the v4 of the series has only been mildly tested with a
>run of tools/testing/vsock/vsock_test. Some code likely needs cleaning
>up, but I'm hoping to get some of the design choices agreed upon before
>spending too much time making it pretty.
>
>This series first supports datagrams in a basic form for virtio, and
>then optimizes the sendpath for all datagram transports.
>
>The result is a very fast datagram communication protocol that
>outperforms even UDP on multi-queue virtio-net w/ vhost on a variety
>of multi-threaded workload samples.
>
>For those that are curious, some summary data comparing UDP and VSOCK
>DGRAM (N=5):
>
>	vCPUS: 16
>	virtio-net queues: 16
>	payload size: 4KB
>	Setup: bare metal + vm (non-nested)
>
>	UDP: 287.59 MB/s
>	VSOCK DGRAM: 509.2 MB/s
>
>Some notes about the implementation...
>
>This datagram implementation forces datagrams to self-throttle according
>to the threshold set by sk_sndbuf. It behaves similar to the credits
>used by streams in its effect on throughput and memory consumption, but
>it is not influenced by the receiving socket as credits are.
>
>The device drops packets silently.
>
>As discussed previously, this series introduces datagrams and defers
>fairness to future work. See discussion in v2 for more context around
>datagrams, fairness, and this implementation.
>
>Signed-off-by: Bobby Eshleman <bobby.eshleman@bytedance.com>
>Signed-off-by: Amery Hung <amery.hung@bytedance.com>
>---
>Changes in v6:
>- allow empty transport in datagram vsock
>- add empty transport checks in various paths
>- transport layer now saves source cid and port to control buffer of skb
>  to remove the dependency of transport in recvmsg()
>- fix virtio dgram_enqueue() by looking up the transport to be used when
>  using sendto(2)
>- fix skb memory leaks in two places
>- add dgram auto-bind test
>- Link to v5: https://lore.kernel.org/r/20230413-b4-vsock-dgram-v5-0-581bd37fdb26@bytedance.com
>
>Changes in v5:
>- teach vhost to drop dgram when a datagram exceeds the receive buffer
>  - now uses MSG_ERRQUEUE and depends on Arseniy's zerocopy patch:
>	"vsock: read from socket's error queue"
>- replace multiple ->dgram_* callbacks with single ->dgram_addr_init()
>  callback
>- refactor virtio dgram skb allocator to reduce conflicts w/ zerocopy series
>- add _fallback/_FALLBACK suffix to dgram transport variables/macros
>- add WARN_ONCE() for table_size / VSOCK_HASH issue
>- add static to vsock_find_bound_socket_common
>- dedupe code in vsock_dgram_sendmsg() using module_got var
>- drop concurrent sendmsg() for dgram and defer to future series
>- Add more tests
>  - test EHOSTUNREACH in errqueue
>  - test stream + dgram address collision
>- improve clarity of dgram msg bounds test code
>- Link to v4: https://lore.kernel.org/r/20230413-b4-vsock-dgram-v4-0-0cebbb2ae899@bytedance.com
>
>Changes in v4:
>- style changes
>  - vsock: use sk_vsock(vsk) in vsock_dgram_recvmsg instead of
>    &sk->vsk
>  - vsock: fix xmas tree declaration
>  - vsock: fix spacing issues
>  - virtio/vsock: virtio_transport_recv_dgram returns void because err
>    unused
>- sparse analysis warnings/errors
>  - virtio/vsock: fix unitialized skerr on destroy
>  - virtio/vsock: fix uninitialized err var on goto out
>  - vsock: fix declarations that need static
>  - vsock: fix __rcu annotation order
>- bugs
>  - vsock: fix null ptr in remote_info code
>  - vsock/dgram: make transport_dgram a fallback instead of first
>    priority
>  - vsock: remove redundant rcu read lock acquire in getname()
>- tests
>  - add more tests (message bounds and more)
>  - add vsock_dgram_bind() helper
>  - add vsock_dgram_connect() helper
>
>Changes in v3:
>- Support multi-transport dgram, changing logic in connect/bind
>  to support VMCI case
>- Support per-pkt transport lookup for sendto() case
>- Fix dgram_allow() implementation
>- Fix dgram feature bit number (now it is 3)
>- Fix binding so dgram and connectible (cid,port) spaces are
>  non-overlapping
>- RCU protect transport ptr so connect() calls never leave
>  a lockless read of the transport and remote_addr are always
>  in sync
>- Link to v2: https://lore.kernel.org/r/20230413-b4-vsock-dgram-v2-0-079cc7cee62e@bytedance.com
>
>
>Bobby Eshleman (14):
>  af_vsock: generalize vsock_dgram_recvmsg() to all transports
>  af_vsock: refactor transport lookup code
>  af_vsock: support multi-transport datagrams
>  af_vsock: generalize bind table functions
>  af_vsock: use a separate dgram bind table
>  virtio/vsock: add VIRTIO_VSOCK_TYPE_DGRAM
>  virtio/vsock: add common datagram send path
>  af_vsock: add vsock_find_bound_dgram_socket()
>  virtio/vsock: add common datagram recv path
>  virtio/vsock: add VIRTIO_VSOCK_F_DGRAM feature bit
>  vhost/vsock: implement datagram support
>  vsock/loopback: implement datagram support
>  virtio/vsock: implement datagram support
>  test/vsock: add vsock dgram tests
>
> drivers/vhost/vsock.c                   |   62 +-
> include/linux/virtio_vsock.h            |    9 +-
> include/net/af_vsock.h                  |   24 +-
> include/uapi/linux/virtio_vsock.h       |    2 +
> net/vmw_vsock/af_vsock.c                |  343 ++++++--
> net/vmw_vsock/hyperv_transport.c        |   13 -
> net/vmw_vsock/virtio_transport.c        |   24 +-
> net/vmw_vsock/virtio_transport_common.c |  188 ++++-
> net/vmw_vsock/vmci_transport.c          |   61 +-
> net/vmw_vsock/vsock_loopback.c          |    9 +-
> tools/testing/vsock/util.c              |  177 +++-
> tools/testing/vsock/util.h              |   10 +
> tools/testing/vsock/vsock_test.c        | 1032 ++++++++++++++++++++---
> 13 files changed, 1638 insertions(+), 316 deletions(-)
>
>-- 
>2.20.1
>


^ permalink raw reply

* Re: [PATCH] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency and throughput.
From: Simon Horman @ 2025-07-22 11:19 UTC (permalink / raw)
  To: Dipayaan Roy
  Cc: kuba, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
	edumazet, pabeni, longli, kotaranov, ast, daniel, hawk,
	john.fastabend, sdf, lorenzo, michal.kubiak, ernis, shradhagupta,
	shirazsaleem, rosenp, netdev, linux-hyperv, linux-rdma, bpf,
	ssengar
In-Reply-To: <20250721101417.GA18873@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>

On Mon, Jul 21, 2025 at 03:14:17AM -0700, Dipayaan Roy wrote:
> This patch enhances RX buffer handling in the mana driver by allocating
> pages from a page pool and slicing them into MTU-sized fragments, rather
> than dedicating a full page per packet. This approach is especially
> beneficial on systems with 64KB page sizes.
> 
> Key improvements:
> 
> - Proper integration of page pool for RX buffer allocations.
> - MTU-sized buffer slicing to improve memory utilization.
> - Reduce overall per Rx queue memory footprint.
> - Automatic fallback to full-page buffers when:
>    * Jumbo frames are enabled (MTU > PAGE_SIZE / 2).
>    * The XDP path is active, to avoid complexities with fragment reuse.
> - Removal of redundant pre-allocated RX buffers used in scenarios like MTU
>   changes, ensuring consistency in RX buffer allocation.
> 
> Testing on VMs with 64KB pages shows around 200% throughput improvement.
> Memory efficiency is significantly improved due to reduced wastage in page
> allocations. Example: We are now able to fit 35 Rx buffers in a single 64KB
> page for MTU size of 1500, instead of 1 Rx buffer per page previously.
> 
> Tested:
> 
> - iperf3, iperf2, and nttcp benchmarks.
> - Jumbo frames with MTU 9000.
> - Native XDP programs (XDP_PASS, XDP_DROP, XDP_TX, XDP_REDIRECT) for
>   testing the driver’s XDP path.
> - Page leak detection (kmemleak).
> - Driver load/unload, reboot, and stress scenarios.
> 
> Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>

Hi,

Some minor feedback from my side.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c

...

> -int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu, int num_queues)
> -{
> -	struct device *dev;
> -	struct page *page;
> -	dma_addr_t da;
> -	int num_rxb;
> -	void *va;
> -	int i;
> -
> -	mana_get_rxbuf_cfg(new_mtu, &mpc->rxbpre_datasize,
> -			   &mpc->rxbpre_alloc_size, &mpc->rxbpre_headroom);
> -
> -	dev = mpc->ac->gdma_dev->gdma_context->dev;
> -
> -	num_rxb = num_queues * mpc->rx_queue_size;
> -
> -	WARN(mpc->rxbufs_pre, "mana rxbufs_pre exists\n");
> -	mpc->rxbufs_pre = kmalloc_array(num_rxb, sizeof(void *), GFP_KERNEL);
> -	if (!mpc->rxbufs_pre)
> -		goto error;
>  
> -	mpc->das_pre = kmalloc_array(num_rxb, sizeof(dma_addr_t), GFP_KERNEL);
> -	if (!mpc->das_pre)
> -		goto error;
> -
> -	mpc->rxbpre_total = 0;
> -
> -	for (i = 0; i < num_rxb; i++) {
> -		page = dev_alloc_pages(get_order(mpc->rxbpre_alloc_size));
> -		if (!page)
> -			goto error;
> -
> -		va = page_to_virt(page);
> -
> -		da = dma_map_single(dev, va + mpc->rxbpre_headroom,
> -				    mpc->rxbpre_datasize, DMA_FROM_DEVICE);
> -		if (dma_mapping_error(dev, da)) {
> -			put_page(page);
> -			goto error;
> +	/* For xdp and jumbo frames make sure only one packet fits per page */
> +	if (((mtu + MANA_RXBUF_PAD) > PAGE_SIZE / 2) || rcu_access_pointer(apc->bpf_prog)) {

The line above seems to have unnecessary parentheses.
And should be line wrapped to be 80 columns wide or less,
as is still preferred by Networking code.
The latter condition is flagged by checkpatch.pl --max-line-length=80

	if (mtu + MANA_RXBUF_PAD > PAGE_SIZE / 2 ||
	    rcu_access_pointer(apc->bpf_prog)) {

(The above is completely untested)

Also, I am a little confused by the use of rcu_access_pointer()
above and below, as bpf_prog does not seem to be managed by RCU.

Flagged by Sparse.

> +		if (rcu_access_pointer(apc->bpf_prog)) {
> +			*headroom = XDP_PACKET_HEADROOM;
> +			*alloc_size = PAGE_SIZE;
> +		} else {
> +			*headroom = 0; /* no support for XDP */
> +			*alloc_size = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD + *headroom);
>  		}
>  
> -		mpc->rxbufs_pre[i] = va;
> -		mpc->das_pre[i] = da;
> -		mpc->rxbpre_total = i + 1;
> +		*frag_count = 1;
> +		return;
>  	}
>  
> -	return 0;
> +	/* Standard MTU case - optimize for multiple packets per page */
> +	*headroom = 0;
>  
> -error:
> -	netdev_err(mpc->ndev, "Failed to pre-allocate RX buffers for %d queues\n", num_queues);
> -	mana_pre_dealloc_rxbufs(mpc);
> -	return -ENOMEM;
> +	/* Calculate base buffer size needed */
> +	u32 len = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD + *headroom);
> +	u32 buf_size = ALIGN(len, MANA_RX_FRAG_ALIGNMENT);
> +
> +	/* Calculate how many packets can fit in a page */
> +	*frag_count = PAGE_SIZE / buf_size;
> +	*alloc_size = buf_size;
>  }

...

> -static void *mana_get_rxfrag(struct mana_rxq *rxq, struct device *dev,
> -			     dma_addr_t *da, bool *from_pool)
> +static void *mana_get_rxfrag(struct mana_rxq *rxq,
> +			     struct device *dev, dma_addr_t *da, bool *from_pool)
>  {
>  	struct page *page;
> +	u32 offset;
>  	void *va;
> -
>  	*from_pool = false;
>  
> -	/* Reuse XDP dropped page if available */
> -	if (rxq->xdp_save_va) {
> -		va = rxq->xdp_save_va;
> -		rxq->xdp_save_va = NULL;
> -	} else {
> -		page = page_pool_dev_alloc_pages(rxq->page_pool);
> -		if (!page)
> +	/* Don't use fragments for jumbo frames or XDP (i.e when fragment = 1 per page) */
> +	if (rxq->frag_count == 1) {
> +		/* Reuse XDP dropped page if available */
> +		if (rxq->xdp_save_va) {
> +			va = rxq->xdp_save_va;
> +			rxq->xdp_save_va = NULL;
> +		} else {
> +			page = page_pool_dev_alloc_pages(rxq->page_pool);
> +			if (!page)
> +				return NULL;
> +
> +			*from_pool = true;
> +			va = page_to_virt(page);
> +		}
> +
> +		*da = dma_map_single(dev, va + rxq->headroom, rxq->datasize,
> +				     DMA_FROM_DEVICE);
> +		if (dma_mapping_error(dev, *da)) {
> +			if (*from_pool)
> +				page_pool_put_full_page(rxq->page_pool, page, false);
> +			else
> +				put_page(virt_to_head_page(va));

The put logic above seems to appear in this patch
more than once. IMHO a helper would be nice.

> +
>  			return NULL;
> +		}
>  
> -		*from_pool = true;
> -		va = page_to_virt(page);
> +		return va;
>  	}

...

-- 
pw-bot: changes-requested

^ permalink raw reply

* Re: [PATCH v5 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-07-22 11:09 UTC (permalink / raw)
  To: Michael Kelley, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
	Dexuan Cui
  Cc: Roman Kisel, Anirudh Rayabharam, Saurabh Sengar,
	Stanislav Kinsburskii, Nuno Das Neves, ALOK TIWARI,
	linux-kernel@vger.kernel.org, linux-hyperv@vger.kernel.org
In-Reply-To: <SN6PR02MB415781ABC3D523B719BDE280D450A@SN6PR02MB4157.namprd02.prod.outlook.com>



On 7/18/2025 8:37 PM, Michael Kelley wrote:
> From: Naman Jain <namjain@linux.microsoft.com> Sent: Thursday, July 17, 2025 9:36 PM
>>
>> On 7/9/2025 10:49 PM, Michael Kelley wrote:
>>> From: Naman Jain <namjain@linux.microsoft.com> Sent: Wednesday, June 11, 2025 12:27 AM
> 
> [snip]
> 
>>
>>> Separately, "allow_bitmap" size is 64K bytes, or 512K bits. Is that the
>>> correct size?  From looking at mshv_vtl_hvcall_is_allowed(), I think this
>>> bitmap is indexed by the HV call code, which is a 16 bit value. So you
>>> only need 64K bits, and the size is too big by a factor of 8. In any case,
>>> it seems like the size should not be expressed in terms of PAGE_SIZE.
>>
>> There are HVcall codes which are of type u16. So max(HVcall code) =
>> 0xffff.
>>
>> For every HVcall that needs to be allowed, we are saving HVcall code
>> info in a bitmap in below fashion:
>> if x = HVCall code and bitmap is an array of u64, of size
>> ((0xffff/64=1023) + 1)
>>
>> bitmap[x / 64] = (u64)1 << (x%64);
>>
>> Later on in mshv_vtl_hvcall_is_allowed(), we calculate the array index
>> by dividing it by 64, and then see if call_code/64 bit is set.
> 
> I didn't add comments in mshv_vtl_hvcall_is_allowed(), but that code
> can be simplified by recognizing that the Linux kernel bitmap utilities
> can operate on bitmaps that are much larger than just 64 bits. Let's
> assume that the allow_bitmap field in struct mshv_vtl_hvcall_fds has
> 64K bits, regardless of whether it is declared as an array of u64,
> an array of u16, or an array of u8. Then mshv_vtl_hvcall_is_allowed()
> can be implemented as a single line:
> 
> 	return test_bit(call_code, fd->allow_bitmap);
> 
> There's no need to figure out which array element contains the bit,
> or to construct a mask to select that particular bit in the array element.
> And since call_code is a u16, test_bit won't access outside the allocated
> 64K bits.
> 

I understood it now. This works and is much better. Will incorporate it
in next patch.

>>
>> Coming to size of allow_bitmap[], it is independent of PAGE_SIZE, and
>> can be safely initialized to 1024 (reducing by a factor of 8).
>> bitmap_size's maximum value is going to be 1024 in current
>> implementation, picking u64 was not mandatory, u16 will also work. Also,
>> item_index is also u16, so I should make bitmap_size as u16.
> 
> The key question for me is whether bitmap_size describes the number
> of bits in allow_bitmap, or whether it describes the number of array
> elements in the declared allow_bitmap array. It's more typical to
> describe a bitmap size as the number of bits. Then the value is
> independent of the array element size, as the array element size
> usually doesn't really matter anyway if using the Linux kernel's
> bitmap utilities. The array element size only matters in allocating
> the correct amount of space is for whatever number of bits are
> needed in the bitmap.
> 

I tried to put your suggestions in code. Please let me know if below 
works. I tested this and it works. Just that, I am a little hesitant in 
changing things on Userspace side of it, which passes these parameters 
in IOCTL. This way, userspace remains the same, the confusion of names 
may go away, and the code becomes simpler.

--- a/include/uapi/linux/mshv.h
+++ b/include/uapi/linux/mshv.h
@@ -332,7 +332,7 @@ struct mshv_vtl_set_poll_file {
  };

  struct mshv_vtl_hvcall_setup {
-       __u64 bitmap_size;
+       __u64 bitmap_array_size;
         __u64 allow_bitmap_ptr; /* pointer to __u64 */
  };


--- a/drivers/hv/mshv_vtl_main.c
+++ b/drivers/hv/mshv_vtl_main.c
@@ -52,10 +52,12 @@ static bool has_message;
  static struct eventfd_ctx *flag_eventfds[HV_EVENT_FLAGS_COUNT];
  static DEFINE_MUTEX(flag_lock);
  static bool __read_mostly mshv_has_reg_page;
-#define MAX_BITMAP_SIZE 1024
+
+/* hvcall code is of type u16, allocate a bitmap of size (1 << 16) to 
accomodate it */
+#define MAX_BITMAP_SIZE (1 << 16)

  struct mshv_vtl_hvcall_fd {
-       u64 allow_bitmap[MAX_BITMAP_SIZE];
+       u64 allow_bitmap[MAX_BITMAP_SIZE / 64];
         bool allow_map_initialized;
         /*
          * Used to protect hvcall setup in IOCTLs

@@ -1204,12 +1207,12 @@ static int mshv_vtl_hvcall_do_setup(struct 
mshv_vtl_hvcall_fd *fd,
                            sizeof(struct mshv_vtl_hvcall_setup))) {
                 return -EFAULT;
         }
-       if (hvcall_setup.bitmap_size > ARRAY_SIZE(fd->allow_bitmap)) {
+       if (hvcall_setup.bitmap_array_size > ARRAY_SIZE(fd->allow_bitmap)) {
                 return -EINVAL;
         }
         if (copy_from_user(&fd->allow_bitmap,
                            (void __user *)hvcall_setup.allow_bitmap_ptr,
-                          hvcall_setup.bitmap_size)) {
+                          hvcall_setup.bitmap_array_size)) {
                 return -EFAULT;
         }

@@ -1221,11 +1224,7 @@ static int mshv_vtl_hvcall_do_setup(struct 
mshv_vtl_hvcall_fd *fd,

  static bool mshv_vtl_hvcall_is_allowed(struct mshv_vtl_hvcall_fd *fd, 
u16 call_code)
  {
-       u8 bits_per_item = 8 * sizeof(fd->allow_bitmap[0]);
-       u16 item_index = call_code / bits_per_item;
-       u64 mask = 1ULL << (call_code % bits_per_item);
-
-       return fd->allow_bitmap[item_index] & mask;
+       return test_bit(call_code, (unsigned long *)fd->allow_bitmap);
  }

> [snip]
> 
>>>> +
>>>> +	event_flags = (union hv_synic_event_flags *)per_cpu->synic_event_page +
>>>> +			VTL2_VMBUS_SINT_INDEX;
>>>> +	for (i = 0; i < HV_EVENT_FLAGS_LONG_COUNT; i++) {
>>>> +		if (READ_ONCE(event_flags->flags[i])) {
>>>> +			word = xchg(&event_flags->flags[i], 0);
>>>> +			for_each_set_bit(j, &word, BITS_PER_LONG) {
>>>
>>> Is there a reason for the complexity in finding and resetting bits that are
>>> set in the sync_event_page?  See the code in vmbus_chan_sched() that I
>>> think is doing the same thing, but with simpler code.
>>
>> I am sorry, but I am not sure how this can be written similar to
>> vmbus_chan_sched(). We don't have eventfd signaling mechanism there.
>> Can you please share some more info/code snippet of what you were
>> suggesting?
> 
> See below.
> 
>>
>>
>>>
>>>> +				rcu_read_lock();
>>>> +				eventfd = READ_ONCE(flag_eventfds[i * BITS_PER_LONG + j]);
>>>> +				if (eventfd)
>>>> +					eventfd_signal(eventfd);
>>>> +				rcu_read_unlock();
>>>> +			}
>>>> +		}
>>>> +	}
> 
> Here's what I would suggest. As with the hvcall allow_bitmap, this uses
> the Linux kernel bitmap utilities' ability to operate on large bitmaps, instead
> of going through each ulong in the array, and then going through each bit
> in the ulong.
> 
> event_flags = (union hv_synic_event_flags *)per_cpu->synic_event_page + VTL2_VMBUS_SINT_INDEX;
> 
> for_each_set_bit(i, event_flags->flags, HV_EVENT_FLAGS_COUNT) {
> 	if (!sync_test_and_clear_bit(i, event_flags->flags))
> 		continue;
> 	rcu_read_lock();
> 	eventfd = READ_ONCE(flag_eventfds[i]);
> 	if (eventfd)
> 		eventfd_signal(eventfd);
> 	rcu_read_unlock();
> }
> 
> I haven't even compile tested the above, but hopefully you get the
> idea and can fix any stupid mistakes. Note that HV_EVENT_FLAGS_COUNT
> is a bit count, not a count of ulong's. And with the above code, you don't
> need to add a definition of HV_EVENT_FLAGS_LONG_COUNT.

Thanks for sharing this, it works. Will change it in next patch.

> 
> [snip]
> 
>>>> +	pgmap = kzalloc(sizeof(*pgmap), GFP_KERNEL);
>>>> +	if (!pgmap)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	pgmap->ranges[0].start = PFN_PHYS(vtl0_mem.start_pfn);
>>>> +	pgmap->ranges[0].end = PFN_PHYS(vtl0_mem.last_pfn) - 1;
>>>
>>> Perhaps this should be
>>>
>>> 	pgmap->ranges[0].end = PFN_PHYS(vtl0_mem.last_pfn + 1) - 1
>>>
>>> otherwise the last page won't be included in the range. Or is excluding the
>>> last page intentional?
>>
>> Excluding the last page is intentional. Hence there is a check for this
>> as well:
>> if (vtl0_mem.last_pfn <= vtl0_mem.start_pfn) {
>>
> 
> OK, this test requires that at least 2 PFNs be provided, because the
> last one will be excluded.
> 
> I'd suggest adding a comment that the last page is intentionally
> excluded, and why it is excluded. Somebody in future looking at this
> code will appreciate the explanation. :-)
> 

Acked.


> [snip]
> 
>>>
>>>> +
>>>> +	if (!cpu_online(input.cpu))
>>>> +		return -EINVAL;
>>>
>>> Having tested that the target CPU is online, does anything ensure that the
>>> CPU stays online during the completion of this function? Usually the
>>> cpus_read_lock() needs to be held to ensure that an online CPU stays
>>> online for the duration of an operation.
>>
>> Added cpus_read_lock() block around per_cpu_ptr operation. In general,
>> CPUs are never hotplugged in kernel from our Usecase POV. I have omitted
>> adding these locks at other places for now. Please let me know your
>> thoughts on this, in case you feel we need to have it.
>>
> 
> My understanding of VTL2 behavior is limited, so let me ask some clarifying
> questions. If a vCPU is running in VTL0, then presumably that vCPU is also
> running in VTL2. If that vCPU is then taken offline in VTL0, does it stay
> online in VTL2? And then if the vCPU is brought back online in VTL0,
> nothing changes in VTL2, correct?
> 
> If that is the correct understanding, and vCPUs never go offline in VTL2,
> it would be more robust to enforce that. For example, in hv_vtl_setup_synic()
> where cpuhp_setup_state() is called, the teardown argument is currently
> NULL. You could provide a teardown function that just returns an error.
> Then any attempts to take a vCPU offline in VTL2 would fail, and the vCPU
> would stay online. However, some additional logic might be needed to
> ensure that normal shutdown and the panic case work correctly -- I'm not
> sure what VTL2 needs to do for these scenarios.
> 
> All that said, if you can be sure that vCPUs don't go offline in VTL2,
> I would be OK with not adding the cpus_read_lock(). Perhaps a comment
> would be helpful in the places where you are not using cpus_read_lock()
> for this reason, assuming there is a reasonable number of such places.

Not to make any assumptions, I am trying to gather more information on
this, I will respond to it soon.

> 
> [snip]
> 
>>>> +
>>>> +struct mshv_vtl_hvcall_setup {
>>>> +	__u64 bitmap_size;
>>>
>>> What are the units of "bitmap_size"?  Bits? Bytes? u64?
>>
>> It would be length of bitmap array.
> 
> To me "length of bitmap array" is still ambiguous. Is it the
> number of elements in the declared array field? Per my
> earlier comments, I think the number of bits in the bitmap
> would be more typical.

does bitmap_array_size work?

> 
>>
>>>
>>>> +	__u64 allow_bitmap_ptr; /* pointer to __u64 */
>>>> +};
> 
> Michael


Regards,
Naman


^ permalink raw reply

* Re: [PATCH v5 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Naman Jain @ 2025-07-22  9:16 UTC (permalink / raw)
  To: Markus Elfring, Roman Kisel, Saurabh Sengar, linux-hyperv,
	Dexuan Cui, Haiyang Zhang, K. Y. Srinivasan, Wei Liu
  Cc: LKML, Alok Tiwari, Anirudh Rayabharam, Nuno Das Neves,
	Stanislav Kinsburskii
In-Reply-To: <68120ea5-d3ba-4077-a605-50a0b5188761@web.de>



On 7/20/2025 12:00 AM, Markus Elfring wrote:
> …
>> +++ b/drivers/hv/mshv_vtl_main.c
>> @@ -0,0 +1,1783 @@
> …
>> +static int mshv_vtl_sint_ioctl_set_eventfd(struct mshv_vtl_set_eventfd __user *arg)
>> +{
> …
>> +	mutex_lock(&flag_lock);
>> +	old_eventfd = flag_eventfds[set_eventfd.flag];
>> +	WRITE_ONCE(flag_eventfds[set_eventfd.flag], eventfd);
>> +	mutex_unlock(&flag_lock);
> …
> 
> Under which circumstances would you become interested to apply a statement
> like “guard(mutex)(&flag_lock);”?
> https://elixir.bootlin.com/linux/v6.16-rc6/source/include/linux/mutex.h#L225
> 
> Regards,
> Markus

I didn't know about it, TBH. I will use it in this driver in all places.
I have tested the same in my setup.

Thanks.

Regards,
Naman

^ permalink raw reply

* Re: [PATCH net] net: core: Fix the loop in default_device_exit_net()
From: Eric Dumazet @ 2025-07-22  6:51 UTC (permalink / raw)
  To: Haiyang Zhang
  Cc: linux-hyperv, netdev, haiyangz, kys, wei.liu, kuba, pabeni, horms,
	davem, sdf, kuniyu, ahmed.zaki, aleksander.lobakin, linux-kernel,
	stable, #, 5.4+
In-Reply-To: <1752870014-28909-1-git-send-email-haiyangz@linux.microsoft.com>

On Fri, Jul 18, 2025 at 1:21 PM Haiyang Zhang
<haiyangz@linux.microsoft.com> wrote:
>
> From: Haiyang Zhang <haiyangz@microsoft.com>
>
> The loop in default_device_exit_net() won't be able to properly detect the
> head then stop, and will hit NULL pointer, when a driver, like hv_netvsc,
> automatically moves the slave device together with the master device.
>
> To fix this, add a helper function to return the first migratable netdev
> correctly, no matter one or two devices were removed from this net's list
> in the last iteration.
>
> Cc: stable@vger.kernel.org # 5.4+

We (network maintainers) prefer a Fixes: tag, so that we can look at
the blamed patch, rather than trusting your '5.4' hint.

Without a Fixes tag, you are forcing each reviewer to do the
archeology work, and possibly completely miss your point.

^ permalink raw reply

* Re: [PATCH RESEND] netvsc: transfer lower device max tso size
From: Cindy Lu @ 2025-07-22  2:46 UTC (permalink / raw)
  To: Jakub Kicinski, Long Li, stephen
  Cc: Jason Wang, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Michael Kelley, Shradha Gupta, Kees Cook,
	Stanislav Fomichev, Kuniyuki Iwashima, Alexander Lobakin,
	Guillaume Nault, Joe Damato, Ahmed Zaki,
	open list:Hyper-V/Azure CORE AND DRIVERS,
	open list:NETWORKING DRIVERS, open list
In-Reply-To: <CACLfguXG7Mpsp=z4zCE7H4CMA_s9qV86SkeL7Q=WxChXcFpNfA@mail.gmail.com>

On Tue, Jul 22, 2025 at 10:04 AM Cindy Lu <lulu@redhat.com> wrote:
>
> On Tue, Jul 22, 2025 at 9:18 AM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > On Tue, 22 Jul 2025 09:04:20 +0800 Jason Wang wrote:
> > > On Tue, Jul 22, 2025 at 7:28 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > > > On Fri, 18 Jul 2025 14:17:55 +0800 Cindy Lu wrote:
> > > > > Subject: [PATCH RESEND] netvsc: transfer lower device max tso size
> > > >
> > > > You say RESEND but I don't see a link to previous posting anywhere.
> >
> > Someone should respond to this part, please.
> >
> Hi Jakub,
> sorry for the confusion. I previously sent this mail
> (https://lore.kernel.org/all/20250718060615.237986-1-lulu@redhat.com/)
> to the wrong mailing list, so I'm resended it here.
> I've also submitted a v2 of this patch:
> https://lore.kernel.org/all/20250718082909.243488-1-lulu@redhat.com/
> Sorry again for the mix-up.
> thanks
>
> cindy
>
> > > > I'd rather we didn't extend the magic behavior of hyperv/netvsc any
> > > > further.
> > >
> > > Are you referring to the netdev coupling model of the VF acceleration?
> >
> > Yes, it tries to apply whole bunch of policy automatically in
> > the kernel.
> >
> > > > We have enough problems with it.
> > >
> > > But this fixes a real problem, otherwise nested VM performance will be
> > > broken due to the GSO software segmentation.
> >
> > Perhaps, possibly, a migration plan can be devised, away from the
> > netvsc model, so we don't have to deal with nuggets of joy like:
> > https://lore.kernel.org/all/1752870014-28909-1-git-send-email-haiyangz@linux.microsoft.com/

I'm also including Stephen Hemminger and Long Li in this thread and
would greatly appreciate any suggestions.

Thanks
cindy

> >


^ permalink raw reply

* Re: [PATCH RESEND] netvsc: transfer lower device max tso size
From: Cindy Lu @ 2025-07-22  2:04 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Jason Wang, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Michael Kelley, Shradha Gupta, Kees Cook,
	Stanislav Fomichev, Kuniyuki Iwashima, Alexander Lobakin,
	Guillaume Nault, Joe Damato, Ahmed Zaki,
	open list:Hyper-V/Azure CORE AND DRIVERS,
	open list:NETWORKING DRIVERS, open list
In-Reply-To: <20250721181807.752af6a4@kernel.org>

On Tue, Jul 22, 2025 at 9:18 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 22 Jul 2025 09:04:20 +0800 Jason Wang wrote:
> > On Tue, Jul 22, 2025 at 7:28 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > > On Fri, 18 Jul 2025 14:17:55 +0800 Cindy Lu wrote:
> > > > Subject: [PATCH RESEND] netvsc: transfer lower device max tso size
> > >
> > > You say RESEND but I don't see a link to previous posting anywhere.
>
> Someone should respond to this part, please.
>
Hi Jakub,
sorry for the confusion. I previously sent this mail
(https://lore.kernel.org/all/20250718060615.237986-1-lulu@redhat.com/)
to the wrong mailing list, so I'm resended it here.
I've also submitted a v2 of this patch:
https://lore.kernel.org/all/20250718082909.243488-1-lulu@redhat.com/
Sorry again for the mix-up.
thanks

cindy

> > > I'd rather we didn't extend the magic behavior of hyperv/netvsc any
> > > further.
> >
> > Are you referring to the netdev coupling model of the VF acceleration?
>
> Yes, it tries to apply whole bunch of policy automatically in
> the kernel.
>
> > > We have enough problems with it.
> >
> > But this fixes a real problem, otherwise nested VM performance will be
> > broken due to the GSO software segmentation.
>
> Perhaps, possibly, a migration plan can be devised, away from the
> netvsc model, so we don't have to deal with nuggets of joy like:
> https://lore.kernel.org/all/1752870014-28909-1-git-send-email-haiyangz@linux.microsoft.com/
>


^ permalink raw reply

* Re: [PATCH RESEND] netvsc: transfer lower device max tso size
From: Jakub Kicinski @ 2025-07-22  1:18 UTC (permalink / raw)
  To: Jason Wang
  Cc: Cindy Lu, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Michael Kelley, Shradha Gupta, Kees Cook,
	Stanislav Fomichev, Kuniyuki Iwashima, Alexander Lobakin,
	Guillaume Nault, Joe Damato, Ahmed Zaki,
	open list:Hyper-V/Azure CORE AND DRIVERS,
	open list:NETWORKING DRIVERS, open list
In-Reply-To: <CACGkMEtqhjTjdxPc=eqMxPNKFsKKA+5YP+uqWtonm=onm0gCrg@mail.gmail.com>

On Tue, 22 Jul 2025 09:04:20 +0800 Jason Wang wrote:
> On Tue, Jul 22, 2025 at 7:28 AM Jakub Kicinski <kuba@kernel.org> wrote:
> > On Fri, 18 Jul 2025 14:17:55 +0800 Cindy Lu wrote:  
> > > Subject: [PATCH RESEND] netvsc: transfer lower device max tso size  
> >
> > You say RESEND but I don't see a link to previous posting anywhere.

Someone should respond to this part, please.

> > I'd rather we didn't extend the magic behavior of hyperv/netvsc any
> > further.  
> 
> Are you referring to the netdev coupling model of the VF acceleration?

Yes, it tries to apply whole bunch of policy automatically in 
the kernel.

> > We have enough problems with it.
> 
> But this fixes a real problem, otherwise nested VM performance will be
> broken due to the GSO software segmentation.

Perhaps, possibly, a migration plan can be devised, away from the
netvsc model, so we don't have to deal with nuggets of joy like:
https://lore.kernel.org/all/1752870014-28909-1-git-send-email-haiyangz@linux.microsoft.com/

^ permalink raw reply

* Re: [PATCH RESEND] netvsc: transfer lower device max tso size
From: Jason Wang @ 2025-07-22  1:04 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Cindy Lu, K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Andrew Lunn, David S. Miller, Eric Dumazet, Paolo Abeni,
	Simon Horman, Michael Kelley, Shradha Gupta, Kees Cook,
	Stanislav Fomichev, Kuniyuki Iwashima, Alexander Lobakin,
	Guillaume Nault, Joe Damato, Ahmed Zaki,
	open list:Hyper-V/Azure CORE AND DRIVERS,
	open list:NETWORKING DRIVERS, open list
In-Reply-To: <20250721162834.484d352a@kernel.org>

On Tue, Jul 22, 2025 at 7:28 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 18 Jul 2025 14:17:55 +0800 Cindy Lu wrote:
> > Subject: [PATCH RESEND] netvsc: transfer lower device max tso size
>
> You say RESEND but I don't see a link to previous posting anywhere.
>
> I'd rather we didn't extend the magic behavior of hyperv/netvsc any
> further.

Are you referring to the netdev coupling model of the VF acceleration?

> We have enough problems with it.
>

But this fixes a real problem, otherwise nested VM performance will be
broken due to the GSO software segmentation.

Thanks


^ permalink raw reply

* Re: [PATCH] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency and throughput.
From: Jacob Keller @ 2025-07-21 23:51 UTC (permalink / raw)
  To: Dipayaan Roy, kuba, kys, haiyangz, wei.liu, decui, andrew+netdev,
	davem, edumazet, pabeni, longli, kotaranov, horms, ast, daniel,
	hawk, john.fastabend, sdf, lorenzo, michal.kubiak, ernis,
	shradhagupta, shirazsaleem, rosenp, netdev, linux-hyperv,
	linux-rdma, bpf, ssengar
In-Reply-To: <20250721101417.GA18873@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net>


[-- Attachment #1.1: Type: text/plain, Size: 1516 bytes --]



On 7/21/2025 3:14 AM, Dipayaan Roy wrote:
> This patch enhances RX buffer handling in the mana driver by allocating
> pages from a page pool and slicing them into MTU-sized fragments, rather
> than dedicating a full page per packet. This approach is especially
> beneficial on systems with 64KB page sizes.
> 
> Key improvements:
> 
> - Proper integration of page pool for RX buffer allocations.
> - MTU-sized buffer slicing to improve memory utilization.
> - Reduce overall per Rx queue memory footprint.
> - Automatic fallback to full-page buffers when:
>    * Jumbo frames are enabled (MTU > PAGE_SIZE / 2).
>    * The XDP path is active, to avoid complexities with fragment reuse.
> - Removal of redundant pre-allocated RX buffers used in scenarios like MTU
>   changes, ensuring consistency in RX buffer allocation.
> 
> Testing on VMs with 64KB pages shows around 200% throughput improvement.
> Memory efficiency is significantly improved due to reduced wastage in page
> allocations. Example: We are now able to fit 35 Rx buffers in a single 64KB
> page for MTU size of 1500, instead of 1 Rx buffer per page previously.
> 
Nice to see such improvements while also reducing the overall driver
code footprint!

Do you happen to have numbers on some of the smaller page sizes? I'm not
sure how common 64KB is, but it seems like this would still be quite
beneficial even if you had 4K or 8K pages when operating at 1500 MTU.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 236 bytes --]

^ permalink raw reply

* Re: [PATCH RESEND] netvsc: transfer lower device max tso size
From: Jakub Kicinski @ 2025-07-21 23:28 UTC (permalink / raw)
  To: Cindy Lu
  Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Andrew Lunn,
	David S. Miller, Eric Dumazet, Paolo Abeni, Simon Horman,
	Michael Kelley, Shradha Gupta, Kees Cook, Jason Wang,
	Stanislav Fomichev, Kuniyuki Iwashima, Alexander Lobakin,
	Guillaume Nault, Joe Damato, Ahmed Zaki,
	open list:Hyper-V/Azure CORE AND DRIVERS,
	open list:NETWORKING DRIVERS, open list
In-Reply-To: <20250718061812.238412-1-lulu@redhat.com>

On Fri, 18 Jul 2025 14:17:55 +0800 Cindy Lu wrote:
> Subject: [PATCH RESEND] netvsc: transfer lower device max tso size

You say RESEND but I don't see a link to previous posting anywhere.

I'd rather we didn't extend the magic behavior of hyperv/netvsc any
further. We have enough problems with it.

^ permalink raw reply

* RE: [PATCH v4 0/7] hyperv: Introduce new way to manage hypercall args
From: Michael Kelley @ 2025-07-21 19:30 UTC (permalink / raw)
  To: Roman Kisel, Easwar Hariharan, Nuno Das Neves, Naman Jain
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
	lpieralisi@kernel.org, kw@linux.com, mani@kernel.org,
	robh@kernel.org, bhelgaas@google.com, arnd@arndb.de,
	x86@kernel.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arch@vger.kernel.org
In-Reply-To: <5264d73d-ae1b-4173-b304-b92e18a3befb@linux.microsoft.com>

From: Roman Kisel <romank@linux.microsoft.com> Sent: Monday, July 21, 2025 10:15 AM
> 
> On 7/20/2025 7:19 PM, Michael Kelley wrote:
> > From: Roman Kisel <romank@linux.microsoft.com> Sent: Friday, July 18, 2025 6:16
> PM
> 
> [...]
> 
> >
> > Thanks for any testing you can do on standalone test machines without
> > needing test clusters in Azure. It will be hard to get test coverage on
> > *every* hypercall call site that is modified by the patch set, but doing
> > basic smoke testing of running in the root partition and in VTL2 will
> > cover more than I can cover running in a VTL0 guest on my laptop or
> > in Azure. Fortunately, the changes overall in this patch set are pretty
> > straightforward, and my testing of VTL0 guests didn’t turn up any bugs.
> > I'm hoping that additional smoke testing is more about gaining
> > confidence than finding actual bugs.  (Famous last words ....)
> >
> 
> Thank you a million times for pushing the bar higher and supporting the
> code :)
> 
> >> VTL2 currently uses a limited number hypercalls that are set as enabled
> >> in the OpenVMM code (`set_allowed_hypercalls`). You could take a look
> >> and conclude if these hypercalls require any adjustments in the patches.
> >
> > My patch set already covers all the hypercall call sites that originate in
> > VTL2 code. Again, a basic smoke test should help gain confidence, or
> > show that any confidence is misplaced :-)
> >
> 
> Very nice, should be smooth sailing then :)
> 
> >> My opinion has been to have two pages (input and output ones). As the
> >> new code introduces just one page I do feel a bit apprehensive, got no
> >> hard evidence that this is a bad approach though. If we tweak the code
> >> to have 2 pages, perhaps there would be no need to run a full-blown
> >> validation, and even smoke tests will suffice?
> >
> > My view is that the 1 page vs. 2 pages is much less of a risk than just
> > some coding error in introducing the new interfaces. The 1 page vs.
> > 2 pages should only affect the batch size for rep hypercalls, and the
> > existing code already handles different batch sizes. So I'm not as
> > concerned about that risk. Wei Liu in the maintainer here, so I'll
> > certainly follow his judgment and guidance on what is needed to
> > be confident in this patch set.
> >
> 
> I agree with your risk assessment. Perhaps I am playing too much of
> a spec lawyer yet it states
> 
> 1) Input and output area may not intersect,
> 2) Either can be up to 4KiB of size.
> 
> Hence, one (be that for feature development or one-off debugging) would
> be within their right to implement a hypercall that accepts 4KiB of
> data and returns 4KiB of data. My understanding that after this patch,
> that won't work out-of-the-box, and would need some fixing in the
> kernel.
> 
> Perhaps, we could have a KConfig option to let the user choose if they
> need 2 pages instead of making the user figure out what needs to be
> fixed in the kernel?
> 

I'm not seeing the scenario where a Kconfig option is useful. Here's
my thinking:

A putative new hypercall that requires more than 4K for the sum of
the input and output would not be a rep hypercall. So the hypercall
has some large structure for the input and/or the output. Such a
hypercall would be handled one of two ways in the kernel:

(1) New code must be added in the kernel that includes a hypercall
call site to invoke this hypercall. The new code populates the large
input structure, and/or processes the large output structure in order
to accomplish whatever the Linux kernel needs done that the
hypercall helps with.

(2) The hypercall is invoked for the root partition or VTL2 via one
of the existing ioctl's that allow user space in the root partition or
VTL2 to invoke arbitrary hypercalls and get the results.

For (1), new code is being added to the kernel. So changing the
#define HV_HVCALL_ARG_PAGES from "1" to "2" is just part
of the code changes needed for the new hypercall.

For (2), the ioctls, such as mshv_ioctl_passthru_hvcall(), don't
use the pre-allocated hyperv_pcpu_input/output_arg. They do
a separate allocation of a full page for input and a full page for
output because of the complexities of copying the input
and output args from/to user space. So if the putative new
hypercall is invoked via the ioctl, it will just work.

I don't see a scenario where a Kconfig option provides any
value to someone building the kernel. Linux kernel code also
generally eschews adding mechanism to support future
scenarios that might or might not happen. Sure, we design
code to be extensible, but we don't generally add options or
APIs that aren't currently needed. I've consistently gotten
feedback to add such when/if the need actually arises. I
think the #define HV_HVCALL_ARG_PAGES 1 provides the
right balance.

Michael

^ permalink raw reply

* Re: [PATCH v4 0/7] hyperv: Introduce new way to manage hypercall args
From: Roman Kisel @ 2025-07-21 17:14 UTC (permalink / raw)
  To: Michael Kelley, Easwar Hariharan, Nuno Das Neves, Naman Jain
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
	lpieralisi@kernel.org, kw@linux.com, mani@kernel.org,
	robh@kernel.org, bhelgaas@google.com, arnd@arndb.de,
	x86@kernel.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arch@vger.kernel.org
In-Reply-To: <SN6PR02MB4157A2A90910918AE9B6327BD45DA@SN6PR02MB4157.namprd02.prod.outlook.com>



On 7/20/2025 7:19 PM, Michael Kelley wrote:
> From: Roman Kisel <romank@linux.microsoft.com> Sent: Friday, July 18, 2025 6:16 PM

[...]

> 
> Thanks for any testing you can do on standalone test machines without
> needing test clusters in Azure. It will be hard to get test coverage on
> *every* hypercall call site that is modified by the patch set, but doing
> basic smoke testing of running in the root partition and in VTL2 will
> cover more than I can cover running in a VTL0 guest on my laptop or
> in Azure. Fortunately, the changes overall in this patch set are pretty
> straightforward, and my testing of VTL0 guests didn’t turn up any bugs.
> I'm hoping that additional smoke testing is more about gaining
> confidence than finding actual bugs.  (Famous last words ....)
> 

Thank you a million times for pushing the bar higher and supporting the
code :)

>> VTL2 currently uses a limited number hypercalls that are set as enabled
>> in the OpenVMM code (`set_allowed_hypercalls`). You could take a look
>> and conclude if these hypercalls require any adjustments in the patches.
> 
> My patch set already covers all the hypercall call sites that originate in
> VTL2 code. Again, a basic smoke test should help gain confidence, or
> show that any confidence is misplaced :-)
> 

Very nice, should be smooth sailing then :)

>> My opinion has been to have two pages (input and output ones). As the
>> new code introduces just one page I do feel a bit apprehensive, got no
>> hard evidence that this is a bad approach though. If we tweak the code
>> to have 2 pages, perhaps there would be no need to run a full-blown
>> validation, and even smoke tests will suffice?
> 
> My view is that the 1 page vs. 2 pages is much less of a risk than just
> some coding error in introducing the new interfaces. The 1 page vs.
> 2 pages should only affect the batch size for rep hypercalls, and the
> existing code already handles different batch sizes. So I'm not as
> concerned about that risk. Wei Liu in the maintainer here, so I'll
> certainly follow his judgment and guidance on what is needed to
> be confident in this patch set.
> 

I agree with your risk assessment. Perhaps I am playing too much of
a spec lawyer yet it states

1) Input and output area may not intersect,
2) Either can be up to 4KiB of size.

Hence, one (be that for feature development or one-off debugging) would
be within their right to implement a hypercall that accepts 4KiB of
data and returns 4KiB of data. My understanding that after this patch,
that won't work out-of-the-box, and would need some fixing in the
kernel.

Perhaps, we could have a KConfig option to let the user choose if they
need 2 pages instead of making the user figure out what needs to be
fixed in the kernel?

> Michael

-- 
Thank you,
Roman


^ permalink raw reply

* [PATCH] net: mana: Use page pool fragments for RX buffers instead of full pages to improve memory efficiency and throughput.
From: Dipayaan Roy @ 2025-07-21 10:14 UTC (permalink / raw)
  To: kuba, kys, haiyangz, wei.liu, decui, andrew+netdev, davem,
	edumazet, pabeni, longli, kotaranov, horms, ast, daniel, hawk,
	john.fastabend, sdf, lorenzo, michal.kubiak, ernis, shradhagupta,
	shirazsaleem, rosenp, netdev, linux-hyperv, linux-rdma, bpf,
	dipayanroy, ssengar

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=unknown-8bit, Size: 16754 bytes --]

This patch enhances RX buffer handling in the mana driver by allocating
pages from a page pool and slicing them into MTU-sized fragments, rather
than dedicating a full page per packet. This approach is especially
beneficial on systems with 64KB page sizes.

Key improvements:

- Proper integration of page pool for RX buffer allocations.
- MTU-sized buffer slicing to improve memory utilization.
- Reduce overall per Rx queue memory footprint.
- Automatic fallback to full-page buffers when:
   * Jumbo frames are enabled (MTU > PAGE_SIZE / 2).
   * The XDP path is active, to avoid complexities with fragment reuse.
- Removal of redundant pre-allocated RX buffers used in scenarios like MTU
  changes, ensuring consistency in RX buffer allocation.

Testing on VMs with 64KB pages shows around 200% throughput improvement.
Memory efficiency is significantly improved due to reduced wastage in page
allocations. Example: We are now able to fit 35 Rx buffers in a single 64KB
page for MTU size of 1500, instead of 1 Rx buffer per page previously.

Tested:

- iperf3, iperf2, and nttcp benchmarks.
- Jumbo frames with MTU 9000.
- Native XDP programs (XDP_PASS, XDP_DROP, XDP_TX, XDP_REDIRECT) for
  testing the driver’s XDP path.
- Page leak detection (kmemleak).
- Driver load/unload, reboot, and stress scenarios.

Signed-off-by: Dipayaan Roy <dipayanroy@linux.microsoft.com>
---
 .../net/ethernet/microsoft/mana/mana_bpf.c    |  22 +-
 drivers/net/ethernet/microsoft/mana/mana_en.c | 284 ++++++------------
 .../ethernet/microsoft/mana/mana_ethtool.c    |  13 -
 include/net/mana/mana.h                       |  13 +-
 4 files changed, 115 insertions(+), 217 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/mana_bpf.c b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
index d30721d4516f..96813b6c184f 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_bpf.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_bpf.c
@@ -174,6 +174,7 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
 	struct mana_port_context *apc = netdev_priv(ndev);
 	struct bpf_prog *old_prog;
 	struct gdma_context *gc;
+	int err;
 
 	gc = apc->ac->gdma_dev->gdma_context;
 
@@ -198,14 +199,33 @@ static int mana_xdp_set(struct net_device *ndev, struct bpf_prog *prog,
 	if (old_prog)
 		bpf_prog_put(old_prog);
 
-	if (apc->port_is_up)
+	if (apc->port_is_up) {
+		/* Re-create rxq's after xdp prog was loaded or unloaded.
+		 * Ex: re create rxq's to switch from full pages to smaller
+		 * size page fragments when xdp prog is unloaded and vice-versa.
+		 */
+
+		err = mana_detach(ndev, false);
+		if (err) {
+			netdev_err(ndev, "mana_detach failed at xdp set: %d\n", err);
+			goto out;
+		}
+
+		err = mana_attach(ndev);
+		if (err) {
+			netdev_err(ndev, "mana_attach failed at xdp set: %d\n", err);
+			goto out;
+		}
+
 		mana_chn_setxdp(apc, prog);
+	}
 
 	if (prog)
 		ndev->max_mtu = MANA_XDP_MTU_MAX;
 	else
 		ndev->max_mtu = gc->adapter_mtu - ETH_HLEN;
 
+out:
 	return 0;
 }
 
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index a7973651ae51..a474c59c907c 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -548,171 +548,45 @@ static u16 mana_select_queue(struct net_device *ndev, struct sk_buff *skb,
 	return txq;
 }
 
-/* Release pre-allocated RX buffers */
-void mana_pre_dealloc_rxbufs(struct mana_port_context *mpc)
-{
-	struct device *dev;
-	int i;
-
-	dev = mpc->ac->gdma_dev->gdma_context->dev;
-
-	if (!mpc->rxbufs_pre)
-		goto out1;
-
-	if (!mpc->das_pre)
-		goto out2;
-
-	while (mpc->rxbpre_total) {
-		i = --mpc->rxbpre_total;
-		dma_unmap_single(dev, mpc->das_pre[i], mpc->rxbpre_datasize,
-				 DMA_FROM_DEVICE);
-		put_page(virt_to_head_page(mpc->rxbufs_pre[i]));
-	}
-
-	kfree(mpc->das_pre);
-	mpc->das_pre = NULL;
-
-out2:
-	kfree(mpc->rxbufs_pre);
-	mpc->rxbufs_pre = NULL;
-
-out1:
-	mpc->rxbpre_datasize = 0;
-	mpc->rxbpre_alloc_size = 0;
-	mpc->rxbpre_headroom = 0;
-}
-
-/* Get a buffer from the pre-allocated RX buffers */
-static void *mana_get_rxbuf_pre(struct mana_rxq *rxq, dma_addr_t *da)
-{
-	struct net_device *ndev = rxq->ndev;
-	struct mana_port_context *mpc;
-	void *va;
-
-	mpc = netdev_priv(ndev);
-
-	if (!mpc->rxbufs_pre || !mpc->das_pre || !mpc->rxbpre_total) {
-		netdev_err(ndev, "No RX pre-allocated bufs\n");
-		return NULL;
-	}
-
-	/* Check sizes to catch unexpected coding error */
-	if (mpc->rxbpre_datasize != rxq->datasize) {
-		netdev_err(ndev, "rxbpre_datasize mismatch: %u: %u\n",
-			   mpc->rxbpre_datasize, rxq->datasize);
-		return NULL;
-	}
-
-	if (mpc->rxbpre_alloc_size != rxq->alloc_size) {
-		netdev_err(ndev, "rxbpre_alloc_size mismatch: %u: %u\n",
-			   mpc->rxbpre_alloc_size, rxq->alloc_size);
-		return NULL;
-	}
-
-	if (mpc->rxbpre_headroom != rxq->headroom) {
-		netdev_err(ndev, "rxbpre_headroom mismatch: %u: %u\n",
-			   mpc->rxbpre_headroom, rxq->headroom);
-		return NULL;
-	}
-
-	mpc->rxbpre_total--;
-
-	*da = mpc->das_pre[mpc->rxbpre_total];
-	va = mpc->rxbufs_pre[mpc->rxbpre_total];
-	mpc->rxbufs_pre[mpc->rxbpre_total] = NULL;
-
-	/* Deallocate the array after all buffers are gone */
-	if (!mpc->rxbpre_total)
-		mana_pre_dealloc_rxbufs(mpc);
-
-	return va;
-}
-
 /* Get RX buffer's data size, alloc size, XDP headroom based on MTU */
-static void mana_get_rxbuf_cfg(int mtu, u32 *datasize, u32 *alloc_size,
-			       u32 *headroom)
+static void mana_get_rxbuf_cfg(struct mana_port_context *apc,
+			       int mtu, u32 *datasize, u32 *alloc_size,
+			       u32 *headroom, u32 *frag_count)
 {
-	if (mtu > MANA_XDP_MTU_MAX)
-		*headroom = 0; /* no support for XDP */
-	else
-		*headroom = XDP_PACKET_HEADROOM;
-
-	*alloc_size = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD + *headroom);
-
-	/* Using page pool in this case, so alloc_size is PAGE_SIZE */
-	if (*alloc_size < PAGE_SIZE)
-		*alloc_size = PAGE_SIZE;
-
+	/* Calculate datasize first (consistent across all cases) */
 	*datasize = mtu + ETH_HLEN;
-}
-
-int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu, int num_queues)
-{
-	struct device *dev;
-	struct page *page;
-	dma_addr_t da;
-	int num_rxb;
-	void *va;
-	int i;
-
-	mana_get_rxbuf_cfg(new_mtu, &mpc->rxbpre_datasize,
-			   &mpc->rxbpre_alloc_size, &mpc->rxbpre_headroom);
-
-	dev = mpc->ac->gdma_dev->gdma_context->dev;
-
-	num_rxb = num_queues * mpc->rx_queue_size;
-
-	WARN(mpc->rxbufs_pre, "mana rxbufs_pre exists\n");
-	mpc->rxbufs_pre = kmalloc_array(num_rxb, sizeof(void *), GFP_KERNEL);
-	if (!mpc->rxbufs_pre)
-		goto error;
 
-	mpc->das_pre = kmalloc_array(num_rxb, sizeof(dma_addr_t), GFP_KERNEL);
-	if (!mpc->das_pre)
-		goto error;
-
-	mpc->rxbpre_total = 0;
-
-	for (i = 0; i < num_rxb; i++) {
-		page = dev_alloc_pages(get_order(mpc->rxbpre_alloc_size));
-		if (!page)
-			goto error;
-
-		va = page_to_virt(page);
-
-		da = dma_map_single(dev, va + mpc->rxbpre_headroom,
-				    mpc->rxbpre_datasize, DMA_FROM_DEVICE);
-		if (dma_mapping_error(dev, da)) {
-			put_page(page);
-			goto error;
+	/* For xdp and jumbo frames make sure only one packet fits per page */
+	if (((mtu + MANA_RXBUF_PAD) > PAGE_SIZE / 2) || rcu_access_pointer(apc->bpf_prog)) {
+		if (rcu_access_pointer(apc->bpf_prog)) {
+			*headroom = XDP_PACKET_HEADROOM;
+			*alloc_size = PAGE_SIZE;
+		} else {
+			*headroom = 0; /* no support for XDP */
+			*alloc_size = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD + *headroom);
 		}
 
-		mpc->rxbufs_pre[i] = va;
-		mpc->das_pre[i] = da;
-		mpc->rxbpre_total = i + 1;
+		*frag_count = 1;
+		return;
 	}
 
-	return 0;
+	/* Standard MTU case - optimize for multiple packets per page */
+	*headroom = 0;
 
-error:
-	netdev_err(mpc->ndev, "Failed to pre-allocate RX buffers for %d queues\n", num_queues);
-	mana_pre_dealloc_rxbufs(mpc);
-	return -ENOMEM;
+	/* Calculate base buffer size needed */
+	u32 len = SKB_DATA_ALIGN(mtu + MANA_RXBUF_PAD + *headroom);
+	u32 buf_size = ALIGN(len, MANA_RX_FRAG_ALIGNMENT);
+
+	/* Calculate how many packets can fit in a page */
+	*frag_count = PAGE_SIZE / buf_size;
+	*alloc_size = buf_size;
 }
 
 static int mana_change_mtu(struct net_device *ndev, int new_mtu)
 {
-	struct mana_port_context *mpc = netdev_priv(ndev);
 	unsigned int old_mtu = ndev->mtu;
 	int err;
 
-	/* Pre-allocate buffers to prevent failure in mana_attach later */
-	err = mana_pre_alloc_rxbufs(mpc, new_mtu, mpc->num_queues);
-	if (err) {
-		netdev_err(ndev, "Insufficient memory for new MTU\n");
-		return err;
-	}
-
 	err = mana_detach(ndev, false);
 	if (err) {
 		netdev_err(ndev, "mana_detach failed: %d\n", err);
@@ -728,7 +602,6 @@ static int mana_change_mtu(struct net_device *ndev, int new_mtu)
 	}
 
 out:
-	mana_pre_dealloc_rxbufs(mpc);
 	return err;
 }
 
@@ -1841,8 +1714,11 @@ static void mana_rx_skb(void *buf_va, bool from_pool,
 
 drop:
 	if (from_pool) {
-		page_pool_recycle_direct(rxq->page_pool,
-					 virt_to_head_page(buf_va));
+		if (rxq->frag_count == 1)
+			page_pool_recycle_direct(rxq->page_pool,
+						 virt_to_head_page(buf_va));
+		else
+			page_pool_free_va(rxq->page_pool, buf_va, true);
 	} else {
 		WARN_ON_ONCE(rxq->xdp_save_va);
 		/* Save for reuse */
@@ -1854,37 +1730,50 @@ static void mana_rx_skb(void *buf_va, bool from_pool,
 	return;
 }
 
-static void *mana_get_rxfrag(struct mana_rxq *rxq, struct device *dev,
-			     dma_addr_t *da, bool *from_pool)
+static void *mana_get_rxfrag(struct mana_rxq *rxq,
+			     struct device *dev, dma_addr_t *da, bool *from_pool)
 {
 	struct page *page;
+	u32 offset;
 	void *va;
-
 	*from_pool = false;
 
-	/* Reuse XDP dropped page if available */
-	if (rxq->xdp_save_va) {
-		va = rxq->xdp_save_va;
-		rxq->xdp_save_va = NULL;
-	} else {
-		page = page_pool_dev_alloc_pages(rxq->page_pool);
-		if (!page)
+	/* Don't use fragments for jumbo frames or XDP (i.e when fragment = 1 per page) */
+	if (rxq->frag_count == 1) {
+		/* Reuse XDP dropped page if available */
+		if (rxq->xdp_save_va) {
+			va = rxq->xdp_save_va;
+			rxq->xdp_save_va = NULL;
+		} else {
+			page = page_pool_dev_alloc_pages(rxq->page_pool);
+			if (!page)
+				return NULL;
+
+			*from_pool = true;
+			va = page_to_virt(page);
+		}
+
+		*da = dma_map_single(dev, va + rxq->headroom, rxq->datasize,
+				     DMA_FROM_DEVICE);
+		if (dma_mapping_error(dev, *da)) {
+			if (*from_pool)
+				page_pool_put_full_page(rxq->page_pool, page, false);
+			else
+				put_page(virt_to_head_page(va));
+
 			return NULL;
+		}
 
-		*from_pool = true;
-		va = page_to_virt(page);
+		return va;
 	}
 
-	*da = dma_map_single(dev, va + rxq->headroom, rxq->datasize,
-			     DMA_FROM_DEVICE);
-	if (dma_mapping_error(dev, *da)) {
-		if (*from_pool)
-			page_pool_put_full_page(rxq->page_pool, page, false);
-		else
-			put_page(virt_to_head_page(va));
-
+	page =  page_pool_dev_alloc_frag(rxq->page_pool, &offset, rxq->alloc_size);
+	if (!page)
 		return NULL;
-	}
+
+	va  = page_to_virt(page) + offset;
+	*da = page_pool_get_dma_addr(page) + offset + rxq->headroom;
+	*from_pool = true;
 
 	return va;
 }
@@ -1901,9 +1790,9 @@ static void mana_refill_rx_oob(struct device *dev, struct mana_rxq *rxq,
 	va = mana_get_rxfrag(rxq, dev, &da, &from_pool);
 	if (!va)
 		return;
-
-	dma_unmap_single(dev, rxoob->sgl[0].address, rxq->datasize,
-			 DMA_FROM_DEVICE);
+	if (!rxoob->from_pool || rxq->frag_count == 1)
+		dma_unmap_single(dev, rxoob->sgl[0].address, rxq->datasize,
+				 DMA_FROM_DEVICE);
 	*old_buf = rxoob->buf_va;
 	*old_fp = rxoob->from_pool;
 
@@ -2314,15 +2203,19 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
 		if (!rx_oob->buf_va)
 			continue;
 
-		dma_unmap_single(dev, rx_oob->sgl[0].address,
-				 rx_oob->sgl[0].size, DMA_FROM_DEVICE);
-
 		page = virt_to_head_page(rx_oob->buf_va);
 
-		if (rx_oob->from_pool)
-			page_pool_put_full_page(rxq->page_pool, page, false);
-		else
-			put_page(page);
+		if (rxq->frag_count == 1) {
+			dma_unmap_single(dev, rx_oob->sgl[0].address, rx_oob->sgl[0].size,
+					 DMA_FROM_DEVICE);
+
+			if (rx_oob->from_pool)
+				page_pool_put_full_page(rxq->page_pool, page, false);
+			else
+				put_page(page);
+		} else {
+			page_pool_free_va(rxq->page_pool, rx_oob->buf_va, true);
+		}
 
 		rx_oob->buf_va = NULL;
 	}
@@ -2338,16 +2231,11 @@ static void mana_destroy_rxq(struct mana_port_context *apc,
 static int mana_fill_rx_oob(struct mana_recv_buf_oob *rx_oob, u32 mem_key,
 			    struct mana_rxq *rxq, struct device *dev)
 {
-	struct mana_port_context *mpc = netdev_priv(rxq->ndev);
 	bool from_pool = false;
 	dma_addr_t da;
 	void *va;
 
-	if (mpc->rxbufs_pre)
-		va = mana_get_rxbuf_pre(rxq, &da);
-	else
-		va = mana_get_rxfrag(rxq, dev, &da, &from_pool);
-
+	va = mana_get_rxfrag(rxq, dev, &da, &from_pool);
 	if (!va)
 		return -ENOMEM;
 
@@ -2428,11 +2316,22 @@ static int mana_create_page_pool(struct mana_rxq *rxq, struct gdma_context *gc)
 	struct page_pool_params pprm = {};
 	int ret;
 
-	pprm.pool_size = mpc->rx_queue_size;
+	pprm.pool_size = mpc->rx_queue_size / rxq->frag_count + 1;
 	pprm.nid = gc->numa_node;
 	pprm.napi = &rxq->rx_cq.napi;
 	pprm.netdev = rxq->ndev;
 	pprm.order = get_order(rxq->alloc_size);
+	pprm.queue_idx = rxq->rxq_idx;
+	pprm.dev = gc->dev;
+
+	/* Let the page pool do the dma map when page sharing with multiple fragments
+	 * enabled for rx buffers.
+	 */
+	if (rxq->frag_count > 1) {
+		pprm.flags =  PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV;
+		pprm.max_len = PAGE_SIZE;
+		pprm.dma_dir = DMA_FROM_DEVICE;
+	}
 
 	rxq->page_pool = page_pool_create(&pprm);
 
@@ -2471,9 +2370,8 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
 	rxq->rxq_idx = rxq_idx;
 	rxq->rxobj = INVALID_MANA_HANDLE;
 
-	mana_get_rxbuf_cfg(ndev->mtu, &rxq->datasize, &rxq->alloc_size,
-			   &rxq->headroom);
-
+	mana_get_rxbuf_cfg(apc, ndev->mtu, &rxq->datasize, &rxq->alloc_size,
+			   &rxq->headroom, &rxq->frag_count);
 	/* Create page pool for RX queue */
 	err = mana_create_page_pool(rxq, gc);
 	if (err) {
diff --git a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
index a1afa75a9463..7ede03c74fb9 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_ethtool.c
@@ -396,12 +396,6 @@ static int mana_set_channels(struct net_device *ndev,
 	unsigned int old_count = apc->num_queues;
 	int err;
 
-	err = mana_pre_alloc_rxbufs(apc, ndev->mtu, new_count);
-	if (err) {
-		netdev_err(ndev, "Insufficient memory for new allocations");
-		return err;
-	}
-
 	err = mana_detach(ndev, false);
 	if (err) {
 		netdev_err(ndev, "mana_detach failed: %d\n", err);
@@ -416,7 +410,6 @@ static int mana_set_channels(struct net_device *ndev,
 	}
 
 out:
-	mana_pre_dealloc_rxbufs(apc);
 	return err;
 }
 
@@ -465,12 +458,7 @@ static int mana_set_ringparam(struct net_device *ndev,
 
 	/* pre-allocating new buffers to prevent failures in mana_attach() later */
 	apc->rx_queue_size = new_rx;
-	err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues);
 	apc->rx_queue_size = old_rx;
-	if (err) {
-		netdev_err(ndev, "Insufficient memory for new allocations\n");
-		return err;
-	}
 
 	err = mana_detach(ndev, false);
 	if (err) {
@@ -488,7 +476,6 @@ static int mana_set_ringparam(struct net_device *ndev,
 		apc->rx_queue_size = old_rx;
 	}
 out:
-	mana_pre_dealloc_rxbufs(apc);
 	return err;
 }
 
diff --git a/include/net/mana/mana.h b/include/net/mana/mana.h
index e1030a7d2daa..99a3847b0f9d 100644
--- a/include/net/mana/mana.h
+++ b/include/net/mana/mana.h
@@ -65,6 +65,8 @@ enum TRI_STATE {
 #define MANA_STATS_RX_COUNT 5
 #define MANA_STATS_TX_COUNT 11
 
+#define MANA_RX_FRAG_ALIGNMENT 64
+
 struct mana_stats_rx {
 	u64 packets;
 	u64 bytes;
@@ -328,6 +330,7 @@ struct mana_rxq {
 	u32 datasize;
 	u32 alloc_size;
 	u32 headroom;
+	u32 frag_count;
 
 	mana_handle_t rxobj;
 
@@ -503,14 +506,6 @@ struct mana_port_context {
 	/* This points to an array of num_queues of RQ pointers. */
 	struct mana_rxq **rxqs;
 
-	/* pre-allocated rx buffer array */
-	void **rxbufs_pre;
-	dma_addr_t *das_pre;
-	int rxbpre_total;
-	u32 rxbpre_datasize;
-	u32 rxbpre_alloc_size;
-	u32 rxbpre_headroom;
-
 	struct bpf_prog *bpf_prog;
 
 	/* Create num_queues EQs, SQs, SQ-CQs, RQs and RQ-CQs, respectively. */
@@ -574,8 +569,6 @@ int mana_query_link_cfg(struct mana_port_context *apc);
 int mana_set_bw_clamp(struct mana_port_context *apc, u32 speed,
 		      int enable_clamping);
 void mana_query_phy_stats(struct mana_port_context *apc);
-int mana_pre_alloc_rxbufs(struct mana_port_context *apc, int mtu, int num_queues);
-void mana_pre_dealloc_rxbufs(struct mana_port_context *apc);
 
 extern const struct ethtool_ops mana_ethtool_ops;
 extern struct dentry *mana_debugfs_root;
-- 
2.43.0


^ permalink raw reply related

* RE: [PATCH v4 0/7] hyperv: Introduce new way to manage hypercall args
From: Michael Kelley @ 2025-07-21  2:19 UTC (permalink / raw)
  To: Roman Kisel, Easwar Hariharan, Nuno Das Neves, Naman Jain
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
	lpieralisi@kernel.org, kw@linux.com, mani@kernel.org,
	robh@kernel.org, bhelgaas@google.com, arnd@arndb.de,
	x86@kernel.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arch@vger.kernel.org
In-Reply-To: <ed1e8508-7085-4620-af25-3a8795c1afe8@linux.microsoft.com>

From: Roman Kisel <romank@linux.microsoft.com> Sent: Friday, July 18, 2025 6:16 PM
> 
> On 7/18/2025 1:25 PM, Easwar Hariharan wrote:
> > On 7/18/2025 10:13 AM, Michael Kelley wrote:
> >> From: Easwar Hariharan <eahariha@linux.microsoft.com> Sent: Friday, July 18, 2025
> 9:33 AM
> >>>
> >>> On 7/17/2025 9:55 PM, mhkelley58@gmail.com wrote:
> >>>> From: Michael Kelley <mhklinux@outlook.com>
> >>>>
> >>
> >> [snip]
> >>
> >>>>
> >>>> The new code compiles and runs successfully on x86 and arm64. However,
> >>>> basic smoke tests cover only a limited number of hypercall call sites
> >>>> that have been modified. I don't have the hardware or Hyper-V
> >>>> configurations needed to test running in the Hyper-V root partition
> >>>> or running in a VTL other than VTL 0. The related hypercall call sites
> >>>> still need to be tested to make sure I didn't break anything. Hopefully
> >>>> someone with the necessary configurations and Hyper-V versions can
> >>>> help with that testing.
> >>
> >> Easwar --
> >>
> >> Thanks for reviewing.
> >>
> >> Any chance you (or someone else) could do a quick smoke test of this
> >> patch set when running in the Hyper-V root partition, and separately,
> >> when running in VTL2?  Some hypercall call sites are modified that
> >> don't get called in normal VTL0 execution. It just needs a quick
> >> verification that nothing is obviously broken for the root partition and
> >> VTL2 cases.
> >>
> >> Michael
> >>
> >
> > I'm working almost entirely in VTL0, so I'd call on Nuno, Naman, and Roman (cc'ed) to
> help.
> >
> 
> Michael,
> 
> I'll try to squeeze that in during the next week. Folks should feel free
> to beat me to that :) The caveat would be that there are scenarios that
> are beyond the capabilities of the hardware that I have readily
> available, and would need to run in test clusters in Azure, and these
> are pretty busy.

Thanks for any testing you can do on standalone test machines without
needing test clusters in Azure. It will be hard to get test coverage on
*every* hypercall call site that is modified by the patch set, but doing
basic smoke testing of running in the root partition and in VTL2 will
cover more than I can cover running in a VTL0 guest on my laptop or
in Azure. Fortunately, the changes overall in this patch set are pretty
straightforward, and my testing of VTL0 guests didn’t turn up any bugs.
I'm hoping that additional smoke testing is more about gaining
confidence than finding actual bugs.  (Famous last words ....)

> VTL2 currently uses a limited number hypercalls that are set as enabled
> in the OpenVMM code (`set_allowed_hypercalls`). You could take a look
> and conclude if these hypercalls require any adjustments in the patches.

My patch set already covers all the hypercall call sites that originate in
VTL2 code. Again, a basic smoke test should help gain confidence, or
show that any confidence is misplaced :-)

> My opinion has been to have two pages (input and output ones). As the
> new code introduces just one page I do feel a bit apprehensive, got no
> hard evidence that this is a bad approach though. If we tweak the code
> to have 2 pages, perhaps there would be no need to run a full-blown
> validation, and even smoke tests will suffice? 

My view is that the 1 page vs. 2 pages is much less of a risk than just
some coding error in introducing the new interfaces. The 1 page vs.
2 pages should only affect the batch size for rep hypercalls, and the
existing code already handles different batch sizes. So I'm not as
concerned about that risk. Wei Liu in the maintainer here, so I'll
certainly follow his judgment and guidance on what is needed to
be confident in this patch set.

Michael

^ permalink raw reply

* Re: [GIT PULL] Hyper-V fixes for 6.16-rc7
From: pr-tracker-bot @ 2025-07-20 16:35 UTC (permalink / raw)
  To: Wei Liu
  Cc: Linus Torvalds, Wei Liu, Linux on Hyper-V List, Linux Kernel List,
	kys, haiyangz, decui
In-Reply-To: <aHyJD7tQPytwl0nS@liuwe-devbox-ubuntu-v2.tail21d00.ts.net>

The pull request you sent on Sun, 20 Jul 2025 06:13:35 +0000:

> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed-20250718

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/5f054ef2e0f1ca7d32ac48e275d08e2ac29d84f3

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html

^ permalink raw reply

* [GIT PULL] Hyper-V fixes for 6.16-rc7
From: Wei Liu @ 2025-07-20  6:13 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Wei Liu, Linux on Hyper-V List, Linux Kernel List, kys, haiyangz,
	decui

Hi Linus,

The following changes since commit d7b8f8e20813f0179d8ef519541a3527e7661d3a:

  Linux 6.16-rc5 (2025-07-06 14:10:26 -0700)

are available in the Git repository at:

  ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/hyperv/linux.git tags/hyperv-fixes-signed-20250718

for you to fetch changes up to a4131a50d072b369bfed0b41e741c41fd8048641:

  tools/hv: fcopy: Fix irregularities with size of ring buffer (2025-07-15 06:25:33 +0000)

----------------------------------------------------------------
hyperv-fixes for v6.16-rc7
 - Select use CONFIG_SYSFB only if EFI is enabled (Michael Kelley)
 - An assorted set of fixes to remove warnings for missing
   export.h header inclusion (Naman Jain)
 - An assorted set of fixes for when Linux run as the root partition for
   Microsoft Hypervisor (Mukesh Rathor, Nuno Das Neves, Stanislav
   Kinsburskii)
 - Fix the check for HYPERVISOR_CALLBACK_VECTOR (Naman Jain)
 - Fix fcopy tool to handle irregularities with size of ring buffer
   (Naman Jain)
 - Fix incorrect file path conversion in fcopy tool (Yasumasa Suenaga)
----------------------------------------------------------------
Michael Kelley (1):
      Drivers: hv: Select CONFIG_SYSFB only if EFI is enabled

Mukesh Rathor (1):
      PCI: hv: Don't load the driver for baremetal root partition

Naman Jain (7):
      Drivers: hv: Fix the check for HYPERVISOR_CALLBACK_VECTOR
      Drivers: hv: Fix warnings for missing export.h header inclusion
      x86/hyperv: Fix warnings for missing export.h header inclusion
      clocksource: hyper-v: Fix warnings for missing export.h header inclusion
      PCI: hv: Fix warnings for missing export.h header inclusion
      net: mana: Fix warnings for missing export.h header inclusion
      tools/hv: fcopy: Fix irregularities with size of ring buffer

Nuno Das Neves (3):
      x86/hyperv: Fix usage of cpu_online_mask to get valid cpu
      x86/hyperv: Clean up hv_map/unmap_interrupt() return values
      Drivers: hv: Use nested hypercall for post message and signal event

Stanislav Kinsburskii (2):
      x86/hyperv: Expose hv_map_msi_interrupt()
      PCI: hv: Use the correct hypercall for unmasking interrupts on nested

Yasumasa Suenaga (1):
      tools/hv: fcopy: Fix incorrect file path conversion

 arch/x86/hyperv/hv_init.c                       |   1 +
 arch/x86/hyperv/irqdomain.c                     |  69 +++++++------
 arch/x86/hyperv/ivm.c                           |   1 +
 arch/x86/hyperv/nested.c                        |   1 +
 arch/x86/include/asm/mshyperv.h                 |  22 +---
 drivers/clocksource/hyperv_timer.c              |   1 +
 drivers/hv/Kconfig                              |   2 +-
 drivers/hv/channel.c                            |   1 +
 drivers/hv/channel_mgmt.c                       |   1 +
 drivers/hv/connection.c                         |   5 +-
 drivers/hv/hv.c                                 |   6 +-
 drivers/hv/hv_proc.c                            |   1 +
 drivers/hv/mshv_common.c                        |   1 +
 drivers/hv/mshv_root_hv_call.c                  |   1 +
 drivers/hv/ring_buffer.c                        |   1 +
 drivers/hv/vmbus_drv.c                          |   9 +-
 drivers/iommu/hyperv-iommu.c                    |  33 +++---
 drivers/net/ethernet/microsoft/mana/gdma_main.c |   1 +
 drivers/net/ethernet/microsoft/mana/mana_en.c   |   1 +
 drivers/pci/controller/pci-hyperv-intf.c        |   1 +
 drivers/pci/controller/pci-hyperv.c             |  21 +++-
 tools/hv/hv_fcopy_uio_daemon.c                  | 128 ++++++++++++++++++------
 22 files changed, 196 insertions(+), 112 deletions(-)

^ permalink raw reply

* Re: [PATCH net] net: core: Fix the loop in default_device_exit_net()
From: Kuniyuki Iwashima @ 2025-07-19 20:47 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Haiyang Zhang, linux-hyperv, netdev, haiyangz, kys, wei.liu,
	edumazet, pabeni, horms, davem, sdf, ahmed.zaki,
	aleksander.lobakin, linux-kernel
In-Reply-To: <20250718163723.4390bd7d@kernel.org>

On Fri, Jul 18, 2025 at 4:37 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 18 Jul 2025 13:20:14 -0700 Haiyang Zhang wrote:
> > The loop in default_device_exit_net() won't be able to properly detect the
> > head then stop, and will hit NULL pointer, when a driver, like hv_netvsc,
> > automatically moves the slave device together with the master device.
> >
> > To fix this, add a helper function to return the first migratable netdev
> > correctly, no matter one or two devices were removed from this net's list
> > in the last iteration.
>
> FTR I think that what the driver is trying to do is way too hacky, and
> it should be fixed instead. But I defer to Kuniyuki for the final word,
> maybe this change is useful for other reasons..

I agree that it should be fixed on the driver side.  I don't
think of a good reason for the change.

^ permalink raw reply

* Re: [PATCH v5 2/2] Drivers: hv: Introduce mshv_vtl driver
From: Markus Elfring @ 2025-07-19 18:30 UTC (permalink / raw)
  To: Naman Jain, Roman Kisel, Saurabh Sengar, linux-hyperv, Dexuan Cui,
	Haiyang Zhang, K. Y. Srinivasan, Wei Liu
  Cc: LKML, Alok Tiwari, Anirudh Rayabharam, Nuno Das Neves,
	Stanislav Kinsburskii
In-Reply-To: <20250611072704.83199-3-namjain@linux.microsoft.com>

…
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -0,0 +1,1783 @@
> +static int mshv_vtl_sint_ioctl_set_eventfd(struct mshv_vtl_set_eventfd __user *arg)
> +{
> +	mutex_lock(&flag_lock);
> +	old_eventfd = flag_eventfds[set_eventfd.flag];
> +	WRITE_ONCE(flag_eventfds[set_eventfd.flag], eventfd);
> +	mutex_unlock(&flag_lock);
…

Under which circumstances would you become interested to apply a statement
like “guard(mutex)(&flag_lock);”?
https://elixir.bootlin.com/linux/v6.16-rc6/source/include/linux/mutex.h#L225

Regards,
Markus

^ permalink raw reply

* Re: [PATCH v4 0/7] hyperv: Introduce new way to manage hypercall args
From: Roman Kisel @ 2025-07-19  1:15 UTC (permalink / raw)
  To: Easwar Hariharan, Michael Kelley, Nuno Das Neves, Naman Jain
  Cc: kys@microsoft.com, haiyangz@microsoft.com, wei.liu@kernel.org,
	decui@microsoft.com, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, dave.hansen@linux.intel.com, hpa@zytor.com,
	lpieralisi@kernel.org, kw@linux.com, mani@kernel.org,
	robh@kernel.org, bhelgaas@google.com, arnd@arndb.de,
	x86@kernel.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arch@vger.kernel.org
In-Reply-To: <43f8be57-a330-455f-8f9e-f5718ff1aa1a@linux.microsoft.com>



On 7/18/2025 1:25 PM, Easwar Hariharan wrote:
> On 7/18/2025 10:13 AM, Michael Kelley wrote:
>> From: Easwar Hariharan <eahariha@linux.microsoft.com> Sent: Friday, July 18, 2025 9:33 AM
>>>
>>> On 7/17/2025 9:55 PM, mhkelley58@gmail.com wrote:
>>>> From: Michael Kelley <mhklinux@outlook.com>
>>>>
>>
>> [snip]
>>
>>>>
>>>> The new code compiles and runs successfully on x86 and arm64. However,
>>>> basic smoke tests cover only a limited number of hypercall call sites
>>>> that have been modified. I don't have the hardware or Hyper-V
>>>> configurations needed to test running in the Hyper-V root partition
>>>> or running in a VTL other than VTL 0. The related hypercall call sites
>>>> still need to be tested to make sure I didn't break anything. Hopefully
>>>> someone with the necessary configurations and Hyper-V versions can
>>>> help with that testing.
>>
>> Easwar --
>>
>> Thanks for reviewing.
>>
>> Any chance you (or someone else) could do a quick smoke test of this
>> patch set when running in the Hyper-V root partition, and separately,
>> when running in VTL2?  Some hypercall call sites are modified that
>> don't get called in normal VTL0 execution. It just needs a quick
>> verification that nothing is obviously broken for the root partition and
>> VTL2 cases.
>>
>> Michael
>>
> 
> I'm working almost entirely in VTL0, so I'd call on Nuno, Naman, and Roman (cc'ed) to help.
> 

Michael,

I'll try to squeeze that in during the next week. Folks should feel free
to beat me to that :) The caveat would be that there are scenarios that
are beyond the capabilities of the hardware that I have readily
available, and would need to run in test clusters in Azure, and these
are pretty busy.

VTL2 currently uses a limited number hypercalls that are set as enabled
in the OpenVMM code (`set_allowed_hypercalls`). You could take a look
and conclude if these hypercalls require any adjustments in the patches.

My opinion has been to have two pages (input and output ones). As the
new code introduces just one page I do feel a bit apprehensive, got no
hard evidence that this is a bad approach though. If we tweak the code
to have 2 pages, perhaps there would be no need to run a full-blown
validation, and even smoke tests will suffice?

> - Easwar

-- 
Thank you,
Roman


^ permalink raw reply

* Re: [PATCH net] net: core: Fix the loop in default_device_exit_net()
From: Jakub Kicinski @ 2025-07-18 23:37 UTC (permalink / raw)
  To: Haiyang Zhang, kuniyu
  Cc: linux-hyperv, netdev, haiyangz, kys, wei.liu, edumazet, pabeni,
	horms, davem, sdf, ahmed.zaki, aleksander.lobakin, linux-kernel
In-Reply-To: <1752870014-28909-1-git-send-email-haiyangz@linux.microsoft.com>

On Fri, 18 Jul 2025 13:20:14 -0700 Haiyang Zhang wrote:
> The loop in default_device_exit_net() won't be able to properly detect the
> head then stop, and will hit NULL pointer, when a driver, like hv_netvsc,
> automatically moves the slave device together with the master device.
> 
> To fix this, add a helper function to return the first migratable netdev
> correctly, no matter one or two devices were removed from this net's list
> in the last iteration.

FTR I think that what the driver is trying to do is way too hacky, and
it should be fixed instead. But I defer to Kuniyuki for the final word,
maybe this change is useful for other reasons..

^ permalink raw reply

* Re: [PATCH v4 0/7] hyperv: Introduce new way to manage hypercall args
From: Easwar Hariharan @ 2025-07-18 20:25 UTC (permalink / raw)
  To: Michael Kelley, Nuno Das Neves, Naman Jain, Roman Kisel
  Cc: eahariha, kys@microsoft.com, haiyangz@microsoft.com,
	wei.liu@kernel.org, decui@microsoft.com, tglx@linutronix.de,
	mingo@redhat.com, bp@alien8.de, dave.hansen@linux.intel.com,
	hpa@zytor.com, lpieralisi@kernel.org, kw@linux.com,
	mani@kernel.org, robh@kernel.org, bhelgaas@google.com,
	arnd@arndb.de, x86@kernel.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org,
	linux-arch@vger.kernel.org
In-Reply-To: <SN6PR02MB41570625E2F061C5E494C7F4D450A@SN6PR02MB4157.namprd02.prod.outlook.com>

On 7/18/2025 10:13 AM, Michael Kelley wrote:
> From: Easwar Hariharan <eahariha@linux.microsoft.com> Sent: Friday, July 18, 2025 9:33 AM
>>
>> On 7/17/2025 9:55 PM, mhkelley58@gmail.com wrote:
>>> From: Michael Kelley <mhklinux@outlook.com>
>>>
> 
> [snip]
> 
>>>
>>> The new code compiles and runs successfully on x86 and arm64. However,
>>> basic smoke tests cover only a limited number of hypercall call sites
>>> that have been modified. I don't have the hardware or Hyper-V
>>> configurations needed to test running in the Hyper-V root partition
>>> or running in a VTL other than VTL 0. The related hypercall call sites
>>> still need to be tested to make sure I didn't break anything. Hopefully
>>> someone with the necessary configurations and Hyper-V versions can
>>> help with that testing.
> 
> Easwar -- 
> 
> Thanks for reviewing.
> 
> Any chance you (or someone else) could do a quick smoke test of this
> patch set when running in the Hyper-V root partition, and separately,
> when running in VTL2?  Some hypercall call sites are modified that
> don't get called in normal VTL0 execution. It just needs a quick
> verification that nothing is obviously broken for the root partition and
> VTL2 cases.
> 
> Michael
> 

I'm working almost entirely in VTL0, so I'd call on Nuno, Naman, and Roman (cc'ed) to help.

- Easwar

^ permalink raw reply

* [PATCH net] net: core: Fix the loop in default_device_exit_net()
From: Haiyang Zhang @ 2025-07-18 20:20 UTC (permalink / raw)
  To: linux-hyperv, netdev
  Cc: haiyangz, kys, wei.liu, edumazet, kuba, pabeni, horms, davem, sdf,
	kuniyu, ahmed.zaki, aleksander.lobakin, linux-kernel, stable, #,
	5.4+

From: Haiyang Zhang <haiyangz@microsoft.com>

The loop in default_device_exit_net() won't be able to properly detect the
head then stop, and will hit NULL pointer, when a driver, like hv_netvsc,
automatically moves the slave device together with the master device.

To fix this, add a helper function to return the first migratable netdev
correctly, no matter one or two devices were removed from this net's list
in the last iteration.

Cc: stable@vger.kernel.org # 5.4+
Signed-off-by: Haiyang Zhang <haiyangz@microsoft.com>
---
 net/core/dev.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 621a639aeba1..d83f5f12cf70 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -12629,19 +12629,11 @@ static struct pernet_operations __net_initdata netdev_net_ops = {
 	.exit = netdev_exit,
 };
 
-static void __net_exit default_device_exit_net(struct net *net)
+static inline struct net_device *first_migratable_netdev(struct net *net)
 {
-	struct netdev_name_node *name_node, *tmp;
 	struct net_device *dev, *aux;
-	/*
-	 * Push all migratable network devices back to the
-	 * initial network namespace
-	 */
-	ASSERT_RTNL();
-	for_each_netdev_safe(net, dev, aux) {
-		int err;
-		char fb_name[IFNAMSIZ];
 
+	for_each_netdev_safe(net, dev, aux) {
 		/* Ignore unmoveable devices (i.e. loopback) */
 		if (dev->netns_immutable)
 			continue;
@@ -12650,6 +12642,25 @@ static void __net_exit default_device_exit_net(struct net *net)
 		if (dev->rtnl_link_ops && !dev->rtnl_link_ops->netns_refund)
 			continue;
 
+		return dev;
+	}
+
+	return NULL;
+}
+
+static void __net_exit default_device_exit_net(struct net *net)
+{
+	struct netdev_name_node *name_node, *tmp;
+	struct net_device *dev;
+	/*
+	 * Push all migratable network devices back to the
+	 * initial network namespace
+	 */
+	ASSERT_RTNL();
+	while ((dev = first_migratable_netdev(net)) != NULL) {
+		int err;
+		char fb_name[IFNAMSIZ];
+
 		/* Push remaining network devices to init_net */
 		snprintf(fb_name, IFNAMSIZ, "dev%d", dev->ifindex);
 		if (netdev_name_in_use(&init_net, fb_name))
-- 
2.34.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox