diff mbox series

[net-next,v2,3/4] net: lan966x: Add basic XDP support

Message ID 20221106211154.3225784-4-horatiu.vultur@microchip.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net: lan966x: Add xdp support | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for net-next, async
netdev/fixes_present success Fixes tag not required for -next series
netdev/subject_prefix success Link
netdev/cover_letter success Series has a cover letter
netdev/patch_count success Link
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/cc_maintainers warning 1 maintainers not CCed: UNGLinuxDriver@microchip.com
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/module_param success Was 0 now: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Horatiu Vultur Nov. 6, 2022, 9:11 p.m. UTC 
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 <horatiu.vultur@microchip.com>
---
 .../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 | 13 +++
 .../ethernet/microchip/lan966x/lan966x_xdp.c  | 81 +++++++++++++++++++
 5 files changed, 111 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c

Comments

Alexander Lobakin Nov. 7, 2022, 4:13 p.m. UTC | #1 
From: Alexander Lobakin <alexander.lobakin@intel.com>

From: Horatiu Vultur <horatiu.vultur@microchip.com>
Date: Sun, 6 Nov 2022 22:11:53 +0100

> 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 <horatiu.vultur@microchip.com>
> ---
>  .../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 | 13 +++
>  .../ethernet/microchip/lan966x/lan966x_xdp.c  | 81 +++++++++++++++++++
>  5 files changed, 111 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c

[...]

> +bool lan966x_xdp_port_present(struct lan966x_port *port)
> +{
> +	return !!port->xdp_prog;
> +}

Why uninline such a simple check? I realize you want to keep all XDP
stuff inside in the separate file, but doesn't this one looks too
much?

> +
> +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

Thanks,
Olek
Horatiu Vultur Nov. 7, 2022, 9:26 p.m. UTC | #2 
The 11/07/2022 17:13, Alexander Lobakin wrote:

Hi Olek,

> 
> From: Alexander Lobakin <alexander.lobakin@intel.com>
> 
> From: Horatiu Vultur <horatiu.vultur@microchip.com>
> Date: Sun, 6 Nov 2022 22:11:53 +0100
> 
> > 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 <horatiu.vultur@microchip.com>
> > ---
> >  .../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 | 13 +++
> >  .../ethernet/microchip/lan966x/lan966x_xdp.c  | 81 +++++++++++++++++++
> >  5 files changed, 111 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
> 
> [...]
> 
> > +bool lan966x_xdp_port_present(struct lan966x_port *port)
> > +{
> > +     return !!port->xdp_prog;
> > +}
> 
> Why uninline such a simple check? I realize you want to keep all XDP
> stuff inside in the separate file, but doesn't this one looks too
> much?

I was kind of hoping that the compiler will inline it for me.
But I can add it in the header file to inline it.

> 
> > +
> > +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
> 
> Thanks,
> Olek
Alexander Lobakin Nov. 8, 2022, 11:26 a.m. UTC | #3 
From: Horatiu Vultur <horatiu.vultur@microchip.com>
Date: Mon, 7 Nov 2022 22:26:18 +0100

> The 11/07/2022 17:13, Alexander Lobakin wrote:
> 
> Hi Olek,
> 
> > 
> > From: Alexander Lobakin <alexander.lobakin@intel.com>
> > 
> > From: Horatiu Vultur <horatiu.vultur@microchip.com>
> > Date: Sun, 6 Nov 2022 22:11:53 +0100
> > 
> > > 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 <horatiu.vultur@microchip.com>
> > > ---
> > >  .../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 | 13 +++
> > >  .../ethernet/microchip/lan966x/lan966x_xdp.c  | 81 +++++++++++++++++++
> > >  5 files changed, 111 insertions(+), 2 deletions(-)
> > >  create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
> > 
> > [...]
> > 
> > > +bool lan966x_xdp_port_present(struct lan966x_port *port)
> > > +{
> > > +     return !!port->xdp_prog;
> > > +}
> > 
> > Why uninline such a simple check? I realize you want to keep all XDP
> > stuff inside in the separate file, but doesn't this one looks too
> > much?
> 
> I was kind of hoping that the compiler will inline it for me.
> But I can add it in the header file to inline it.

That is very unlikely for the compilers to uninline an extern
function. LTO is able to do that, but even then it's not
guaranteed. So I'd keep it in a header file as an inline.

> 
> > 
> > > +
> > > +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
> > 
> > Thanks,
> > Olek
> 
> -- 
> /Horatiu

Thanks,
Olek
diff mbox series

Patch

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..18f727f397fd6 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,14 @@  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);
+bool lan966x_xdp_port_present(struct lan966x_port *port);
+
 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..b8f30abf4f795
--- /dev/null
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_xdp.c
@@ -0,0 +1,81 @@ 
+// 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;
+	}
+}
+
+bool lan966x_xdp_port_present(struct lan966x_port *port)
+{
+	return !!port->xdp_prog;
+}
+
+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);
+}