On 16-08-27 10:55 PM, Or Gerlitz wrote: > On Sat, Aug 27, 2016 at 10:11 AM, John Fastabend > <john.fastab...@gmail.com> wrote: >> From: Alexei Starovoitov <a...@fb.com> > >> This patch adds initial support for XDP on e1000 driver. Note e1000 >> driver does not support page recycling in general which could be >> added as a further improvement. However for XDP_DROP and XDP_XMIT >> the xdp code paths will recycle pages. > >> @@ -4188,15 +4305,57 @@ static bool e1000_clean_jumbo_rx_irq(struct >> e1000_adapter *adapter, >> prefetch(next_rxd); >> >> next_buffer = &rx_ring->buffer_info[i]; >> - > > nit, better to avoid random cleanups in a patch adding new (&& cool) > functionality >
Yep thanks. [...] >> + case XDP_TX: >> + dma_sync_single_for_device(&pdev->dev, >> + dma, >> + length, >> + DMA_TO_DEVICE); >> + e1000_xmit_raw_frame(buffer_info, length, >> + netdev, adapter); >> + /* Fallthrough to re-use mappedg page after xmit */ > > Did you want to say "mapped"? wasn't sure what's the role of "g" @ the end Yep but see below... > >> + case XDP_DROP: >> + default: >> + /* re-use mapped page. keep buffer_info->dma >> + * as-is, so that >> e1000_alloc_jumbo_rx_buffers >> + * only needs to put it back into rx ring >> + */ > > if we're on the XDP_TX pass, don't we need to actually see that frame > has been xmitted > before re using the page? > Agreed this seems to be too ambitious in the XDP_TX case. Thanks for the help. Unless Alexei has some reason why it works I'll go ahead and consume the buffer here. I think setting + bi->rxbuf.page = NULL; at the end of the XDP_TX case should fix it but I'll test it again, Thanks again I guess this is what I get for trying to push patches out on Friday night. >> + total_rx_bytes += length; >> + total_rx_packets++; >> + goto next_desc; >> + } >> + } >> + >> dma_unmap_page(&pdev->dev, buffer_info->dma, >> adapter->rx_buffer_len, DMA_FROM_DEVICE); >> buffer_info->dma = 0;
