* [PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool
@ 2024-08-26 18:44 Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 1/5] ionic: debug line for Tx completion errors Brett Creeley
` (4 more replies)
0 siblings, 5 replies; 14+ messages in thread
From: Brett Creeley @ 2024-08-26 18:44 UTC (permalink / raw)
To: netdev, davem, kuba, edumazet, pabeni; +Cc: shannon.nelson, brett.creeley
From: Shannon Nelson <shannon.nelson@amd.com>
Our home-grown buffer management needs to go away and we need to play
nicely with the page_pool infrastructure. This patchset cleans up some
of our API use and converts the Rx traffic queues to use page_pool.
The first few patches are for tidying up things before the last patch
which does the actual conversion.
The result is code that more closely follows current patterns, as well as
a either a performance boost or equivalent performance as seen with
iperf testing:
mss netio tx_pps rx_pps total_pps tx_bw rx_bw total_bw
---- ------- ---------- ---------- ----------- ------- ------- ----------
Before:
256 bidir 13,839,293 15,515,227 29,354,520 34 38 71
512 bidir 13,913,249 14,671,693 28,584,942 62 65 127
1024 bidir 13,006,189 13,695,413 26,701,602 109 115 224
1448 bidir 12,489,905 12,791,734 25,281,639 145 149 294
2048 bidir 9,195,622 9,247,649 18,443,271 148 149 297
4096 bidir 5,149,716 5,247,917 10,397,633 160 163 323
8192 bidir 3,029,993 3,008,882 6,038,875 179 179 358
9000 bidir 2,789,358 2,800,744 5,590,102 181 180 361
After:
256 bidir 21,540,037 21,344,644 42,884,681 52 52 104
512 bidir 23,170,014 19,207,260 42,377,274 103 85 188
1024 bidir 17,934,280 17,819,247 35,753,527 150 149 299
1448 bidir 15,242,515 14,907,030 30,149,545 167 174 341
2048 bidir 10,692,542 10,663,023 21,355,565 177 176 353
4096 bidir 6,024,977 6,083,580 12,108,557 187 180 367
8192 bidir 3,090,449 3,048,266 6,138,715 180 176 356
9000 bidir 2,859,146 2,864,226 5,723,372 178 180 358
v2:
- split out prep work into separate patches
- reworked to always use rxq_info structs
- be sure we're in NAPI when calling xdp_return_frame_rx_napi()
- fixed up issues with buffer size and lifetime management
v1/RFC:
- Link: https://lore.kernel.org/netdev/20240625165658.34598-1-shannon.nelson@amd.com/
Shannon Nelson (5):
ionic: debug line for Tx completion errors
ionic: rename ionic_xdp_rx_put_bufs
ionic: use per-queue xdp_prog
ionic: always use rxq_info
ionic: convert Rx queue buffers to use page_pool
drivers/net/ethernet/pensando/Kconfig | 1 +
.../net/ethernet/pensando/ionic/ionic_dev.h | 21 +-
.../net/ethernet/pensando/ionic/ionic_lif.c | 121 +++---
.../net/ethernet/pensando/ionic/ionic_txrx.c | 372 +++++++++---------
4 files changed, 267 insertions(+), 248 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 net-next 1/5] ionic: debug line for Tx completion errors
2024-08-26 18:44 [PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
@ 2024-08-26 18:44 ` Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 2/5] ionic: rename ionic_xdp_rx_put_bufs Brett Creeley
` (3 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Brett Creeley @ 2024-08-26 18:44 UTC (permalink / raw)
To: netdev, davem, kuba, edumazet, pabeni; +Cc: shannon.nelson, brett.creeley
From: Shannon Nelson <shannon.nelson@amd.com>
Here's a little debugging aid in case the device starts throwing
Tx completion errors.
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index fc79baad4561..ccdc0eefabe4 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -648,7 +648,14 @@ static void ionic_rx_clean(struct ionic_queue *q,
stats = q_to_rx_stats(q);
- if (comp->status) {
+ if (unlikely(comp->status)) {
+ /* Most likely status==2 and the pkt received was bigger
+ * than the buffer available: comp->len will show the
+ * pkt size received that didn't fit the advertised desc.len
+ */
+ dev_dbg(q->dev, "q%d drop comp->status %d comp->len %d desc->len %d\n",
+ q->index, comp->status, comp->len, q->rxq[q->head_idx].len);
+
stats->dropped++;
return;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 net-next 2/5] ionic: rename ionic_xdp_rx_put_bufs
2024-08-26 18:44 [PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 1/5] ionic: debug line for Tx completion errors Brett Creeley
@ 2024-08-26 18:44 ` Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog Brett Creeley
` (2 subsequent siblings)
4 siblings, 0 replies; 14+ messages in thread
From: Brett Creeley @ 2024-08-26 18:44 UTC (permalink / raw)
To: netdev, davem, kuba, edumazet, pabeni; +Cc: shannon.nelson, brett.creeley
From: Shannon Nelson <shannon.nelson@amd.com>
We aren't "putting" buf, we're just unlinking them from our tracking in
order to let the XDP_TX and XDP_REDIRECT tx clean paths take care of the
pages when they are done with them. This rename clears up the intent.
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
drivers/net/ethernet/pensando/ionic/ionic_txrx.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index ccdc0eefabe4..d62b2b60b133 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -481,9 +481,9 @@ int ionic_xdp_xmit(struct net_device *netdev, int n,
return nxmit;
}
-static void ionic_xdp_rx_put_bufs(struct ionic_queue *q,
- struct ionic_buf_info *buf_info,
- int nbufs)
+static void ionic_xdp_rx_unlink_bufs(struct ionic_queue *q,
+ struct ionic_buf_info *buf_info,
+ int nbufs)
{
int i;
@@ -600,7 +600,7 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
netdev_dbg(netdev, "tx ionic_xdp_post_frame err %d\n", err);
goto out_xdp_abort;
}
- ionic_xdp_rx_put_bufs(rxq, buf_info, nbufs);
+ ionic_xdp_rx_unlink_bufs(rxq, buf_info, nbufs);
stats->xdp_tx++;
/* the Tx completion will free the buffers */
@@ -612,7 +612,7 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
netdev_dbg(netdev, "xdp_do_redirect err %d\n", err);
goto out_xdp_abort;
}
- ionic_xdp_rx_put_bufs(rxq, buf_info, nbufs);
+ ionic_xdp_rx_unlink_bufs(rxq, buf_info, nbufs);
rxq->xdp_flush = true;
stats->xdp_redirect++;
break;
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog
2024-08-26 18:44 [PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 1/5] ionic: debug line for Tx completion errors Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 2/5] ionic: rename ionic_xdp_rx_put_bufs Brett Creeley
@ 2024-08-26 18:44 ` Brett Creeley
2024-08-27 11:44 ` Larysa Zaremba
2024-08-26 18:44 ` [PATCH v2 net-next 4/5] ionic: always use rxq_info Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
4 siblings, 1 reply; 14+ messages in thread
From: Brett Creeley @ 2024-08-26 18:44 UTC (permalink / raw)
To: netdev, davem, kuba, edumazet, pabeni; +Cc: shannon.nelson, brett.creeley
From: Shannon Nelson <shannon.nelson@amd.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@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
.../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
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 net-next 4/5] ionic: always use rxq_info
2024-08-26 18:44 [PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
` (2 preceding siblings ...)
2024-08-26 18:44 ` [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog Brett Creeley
@ 2024-08-26 18:44 ` Brett Creeley
2024-08-27 12:08 ` Larysa Zaremba
2024-08-26 18:44 ` [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
4 siblings, 1 reply; 14+ messages in thread
From: Brett Creeley @ 2024-08-26 18:44 UTC (permalink / raw)
To: netdev, davem, kuba, edumazet, pabeni; +Cc: shannon.nelson, brett.creeley
From: Shannon Nelson <shannon.nelson@amd.com>
Instead of setting up and tearing down the rxq_info only when the XDP
program is loaded or unloaded, we will build the rxq_info whether or not
XDP is in use. This is the more common use pattern and better supports
future conversion to page_pool. Since the rxq_info wants the napi_id
we re-order things slightly to tie this into the queue init and deinit
functions where we do the add and delete of napi.
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
.../net/ethernet/pensando/ionic/ionic_lif.c | 55 +++++++------------
1 file changed, 19 insertions(+), 36 deletions(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 0fba2df33915..4a7763ec061f 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -46,8 +46,9 @@ static int ionic_start_queues(struct ionic_lif *lif);
static void ionic_stop_queues(struct ionic_lif *lif);
static void ionic_lif_queue_identify(struct ionic_lif *lif);
-static int ionic_xdp_queues_config(struct ionic_lif *lif);
-static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q);
+static void ionic_xdp_queues_config(struct ionic_lif *lif);
+static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id);
+static void ionic_unregister_rxq_info(struct ionic_queue *q);
static void ionic_dim_work(struct work_struct *work)
{
@@ -380,6 +381,7 @@ static void ionic_lif_qcq_deinit(struct ionic_lif *lif, struct ionic_qcq *qcq)
if (!(qcq->flags & IONIC_QCQ_F_INITED))
return;
+ ionic_unregister_rxq_info(&qcq->q);
if (qcq->flags & IONIC_QCQ_F_INTR) {
ionic_intr_mask(idev->intr_ctrl, qcq->intr.index,
IONIC_INTR_MASK_SET);
@@ -437,9 +439,7 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
qcq->sg_base_pa = 0;
}
- ionic_xdp_unregister_rxq_info(&qcq->q);
ionic_qcq_intr_free(lif, qcq);
-
vfree(qcq->q.info);
qcq->q.info = NULL;
}
@@ -925,6 +925,11 @@ static int ionic_lif_rxq_init(struct ionic_lif *lif, struct ionic_qcq *qcq)
netif_napi_add(lif->netdev, &qcq->napi, ionic_rx_napi);
else
netif_napi_add(lif->netdev, &qcq->napi, ionic_txrx_napi);
+ err = ionic_register_rxq_info(q, qcq->napi.napi_id);
+ if (err) {
+ netif_napi_del(&qcq->napi);
+ return err;
+ }
qcq->flags |= IONIC_QCQ_F_INITED;
@@ -2143,9 +2148,7 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
int derr = 0;
int i, err;
- err = ionic_xdp_queues_config(lif);
- if (err)
- return err;
+ ionic_xdp_queues_config(lif);
for (i = 0; i < lif->nxqs; i++) {
if (!(lif->rxqcqs[i] && lif->txqcqs[i])) {
@@ -2192,8 +2195,6 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
derr = ionic_qcq_disable(lif, lif->rxqcqs[i], derr);
}
- ionic_xdp_queues_config(lif);
-
return err;
}
@@ -2651,7 +2652,7 @@ static void ionic_vf_attr_replay(struct ionic_lif *lif)
ionic_vf_start(ionic);
}
-static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q)
+static void ionic_unregister_rxq_info(struct ionic_queue *q)
{
struct xdp_rxq_info *xi;
@@ -2665,7 +2666,7 @@ static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q)
kfree(xi);
}
-static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
+static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
{
struct xdp_rxq_info *rxq_info;
int err;
@@ -2698,45 +2699,27 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
return err;
}
-static int ionic_xdp_queues_config(struct ionic_lif *lif)
+static void ionic_xdp_queues_config(struct ionic_lif *lif)
{
struct bpf_prog *xdp_prog;
unsigned int i;
- int err;
if (!lif->rxqcqs)
- return 0;
+ return;
- /* There's no need to rework memory if not going to/from NULL program. */
+ /* Nothing to do 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;
+ return;
for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
struct ionic_queue *q = &lif->rxqcqs[i]->q;
- if (q->xdp_prog) {
- ionic_xdp_unregister_rxq_info(q);
+ if (q->xdp_prog)
q->xdp_prog = NULL;
- continue;
- }
-
- err = ionic_xdp_register_rxq_info(q, lif->rxqcqs[i]->napi.napi_id);
- if (err) {
- dev_err(lif->ionic->dev, "failed to register RX queue %d info for XDP, err %d\n",
- i, err);
- goto err_out;
- }
- q->xdp_prog = xdp_prog;
+ else
+ q->xdp_prog = xdp_prog;
}
-
- return 0;
-
-err_out:
- for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++)
- ionic_xdp_unregister_rxq_info(&lif->rxqcqs[i]->q);
-
- return err;
}
static int ionic_xdp_config(struct net_device *netdev, struct netdev_bpf *bpf)
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool
2024-08-26 18:44 [PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
` (3 preceding siblings ...)
2024-08-26 18:44 ` [PATCH v2 net-next 4/5] ionic: always use rxq_info Brett Creeley
@ 2024-08-26 18:44 ` Brett Creeley
2024-08-27 19:59 ` kernel test robot
2024-08-27 22:22 ` kernel test robot
4 siblings, 2 replies; 14+ messages in thread
From: Brett Creeley @ 2024-08-26 18:44 UTC (permalink / raw)
To: netdev, davem, kuba, edumazet, pabeni; +Cc: shannon.nelson, brett.creeley
From: Shannon Nelson <shannon.nelson@amd.com>
Our home-grown buffer management needs to go away and we need
to be playing nicely with the page_pool infrastructure. This
converts the Rx traffic queues to use page_pool.
Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
Signed-off-by: Brett Creeley <brett.creeley@amd.com>
---
drivers/net/ethernet/pensando/Kconfig | 1 +
.../net/ethernet/pensando/ionic/ionic_dev.h | 14 +-
.../net/ethernet/pensando/ionic/ionic_lif.c | 58 ++-
.../net/ethernet/pensando/ionic/ionic_txrx.c | 344 +++++++++---------
4 files changed, 221 insertions(+), 196 deletions(-)
diff --git a/drivers/net/ethernet/pensando/Kconfig b/drivers/net/ethernet/pensando/Kconfig
index 3f7519e435b8..01fe76786f77 100644
--- a/drivers/net/ethernet/pensando/Kconfig
+++ b/drivers/net/ethernet/pensando/Kconfig
@@ -23,6 +23,7 @@ config IONIC
depends on PTP_1588_CLOCK_OPTIONAL
select NET_DEVLINK
select DIMLIB
+ select PAGE_POOL
help
This enables the support for the Pensando family of Ethernet
adapters. More specific information on this driver can be
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_dev.h b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
index 19ae68a86a0b..fc93c5c4dbe6 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_dev.h
+++ b/drivers/net/ethernet/pensando/ionic/ionic_dev.h
@@ -182,9 +182,6 @@ struct ionic_qcq;
#define IONIC_MAX_BUF_LEN ((u16)-1)
#define IONIC_PAGE_SIZE PAGE_SIZE
-#define IONIC_PAGE_SPLIT_SZ (PAGE_SIZE / 2)
-#define IONIC_PAGE_GFP_MASK (GFP_ATOMIC | __GFP_NOWARN |\
- __GFP_COMP | __GFP_MEMALLOC)
#define IONIC_XDP_MAX_LINEAR_MTU (IONIC_PAGE_SIZE - \
(VLAN_ETH_HLEN + \
@@ -248,11 +245,6 @@ struct ionic_queue {
struct ionic_rxq_desc *rxq;
struct ionic_admin_cmd *adminq;
};
- union {
- void __iomem *cmb_base;
- struct ionic_txq_desc __iomem *cmb_txq;
- struct ionic_rxq_desc __iomem *cmb_rxq;
- };
union {
void *sg_base;
struct ionic_txq_sg_desc *txq_sgl;
@@ -261,8 +253,14 @@ struct ionic_queue {
};
struct xdp_rxq_info *xdp_rxq_info;
struct bpf_prog *xdp_prog;
+ struct page_pool *page_pool;
struct ionic_queue *partner;
+ union {
+ void __iomem *cmb_base;
+ struct ionic_txq_desc __iomem *cmb_txq;
+ struct ionic_rxq_desc __iomem *cmb_rxq;
+ };
unsigned int type;
unsigned int hw_index;
dma_addr_t base_pa;
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
index 4a7763ec061f..879f2e25ec31 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
@@ -13,6 +13,7 @@
#include <linux/cpumask.h>
#include <linux/crash_dump.h>
#include <linux/vmalloc.h>
+#include <net/page_pool/helpers.h>
#include "ionic.h"
#include "ionic_bus.h"
@@ -439,6 +440,9 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
qcq->sg_base_pa = 0;
}
+ page_pool_destroy(qcq->q.page_pool);
+ qcq->q.page_pool = NULL;
+
ionic_qcq_intr_free(lif, qcq);
vfree(qcq->q.info);
qcq->q.info = NULL;
@@ -579,6 +583,33 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
goto err_out_free_qcq;
}
+ if (type == IONIC_QTYPE_RXQ) {
+ struct page_pool_params pp_params = {
+ .flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
+ .order = 0,
+ .pool_size = num_descs,
+ .nid = NUMA_NO_NODE,
+ .dev = lif->ionic->dev,
+ .napi = &new->napi,
+ .dma_dir = DMA_FROM_DEVICE,
+ .max_len = PAGE_SIZE,
+ .netdev = lif->netdev,
+ };
+ struct bpf_prog *xdp_prog;
+
+ xdp_prog = READ_ONCE(lif->xdp_prog);
+ if (xdp_prog)
+ pp_params.dma_dir = DMA_BIDIRECTIONAL;
+
+ new->q.page_pool = page_pool_create(&pp_params);
+ if (IS_ERR(new->q.page_pool)) {
+ netdev_err(lif->netdev, "Cannot create page_pool\n");
+ err = PTR_ERR(new->q.page_pool);
+ new->q.page_pool = NULL;
+ goto err_out_free_q_info;
+ }
+ }
+
new->q.type = type;
new->q.max_sg_elems = lif->qtype_info[type].max_sg_elems;
@@ -586,12 +617,12 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
desc_size, sg_desc_size, pid);
if (err) {
netdev_err(lif->netdev, "Cannot initialize queue\n");
- goto err_out_free_q_info;
+ goto err_out_free_page_pool;
}
err = ionic_alloc_qcq_interrupt(lif, new);
if (err)
- goto err_out_free_q_info;
+ goto err_out_free_page_pool;
err = ionic_cq_init(lif, &new->cq, &new->intr, num_descs, cq_desc_size);
if (err) {
@@ -712,6 +743,8 @@ static int ionic_qcq_alloc(struct ionic_lif *lif, unsigned int type,
devm_free_irq(dev, new->intr.vector, &new->napi);
ionic_intr_free(lif->ionic, new->intr.index);
}
+err_out_free_page_pool:
+ page_pool_destroy(new->q.page_pool);
err_out_free_q_info:
vfree(new->q.info);
err_out_free_qcq:
@@ -2677,15 +2710,15 @@ static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
err = xdp_rxq_info_reg(rxq_info, q->lif->netdev, q->index, napi_id);
if (err) {
- dev_err(q->dev, "Queue %d xdp_rxq_info_reg failed, err %d\n",
- q->index, err);
+ netdev_err(q->lif->netdev, "q%d xdp_rxq_info_reg failed, err %d\n",
+ q->index, err);
goto err_out;
}
- err = xdp_rxq_info_reg_mem_model(rxq_info, MEM_TYPE_PAGE_ORDER0, NULL);
+ err = xdp_rxq_info_reg_mem_model(rxq_info, MEM_TYPE_PAGE_POOL, q->page_pool);
if (err) {
- dev_err(q->dev, "Queue %d xdp_rxq_info_reg_mem_model failed, err %d\n",
- q->index, err);
+ netdev_err(q->lif->netdev, "q%d xdp_rxq_info_reg_mem_model failed, err %d\n",
+ q->index, err);
xdp_rxq_info_unreg(rxq_info);
goto err_out;
}
@@ -2855,7 +2888,16 @@ static int ionic_cmb_reconfig(struct ionic_lif *lif,
static void ionic_swap_queues(struct ionic_qcq *a, struct ionic_qcq *b)
{
- /* only swapping the queues, not the napi, flags, or other stuff */
+ /* only swapping the queues and napi, not flags or other stuff */
+ swap(a->napi, b->napi);
+
+ if (a->q.type == IONIC_QTYPE_RXQ) {
+ swap(a->q.page_pool, b->q.page_pool);
+ a->q.page_pool->p.napi = &a->napi;
+ if (b->q.page_pool) /* is NULL when increasing queue count */
+ b->q.page_pool->p.napi = &b->napi;
+ }
+
swap(a->q.features, b->q.features);
swap(a->q.num_descs, b->q.num_descs);
swap(a->q.desc_size, b->q.desc_size);
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
index 858ab4fd9218..35e3751dd5a7 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
@@ -6,6 +6,7 @@
#include <linux/if_vlan.h>
#include <net/ip6_checksum.h>
#include <net/netdev_queues.h>
+#include <net/page_pool/helpers.h>
#include "ionic.h"
#include "ionic_lif.h"
@@ -118,108 +119,57 @@ static void *ionic_rx_buf_va(struct ionic_buf_info *buf_info)
static dma_addr_t ionic_rx_buf_pa(struct ionic_buf_info *buf_info)
{
- return buf_info->dma_addr + buf_info->page_offset;
+ return page_pool_get_dma_addr(buf_info->page) + buf_info->page_offset;
}
-static unsigned int ionic_rx_buf_size(struct ionic_buf_info *buf_info)
+static void __ionic_rx_put_buf(struct ionic_queue *q,
+ struct ionic_buf_info *buf_info,
+ bool recycle_direct)
{
- return min_t(u32, IONIC_MAX_BUF_LEN, IONIC_PAGE_SIZE - buf_info->page_offset);
-}
-
-static int ionic_rx_page_alloc(struct ionic_queue *q,
- struct ionic_buf_info *buf_info)
-{
- struct device *dev = q->dev;
- dma_addr_t dma_addr;
- struct page *page;
-
- page = alloc_pages(IONIC_PAGE_GFP_MASK, 0);
- if (unlikely(!page)) {
- net_err_ratelimited("%s: %s page alloc failed\n",
- dev_name(dev), q->name);
- q_to_rx_stats(q)->alloc_err++;
- return -ENOMEM;
- }
-
- dma_addr = dma_map_page(dev, page, 0,
- IONIC_PAGE_SIZE, DMA_FROM_DEVICE);
- if (unlikely(dma_mapping_error(dev, dma_addr))) {
- __free_pages(page, 0);
- net_err_ratelimited("%s: %s dma map failed\n",
- dev_name(dev), q->name);
- q_to_rx_stats(q)->dma_map_err++;
- return -EIO;
- }
-
- buf_info->dma_addr = dma_addr;
- buf_info->page = page;
- buf_info->page_offset = 0;
-
- return 0;
-}
-
-static void ionic_rx_page_free(struct ionic_queue *q,
- struct ionic_buf_info *buf_info)
-{
- struct device *dev = q->dev;
-
- if (unlikely(!buf_info)) {
- net_err_ratelimited("%s: %s invalid buf_info in free\n",
- dev_name(dev), q->name);
- return;
- }
-
if (!buf_info->page)
return;
- dma_unmap_page(dev, buf_info->dma_addr, IONIC_PAGE_SIZE, DMA_FROM_DEVICE);
- __free_pages(buf_info->page, 0);
+ page_pool_put_full_page(q->page_pool, buf_info->page, recycle_direct);
buf_info->page = NULL;
+ buf_info->len = 0;
+ buf_info->page_offset = 0;
}
-static bool ionic_rx_buf_recycle(struct ionic_queue *q,
- struct ionic_buf_info *buf_info, u32 len)
-{
- u32 size;
-
- /* don't re-use pages allocated in low-mem condition */
- if (page_is_pfmemalloc(buf_info->page))
- return false;
-
- /* don't re-use buffers from non-local numa nodes */
- if (page_to_nid(buf_info->page) != numa_mem_id())
- return false;
-
- 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;
- get_page(buf_info->page);
+static void ionic_rx_put_buf(struct ionic_queue *q,
+ struct ionic_buf_info *buf_info)
+{
+ __ionic_rx_put_buf(q, buf_info, false);
+}
- return true;
+static void ionic_rx_put_buf_direct(struct ionic_queue *q,
+ struct ionic_buf_info *buf_info)
+{
+ __ionic_rx_put_buf(q, buf_info, true);
}
static void ionic_rx_add_skb_frag(struct ionic_queue *q,
struct sk_buff *skb,
struct ionic_buf_info *buf_info,
- u32 off, u32 len,
+ u32 headroom, u32 len,
bool synced)
{
if (!synced)
- dma_sync_single_range_for_cpu(q->dev, ionic_rx_buf_pa(buf_info),
- off, len, DMA_FROM_DEVICE);
+ page_pool_dma_sync_for_cpu(q->page_pool,
+ buf_info->page,
+ buf_info->page_offset + headroom,
+ len);
skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
- buf_info->page, buf_info->page_offset + off,
- len,
- IONIC_PAGE_SIZE);
+ buf_info->page, buf_info->page_offset + headroom,
+ len, buf_info->len);
- if (!ionic_rx_buf_recycle(q, buf_info, len)) {
- dma_unmap_page(q->dev, buf_info->dma_addr,
- IONIC_PAGE_SIZE, DMA_FROM_DEVICE);
- buf_info->page = NULL;
- }
+ /* napi_gro_frags() will release/recycle the
+ * page_pool buffers from the frags list
+ */
+ buf_info->page = NULL;
+ buf_info->len = 0;
+ buf_info->page_offset = 0;
}
static struct sk_buff *ionic_rx_build_skb(struct ionic_queue *q,
@@ -244,12 +194,13 @@ static struct sk_buff *ionic_rx_build_skb(struct ionic_queue *q,
q_to_rx_stats(q)->alloc_err++;
return NULL;
}
+ skb_mark_for_recycle(skb);
if (headroom)
frag_len = min_t(u16, len,
IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN);
else
- frag_len = min_t(u16, len, ionic_rx_buf_size(buf_info));
+ frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
if (unlikely(!buf_info->page))
goto err_bad_buf_page;
@@ -260,7 +211,7 @@ static struct sk_buff *ionic_rx_build_skb(struct ionic_queue *q,
for (i = 0; i < num_sg_elems; i++, buf_info++) {
if (unlikely(!buf_info->page))
goto err_bad_buf_page;
- frag_len = min_t(u16, len, ionic_rx_buf_size(buf_info));
+ frag_len = min_t(u16, len, buf_info->len);
ionic_rx_add_skb_frag(q, skb, buf_info, 0, frag_len, synced);
len -= frag_len;
}
@@ -277,11 +228,13 @@ static struct sk_buff *ionic_rx_copybreak(struct net_device *netdev,
struct ionic_rx_desc_info *desc_info,
unsigned int headroom,
unsigned int len,
+ unsigned int num_sg_elems,
bool synced)
{
struct ionic_buf_info *buf_info;
struct device *dev = q->dev;
struct sk_buff *skb;
+ int i;
buf_info = &desc_info->bufs[0];
@@ -292,54 +245,52 @@ static struct sk_buff *ionic_rx_copybreak(struct net_device *netdev,
q_to_rx_stats(q)->alloc_err++;
return NULL;
}
-
- if (unlikely(!buf_info->page)) {
- dev_kfree_skb(skb);
- return NULL;
- }
+ skb_mark_for_recycle(skb);
if (!synced)
- dma_sync_single_range_for_cpu(dev, ionic_rx_buf_pa(buf_info),
- headroom, len, DMA_FROM_DEVICE);
+ page_pool_dma_sync_for_cpu(q->page_pool,
+ buf_info->page,
+ buf_info->page_offset + headroom,
+ len);
+
skb_copy_to_linear_data(skb, ionic_rx_buf_va(buf_info) + headroom, len);
- dma_sync_single_range_for_device(dev, ionic_rx_buf_pa(buf_info),
- headroom, len, DMA_FROM_DEVICE);
skb_put(skb, len);
skb->protocol = eth_type_trans(skb, netdev);
+ /* recycle the Rx buffer now that we're done with it */
+ ionic_rx_put_buf_direct(q, buf_info);
+ buf_info++;
+ for (i = 0; i < num_sg_elems; i++, buf_info++)
+ ionic_rx_put_buf_direct(q, buf_info);
+
return skb;
}
static void ionic_xdp_tx_desc_clean(struct ionic_queue *q,
- struct ionic_tx_desc_info *desc_info)
+ struct ionic_tx_desc_info *desc_info,
+ bool in_napi)
{
- unsigned int nbufs = desc_info->nbufs;
- struct ionic_buf_info *buf_info;
- struct device *dev = q->dev;
- int i;
+ struct xdp_frame_bulk bq;
- if (!nbufs)
+ if (!desc_info->nbufs)
return;
- buf_info = desc_info->bufs;
- dma_unmap_single(dev, buf_info->dma_addr,
- buf_info->len, DMA_TO_DEVICE);
- if (desc_info->act == XDP_TX)
- __free_pages(buf_info->page, 0);
- buf_info->page = NULL;
+ xdp_frame_bulk_init(&bq);
+ rcu_read_lock(); /* need for xdp_return_frame_bulk */
- buf_info++;
- for (i = 1; i < nbufs + 1 && buf_info->page; i++, buf_info++) {
- dma_unmap_page(dev, buf_info->dma_addr,
- buf_info->len, DMA_TO_DEVICE);
- if (desc_info->act == XDP_TX)
- __free_pages(buf_info->page, 0);
- buf_info->page = NULL;
+ if (desc_info->act == XDP_TX) {
+ if (likely(in_napi))
+ xdp_return_frame_rx_napi(desc_info->xdpf);
+ else
+ xdp_return_frame(desc_info->xdpf);
+ } else if (desc_info->act == XDP_REDIRECT) {
+ ionic_tx_desc_unmap_bufs(q, desc_info);
+ xdp_return_frame_bulk(desc_info->xdpf, &bq);
}
- if (desc_info->act == XDP_REDIRECT)
- xdp_return_frame(desc_info->xdpf);
+ xdp_flush_frame_bulk(&bq);
+ rcu_read_unlock();
desc_info->nbufs = 0;
desc_info->xdpf = NULL;
@@ -363,9 +314,17 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame,
buf_info = desc_info->bufs;
stats = q_to_tx_stats(q);
- dma_addr = ionic_tx_map_single(q, frame->data, len);
- if (!dma_addr)
- return -EIO;
+ if (act == XDP_TX) {
+ dma_addr = page_pool_get_dma_addr(page) +
+ off + XDP_PACKET_HEADROOM;
+ dma_sync_single_for_device(q->dev, dma_addr,
+ len, DMA_TO_DEVICE);
+ } else /* XDP_REDIRECT */ {
+ dma_addr = ionic_tx_map_single(q, frame->data, len);
+ if (!dma_addr)
+ return -EIO;
+ }
+
buf_info->dma_addr = dma_addr;
buf_info->len = len;
buf_info->page = page;
@@ -387,10 +346,21 @@ static int ionic_xdp_post_frame(struct ionic_queue *q, struct xdp_frame *frame,
frag = sinfo->frags;
elem = ionic_tx_sg_elems(q);
for (i = 0; i < sinfo->nr_frags; i++, frag++, bi++) {
- dma_addr = ionic_tx_map_frag(q, frag, 0, skb_frag_size(frag));
- if (!dma_addr) {
- ionic_tx_desc_unmap_bufs(q, desc_info);
- return -EIO;
+ if (act == XDP_TX) {
+ struct page *pg = skb_frag_page(frag);
+
+ dma_addr = page_pool_get_dma_addr(pg) +
+ skb_frag_off(frag);
+ dma_sync_single_for_device(q->dev, dma_addr,
+ skb_frag_size(frag),
+ DMA_TO_DEVICE);
+ } else {
+ dma_addr = ionic_tx_map_frag(q, frag, 0,
+ skb_frag_size(frag));
+ if (dma_mapping_error(q->dev, dma_addr)) {
+ ionic_tx_desc_unmap_bufs(q, desc_info);
+ return -EIO;
+ }
}
bi->dma_addr = dma_addr;
bi->len = skb_frag_size(frag);
@@ -488,8 +458,6 @@ static void ionic_xdp_rx_unlink_bufs(struct ionic_queue *q,
int i;
for (i = 0; i < nbufs; i++) {
- dma_unmap_page(q->dev, buf_info->dma_addr,
- IONIC_PAGE_SIZE, DMA_FROM_DEVICE);
buf_info->page = NULL;
buf_info++;
}
@@ -516,11 +484,9 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
frag_len = min_t(u16, len, IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN);
xdp_prepare_buff(&xdp_buf, ionic_rx_buf_va(buf_info),
XDP_PACKET_HEADROOM, frag_len, false);
-
- dma_sync_single_range_for_cpu(rxq->dev, ionic_rx_buf_pa(buf_info),
- XDP_PACKET_HEADROOM, frag_len,
- DMA_FROM_DEVICE);
-
+ page_pool_dma_sync_for_cpu(rxq->page_pool, buf_info->page,
+ buf_info->page_offset + XDP_PACKET_HEADROOM,
+ frag_len);
prefetchw(&xdp_buf.data_hard_start);
/* We limit MTU size to one buffer if !xdp_has_frags, so
@@ -542,15 +508,16 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
do {
if (unlikely(sinfo->nr_frags >= MAX_SKB_FRAGS)) {
err = -ENOSPC;
- goto out_xdp_abort;
+ break;
}
frag = &sinfo->frags[sinfo->nr_frags];
sinfo->nr_frags++;
bi++;
- frag_len = min_t(u16, remain_len, ionic_rx_buf_size(bi));
- dma_sync_single_range_for_cpu(rxq->dev, ionic_rx_buf_pa(bi),
- 0, frag_len, DMA_FROM_DEVICE);
+ frag_len = min_t(u16, remain_len, bi->len);
+ page_pool_dma_sync_for_cpu(rxq->page_pool, bi->page,
+ buf_info->page_offset,
+ frag_len);
skb_frag_fill_page_desc(frag, bi->page, 0, frag_len);
sinfo->xdp_frags_size += frag_len;
remain_len -= frag_len;
@@ -569,14 +536,16 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
return false; /* false = we didn't consume the packet */
case XDP_DROP:
- ionic_rx_page_free(rxq, buf_info);
+ ionic_rx_put_buf_direct(rxq, buf_info);
stats->xdp_drop++;
break;
case XDP_TX:
xdpf = xdp_convert_buff_to_frame(&xdp_buf);
- if (!xdpf)
- goto out_xdp_abort;
+ if (!xdpf) {
+ err = -ENOSPC;
+ break;
+ }
txq = rxq->partner;
nq = netdev_get_tx_queue(netdev, txq->index);
@@ -588,7 +557,8 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
ionic_q_space_avail(txq),
1, 1)) {
__netif_tx_unlock(nq);
- goto out_xdp_abort;
+ err = -EIO;
+ break;
}
err = ionic_xdp_post_frame(txq, xdpf, XDP_TX,
@@ -598,19 +568,17 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
__netif_tx_unlock(nq);
if (unlikely(err)) {
netdev_dbg(netdev, "tx ionic_xdp_post_frame err %d\n", err);
- goto out_xdp_abort;
+ break;
}
ionic_xdp_rx_unlink_bufs(rxq, buf_info, nbufs);
stats->xdp_tx++;
-
- /* the Tx completion will free the buffers */
break;
case XDP_REDIRECT:
err = xdp_do_redirect(netdev, &xdp_buf, xdp_prog);
if (unlikely(err)) {
netdev_dbg(netdev, "xdp_do_redirect err %d\n", err);
- goto out_xdp_abort;
+ break;
}
ionic_xdp_rx_unlink_bufs(rxq, buf_info, nbufs);
rxq->xdp_flush = true;
@@ -619,15 +587,15 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats,
case XDP_ABORTED:
default:
- goto out_xdp_abort;
+ err = -EIO;
+ break;
}
- return true;
-
-out_xdp_abort:
- trace_xdp_exception(netdev, xdp_prog, xdp_action);
- ionic_rx_page_free(rxq, buf_info);
- stats->xdp_aborted++;
+ if (err) {
+ ionic_rx_put_buf_direct(rxq, buf_info);
+ trace_xdp_exception(netdev, xdp_prog, xdp_action);
+ stats->xdp_aborted++;
+ }
return true;
}
@@ -673,7 +641,8 @@ static void ionic_rx_clean(struct ionic_queue *q,
use_copybreak = len <= q->lif->rx_copybreak;
if (use_copybreak)
skb = ionic_rx_copybreak(netdev, q, desc_info,
- headroom, len, synced);
+ headroom, len,
+ comp->num_sg_elems, synced);
else
skb = ionic_rx_build_skb(q, desc_info, headroom, len,
comp->num_sg_elems, synced);
@@ -794,6 +763,9 @@ void ionic_rx_fill(struct ionic_queue *q)
struct ionic_buf_info *buf_info;
unsigned int fill_threshold;
struct ionic_rxq_desc *desc;
+ unsigned int first_frag_len;
+ unsigned int first_buf_len;
+ unsigned int headroom = 0;
unsigned int remain_len;
unsigned int frag_len;
unsigned int nfrags;
@@ -811,35 +783,42 @@ void ionic_rx_fill(struct ionic_queue *q)
len = netdev->mtu + VLAN_ETH_HLEN;
- for (i = n_fill; i; i--) {
- unsigned int headroom = 0;
- unsigned int buf_len;
+ if (q->xdp_prog) {
+ /* Always alloc the full size buffer, but only need
+ * the actual frag_len in the descriptor
+ * XDP uses space in the first buffer, so account for
+ * head room, tail room, and ip header in the first frag size.
+ */
+ headroom = XDP_PACKET_HEADROOM;
+ first_buf_len = IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN + headroom;
+ first_frag_len = min_t(u16, len + headroom, first_buf_len);
+ } else {
+ /* Use MTU size if smaller than max buffer size */
+ first_frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
+ first_buf_len = first_frag_len;
+ }
+ for (i = n_fill; i; i--) {
+ /* fill main descriptor - buf[0] */
nfrags = 0;
remain_len = len;
desc = &q->rxq[q->head_idx];
desc_info = &q->rx_info[q->head_idx];
buf_info = &desc_info->bufs[0];
- if (!buf_info->page) { /* alloc a new buffer? */
- if (unlikely(ionic_rx_page_alloc(q, buf_info))) {
- desc->addr = 0;
- desc->len = 0;
- return;
- }
- }
-
- /* fill main descriptor - buf[0]
- * XDP uses space in the first buffer, so account for
- * head room, tail room, and ip header in the first frag size.
- */
- if (q->xdp_prog) {
- buf_len = IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN;
- headroom = XDP_PACKET_HEADROOM;
- } else {
- buf_len = ionic_rx_buf_size(buf_info);
+ buf_info->len = first_buf_len;
+ frag_len = first_frag_len - headroom;
+
+ /* get a new buffer if we can't reuse one */
+ if (!buf_info->page)
+ buf_info->page = page_pool_alloc(q->page_pool,
+ &buf_info->page_offset,
+ &buf_info->len,
+ GFP_ATOMIC);
+ if (unlikely(!buf_info->page)) {
+ buf_info->len = 0;
+ return;
}
- frag_len = min_t(u16, len, buf_len);
desc->addr = cpu_to_le64(ionic_rx_buf_pa(buf_info) + headroom);
desc->len = cpu_to_le16(frag_len);
@@ -850,16 +829,26 @@ void ionic_rx_fill(struct ionic_queue *q)
/* fill sg descriptors - buf[1..n] */
sg_elem = q->rxq_sgl[q->head_idx].elems;
for (j = 0; remain_len > 0 && j < q->max_sg_elems; j++, sg_elem++) {
- if (!buf_info->page) { /* alloc a new sg buffer? */
- if (unlikely(ionic_rx_page_alloc(q, buf_info))) {
- sg_elem->addr = 0;
- sg_elem->len = 0;
+ frag_len = min_t(u16, remain_len, IONIC_PAGE_SIZE);
+
+ /* Recycle any leftover buffers that are too small to reuse */
+ if (unlikely(buf_info->page && buf_info->len < frag_len))
+ ionic_rx_put_buf_direct(q, buf_info);
+
+ /* Get new buffer if needed */
+ if (!buf_info->page) {
+ buf_info->len = frag_len;
+ buf_info->page = page_pool_alloc(q->page_pool,
+ &buf_info->page_offset,
+ &buf_info->len,
+ GFP_ATOMIC);
+ if (unlikely(!buf_info->page)) {
+ buf_info->len = 0;
return;
}
}
sg_elem->addr = cpu_to_le64(ionic_rx_buf_pa(buf_info));
- frag_len = min_t(u16, remain_len, ionic_rx_buf_size(buf_info));
sg_elem->len = cpu_to_le16(frag_len);
remain_len -= frag_len;
buf_info++;
@@ -889,17 +878,12 @@ void ionic_rx_fill(struct ionic_queue *q)
void ionic_rx_empty(struct ionic_queue *q)
{
struct ionic_rx_desc_info *desc_info;
- struct ionic_buf_info *buf_info;
unsigned int i, j;
for (i = 0; i < q->num_descs; i++) {
desc_info = &q->rx_info[i];
- for (j = 0; j < ARRAY_SIZE(desc_info->bufs); j++) {
- buf_info = &desc_info->bufs[j];
- if (buf_info->page)
- ionic_rx_page_free(q, buf_info);
- }
-
+ for (j = 0; j < ARRAY_SIZE(desc_info->bufs); j++)
+ ionic_rx_put_buf(q, &desc_info->bufs[j]);
desc_info->nbufs = 0;
}
@@ -1172,7 +1156,7 @@ static void ionic_tx_clean(struct ionic_queue *q,
struct sk_buff *skb;
if (desc_info->xdpf) {
- ionic_xdp_tx_desc_clean(q->partner, desc_info);
+ ionic_xdp_tx_desc_clean(q->partner, desc_info, in_napi);
stats->clean++;
if (unlikely(__netif_subqueue_stopped(q->lif->netdev, q->index)))
--
2.17.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog
2024-08-26 18:44 ` [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog Brett Creeley
@ 2024-08-27 11:44 ` Larysa Zaremba
2024-08-27 11:57 ` Larysa Zaremba
2024-08-27 21:40 ` Brett Creeley
0 siblings, 2 replies; 14+ messages in thread
From: Larysa Zaremba @ 2024-08-27 11:44 UTC (permalink / raw)
To: Brett Creeley; +Cc: netdev, davem, kuba, edumazet, pabeni, shannon.nelson
On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
> From: Shannon Nelson <shannon.nelson@amd.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@amd.com>
> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.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
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog
2024-08-27 11:44 ` Larysa Zaremba
@ 2024-08-27 11:57 ` Larysa Zaremba
2024-08-27 22:27 ` Brett Creeley
2024-08-27 21:40 ` Brett Creeley
1 sibling, 1 reply; 14+ messages in thread
From: Larysa Zaremba @ 2024-08-27 11:57 UTC (permalink / raw)
To: Brett Creeley; +Cc: netdev, davem, kuba, edumazet, pabeni, shannon.nelson
On Tue, Aug 27, 2024 at 01:44:10PM +0200, Larysa Zaremba wrote:
> On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
> > From: Shannon Nelson <shannon.nelson@amd.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@amd.com>
> > Signed-off-by: Brett Creeley <brett.creeley@amd.com>
>
> Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>
>
I would like to rewoke the tag, see below why.
> 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;
In a case when we replace a non-NULL program with another non-NULL program this
would create a situation where lif and queues have different pointers on them.
> >
> > 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;
[...]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next 4/5] ionic: always use rxq_info
2024-08-26 18:44 ` [PATCH v2 net-next 4/5] ionic: always use rxq_info Brett Creeley
@ 2024-08-27 12:08 ` Larysa Zaremba
2024-08-27 22:32 ` Brett Creeley
0 siblings, 1 reply; 14+ messages in thread
From: Larysa Zaremba @ 2024-08-27 12:08 UTC (permalink / raw)
To: Brett Creeley; +Cc: netdev, davem, kuba, edumazet, pabeni, shannon.nelson
On Mon, Aug 26, 2024 at 11:44:21AM -0700, Brett Creeley wrote:
> From: Shannon Nelson <shannon.nelson@amd.com>
>
> Instead of setting up and tearing down the rxq_info only when the XDP
> program is loaded or unloaded, we will build the rxq_info whether or not
> XDP is in use. This is the more common use pattern and better supports
> future conversion to page_pool. Since the rxq_info wants the napi_id
> we re-order things slightly to tie this into the queue init and deinit
> functions where we do the add and delete of napi.
>
> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
> ---
> .../net/ethernet/pensando/ionic/ionic_lif.c | 55 +++++++------------
> 1 file changed, 19 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> index 0fba2df33915..4a7763ec061f 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
> @@ -46,8 +46,9 @@ static int ionic_start_queues(struct ionic_lif *lif);
> static void ionic_stop_queues(struct ionic_lif *lif);
> static void ionic_lif_queue_identify(struct ionic_lif *lif);
>
> -static int ionic_xdp_queues_config(struct ionic_lif *lif);
> -static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q);
> +static void ionic_xdp_queues_config(struct ionic_lif *lif);
> +static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id);
> +static void ionic_unregister_rxq_info(struct ionic_queue *q);
>
> static void ionic_dim_work(struct work_struct *work)
> {
> @@ -380,6 +381,7 @@ static void ionic_lif_qcq_deinit(struct ionic_lif *lif, struct ionic_qcq *qcq)
> if (!(qcq->flags & IONIC_QCQ_F_INITED))
> return;
>
> + ionic_unregister_rxq_info(&qcq->q);
> if (qcq->flags & IONIC_QCQ_F_INTR) {
> ionic_intr_mask(idev->intr_ctrl, qcq->intr.index,
> IONIC_INTR_MASK_SET);
> @@ -437,9 +439,7 @@ static void ionic_qcq_free(struct ionic_lif *lif, struct ionic_qcq *qcq)
> qcq->sg_base_pa = 0;
> }
>
> - ionic_xdp_unregister_rxq_info(&qcq->q);
> ionic_qcq_intr_free(lif, qcq);
> -
> vfree(qcq->q.info);
> qcq->q.info = NULL;
> }
> @@ -925,6 +925,11 @@ static int ionic_lif_rxq_init(struct ionic_lif *lif, struct ionic_qcq *qcq)
> netif_napi_add(lif->netdev, &qcq->napi, ionic_rx_napi);
> else
> netif_napi_add(lif->netdev, &qcq->napi, ionic_txrx_napi);
> + err = ionic_register_rxq_info(q, qcq->napi.napi_id);
> + if (err) {
> + netif_napi_del(&qcq->napi);
> + return err;
> + }
>
> qcq->flags |= IONIC_QCQ_F_INITED;
>
> @@ -2143,9 +2148,7 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
> int derr = 0;
> int i, err;
>
> - err = ionic_xdp_queues_config(lif);
> - if (err)
> - return err;
> + ionic_xdp_queues_config(lif);
>
> for (i = 0; i < lif->nxqs; i++) {
> if (!(lif->rxqcqs[i] && lif->txqcqs[i])) {
> @@ -2192,8 +2195,6 @@ static int ionic_txrx_enable(struct ionic_lif *lif)
> derr = ionic_qcq_disable(lif, lif->rxqcqs[i], derr);
> }
>
> - ionic_xdp_queues_config(lif);
> -
> return err;
> }
>
> @@ -2651,7 +2652,7 @@ static void ionic_vf_attr_replay(struct ionic_lif *lif)
> ionic_vf_start(ionic);
> }
>
> -static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q)
> +static void ionic_unregister_rxq_info(struct ionic_queue *q)
> {
> struct xdp_rxq_info *xi;
>
> @@ -2665,7 +2666,7 @@ static void ionic_xdp_unregister_rxq_info(struct ionic_queue *q)
> kfree(xi);
> }
>
> -static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
> +static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
> {
> struct xdp_rxq_info *rxq_info;
> int err;
> @@ -2698,45 +2699,27 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
> return err;
> }
>
> -static int ionic_xdp_queues_config(struct ionic_lif *lif)
> +static void ionic_xdp_queues_config(struct ionic_lif *lif)
I think this function should also get a new name that would reflect the fact
that it is just updating the XDP prog on rings. Also, ionic_xdp_queues_config
sounds a little bit like configuring XDP_TX/xdp_xmit queues, but here we have rx
queues only.
> {
> struct bpf_prog *xdp_prog;
> unsigned int i;
> - int err;
>
> if (!lif->rxqcqs)
> - return 0;
> + return;
>
> - /* There's no need to rework memory if not going to/from NULL program. */
> + /* Nothing to do 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;
> + return;
>
> for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
> struct ionic_queue *q = &lif->rxqcqs[i]->q;
>
> - if (q->xdp_prog) {
> - ionic_xdp_unregister_rxq_info(q);
> + if (q->xdp_prog)
> q->xdp_prog = NULL;
> - continue;
> - }
> -
> - err = ionic_xdp_register_rxq_info(q, lif->rxqcqs[i]->napi.napi_id);
> - if (err) {
> - dev_err(lif->ionic->dev, "failed to register RX queue %d info for XDP, err %d\n",
> - i, err);
> - goto err_out;
> - }
> - q->xdp_prog = xdp_prog;
> + else
> + q->xdp_prog = xdp_prog;
> }
> -
> - return 0;
> -
> -err_out:
> - for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++)
> - ionic_xdp_unregister_rxq_info(&lif->rxqcqs[i]->q);
> -
> - return err;
> }
>
> static int ionic_xdp_config(struct net_device *netdev, struct netdev_bpf *bpf)
> --
> 2.17.1
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool
2024-08-26 18:44 ` [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
@ 2024-08-27 19:59 ` kernel test robot
2024-08-27 22:22 ` kernel test robot
1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2024-08-27 19:59 UTC (permalink / raw)
To: Brett Creeley, netdev, davem, kuba, edumazet, pabeni
Cc: oe-kbuild-all, shannon.nelson, brett.creeley
Hi Brett,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Brett-Creeley/ionic-debug-line-for-Tx-completion-errors/20240827-024626
base: net-next/main
patch link: https://lore.kernel.org/r/20240826184422.21895-6-brett.creeley%40amd.com
patch subject: [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool
config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20240828/202408280318.gE2lxcpO-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408280318.gE2lxcpO-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408280318.gE2lxcpO-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/kernel.h:28,
from include/linux/skbuff.h:13,
from include/linux/ip.h:16,
from drivers/net/ethernet/pensando/ionic/ionic_txrx.c:4:
drivers/net/ethernet/pensando/ionic/ionic_txrx.c: In function 'ionic_rx_build_skb':
>> include/linux/minmax.h:93:37: warning: conversion from 'long unsigned int' to 'u16' {aka 'short unsigned int'} changes value from '65536' to '0' [-Woverflow]
93 | ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
| ^
include/linux/minmax.h:96:9: note: in expansion of macro '__cmp_once_unique'
96 | __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ^~~~~~~~~~~~~~~~~
include/linux/minmax.h:213:27: note: in expansion of macro '__cmp_once'
213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
| ^~~~~~~~~~
drivers/net/ethernet/pensando/ionic/ionic_txrx.c:203:28: note: in expansion of macro 'min_t'
203 | frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
| ^~~~~
drivers/net/ethernet/pensando/ionic/ionic_txrx.c: In function 'ionic_rx_fill':
>> include/linux/minmax.h:93:37: warning: conversion from 'long unsigned int' to 'u16' {aka 'short unsigned int'} changes value from '65536' to '0' [-Woverflow]
93 | ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
| ^
include/linux/minmax.h:96:9: note: in expansion of macro '__cmp_once_unique'
96 | __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ^~~~~~~~~~~~~~~~~
include/linux/minmax.h:213:27: note: in expansion of macro '__cmp_once'
213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
| ^~~~~~~~~~
drivers/net/ethernet/pensando/ionic/ionic_txrx.c:797:34: note: in expansion of macro 'min_t'
797 | first_frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
| ^~~~~
>> include/linux/minmax.h:93:37: warning: conversion from 'long unsigned int' to 'u16' {aka 'short unsigned int'} changes value from '65536' to '0' [-Woverflow]
93 | ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
| ^
include/linux/minmax.h:96:9: note: in expansion of macro '__cmp_once_unique'
96 | __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ^~~~~~~~~~~~~~~~~
include/linux/minmax.h:213:27: note: in expansion of macro '__cmp_once'
213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
| ^~~~~~~~~~
drivers/net/ethernet/pensando/ionic/ionic_txrx.c:832:36: note: in expansion of macro 'min_t'
832 | frag_len = min_t(u16, remain_len, IONIC_PAGE_SIZE);
| ^~~~~
vim +93 include/linux/minmax.h
d03eba99f5bf7c David Laight 2023-09-18 91
017fa3e8918784 Linus Torvalds 2024-07-28 92 #define __cmp_once_unique(op, type, x, y, ux, uy) \
017fa3e8918784 Linus Torvalds 2024-07-28 @93 ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
017fa3e8918784 Linus Torvalds 2024-07-28 94
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog
2024-08-27 11:44 ` Larysa Zaremba
2024-08-27 11:57 ` Larysa Zaremba
@ 2024-08-27 21:40 ` Brett Creeley
1 sibling, 0 replies; 14+ messages in thread
From: Brett Creeley @ 2024-08-27 21:40 UTC (permalink / raw)
To: Larysa Zaremba, Brett Creeley
Cc: netdev, davem, kuba, edumazet, pabeni, shannon.nelson
On 8/27/2024 4:44 AM, Larysa Zaremba wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
>> From: Shannon Nelson <shannon.nelson@amd.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@amd.com>
>> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
>
> Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.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.
Sure, will do. Thanks for the review.
Brett
<snip>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool
2024-08-26 18:44 ` [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
2024-08-27 19:59 ` kernel test robot
@ 2024-08-27 22:22 ` kernel test robot
1 sibling, 0 replies; 14+ messages in thread
From: kernel test robot @ 2024-08-27 22:22 UTC (permalink / raw)
To: Brett Creeley, netdev, davem, kuba, edumazet, pabeni
Cc: llvm, oe-kbuild-all, shannon.nelson, brett.creeley
Hi Brett,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Brett-Creeley/ionic-debug-line-for-Tx-completion-errors/20240827-024626
base: net-next/main
patch link: https://lore.kernel.org/r/20240826184422.21895-6-brett.creeley%40amd.com
patch subject: [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool
config: powerpc-allyesconfig (https://download.01.org/0day-ci/archive/20240828/202408280648.f6Hb9cX3-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 08e5a1de8227512d4774a534b91cb2353cef6284)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240828/202408280648.f6Hb9cX3-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202408280648.f6Hb9cX3-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from drivers/net/ethernet/pensando/ionic/ionic_txrx.c:4:
In file included from include/linux/ip.h:16:
In file included from include/linux/skbuff.h:17:
In file included from include/linux/bvec.h:10:
In file included from include/linux/highmem.h:8:
In file included from include/linux/cacheflush.h:5:
In file included from arch/powerpc/include/asm/cacheflush.h:7:
In file included from include/linux/mm.h:2228:
include/linux/vmstat.h:503:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
503 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
504 | item];
| ~~~~
include/linux/vmstat.h:510:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
510 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
511 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
include/linux/vmstat.h:517:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
517 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
include/linux/vmstat.h:523:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
523 | return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~ ^
524 | NR_VM_NUMA_EVENT_ITEMS +
| ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/net/ethernet/pensando/ionic/ionic_txrx.c:203:30: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 65536 to 0 [-Wconstant-conversion]
203 | frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
| ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
drivers/net/ethernet/pensando/ionic/ionic_dev.h:184:28: note: expanded from macro 'IONIC_PAGE_SIZE'
184 | #define IONIC_PAGE_SIZE PAGE_SIZE
| ^
arch/powerpc/include/asm/page.h:25:34: note: expanded from macro 'PAGE_SIZE'
25 | #define PAGE_SIZE (ASM_CONST(1) << PAGE_SHIFT)
| ^
include/linux/minmax.h:213:52: note: expanded from macro 'min_t'
213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~
include/linux/minmax.h:96:33: note: expanded from macro '__cmp_once'
96 | __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:93:31: note: expanded from macro '__cmp_once_unique'
93 | ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
| ~~ ^
drivers/net/ethernet/pensando/ionic/ionic_txrx.c:797:36: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 65536 to 0 [-Wconstant-conversion]
797 | first_frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
| ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
drivers/net/ethernet/pensando/ionic/ionic_dev.h:184:28: note: expanded from macro 'IONIC_PAGE_SIZE'
184 | #define IONIC_PAGE_SIZE PAGE_SIZE
| ^
arch/powerpc/include/asm/page.h:25:34: note: expanded from macro 'PAGE_SIZE'
25 | #define PAGE_SIZE (ASM_CONST(1) << PAGE_SHIFT)
| ^
include/linux/minmax.h:213:52: note: expanded from macro 'min_t'
213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~
include/linux/minmax.h:96:33: note: expanded from macro '__cmp_once'
96 | __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:93:31: note: expanded from macro '__cmp_once_unique'
93 | ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
| ~~ ^
drivers/net/ethernet/pensando/ionic/ionic_txrx.c:832:38: warning: implicit conversion from 'unsigned long' to 'u16' (aka 'unsigned short') changes value from 65536 to 0 [-Wconstant-conversion]
832 | frag_len = min_t(u16, remain_len, IONIC_PAGE_SIZE);
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
drivers/net/ethernet/pensando/ionic/ionic_dev.h:184:28: note: expanded from macro 'IONIC_PAGE_SIZE'
184 | #define IONIC_PAGE_SIZE PAGE_SIZE
| ^
arch/powerpc/include/asm/page.h:25:34: note: expanded from macro 'PAGE_SIZE'
25 | #define PAGE_SIZE (ASM_CONST(1) << PAGE_SHIFT)
| ^
include/linux/minmax.h:213:52: note: expanded from macro 'min_t'
213 | #define min_t(type, x, y) __cmp_once(min, type, x, y)
| ~~~~~~~~~~~~~~~~~~~~~~~~~^~
include/linux/minmax.h:96:33: note: expanded from macro '__cmp_once'
96 | __cmp_once_unique(op, type, x, y, __UNIQUE_ID(x_), __UNIQUE_ID(y_))
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:93:31: note: expanded from macro '__cmp_once_unique'
93 | ({ type ux = (x); type uy = (y); __cmp(op, ux, uy); })
| ~~ ^
7 warnings generated.
vim +203 drivers/net/ethernet/pensando/ionic/ionic_txrx.c
174
175 static struct sk_buff *ionic_rx_build_skb(struct ionic_queue *q,
176 struct ionic_rx_desc_info *desc_info,
177 unsigned int headroom,
178 unsigned int len,
179 unsigned int num_sg_elems,
180 bool synced)
181 {
182 struct ionic_buf_info *buf_info;
183 struct sk_buff *skb;
184 unsigned int i;
185 u16 frag_len;
186
187 buf_info = &desc_info->bufs[0];
188 prefetchw(buf_info->page);
189
190 skb = napi_get_frags(&q_to_qcq(q)->napi);
191 if (unlikely(!skb)) {
192 net_warn_ratelimited("%s: SKB alloc failed on %s!\n",
193 dev_name(q->dev), q->name);
194 q_to_rx_stats(q)->alloc_err++;
195 return NULL;
196 }
197 skb_mark_for_recycle(skb);
198
199 if (headroom)
200 frag_len = min_t(u16, len,
201 IONIC_XDP_MAX_LINEAR_MTU + VLAN_ETH_HLEN);
202 else
> 203 frag_len = min_t(u16, len, IONIC_PAGE_SIZE);
204
205 if (unlikely(!buf_info->page))
206 goto err_bad_buf_page;
207 ionic_rx_add_skb_frag(q, skb, buf_info, headroom, frag_len, synced);
208 len -= frag_len;
209 buf_info++;
210
211 for (i = 0; i < num_sg_elems; i++, buf_info++) {
212 if (unlikely(!buf_info->page))
213 goto err_bad_buf_page;
214 frag_len = min_t(u16, len, buf_info->len);
215 ionic_rx_add_skb_frag(q, skb, buf_info, 0, frag_len, synced);
216 len -= frag_len;
217 }
218
219 return skb;
220
221 err_bad_buf_page:
222 dev_kfree_skb(skb);
223 return NULL;
224 }
225
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog
2024-08-27 11:57 ` Larysa Zaremba
@ 2024-08-27 22:27 ` Brett Creeley
0 siblings, 0 replies; 14+ messages in thread
From: Brett Creeley @ 2024-08-27 22:27 UTC (permalink / raw)
To: Larysa Zaremba, Brett Creeley
Cc: netdev, davem, kuba, edumazet, pabeni, shannon.nelson
On 8/27/2024 4:57 AM, Larysa Zaremba wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Tue, Aug 27, 2024 at 01:44:10PM +0200, Larysa Zaremba wrote:
>> On Mon, Aug 26, 2024 at 11:44:20AM -0700, Brett Creeley wrote:
>>> From: Shannon Nelson <shannon.nelson@amd.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@amd.com>
>>> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
>>
>> Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>
>>
>
> I would like to rewoke the tag, see below why.
>
>> 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;
>
> In a case when we replace a non-NULL program with another non-NULL program this
> would create a situation where lif and queues have different pointers on them.
Yeah, you are right. Good catch. We will get this fixed up in the next
version.
Thanks,
Brett
>
>>>
>>> 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;
>
> [...]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 net-next 4/5] ionic: always use rxq_info
2024-08-27 12:08 ` Larysa Zaremba
@ 2024-08-27 22:32 ` Brett Creeley
0 siblings, 0 replies; 14+ messages in thread
From: Brett Creeley @ 2024-08-27 22:32 UTC (permalink / raw)
To: Larysa Zaremba, Brett Creeley
Cc: netdev, davem, kuba, edumazet, pabeni, shannon.nelson
On 8/27/2024 5:08 AM, Larysa Zaremba wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Mon, Aug 26, 2024 at 11:44:21AM -0700, Brett Creeley wrote:
>> From: Shannon Nelson <shannon.nelson@amd.com>
>>
>> Instead of setting up and tearing down the rxq_info only when the XDP
>> program is loaded or unloaded, we will build the rxq_info whether or not
>> XDP is in use. This is the more common use pattern and better supports
>> future conversion to page_pool. Since the rxq_info wants the napi_id
>> we re-order things slightly to tie this into the queue init and deinit
>> functions where we do the add and delete of napi.
>>
>> Signed-off-by: Shannon Nelson <shannon.nelson@amd.com>
>> Signed-off-by: Brett Creeley <brett.creeley@amd.com>
>> ---
>> .../net/ethernet/pensando/ionic/ionic_lif.c | 55 +++++++------------
>> 1 file changed, 19 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_lif.c b/drivers/net/ethernet/pensando/ionic/ionic_lif.c
<snip>
>> -static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
>> +static int ionic_register_rxq_info(struct ionic_queue *q, unsigned int napi_id)
>> {
>> struct xdp_rxq_info *rxq_info;
>> int err;
>> @@ -2698,45 +2699,27 @@ static int ionic_xdp_register_rxq_info(struct ionic_queue *q, unsigned int napi_
>> return err;
>> }
>>
>> -static int ionic_xdp_queues_config(struct ionic_lif *lif)
>> +static void ionic_xdp_queues_config(struct ionic_lif *lif)
>
> I think this function should also get a new name that would reflect the fact
> that it is just updating the XDP prog on rings. Also, ionic_xdp_queues_config
> sounds a little bit like configuring XDP_TX/xdp_xmit queues, but here we have rx
> queues only.
Yeah that makes sense. We can rename it to something closer to the
function's contents/purpose.
Thanks,
Brett
>
>> {
>> struct bpf_prog *xdp_prog;
>> unsigned int i;
>> - int err;
>>
>> if (!lif->rxqcqs)
>> - return 0;
>> + return;
>>
>> - /* There's no need to rework memory if not going to/from NULL program. */
>> + /* Nothing to do 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;
>> + return;
>>
>> for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++) {
>> struct ionic_queue *q = &lif->rxqcqs[i]->q;
>>
>> - if (q->xdp_prog) {
>> - ionic_xdp_unregister_rxq_info(q);
>> + if (q->xdp_prog)
>> q->xdp_prog = NULL;
>> - continue;
>> - }
>> -
>> - err = ionic_xdp_register_rxq_info(q, lif->rxqcqs[i]->napi.napi_id);
>> - if (err) {
>> - dev_err(lif->ionic->dev, "failed to register RX queue %d info for XDP, err %d\n",
>> - i, err);
>> - goto err_out;
>> - }
>> - q->xdp_prog = xdp_prog;
>> + else
>> + q->xdp_prog = xdp_prog;
>> }
>> -
>> - return 0;
>> -
>> -err_out:
>> - for (i = 0; i < lif->ionic->nrxqs_per_lif && lif->rxqcqs[i]; i++)
>> - ionic_xdp_unregister_rxq_info(&lif->rxqcqs[i]->q);
>> -
>> - return err;
>> }
>>
>> static int ionic_xdp_config(struct net_device *netdev, struct netdev_bpf *bpf)
>> --
>> 2.17.1
>>
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-08-27 22:32 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-26 18:44 [PATCH v2 net-next 0/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 1/5] ionic: debug line for Tx completion errors Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 2/5] ionic: rename ionic_xdp_rx_put_bufs Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 3/5] ionic: use per-queue xdp_prog Brett Creeley
2024-08-27 11:44 ` Larysa Zaremba
2024-08-27 11:57 ` Larysa Zaremba
2024-08-27 22:27 ` Brett Creeley
2024-08-27 21:40 ` Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 4/5] ionic: always use rxq_info Brett Creeley
2024-08-27 12:08 ` Larysa Zaremba
2024-08-27 22:32 ` Brett Creeley
2024-08-26 18:44 ` [PATCH v2 net-next 5/5] ionic: convert Rx queue buffers to use page_pool Brett Creeley
2024-08-27 19:59 ` kernel test robot
2024-08-27 22:22 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).