Netdev List
 help / color / mirror / Atom feed
* Re: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Jason Gunthorpe @ 2019-07-04 13:53 UTC (permalink / raw)
  To: Greg KH
  Cc: Jeff Kirsher, davem@davemloft.net, dledford@redhat.com,
	Tony Nguyen, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
	mustafa.ismail@intel.com, shiraz.saleem@intel.com, Dave Ertman,
	Andrew Bowers
In-Reply-To: <20190704134612.GB10963@kroah.com>

On Thu, Jul 04, 2019 at 03:46:12PM +0200, Greg KH wrote:
> On Thu, Jul 04, 2019 at 12:48:29PM +0000, Jason Gunthorpe wrote:
> > On Thu, Jul 04, 2019 at 02:42:47PM +0200, Greg KH wrote:
> > > On Thu, Jul 04, 2019 at 12:37:33PM +0000, Jason Gunthorpe wrote:
> > > > On Thu, Jul 04, 2019 at 02:29:50PM +0200, Greg KH wrote:
> > > > > On Thu, Jul 04, 2019 at 12:16:41PM +0000, Jason Gunthorpe wrote:
> > > > > > On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> > > > > > > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> > > > > > > 
> > > > > > > The RDMA block does not advertise on the PCI bus or any other bus.
> > > > > > > Thus the ice driver needs to provide access to the RDMA hardware block
> > > > > > > via a virtual bus; utilize the platform bus to provide this access.
> > > > > > > 
> > > > > > > This patch initializes the driver to support RDMA as well as creates
> > > > > > > and registers a platform device for the RDMA driver to register to. At
> > > > > > > this point the driver is fully initialized to register a platform
> > > > > > > driver, however, can not yet register as the ops have not been
> > > > > > > implemented.
> > > > > > 
> > > > > > I think you need Greg's ack on all this driver stuff - particularly
> > > > > > that a platform_device is OK.
> > > > > 
> > > > > A platform_device is almost NEVER ok.
> > > > > 
> > > > > Don't abuse it, make a real device on a real bus.  If you don't have a
> > > > > real bus and just need to create a device to hang other things off of,
> > > > > then use the virtual one, that's what it is there for.
> > > > 
> > > > Ideally I'd like to see all the RDMA drivers that connect to ethernet
> > > > drivers use some similar scheme.
> > > 
> > > Why?  They should be attached to a "real" device, why make any up?
> > 
> > ? A "real" device, like struct pci_device, can only bind to one
> > driver. How can we bind it concurrently to net, rdma, scsi, etc?
> 
> MFD was designed for this very problem.
> 
> > > > This is for a PCI device that plugs into multiple subsystems in the
> > > > kernel, ie it has net driver functionality, rdma functionality, some
> > > > even have SCSI functionality
> > > 
> > > Sounds like a MFD device, why aren't you using that functionality
> > > instead?
> > 
> > This was also my advice, but in another email Jeff says:
> > 
> >   MFD architecture was also considered, and we selected the simpler
> >   platform model. Supporting a MFD architecture would require an
> >   additional MFD core driver, individual platform netdev, RDMA function
> >   drivers, and stripping a large portion of the netdev drivers into
> >   MFD core. The sub-devices registered by MFD core for function
> >   drivers are indeed platform devices.  
> 
> So, "mfd is too hard, let's abuse a platform device" is ok?
> 
> People have been wanting to do MFD drivers for PCI devices for a long
> time, it's about time someone actually did the work for it, I bet it
> will not be all that complex if tiny embedded drivers can do it :)

Okay, sounds like a NAK to me. I'll drop these patches from the RDMA
patchworks and Jeff can work through the MFD stuff first.

Jason

^ permalink raw reply

* Re: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Greg KH @ 2019-07-04 13:46 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: Jeff Kirsher, davem@davemloft.net, dledford@redhat.com,
	Tony Nguyen, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
	mustafa.ismail@intel.com, shiraz.saleem@intel.com, Dave Ertman,
	Andrew Bowers
In-Reply-To: <20190704124824.GK3401@mellanox.com>

On Thu, Jul 04, 2019 at 12:48:29PM +0000, Jason Gunthorpe wrote:
> On Thu, Jul 04, 2019 at 02:42:47PM +0200, Greg KH wrote:
> > On Thu, Jul 04, 2019 at 12:37:33PM +0000, Jason Gunthorpe wrote:
> > > On Thu, Jul 04, 2019 at 02:29:50PM +0200, Greg KH wrote:
> > > > On Thu, Jul 04, 2019 at 12:16:41PM +0000, Jason Gunthorpe wrote:
> > > > > On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> > > > > > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> > > > > > 
> > > > > > The RDMA block does not advertise on the PCI bus or any other bus.
> > > > > > Thus the ice driver needs to provide access to the RDMA hardware block
> > > > > > via a virtual bus; utilize the platform bus to provide this access.
> > > > > > 
> > > > > > This patch initializes the driver to support RDMA as well as creates
> > > > > > and registers a platform device for the RDMA driver to register to. At
> > > > > > this point the driver is fully initialized to register a platform
> > > > > > driver, however, can not yet register as the ops have not been
> > > > > > implemented.
> > > > > 
> > > > > I think you need Greg's ack on all this driver stuff - particularly
> > > > > that a platform_device is OK.
> > > > 
> > > > A platform_device is almost NEVER ok.
> > > > 
> > > > Don't abuse it, make a real device on a real bus.  If you don't have a
> > > > real bus and just need to create a device to hang other things off of,
> > > > then use the virtual one, that's what it is there for.
> > > 
> > > Ideally I'd like to see all the RDMA drivers that connect to ethernet
> > > drivers use some similar scheme.
> > 
> > Why?  They should be attached to a "real" device, why make any up?
> 
> ? A "real" device, like struct pci_device, can only bind to one
> driver. How can we bind it concurrently to net, rdma, scsi, etc?

MFD was designed for this very problem.

> > > This is for a PCI device that plugs into multiple subsystems in the
> > > kernel, ie it has net driver functionality, rdma functionality, some
> > > even have SCSI functionality
> > 
> > Sounds like a MFD device, why aren't you using that functionality
> > instead?
> 
> This was also my advice, but in another email Jeff says:
> 
>   MFD architecture was also considered, and we selected the simpler
>   platform model. Supporting a MFD architecture would require an
>   additional MFD core driver, individual platform netdev, RDMA function
>   drivers, and stripping a large portion of the netdev drivers into
>   MFD core. The sub-devices registered by MFD core for function
>   drivers are indeed platform devices.  

So, "mfd is too hard, let's abuse a platform device" is ok?

People have been wanting to do MFD drivers for PCI devices for a long
time, it's about time someone actually did the work for it, I bet it
will not be all that complex if tiny embedded drivers can do it :)

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH net-next v5 5/5] selftests: tc-tests: actions: add MPLS tests
From: John Hurley @ 2019-07-04 13:40 UTC (permalink / raw)
  To: Davide Caratti
  Cc: Linux Netdev List, David Miller, Jiri Pirko, Cong Wang,
	David Ahern, Willem de Bruijn, Simon Horman, Jakub Kicinski,
	oss-drivers
In-Reply-To: <0130c56ef79f8bf360ddb0b01db5e7684f0bf62a.camel@redhat.com>

On Thu, Jul 4, 2019 at 9:40 AM Davide Caratti <dcaratti@redhat.com> wrote:
>
> On Wed, 2019-07-03 at 01:25 +0100, John Hurley wrote:
> > Add a new series of selftests to verify the functionality of act_mpls in
> > TC.
> >
> > Signed-off-by: John Hurley <john.hurley@netronome.com>
> > Reviewed-by: Simon Horman <simon.horman@netronome.com>
> > Acked-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> >  .../tc-testing/tc-tests/actions/mpls.json          | 812 +++++++++++++++++++++
> >  1 file changed, 812 insertions(+)
> >  create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/mpls.json
> >
>
> hello John,
>
> (sorry for noticing this late). some scripts use
>
> tools/testing/selftests/tc-testing/config
>
> to rebuild vmlinux before running TDC. I think you should add a line
> there that sets CONFIG_NET_ACT_MPLS=y.
>
> WDYT?

Hi Davide,
Thanks for pointing this out.
Yes, I'll add it.
Thanks

>
> thanks!
> --
> davide
>

^ permalink raw reply

* Re: i.mx6ul with DSA in multi chip addressing mode - no MDIO access
From: Andrew Lunn @ 2019-07-04 13:27 UTC (permalink / raw)
  To: Benjamin Beckmeyer; +Cc: netdev
In-Reply-To: <d1181129-ec9d-01c1-3102-e1dc5dec0378@eks-engel.de>

On Thu, Jul 04, 2019 at 10:54:47AM +0200, Benjamin Beckmeyer wrote:
> 
> On 03.07.19 17:55, Andrew Lunn wrote:
> > On Wed, Jul 03, 2019 at 03:10:34PM +0200, Benjamin Beckmeyer wrote:
> >> Hey folks,
> >>
> >> I'm having a problem with a custom i.mx6ul board. When DSA is loaded I can't 
> >> get access to the switch via MDIO, but the DSA is working properly. I set up
> >> a bridge for testing and the switch is in forwarding mode and i can ping the 
> >> board. But the MDIO access isn't working at address 2 for the switch. When I 
> >> delete the DSA from the devicetree and start the board up, I can access the 
> >> switch via MDIO.
> >>
> >> With DSA up and running:
> >>
> >> mii -i 2 0 0x9800
> >> mii -i 2 1
> >> phyid:2, reg:0x01 -> 0x4000
> >> mii -i 2 0 0x9803
> >> mii -i 2 1
> >> phyid:2, reg:0x01 -> 0x4000
> >> mii -i 2 1 0x1883
> >> mii -i 2 1
> >> phyid:2, reg:0x01 -> 0x4000
> > Hi Benjamin
> >
> > I'm guessing that the driver is also using register 0 and 1 at the
> > same time you are, e.g. to poll the PHYs for link status etc.
> >
> > There are trace points for MDIO, so you can get the kernel to log all
> > registers access. That should confirm if i'm right.
> >
> > 	  Andrew
> 
> Hi Andrew,
> you were absolutly right. The bus is really busy the whole time, I've 
> checked that with the tracepoints in mdio_access.
> 
> But I'm still wondering why isn't that with a single chip addressing 
> mode configured switch? I mean, okay, the switch has more ports, but
> I've checked the accesses for both. The 6321(single chip addressing 
> mode) has around 4-5 accesses to the MDIO bus and the 6390(multi chip 
> addressing mode) has around 600 accesses per second. 

Hi Benjamin

In single chip mode, reading a register is atomic. With multi-chip,
you need to access two registers, so it clearly is not atomic. And so
any other action on the bus will cause you problems when doing things
from user space without being able to take the register mutex.

But 4-5 vs 600 suggests you don't have the interrupt line in your
device tree. If you have the interrupt line connected to a GPIO, and
the driver knows about it, it has no need to poll the PHYs. I also
added support for 'polled interrupts', as a fall back when then
interrupt is not listed in device tree. 10 times a second the driver
polls the interrupt status register, and if any interrupts have
happened within the switch, it triggers the needed handlers. Reading
one status register every 100ms is much less effort than reading all
the PHY status registers once per second.

Still, 600 per second sounds too high. Do you have an SNMP agent
getting statistics?

	Andrew

^ permalink raw reply

* [PATCH net-next v2 4/4] qed*: Add devlink support for configuration attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-04 13:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190704132011.13600-1-skalluru@marvell.com>

This patch adds implementation for devlink callbacks for reading and
configuring the device attributes.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 Documentation/networking/devlink-params-qede.txt |  72 ++++++++
 drivers/net/ethernet/qlogic/qed/qed_main.c       |  38 +++++
 drivers/net/ethernet/qlogic/qede/qede.h          |   3 +
 drivers/net/ethernet/qlogic/qede/qede_devlink.c  | 202 ++++++++++++++++++++++-
 drivers/net/ethernet/qlogic/qede/qede_devlink.h  |  23 +++
 include/linux/qed/qed_if.h                       |  16 ++
 6 files changed, 353 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/networking/devlink-params-qede.txt

diff --git a/Documentation/networking/devlink-params-qede.txt b/Documentation/networking/devlink-params-qede.txt
new file mode 100644
index 0000000..f78a993
--- /dev/null
+++ b/Documentation/networking/devlink-params-qede.txt
@@ -0,0 +1,72 @@
+enable_sriov		[DEVICE, GENERIC]
+			Configuration mode: Permanent
+
+iwarp_cmt		[DEVICE, DRIVER-SPECIFIC]
+			Enable iWARP support over 100G device (CMT mode).
+			Type: Boolean
+			Configuration mode: runtime
+
+entity_id		[DEVICE, DRIVER-SPECIFIC]
+			Set the entity ID value to be used for this device
+			while reading/configuring the devlink attributes.
+			Type: u8
+			Configuration mode: runtime
+
+device_capabilities	[DEVICE, DRIVER-SPECIFIC]
+			Set the entity ID value to be used for this device
+			while reading/configuring the devlink attributes.
+			Type: u8
+			Configuration mode: runtime
+
+mf_mode			[DEVICE, DRIVER-SPECIFIC]
+			Configure Multi Function mode for the device.
+			Supported MF modes and the assoicated values are,
+			    MF allowed(0), Default(1), SPIO4(2), NPAR1.0(3),
+			    NPAR1.5(4), NPAR2.0(5), BD(6) and UFP(7)
+			Type: u8
+			Configuration mode: Permanent
+
+dcbx_mode		[PORT, DRIVER-SPECIFIC]
+			Configure DCBX mode for the device.
+			Supported dcbx modes are,
+			    Disabled(0), IEEE(1), CEE(2) and Dynamic(3)
+			Type: u8
+			Configuration mode: Permanent
+
+preboot_oprom		[PORT, DRIVER-SPECIFIC]
+			Enable Preboot Option ROM.
+			Type: Boolean
+			Configuration mode: Permanent
+
+preboot_boot_protocol	[PORT, DRIVER-SPECIFIC]
+			Configure preboot Boot protocol.
+			Possible values are,
+			    PXE(0), iSCSI Boot(3), FCoE Boot(4) and NONE(7)
+			Type: u8
+			Configuration mode: Permanent
+
+preboot_vlan		[PORT, DRIVER-SPECIFIC]
+			Preboot VLAN.
+			Type: u16
+			Configuration mode: Permanent
+
+preboot_vlan_value	[PORT, DRIVER-SPECIFIC]
+			Configure Preboot VLAN value.
+			Type: u16
+			Configuration mode: Permanent
+
+mba_delay_time		[PORT, DRIVER-SPECIFIC]
+			Configure MBA Delay Time. Supported range is [0-15].
+			Type: u8
+			Configuration mode: Permanent
+
+mba_setup_hot_key	[PORT, DRIVER-SPECIFIC]
+			Configure MBA setup Hot Key. Possible values are,
+			Ctrl S(0) and Ctrl B(1).
+			Type: u8
+			Configuration mode: Permanent
+
+mba_hide_setup_prompt	[PORT, DRIVER-SPECIFIC]
+			Configure MBA hide setup prompt.
+			Type: Boolean
+			Configuration mode: Permanent
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index f0183e2..3d43140 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -2388,6 +2388,42 @@ static void qed_set_iwarp_cmt(struct qed_dev *cdev, bool iwarp_cmt)
 	cdev->iwarp_cmt = iwarp_cmt;
 }
 
+static int qed_get_cfg_attr(struct qed_dev *cdev, u16 cmd, u8 entity, u32 flags,
+			    u8 *buf, int *len)
+{
+	struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
+	struct qed_ptt *ptt;
+	int status = 0;
+
+	ptt = qed_ptt_acquire(hwfn);
+	if (!ptt)
+		return -EAGAIN;
+
+	status = qed_mcp_nvm_get_cfg(hwfn, ptt, cmd, entity, flags, buf, len);
+
+	qed_ptt_release(hwfn, ptt);
+
+	return status;
+}
+
+static int qed_set_cfg_attr(struct qed_dev *cdev, u16 cmd, u8 entity, u32 flags,
+			    u8 *buf, int len)
+{
+	struct qed_hwfn *hwfn = QED_LEADING_HWFN(cdev);
+	struct qed_ptt *ptt;
+	int status = 0;
+
+	ptt = qed_ptt_acquire(hwfn);
+	if (!ptt)
+		return -EAGAIN;
+
+	status = qed_mcp_nvm_set_cfg(hwfn, ptt, cmd, entity, flags, buf, len);
+
+	qed_ptt_release(hwfn, ptt);
+
+	return status;
+}
+
 static struct qed_selftest_ops qed_selftest_ops_pass = {
 	.selftest_memory = &qed_selftest_memory,
 	.selftest_interrupt = &qed_selftest_interrupt,
@@ -2438,6 +2474,8 @@ static void qed_set_iwarp_cmt(struct qed_dev *cdev, bool iwarp_cmt)
 	.get_affin_hwfn_idx = &qed_get_affin_hwfn_idx,
 	.get_iwarp_cmt = &qed_get_iwarp_cmt,
 	.set_iwarp_cmt = &qed_set_iwarp_cmt,
+	.get_cfg_attr = &qed_get_cfg_attr,
+	.set_cfg_attr = &qed_set_cfg_attr,
 };
 
 void qed_get_protocol_stats(struct qed_dev *cdev,
diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index 35ad5cd..e175d30 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -54,6 +54,7 @@
 
 #include <net/pkt_cls.h>
 #include <net/tc_act/tc_gact.h>
+#include <net/devlink.h>
 
 #define QEDE_MAJOR_VERSION		8
 #define QEDE_MINOR_VERSION		37
@@ -264,6 +265,8 @@ struct qede_dev {
 	struct bpf_prog *xdp_prog;
 
 	struct devlink			*dl;
+	struct devlink_port		dl_port;
+	u8				cfg_entity_id;
 };
 
 enum QEDE_STATE {
diff --git a/drivers/net/ethernet/qlogic/qede/qede_devlink.c b/drivers/net/ethernet/qlogic/qede/qede_devlink.c
index 3f362ac..076a66a 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_devlink.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_devlink.c
@@ -2,6 +2,31 @@
 #include "qede.h"
 #include "qede_devlink.h"
 
+static const struct qede_devlink_cfg_param cfg_params[] = {
+	{DEVLINK_PARAM_GENERIC_ID_ENABLE_SRIOV, QED_NVM_CFG_ID_ENABLE_SRIOV,
+	 DEVLINK_PARAM_TYPE_BOOL},
+	{QEDE_DEVLINK_ENTITY_ID, 0, DEVLINK_PARAM_TYPE_U8},
+	{QEDE_DEVLINK_DEVICE_CAPABILITIES,
+	 QED_NVM_CFG_ID_DEVICE_CAPABILITIES, DEVLINK_PARAM_TYPE_U8},
+	{QEDE_DEVLINK_MF_MODE, QED_NVM_CFG_ID_MF_MODE, DEVLINK_PARAM_TYPE_U8},
+	{QEDE_DEVLINK_DCBX_MODE, QED_NVM_CFG_ID_DCBX_MODE,
+	 DEVLINK_PARAM_TYPE_U8},
+	{QEDE_DEVLINK_PREBOOT_OPROM, QED_NVM_CFG_ID_PREBOOT_OPROM,
+	 DEVLINK_PARAM_TYPE_BOOL},
+	{QEDE_DEVLINK_PREBOOT_BOOT_PROTOCOL,
+	 QED_NVM_CFG_ID_PREBOOT_BOOT_PROTOCOL, DEVLINK_PARAM_TYPE_U8},
+	{QEDE_DEVLINK_PREBOOT_VLAN, QED_NVM_CFG_ID_PREBOOT_VLAN,
+	 DEVLINK_PARAM_TYPE_U16},
+	{QEDE_DEVLINK_PREBOOT_VLAN_VALUE, QED_NVM_CFG_ID_PREBOOT_VLAN_VALUE,
+	 DEVLINK_PARAM_TYPE_U16},
+	{QEDE_DEVLINK_MBA_DELAY_TIME, QED_NVM_CFG_ID_MBA_DELAY_TIME,
+	 DEVLINK_PARAM_TYPE_U8},
+	{QEDE_DEVLINK_MBA_SETUP_HOT_KEY, QED_NVM_CFG_ID_MBA_SETUP_HOT_KEY,
+	 DEVLINK_PARAM_TYPE_U8},
+	{QEDE_DEVLINK_MBA_HIDE_SETUP_PROMPT,
+	 QED_NVM_CFG_ID_MBA_HIDE_SETUP_PROMPT, DEVLINK_PARAM_TYPE_BOOL},
+};
+
 static int qede_dl_param_get(struct devlink *dl, u32 id,
 			     struct devlink_param_gset_ctx *ctx)
 {
@@ -28,11 +53,159 @@ static int qede_dl_param_set(struct devlink *dl, u32 id,
 	return 0;
 }
 
+static int qede_dl_get_perm_cfg(struct devlink *dl, u32 id,
+				struct devlink_param_gset_ctx *ctx)
+{
+	u8 buf[QEDE_DL_PARAM_BUF_LEN];
+	struct qede_devlink *qede_dl;
+	int rc, idx, len = 0;
+	struct qede_dev *edev;
+	u32 flags;
+
+	qede_dl = devlink_priv(dl);
+	edev = qede_dl->edev;
+
+	if (id == QEDE_DEVLINK_ENTITY_ID) {
+		ctx->val.vu8 = edev->cfg_entity_id;
+		return 0;
+	}
+
+	for (idx = 0; idx < ARRAY_SIZE(cfg_params); idx++)
+		if (cfg_params[idx].id == id)
+			break;
+
+	if (idx == ARRAY_SIZE(cfg_params)) {
+		DP_ERR(edev, "Invalid command id %d\n", id);
+		return -EINVAL;
+	}
+
+	memset(buf, 0, QEDE_DL_PARAM_BUF_LEN);
+	flags = edev->cfg_entity_id ? QEDE_DL_PARAM_PF_GET_FLAGS :
+		QEDE_DL_PARAM_GET_FLAGS;
+
+	rc = edev->ops->common->get_cfg_attr(edev->cdev, cfg_params[idx].cmd,
+					     edev->cfg_entity_id, flags, buf,
+					     &len);
+	if (rc)
+		DP_ERR(edev, "Error = %d\n", rc);
+	else
+		memcpy(&ctx->val, buf, len);
+
+	return rc;
+}
+
+static int qede_dl_set_perm_cfg(struct devlink *dl, u32 id,
+				struct devlink_param_gset_ctx *ctx)
+{
+	u8 buf[QEDE_DL_PARAM_BUF_LEN];
+	struct qede_devlink *qede_dl;
+	int rc, idx, len = 0;
+	struct qede_dev *edev;
+	u32 flags;
+
+	qede_dl = devlink_priv(dl);
+	edev = qede_dl->edev;
+
+	if (id == QEDE_DEVLINK_ENTITY_ID) {
+		edev->cfg_entity_id = ctx->val.vu8;
+		return 0;
+	}
+
+	for (idx = 0; idx < ARRAY_SIZE(cfg_params); idx++)
+		if (cfg_params[idx].id == id)
+			break;
+
+	if (idx == ARRAY_SIZE(cfg_params)) {
+		DP_ERR(edev, "Invalid command id %d\n", id);
+		return -EINVAL;
+	}
+
+	memset(buf, 0, QEDE_DL_PARAM_BUF_LEN);
+	switch (cfg_params[idx].type) {
+	case DEVLINK_PARAM_TYPE_BOOL:
+		len = 1;
+		break;
+	case DEVLINK_PARAM_TYPE_U8:
+		len = 1;
+		break;
+	case DEVLINK_PARAM_TYPE_U16:
+		len = 2;
+		break;
+	case DEVLINK_PARAM_TYPE_U32:
+		len = 4;
+		break;
+	case DEVLINK_PARAM_TYPE_STRING:
+		len = strlen(ctx->val.vstr);
+		break;
+	}
+
+	memcpy(buf, &ctx->val, len);
+	flags = edev->cfg_entity_id ? QEDE_DL_PARAM_PF_SET_FLAGS :
+		QEDE_DL_PARAM_SET_FLAGS;
+
+	rc = edev->ops->common->set_cfg_attr(edev->cdev, cfg_params[idx].cmd,
+					     edev->cfg_entity_id, flags, buf,
+					     len);
+	if (rc)
+		DP_ERR(edev, "Error = %d\n", rc);
+
+	return rc;
+}
+
 static const struct devlink_param qede_devlink_params[] = {
 	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_PARAM_ID_IWARP_CMT,
 			     "iwarp_cmt", DEVLINK_PARAM_TYPE_BOOL,
 			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
 			     qede_dl_param_get, qede_dl_param_set, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_ENTITY_ID,
+			     "entity_id", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_GENERIC(ENABLE_SRIOV, BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			      qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_MF_MODE,
+			     "mf_mode", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_DEVICE_CAPABILITIES,
+			     "device_capabilities", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+};
+
+static const struct devlink_param qede_devlink_port_params[] = {
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_DCBX_MODE,
+			     "dcbx_mode", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_PREBOOT_OPROM,
+			     "preboot_oprom", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_PREBOOT_BOOT_PROTOCOL,
+			     "preboot_boot_protocol", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_PREBOOT_VLAN,
+			     "preboot_vlan", DEVLINK_PARAM_TYPE_U16,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_PREBOOT_VLAN_VALUE,
+			     "preboot_vlan_value", DEVLINK_PARAM_TYPE_U16,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_MBA_DELAY_TIME,
+			     "mba_delay_time", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_MBA_SETUP_HOT_KEY,
+			     "mba_setup_hot_key", DEVLINK_PARAM_TYPE_U8,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_MBA_HIDE_SETUP_PROMPT,
+			     "mba_hide_setup_prompt", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_PERMANENT),
+			     qede_dl_get_perm_cfg, qede_dl_set_perm_cfg, NULL),
 };
 
 static const struct devlink_ops qede_dl_ops;
@@ -66,11 +239,34 @@ int qede_devlink_register(struct qede_dev *edev)
 	devlink_param_driverinit_value_set(dl, QEDE_DEVLINK_PARAM_ID_IWARP_CMT,
 					   value);
 
+	devlink_port_attrs_set(&edev->dl_port, DEVLINK_PORT_FLAVOUR_PHYSICAL,
+			       0, false, 0, NULL, 0);
+	rc = devlink_port_register(dl, &edev->dl_port, 0);
+	if (rc) {
+		DP_ERR(edev, "devlink_port_register failed");
+		goto err_param_unregister;
+	}
+	devlink_port_type_eth_set(&edev->dl_port, edev->ndev);
+
+	rc = devlink_port_params_register(&edev->dl_port,
+					  qede_devlink_port_params,
+					  ARRAY_SIZE(qede_devlink_port_params));
+	if (rc) {
+		DP_ERR(edev, "devlink_port_params_register failed");
+		goto err_port_unregister;
+	}
+
 	devlink_params_publish(dl);
+	devlink_port_params_publish(&edev->dl_port);
 	edev->ops->common->set_iwarp_cmt(edev->cdev, false);
 
 	return 0;
 
+err_port_unregister:
+	devlink_port_unregister(&edev->dl_port);
+err_param_unregister:
+	devlink_params_unregister(edev->dl, qede_devlink_params,
+				  ARRAY_SIZE(qede_devlink_params));
 err_unregister:
 	devlink_unregister(dl);
 
@@ -86,9 +282,13 @@ void qede_devlink_unregister(struct qede_dev *edev)
 	if (!edev->dl)
 		return;
 
+	devlink_port_params_unpublish(&edev->dl_port);
+	devlink_params_unpublish(edev->dl);
+	devlink_port_params_unregister(&edev->dl_port, qede_devlink_port_params,
+				       ARRAY_SIZE(qede_devlink_port_params));
 	devlink_params_unregister(edev->dl, qede_devlink_params,
 				  ARRAY_SIZE(qede_devlink_params));
-
+	devlink_port_unregister(&edev->dl_port);
 	devlink_unregister(edev->dl);
 	devlink_free(edev->dl);
 }
diff --git a/drivers/net/ethernet/qlogic/qede/qede_devlink.h b/drivers/net/ethernet/qlogic/qede/qede_devlink.h
index 5aa79dd..57b5fcc 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_devlink.h
+++ b/drivers/net/ethernet/qlogic/qede/qede_devlink.h
@@ -3,6 +3,12 @@
 #define _QEDE_DEVLINK_H
 #include <net/devlink.h>
 
+#define QEDE_DL_PARAM_GET_FLAGS		0xA
+#define QEDE_DL_PARAM_SET_FLAGS		0xE
+#define QEDE_DL_PARAM_PF_GET_FLAGS	0x1A
+#define QEDE_DL_PARAM_PF_SET_FLAGS	0x1E
+#define QEDE_DL_PARAM_BUF_LEN		32
+
 struct qede_devlink {
 	struct qede_dev *edev;
 };
@@ -10,6 +16,23 @@ struct qede_devlink {
 enum qede_devlink_param_id {
 	QEDE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
 	QEDE_DEVLINK_PARAM_ID_IWARP_CMT,
+	QEDE_DEVLINK_ENTITY_ID,
+	QEDE_DEVLINK_DEVICE_CAPABILITIES,
+	QEDE_DEVLINK_MF_MODE,
+	QEDE_DEVLINK_DCBX_MODE,
+	QEDE_DEVLINK_PREBOOT_OPROM,
+	QEDE_DEVLINK_PREBOOT_BOOT_PROTOCOL,
+	QEDE_DEVLINK_PREBOOT_VLAN,
+	QEDE_DEVLINK_PREBOOT_VLAN_VALUE,
+	QEDE_DEVLINK_MBA_DELAY_TIME,
+	QEDE_DEVLINK_MBA_SETUP_HOT_KEY,
+	QEDE_DEVLINK_MBA_HIDE_SETUP_PROMPT,
+};
+
+struct qede_devlink_cfg_param {
+	u16 id;
+	u16 cmd;
+	enum devlink_param_type type;
 };
 
 int qede_devlink_register(struct qede_dev *edev);
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index 7c41304..6430762 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -48,6 +48,18 @@
 #include <linux/qed/qed_chain.h>
 #include <linux/io-64-nonatomic-lo-hi.h>
 
+#define QED_NVM_CFG_ID_MF_MODE			9
+#define QED_NVM_CFG_ID_DCBX_MODE		26
+#define QED_NVM_CFG_ID_PREBOOT_OPROM		59
+#define QED_NVM_CFG_ID_MBA_DELAY_TIME		61
+#define QED_NVM_CFG_ID_MBA_SETUP_HOT_KEY	62
+#define QED_NVM_CFG_ID_MBA_HIDE_SETUP_PROMPT	63
+#define QED_NVM_CFG_ID_PREBOOT_BOOT_PROTOCOL	69
+#define QED_NVM_CFG_ID_ENABLE_SRIOV		70
+#define QED_NVM_CFG_ID_DEVICE_CAPABILITIES	117
+#define QED_NVM_CFG_ID_PREBOOT_VLAN_VALUE	132
+#define QED_NVM_CFG_ID_PREBOOT_VLAN		133
+
 enum dcbx_protocol_type {
 	DCBX_PROTOCOL_ISCSI,
 	DCBX_PROTOCOL_FCOE,
@@ -1134,6 +1146,10 @@ struct qed_common_ops {
 
 	bool (*get_iwarp_cmt)(struct qed_dev *cdev);
 	void (*set_iwarp_cmt)(struct qed_dev *cdev, bool iwarp_cmt);
+	int (*get_cfg_attr)(struct qed_dev *cdev, u16 cmd, u8 entity, u32 flags,
+			    u8 *buf, int *len);
+	int (*set_cfg_attr)(struct qed_dev *cdev, u16 cmd, u8 entity, u32 flags,
+			    u8 *buf, int len);
 };
 
 #define MASK_FIELD(_name, _value) \
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net-next v2 3/4] qed*: Add new file for devlink implementation.
From: Sudarsana Reddy Kalluru @ 2019-07-04 13:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190704132011.13600-1-skalluru@marvell.com>

Moving devlink implementation from qed to qede driver. The change is
required as qede is the actual PCI driver and qed is only the control
driver.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed.h           |   1 -
 drivers/net/ethernet/qlogic/qed/qed_main.c      | 122 +++---------------------
 drivers/net/ethernet/qlogic/qede/Makefile       |   2 +-
 drivers/net/ethernet/qlogic/qede/qede.h         |   2 +
 drivers/net/ethernet/qlogic/qede/qede_devlink.c |  94 ++++++++++++++++++
 drivers/net/ethernet/qlogic/qede/qede_devlink.h |  18 ++++
 drivers/net/ethernet/qlogic/qede/qede_main.c    |  13 +++
 include/linux/qed/qed_if.h                      |   3 +
 8 files changed, 143 insertions(+), 112 deletions(-)
 create mode 100644 drivers/net/ethernet/qlogic/qede/qede_devlink.c
 create mode 100644 drivers/net/ethernet/qlogic/qede/qede_devlink.h

diff --git a/drivers/net/ethernet/qlogic/qed/qed.h b/drivers/net/ethernet/qlogic/qed/qed.h
index 89fe091..cebd822 100644
--- a/drivers/net/ethernet/qlogic/qed/qed.h
+++ b/drivers/net/ethernet/qlogic/qed/qed.h
@@ -864,7 +864,6 @@ struct qed_dev {
 	u32 rdma_max_srq_sge;
 	u16 tunn_feature_mask;
 
-	struct devlink			*dl;
 	bool				iwarp_cmt;
 };
 
diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
index 829dd60..f0183e2 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_main.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
@@ -48,7 +48,6 @@
 #include <linux/crc32.h>
 #include <linux/qed/qed_if.h>
 #include <linux/qed/qed_ll2_if.h>
-#include <net/devlink.h>
 
 #include "qed.h"
 #include "qed_sriov.h"
@@ -343,107 +342,6 @@ static int qed_set_power_state(struct qed_dev *cdev, pci_power_t state)
 	return 0;
 }
 
-struct qed_devlink {
-	struct qed_dev *cdev;
-};
-
-enum qed_devlink_param_id {
-	QED_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
-	QED_DEVLINK_PARAM_ID_IWARP_CMT,
-};
-
-static int qed_dl_param_get(struct devlink *dl, u32 id,
-			    struct devlink_param_gset_ctx *ctx)
-{
-	struct qed_devlink *qed_dl;
-	struct qed_dev *cdev;
-
-	qed_dl = devlink_priv(dl);
-	cdev = qed_dl->cdev;
-	ctx->val.vbool = cdev->iwarp_cmt;
-
-	return 0;
-}
-
-static int qed_dl_param_set(struct devlink *dl, u32 id,
-			    struct devlink_param_gset_ctx *ctx)
-{
-	struct qed_devlink *qed_dl;
-	struct qed_dev *cdev;
-
-	qed_dl = devlink_priv(dl);
-	cdev = qed_dl->cdev;
-	cdev->iwarp_cmt = ctx->val.vbool;
-
-	return 0;
-}
-
-static const struct devlink_param qed_devlink_params[] = {
-	DEVLINK_PARAM_DRIVER(QED_DEVLINK_PARAM_ID_IWARP_CMT,
-			     "iwarp_cmt", DEVLINK_PARAM_TYPE_BOOL,
-			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
-			     qed_dl_param_get, qed_dl_param_set, NULL),
-};
-
-static const struct devlink_ops qed_dl_ops;
-
-static int qed_devlink_register(struct qed_dev *cdev)
-{
-	union devlink_param_value value;
-	struct qed_devlink *qed_dl;
-	struct devlink *dl;
-	int rc;
-
-	dl = devlink_alloc(&qed_dl_ops, sizeof(*qed_dl));
-	if (!dl)
-		return -ENOMEM;
-
-	qed_dl = devlink_priv(dl);
-
-	cdev->dl = dl;
-	qed_dl->cdev = cdev;
-
-	rc = devlink_register(dl, &cdev->pdev->dev);
-	if (rc)
-		goto err_free;
-
-	rc = devlink_params_register(dl, qed_devlink_params,
-				     ARRAY_SIZE(qed_devlink_params));
-	if (rc)
-		goto err_unregister;
-
-	value.vbool = false;
-	devlink_param_driverinit_value_set(dl,
-					   QED_DEVLINK_PARAM_ID_IWARP_CMT,
-					   value);
-
-	devlink_params_publish(dl);
-	cdev->iwarp_cmt = false;
-
-	return 0;
-
-err_unregister:
-	devlink_unregister(dl);
-
-err_free:
-	cdev->dl = NULL;
-	devlink_free(dl);
-
-	return rc;
-}
-
-static void qed_devlink_unregister(struct qed_dev *cdev)
-{
-	if (!cdev->dl)
-		return;
-
-	devlink_params_unregister(cdev->dl, qed_devlink_params,
-				  ARRAY_SIZE(qed_devlink_params));
-
-	devlink_unregister(cdev->dl);
-	devlink_free(cdev->dl);
-}
-
 /* probing */
 static struct qed_dev *qed_probe(struct pci_dev *pdev,
 				 struct qed_probe_params *params)
@@ -472,12 +370,6 @@ static struct qed_dev *qed_probe(struct pci_dev *pdev,
 	}
 	DP_INFO(cdev, "PCI init completed successfully\n");
 
-	rc = qed_devlink_register(cdev);
-	if (rc) {
-		DP_INFO(cdev, "Failed to register devlink.\n");
-		goto err2;
-	}
-
 	rc = qed_hw_prepare(cdev, QED_PCI_DEFAULT);
 	if (rc) {
 		DP_ERR(cdev, "hw prepare failed\n");
@@ -507,8 +399,6 @@ static void qed_remove(struct qed_dev *cdev)
 
 	qed_set_power_state(cdev, PCI_D3hot);
 
-	qed_devlink_unregister(cdev);
-
 	qed_free_cdev(cdev);
 }
 
@@ -2488,6 +2378,16 @@ static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)
 	return QED_AFFIN_HWFN_IDX(cdev);
 }
 
+static bool qed_get_iwarp_cmt(struct qed_dev *cdev)
+{
+	return cdev->iwarp_cmt;
+}
+
+static void qed_set_iwarp_cmt(struct qed_dev *cdev, bool iwarp_cmt)
+{
+	cdev->iwarp_cmt = iwarp_cmt;
+}
+
 static struct qed_selftest_ops qed_selftest_ops_pass = {
 	.selftest_memory = &qed_selftest_memory,
 	.selftest_interrupt = &qed_selftest_interrupt,
@@ -2536,6 +2436,8 @@ static u8 qed_get_affin_hwfn_idx(struct qed_dev *cdev)
 	.db_recovery_del = &qed_db_recovery_del,
 	.read_module_eeprom = &qed_read_module_eeprom,
 	.get_affin_hwfn_idx = &qed_get_affin_hwfn_idx,
+	.get_iwarp_cmt = &qed_get_iwarp_cmt,
+	.set_iwarp_cmt = &qed_set_iwarp_cmt,
 };
 
 void qed_get_protocol_stats(struct qed_dev *cdev,
diff --git a/drivers/net/ethernet/qlogic/qede/Makefile b/drivers/net/ethernet/qlogic/qede/Makefile
index 3fc91d1..4585864 100644
--- a/drivers/net/ethernet/qlogic/qede/Makefile
+++ b/drivers/net/ethernet/qlogic/qede/Makefile
@@ -1,6 +1,6 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_QEDE) := qede.o
 
-qede-y := qede_main.o qede_fp.o qede_filter.o qede_ethtool.o qede_ptp.o
+qede-y := qede_main.o qede_fp.o qede_filter.o qede_ethtool.o qede_ptp.o qede_devlink.o
 qede-$(CONFIG_DCB) += qede_dcbnl.o
 qede-$(CONFIG_QED_RDMA) += qede_rdma.o
diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
index b972ab0..35ad5cd 100644
--- a/drivers/net/ethernet/qlogic/qede/qede.h
+++ b/drivers/net/ethernet/qlogic/qede/qede.h
@@ -262,6 +262,8 @@ struct qede_dev {
 	struct qede_rdma_dev		rdma_info;
 
 	struct bpf_prog *xdp_prog;
+
+	struct devlink			*dl;
 };
 
 enum QEDE_STATE {
diff --git a/drivers/net/ethernet/qlogic/qede/qede_devlink.c b/drivers/net/ethernet/qlogic/qede/qede_devlink.c
new file mode 100644
index 0000000..3f362ac
--- /dev/null
+++ b/drivers/net/ethernet/qlogic/qede/qede_devlink.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "qede.h"
+#include "qede_devlink.h"
+
+static int qede_dl_param_get(struct devlink *dl, u32 id,
+			     struct devlink_param_gset_ctx *ctx)
+{
+	struct qede_devlink *qede_dl;
+	struct qede_dev *edev;
+
+	qede_dl = devlink_priv(dl);
+	edev = qede_dl->edev;
+	ctx->val.vbool = edev->ops->common->get_iwarp_cmt(edev->cdev);
+
+	return 0;
+}
+
+static int qede_dl_param_set(struct devlink *dl, u32 id,
+			     struct devlink_param_gset_ctx *ctx)
+{
+	struct qede_devlink *qede_dl;
+	struct qede_dev *edev;
+
+	qede_dl = devlink_priv(dl);
+	edev = qede_dl->edev;
+	edev->ops->common->set_iwarp_cmt(edev->cdev, ctx->val.vbool);
+
+	return 0;
+}
+
+static const struct devlink_param qede_devlink_params[] = {
+	DEVLINK_PARAM_DRIVER(QEDE_DEVLINK_PARAM_ID_IWARP_CMT,
+			     "iwarp_cmt", DEVLINK_PARAM_TYPE_BOOL,
+			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+			     qede_dl_param_get, qede_dl_param_set, NULL),
+};
+
+static const struct devlink_ops qede_dl_ops;
+
+int qede_devlink_register(struct qede_dev *edev)
+{
+	union devlink_param_value value;
+	struct qede_devlink *qede_dl;
+	struct devlink *dl;
+	int rc;
+
+	dl = devlink_alloc(&qede_dl_ops, sizeof(*qede_dl));
+	if (!dl)
+		return -ENOMEM;
+
+	qede_dl = devlink_priv(dl);
+
+	edev->dl = dl;
+	qede_dl->edev = edev;
+
+	rc = devlink_register(dl, &edev->pdev->dev);
+	if (rc)
+		goto err_free;
+
+	rc = devlink_params_register(dl, qede_devlink_params,
+				     ARRAY_SIZE(qede_devlink_params));
+	if (rc)
+		goto err_unregister;
+
+	value.vbool = false;
+	devlink_param_driverinit_value_set(dl, QEDE_DEVLINK_PARAM_ID_IWARP_CMT,
+					   value);
+
+	devlink_params_publish(dl);
+	edev->ops->common->set_iwarp_cmt(edev->cdev, false);
+
+	return 0;
+
+err_unregister:
+	devlink_unregister(dl);
+
+err_free:
+	edev->dl = NULL;
+	devlink_free(dl);
+
+	return rc;
+}
+
+void qede_devlink_unregister(struct qede_dev *edev)
+{
+	if (!edev->dl)
+		return;
+
+	devlink_params_unregister(edev->dl, qede_devlink_params,
+				  ARRAY_SIZE(qede_devlink_params));
+
+	devlink_unregister(edev->dl);
+	devlink_free(edev->dl);
+}
diff --git a/drivers/net/ethernet/qlogic/qede/qede_devlink.h b/drivers/net/ethernet/qlogic/qede/qede_devlink.h
new file mode 100644
index 0000000..5aa79dd
--- /dev/null
+++ b/drivers/net/ethernet/qlogic/qede/qede_devlink.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _QEDE_DEVLINK_H
+#define _QEDE_DEVLINK_H
+#include <net/devlink.h>
+
+struct qede_devlink {
+	struct qede_dev *edev;
+};
+
+enum qede_devlink_param_id {
+	QEDE_DEVLINK_PARAM_ID_BASE = DEVLINK_PARAM_GENERIC_ID_MAX,
+	QEDE_DEVLINK_PARAM_ID_IWARP_CMT,
+};
+
+int qede_devlink_register(struct qede_dev *edev);
+void qede_devlink_unregister(struct qede_dev *edev);
+
+#endif
diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
index d4a2966..cef946f 100644
--- a/drivers/net/ethernet/qlogic/qede/qede_main.c
+++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
@@ -62,6 +62,7 @@
 #include <linux/vmalloc.h>
 #include "qede.h"
 #include "qede_ptp.h"
+#include "qede_devlink.h"
 
 static char version[] =
 	"QLogic FastLinQ 4xxxx Ethernet Driver qede " DRV_MODULE_VERSION "\n";
@@ -1177,8 +1178,18 @@ static int __qede_probe(struct pci_dev *pdev, u32 dp_module, u8 dp_level,
 	edev->rx_copybreak = QEDE_RX_HDR_SIZE;
 
 	qede_log_probe(edev);
+
+	rc = qede_devlink_register(edev);
+	if (rc) {
+		DP_INFO(edev, "Failed to register devlink.\n");
+		goto err5;
+	}
+
 	return 0;
 
+err5:
+	if (!is_vf)
+		qede_ptp_disable(edev);
 err4:
 	qede_rdma_dev_remove(edev, (mode == QEDE_PROBE_RECOVERY));
 err3:
@@ -1227,6 +1238,8 @@ static void __qede_remove(struct pci_dev *pdev, enum qede_remove_mode mode)
 
 	DP_INFO(edev, "Starting qede_remove\n");
 
+	qede_devlink_unregister(edev);
+
 	qede_rdma_dev_remove(edev, (mode == QEDE_REMOVE_RECOVERY));
 
 	if (mode != QEDE_REMOVE_RECOVERY) {
diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
index eef02e6..7c41304 100644
--- a/include/linux/qed/qed_if.h
+++ b/include/linux/qed/qed_if.h
@@ -1131,6 +1131,9 @@ struct qed_common_ops {
  * @param cdev
  */
 	u8 (*get_affin_hwfn_idx)(struct qed_dev *cdev);
+
+	bool (*get_iwarp_cmt)(struct qed_dev *cdev);
+	void (*set_iwarp_cmt)(struct qed_dev *cdev, bool iwarp_cmt);
 };
 
 #define MASK_FIELD(_name, _value) \
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net-next v2 2/4] qed: Add APIs for device attributes configuration.
From: Sudarsana Reddy Kalluru @ 2019-07-04 13:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190704132011.13600-1-skalluru@marvell.com>

The patch adds driver APIs for reading/configuring the device attributes.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 drivers/net/ethernet/qlogic/qed/qed_hsi.h | 17 ++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.c | 64 +++++++++++++++++++++++++++++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h | 14 +++++++
 3 files changed, 95 insertions(+)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_hsi.h b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
index e054f6c..557a12e 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hsi.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_hsi.h
@@ -12580,6 +12580,8 @@ struct public_drv_mb {
 #define DRV_MSG_CODE_BW_UPDATE_ACK		0x32000000
 #define DRV_MSG_CODE_NIG_DRAIN			0x30000000
 #define DRV_MSG_CODE_S_TAG_UPDATE_ACK		0x3b000000
+#define DRV_MSG_CODE_GET_NVM_CFG_OPTION		0x003e0000
+#define DRV_MSG_CODE_SET_NVM_CFG_OPTION		0x003f0000
 #define DRV_MSG_CODE_INITIATE_PF_FLR            0x02010000
 #define DRV_MSG_CODE_VF_DISABLED_DONE		0xc0000000
 #define DRV_MSG_CODE_CFG_VF_MSIX		0xc0010000
@@ -12748,6 +12750,21 @@ struct public_drv_mb {
 #define DRV_MB_PARAM_FEATURE_SUPPORT_PORT_EEE		0x00000002
 #define DRV_MB_PARAM_FEATURE_SUPPORT_FUNC_VLINK		0x00010000
 
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ID_SHIFT		0
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ID_MASK		0x0000FFFF
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ALL_SHIFT		16
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ALL_MASK		0x00010000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_INIT_SHIFT		17
+#define DRV_MB_PARAM_NVM_CFG_OPTION_INIT_MASK		0x00020000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT_SHIFT	18
+#define DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT_MASK		0x00040000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_FREE_SHIFT		19
+#define DRV_MB_PARAM_NVM_CFG_OPTION_FREE_MASK		0x00080000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL_SHIFT	20
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL_MASK	0x00100000
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID_SHIFT	24
+#define DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID_MASK	0x0f000000
+
 	u32 fw_mb_header;
 #define FW_MSG_CODE_MASK			0xffff0000
 #define FW_MSG_CODE_UNSUPPORTED                 0x00000000
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index 758702c..573911a 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -3750,3 +3750,67 @@ int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 
 	return 0;
 }
+
+int qed_mcp_nvm_get_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 *p_len)
+{
+	u32 mb_param = 0, resp, param;
+	int rc;
+
+	QED_MFW_SET_FIELD(mb_param, DRV_MB_PARAM_NVM_CFG_OPTION_ID, option_id);
+	if (flags & QED_NVM_CFG_OPTION_INIT)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_INIT, 1);
+	if (flags & QED_NVM_CFG_OPTION_FREE)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_FREE, 1);
+	if (flags & QED_NVM_CFG_OPTION_ENTITY_SEL) {
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL, 1);
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID,
+				  entity_id);
+	}
+
+	rc = qed_mcp_nvm_rd_cmd(p_hwfn, p_ptt,
+				DRV_MSG_CODE_GET_NVM_CFG_OPTION,
+				mb_param, &resp, &param, p_len, (u32 *)p_buf);
+
+	return rc;
+}
+
+int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 len)
+{
+	u32 mb_param = 0, resp, param;
+	int rc;
+
+	QED_MFW_SET_FIELD(mb_param, DRV_MB_PARAM_NVM_CFG_OPTION_ID, option_id);
+	if (flags & QED_NVM_CFG_OPTION_ALL)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ALL, 1);
+	if (flags & QED_NVM_CFG_OPTION_INIT)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_INIT, 1);
+	if (flags & QED_NVM_CFG_OPTION_COMMIT)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_COMMIT, 1);
+	if (flags & QED_NVM_CFG_OPTION_FREE)
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_FREE, 1);
+	if (flags & QED_NVM_CFG_OPTION_ENTITY_SEL) {
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_SEL, 1);
+		QED_MFW_SET_FIELD(mb_param,
+				  DRV_MB_PARAM_NVM_CFG_OPTION_ENTITY_ID,
+				  entity_id);
+	}
+
+	rc = qed_mcp_nvm_wr_cmd(p_hwfn, p_ptt,
+				DRV_MSG_CODE_SET_NVM_CFG_OPTION,
+				mb_param, &resp, &param, len, (u32 *)p_buf);
+
+	return rc;
+}
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.h b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
index e4f8fe4..550b4dd 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.h
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.h
@@ -251,6 +251,12 @@ struct qed_mfw_tlv_generic {
 	struct qed_mfw_tlv_iscsi iscsi;
 };
 
+#define QED_NVM_CFG_OPTION_ALL		BIT(0)
+#define QED_NVM_CFG_OPTION_INIT		BIT(1)
+#define QED_NVM_CFG_OPTION_COMMIT       BIT(2)
+#define QED_NVM_CFG_OPTION_FREE		BIT(3)
+#define QED_NVM_CFG_OPTION_ENTITY_SEL	BIT(4)
+
 /**
  * @brief - returns the link params of the hw function
  *
@@ -1202,4 +1208,12 @@ void qed_mcp_resc_lock_default_init(struct qed_resc_lock_params *p_lock,
  */
 int qed_mcp_get_ppfid_bitmap(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt);
 
+int qed_mcp_nvm_get_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 *p_len);
+
+int qed_mcp_nvm_set_cfg(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt,
+			u16 option_id, u8 entity_id, u16 flags, u8 *p_buf,
+			u32 len);
+
 #endif
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net-next v2 1/4] devlink: Add APIs to publish/unpublish the port parameters.
From: Sudarsana Reddy Kalluru @ 2019-07-04 13:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, mkalderon, aelior
In-Reply-To: <20190704132011.13600-1-skalluru@marvell.com>

Kernel has no interface to publish the devlink port parameters. This is
required for exporting the port params to the user space, so that user
can read or update the port params.
This patch adds devlink interfaces (for drivers) to publish/unpublish the
devlink port parameters.

Signed-off-by: Sudarsana Reddy Kalluru <skalluru@marvell.com>
Signed-off-by: Ariel Elior <aelior@marvell.com>
---
 include/net/devlink.h |  2 ++
 net/core/devlink.c    | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 6625ea0..47a1e8f 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -653,6 +653,8 @@ int devlink_port_params_register(struct devlink_port *devlink_port,
 void devlink_port_params_unregister(struct devlink_port *devlink_port,
 				    const struct devlink_param *params,
 				    size_t params_count);
+void devlink_port_params_publish(struct devlink_port *devlink_port);
+void devlink_port_params_unpublish(struct devlink_port *ddevlink_port);
 int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 				       union devlink_param_value *init_val);
 int devlink_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 89c5337..0cd7994 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -6380,6 +6380,48 @@ void devlink_port_params_unregister(struct devlink_port *devlink_port,
 }
 EXPORT_SYMBOL_GPL(devlink_port_params_unregister);
 
+/**
+ *	devlink_port_params_publish - publish port configuration parameters
+ *
+ *	@devlink_port: devlink port
+ *
+ *	Publish previously registered port configuration parameters.
+ */
+void devlink_port_params_publish(struct devlink_port *devlink_port)
+{
+	struct devlink_param_item *param_item;
+
+	list_for_each_entry(param_item, &devlink_port->param_list, list) {
+		if (param_item->published)
+			continue;
+		param_item->published = true;
+		devlink_param_notify(devlink_port->devlink, devlink_port->index,
+				     param_item, DEVLINK_CMD_PORT_PARAM_NEW);
+	}
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_publish);
+
+/**
+ *	devlink_port_params_unpublish - unpublish port configuration parameters
+ *
+ *	@devlink_port: devlink port
+ *
+ *	Unpublish previously registered port configuration parameters.
+ */
+void devlink_port_params_unpublish(struct devlink_port *devlink_port)
+{
+	struct devlink_param_item *param_item;
+
+	list_for_each_entry(param_item, &devlink_port->param_list, list) {
+		if (!param_item->published)
+			continue;
+		param_item->published = false;
+		devlink_param_notify(devlink_port->devlink, devlink_port->index,
+				     param_item, DEVLINK_CMD_PORT_PARAM_DEL);
+	}
+}
+EXPORT_SYMBOL_GPL(devlink_port_params_unpublish);
+
 static int
 __devlink_param_driverinit_value_get(struct list_head *param_list, u32 param_id,
 				     union devlink_param_value *init_val)
-- 
1.8.3.1


^ permalink raw reply related

* [PATCH net-next v2 0/4] qed*/devlink: Devlink support for config attributes.
From: Sudarsana Reddy Kalluru @ 2019-07-04 13:20 UTC (permalink / raw)
  To: davem; +Cc: netdev, mkalderon, aelior

The patch series adds support for managing the config attributes using
devlink interfaces.

Patch (1) adds the APIs for publishing the devlink port params. Clubbing
this qed patches as per the review comment (i.e., need at least one
consumer for the new APIs).

Patches (2)-(4) adds the qed/qede devlink support for managing the
device/port attributes.

Please consider applying it to 'net-next' tree.

Sudarsana Reddy Kalluru (4):
  devlink: Add APIs to publish/unpublish the port parameters.
  qed: Add APIs for device attributes configuration.
  qed*: Add new file for devlink implementation.
  qed*: Add devlink support for configuration attributes.

 Documentation/networking/devlink-params-qede.txt |  72 ++++++
 drivers/net/ethernet/qlogic/qed/qed.h            |   1 -
 drivers/net/ethernet/qlogic/qed/qed_hsi.h        |  17 ++
 drivers/net/ethernet/qlogic/qed/qed_main.c       | 160 ++++--------
 drivers/net/ethernet/qlogic/qed/qed_mcp.c        |  64 +++++
 drivers/net/ethernet/qlogic/qed/qed_mcp.h        |  14 ++
 drivers/net/ethernet/qlogic/qede/Makefile        |   2 +-
 drivers/net/ethernet/qlogic/qede/qede.h          |   5 +
 drivers/net/ethernet/qlogic/qede/qede_devlink.c  | 294 +++++++++++++++++++++++
 drivers/net/ethernet/qlogic/qede/qede_devlink.h  |  41 ++++
 drivers/net/ethernet/qlogic/qede/qede_main.c     |  13 +
 include/linux/qed/qed_if.h                       |  19 ++
 include/net/devlink.h                            |   2 +
 net/core/devlink.c                               |  42 ++++
 14 files changed, 634 insertions(+), 112 deletions(-)
 create mode 100644 Documentation/networking/devlink-params-qede.txt
 create mode 100644 drivers/net/ethernet/qlogic/qede/qede_devlink.c
 create mode 100644 drivers/net/ethernet/qlogic/qede/qede_devlink.h

-- 
1.8.3.1


^ permalink raw reply

* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Johannes Berg @ 2019-07-04 13:10 UTC (permalink / raw)
  To: Michal Kubecek, netdev
  Cc: Jiri Pirko, David Miller, Jakub Kicinski, Andrew Lunn,
	Florian Fainelli, John Linville, Stephen Hemminger, linux-kernel
In-Reply-To: <20190704125315.GT20101@unicorn.suse.cz>

On Thu, 2019-07-04 at 14:53 +0200, Michal Kubecek wrote:
> 
> >  value: 0b00000000'000000xx'xxxxxxxx'xxxxxxxx
> >  mask:  0b00000000'00000011'11111111'11111111
> 
> One scenario that I can see from the top of my head would be user
> running
> 
>   ethtool -s <dev> advertise 0x...

The "0x..." here would be the *value* in the NLA_BITFIELD32 parlance,
right?

What would the "selector" be? I assume the selector would be "whatever
ethtool knows about"?

> with hex value representing some subset of link modes. Now if ethtool
> version is behind kernel and recognizes fewer link modes than kernel
> but in a way that the number rounded up to bytes or words would be the
> same, kernel has no way to recognize of those zero bits on top of the
> mask are zero on purpose or just because userspace doesn't know about
> them. In general, I believe the absence of bit length information is
> something protocols would have to work around sometimes.
> 
> The submitted implementation doesn't have this problem as it can tell
> kernel "this is a list" (i.e. I'm not sending a value/mask pair, I want
> exactly these bits to be set).

OK, here I guess I see what you mean. You're saying if ethtool were to
send a value/mask of "0..0100/0..0111" you wouldn't know what to do with
BIT(4) as long as the kernel knows about that bit?

I guess the difference now is depending on the operation. NLA_BITFIELD32
is sort of built on the assumption of having a "toggle" operation. If
you want to have a "set to" operation, then you don't really need the
selector/mask at all, just the value.

johannes


^ permalink raw reply

* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Ilias Apalodimas @ 2019-07-04 13:06 UTC (permalink / raw)
  To: Jose Abreu
  Cc: Jesper Dangaard Brouer, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, Joao Pinto,
	David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
	Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai
In-Reply-To: <BYAPR12MB3269D4FAAC5307A224D60A08D3FA0@BYAPR12MB3269.namprd12.prod.outlook.com>

Hi Jose, 

> Thank you all for your review comments !
> 
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> 
> > That's why i was concerned on what will happen on > 1000b frames and what the
> > memory pressure is going to be. 
> > The trade off here is copying vs mapping/unmapping.
> 
> Well, the performance numbers I mentioned are for TSO with default MTU 
> (1500) and using iperf3 with zero-copy. Here follows netperf:
> 

Ok i guess this should be fine. Here's why.
You'll allocate an extra memory from page pool API which equals
the number of descriptors * 1 page.
You also allocate SKB's to copy the data and recycle the page pool buffers.
So page_pool won't add any significant memory pressure since we expect *all*
it's buffers to be recycled. 
The SKBs are allocated anyway in the current driver so bottom line you trade off
some memory (the page_pool buffers) + a memcpy per packet and skip the dma
map/unmap which is the bottleneck in your hardware. 
I think it's fine

Cheers
/Ilias

^ permalink raw reply

* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Peter Zijlstra @ 2019-07-04 13:05 UTC (permalink / raw)
  To: Kris Van Hees
  Cc: netdev, bpf, dtrace-devel, linux-kernel, rostedt, mhiramat, acme,
	ast, daniel, Chris Mason
In-Reply-To: <201907040314.x643EUoA017906@aserv0122.oracle.com>

On Wed, Jul 03, 2019 at 08:14:30PM -0700, Kris Van Hees wrote:
> +int dt_bpf_attach(int event_id, int bpf_fd)
> +{
> +	int			event_fd;
> +	int			rc;
> +	struct perf_event_attr	attr = {};
> +
> +	attr.type = PERF_TYPE_TRACEPOINT;
> +	attr.sample_type = PERF_SAMPLE_RAW;
> +	attr.sample_period = 1;
> +	attr.wakeup_events = 1;
> +	attr.config = event_id;
> +
> +	/* Register the event (based on its id), and obtain a fd. */
> +	event_fd = perf_event_open(&attr, -1, 0, -1, 0);
> +	if (event_fd < 0) {
> +		perror("sys_perf_event_open");
> +		return -1;
> +	}
> +
> +	/* Enable the probe. */
> +	rc = ioctl(event_fd, PERF_EVENT_IOC_ENABLE, 0);

AFAICT you didn't use attr.disabled = 1, so this IOC_ENABLE is
completely superfluous.

> +	if (rc < 0) {
> +		perror("PERF_EVENT_IOC_ENABLE");
> +		return -1;
> +	}
> +
> +	/* Associate the BPF program with the event. */
> +	rc = ioctl(event_fd, PERF_EVENT_IOC_SET_BPF, bpf_fd);
> +	if (rc < 0) {
> +		perror("PERF_EVENT_IOC_SET_BPF");
> +		return -1;
> +	}
> +
> +	return 0;
> +}

^ permalink raw reply

* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Peter Zijlstra @ 2019-07-04 13:03 UTC (permalink / raw)
  To: Kris Van Hees
  Cc: netdev, bpf, dtrace-devel, linux-kernel, rostedt, mhiramat, acme,
	ast, daniel, Chris Mason
In-Reply-To: <201907040314.x643EUoA017906@aserv0122.oracle.com>

On Wed, Jul 03, 2019 at 08:14:30PM -0700, Kris Van Hees wrote:
> +/*
> + * Read the data_head offset from the header page of the ring buffer.  The
> + * argument is declared 'volatile' because it references a memory mapped page
> + * that the kernel may be writing to while we access it here.
> + */
> +static u64 read_rb_head(volatile struct perf_event_mmap_page *rb_page)
> +{
> +	u64	head = rb_page->data_head;
> +
> +	asm volatile("" ::: "memory");
> +
> +	return head;
> +}
> +
> +/*
> + * Write the data_tail offset in the header page of the ring buffer.  The
> + * argument is declared 'volatile' because it references a memory mapped page
> + * that the kernel may be writing to while we access it here.

s/writing/reading/

> + */
> +static void write_rb_tail(volatile struct perf_event_mmap_page *rb_page,
> +			  u64 tail)
> +{
> +	asm volatile("" ::: "memory");
> +
> +	rb_page->data_tail = tail;
> +}

That volatile usage is atrocious (kernel style would have you use
{READ,WRITE}_ONCE()). Also your comments fail to mark these as
load_acquire and store_release. And by only using a compiler barrier
you're hard assuming TSO, which is somewhat fragile at best.

Alternatively, you can use the C11 bits and write:

	return __atomic_load_n(&rb_page->data_head, __ATOMIC_ACQUIRE);

	__atomic_store_n(&rb_page->data_tail, tail, __ATOMIC_RELEASE);

> +/*
> + * Process and output the probe data at the supplied address.
> + */
> +static int output_event(int cpu, u64 *buf)
> +{
> +	u8				*data = (u8 *)buf;
> +	struct perf_event_header	*hdr;
> +
> +	hdr = (struct perf_event_header *)data;
> +	data += sizeof(struct perf_event_header);
> +
> +	if (hdr->type == PERF_RECORD_SAMPLE) {
> +		u8		*ptr = data;
> +		u32		i, size, probe_id;
> +
> +		/*
> +		 * struct {
> +		 *	struct perf_event_header	header;
> +		 *	u32				size;
> +		 *	u32				probe_id;
> +		 *	u32				gap;
> +		 *	u64				data[n];
> +		 * }
> +		 * and data points to the 'size' member at this point.
> +		 */
> +		if (ptr > (u8 *)buf + hdr->size) {
> +			fprintf(stderr, "BAD: corrupted sample header\n");
> +			goto out;
> +		}
> +
> +		size = *(u32 *)data;
> +		data += sizeof(size);
> +		ptr += sizeof(size) + size;
> +		if (ptr != (u8 *)buf + hdr->size) {
> +			fprintf(stderr, "BAD: invalid sample size\n");
> +			goto out;
> +		}
> +
> +		probe_id = *(u32 *)data;
> +		data += sizeof(probe_id);
> +		size -= sizeof(probe_id);
> +		data += sizeof(u32);		/* skip 32-bit gap */
> +		size -= sizeof(u32);
> +		buf = (u64 *)data;
> +
> +		printf("%3d %6d ", cpu, probe_id);
> +		for (i = 0, size /= sizeof(u64); i < size; i++)
> +			printf("%#016lx ", buf[i]);
> +		printf("\n");
> +	} else if (hdr->type == PERF_RECORD_LOST) {
> +		u64	lost;
> +
> +		/*
> +		 * struct {
> +		 *	struct perf_event_header	header;
> +		 *	u64				id;
> +		 *	u64				lost;
> +		 * }
> +		 * and data points to the 'id' member at this point.
> +		 */
> +		lost = *(u64 *)(data + sizeof(u64));
> +
> +		printf("[%ld probes dropped]\n", lost);
> +	} else
> +		fprintf(stderr, "UNKNOWN: record type %d\n", hdr->type);
> +
> +out:
> +	return hdr->size;
> +}

I see a distinct lack of wrapping support. AFAICT when buf+hdr->size
wraps you're doing out-of-bounds accesses.

> +/*
> + * Process the available probe data in the given buffer.
> + */
> +static void process_data(struct dtrace_buffer *buf)
> +{
> +	/* This is volatile because the kernel may be updating the content. */
> +	volatile struct perf_event_mmap_page	*rb_page = buf->base;
> +	u8					*base = (u8 *)buf->base +
> +							buf->page_size;
> +	u64					head = read_rb_head(rb_page);
> +
> +	while (rb_page->data_tail != head) {
> +		u64	tail = rb_page->data_tail;
> +		u64	*ptr = (u64 *)(base + tail % buf->data_size);
> +		int	len;
> +
> +		len = output_event(buf->cpu, ptr);
> +
> +		write_rb_tail(rb_page, tail + len);
> +		head = read_rb_head(rb_page);
> +	}
> +}

more volatile yuck.

Also:

	for (;;) {
		head = __atomic_load_n(&rb_page->data_head, __ATOMIC_ACQUIRE);
		tail = __atomic_load_n(&rb_page->data_tail, __ATOMIC_RELAXED);

		if (head == tail)
			break;

		do {
			hdr = buf->base + (tail & ((1UL << buf->data_shift) - 1));
			if ((tail >> buf->data_shift) !=
			    ((tail + hdr->size) >> buf->data_shift))
				/* handle wrap case */
			else
				/* normal case */

			tail += hdr->size;
		} while (tail != head);

		__atomic_store_n(&rb_page->data_tail, tail, __ATOMIC_RELEASE);
	}

Or something.

> +/*
> + * Wait for data to become available in any of the buffers.
> + */
> +int dt_buffer_poll(int epoll_fd, int timeout)
> +{
> +	struct epoll_event	events[dt_numcpus];
> +	int			i, cnt;
> +
> +	cnt = epoll_wait(epoll_fd, events, dt_numcpus, timeout);
> +	if (cnt < 0)
> +		return -errno;
> +
> +	for (i = 0; i < cnt; i++)
> +		process_data((struct dtrace_buffer *)events[i].data.ptr);
> +
> +	return cnt;
> +}

Or make sure to read on the CPU by having a poll thread per CPU, then
you can do away with the memory barriers.

^ permalink raw reply

* RE: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Jose Abreu @ 2019-07-04 12:59 UTC (permalink / raw)
  To: Ilias Apalodimas, Jesper Dangaard Brouer
  Cc: Jose Abreu, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org, Joao Pinto,
	David S . Miller, Giuseppe Cavallaro, Alexandre Torgue,
	Maxime Coquelin, Maxime Ripard, Chen-Yu Tsai
In-Reply-To: <20190704120441.GA6866@apalos>

Thank you all for your review comments !

From: Ilias Apalodimas <ilias.apalodimas@linaro.org>

> That's why i was concerned on what will happen on > 1000b frames and what the
> memory pressure is going to be. 
> The trade off here is copying vs mapping/unmapping.

Well, the performance numbers I mentioned are for TSO with default MTU 
(1500) and using iperf3 with zero-copy. Here follows netperf:

---
# netperf -c -C -H 1.2.3.2 -T 7,7 -t TCP_SENDFILE
TCP SENDFILE TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 1.2.3.2 
(1.2.3.2) port 0 AF_INET : demo : cpu bind
Recv   Send    Send                          Utilization       Service 
Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    
Recv
Size   Size    Size     Time     Throughput  local    remote   local   
remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   
us/KB

131072  16384  16384    10.00      9132.37   6.13     11.79    0.440   
0.846  

---
# netperf -c -C -H 1.2.3.2 -T 7,7 -t TCP_STREAM
MIGRATED TCP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 
1.2.3.2 (1.2.3.2) port 0 AF_INET : demo : cpu bind
Recv   Send    Send                          Utilization       Service 
Demand
Socket Socket  Message  Elapsed              Send     Recv     Send    
Recv
Size   Size    Size     Time     Throughput  local    remote   local   
remote
bytes  bytes   bytes    secs.    10^6bits/s  % S      % S      us/KB   
us/KB

131072  16384  16384    10.01      9041.21   3.20     11.75    0.232   
0.852  

---
# netperf -c -C -H 1.2.3.2 -T 7,7 -t UDP_STREAM
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 
1.2.3.2 (1.2.3.2) port 0 AF_INET : demo : cpu bind
Socket  Message  Elapsed      Messages                   CPU      
Service
Size    Size     Time         Okay Errors   Throughput   Util     Demand
bytes   bytes    secs            #      #   10^6bits/sec % SS     us/KB

212992   65507   10.00      114455      0     5997.0     12.55    1.371 
212992           10.00      114455            5997.0     8.12     0.887

---
# netperf -c -C -H 1.2.3.2 -T 7,7 -t UDP_STREAM -- -m 64
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 
1.2.3.2 (1.2.3.2) port 0 AF_INET : demo : cpu bind
Socket  Message  Elapsed      Messages                   CPU      
Service
Size    Size     Time         Okay Errors   Throughput   Util     Demand
bytes   bytes    secs            #      #   10^6bits/sec % SS     us/KB

212992      64   10.00     4013480      0      205.4     12.51    39.918
212992           10.00     4013480             205.4     7.99     25.482

---
# netperf -c -C -H 1.2.3.2 -T 7,7 -t UDP_STREAM -- -m 128
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 
1.2.3.2 (1.2.3.2) port 0 AF_INET : demo : cpu bind
Socket  Message  Elapsed      Messages                   CPU      
Service
Size    Size     Time         Okay Errors   Throughput   Util     Demand
bytes   bytes    secs            #      #   10^6bits/sec % SS     us/KB

212992     128   10.00     3950480      0      404.4     12.50    20.255
212992           10.00     3950442             404.4     7.70     12.485

---
# netperf -c -C -H 1.2.3.2 -T 7,7 -t UDP_STREAM -- -m 1024
MIGRATED UDP STREAM TEST from 0.0.0.0 (0.0.0.0) port 0 AF_INET to 
1.2.3.2 (1.2.3.2) port 0 AF_INET : demo : cpu bind
Socket  Message  Elapsed      Messages                   CPU      
Service
Size    Size     Time         Okay Errors   Throughput   Util     Demand
bytes   bytes    secs            #      #   10^6bits/sec % SS     us/KB

212992    1024   10.00     3466506      0     2838.8     12.50    2.886 
212992           10.00     3466506            2838.8     7.39     1.707

^ permalink raw reply

* Re: [PATCH net-next v6 06/15] ethtool: netlink bitset handling
From: Michal Kubecek @ 2019-07-04 12:53 UTC (permalink / raw)
  To: netdev
  Cc: Johannes Berg, Jiri Pirko, David Miller, Jakub Kicinski,
	Andrew Lunn, Florian Fainelli, John Linville, Stephen Hemminger,
	linux-kernel
In-Reply-To: <2f1a8edb0b000b4eb7adcaca0d1fb05fdd73a587.camel@sipsolutions.net>

On Thu, Jul 04, 2019 at 02:21:52PM +0200, Johannes Berg wrote:
> On Thu, 2019-07-04 at 14:17 +0200, Michal Kubecek wrote:
> > On Thu, Jul 04, 2019 at 02:03:02PM +0200, Johannes Berg wrote:
> > > On Thu, 2019-07-04 at 13:52 +0200, Michal Kubecek wrote:
> > > > 
> > > > There is still the question if it it should be implemented as a nested
> > > > attribute which could look like the current compact form without the
> > > > "list" flag (if there is no mask, it's a list). Or an unstructured data
> > > > block consisting of u32 bit length 
> > > 
> > > You wouldn't really need the length, since the attribute has a length
> > > already :-)
> > 
> > It has byte length, not bit length. The bitmaps we are dealing with
> > can have any bit length, not necessarily multiples of 8 (or even 32).
> 
> Not sure why that matters? You have the mask, so you don't really need
> to additionally say that you're only going up to a certain bit?
> 
> I mean, say you want to set some bits <=17, why would you need to say
> that they're <=17 if you have a
>  value: 0b00000000'000000xx'xxxxxxxx'xxxxxxxx
>  mask:  0b00000000'00000011'11111111'11111111

One scenario that I can see from the top of my head would be user
running

  ethtool -s <dev> advertise 0x...

with hex value representing some subset of link modes. Now if ethtool
version is behind kernel and recognizes fewer link modes than kernel
but in a way that the number rounded up to bytes or words would be the
same, kernel has no way to recognize of those zero bits on top of the
mask are zero on purpose or just because userspace doesn't know about
them. In general, I believe the absence of bit length information is
something protocols would have to work around sometimes.

The submitted implementation doesn't have this problem as it can tell
kernel "this is a list" (i.e. I'm not sending a value/mask pair, I want
exactly these bits to be set). Thus it can easily implement requests of
both types (value/mask or just value):

  ethtool -s <dev> advertise 0x2f
  ethtool -s <dev> advertise 0x08/0x0c
  ethtool -s <dev> advertise 100baseT/Full off 1000baseT/Full on

and could be as easily extended to support also

  ethtool -s <dev> advertise 100baseT/Full 1000baseT/Full

Michal

^ permalink raw reply

* Re: [PATCH net-next 3/3] net: stmmac: Introducing support for Page Pool
From: Ilias Apalodimas @ 2019-07-04 12:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jesper Dangaard Brouer, Jose Abreu, Joao Pinto, Alexandre Torgue,
	Maxime Ripard, Networking, Linux Kernel Mailing List,
	David S . Miller, Chen-Yu Tsai, Maxime Coquelin,
	Giuseppe Cavallaro, linux-stm32, Linux ARM
In-Reply-To: <CAK8P3a3GC6f-xHG7MqZRLhY66Ui4HQVi=4WXR703wqfMNY6A5A@mail.gmail.com>

On Thu, Jul 04, 2019 at 02:14:28PM +0200, Arnd Bergmann wrote:
> On Thu, Jul 4, 2019 at 12:31 PM Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> > > On Wed,  3 Jul 2019 12:37:50 +0200
> > > Jose Abreu <Jose.Abreu@synopsys.com> wrote:
> 
> > 1. page pool allocs packet. The API doesn't sync but i *think* you don't have to
> > explicitly since the CPU won't touch that buffer until the NAPI handler kicks
> > in. On the napi handler you need to dma_sync_single_for_cpu() and process the
> > packet.
> 
> > So bvottom line i *think* we can skip the dma_sync_single_for_device() on the
> > initial allocation *only*. If am terribly wrong please let me know :)
> 
> I think you have to do a sync_single_for_device /somewhere/ before the
> buffer is given to the device. On a non-cache-coherent machine with
> a write-back cache, there may be dirty cache lines that get written back
> after the device DMA's data into it (e.g. from a previous memset
> from before the buffer got freed), so you absolutely need to flush any
> dirty cache lines on it first.
Ok my bad here i forgot to add "when coherency is there", since the driver
i had in mind runs on such a device (i think this is configurable though so i'll
add the sync explicitly to make sure we won't break any configurations).

In general you are right, thanks for the explanation!
> You may also need to invalidate the cache lines in the following
> sync_single_for_cpu() to eliminate clean cache lines with stale data
> that got there when speculatively reading between the cache-invalidate
> and the DMA.
> 
>        Arnd


Thanks!
/Ilias

^ permalink raw reply

* Re: [PATCH 2/3] module: Fix up module_notifier return values.
From: Robert Richter @ 2019-07-04 12:48 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Frank Ch. Eigler, Mathieu Desnoyers, Jessica Yu, linux-kernel,
	Josh Poimboeuf, jikos, mbenes, Petr Mladek, Alexei Starovoitov,
	Daniel Borkmann, Andrew Morton, rostedt, Ingo Molnar,
	Martin KaFai Lau, Song Liu, Yonghong Song, paulmck,
	Joel Fernandes, Google, Ard Biesheuvel, Thomas Gleixner,
	oprofile-list, netdev, bpf
In-Reply-To: <20190625074214.GR3436@hirez.programming.kicks-ass.net>

On 25.06.19 09:42:14, Peter Zijlstra wrote:
> On Mon, Jun 24, 2019 at 04:58:10PM -0400, Frank Ch. Eigler wrote:

> > From peterz's comments, the patches, it's not obvious to me how one is
> > to choose between 0 (NOTIFY_DONE) and 1 (NOTIFY_OK) in the case of a
> > routine success.
> 
> I'm not sure either; what I think I choice was:
> 
>  - if I want to completely ignore the callback, use DONE (per the
>    "Don't care" comment).
> 
>  - if we finished the notifier without error, use OK or
>    notifier_from_errno(0).
> 
> But yes, its a bit of a shit interface.

It looks like it was rarely used in earlier kernels as some sort of
error detection for the notifier calls, e.g.:

1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-int profile_handoff_task(struct task_struct * task)
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-{
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-      int ret;
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-      read_lock(&handoff_lock);
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-      ret = notifier_call_chain(&task_free_notifier, 0, task);
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-      read_unlock(&handoff_lock);
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c:      return (ret == NOTIFY_OK) ? 1 : 0;
1da177e4c3f41524e886b7f1b8a0c1fc7321cac2:kernel/profile.c-}

So NOTIFY_OK was used to state there is no error, while NOTIFY_DONE
says the notifier was executed and there might have been errors. The
caller may distinguish the results then.

-Robert

^ permalink raw reply

* Re: [net-next 1/3] ice: Initialize and register platform device to provide RDMA
From: Jason Gunthorpe @ 2019-07-04 12:48 UTC (permalink / raw)
  To: Greg KH
  Cc: Jeff Kirsher, davem@davemloft.net, dledford@redhat.com,
	Tony Nguyen, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	nhorman@redhat.com, sassmann@redhat.com, poswald@suse.com,
	mustafa.ismail@intel.com, shiraz.saleem@intel.com, Dave Ertman,
	Andrew Bowers
In-Reply-To: <20190704124247.GA6807@kroah.com>

On Thu, Jul 04, 2019 at 02:42:47PM +0200, Greg KH wrote:
> On Thu, Jul 04, 2019 at 12:37:33PM +0000, Jason Gunthorpe wrote:
> > On Thu, Jul 04, 2019 at 02:29:50PM +0200, Greg KH wrote:
> > > On Thu, Jul 04, 2019 at 12:16:41PM +0000, Jason Gunthorpe wrote:
> > > > On Wed, Jul 03, 2019 at 07:12:50PM -0700, Jeff Kirsher wrote:
> > > > > From: Tony Nguyen <anthony.l.nguyen@intel.com>
> > > > > 
> > > > > The RDMA block does not advertise on the PCI bus or any other bus.
> > > > > Thus the ice driver needs to provide access to the RDMA hardware block
> > > > > via a virtual bus; utilize the platform bus to provide this access.
> > > > > 
> > > > > This patch initializes the driver to support RDMA as well as creates
> > > > > and registers a platform device for the RDMA driver to register to. At
> > > > > this point the driver is fully initialized to register a platform
> > > > > driver, however, can not yet register as the ops have not been
> > > > > implemented.
> > > > 
> > > > I think you need Greg's ack on all this driver stuff - particularly
> > > > that a platform_device is OK.
> > > 
> > > A platform_device is almost NEVER ok.
> > > 
> > > Don't abuse it, make a real device on a real bus.  If you don't have a
> > > real bus and just need to create a device to hang other things off of,
> > > then use the virtual one, that's what it is there for.
> > 
> > Ideally I'd like to see all the RDMA drivers that connect to ethernet
> > drivers use some similar scheme.
> 
> Why?  They should be attached to a "real" device, why make any up?

? A "real" device, like struct pci_device, can only bind to one
driver. How can we bind it concurrently to net, rdma, scsi, etc?

> > This is for a PCI device that plugs into multiple subsystems in the
> > kernel, ie it has net driver functionality, rdma functionality, some
> > even have SCSI functionality
> 
> Sounds like a MFD device, why aren't you using that functionality
> instead?

This was also my advice, but in another email Jeff says:

  MFD architecture was also considered, and we selected the simpler
  platform model. Supporting a MFD architecture would require an
  additional MFD core driver, individual platform netdev, RDMA function
  drivers, and stripping a large portion of the netdev drivers into
  MFD core. The sub-devices registered by MFD core for function
  drivers are indeed platform devices.  

Thanks,
Jason

^ permalink raw reply

* Re: [PATCH net-next 8/8] net: mscc: PTP Hardware Clock (PHC) support
From: kbuild test robot @ 2019-07-04 12:48 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: kbuild-all, davem, richardcochran, alexandre.belloni,
	UNGLinuxDriver, ralf, paul.burton, jhogan, Antoine Tenart, netdev,
	linux-mips, thomas.petazzoni, allan.nielsen
In-Reply-To: <20190701100327.6425-9-antoine.tenart@bootlin.com>

[-- Attachment #1: Type: text/plain, Size: 4549 bytes --]

Hi Antoine,

I love your patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Antoine-Tenart/net-mscc-PTP-Hardware-Clock-PHC-support/20190702-182042
config: x86_64-randconfig-s2-07041950 (attached as .config)
compiler: gcc-7 (Debian 7.4.0-9) 7.4.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>

Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings

All warnings (new ones prefixed by >>):

   In file included from include/linux/irqflags.h:16:0,
                    from arch/x86/include/asm/processor.h:33,
                    from arch/x86/include/asm/cpufeature.h:5,
                    from arch/x86/include/asm/thread_info.h:53,
                    from include/linux/thread_info.h:38,
                    from arch/x86/include/asm/preempt.h:7,
                    from include/linux/preempt.h:78,
                    from include/linux/spinlock.h:51,
                    from include/linux/seqlock.h:36,
                    from include/linux/time.h:6,
                    from include/linux/skbuff.h:15,
                    from include/linux/if_ether.h:19,
                    from include/linux/etherdevice.h:20,
                    from drivers/net/ethernet/mscc/ocelot.c:7:
   drivers/net/ethernet/mscc/ocelot.c: In function 'ocelot_ptp_adjfine':
>> arch/x86/include/asm/irqflags.h:41:2: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
     asm volatile("push %0 ; popf"
     ^~~
   drivers/net/ethernet/mscc/ocelot.c:1904:16: note: 'flags' was declared here
     unsigned long flags;
                   ^~~~~
--
   In file included from include/linux/irqflags.h:16:0,
                    from arch/x86/include/asm/processor.h:33,
                    from arch/x86/include/asm/cpufeature.h:5,
                    from arch/x86/include/asm/thread_info.h:53,
                    from include/linux/thread_info.h:38,
                    from arch/x86/include/asm/preempt.h:7,
                    from include/linux/preempt.h:78,
                    from include/linux/spinlock.h:51,
                    from include/linux/seqlock.h:36,
                    from include/linux/time.h:6,
                    from include/linux/skbuff.h:15,
                    from include/linux/if_ether.h:19,
                    from include/linux/etherdevice.h:20,
                    from drivers/net//ethernet/mscc/ocelot.c:7:
   drivers/net//ethernet/mscc/ocelot.c: In function 'ocelot_ptp_adjfine':
>> arch/x86/include/asm/irqflags.h:41:2: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized]
     asm volatile("push %0 ; popf"
     ^~~
   drivers/net//ethernet/mscc/ocelot.c:1904:16: note: 'flags' was declared here
     unsigned long flags;
                   ^~~~~

vim +/flags +41 arch/x86/include/asm/irqflags.h

6abcd98f include/asm-x86/irqflags.h      Glauber de Oliveira Costa 2008-01-30  37  
1f59a458 arch/x86/include/asm/irqflags.h Nick Desaulniers          2018-08-27  38  extern inline void native_restore_fl(unsigned long flags);
1f59a458 arch/x86/include/asm/irqflags.h Nick Desaulniers          2018-08-27  39  extern inline void native_restore_fl(unsigned long flags)
6abcd98f include/asm-x86/irqflags.h      Glauber de Oliveira Costa 2008-01-30  40  {
cf7f7191 include/asm-x86/irqflags.h      Joe Perches               2008-03-23 @41  	asm volatile("push %0 ; popf"
6abcd98f include/asm-x86/irqflags.h      Glauber de Oliveira Costa 2008-01-30  42  		     : /* no output */
6abcd98f include/asm-x86/irqflags.h      Glauber de Oliveira Costa 2008-01-30  43  		     :"g" (flags)
cf7f7191 include/asm-x86/irqflags.h      Joe Perches               2008-03-23  44  		     :"memory", "cc");
6abcd98f include/asm-x86/irqflags.h      Glauber de Oliveira Costa 2008-01-30  45  }
6abcd98f include/asm-x86/irqflags.h      Glauber de Oliveira Costa 2008-01-30  46  

:::::: The code at line 41 was first introduced by commit
:::::: cf7f7191cf20011e47243b594e433275a6db811b include/asm-x86/irqflags.h: checkpatch cleanups - formatting only

:::::: TO: Joe Perches <joe@perches.com>
:::::: CC: Ingo Molnar <mingo@elte.hu>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38820 bytes --]

^ permalink raw reply

* [PATCH bpf-next v3 6/6] samples/bpf: add use of need_sleep flag in xdpsock
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
  Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
	qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
	ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
	maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>

This commit adds using the need_sleep flag to the xdpsock sample
application. It is turned on by default as we think it is a feature
that seems to always produce a performance benefit, if the application
has been written taking advantage of it. It can be turned off in the
sample app by using the '-m' command line option.

The txpush and l2fwd sub applications have also been updated to
support poll() with multiple sockets.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 samples/bpf/xdpsock_user.c | 192 ++++++++++++++++++++++++++++-----------------
 1 file changed, 120 insertions(+), 72 deletions(-)

diff --git a/samples/bpf/xdpsock_user.c b/samples/bpf/xdpsock_user.c
index 93eaaf7..7d771af 100644
--- a/samples/bpf/xdpsock_user.c
+++ b/samples/bpf/xdpsock_user.c
@@ -67,8 +67,10 @@ static int opt_ifindex;
 static int opt_queue;
 static int opt_poll;
 static int opt_interval = 1;
-static u32 opt_xdp_bind_flags;
+static u32 opt_xdp_bind_flags = XDP_USE_NEED_WAKEUP;
 static int opt_xsk_frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
+static int opt_timeout = 1000;
+static bool opt_might_sleep = true;
 static __u32 prog_id;
 
 struct xsk_umem_info {
@@ -352,6 +354,7 @@ static struct option long_options[] = {
 	{"zero-copy", no_argument, 0, 'z'},
 	{"copy", no_argument, 0, 'c'},
 	{"frame-size", required_argument, 0, 'f'},
+	{"no-might-sleep", no_argument, 0, 'm'},
 	{0, 0, 0, 0}
 };
 
@@ -372,6 +375,7 @@ static void usage(const char *prog)
 		"  -z, --zero-copy      Force zero-copy mode.\n"
 		"  -c, --copy           Force copy mode.\n"
 		"  -f, --frame-size=n   Set the frame size (must be a power of two, default is %d).\n"
+		"  -m, --no-might-sleep Turn off use of driver might sleep flag.\n"
 		"\n";
 	fprintf(stderr, str, prog, XSK_UMEM__DEFAULT_FRAME_SIZE);
 	exit(EXIT_FAILURE);
@@ -384,8 +388,9 @@ static void parse_command_line(int argc, char **argv)
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "Frtli:q:psSNn:czf:", long_options,
-				&option_index);
+
+		c = getopt_long(argc, argv, "Frtli:q:psSNn:czf:m",
+				long_options, &option_index);
 		if (c == -1)
 			break;
 
@@ -429,6 +434,9 @@ static void parse_command_line(int argc, char **argv)
 			break;
 		case 'f':
 			opt_xsk_frame_size = atoi(optarg);
+		case 'm':
+			opt_might_sleep = false;
+			opt_xdp_bind_flags &= ~XDP_USE_NEED_WAKEUP;
 			break;
 		default:
 			usage(basename(argv[0]));
@@ -459,7 +467,8 @@ static void kick_tx(struct xsk_socket_info *xsk)
 	exit_with_error(errno);
 }
 
-static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk)
+static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk,
+				     struct pollfd *fds)
 {
 	u32 idx_cq = 0, idx_fq = 0;
 	unsigned int rcvd;
@@ -468,7 +477,9 @@ static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk)
 	if (!xsk->outstanding_tx)
 		return;
 
-	kick_tx(xsk);
+	if (!opt_might_sleep || xsk_ring_prod__needs_wakeup(&xsk->tx))
+		kick_tx(xsk);
+
 	ndescs = (xsk->outstanding_tx > BATCH_SIZE) ? BATCH_SIZE :
 		xsk->outstanding_tx;
 
@@ -482,6 +493,8 @@ static inline void complete_tx_l2fwd(struct xsk_socket_info *xsk)
 		while (ret != rcvd) {
 			if (ret < 0)
 				exit_with_error(-ret);
+			if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
+				ret = poll(fds, num_socks, opt_timeout);
 			ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd,
 						     &idx_fq);
 		}
@@ -505,7 +518,8 @@ static inline void complete_tx_only(struct xsk_socket_info *xsk)
 	if (!xsk->outstanding_tx)
 		return;
 
-	kick_tx(xsk);
+	if (!opt_might_sleep || xsk_ring_prod__needs_wakeup(&xsk->tx))
+		kick_tx(xsk);
 
 	rcvd = xsk_ring_cons__peek(&xsk->umem->cq, BATCH_SIZE, &idx);
 	if (rcvd > 0) {
@@ -515,20 +529,25 @@ static inline void complete_tx_only(struct xsk_socket_info *xsk)
 	}
 }
 
-static void rx_drop(struct xsk_socket_info *xsk)
+static void rx_drop(struct xsk_socket_info *xsk, struct pollfd *fds)
 {
 	unsigned int rcvd, i;
 	u32 idx_rx = 0, idx_fq = 0;
 	int ret;
 
 	rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
-	if (!rcvd)
+	if (!rcvd) {
+		if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
+			ret = poll(fds, num_socks, opt_timeout);
 		return;
+	}
 
 	ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
 	while (ret != rcvd) {
 		if (ret < 0)
 			exit_with_error(-ret);
+		if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
+			ret = poll(fds, num_socks, opt_timeout);
 		ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
 	}
 
@@ -549,42 +568,65 @@ static void rx_drop(struct xsk_socket_info *xsk)
 static void rx_drop_all(void)
 {
 	struct pollfd fds[MAX_SOCKS + 1];
-	int i, ret, timeout, nfds = 1;
+	int i, ret;
 
 	memset(fds, 0, sizeof(fds));
 
 	for (i = 0; i < num_socks; i++) {
 		fds[i].fd = xsk_socket__fd(xsks[i]->xsk);
 		fds[i].events = POLLIN;
-		timeout = 1000; /* 1sn */
 	}
 
 	for (;;) {
 		if (opt_poll) {
-			ret = poll(fds, nfds, timeout);
+			ret = poll(fds, num_socks, opt_timeout);
 			if (ret <= 0)
 				continue;
 		}
 
 		for (i = 0; i < num_socks; i++)
-			rx_drop(xsks[i]);
+			rx_drop(xsks[i], fds);
+	}
+}
+
+static void tx_only(struct xsk_socket_info *xsk, u32 frame_nb)
+{
+	u32 idx;
+
+	if (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) == BATCH_SIZE) {
+		unsigned int i;
+
+		for (i = 0; i < BATCH_SIZE; i++) {
+			xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->addr	=
+				(frame_nb + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
+			xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->len =
+				sizeof(pkt_data) - 1;
+		}
+
+		xsk_ring_prod__submit(&xsk->tx, BATCH_SIZE);
+		xsk->outstanding_tx += BATCH_SIZE;
+		frame_nb += BATCH_SIZE;
+		frame_nb %= NUM_FRAMES;
 	}
+
+	complete_tx_only(xsk);
 }
 
-static void tx_only(struct xsk_socket_info *xsk)
+static void tx_only_all(void)
 {
-	int timeout, ret, nfds = 1;
-	struct pollfd fds[nfds + 1];
-	u32 idx, frame_nb = 0;
+	struct pollfd fds[MAX_SOCKS];
+	u32 frame_nb[MAX_SOCKS] = {};
+	int i, ret;
 
 	memset(fds, 0, sizeof(fds));
-	fds[0].fd = xsk_socket__fd(xsk->xsk);
-	fds[0].events = POLLOUT;
-	timeout = 1000; /* 1sn */
+	for (i = 0; i < num_socks; i++) {
+		fds[0].fd = xsk_socket__fd(xsks[i]->xsk);
+		fds[0].events = POLLOUT;
+	}
 
 	for (;;) {
 		if (opt_poll) {
-			ret = poll(fds, nfds, timeout);
+			ret = poll(fds, num_socks, opt_timeout);
 			if (ret <= 0)
 				continue;
 
@@ -592,69 +634,75 @@ static void tx_only(struct xsk_socket_info *xsk)
 				continue;
 		}
 
-		if (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) ==
-		    BATCH_SIZE) {
-			unsigned int i;
-
-			for (i = 0; i < BATCH_SIZE; i++) {
-				xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->addr
-					= (frame_nb + i) * opt_xsk_frame_size;
-				xsk_ring_prod__tx_desc(&xsk->tx, idx + i)->len =
-					sizeof(pkt_data) - 1;
-			}
-
-			xsk_ring_prod__submit(&xsk->tx, BATCH_SIZE);
-			xsk->outstanding_tx += BATCH_SIZE;
-			frame_nb += BATCH_SIZE;
-			frame_nb %= NUM_FRAMES;
-		}
-
-		complete_tx_only(xsk);
+		for (i = 0; i < num_socks; i++)
+			tx_only(xsks[i], frame_nb[i]);
 	}
 }
 
-static void l2fwd(struct xsk_socket_info *xsk)
+static void l2fwd(struct xsk_socket_info *xsk, struct pollfd *fds)
 {
-	for (;;) {
-		unsigned int rcvd, i;
-		u32 idx_rx = 0, idx_tx = 0;
-		int ret;
+	unsigned int rcvd, i;
+	u32 idx_rx = 0, idx_tx = 0;
+	int ret;
 
-		for (;;) {
-			complete_tx_l2fwd(xsk);
+	complete_tx_l2fwd(xsk, fds);
 
-			rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE,
-						   &idx_rx);
-			if (rcvd > 0)
-				break;
-		}
+	rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
+	if (!rcvd) {
+		if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq))
+			ret = poll(fds, num_socks, opt_timeout);
+		return;
+	}
 
+	ret = xsk_ring_prod__reserve(&xsk->tx, rcvd, &idx_tx);
+	while (ret != rcvd) {
+		if (ret < 0)
+			exit_with_error(-ret);
+		if (xsk_ring_prod__needs_wakeup(&xsk->tx))
+			kick_tx(xsk);
 		ret = xsk_ring_prod__reserve(&xsk->tx, rcvd, &idx_tx);
-		while (ret != rcvd) {
-			if (ret < 0)
-				exit_with_error(-ret);
-			ret = xsk_ring_prod__reserve(&xsk->tx, rcvd, &idx_tx);
-		}
+	}
+
+	for (i = 0; i < rcvd; i++) {
+		u64 addr = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx)->addr;
+		u32 len = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++)->len;
+		char *pkt = xsk_umem__get_data(xsk->umem->buffer, addr);
+
+		swap_mac_addresses(pkt);
 
-		for (i = 0; i < rcvd; i++) {
-			u64 addr = xsk_ring_cons__rx_desc(&xsk->rx,
-							  idx_rx)->addr;
-			u32 len = xsk_ring_cons__rx_desc(&xsk->rx,
-							 idx_rx++)->len;
-			char *pkt = xsk_umem__get_data(xsk->umem->buffer, addr);
+		hex_dump(pkt, len, addr);
+		xsk_ring_prod__tx_desc(&xsk->tx, idx_tx)->addr = addr;
+		xsk_ring_prod__tx_desc(&xsk->tx, idx_tx++)->len = len;
+	}
 
-			swap_mac_addresses(pkt);
+	xsk_ring_prod__submit(&xsk->tx, rcvd);
+	xsk_ring_cons__release(&xsk->rx, rcvd);
 
-			hex_dump(pkt, len, addr);
-			xsk_ring_prod__tx_desc(&xsk->tx, idx_tx)->addr = addr;
-			xsk_ring_prod__tx_desc(&xsk->tx, idx_tx++)->len = len;
-		}
+	xsk->rx_npkts += rcvd;
+	xsk->outstanding_tx += rcvd;
+}
+
+static void l2fwd_all(void)
+{
+	struct pollfd fds[MAX_SOCKS];
+	int i, ret;
+
+	memset(fds, 0, sizeof(fds));
+
+	for (i = 0; i < num_socks; i++) {
+		fds[i].fd = xsk_socket__fd(xsks[i]->xsk);
+		fds[i].events = POLLOUT | POLLIN;
+	}
 
-		xsk_ring_prod__submit(&xsk->tx, rcvd);
-		xsk_ring_cons__release(&xsk->rx, rcvd);
+	for (;;) {
+		if (opt_poll) {
+			ret = poll(fds, num_socks, opt_timeout);
+			if (ret <= 0)
+				continue;
+		}
 
-		xsk->rx_npkts += rcvd;
-		xsk->outstanding_tx += rcvd;
+		for (i = 0; i < num_socks; i++)
+			l2fwd(xsks[i], fds);
 	}
 }
 
@@ -705,9 +753,9 @@ int main(int argc, char **argv)
 	if (opt_bench == BENCH_RXDROP)
 		rx_drop_all();
 	else if (opt_bench == BENCH_TXONLY)
-		tx_only(xsks[0]);
+		tx_only_all();
 	else
-		l2fwd(xsks[0]);
+		l2fwd_all();
 
 	return 0;
 }
-- 
2.7.4


^ permalink raw reply related

* [PATCH bpf-next v3 5/6] libbpf: add support for need_wakeup flag in AF_XDP part
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
  Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
	qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
	ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
	maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>

This commit adds support for the new need_wakeup flag in AF_XDP. The
xsk_socket__create function is updated to handle this and a new
function is introduced called xsk_ring_prod__needs_wakeup(). This
function can be used by the application to check if Rx and/or Tx
processing needs to be explicitly woken up.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/include/uapi/linux/if_xdp.h | 13 +++++++++++++
 tools/lib/bpf/xsk.c               |  4 ++++
 tools/lib/bpf/xsk.h               |  6 ++++++
 3 files changed, 23 insertions(+)

diff --git a/tools/include/uapi/linux/if_xdp.h b/tools/include/uapi/linux/if_xdp.h
index faaa5ca..62b80d5 100644
--- a/tools/include/uapi/linux/if_xdp.h
+++ b/tools/include/uapi/linux/if_xdp.h
@@ -16,6 +16,15 @@
 #define XDP_SHARED_UMEM	(1 << 0)
 #define XDP_COPY	(1 << 1) /* Force copy-mode */
 #define XDP_ZEROCOPY	(1 << 2) /* Force zero-copy mode */
+/* If this option is set, the driver might go sleep and in that case
+ * the XDP_RING_NEED_WAKEUP flag in the fill and/or Tx rings will be
+ * set. If it is set, the application need to explicitly wake up the
+ * driver with a poll() (Rx and Tx) or sendto() (Tx only). If you are
+ * running the driver and the application on the same core, you should
+ * use this option so that the kernel will yield to the user space
+ * application.
+ */
+#define XDP_USE_NEED_WAKEUP (1 << 3)
 
 struct sockaddr_xdp {
 	__u16 sxdp_family;
@@ -25,10 +34,14 @@ struct sockaddr_xdp {
 	__u32 sxdp_shared_umem_fd;
 };
 
+/* XDP_RING flags */
+#define XDP_RING_NEED_WAKEUP (1 << 0)
+
 struct xdp_ring_offset {
 	__u64 producer;
 	__u64 consumer;
 	__u64 desc;
+	__u64 flags;
 };
 
 struct xdp_mmap_offsets {
diff --git a/tools/lib/bpf/xsk.c b/tools/lib/bpf/xsk.c
index b337402..5665b4d 100644
--- a/tools/lib/bpf/xsk.c
+++ b/tools/lib/bpf/xsk.c
@@ -224,6 +224,7 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
 	fill->size = umem->config.fill_size;
 	fill->producer = map + off.fr.producer;
 	fill->consumer = map + off.fr.consumer;
+	fill->flags = map + off.fr.flags;
 	fill->ring = map + off.fr.desc;
 	fill->cached_cons = umem->config.fill_size;
 
@@ -241,6 +242,7 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area, __u64 size,
 	comp->size = umem->config.comp_size;
 	comp->producer = map + off.cr.producer;
 	comp->consumer = map + off.cr.consumer;
+	comp->flags = map + off.cr.flags;
 	comp->ring = map + off.cr.desc;
 
 	*umem_ptr = umem;
@@ -564,6 +566,7 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 		rx->size = xsk->config.rx_size;
 		rx->producer = rx_map + off.rx.producer;
 		rx->consumer = rx_map + off.rx.consumer;
+		rx->flags = rx_map + off.rx.flags;
 		rx->ring = rx_map + off.rx.desc;
 	}
 	xsk->rx = rx;
@@ -583,6 +586,7 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
 		tx->size = xsk->config.tx_size;
 		tx->producer = tx_map + off.tx.producer;
 		tx->consumer = tx_map + off.tx.consumer;
+		tx->flags = tx_map + off.tx.flags;
 		tx->ring = tx_map + off.tx.desc;
 		tx->cached_cons = xsk->config.tx_size;
 	}
diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 833a6e6..aa1d612 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -32,6 +32,7 @@ struct name { \
 	__u32 *producer; \
 	__u32 *consumer; \
 	void *ring; \
+	__u32 *flags; \
 }
 
 DEFINE_XSK_RING(xsk_ring_prod);
@@ -76,6 +77,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
 	return &descs[idx & rx->mask];
 }
 
+static inline int xsk_ring_prod__needs_wakeup(const struct xsk_ring_prod *r)
+{
+	return *r->flags & XDP_RING_NEED_WAKEUP;
+}
+
 static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
 {
 	__u32 free_entries = r->cached_cons - r->cached_prod;
-- 
2.7.4


^ permalink raw reply related

* [PATCH bpf-next v3 4/6] ixgbe: add support for AF_XDP need_wakup feature
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
  Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
	qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
	ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
	maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>

This patch adds support for the need_wakeup feature of AF_XDP. If the
application has told the kernel that it might sleep using the new bind
flag XDP_USE_NEED_WAKEUP, the driver will then set this flag if it has
no more buffers on the NIC Rx ring and yield to the application. For
Tx, it will set the flag if it has no outstanding Tx completion
interrupts and return to the application.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index e598af9..3fbc4e4 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -547,6 +547,14 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
 	q_vector->rx.total_packets += total_rx_packets;
 	q_vector->rx.total_bytes += total_rx_bytes;
 
+	if (xsk_umem_uses_might_sleep(rx_ring->xsk_umem)) {
+		if (failure || rx_ring->next_to_clean == rx_ring->next_to_use)
+			xsk_set_rx_need_wakeup(rx_ring->xsk_umem);
+		else
+			xsk_clear_rx_need_wakeup(rx_ring->xsk_umem);
+
+		return (int)total_rx_packets;
+	}
 	return failure ? budget : (int)total_rx_packets;
 }
 
@@ -689,6 +697,14 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
 		xsk_umem_complete_tx(umem, xsk_frames);
 
 	xmit_done = ixgbe_xmit_zc(tx_ring, q_vector->tx.work_limit);
+
+	if (xsk_umem_uses_might_sleep(tx_ring->xsk_umem)) {
+		if (tx_ring->next_to_clean == tx_ring->next_to_use)
+			xsk_set_tx_need_wakeup(tx_ring->xsk_umem);
+		else
+			xsk_clear_tx_need_wakeup(tx_ring->xsk_umem);
+	}
+
 	return budget > 0 && xmit_done;
 }
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH bpf-next v3 3/6] i40e: add support for AF_XDP need_wakup feature
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
  Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
	qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
	ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
	maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>

This patch adds support for the need_wakeup feature of AF_XDP. If the
application has told the kernel that it might sleep using the new bind
flag XDP_USE_NEED_WAKEUP, the driver will then set this flag if it has
no more buffers on the NIC Rx ring and yield to the application. For
Tx, it will set the flag if it has no outstanding Tx completion
interrupts and return to the application.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_xsk.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index d0ff5d8..18de7b6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -626,6 +626,15 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget)
 
 	i40e_finalize_xdp_rx(rx_ring, xdp_xmit);
 	i40e_update_rx_stats(rx_ring, total_rx_bytes, total_rx_packets);
+
+	if (xsk_umem_uses_might_sleep(rx_ring->xsk_umem)) {
+		if (failure || rx_ring->next_to_clean == rx_ring->next_to_use)
+			xsk_set_rx_need_wakeup(rx_ring->xsk_umem);
+		else
+			xsk_clear_rx_need_wakeup(rx_ring->xsk_umem);
+
+		return (int)total_rx_packets;
+	}
 	return failure ? budget : (int)total_rx_packets;
 }
 
@@ -761,6 +770,13 @@ bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi,
 out_xmit:
 	xmit_done = i40e_xmit_zc(tx_ring, budget);
 
+	if (xsk_umem_uses_might_sleep(tx_ring->xsk_umem)) {
+		if (tx_ring->next_to_clean == tx_ring->next_to_use)
+			xsk_set_tx_need_wakeup(tx_ring->xsk_umem);
+		else
+			xsk_clear_tx_need_wakeup(tx_ring->xsk_umem);
+	}
+
 	return work_done && xmit_done;
 }
 
-- 
2.7.4


^ permalink raw reply related

* [PATCH bpf-next v3 2/6] xsk: add support for need_wakeup flag in AF_XDP rings
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
  Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
	qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
	ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
	maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>

This commit adds support for a new flag called need_wakeup in the
AF_XDP Tx and fill rings. When this flag is set, it means that the
application has to explicitly wake up the kernel Rx (for the bit in
the fill ring) or kernel Tx (for bit in the Tx ring) processing by
issuing a syscall. Poll() can wake up both depending on the flags
submitted and sendto() will wake up tx processing only.

The main reason for introducing this new flag is to be able to
efficiently support the case when application and driver is executing
on the same core. Previously, the driver was just busy-spinning on the
fill ring if it ran out of buffers in the HW and there were none on
the fill ring. This approach works when the application is running on
another core as it can replenish the fill ring while the driver is
busy-spinning. Though, this is a lousy approach if both of them are
running on the same core as the probability of the fill ring getting
more entries when the driver is busy-spinning is zero. With this new
feature the driver now sets the need_wakeup flag and returns to the
application. The application can then replenish the fill queue and
then explicitly wake up the Rx processing in the kernel using the
syscall poll(). For Tx, the flag is only set to one if the driver has
no outstanding Tx completion interrupts. If it has some, the flag is
zero as it will be woken up by a completion interrupt anyway.

As a nice side effect, this new flag also improves the performance of
the case where application and driver are running on two different
cores as it reduces the number of syscalls to the kernel. The kernel
tells user space if it needs to be woken up by a syscall, and this
eliminates many of the syscalls.

This flag needs some simple driver support. If the driver does not
support this, the Rx flag is always zero and the Tx flag is always
one. This makes any application relying on this feature default to the
old behaviour of not requiring any syscalls in the Rx path and always
having to call sendto() in the Tx path.

For backwards compatibility reasons, this feature has to be explicitly
turned on using a new bind flag (XDP_USE_NEED_WAKEUP). I recommend
that you always turn it on as it so far always have had a positive
performance impact.

The name and inspiration of the flag has been taken from io_uring by
Jens Axboe. Details about this feature in io_uring can be found in
http://kernel.dk/io_uring.pdf, section 8.3.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 include/linux/netdevice.h   |   4 ++
 include/net/xdp_sock.h      |  33 +++++++++-
 include/uapi/linux/if_xdp.h |  13 ++++
 net/xdp/xdp_umem.c          |   9 +++
 net/xdp/xsk.c               | 146 ++++++++++++++++++++++++++++++++++++++------
 net/xdp/xsk.h               |  13 ++++
 net/xdp/xsk_queue.h         |   1 +
 7 files changed, 199 insertions(+), 20 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 60eef29..a206c1d 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -872,6 +872,9 @@ struct bpf_prog_offload_ops;
 struct netlink_ext_ack;
 struct xdp_umem;
 
+/* Flags for the flags field in XDP_SETUP_XSK_UMEM */
+#define XSK_DRV_CAN_SLEEP (1 << 0) /* The driver is alowed to sleep. */
+
 struct netdev_bpf {
 	enum bpf_netdev_command command;
 	union {
@@ -894,6 +897,7 @@ struct netdev_bpf {
 		/* XDP_SETUP_XSK_UMEM */
 		struct {
 			struct xdp_umem *umem;
+			u32 flags;
 			u16 queue_id;
 		} xsk;
 	};
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index 057b159..66c3755 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -27,6 +27,9 @@ struct xdp_umem_fq_reuse {
 	u64 handles[];
 };
 
+/* Flags for the umem flags field. */
+#define XDP_UMEM_MIGHT_SLEEP (1 << 0)
+
 struct xdp_umem {
 	struct xsk_queue *fq;
 	struct xsk_queue *cq;
@@ -41,10 +44,12 @@ struct xdp_umem {
 	struct work_struct work;
 	struct page **pgs;
 	u32 npgs;
+	u16 queue_id;
+	u8 need_wakeup;
+	u8 flags;
 	int id;
 	struct net_device *dev;
 	struct xdp_umem_fq_reuse *fq_reuse;
-	u16 queue_id;
 	bool zc;
 	spinlock_t xsk_list_lock;
 	struct list_head xsk_list;
@@ -88,6 +93,11 @@ struct xdp_umem_fq_reuse *xsk_reuseq_swap(struct xdp_umem *umem,
 					  struct xdp_umem_fq_reuse *newq);
 void xsk_reuseq_free(struct xdp_umem_fq_reuse *rq);
 struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev, u16 queue_id);
+void xsk_set_rx_need_wakeup(struct xdp_umem *umem);
+void xsk_set_tx_need_wakeup(struct xdp_umem *umem);
+void xsk_clear_rx_need_wakeup(struct xdp_umem *umem);
+void xsk_clear_tx_need_wakeup(struct xdp_umem *umem);
+bool xsk_umem_uses_might_sleep(struct xdp_umem *umem);
 
 static inline char *xdp_umem_get_data(struct xdp_umem *umem, u64 addr)
 {
@@ -234,6 +244,27 @@ static inline void xsk_umem_fq_reuse(struct xdp_umem *umem, u64 addr)
 {
 }
 
+static inline void xsk_set_rx_need_wakeup(struct xdp_umem *umem)
+{
+}
+
+static inline void xsk_set_tx_need_wakeup(struct xdp_umem *umem)
+{
+}
+
+static inline void xsk_clear_rx_need_wakeup(struct xdp_umem *umem)
+{
+}
+
+static inline void xsk_clear_tx_need_wakeup(struct xdp_umem *umem)
+{
+}
+
+static inline bool xsk_umem_uses_might_sleep(struct xdp_umem *umem)
+{
+	return false;
+}
+
 #endif /* CONFIG_XDP_SOCKETS */
 
 #endif /* _LINUX_XDP_SOCK_H */
diff --git a/include/uapi/linux/if_xdp.h b/include/uapi/linux/if_xdp.h
index faaa5ca..62b80d5 100644
--- a/include/uapi/linux/if_xdp.h
+++ b/include/uapi/linux/if_xdp.h
@@ -16,6 +16,15 @@
 #define XDP_SHARED_UMEM	(1 << 0)
 #define XDP_COPY	(1 << 1) /* Force copy-mode */
 #define XDP_ZEROCOPY	(1 << 2) /* Force zero-copy mode */
+/* If this option is set, the driver might go sleep and in that case
+ * the XDP_RING_NEED_WAKEUP flag in the fill and/or Tx rings will be
+ * set. If it is set, the application need to explicitly wake up the
+ * driver with a poll() (Rx and Tx) or sendto() (Tx only). If you are
+ * running the driver and the application on the same core, you should
+ * use this option so that the kernel will yield to the user space
+ * application.
+ */
+#define XDP_USE_NEED_WAKEUP (1 << 3)
 
 struct sockaddr_xdp {
 	__u16 sxdp_family;
@@ -25,10 +34,14 @@ struct sockaddr_xdp {
 	__u32 sxdp_shared_umem_fd;
 };
 
+/* XDP_RING flags */
+#define XDP_RING_NEED_WAKEUP (1 << 0)
+
 struct xdp_ring_offset {
 	__u64 producer;
 	__u64 consumer;
 	__u64 desc;
+	__u64 flags;
 };
 
 struct xdp_mmap_offsets {
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 803554b..6b05cd0 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -105,6 +105,15 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
 
 	umem->dev = dev;
 	umem->queue_id = queue_id;
+	if (flags & XDP_USE_NEED_WAKEUP) {
+		umem->flags |= XDP_UMEM_MIGHT_SLEEP;
+		/* Tx needs to be explicitly woken up the first time.
+		 * Also for supporting drivers that do not implement this
+		 * feature. They will always have to call sendto().
+		 */
+		xsk_set_tx_need_wakeup(umem);
+	}
+
 	if (force_copy)
 		/* For copy-mode, we are done. */
 		goto out_rtnl_unlock;
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index cf8898f..ea2143f 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -55,6 +55,66 @@ void xsk_umem_discard_addr(struct xdp_umem *umem)
 }
 EXPORT_SYMBOL(xsk_umem_discard_addr);
 
+void xsk_set_rx_need_wakeup(struct xdp_umem *umem)
+{
+	if (umem->need_wakeup & XDP_WAKEUP_RX)
+		return;
+
+	umem->fq->ring->flags |= XDP_RING_NEED_WAKEUP;
+	umem->need_wakeup |= XDP_WAKEUP_RX;
+}
+EXPORT_SYMBOL(xsk_set_rx_need_wakeup);
+
+void xsk_set_tx_need_wakeup(struct xdp_umem *umem)
+{
+	struct xdp_sock *xs;
+
+	if (umem->need_wakeup & XDP_WAKEUP_TX)
+		return;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(xs, &umem->xsk_list, list) {
+		xs->tx->ring->flags |= XDP_RING_NEED_WAKEUP;
+	}
+	rcu_read_unlock();
+
+	umem->need_wakeup |= XDP_WAKEUP_TX;
+}
+EXPORT_SYMBOL(xsk_set_tx_need_wakeup);
+
+void xsk_clear_rx_need_wakeup(struct xdp_umem *umem)
+{
+	if (!(umem->need_wakeup & XDP_WAKEUP_RX))
+		return;
+
+	umem->fq->ring->flags &= ~XDP_RING_NEED_WAKEUP;
+	umem->need_wakeup &= ~XDP_WAKEUP_RX;
+}
+EXPORT_SYMBOL(xsk_clear_rx_need_wakeup);
+
+void xsk_clear_tx_need_wakeup(struct xdp_umem *umem)
+{
+	struct xdp_sock *xs;
+
+	if (!(umem->need_wakeup & XDP_WAKEUP_TX))
+		return;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(xs, &umem->xsk_list, list) {
+		xs->tx->ring->flags &= ~XDP_RING_NEED_WAKEUP;
+	}
+	rcu_read_unlock();
+
+	umem->need_wakeup &= ~XDP_WAKEUP_TX;
+}
+EXPORT_SYMBOL(xsk_clear_tx_need_wakeup);
+
+bool xsk_umem_uses_might_sleep(struct xdp_umem *umem)
+{
+	return umem->flags & XDP_UMEM_MIGHT_SLEEP;
+}
+EXPORT_SYMBOL(xsk_umem_uses_might_sleep);
+
 static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 {
 	void *to_buf, *from_buf;
@@ -311,6 +371,12 @@ static unsigned int xsk_poll(struct file *file, struct socket *sock,
 	unsigned int mask = datagram_poll(file, sock, wait);
 	struct sock *sk = sock->sk;
 	struct xdp_sock *xs = xdp_sk(sk);
+	struct net_device *dev = xs->dev;
+	struct xdp_umem *umem = xs->umem;
+
+	if (umem->need_wakeup)
+		dev->netdev_ops->ndo_xsk_wakeup(dev, xs->queue_id,
+						umem->need_wakeup);
 
 	if (xs->rx && !xskq_empty_desc(xs->rx))
 		mask |= POLLIN | POLLRDNORM;
@@ -411,7 +477,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
 		return -EINVAL;
 
 	flags = sxdp->sxdp_flags;
-	if (flags & ~(XDP_SHARED_UMEM | XDP_COPY | XDP_ZEROCOPY))
+	if (flags & ~(XDP_SHARED_UMEM | XDP_COPY | XDP_ZEROCOPY |
+		      XDP_USE_NEED_WAKEUP))
 		return -EINVAL;
 
 	mutex_lock(&xs->mutex);
@@ -437,7 +504,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr *addr, int addr_len)
 		struct xdp_sock *umem_xs;
 		struct socket *sock;
 
-		if ((flags & XDP_COPY) || (flags & XDP_ZEROCOPY)) {
+		if ((flags & XDP_COPY) || (flags & XDP_ZEROCOPY) ||
+		    (flags & XDP_USE_NEED_WAKEUP)) {
 			/* Cannot specify flags for shared sockets. */
 			err = -EINVAL;
 			goto out_unlock;
@@ -525,6 +593,9 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
 		mutex_lock(&xs->mutex);
 		q = (optname == XDP_TX_RING) ? &xs->tx : &xs->rx;
 		err = xsk_init_queue(entries, q, false);
+		if (!err && optname == XDP_TX_RING)
+			/* Tx needs to be explicitly woken up the first time */
+			xs->tx->ring->flags |= XDP_RING_NEED_WAKEUP;
 		mutex_unlock(&xs->mutex);
 		return err;
 	}
@@ -582,6 +653,20 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
 	return -ENOPROTOOPT;
 }
 
+static void xsk_enter_rxtx_offsets(struct xdp_ring_offset_v1 *ring)
+{
+	ring->producer = offsetof(struct xdp_rxtx_ring, ptrs.producer);
+	ring->consumer = offsetof(struct xdp_rxtx_ring, ptrs.consumer);
+	ring->desc = offsetof(struct xdp_rxtx_ring, desc);
+}
+
+static void xsk_enter_umem_offsets(struct xdp_ring_offset_v1 *ring)
+{
+	ring->producer = offsetof(struct xdp_umem_ring, ptrs.producer);
+	ring->consumer = offsetof(struct xdp_umem_ring, ptrs.consumer);
+	ring->desc = offsetof(struct xdp_umem_ring, desc);
+}
+
 static int xsk_getsockopt(struct socket *sock, int level, int optname,
 			  char __user *optval, int __user *optlen)
 {
@@ -621,26 +706,49 @@ static int xsk_getsockopt(struct socket *sock, int level, int optname,
 	case XDP_MMAP_OFFSETS:
 	{
 		struct xdp_mmap_offsets off;
+		struct xdp_mmap_offsets_v1 off_v1;
+		bool flags_supported = true;
+		void *to_copy;
 
-		if (len < sizeof(off))
+		if (len < sizeof(off_v1))
 			return -EINVAL;
+		else if (len < sizeof(off))
+			flags_supported = false;
+
+		if (flags_supported) {
+			/* xdp_ring_offset is identical to xdp_ring_offset_v1
+			 * except for the flags field added to the end.
+			 */
+			xsk_enter_rxtx_offsets((struct xdp_ring_offset_v1 *)
+					       &off.rx);
+			xsk_enter_rxtx_offsets((struct xdp_ring_offset_v1 *)
+					       &off.tx);
+			xsk_enter_umem_offsets((struct xdp_ring_offset_v1 *)
+					       &off.fr);
+			xsk_enter_umem_offsets((struct xdp_ring_offset_v1 *)
+					       &off.cr);
+			off.rx.flags = offsetof(struct xdp_rxtx_ring,
+						ptrs.flags);
+			off.tx.flags = offsetof(struct xdp_rxtx_ring,
+						ptrs.flags);
+			off.fr.flags = offsetof(struct xdp_umem_ring,
+						ptrs.flags);
+			off.cr.flags = offsetof(struct xdp_umem_ring,
+						ptrs.flags);
+
+			len = sizeof(off);
+			to_copy = &off;
+		} else {
+			xsk_enter_rxtx_offsets(&off_v1.rx);
+			xsk_enter_rxtx_offsets(&off_v1.tx);
+			xsk_enter_umem_offsets(&off_v1.fr);
+			xsk_enter_umem_offsets(&off_v1.cr);
+
+			len = sizeof(off_v1);
+			to_copy = &off_v1;
+		}
 
-		off.rx.producer = offsetof(struct xdp_rxtx_ring, ptrs.producer);
-		off.rx.consumer = offsetof(struct xdp_rxtx_ring, ptrs.consumer);
-		off.rx.desc	= offsetof(struct xdp_rxtx_ring, desc);
-		off.tx.producer = offsetof(struct xdp_rxtx_ring, ptrs.producer);
-		off.tx.consumer = offsetof(struct xdp_rxtx_ring, ptrs.consumer);
-		off.tx.desc	= offsetof(struct xdp_rxtx_ring, desc);
-
-		off.fr.producer = offsetof(struct xdp_umem_ring, ptrs.producer);
-		off.fr.consumer = offsetof(struct xdp_umem_ring, ptrs.consumer);
-		off.fr.desc	= offsetof(struct xdp_umem_ring, desc);
-		off.cr.producer = offsetof(struct xdp_umem_ring, ptrs.producer);
-		off.cr.consumer = offsetof(struct xdp_umem_ring, ptrs.consumer);
-		off.cr.desc	= offsetof(struct xdp_umem_ring, desc);
-
-		len = sizeof(off);
-		if (copy_to_user(optval, &off, len))
+		if (copy_to_user(optval, to_copy, len))
 			return -EFAULT;
 		if (put_user(len, optlen))
 			return -EFAULT;
diff --git a/net/xdp/xsk.h b/net/xdp/xsk.h
index ba81206..4cfd106 100644
--- a/net/xdp/xsk.h
+++ b/net/xdp/xsk.h
@@ -4,6 +4,19 @@
 #ifndef XSK_H_
 #define XSK_H_
 
+struct xdp_ring_offset_v1 {
+	__u64 producer;
+	__u64 consumer;
+	__u64 desc;
+};
+
+struct xdp_mmap_offsets_v1 {
+	struct xdp_ring_offset_v1 rx;
+	struct xdp_ring_offset_v1 tx;
+	struct xdp_ring_offset_v1 fr;
+	struct xdp_ring_offset_v1 cr;
+};
+
 static inline struct xdp_sock *xdp_sk(struct sock *sk)
 {
 	return (struct xdp_sock *)sk;
diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index 12b4978..25e41d4 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -16,6 +16,7 @@
 struct xdp_ring {
 	u32 producer ____cacheline_aligned_in_smp;
 	u32 consumer ____cacheline_aligned_in_smp;
+	u32 flags;
 };
 
 /* Used for the RX and TX queues for packets */
-- 
2.7.4


^ permalink raw reply related

* [PATCH bpf-next v3 1/6] xsk: replace ndo_xsk_async_xmit with ndo_xsk_wakeup
From: Magnus Karlsson @ 2019-07-04 12:42 UTC (permalink / raw)
  To: magnus.karlsson, bjorn.topel, ast, daniel, netdev, brouer
  Cc: bpf, bruce.richardson, ciara.loftus, jakub.kicinski, xiaolong.ye,
	qi.z.zhang, maximmi, sridhar.samudrala, kevin.laatz,
	ilias.apalodimas, kiran.patil, axboe, maciej.fijalkowski,
	maciejromanfijalkowski, intel-wired-lan
In-Reply-To: <1562244134-19069-1-git-send-email-magnus.karlsson@intel.com>

This commit replaces ndo_xsk_async_xmit with ndo_xsk_wakeup. This new
ndo provides the same functionality as before but with the addition of
a new flags field that is used to specifiy if Rx, Tx or both should be
woken up. The previous ndo only woke up Tx, as implied by the
name. The i40e and ixgbe drivers (which are all the supported ones)
are updated with this new interface.

This new ndo will be used by the new need_wakeup functionality of XDP
sockets that need to be able to wake up both Rx and Tx driver
processing.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 drivers/net/ethernet/intel/i40e/i40e_main.c          |  5 +++--
 drivers/net/ethernet/intel/i40e/i40e_xsk.c           |  7 ++++---
 drivers/net/ethernet/intel/i40e/i40e_xsk.h           |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c        |  5 +++--
 drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h |  2 +-
 drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c         |  4 ++--
 drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c  |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h  |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c    |  2 +-
 include/linux/netdevice.h                            | 14 ++++++++++++--
 net/xdp/xdp_umem.c                                   |  3 +--
 net/xdp/xsk.c                                        |  3 ++-
 12 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
index 7c43ec5..eee429d 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_main.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
@@ -12022,7 +12022,8 @@ static int i40e_xdp_setup(struct i40e_vsi *vsi,
 	if (need_reset && prog)
 		for (i = 0; i < vsi->num_queue_pairs; i++)
 			if (vsi->xdp_rings[i]->xsk_umem)
-				(void)i40e_xsk_async_xmit(vsi->netdev, i);
+				(void)i40e_xsk_wakeup(vsi->netdev, i,
+						      XDP_WAKEUP_RX);
 
 	return 0;
 }
@@ -12344,7 +12345,7 @@ static const struct net_device_ops i40e_netdev_ops = {
 	.ndo_bridge_setlink	= i40e_ndo_bridge_setlink,
 	.ndo_bpf		= i40e_xdp,
 	.ndo_xdp_xmit		= i40e_xdp_xmit,
-	.ndo_xsk_async_xmit	= i40e_xsk_async_xmit,
+	.ndo_xsk_wakeup	        = i40e_xsk_wakeup,
 };
 
 /**
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 32bad01..d0ff5d8 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -116,7 +116,7 @@ static int i40e_xsk_umem_enable(struct i40e_vsi *vsi, struct xdp_umem *umem,
 			return err;
 
 		/* Kick start the NAPI context so that receiving will start */
-		err = i40e_xsk_async_xmit(vsi->netdev, qid);
+		err = i40e_xsk_wakeup(vsi->netdev, qid, XDP_WAKEUP_RX);
 		if (err)
 			return err;
 	}
@@ -765,13 +765,14 @@ bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi,
 }
 
 /**
- * i40e_xsk_async_xmit - Implements the ndo_xsk_async_xmit
+ * i40e_xsk_wakeup - Implements the ndo_xsk_wakeup
  * @dev: the netdevice
  * @queue_id: queue id to wake up
+ * @flags: ignored in our case since we have Rx and Tx in the same NAPI.
  *
  * Returns <0 for errors, 0 otherwise.
  **/
-int i40e_xsk_async_xmit(struct net_device *dev, u32 queue_id)
+int i40e_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags)
 {
 	struct i40e_netdev_priv *np = netdev_priv(dev);
 	struct i40e_vsi *vsi = np->vsi;
diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.h b/drivers/net/ethernet/intel/i40e/i40e_xsk.h
index 8cc0a2e..9ed59c1 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.h
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.h
@@ -18,6 +18,6 @@ int i40e_clean_rx_irq_zc(struct i40e_ring *rx_ring, int budget);
 
 bool i40e_clean_xdp_tx_irq(struct i40e_vsi *vsi,
 			   struct i40e_ring *tx_ring, int napi_budget);
-int i40e_xsk_async_xmit(struct net_device *dev, u32 queue_id);
+int i40e_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
 
 #endif /* _I40E_XSK_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index b613e72..574d3f9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -10276,7 +10276,8 @@ static int ixgbe_xdp_setup(struct net_device *dev, struct bpf_prog *prog)
 	if (need_reset && prog)
 		for (i = 0; i < adapter->num_rx_queues; i++)
 			if (adapter->xdp_ring[i]->xsk_umem)
-				(void)ixgbe_xsk_async_xmit(adapter->netdev, i);
+				(void)ixgbe_xsk_wakeup(adapter->netdev, i,
+						       XDP_WAKEUP_RX);
 
 	return 0;
 }
@@ -10395,7 +10396,7 @@ static const struct net_device_ops ixgbe_netdev_ops = {
 	.ndo_features_check	= ixgbe_features_check,
 	.ndo_bpf		= ixgbe_xdp,
 	.ndo_xdp_xmit		= ixgbe_xdp_xmit,
-	.ndo_xsk_async_xmit	= ixgbe_xsk_async_xmit,
+	.ndo_xsk_wakeup         = ixgbe_xsk_wakeup,
 };
 
 static void ixgbe_disable_txr_hw(struct ixgbe_adapter *adapter,
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
index d93a690..6d01700 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_txrx_common.h
@@ -42,7 +42,7 @@ int ixgbe_clean_rx_irq_zc(struct ixgbe_q_vector *q_vector,
 void ixgbe_xsk_clean_rx_ring(struct ixgbe_ring *rx_ring);
 bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
 			    struct ixgbe_ring *tx_ring, int napi_budget);
-int ixgbe_xsk_async_xmit(struct net_device *dev, u32 queue_id);
+int ixgbe_xsk_wakeup(struct net_device *dev, u32 queue_id, u32 flags);
 void ixgbe_xsk_clean_tx_ring(struct ixgbe_ring *tx_ring);
 
 #endif /* #define _IXGBE_TXRX_COMMON_H_ */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
index 6b60955..e598af9 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_xsk.c
@@ -100,7 +100,7 @@ static int ixgbe_xsk_umem_enable(struct ixgbe_adapter *adapter,
 		ixgbe_txrx_ring_enable(adapter, qid);
 
 		/* Kick start the NAPI context so that receiving will start */
-		err = ixgbe_xsk_async_xmit(adapter->netdev, qid);
+		err = ixgbe_xsk_wakeup(adapter->netdev, qid, XDP_WAKEUP_RX);
 		if (err)
 			return err;
 	}
@@ -692,7 +692,7 @@ bool ixgbe_clean_xdp_tx_irq(struct ixgbe_q_vector *q_vector,
 	return budget > 0 && xmit_done;
 }
 
-int ixgbe_xsk_async_xmit(struct net_device *dev, u32 qid)
+int ixgbe_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
 	struct ixgbe_ring *ring;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
index 35e188cf..9704634 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
@@ -7,7 +7,7 @@
 #include "en/params.h"
 #include <net/xdp_sock.h>
 
-int mlx5e_xsk_async_xmit(struct net_device *dev, u32 qid)
+int mlx5e_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags)
 {
 	struct mlx5e_priv *priv = netdev_priv(dev);
 	struct mlx5e_params *params = &priv->channels.params;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h
index 7add18b..9c50515 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.h
@@ -8,7 +8,7 @@
 
 /* TX data path */
 
-int mlx5e_xsk_async_xmit(struct net_device *dev, u32 qid);
+int mlx5e_xsk_wakeup(struct net_device *dev, u32 qid, u32 flags);
 
 bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget);
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 67b562c..c862866 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4542,7 +4542,7 @@ const struct net_device_ops mlx5e_netdev_ops = {
 	.ndo_tx_timeout          = mlx5e_tx_timeout,
 	.ndo_bpf		 = mlx5e_xdp,
 	.ndo_xdp_xmit            = mlx5e_xdp_xmit,
-	.ndo_xsk_async_xmit      = mlx5e_xsk_async_xmit,
+	.ndo_xsk_wakeup          = mlx5e_xsk_wakeup,
 #ifdef CONFIG_MLX5_EN_ARFS
 	.ndo_rx_flow_steer	 = mlx5e_rx_flow_steer,
 #endif
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eeacebd..60eef29 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -899,6 +899,10 @@ struct netdev_bpf {
 	};
 };
 
+/* Flags for ndo_xsk_wakeup. */
+#define XDP_WAKEUP_RX (1 << 0)
+#define XDP_WAKEUP_TX (1 << 1)
+
 #ifdef CONFIG_XFRM_OFFLOAD
 struct xfrmdev_ops {
 	int	(*xdo_dev_state_add) (struct xfrm_state *x);
@@ -1225,6 +1229,12 @@ struct tlsdev_ops;
  *	that got dropped are freed/returned via xdp_return_frame().
  *	Returns negative number, means general error invoking ndo, meaning
  *	no frames were xmit'ed and core-caller will free all frames.
+ * int (*ndo_xsk_wakeup)(struct net_device *dev, u32 queue_id, u32 flags);
+ *      This function is used to wake up the softirq, ksoftirqd or kthread
+ *	responsible for sending and/or receiving packets on a specific
+ *	queue id bound to an AF_XDP socket. The flags field specifies if
+ *	only RX, only Tx, or both should be woken up using the flags
+ *	XDP_WAKEUP_RX and XDP_WAKEUP_TX.
  * struct devlink_port *(*ndo_get_devlink_port)(struct net_device *dev);
  *	Get devlink port instance associated with a given netdev.
  *	Called with a reference on the netdevice and devlink locks only,
@@ -1424,8 +1434,8 @@ struct net_device_ops {
 	int			(*ndo_xdp_xmit)(struct net_device *dev, int n,
 						struct xdp_frame **xdp,
 						u32 flags);
-	int			(*ndo_xsk_async_xmit)(struct net_device *dev,
-						      u32 queue_id);
+	int			(*ndo_xsk_wakeup)(struct net_device *dev,
+						  u32 queue_id, u32 flags);
 	struct devlink_port *	(*ndo_get_devlink_port)(struct net_device *dev);
 };
 
diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
index 9c6de4f..803554b 100644
--- a/net/xdp/xdp_umem.c
+++ b/net/xdp/xdp_umem.c
@@ -109,8 +109,7 @@ int xdp_umem_assign_dev(struct xdp_umem *umem, struct net_device *dev,
 		/* For copy-mode, we are done. */
 		goto out_rtnl_unlock;
 
-	if (!dev->netdev_ops->ndo_bpf ||
-	    !dev->netdev_ops->ndo_xsk_async_xmit) {
+	if (!dev->netdev_ops->ndo_bpf || !dev->netdev_ops->ndo_xsk_wakeup) {
 		err = -EOPNOTSUPP;
 		goto err_unreg_umem;
 	}
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 74417a8..cf8898f 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -200,7 +200,8 @@ static int xsk_zc_xmit(struct sock *sk)
 	struct xdp_sock *xs = xdp_sk(sk);
 	struct net_device *dev = xs->dev;
 
-	return dev->netdev_ops->ndo_xsk_async_xmit(dev, xs->queue_id);
+	return dev->netdev_ops->ndo_xsk_wakeup(dev, xs->queue_id,
+					       XDP_WAKEUP_TX);
 }
 
 static void xsk_destruct_skb(struct sk_buff *skb)
-- 
2.7.4


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox