lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ