Messages in this thread | ![/](/images/icornerl.gif) | | From | Haiyang Zhang <> | Subject | RE: [PATCH V6 5/5] net: netvsc: Add Isolation VM support for netvsc driver | Date | Thu, 9 Dec 2021 20:40:02 +0000 |
| |
> -----Original Message----- > From: Michael Kelley (LINUX) <mikelley@microsoft.com> > Sent: Thursday, December 9, 2021 2:54 PM > To: Haiyang Zhang <haiyangz@microsoft.com>; Tianyu Lan <ltykernel@gmail.com>; KY > Srinivasan <kys@microsoft.com>; Stephen Hemminger <sthemmin@microsoft.com>; > wei.liu@kernel.org; Dexuan Cui <decui@microsoft.com>; tglx@linutronix.de; mingo@redhat.com; > bp@alien8.de; dave.hansen@linux.intel.com; x86@kernel.org; hpa@zytor.com; > davem@davemloft.net; kuba@kernel.org; jejb@linux.ibm.com; martin.petersen@oracle.com; > arnd@arndb.de; hch@infradead.org; m.szyprowski@samsung.com; robin.murphy@arm.com; Tianyu > Lan <Tianyu.Lan@microsoft.com>; thomas.lendacky@amd.com > Cc: iommu@lists.linux-foundation.org; linux-arch@vger.kernel.org; linux- > hyperv@vger.kernel.org; linux-kernel@vger.kernel.org; linux-scsi@vger.kernel.org; > netdev@vger.kernel.org; vkuznets <vkuznets@redhat.com>; brijesh.singh@amd.com; > konrad.wilk@oracle.com; hch@lst.de; joro@8bytes.org; parri.andrea@gmail.com; > dave.hansen@intel.com > Subject: RE: [PATCH V6 5/5] net: netvsc: Add Isolation VM support for netvsc driver > > From: Haiyang Zhang <haiyangz@microsoft.com> Sent: Wednesday, December 8, 2021 12:14 PM > > > From: Tianyu Lan <ltykernel@gmail.com> > > > Sent: Tuesday, December 7, 2021 2:56 AM > > [snip] > > > > static inline int netvsc_send_pkt( > > > struct hv_device *device, > > > struct hv_netvsc_packet *packet, > > > @@ -986,14 +1105,24 @@ static inline int netvsc_send_pkt( > > > > > > trace_nvsp_send_pkt(ndev, out_channel, rpkt); > > > > > > + packet->dma_range = NULL; > > > if (packet->page_buf_cnt) { > > > if (packet->cp_partial) > > > pb += packet->rmsg_pgcnt; > > > > > > + ret = netvsc_dma_map(ndev_ctx->device_ctx, packet, pb); > > > + if (ret) { > > > + ret = -EAGAIN; > > > + goto exit; > > > + } > > > > Returning EAGAIN will let the upper network layer busy retry, > > which may make things worse. > > I suggest to return ENOSPC here like another place in this > > function, which will just drop the packet, and let the network > > protocol/app layer decide how to recover. > > > > Thanks, > > - Haiyang > > I made the original suggestion to return -EAGAIN here. A > DMA mapping failure should occur only if swiotlb bounce > buffer space is unavailable, which is a transient condition. > The existing code already stops the queue and returns > -EAGAIN when the ring buffer is full, which is also a transient > condition. My sense is that the two conditions should be > handled the same way. Or is there a reason why a ring > buffer full condition should stop the queue and retry, while > a mapping failure should drop the packet?
The netvsc_dma_map() fails in these two places. The dma_map_single() is doing the maping with swiotlb bounce buffer, correct? And it will become successful after the previous packets are unmapped?
+ packet->dma_range = kcalloc(page_count, + sizeof(*packet->dma_range), + GFP_KERNEL);
+ dma = dma_map_single(&hv_dev->device, src, len, + DMA_TO_DEVICE);
I recalled your previous suggestion now, and agree with you that we can treat it the same way (return -EAGAIN) in this case. And the existing code will stop the queue temporarily.
Thanks, - Haiyang
| ![\](/images/icornerr.gif) |