[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <62858EAE-68E0-4B30-BDB2-5157D748D1BB@gmail.com>
Date: Tue, 02 Jul 2019 09:44:40 -0700
From: "Jonathan Lemon" <jonathan.lemon@...il.com>
To: "Magnus Karlsson" <magnus.karlsson@...il.com>
Cc: "Network Development" <netdev@...r.kernel.org>,
"Björn Töpel" <bjorn.topel@...el.com>,
"Karlsson, Magnus" <magnus.karlsson@...el.com>,
"Jakub Kicinski" <jakub.kicinski@...ronome.com>,
jeffrey.t.kirsher@...el.com, kernel-team@...com
Subject: Re: [PATCH 2/3 bpf-next] i40e: Support zero-copy XDP_TX on the RX
path for AF_XDP sockets.
On 1 Jul 2019, at 4:04, Magnus Karlsson wrote:
> On Sat, Jun 29, 2019 at 12:18 AM Jonathan Lemon
> <jonathan.lemon@...il.com> wrote:
>>
>> When the XDP program attached to a zero-copy AF_XDP socket returns
>> XDP_TX,
>> queue the umem frame on the XDP TX ring. Space on the recycle stack
>> is
>> pre-allocated when the xsk is created. (taken from tx_ring, since
>> the
>> xdp ring is not initialized yet)
>>
>> Signed-off-by: Jonathan Lemon <jonathan.lemon@...il.com>
>> ---
>> drivers/net/ethernet/intel/i40e/i40e_txrx.h | 1 +
>> drivers/net/ethernet/intel/i40e/i40e_xsk.c | 54
>> +++++++++++++++++++--
>> 2 files changed, 51 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
>> b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
>> index 100e92d2982f..3e7954277737 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.h
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.h
>> @@ -274,6 +274,7 @@ static inline unsigned int
>> i40e_txd_use_count(unsigned int size)
>> #define I40E_TX_FLAGS_TSYN BIT(8)
>> #define I40E_TX_FLAGS_FD_SB BIT(9)
>> #define I40E_TX_FLAGS_UDP_TUNNEL BIT(10)
>> +#define I40E_TX_FLAGS_ZC_FRAME BIT(11)
>> #define I40E_TX_FLAGS_VLAN_MASK 0xffff0000
>> #define I40E_TX_FLAGS_VLAN_PRIO_MASK 0xe0000000
>> #define I40E_TX_FLAGS_VLAN_PRIO_SHIFT 29
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> index ce8650d06962..020f9859215d 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
>> @@ -91,7 +91,8 @@ static int i40e_xsk_umem_enable(struct i40e_vsi
>> *vsi, struct xdp_umem *umem,
>> qid >= netdev->real_num_tx_queues)
>> return -EINVAL;
>>
>> - if (!xsk_umem_recycle_alloc(umem, vsi->rx_rings[0]->count))
>> + if (!xsk_umem_recycle_alloc(umem, vsi->rx_rings[0]->count +
>> + vsi->tx_rings[0]->count))
>> return -ENOMEM;
>>
>> err = i40e_xsk_umem_dma_map(vsi, umem);
>> @@ -175,6 +176,48 @@ int i40e_xsk_umem_setup(struct i40e_vsi *vsi,
>> struct xdp_umem *umem,
>> i40e_xsk_umem_disable(vsi, qid);
>> }
>>
>> +static int i40e_xmit_rcvd_zc(struct i40e_ring *rx_ring, struct
>> xdp_buff *xdp)
>
> This function looks very much like i40e_xmit_xdp_ring(). How can we
> refactor them to make them share more code and not lose performance at
> the same time? This comment is also valid for the ixgbe driver patch
> that follows.
The next patch will split these into a small preamble setup and then
call a common send function.
--
Jonathan
>
> Thanks: Magnus
>
>> +{
>> + struct i40e_ring *xdp_ring;
>> + struct i40e_tx_desc *tx_desc;
>> + struct i40e_tx_buffer *tx_bi;
>> + struct xdp_frame *xdpf;
>> + dma_addr_t dma;
>> +
>> + xdp_ring = rx_ring->vsi->xdp_rings[rx_ring->queue_index];
>> +
>> + if (!unlikely(I40E_DESC_UNUSED(xdp_ring))) {
>> + xdp_ring->tx_stats.tx_busy++;
>> + return I40E_XDP_CONSUMED;
>> + }
>> + xdpf = convert_to_xdp_frame_keep_zc(xdp);
>> + if (unlikely(!xdpf))
>> + return I40E_XDP_CONSUMED;
>> + xdpf->handle = xdp->handle;
>> +
>> + dma = xdp_umem_get_dma(rx_ring->xsk_umem, xdp->handle);
>> + tx_bi = &xdp_ring->tx_bi[xdp_ring->next_to_use];
>> + tx_bi->bytecount = xdpf->len;
>> + tx_bi->gso_segs = 1;
>> + tx_bi->xdpf = xdpf;
>> + tx_bi->tx_flags = I40E_TX_FLAGS_ZC_FRAME;
>> +
>> + tx_desc = I40E_TX_DESC(xdp_ring, xdp_ring->next_to_use);
>> + tx_desc->buffer_addr = cpu_to_le64(dma);
>> + tx_desc->cmd_type_offset_bsz =
>> build_ctob(I40E_TX_DESC_CMD_ICRC |
>> +
>> I40E_TX_DESC_CMD_EOP,
>> + 0, xdpf->len, 0);
>> + smp_wmb();
>> +
>> + xdp_ring->next_to_use++;
>> + if (xdp_ring->next_to_use == xdp_ring->count)
>> + xdp_ring->next_to_use = 0;
>> +
>> + tx_bi->next_to_watch = tx_desc;
>> +
>> + return I40E_XDP_TX;
>> +}
>> +
>> /**
>> * i40e_run_xdp_zc - Executes an XDP program on an xdp_buff
>> * @rx_ring: Rx ring
>> @@ -187,7 +230,6 @@ int i40e_xsk_umem_setup(struct i40e_vsi *vsi,
>> struct xdp_umem *umem,
>> static int i40e_run_xdp_zc(struct i40e_ring *rx_ring, struct
>> xdp_buff *xdp)
>> {
>> int err, result = I40E_XDP_PASS;
>> - struct i40e_ring *xdp_ring;
>> struct bpf_prog *xdp_prog;
>> u32 act;
>>
>> @@ -202,8 +244,7 @@ static int i40e_run_xdp_zc(struct i40e_ring
>> *rx_ring, struct xdp_buff *xdp)
>> case XDP_PASS:
>> break;
>> case XDP_TX:
>> - xdp_ring =
>> rx_ring->vsi->xdp_rings[rx_ring->queue_index];
>> - result = i40e_xmit_xdp_tx_ring(xdp, xdp_ring);
>> + result = i40e_xmit_rcvd_zc(rx_ring, xdp);
>> break;
>> case XDP_REDIRECT:
>> err = xdp_do_redirect(rx_ring->netdev, xdp,
>> xdp_prog);
>> @@ -628,6 +669,11 @@ static bool i40e_xmit_zc(struct i40e_ring
>> *xdp_ring, unsigned int budget)
>> static void i40e_clean_xdp_tx_buffer(struct i40e_ring *tx_ring,
>> struct i40e_tx_buffer *tx_bi)
>> {
>> + if (tx_bi->tx_flags & I40E_TX_FLAGS_ZC_FRAME) {
>> + xsk_umem_recycle_addr(tx_ring->xsk_umem,
>> tx_bi->xdpf->handle);
>> + tx_bi->tx_flags = 0;
>> + return;
>> + }
>> xdp_return_frame(tx_bi->xdpf);
>> dma_unmap_single(tx_ring->dev,
>> dma_unmap_addr(tx_bi, dma),
>> --
>> 2.17.1
>>
Powered by blists - more mailing lists