2022-11-09 20:54:50

by Horatiu Vultur

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

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

The first 2 patches are just moving things around just to simplify
the code for when the xdp is added.
Patch 3 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 4 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

---
v2->v3:
- inline lan966x_xdp_port_present
- update max_len of page_pool_params not to be the page size anymore but
actually be rx->max_mtu.

v1->v2:
- rebase on net-next, once the fixes for FDMA and MTU were accepted
- drop patch 2, which changes the MTU as is not needed anymore
- allow to run xdp programs on frames bigger than 4KB

Horatiu Vultur (4):
net: lan966x: Add define IFH_LEN_BYTES
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 | 181 +++++++++++-------
.../ethernet/microchip/lan966x/lan966x_ifh.h | 1 +
.../ethernet/microchip/lan966x/lan966x_main.c | 7 +-
.../ethernet/microchip/lan966x/lan966x_main.h | 33 ++++
.../ethernet/microchip/lan966x/lan966x_xdp.c | 76 ++++++++
7 files changed, 236 insertions(+), 66 deletions(-)
create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c

--
2.38.0



2022-11-09 20:55:45

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next v3 3/4] net: lan966x: Add basic XDP support

Introduce basic XDP support to lan966x driver. Currently the driver
supports only the actions XDP_PASS, XDP_DROP and XDP_ABORTED.

Signed-off-by: Horatiu Vultur <[email protected]>
---
.../net/ethernet/microchip/lan966x/Makefile | 3 +-
.../ethernet/microchip/lan966x/lan966x_fdma.c | 11 ++-
.../ethernet/microchip/lan966x/lan966x_main.c | 5 ++
.../ethernet/microchip/lan966x/lan966x_main.h | 16 ++++
.../ethernet/microchip/lan966x/lan966x_xdp.c | 76 +++++++++++++++++++
5 files changed, 109 insertions(+), 2 deletions(-)
create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c

diff --git a/drivers/net/ethernet/microchip/lan966x/Makefile b/drivers/net/ethernet/microchip/lan966x/Makefile
index 962f7c5f9e7dd..251a7d561d633 100644
--- a/drivers/net/ethernet/microchip/lan966x/Makefile
+++ b/drivers/net/ethernet/microchip/lan966x/Makefile
@@ -11,4 +11,5 @@ lan966x-switch-objs := lan966x_main.o lan966x_phylink.o lan966x_port.o \
lan966x_ptp.o lan966x_fdma.o lan966x_lag.o \
lan966x_tc.o lan966x_mqprio.o lan966x_taprio.o \
lan966x_tbf.o lan966x_cbs.o lan966x_ets.o \
- lan966x_tc_matchall.o lan966x_police.o lan966x_mirror.o
+ lan966x_tc_matchall.o lan966x_police.o lan966x_mirror.o \
+ lan966x_xdp.o
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
index d37765ddd53ae..fa4198c617667 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_fdma.c
@@ -423,6 +423,7 @@ static bool lan966x_fdma_rx_more_frames(struct lan966x_rx *rx)
static int lan966x_fdma_rx_check_frame(struct lan966x_rx *rx, u64 *src_port)
{
struct lan966x *lan966x = rx->lan966x;
+ struct lan966x_port *port;
struct lan966x_db *db;
struct page *page;

@@ -443,7 +444,11 @@ static int lan966x_fdma_rx_check_frame(struct lan966x_rx *rx, u64 *src_port)
if (WARN_ON(*src_port >= lan966x->num_phys_ports))
return FDMA_ERROR;

- return FDMA_PASS;
+ port = lan966x->ports[*src_port];
+ if (!lan966x_xdp_port_present(port))
+ return FDMA_PASS;
+
+ return lan966x_xdp_run(port, page, FDMA_DCB_STATUS_BLOCKL(db->status));
}

static struct sk_buff *lan966x_fdma_rx_get_frame(struct lan966x_rx *rx,
@@ -524,6 +529,10 @@ static int lan966x_fdma_napi_poll(struct napi_struct *napi, int weight)
lan966x_fdma_rx_free_page(rx);
lan966x_fdma_rx_advance_dcb(rx);
goto allocate_new;
+ case FDMA_DROP:
+ lan966x_fdma_rx_free_page(rx);
+ lan966x_fdma_rx_advance_dcb(rx);
+ continue;
}

skb = lan966x_fdma_rx_get_frame(rx, src_port);
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index 1a27946ccaf44..42be5d0f1f015 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -468,6 +468,7 @@ static const struct net_device_ops lan966x_port_netdev_ops = {
.ndo_get_port_parent_id = lan966x_port_get_parent_id,
.ndo_eth_ioctl = lan966x_port_ioctl,
.ndo_setup_tc = lan966x_tc_setup,
+ .ndo_bpf = lan966x_xdp,
};

bool lan966x_netdevice_check(const struct net_device *dev)
@@ -694,6 +695,7 @@ static void lan966x_cleanup_ports(struct lan966x *lan966x)
if (port->dev)
unregister_netdev(port->dev);

+ lan966x_xdp_port_deinit(port);
if (lan966x->fdma && lan966x->fdma_ndev == port->dev)
lan966x_fdma_netdev_deinit(lan966x, port->dev);

@@ -1136,6 +1138,9 @@ static int lan966x_probe(struct platform_device *pdev)
lan966x->ports[p]->serdes = serdes;

lan966x_port_init(lan966x->ports[p]);
+ err = lan966x_xdp_port_init(lan966x->ports[p]);
+ if (err)
+ goto cleanup_ports;
}

lan966x_mdb_init(lan966x);
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
index 464fb5e4a8ff6..b3b0619b17c61 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
@@ -103,10 +103,12 @@ enum macaccess_entry_type {
/* FDMA return action codes for checking if the frame is valid
* FDMA_PASS, frame is valid and can be used
* FDMA_ERROR, something went wrong, stop getting more frames
+ * FDMA_DROP, frame is dropped, but continue to get more frames
*/
enum lan966x_fdma_action {
FDMA_PASS = 0,
FDMA_ERROR,
+ FDMA_DROP,
};

struct lan966x_port;
@@ -329,6 +331,9 @@ struct lan966x_port {
enum netdev_lag_hash hash_type;

struct lan966x_port_tc tc;
+
+ struct bpf_prog *xdp_prog;
+ struct xdp_rxq_info xdp_rxq;
};

extern const struct phylink_mac_ops lan966x_phylink_mac_ops;
@@ -536,6 +541,17 @@ void lan966x_mirror_port_stats(struct lan966x_port *port,
struct flow_stats *stats,
bool ingress);

+int lan966x_xdp_port_init(struct lan966x_port *port);
+void lan966x_xdp_port_deinit(struct lan966x_port *port);
+int lan966x_xdp(struct net_device *dev, struct netdev_bpf *xdp);
+int lan966x_xdp_run(struct lan966x_port *port,
+ struct page *page,
+ u32 data_len);
+static inline bool lan966x_xdp_port_present(struct lan966x_port *port)
+{
+ return !!port->xdp_prog;
+}
+
static inline void __iomem *lan_addr(void __iomem *base[],
int id, int tinst, int tcnt,
int gbase, int ginst,
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c b/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
new file mode 100644
index 0000000000000..e77d9f2aad2b4
--- /dev/null
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
@@ -0,0 +1,76 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <linux/bpf.h>
+#include <linux/bpf_trace.h>
+#include <linux/filter.h>
+
+#include "lan966x_main.h"
+
+static int lan966x_xdp_setup(struct net_device *dev, struct netdev_bpf *xdp)
+{
+ struct lan966x_port *port = netdev_priv(dev);
+ struct lan966x *lan966x = port->lan966x;
+ struct bpf_prog *old_prog;
+
+ if (!lan966x->fdma) {
+ NL_SET_ERR_MSG_MOD(xdp->extack,
+ "Allow to set xdp only when using fdma");
+ return -EOPNOTSUPP;
+ }
+
+ old_prog = xchg(&port->xdp_prog, xdp->prog);
+ if (old_prog)
+ bpf_prog_put(old_prog);
+
+ return 0;
+}
+
+int lan966x_xdp(struct net_device *dev, struct netdev_bpf *xdp)
+{
+ switch (xdp->command) {
+ case XDP_SETUP_PROG:
+ return lan966x_xdp_setup(dev, xdp);
+ default:
+ return -EINVAL;
+ }
+}
+
+int lan966x_xdp_run(struct lan966x_port *port, struct page *page, u32 data_len)
+{
+ struct bpf_prog *xdp_prog = port->xdp_prog;
+ struct lan966x *lan966x = port->lan966x;
+ struct xdp_buff xdp;
+ u32 act;
+
+ xdp_init_buff(&xdp, PAGE_SIZE << lan966x->rx.page_order,
+ &port->xdp_rxq);
+ xdp_prepare_buff(&xdp, page_address(page), IFH_LEN_BYTES,
+ data_len - IFH_LEN_BYTES, false);
+ act = bpf_prog_run_xdp(xdp_prog, &xdp);
+ switch (act) {
+ case XDP_PASS:
+ return FDMA_PASS;
+ default:
+ bpf_warn_invalid_xdp_action(port->dev, xdp_prog, act);
+ fallthrough;
+ case XDP_ABORTED:
+ trace_xdp_exception(port->dev, xdp_prog, act);
+ fallthrough;
+ case XDP_DROP:
+ return FDMA_DROP;
+ }
+}
+
+int lan966x_xdp_port_init(struct lan966x_port *port)
+{
+ struct lan966x *lan966x = port->lan966x;
+
+ return xdp_rxq_info_reg(&port->xdp_rxq, port->dev, 0,
+ lan966x->napi.napi_id);
+}
+
+void lan966x_xdp_port_deinit(struct lan966x_port *port)
+{
+ if (xdp_rxq_info_is_reg(&port->xdp_rxq))
+ xdp_rxq_info_unreg(&port->xdp_rxq);
+}
--
2.38.0


2022-11-09 21:04:16

by Horatiu Vultur

[permalink] [raw]
Subject: [PATCH net-next v3 4/4] 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 | 89 ++++++++++---------
.../ethernet/microchip/lan966x/lan966x_main.h | 8 ++
3 files changed, 58 insertions(+), 40 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 fa4198c617667..5fbbd479cfb06 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,25 @@ 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 = rx->max_mtu -
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)),
+ };
+
+ rx->page_pool = page_pool_create(&pp_params);
+ return PTR_ERR_OR_ZERO(rx->page_pool);
+}
+
static int lan966x_fdma_rx_alloc(struct lan966x_rx *rx)
{
struct lan966x *lan966x = rx->lan966x;
@@ -93,6 +90,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);
@@ -436,10 +436,6 @@ static int lan966x_fdma_rx_check_frame(struct lan966x_rx *rx, u64 *src_port)
FDMA_DCB_STATUS_BLOCKL(db->status),
DMA_FROM_DEVICE);

- dma_unmap_single_attrs(lan966x->dev, (dma_addr_t)db->dataptr,
- PAGE_SIZE << rx->page_order, DMA_FROM_DEVICE,
- DMA_ATTR_SKIP_CPU_SYNC);
-
lan966x_ifh_get_src_port(page_address(page), src_port);
if (WARN_ON(*src_port >= lan966x->num_phys_ports))
return FDMA_ERROR;
@@ -468,6 +464,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);
@@ -495,7 +493,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;
}
@@ -740,6 +738,7 @@ static int lan966x_qsys_sw_status(struct lan966x *lan966x)

static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
{
+ struct page_pool *page_pool;
dma_addr_t rx_dma;
void *rx_dcbs;
u32 size;
@@ -748,6 +747,7 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
/* Store these for later to free them */
rx_dma = lan966x->rx.dma;
rx_dcbs = lan966x->rx.dcbs;
+ page_pool = lan966x->rx.page_pool;

napi_synchronize(&lan966x->napi);
napi_disable(&lan966x->napi);
@@ -756,6 +756,7 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
lan966x_fdma_rx_disable(&lan966x->rx);
lan966x_fdma_rx_free_pages(&lan966x->rx);
lan966x->rx.page_order = round_up(new_mtu, PAGE_SIZE) / PAGE_SIZE - 1;
+ lan966x->rx.max_mtu = new_mtu;
err = lan966x_fdma_rx_alloc(&lan966x->rx);
if (err)
goto restore;
@@ -765,11 +766,14 @@ 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_wakeup_netdev(lan966x);
napi_enable(&lan966x->napi);

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);
@@ -777,19 +781,22 @@ static int lan966x_fdma_reload(struct lan966x *lan966x, int new_mtu)
return err;
}

+static int lan966x_fdma_get_max_frame(struct lan966x *lan966x)
+{
+ return lan966x_fdma_get_max_mtu(lan966x) +
+ IFH_LEN_BYTES +
+ SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) +
+ VLAN_HLEN * 2;
+}
+
int lan966x_fdma_change_mtu(struct lan966x *lan966x)
{
int max_mtu;
int err;
u32 val;

- max_mtu = lan966x_fdma_get_max_mtu(lan966x);
- max_mtu += IFH_LEN_BYTES;
- max_mtu += SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
- max_mtu += VLAN_HLEN * 2;
-
- if (round_up(max_mtu, PAGE_SIZE) / PAGE_SIZE - 1 ==
- lan966x->rx.page_order)
+ max_mtu = lan966x_fdma_get_max_frame(lan966x);
+ if (max_mtu == lan966x->rx.max_mtu)
return 0;

/* Disable the CPU port */
@@ -844,6 +851,7 @@ int lan966x_fdma_init(struct lan966x *lan966x)

lan966x->rx.lan966x = lan966x;
lan966x->rx.channel_id = FDMA_XTR_CHANNEL;
+ lan966x->rx.max_mtu = lan966x_fdma_get_max_frame(lan966x);
lan966x->tx.lan966x = lan966x;
lan966x->tx.channel_id = FDMA_INJ_CHANNEL;
lan966x->tx.last_in_use = -1;
@@ -876,5 +884,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 b3b0619b17c61..bc93051aa0798 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>
@@ -161,7 +162,14 @@ struct lan966x_rx {
*/
u8 page_order;

+ /* Represents the max size frame that it can receive to the CPU. This
+ * includes the IFH + VLAN tags + frame + skb_shared_info
+ */
+ u32 max_mtu;
+
u8 channel_id;
+
+ struct page_pool *page_pool;
};

struct lan966x_tx_dcb_buf {
--
2.38.0


2022-11-10 11:56:29

by Alexander Lobakin

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

From: Horatiu Vultur <[email protected]>
Date: Wed, 9 Nov 2022 21:46:09 +0100

> Add support for xdp in lan966x driver. Currently only XDP_PASS and
> XDP_DROP are supported.
>
> The first 2 patches are just moving things around just to simplify
> the code for when the xdp is added.
> Patch 3 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 4 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

A bit confusing name 'max_mtu' which in fact represents the max
frame len + skb overhead (4th patch), but it's more of a personal
taste probably.

For the series:

Reviewed-by: Alexander Lobakin <[email protected]>

Nice stuff! I hear time to time that XDP is for 10G+ NICs only, but
I'm not a fan of such, and this series proves once again XDP fits
any hardware ^.^

>
> ---
> v2->v3:
> - inline lan966x_xdp_port_present
> - update max_len of page_pool_params not to be the page size anymore but
> actually be rx->max_mtu.
>
> v1->v2:
> - rebase on net-next, once the fixes for FDMA and MTU were accepted
> - drop patch 2, which changes the MTU as is not needed anymore
> - allow to run xdp programs on frames bigger than 4KB
>
> Horatiu Vultur (4):
> net: lan966x: Add define IFH_LEN_BYTES
> 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 | 181 +++++++++++-------
> .../ethernet/microchip/lan966x/lan966x_ifh.h | 1 +
> .../ethernet/microchip/lan966x/lan966x_main.c | 7 +-
> .../ethernet/microchip/lan966x/lan966x_main.h | 33 ++++
> .../ethernet/microchip/lan966x/lan966x_xdp.c | 76 ++++++++
> 7 files changed, 236 insertions(+), 66 deletions(-)
> create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
>
> --
> 2.38.0

Thanks,
Olek

2022-11-10 14:17:45

by Andrew Lunn

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

> Nice stuff! I hear time to time that XDP is for 10G+ NICs only, but
> I'm not a fan of such, and this series proves once again XDP fits
> any hardware ^.^

The Freescale FEC recently gained XDP support. Many variants of it are
Fast Ethernet only.

What i found most interesting about that patchset was that the use of
the page_ppol API made the driver significantly faster for the general
case as well as XDP.

Andrew

2022-11-10 17:10:11

by Alexander Lobakin

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

From: Andrew Lunn <[email protected]>
Date: Thu, 10 Nov 2022 14:57:35 +0100

> > Nice stuff! I hear time to time that XDP is for 10G+ NICs only, but
> > I'm not a fan of such, and this series proves once again XDP fits
> > any hardware ^.^
>
> The Freescale FEC recently gained XDP support. Many variants of it are
> Fast Ethernet only.
>
> What i found most interesting about that patchset was that the use of
> the page_ppol API made the driver significantly faster for the general
> case as well as XDP.

The driver didn't have any page recycling or page splitting logics,
while Page Pool recycles even pages from skbs if
skb_mark_for_recycle() is used, which is the case here. So it
significantly reduced the number of new page allocations for Rx, if
there still are any at all.
Plus, Page Pool allocates pages by bulks (of 16 IIRC), not one by
one, that reduces CPU overhead as well.

>
> Andrew

Thanks,
Olek

2022-11-10 20:39:16

by Horatiu Vultur

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

The 11/10/2022 17:21, Alexander Lobakin wrote:

Hi,

>
> From: Andrew Lunn <[email protected]>
> Date: Thu, 10 Nov 2022 14:57:35 +0100
>
> > > Nice stuff! I hear time to time that XDP is for 10G+ NICs only, but
> > > I'm not a fan of such, and this series proves once again XDP fits
> > > any hardware ^.^
> >
> > The Freescale FEC recently gained XDP support. Many variants of it are
> > Fast Ethernet only.
> >
> > What i found most interesting about that patchset was that the use of
> > the page_ppol API made the driver significantly faster for the general
> > case as well as XDP.
>
> The driver didn't have any page recycling or page splitting logics,
> while Page Pool recycles even pages from skbs if
> skb_mark_for_recycle() is used, which is the case here. So it
> significantly reduced the number of new page allocations for Rx, if
> there still are any at all.
> Plus, Page Pool allocates pages by bulks (of 16 IIRC), not one by
> one, that reduces CPU overhead as well.

Just to make sure that everything is clear, those results that I have
shown in the cover letter are without any XDP programs on the
interfaces. Because I thought that is the correct comparison of the
results before and after all these changes.

Once I add an XDP program on the interface the performance drops. The
program will look for some ether types and always return XDP_PASS.

These are the results when I have such a XDP program on the interface:
[ ID] Interval Transfer Bitrate Retr
[ 5] 0.00-10.01 sec 486 MBytes 408 Mbits/sec 0 sender
[ 5] 0.00-10.00 sec 483 MBytes 405 Mbits/sec receiver

>
> >
> > Andrew
>
> Thanks,
> Olek

--
/Horatiu

2022-11-10 23:11:35

by Andrew Lunn

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

On Thu, Nov 10, 2022 at 05:21:48PM +0100, Alexander Lobakin wrote:
> From: Andrew Lunn <[email protected]>
> Date: Thu, 10 Nov 2022 14:57:35 +0100
>
> > > Nice stuff! I hear time to time that XDP is for 10G+ NICs only, but
> > > I'm not a fan of such, and this series proves once again XDP fits
> > > any hardware ^.^
> >
> > The Freescale FEC recently gained XDP support. Many variants of it are
> > Fast Ethernet only.
> >
> > What i found most interesting about that patchset was that the use of
> > the page_ppol API made the driver significantly faster for the general
> > case as well as XDP.
>
> The driver didn't have any page recycling or page splitting logics,
> while Page Pool recycles even pages from skbs if
> skb_mark_for_recycle() is used, which is the case here. So it
> significantly reduced the number of new page allocations for Rx, if
> there still are any at all.

When reviewing new drivers we should be pushing them towards using the
page pool API. It seems to do better than the average role your own
implementation.

Andrew

2022-11-11 11:37:45

by patchwork-bot+netdevbpf

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

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <[email protected]>:

On Wed, 9 Nov 2022 21:46:09 +0100 you wrote:
> Add support for xdp in lan966x driver. Currently only XDP_PASS and
> XDP_DROP are supported.
>
> The first 2 patches are just moving things around just to simplify
> the code for when the xdp is added.
> Patch 3 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 4 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.
>
> [...]

Here is the summary with links:
- [net-next,v3,1/4] net: lan966x: Add define IFH_LEN_BYTES
https://git.kernel.org/netdev/net-next/c/e83163b66a37
- [net-next,v3,2/4] net: lan966x: Split function lan966x_fdma_rx_get_frame
https://git.kernel.org/netdev/net-next/c/4a00b0c712e3
- [net-next,v3,3/4] net: lan966x: Add basic XDP support
https://git.kernel.org/netdev/net-next/c/6a2159be7604
- [net-next,v3,4/4] net: lan96x: Use page_pool API
https://git.kernel.org/netdev/net-next/c/11871aba1974

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html