[v4] net/gve: allocate RX QPL pages using malloc
Checks
Commit Message
From: Praveen Kaligineedi <pkaligineedi@google.com>
Allocating QPL for an RX queue might fail if enough contiguous IOVA
memory cannot be allocated. This can commonly occur when using 2MB huge
pages because the 1024 4K buffers are allocated for each RX ring by
default, resulting in 4MB for each ring. However, the only requirement
for RX QPLs is that each 4K buffer be IOVA contiguous, not the entire
QPL. Therefore, malloc will be used to allocate RX QPLs instead.
Note that TX queues require the entire QPL to be IOVA contiguous, so it
will continue to use the memzone-based allocation.
v2: Updated RX path to use malloc exclusively
v3: Changed commit description to match updated code
v4: Add fixes tag to allow 2M hugepages to be used on older versions of
DPDK
Fixes: a46583cf43c8 ("net/gve: support Rx/Tx")
Cc: junfeng.guo@intel.com
Cc: stable@dpdk.org
Signed-off-by: Praveen Kaligineedi <pkaligineedi@google.com>
Signed-off-by: Joshua Washington <joshwash@google.com>
---
drivers/net/gve/gve_ethdev.c | 102 ++++++++++++++++++++++++++++-------
drivers/net/gve/gve_ethdev.h | 5 +-
drivers/net/gve/gve_rx.c | 2 +-
3 files changed, 89 insertions(+), 20 deletions(-)
Comments
Hello,
I wish for this fix to be backported to stable release 22.11 and 23.11
for DPDK. However, the patch as it is does not apply cleanly to either
release. What is the recommended way to include this patch in older
releases of DPDK? Should I create separate stable backport patches
that apply cleanly to each of the stable releases? Or will this issue
be resolved once stable maintainers attempt to apply the patch on
older releases?
Thanks,
Josh
On Thu, 9 Jan 2025 12:05:43 -0800
Joshua Washington <joshwash@google.com> wrote:
> Hello,
>
> I wish for this fix to be backported to stable release 22.11 and 23.11
> for DPDK. However, the patch as it is does not apply cleanly to either
> release. What is the recommended way to include this patch in older
> releases of DPDK? Should I create separate stable backport patches
> that apply cleanly to each of the stable releases? Or will this issue
> be resolved once stable maintainers attempt to apply the patch on
> older releases?
>
> Thanks,
> Josh
1. You must wait until it accepted in to main branch for an upcoming release.
2. Make sure it you add Fixes tag and Cc stable@dpdk.org
The stable maintainers pick it up from there.
3. If patch does not apply cleanly, they send out a "help with backporting" mail
before release with instructions.
On Thu, 9 Jan 2025 11:46:38 -0800
Joshua Washington <joshwash@google.com> wrote:
> Kaligineedi <pkaligineedi@google.com>
>
> Allocating QPL for an RX queue might fail if enough contiguous IOVA
> memory cannot be allocated. This can commonly occur when using 2MB huge
> pages because the 1024 4K buffers are allocated for each RX ring by
> default, resulting in 4MB for each ring. However, the only requirement
> for RX QPLs is that each 4K buffer be IOVA contiguous, not the entire
> QPL. Therefore, malloc will be used to allocate RX QPLs instead.
>
> Note that TX queues require the entire QPL to be IOVA contiguous, so it
> will continue to use the memzone-based allocation.
>
> v2: Updated RX path to use malloc exclusively
>
> v3: Changed commit description to match updated code
>
> v4: Add fixes tag to allow 2M hugepages to be used on older versions of
> DPDK
>
> Fixes: a46583cf43c8 ("net/gve: support Rx/Tx")
> Cc: junfeng.guo@intel.com
> Cc: stable@dpdk.org
>
> Signed-off-by: Praveen Kaligineedi <pkaligineedi@google.com>
> Signed-off-by: Joshua Washington <joshwash@google.com>
> ---
The standard practice is to put revision information below the cut line "---"
so it doesn't end up in the commit message.
I fixed that and applied it to next-net
@@ -22,42 +22,97 @@ gve_write_version(uint8_t *driver_version_register)
writeb('\n', driver_version_register);
}
+static const struct rte_memzone *
+gve_alloc_using_mz(const char *name, uint32_t num_pages)
+{
+ const struct rte_memzone *mz;
+ mz = rte_memzone_reserve_aligned(name, num_pages * PAGE_SIZE,
+ rte_socket_id(),
+ RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
+ if (mz == NULL)
+ PMD_DRV_LOG(ERR, "Failed to alloc memzone %s.", name);
+ return mz;
+}
+
+static int
+gve_alloc_using_malloc(void **bufs, uint32_t num_entries)
+{
+ uint32_t i;
+
+ for (i = 0; i < num_entries; i++) {
+ bufs[i] = rte_malloc_socket(NULL, PAGE_SIZE, PAGE_SIZE, rte_socket_id());
+ if (bufs[i] == NULL) {
+ PMD_DRV_LOG(ERR, "Failed to malloc");
+ goto free_bufs;
+ }
+ }
+ return 0;
+
+free_bufs:
+ while (i > 0)
+ rte_free(bufs[--i]);
+
+ return -ENOMEM;
+}
+
static struct gve_queue_page_list *
-gve_alloc_queue_page_list(const char *name, uint32_t num_pages)
+gve_alloc_queue_page_list(const char *name, uint32_t num_pages, bool is_rx)
{
struct gve_queue_page_list *qpl;
const struct rte_memzone *mz;
- dma_addr_t page_bus;
uint32_t i;
qpl = rte_zmalloc("qpl struct", sizeof(struct gve_queue_page_list), 0);
if (!qpl)
return NULL;
- mz = rte_memzone_reserve_aligned(name, num_pages * PAGE_SIZE,
- rte_socket_id(),
- RTE_MEMZONE_IOVA_CONTIG, PAGE_SIZE);
- if (mz == NULL) {
- PMD_DRV_LOG(ERR, "Failed to alloc %s.", name);
- goto free_qpl_struct;
- }
qpl->page_buses = rte_zmalloc("qpl page buses",
num_pages * sizeof(dma_addr_t), 0);
if (qpl->page_buses == NULL) {
PMD_DRV_LOG(ERR, "Failed to alloc qpl page buses");
- goto free_qpl_memzone;
+ goto free_qpl_struct;
}
- page_bus = mz->iova;
- for (i = 0; i < num_pages; i++) {
- qpl->page_buses[i] = page_bus;
- page_bus += PAGE_SIZE;
+
+ if (is_rx) {
+ /* RX QPL need not be IOVA contiguous.
+ * Allocate 4K size buffers using malloc
+ */
+ qpl->qpl_bufs = rte_zmalloc("qpl bufs",
+ num_pages * sizeof(void *), 0);
+ if (qpl->qpl_bufs == NULL) {
+ PMD_DRV_LOG(ERR, "Failed to alloc qpl bufs");
+ goto free_qpl_page_buses;
+ }
+
+ if (gve_alloc_using_malloc(qpl->qpl_bufs, num_pages))
+ goto free_qpl_page_bufs;
+
+ /* Populate the IOVA addresses */
+ for (i = 0; i < num_pages; i++)
+ qpl->page_buses[i] =
+ rte_malloc_virt2iova(qpl->qpl_bufs[i]);
+ } else {
+ /* TX QPL needs to be IOVA contiguous
+ * Allocate QPL using memzone
+ */
+ mz = gve_alloc_using_mz(name, num_pages);
+ if (!mz)
+ goto free_qpl_page_buses;
+
+ qpl->mz = mz;
+
+ /* Populate the IOVA addresses */
+ for (i = 0; i < num_pages; i++)
+ qpl->page_buses[i] = mz->iova + i * PAGE_SIZE;
}
- qpl->mz = mz;
+
qpl->num_entries = num_pages;
return qpl;
-free_qpl_memzone:
- rte_memzone_free(qpl->mz);
+free_qpl_page_bufs:
+ rte_free(qpl->qpl_bufs);
+free_qpl_page_buses:
+ rte_free(qpl->page_buses);
free_qpl_struct:
rte_free(qpl);
return NULL;
@@ -69,7 +124,18 @@ gve_free_queue_page_list(struct gve_queue_page_list *qpl)
if (qpl->mz) {
rte_memzone_free(qpl->mz);
qpl->mz = NULL;
+ } else if (qpl->qpl_bufs) {
+ uint32_t i;
+
+ for (i = 0; i < qpl->num_entries; i++)
+ rte_free(qpl->qpl_bufs[i]);
+ }
+
+ if (qpl->qpl_bufs) {
+ rte_free(qpl->qpl_bufs);
+ qpl->qpl_bufs = NULL;
}
+
if (qpl->page_buses) {
rte_free(qpl->page_buses);
qpl->page_buses = NULL;
@@ -89,7 +155,7 @@ gve_setup_queue_page_list(struct gve_priv *priv, uint16_t queue_id, bool is_rx,
/* Allocate a new QPL. */
snprintf(qpl_name, sizeof(qpl_name), "gve_%s_%s_qpl%d",
priv->pci_dev->device.name, queue_type_string, queue_id);
- qpl = gve_alloc_queue_page_list(qpl_name, num_pages);
+ qpl = gve_alloc_queue_page_list(qpl_name, num_pages, is_rx);
if (!qpl) {
PMD_DRV_LOG(ERR,
"Failed to alloc %s qpl for queue %hu.",
@@ -62,7 +62,10 @@ struct gve_queue_page_list {
uint32_t id; /* unique id */
uint32_t num_entries;
dma_addr_t *page_buses; /* the dma addrs of the pages */
- const struct rte_memzone *mz;
+ union {
+ const struct rte_memzone *mz; /* memzone allocated for TX queue */
+ void **qpl_bufs; /* RX qpl-buffer list allocated using malloc*/
+ };
};
/* A TX desc ring entry */
@@ -117,7 +117,7 @@ gve_rx_mbuf(struct gve_rx_queue *rxq, struct rte_mbuf *rxe, uint16_t len,
rxq->ctx.mbuf_tail = rxe;
}
if (rxq->is_gqi_qpl) {
- addr = (uint64_t)(rxq->qpl->mz->addr) + rx_id * PAGE_SIZE + padding;
+ addr = (uint64_t)rxq->qpl->qpl_bufs[rx_id] + padding;
rte_memcpy((void *)((size_t)rxe->buf_addr + rxe->data_off),
(void *)(size_t)addr, len);
}