Message ID | 20190808040312.21719-1-firo.yang@suse.com |
---|---|
State | Awaiting Upstream |
Delegated to: | David Miller |
Headers | show |
Series | [v3,1/1] ixgbe: sync the first fragment unconditionally | expand |
> -----Original Message----- > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On > Behalf Of Firo Yang > Sent: Wednesday, August 7, 2019 9:04 PM > To: netdev@vger.kernel.org > Cc: maciejromanfijalkowski@gmail.com; Firo Yang <firo.yang@suse.com>; > linux-kernel@vger.kernel.org; intel-wired-lan@lists.osuosl.org; > jian.w.wen@oracle.com; alexander.h.duyck@linux.intel.com; > davem@davemloft.net > Subject: [Intel-wired-lan] [PATCH v3 1/1] ixgbe: sync the first fragment > unconditionally > > In Xen environment, if Xen-swiotlb is enabled, ixgbe driver could possibly > allocate a page, DMA memory buffer, for the first fragment which is not > suitable for Xen-swiotlb to do DMA operations. > Xen-swiotlb have to internally allocate another page for doing DMA > operations. This mechanism requires syncing the data from the internal page > to the page which ixgbe sends to upper network stack. However, since > commit f3213d932173 ("ixgbe: Update driver to make use of DMA attributes > in Rx path"), the unmap operation is performed with > DMA_ATTR_SKIP_CPU_SYNC. As a result, the sync is not performed. > Since the sync isn't performed, the upper network stack could receive a > incomplete network packet. By incomplete, it means the linear data on the > first fragment(between skb->head and skb->end) is invalid. So we have to > copy the data from the internal xen-swiotlb page to the page which ixgbe > sends to upper network stack through the sync operation. > > More details from Alexander Duyck: > Specifically since we are mapping the frame with > DMA_ATTR_SKIP_CPU_SYNC we have to unmap with that as well. As a result > a sync is not performed on an unmap and must be done manually as we > skipped it for the first frag. As such we need to always sync before possibly > performing a page unmap operation. > > Fixes: f3213d932173 ("ixgbe: Update driver to make use of DMA attributes in > Rx path") > Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com> > Signed-off-by: Firo Yang <firo.yang@suse.com> > --- > Changes from v2: > * Added details on the problem caused by skipping the sync. > * Added more explanation from Alexander Duyck. > > Changes from v1: > * Imporved the patch description. > * Added Reviewed-by: and Fixes: as suggested by Alexander Duyck. > > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 16 +++++++++------- > 1 file changed, 9 insertions(+), 7 deletions(-) Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index cbaf712d6529..200de9838096 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -1825,13 +1825,7 @@ static void ixgbe_pull_tail(struct ixgbe_ring *rx_ring, static void ixgbe_dma_sync_frag(struct ixgbe_ring *rx_ring, struct sk_buff *skb) { - /* if the page was released unmap it, else just sync our portion */ - if (unlikely(IXGBE_CB(skb)->page_released)) { - dma_unmap_page_attrs(rx_ring->dev, IXGBE_CB(skb)->dma, - ixgbe_rx_pg_size(rx_ring), - DMA_FROM_DEVICE, - IXGBE_RX_DMA_ATTR); - } else if (ring_uses_build_skb(rx_ring)) { + if (ring_uses_build_skb(rx_ring)) { unsigned long offset = (unsigned long)(skb->data) & ~PAGE_MASK; dma_sync_single_range_for_cpu(rx_ring->dev, @@ -1848,6 +1842,14 @@ static void ixgbe_dma_sync_frag(struct ixgbe_ring *rx_ring, skb_frag_size(frag), DMA_FROM_DEVICE); } + + /* If the page was released, just unmap it. */ + if (unlikely(IXGBE_CB(skb)->page_released)) { + dma_unmap_page_attrs(rx_ring->dev, IXGBE_CB(skb)->dma, + ixgbe_rx_pg_size(rx_ring), + DMA_FROM_DEVICE, + IXGBE_RX_DMA_ATTR); + } } /**