Message ID | 152180749411.20167.16232784042872304317.stgit@firesoul |
---|---|
State | Changes Requested, archived |
Delegated to: | BPF Maintainers |
Headers | show |
Series | XDP redirect memory return API | expand |
On Fri, Mar 23, 2018 at 5:18 AM, Jesper Dangaard Brouer <brouer@redhat.com> wrote: > Extend struct ixgbe_tx_buffer to store the xdp_mem_info. > > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> > --- > drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 + > drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++++-- > 2 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h > index c1e3a0039ea5..cbc20f199364 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h > @@ -249,6 +249,7 @@ struct ixgbe_tx_buffer { > DEFINE_DMA_UNMAP_ADDR(dma); > DEFINE_DMA_UNMAP_LEN(len); > u32 tx_flags; > + struct xdp_mem_info xdp_mem; Instead of increasing the size of this structure it might make more sense to find free space somewhere in side of it. One thing you could probably look at doing is making it so that the gso_segs, protocol, and tx_flags fields could be put into an anonymous structure that is then part of a union with the xdp_mem_info structure. Then you would just have to tweak things slightly so that the else section for the ring_is_xdp block pulls the code just above it into that section so that those fields are not read. You would need to flip the dma, length, and type field ordering but that shouldn't be much of an issue and it should have no effect on performance since they are all in the same 16 byte aligned block anyway. > }; > > struct ixgbe_rx_buffer { > diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > index 85369423452d..45520eb503ee 100644 > --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c > @@ -1207,7 +1207,7 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector, > > /* free the skb */ > if (ring_is_xdp(tx_ring)) > - page_frag_free(tx_buffer->data); > + xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem); Based on my suggestion above this section would have: total_packets++; > else > napi_consume_skb(tx_buffer->skb, napi_budget); This section would pull the lines that read gso_segs and tx_flags down into this else statement so that the fields go unused. > > @@ -5787,7 +5787,7 @@ static void ixgbe_clean_tx_ring(struct ixgbe_ring *tx_ring) > > /* Free all the Tx ring sk_buffs */ > if (ring_is_xdp(tx_ring)) > - page_frag_free(tx_buffer->data); > + xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem); > else > dev_kfree_skb_any(tx_buffer->skb); > > @@ -8351,6 +8351,8 @@ static int ixgbe_xmit_xdp_ring(struct ixgbe_adapter *adapter, > dma_unmap_len_set(tx_buffer, len, len); > dma_unmap_addr_set(tx_buffer, dma, dma); > tx_buffer->data = xdp->data; > + tx_buffer->xdp_mem = xdp->rxq->mem; > + It would work out better if you moved this up to the lines that are setting the tx_buffer fields. Really you could probably pull the line that is recording xdp->data up as well. > tx_desc->read.buffer_addr = cpu_to_le64(dma); > > /* put descriptor type bits */ >
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe.h b/drivers/net/ethernet/intel/ixgbe/ixgbe.h index c1e3a0039ea5..cbc20f199364 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe.h +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe.h @@ -249,6 +249,7 @@ struct ixgbe_tx_buffer { DEFINE_DMA_UNMAP_ADDR(dma); DEFINE_DMA_UNMAP_LEN(len); u32 tx_flags; + struct xdp_mem_info xdp_mem; }; struct ixgbe_rx_buffer { diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c index 85369423452d..45520eb503ee 100644 --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c @@ -1207,7 +1207,7 @@ static bool ixgbe_clean_tx_irq(struct ixgbe_q_vector *q_vector, /* free the skb */ if (ring_is_xdp(tx_ring)) - page_frag_free(tx_buffer->data); + xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem); else napi_consume_skb(tx_buffer->skb, napi_budget); @@ -5787,7 +5787,7 @@ static void ixgbe_clean_tx_ring(struct ixgbe_ring *tx_ring) /* Free all the Tx ring sk_buffs */ if (ring_is_xdp(tx_ring)) - page_frag_free(tx_buffer->data); + xdp_return_frame(tx_buffer->data, &tx_buffer->xdp_mem); else dev_kfree_skb_any(tx_buffer->skb); @@ -8351,6 +8351,8 @@ static int ixgbe_xmit_xdp_ring(struct ixgbe_adapter *adapter, dma_unmap_len_set(tx_buffer, len, len); dma_unmap_addr_set(tx_buffer, dma, dma); tx_buffer->data = xdp->data; + tx_buffer->xdp_mem = xdp->rxq->mem; + tx_desc->read.buffer_addr = cpu_to_le64(dma); /* put descriptor type bits */
Extend struct ixgbe_tx_buffer to store the xdp_mem_info. Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com> --- drivers/net/ethernet/intel/ixgbe/ixgbe.h | 1 + drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-)