| 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
| ||
|
Message-ID: <Zs28CpU2ZkglgUiZ@lzaremba-mobl.ger.corp.intel.com>
Date: Tue, 27 Aug 2024 13:44:10 +0200
From: Larysa Zaremba <larysa.zaremba@...el.com>
To: Brett Creeley <brett.creeley@....com>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>, <kuba@...nel.org>,
<edumazet@...gle.com>, <pabeni@...hat.com>, <shannon.nelson@....com>
Subject: Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog
On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
> From: Shannon Nelson <shannon.nelson@....com>
>
> We originally were using a per-interface xdp_prog variable to track
> a loaded XDP program since we knew there would never be support for a
> per-queue XDP program. With that, we only built the per queue rxq_info
> struct when an XDP program was loaded and removed it on XDP program unload,
> and used the pointer as an indicator in the Rx hotpath to know to how build
> the buffers. However, that's really not the model generally used, and
> makes a conversion to page_pool Rx buffer cacheing a little problematic.
>
> This patch converts the driver to use the more common approach of using
> a per-queue xdp_prog pointer to work out buffer allocations and need
> for bpf_prog_run_xdp(). We jostle a couple of fields in the queue struct
> in order to keep the new xdp_prog pointer in a warm cacheline.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
> Signed-off-by: Brett Creeley <brett.creeley@....com>
Reviewed-by: Larysa Zaremba <larysa.zaremba@...el.com>
If you happen to send another version, please include in a commit message a note
about READ_ONCE() removal. The removal itself is OK, but an indication that this
was intentional would be nice.
> ---
> .../net/ethernet/pensando/ionic/ionic_dev.h | 7 +++++--
> .../net/ethernet/pensando/ionic/ionic_lif.c | 14 +++++++------
> .../net/ethernet/pensando/ionic/ionic_txrx.c | 21 +++++++++----------
> 3 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> index c647033f3ad2..19ae68a86a0b 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
> @@ -238,9 +238,8 @@ struct ionic_queue {
> unsigned int index;
> unsigned int num_descs;
> unsigned int max_sg_elems;
> +
> u64 features;
> - unsigned int type;
> - unsigned int hw_index;
> unsigned int hw_type;
> bool xdp_flush;
> union {
> @@ -261,7 +260,11 @@ struct ionic_queue {
> struct ionic_rxq_sg_desc *rxq_sgl;
> };
> struct xdp_rxq_info *xdp_rxq_info;
> + struct bpf_prog *xdp_prog;
> struct ionic_queue *partner;
> +
> + unsigned int type;
> + unsigned int hw_index;
> dma_addr_t base_pa;
> dma_addr_t cmb_base_pa;
> dma_addr_t sg_base_pa;
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index aa0cc31dfe6e..0fba2df33915 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -2700,24 +2700,24 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
>
> static int ionic_xdp_queues_config(struct ionic_lif *lif)
> {
> + struct bpf_prog *xdp_prog;
> unsigned int i;
> int err;
>
> if (!lif->rxqcqs)
> return 0;
>
> - /* There's no need to rework memory if not going to/from NULL program.
> - * If there is no lif->xdp_prog, there should also be no q.xdp_rxq_info
> - * This way we don't need to keep an *xdp_prog in every queue struct.
> - */
> - if (!lif->xdp_prog == !lif->rxqcqs[0]->q.xdp_rxq_info)
> + /* There's no need to rework memory if not going to/from NULL program. */
> + xdp_prog = READ_ONCE(lif->xdp_prog);
> + if (!xdp_prog == !lif->rxqcqs[0]->q.xdp_prog)
> return 0;
>
> for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
> struct ionic_queue *q = &lif->rxqcqs[i]->q;
>
> - if (q->xdp_rxq_info) {
> + if (q->xdp_prog) {
> ionic_xdp_unregister_rxq_info(q);
> + q->xdp_prog = NULL;
> continue;
> }
>
> @@ -2727,6 +2727,7 @@ static int ionic_xdp_queues_config(struct ionic_lif *lif)
> i, err);
> goto err_out;
> }
> + q->xdp_prog = xdp_prog;
> }
>
> return 0;
> @@ -2878,6 +2879,7 @@ static void ionic_swap_queues(struct ionic_qcq *a, struct ionic_qcq *b)
> swap(a->q.base, b->q.base);
> swap(a->q.base_pa, b->q.base_pa);
> swap(a->q.info, b->q.info);
> + swap(a->q.xdp_prog, b->q.xdp_prog);
> swap(a->q.xdp_rxq_info, b->q.xdp_rxq_info);
> swap(a->q.partner, b->q.partner);
> swap(a->q_base, b->q_base);
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> index d62b2b60b133..858ab4fd9218 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> @@ -190,7 +190,7 @@ static bool ionic_rx_buf_recycle(struct ionic_queue *q,
> if (page_to_nid(buf_info->page) != numa_mem_id())
> return false;
>
> - size = ALIGN(len, q->xdp_rxq_info ? IONIC_PAGE_SIZE : IONIC_PAGE_SPLIT_SZ);
> + size = ALIGN(len, q->xdp_prog ? IONIC_PAGE_SIZE : IONIC_PAGE_SPLIT_SZ);
> buf_info->page_offset += size;
> if (buf_info->page_offset >= IONIC_PAGE_SIZE)
> return false;
> @@ -639,8 +639,7 @@ static void ionic_rx_clean(struct ionic_queue *q,
> struct net_device *netdev = q->lif->netdev;
> struct ionic_qcq *qcq = q_to_qcq(q);
> struct ionic_rx_stats *stats;
> - struct bpf_prog *xdp_prog;
> - unsigned int headroom;
> + unsigned int headroom = 0;
> struct sk_buff *skb;
> bool synced = false;
> bool use_copybreak;
> @@ -664,14 +663,13 @@ static void ionic_rx_clean(struct ionic_queue *q,
> stats->pkts++;
> stats->bytes += len;
>
> - xdp_prog = READ_ONCE(q->lif->xdp_prog);
> - if (xdp_prog) {
> - if (ionic_run_xdp(stats, netdev, xdp_prog, q, desc_info->bufs, len))
> + if (q->xdp_prog) {
> + if (ionic_run_xdp(stats, netdev, q->xdp_prog, q, desc_info->bufs, len))
> return;
> synced = true;
> + headroom = XDP_PACKET_HEADROOM;
> }
>
> - headroom = q->xdp_rxq_info ? XDP_PACKET_HEADROOM : 0;
> use_copybreak = len <= q->lif->rx_copybreak;
> if (use_copybreak)
> skb = ionic_rx_copybreak(netdev, q, desc_info,
> @@ -814,7 +812,7 @@ void ionic_rx_fill(struct ionic_queue *q)
> len = netdev->mtu + VLAN_ETH_HLEN;
>
> for (i = n_fill; i; i--) {
> - unsigned int headroom;
> + unsigned int headroom = 0;
> unsigned int buf_len;
>
> nfrags = 0;
> @@ -835,11 +833,12 @@ void ionic_rx_fill(struct ionic_queue *q)
> * XDP uses space in the first buffer, so account for
> * head room, tail room, and ip header in the first frag size.
> */
> - headroom = q->xdp_rxq_info ? XDP_PACKET_HEADROOM : 0;
> - if (q->xdp_rxq_info)
> + if (q->xdp_prog) {
> buf_len = IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN;
> - else
> + headroom = XDP_PACKET_HEADROOM;
> + } else {
> buf_len = ionic_rx_buf_size(buf_info);
> + }
> frag_len = min_t(u16, len, buf_len);
>
> desc->addr = cpu_to_le64(ionic_rx_buf_pa(buf_info) + headroom);
> --
> 2.17.1
>
>
Powered by blists - more mailing lists