2022-10-19 14:36:30

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 0/5] net: lan966x: Add xdp support

Add support for xdp in lan966x driver. Currently on XDP_PASS and
XDP_DROP are supported.

The first 3 patches are just moving things around just to simplify
the code for when the xdp is added.
Patch 4 actually adds the xdp. Currently the only supported actions
are XDP_PASS and XDP_DROP. In the future this will be extended with
XDP_TX and XDP_REDIRECT.
Patch 5 changes to use page pool API, because the handling of the
pages is similar with what already lan966x driver is doing. In this
way is possible to remove some of the code.

All these changes give a small improvement on the RX side:
Before:
iperf3 -c 10.96.10.1 -R
[ 5] 0.00-10.01 sec 514 MBytes 430 Mbits/sec 0 sender
[ 5] 0.00-10.00 sec 509 MBytes 427 Mbits/sec receiver

After:
iperf3 -c 10.96.10.1 -R
[ 5] 0.00-10.02 sec 540 MBytes 452 Mbits/sec 0 sender
[ 5] 0.00-10.01 sec 537 MBytes 450 Mbits/sec receiver


Horatiu Vultur (5):
net: lan966x: Add define IFH_LEN_BYTES
net: lan966x: Rename lan966x_fdma_get_max_mtu
net: lan966x: Split function lan966x_fdma_rx_get_frame
net: lan966x: Add basic XDP support
net: lan96x: Use page_pool API

.../net/ethernet/microchip/lan966x/Kconfig | 1 +
.../net/ethernet/microchip/lan966x/Makefile | 3 +-
.../ethernet/microchip/lan966x/lan966x_fdma.c | 194 +++++++++++-------
.../ethernet/microchip/lan966x/lan966x_ifh.h | 1 +
.../ethernet/microchip/lan966x/lan966x_main.c | 26 ++-
.../ethernet/microchip/lan966x/lan966x_main.h | 28 +++
.../ethernet/microchip/lan966x/lan966x_xdp.c | 100 +++++++++
7 files changed, 275 insertions(+), 78 deletions(-)
create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c

--
2.38.0


2022-10-19 14:44:55

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 3/5] net: lan966x: Split function lan966x_fdma_rx_get_frame

The function lan966x_fdma_rx_get_frame was unmapping the frame from
device and check also if the frame was received on a valid port. And
only after that it tried to generate the skb.
Move this check in a different function, in preparation for xdp
support. Such that xdp to be added here and the
lan966x_fdma_rx_get_frame to be used only when giving the skb to upper
layers.

Signed-off-by: Horatiu Vultur <[email protected]>
---
.../ethernet/microchip/lan966x/lan966x_fdma.c | 86 ++++++++++++++-----
.../ethernet/microchip/lan966x/lan966x_main.h | 9 ++
2 files changed, 72 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
index e0b9cd289994f..6b6baf6a3d1ee 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
@@ -54,6 +54,17 @@ static void lan966x_fdma_rx_free_pages(struct lan966x_rx *rx)
}
}

+static void lan966x_fdma_rx_free_page(struct lan966x_rx *rx)
+{
+ struct page *page;
+
+ page = rx->page[rx->dcb_index][rx->db_index];
+ if (unlikely(!page))
+ return;
+
+ __free_pages(page, rx->page_order);
+}
+
static void lan966x_fdma_rx_add_dcb(struct lan966x_rx *rx,
struct lan966x_rx_dcb *dcb,
u64 nextptr)
@@ -116,6 +127,12 @@ static int lan966x_fdma_rx_alloc(struct lan966x_rx *rx)
return 0;
}

+static void lan966x_fdma_rx_advance_dcb(struct lan966x_rx *rx)
+{
+ rx->dcb_index++;
+ rx->dcb_index &= FDMA_DCB_MAX - 1;
+}
+
static void lan966x_fdma_rx_free(struct lan966x_rx *rx)
{
struct lan966x *lan966x = rx->lan966x;
@@ -402,32 +419,50 @@ static bool lan966x_fdma_rx_more_frames(struct lan966x_rx *rx)
return true;
}

-static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx)
+static int lan966x_fdma_rx_check_frame(struct lan966x_rx *rx, u64 *src_port)
{
struct lan966x *lan966x = rx->lan966x;
- u64 src_port, timestamp;
struct lan966x_db *db;
- struct sk_buff *skb;
struct page *page;

- /* Get the received frame and unmap it */
db = &rx->dcbs[rx->dcb_index].db[rx->db_index];
page = rx->page[rx->dcb_index][rx->db_index];
- skb = build_skb(page_address(page), PAGE_SIZE << rx->page_order);
- if (unlikely(!skb))
- goto unmap_page;
+ if (unlikely(!page))
+ return FDMA_RX_ERROR;

dma_unmap_single(lan966x->dev, (dma_addr_t)db->dataptr,
FDMA_DCB_STATUS_BLOCKL(db->status),
DMA_FROM_DEVICE);
+
+ lan966x_ifh_get_src_port(page_address(page), src_port);
+
+ if (WARN_ON(*src_port >= lan966x->num_phys_ports))
+ return FDMA_RX_ERROR;
+
+ return FDMA_RX_OK;
+}
+
+static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx,
+ u64 src_port)
+{
+ struct lan966x *lan966x = rx->lan966x;
+ struct lan966x_db *db;
+ struct sk_buff *skb;
+ struct page *page;
+ u64 timestamp;
+
+ /* Get the received frame */
+ db = &rx->dcbs[rx->dcb_index].db[rx->db_index];
+ page = rx->page[rx->dcb_index][rx->db_index];
+
+ skb = build_skb(page_address(page), PAGE_SIZE << rx->page_order);
+ if (unlikely(!skb))
+ goto free_page;
+
skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status));

- lan966x_ifh_get_src_port(skb->data, &src_port);
lan966x_ifh_get_timestamp(skb->data, &timestamp);

- if (WARN_ON(src_port >= lan966x->num_phys_ports))
- goto free_skb;
-
skb->dev = lan966x->ports[src_port]->dev;
skb_pull(skb, IFH_LEN_BYTES);

@@ -450,12 +485,7 @@ static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx)

return skb;

-free_skb:
- kfree_skb(skb);
-unmap_page:
- dma_unmap_page(lan966x->dev, (dma_addr_t)db->dataptr,
- FDMA_DCB_STATUS_BLOCKL(db->status),
- DMA_FROM_DEVICE);
+free_page:
__free_pages(page, rx->page_order);

return NULL;
@@ -467,10 +497,12 @@ static int lan966x_fdma_napi_poll(struct napi_struct *napi, int weight)
struct lan966x_rx *rx = &lan966x->rx;
int dcb_reload = rx->dcb_index;
struct lan966x_rx_dcb *old_dcb;
+ enum lan966x_fdma_rx_error err;
struct lan966x_db *db;
struct sk_buff *skb;
struct page *page;
int counter = 0;
+ u64 src_port;
u64 nextptr;

lan966x_fdma_tx_clear_buf(lan966x, weight);
@@ -480,19 +512,27 @@ static int lan966x_fdma_napi_poll(struct napi_struct *napi, int weight)
if (!lan966x_fdma_rx_more_frames(rx))
break;

- skb = lan966x_fdma_rx_get_frame(rx);
+ counter++;

- rx->page[rx->dcb_index][rx->db_index] = NULL;
- rx->dcb_index++;
- rx->dcb_index &= FDMA_DCB_MAX - 1;
+ err = lan966x_fdma_rx_check_frame(rx, &src_port);
+ switch (err) {
+ case FDMA_RX_OK:
+ break;
+ case FDMA_RX_ERROR:
+ lan966x_fdma_rx_free_page(rx);
+ lan966x_fdma_rx_advance_dcb(rx);
+ goto allocate_new;
+ }

+ skb = lan966x_fdma_rx_get_frame(rx, src_port);
+ lan966x_fdma_rx_advance_dcb(rx);
if (!skb)
- break;
+ goto allocate_new;

napi_gro_receive(&lan966x->napi, skb);
- counter++;
}

+allocate_new:
/* Allocate new pages and map them */
while (dcb_reload != rx->dcb_index) {
db = &rx->dcbs[dcb_reload].db[rx->db_index];
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
index e05036841f825..608e2cb81b095 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
@@ -98,6 +98,15 @@ enum macaccess_entry_type {
ENTRYTYPE_MACV6,
};

+/* FDMA rx error codes for checking if the frame is valid
+ * FDMA_RX_OK, frame is valid and can be used
+ * FDMA_RX_ERROR, something went wrong, stop getting more frames
+ */
+enum lan966x_fdma_rx_error {
+ FDMA_RX_OK = 0,
+ FDMA_RX_ERROR,
+};
+
struct lan966x_port;

struct lan966x_db {
--
2.38.0

2022-10-19 15:09:04

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next 5/5] net: lan96x: Use page_pool API

Use the page_pool API for allocation, freeing and DMA handling instead
of dev_alloc_pages, __free_pages and dma_map_page.

Signed-off-by: Horatiu Vultur <[email protected]>
---
.../net/ethernet/microchip/lan966x/Kconfig | 1 +
.../ethernet/microchip/lan966x/lan966x_fdma.c | 74 +++++++++++--------
.../ethernet/microchip/lan966x/lan966x_main.h | 3 +
3 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/Kconfig b/drivers/net/ethernet/microchip/lan966x/Kconfig
index 49e1464a43139..b7ae5ce7d3f7a 100644
--- a/drivers/net/ethernet/microchip/lan966x/Kconfig
+++ b/drivers/net/ethernet/microchip/lan966x/Kconfig
@@ -7,5 +7,6 @@ config LAN966X_SWITCH
depends on BRIDGE || BRIDGE=n
select PHYLINK
select PACKING
+ select PAGE_POOL
help
This driver supports the Lan966x network switch device.
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
index 4e8f7e47c5536..ba34a7b7ccd13 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
@@ -10,47 +10,25 @@ static int lan966x_fdma_channel_active(struct lan966x *lan966x)
static struct page *lan966x_fdma_rx_alloc_page(struct lan966x_rx *rx,
struct lan966x_db *db)
{
- struct lan966x *lan966x = rx->lan966x;
- dma_addr_t dma_addr;
struct page *page;

- page = dev_alloc_pages(rx->page_order);
+ page = page_pool_dev_alloc_pages(rx->page_pool);
if (unlikely(!page))
return NULL;

- dma_addr = dma_map_page(lan966x->dev, page, 0,
- PAGE_SIZE << rx->page_order,
- DMA_FROM_DEVICE);
- if (unlikely(dma_mapping_error(lan966x->dev, dma_addr)))
- goto free_page;
-
- db->dataptr = dma_addr;
+ db->dataptr = page_pool_get_dma_addr(page);

return page;
-
-free_page:
- __free_pages(page, rx->page_order);
- return NULL;
}

static void lan966x_fdma_rx_free_pages(struct lan966x_rx *rx)
{
- struct lan966x *lan966x = rx->lan966x;
- struct lan966x_rx_dcb *dcb;
- struct lan966x_db *db;
int i, j;

for (i = 0; i < FDMA_DCB_MAX; ++i) {
- dcb = &rx->dcbs[i];
-
- for (j = 0; j < FDMA_RX_DCB_MAX_DBS; ++j) {
- db = &dcb->db[j];
- dma_unmap_single(lan966x->dev,
- (dma_addr_t)db->dataptr,
- PAGE_SIZE << rx->page_order,
- DMA_FROM_DEVICE);
- __free_pages(rx->page[i][j], rx->page_order);
- }
+ for (j = 0; j < FDMA_RX_DCB_MAX_DBS; ++j)
+ page_pool_put_full_page(rx->page_pool,
+ rx->page[i][j], false);
}
}

@@ -62,7 +40,7 @@ static void lan966x_fdma_rx_free_page(struct lan966x_rx *rx)
if (unlikely(!page))
return;

- __free_pages(page, rx->page_order);
+ page_pool_recycle_direct(rx->page_pool, page);
}

static void lan966x_fdma_rx_add_dcb(struct lan966x_rx *rx,
@@ -84,6 +62,27 @@ static void lan966x_fdma_rx_add_dcb(struct lan966x_rx *rx,
rx->last_entry = dcb;
}

+static int lan966x_fdma_rx_alloc_page_pool(struct lan966x_rx *rx)
+{
+ struct lan966x *lan966x = rx->lan966x;
+ struct page_pool_params pp_params = {
+ .order = rx->page_order,
+ .flags = PP_FLAG_DMA_MAP | PP_FLAG_DMA_SYNC_DEV,
+ .pool_size = FDMA_DCB_MAX,
+ .nid = NUMA_NO_NODE,
+ .dev = lan966x->dev,
+ .dma_dir = DMA_FROM_DEVICE,
+ .offset = 0,
+ .max_len = PAGE_SIZE << rx->page_order,
+ };
+
+ rx->page_pool = page_pool_create(&pp_params);
+ if (IS_ERR(rx->page_pool))
+ return PTR_ERR(rx->page_pool);
+
+ return 0;
+}
+
static int lan966x_fdma_rx_alloc(struct lan966x_rx *rx)
{
struct lan966x *lan966x = rx->lan966x;
@@ -93,6 +92,9 @@ static int lan966x_fdma_rx_alloc(struct lan966x_rx *rx)
int i, j;
int size;

+ if (lan966x_fdma_rx_alloc_page_pool(rx))
+ return PTR_ERR(rx->page_pool);
+
/* calculate how many pages are needed to allocate the dcbs */
size = sizeof(struct lan966x_rx_dcb) * FDMA_DCB_MAX;
size = ALIGN(size, PAGE_SIZE);
@@ -431,9 +433,9 @@ static int lan966x_fdma_rx_check_frame(struct lan966x_rx *rx, u64 *src_port)
if (unlikely(!page))
return FDMA_RX_ERROR;

- dma_unmap_single(lan966x->dev, (dma_addr_t)db->dataptr,
- FDMA_DCB_STATUS_BLOCKL(db->status),
- DMA_FROM_DEVICE);
+ dma_sync_single_for_cpu(lan966x->dev, (dma_addr_t)db->dataptr,
+ FDMA_DCB_STATUS_BLOCKL(db->status),
+ DMA_FROM_DEVICE);

lan966x_ifh_get_src_port(page_address(page), src_port);

@@ -464,6 +466,8 @@ static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx,
if (unlikely(!skb))
goto free_page;

+ skb_mark_for_recycle(skb);
+
skb_put(skb, FDMA_DCB_STATUS_BLOCKL(db->status));

lan966x_ifh_get_timestamp(skb->data, &timestamp);
@@ -491,7 +495,7 @@ static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx,
return skb;

free_page:
- __free_pages(page, rx->page_order);
+ page_pool_recycle_direct(rx->page_pool, page);

return NULL;
}
@@ -718,6 +722,7 @@ static int lan966x_qsys_sw_status(struct lan966x *lan966x)
static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
{
void *rx_dcbs, *tx_dcbs, *tx_dcbs_buf;
+ struct page_pool *page_pool;
dma_addr_t rx_dma, tx_dma;
u32 size;
int err;
@@ -728,6 +733,7 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
rx_dcbs = lan966x->rx.dcbs;
tx_dcbs = lan966x->tx.dcbs;
tx_dcbs_buf = lan966x->tx.dcbs_buf;
+ page_pool = lan966x->rx.page_pool;

napi_synchronize(&lan966x->napi);
napi_disable(&lan966x->napi);
@@ -745,6 +751,8 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
size = ALIGN(size, PAGE_SIZE);
dma_free_coherent(lan966x->dev, size, rx_dcbs, rx_dma);

+ page_pool_destroy(page_pool);
+
lan966x_fdma_tx_disable(&lan966x->tx);
err = lan966x_fdma_tx_alloc(&lan966x->tx);
if (err)
@@ -761,6 +769,7 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)

return err;
restore:
+ lan966x->rx.page_pool = page_pool;
lan966x->rx.dma = rx_dma;
lan966x->rx.dcbs = rx_dcbs;
lan966x_fdma_rx_start(&lan966x->rx);
@@ -872,5 +881,6 @@ void lan966x_fdma_deinit(struct lan966x *lan966x)

lan966x_fdma_rx_free_pages(&lan966x->rx);
lan966x_fdma_rx_free(&lan966x->rx);
+ page_pool_destroy(lan966x->rx.page_pool);
lan966x_fdma_tx_free(&lan966x->tx);
}
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
index e635e529df0d4..9abdc3fb3f7e5 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
@@ -9,6 +9,7 @@
#include <linux/phy.h>
#include <linux/phylink.h>
#include <linux/ptp_clock_kernel.h>
+#include <net/page_pool.h>
#include <net/pkt_cls.h>
#include <net/pkt_sched.h>
#include <net/switchdev.h>
@@ -160,6 +161,8 @@ struct lan966x_rx {
u8 page_order;

u8 channel_id;
+
+ struct page_pool *page_pool;
};

struct lan966x_tx_dcb_buf {
--
2.38.0

2022-10-20 20:55:25

by Horatiu Vultur

[permalink] [raw]
Subject: Re: [PATCH net-next 0/5] net: lan966x: Add xdp support

The 10/19/2022 15:50, Horatiu Vultur wrote:
> Add support for xdp in lan966x driver. Currently on XDP_PASS and
> XDP_DROP are supported.
>
> The first 3 patches are just moving things around just to simplify
> the code for when the xdp is added.
> Patch 4 actually adds the xdp. Currently the only supported actions
> are XDP_PASS and XDP_DROP. In the future this will be extended with
> XDP_TX and XDP_REDIRECT.
> Patch 5 changes to use page pool API, because the handling of the
> pages is similar with what already lan966x driver is doing. In this
> way is possible to remove some of the code.
>
> All these changes give a small improvement on the RX side:
> Before:
> iperf3 -c 10.96.10.1 -R
> [ 5] 0.00-10.01 sec 514 MBytes 430 Mbits/sec 0 sender
> [ 5] 0.00-10.00 sec 509 MBytes 427 Mbits/sec receiver
>
> After:
> iperf3 -c 10.96.10.1 -R
> [ 5] 0.00-10.02 sec 540 MBytes 452 Mbits/sec 0 sender
> [ 5] 0.00-10.01 sec 537 MBytes 450 Mbits/sec receiver

If it is possible I would like to withdraw the submission for this patch
series.
The reason is that patch 2 touches some code that has some issues. The
issues were not introduced in this patch series. So I prefer to send first
the patches to fix those issues which need to go in net and only when those
fixes reaches net-next to resend this patch series. Otherwise will be some
pretty ugly merge conflicts.
So just to make the life easier for everyone, please ignore for now
this patch series.

>
>
> Horatiu Vultur (5):
> net: lan966x: Add define IFH_LEN_BYTES
> net: lan966x: Rename lan966x_fdma_get_max_mtu
> net: lan966x: Split function lan966x_fdma_rx_get_frame
> net: lan966x: Add basic XDP support
> net: lan96x: Use page_pool API
>
> .../net/ethernet/microchip/lan966x/Kconfig | 1 +
> .../net/ethernet/microchip/lan966x/Makefile | 3 +-
> .../ethernet/microchip/lan966x/lan966x_fdma.c | 194 +++++++++++-------
> .../ethernet/microchip/lan966x/lan966x_ifh.h | 1 +
> .../ethernet/microchip/lan966x/lan966x_main.c | 26 ++-
> .../ethernet/microchip/lan966x/lan966x_main.h | 28 +++
> .../ethernet/microchip/lan966x/lan966x_xdp.c | 100 +++++++++
> 7 files changed, 275 insertions(+), 78 deletions(-)
> create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
>
> --
> 2.38.0
>

--
/Horatiu