Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] nfp: control processor DMA support and RJ45
From: Jakub Kicinski @ 2019-02-27  4:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski

Hi!

This series starts with adding support for reporting twisted pair
media type in ethtool.

Remaining patches add support for using DMA with the control/service
processor.  Currently we always copy the command data into card's
memory.  DMA support allows us to have the NSP read the data from
host memory by itself.  Unfortunately, the FW loading and flashing
cannot directly map the buffers for DMA because (a) the firmware
ABI returns const buffers, and (b) the buffers may be vmalloc()ed
in many mysterious/unmappable way.  So just bite the bullet -
allocate new host buffer for the command and copy.

As Dirk explains, the NSP now supports updating all FWs at once
which means the max flashing time grew significantly.  He bumps
the max wait to avoid timeouts.

Dirk van der Merwe (1):
  nfp: nsp: set higher timeout for flash bundle

Jakub Kicinski (4):
  nfp: report RJ45 connector in ethtool
  nfp: nsp: use fractional size of the buffer
  nfp: nsp: move default buffer handling into its own function
  nfp: nsp: allow the use of DMA buffer

 .../ethernet/netronome/nfp/nfpcore/nfp_nsp.c  | 285 +++++++++++++++---
 .../ethernet/netronome/nfp/nfpcore/nfp_nsp.h  |   1 +
 .../netronome/nfp/nfpcore/nfp_nsp_eth.c       |   3 +
 3 files changed, 243 insertions(+), 46 deletions(-)

-- 
2.19.2


^ permalink raw reply

* [PATCH net-next 1/5] nfp: report RJ45 connector in ethtool
From: Jakub Kicinski @ 2019-02-27  4:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190227041933.15194-1-jakub.kicinski@netronome.com>

Add support for reporting twisted pair port type.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h     | 1 +
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
index 246e213f1514..bd9c358c646f 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.h
@@ -49,6 +49,7 @@ enum nfp_eth_interface {
 	NFP_INTERFACE_SFPP	= 10,
 	NFP_INTERFACE_SFP28	= 28,
 	NFP_INTERFACE_QSFP	= 40,
+	NFP_INTERFACE_RJ45	= 45,
 	NFP_INTERFACE_CXP	= 100,
 	NFP_INTERFACE_QSFP28	= 112,
 };
diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
index f6f028fa5db9..311a5be25acb 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp_eth.c
@@ -206,6 +206,9 @@ nfp_eth_calc_port_type(struct nfp_cpp *cpp, struct nfp_eth_table_port *entry)
 	if (entry->interface == NFP_INTERFACE_NONE) {
 		entry->port_type = PORT_NONE;
 		return;
+	} else if (entry->interface == NFP_INTERFACE_RJ45) {
+		entry->port_type = PORT_TP;
+		return;
 	}
 
 	if (entry->media == NFP_MEDIA_FIBRE)
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 2/5] nfp: nsp: use fractional size of the buffer
From: Jakub Kicinski @ 2019-02-27  4:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190227041933.15194-1-jakub.kicinski@netronome.com>

NSP expresses the buffer size in MB and 4 kB blocks.  For small
buffers the kB part may make a difference, so count it in.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 .../net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c    | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index a9d53df0070c..22208b03ff49 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -49,6 +49,7 @@
 #define   NSP_DFLT_BUFFER_ADDRESS	GENMASK_ULL(39, 0)
 
 #define NSP_DFLT_BUFFER_CONFIG	0x20
+#define   NSP_DFLT_BUFFER_SIZE_4KB	GENMASK_ULL(15, 8)
 #define   NSP_DFLT_BUFFER_SIZE_MB	GENMASK_ULL(7, 0)
 
 #define NSP_MAGIC		0xab10
@@ -413,8 +414,8 @@ static int nfp_nsp_command(struct nfp_nsp *state, u16 code)
 static int
 nfp_nsp_command_buf(struct nfp_nsp *nsp, struct nfp_nsp_command_buf_arg *arg)
 {
+	unsigned int def_size, max_size;
 	struct nfp_cpp *cpp = nsp->cpp;
-	unsigned int max_size;
 	u64 reg, cpp_buf;
 	int ret, err;
 	u32 cpp_id;
@@ -433,11 +434,11 @@ nfp_nsp_command_buf(struct nfp_nsp *nsp, struct nfp_nsp_command_buf_arg *arg)
 		return err;
 
 	max_size = max(arg->in_size, arg->out_size);
-	if (FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M < max_size) {
-		nfp_err(cpp, "NSP: default buffer too small for command 0x%04x (%llu < %u)\n",
-			arg->arg.code,
-			FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M,
-			max_size);
+	def_size = FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M +
+		   FIELD_GET(NSP_DFLT_BUFFER_SIZE_4KB, reg) * SZ_4K;
+	if (def_size < max_size) {
+		nfp_err(cpp, "NSP: default buffer too small for command 0x%04x (%u < %u)\n",
+			arg->arg.code, def_size, max_size);
 		return -EINVAL;
 	}
 
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 3/5] nfp: nsp: move default buffer handling into its own function
From: Jakub Kicinski @ 2019-02-27  4:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190227041933.15194-1-jakub.kicinski@netronome.com>

DMA version of NSP communication is coming, move the code which
copies data into the NFP buffer into a separate function.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 .../ethernet/netronome/nfp/nfpcore/nfp_nsp.c  | 93 ++++++++++---------
 1 file changed, 51 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index 22208b03ff49..28262b0fc89a 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -122,16 +122,14 @@ struct nfp_nsp {
  * @code:	NFP SP Command Code
  * @timeout_sec:Timeout value to wait for completion in seconds
  * @option:	NFP SP Command Argument
- * @buff_cpp:	NFP SP Buffer CPP Address info
- * @buff_addr:	NFP SP Buffer Host address
+ * @buf:	NFP SP Buffer Address
  * @error_cb:	Callback for interpreting option if error occurred
  */
 struct nfp_nsp_command_arg {
 	u16 code;
 	unsigned int timeout_sec;
 	u32 option;
-	u32 buff_cpp;
-	u64 buff_addr;
+	u64 buf;
 	void (*error_cb)(struct nfp_nsp *state, u32 ret_val);
 };
 
@@ -345,16 +343,7 @@ __nfp_nsp_command(struct nfp_nsp *state, const struct nfp_nsp_command_arg *arg)
 	if (err)
 		return err;
 
-	if (!FIELD_FIT(NSP_BUFFER_CPP, arg->buff_cpp >> 8) ||
-	    !FIELD_FIT(NSP_BUFFER_ADDRESS, arg->buff_addr)) {
-		nfp_err(cpp, "Host buffer out of reach %08x %016llx\n",
-			arg->buff_cpp, arg->buff_addr);
-		return -EINVAL;
-	}
-
-	err = nfp_cpp_writeq(cpp, nsp_cpp, nsp_buffer,
-			     FIELD_PREP(NSP_BUFFER_CPP, arg->buff_cpp >> 8) |
-			     FIELD_PREP(NSP_BUFFER_ADDRESS, arg->buff_addr));
+	err = nfp_cpp_writeq(cpp, nsp_cpp, nsp_buffer, arg->buf);
 	if (err < 0)
 		return err;
 
@@ -412,36 +401,14 @@ static int nfp_nsp_command(struct nfp_nsp *state, u16 code)
 }
 
 static int
-nfp_nsp_command_buf(struct nfp_nsp *nsp, struct nfp_nsp_command_buf_arg *arg)
+nfp_nsp_command_buf_def(struct nfp_nsp *nsp,
+			struct nfp_nsp_command_buf_arg *arg)
 {
-	unsigned int def_size, max_size;
 	struct nfp_cpp *cpp = nsp->cpp;
 	u64 reg, cpp_buf;
-	int ret, err;
+	int err, ret;
 	u32 cpp_id;
 
-	if (nsp->ver.minor < 13) {
-		nfp_err(cpp, "NSP: Code 0x%04x with buffer not supported (ABI %hu.%hu)\n",
-			arg->arg.code, nsp->ver.major, nsp->ver.minor);
-		return -EOPNOTSUPP;
-	}
-
-	err = nfp_cpp_readq(cpp, nfp_resource_cpp_id(nsp->res),
-			    nfp_resource_address(nsp->res) +
-			    NSP_DFLT_BUFFER_CONFIG,
-			    &reg);
-	if (err < 0)
-		return err;
-
-	max_size = max(arg->in_size, arg->out_size);
-	def_size = FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M +
-		   FIELD_GET(NSP_DFLT_BUFFER_SIZE_4KB, reg) * SZ_4K;
-	if (def_size < max_size) {
-		nfp_err(cpp, "NSP: default buffer too small for command 0x%04x (%u < %u)\n",
-			arg->arg.code, def_size, max_size);
-		return -EINVAL;
-	}
-
 	err = nfp_cpp_readq(cpp, nfp_resource_cpp_id(nsp->res),
 			    nfp_resource_address(nsp->res) +
 			    NSP_DFLT_BUFFER,
@@ -460,15 +427,21 @@ nfp_nsp_command_buf(struct nfp_nsp *nsp, struct nfp_nsp_command_buf_arg *arg)
 	}
 	/* Zero out remaining part of the buffer */
 	if (arg->out_buf && arg->out_size && arg->out_size > arg->in_size) {
-		memset(arg->out_buf, 0, arg->out_size - arg->in_size);
 		err = nfp_cpp_write(cpp, cpp_id, cpp_buf + arg->in_size,
 				    arg->out_buf, arg->out_size - arg->in_size);
 		if (err < 0)
 			return err;
 	}
 
-	arg->arg.buff_cpp = cpp_id;
-	arg->arg.buff_addr = cpp_buf;
+	if (!FIELD_FIT(NSP_BUFFER_CPP, cpp_id >> 8) ||
+	    !FIELD_FIT(NSP_BUFFER_ADDRESS, cpp_buf)) {
+		nfp_err(cpp, "Buffer out of reach %08x %016llx\n",
+			cpp_id, cpp_buf);
+		return -EINVAL;
+	}
+
+	arg->arg.buf = FIELD_PREP(NSP_BUFFER_CPP, cpp_id >> 8) |
+		       FIELD_PREP(NSP_BUFFER_ADDRESS, cpp_buf);
 	ret = __nfp_nsp_command(nsp, &arg->arg);
 	if (ret < 0)
 		return ret;
@@ -483,6 +456,42 @@ nfp_nsp_command_buf(struct nfp_nsp *nsp, struct nfp_nsp_command_buf_arg *arg)
 	return ret;
 }
 
+static int
+nfp_nsp_command_buf(struct nfp_nsp *nsp, struct nfp_nsp_command_buf_arg *arg)
+{
+	unsigned int def_size, max_size;
+	struct nfp_cpp *cpp = nsp->cpp;
+	u64 reg;
+	int err;
+
+	if (nsp->ver.minor < 13) {
+		nfp_err(cpp, "NSP: Code 0x%04x with buffer not supported (ABI %hu.%hu)\n",
+			arg->arg.code, nsp->ver.major, nsp->ver.minor);
+		return -EOPNOTSUPP;
+	}
+
+	err = nfp_cpp_readq(cpp, nfp_resource_cpp_id(nsp->res),
+			    nfp_resource_address(nsp->res) +
+			    NSP_DFLT_BUFFER_CONFIG,
+			    &reg);
+	if (err < 0)
+		return err;
+
+	/* Zero out undefined part of the out buffer */
+	if (arg->out_buf && arg->out_size && arg->out_size > arg->in_size)
+		memset(arg->out_buf, 0, arg->out_size - arg->in_size);
+
+	max_size = max(arg->in_size, arg->out_size);
+	def_size = FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M +
+		   FIELD_GET(NSP_DFLT_BUFFER_SIZE_4KB, reg) * SZ_4K;
+	if (def_size >= max_size)
+		return nfp_nsp_command_buf_def(nsp, arg);
+
+	nfp_err(cpp, "NSP: default buffer too small for command 0x%04x (%u < %u)\n",
+		arg->arg.code, def_size, max_size);
+	return -EINVAL;
+}
+
 int nfp_nsp_wait(struct nfp_nsp *state)
 {
 	const unsigned long wait_until = jiffies + NFP_NSP_TIMEOUT_BOOT * HZ;
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 4/5] nfp: nsp: allow the use of DMA buffer
From: Jakub Kicinski @ 2019-02-27  4:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Jakub Kicinski
In-Reply-To: <20190227041933.15194-1-jakub.kicinski@netronome.com>

Newer versions of NSP can access host memory.  Simplest access
type requires all data to be in one contiguous area.  Since we
don't have the guarantee on where callers of the NSP ABI will
allocate their buffers we allocate a bounce buffer and copy
the data in and out.

Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
Reviewed-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
---
 .../ethernet/netronome/nfp/nfpcore/nfp_nsp.c  | 196 +++++++++++++++++-
 1 file changed, 191 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index 28262b0fc89a..dd6256841a37 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -13,6 +13,7 @@
 #include <linux/firmware.h>
 #include <linux/kernel.h>
 #include <linux/kthread.h>
+#include <linux/overflow.h>
 #include <linux/sizes.h>
 #include <linux/slab.h>
 
@@ -37,6 +38,7 @@
 #define NSP_COMMAND		0x08
 #define   NSP_COMMAND_OPTION	GENMASK_ULL(63, 32)
 #define   NSP_COMMAND_CODE	GENMASK_ULL(31, 16)
+#define   NSP_COMMAND_DMA_BUF	BIT_ULL(1)
 #define   NSP_COMMAND_START	BIT_ULL(0)
 
 /* CPP address to retrieve the data from */
@@ -49,9 +51,12 @@
 #define   NSP_DFLT_BUFFER_ADDRESS	GENMASK_ULL(39, 0)
 
 #define NSP_DFLT_BUFFER_CONFIG	0x20
+#define   NSP_DFLT_BUFFER_DMA_CHUNK_ORDER	GENMASK_ULL(63, 58)
 #define   NSP_DFLT_BUFFER_SIZE_4KB	GENMASK_ULL(15, 8)
 #define   NSP_DFLT_BUFFER_SIZE_MB	GENMASK_ULL(7, 0)
 
+#define NFP_CAP_CMD_DMA_SG	0x28
+
 #define NSP_MAGIC		0xab10
 #define NSP_MAJOR		0
 #define NSP_MINOR		8
@@ -92,6 +97,16 @@ enum nfp_nsp_cmd {
 	SPCODE_VERSIONS		= 21, /* Report FW versions */
 };
 
+struct nfp_nsp_dma_buf {
+	__le32 chunk_cnt;
+	__le32 reserved[3];
+	struct {
+		__le32 size;
+		__le32 reserved;
+		__le64 addr;
+	} descs[];
+};
+
 static const struct {
 	int code;
 	const char *msg;
@@ -120,6 +135,7 @@ struct nfp_nsp {
 /**
  * struct nfp_nsp_command_arg - NFP command argument structure
  * @code:	NFP SP Command Code
+ * @dma:	@buf points to a host buffer, not NSP buffer
  * @timeout_sec:Timeout value to wait for completion in seconds
  * @option:	NFP SP Command Argument
  * @buf:	NFP SP Buffer Address
@@ -127,6 +143,7 @@ struct nfp_nsp {
  */
 struct nfp_nsp_command_arg {
 	u16 code;
+	bool dma;
 	unsigned int timeout_sec;
 	u32 option;
 	u64 buf;
@@ -350,6 +367,7 @@ __nfp_nsp_command(struct nfp_nsp *state, const struct nfp_nsp_command_arg *arg)
 	err = nfp_cpp_writeq(cpp, nsp_cpp, nsp_command,
 			     FIELD_PREP(NSP_COMMAND_OPTION, arg->option) |
 			     FIELD_PREP(NSP_COMMAND_CODE, arg->code) |
+			     FIELD_PREP(NSP_COMMAND_DMA_BUF, arg->dma) |
 			     FIELD_PREP(NSP_COMMAND_START, 1));
 	if (err < 0)
 		return err;
@@ -456,10 +474,174 @@ nfp_nsp_command_buf_def(struct nfp_nsp *nsp,
 	return ret;
 }
 
+static int
+nfp_nsp_command_buf_dma_sg(struct nfp_nsp *nsp,
+			   struct nfp_nsp_command_buf_arg *arg,
+			   unsigned int max_size, unsigned int chunk_order,
+			   unsigned int dma_order)
+{
+	struct nfp_cpp *cpp = nsp->cpp;
+	struct nfp_nsp_dma_buf *desc;
+	struct {
+		dma_addr_t dma_addr;
+		unsigned long len;
+		void *chunk;
+	} *chunks;
+	size_t chunk_size, dma_size;
+	dma_addr_t dma_desc;
+	struct device *dev;
+	unsigned long off;
+	int i, ret, nseg;
+	size_t desc_sz;
+
+	chunk_size = BIT_ULL(chunk_order);
+	dma_size = BIT_ULL(dma_order);
+	nseg = DIV_ROUND_UP(max_size, chunk_size);
+
+	chunks = kzalloc(array_size(sizeof(*chunks), nseg), GFP_KERNEL);
+	if (!chunks)
+		return -ENOMEM;
+
+	off = 0;
+	ret = -ENOMEM;
+	for (i = 0; i < nseg; i++) {
+		unsigned long coff;
+
+		chunks[i].chunk = kmalloc(chunk_size,
+					  GFP_KERNEL | __GFP_NOWARN);
+		if (!chunks[i].chunk)
+			goto exit_free_prev;
+
+		chunks[i].len = min_t(u64, chunk_size, max_size - off);
+
+		coff = 0;
+		if (arg->in_size > off) {
+			coff = min_t(u64, arg->in_size - off, chunk_size);
+			memcpy(chunks[i].chunk, arg->in_buf + off, coff);
+		}
+		memset(chunks[i].chunk + coff, 0, chunk_size - coff);
+
+		off += chunks[i].len;
+	}
+
+	dev = nfp_cpp_device(cpp)->parent;
+
+	for (i = 0; i < nseg; i++) {
+		dma_addr_t addr;
+
+		addr = dma_map_single(dev, chunks[i].chunk, chunks[i].len,
+				      DMA_BIDIRECTIONAL);
+		chunks[i].dma_addr = addr;
+
+		ret = dma_mapping_error(dev, addr);
+		if (ret)
+			goto exit_unmap_prev;
+
+		if (WARN_ONCE(round_down(addr, dma_size) !=
+			      round_down(addr + chunks[i].len - 1, dma_size),
+			      "unaligned DMA address: %pad %lu %zd\n",
+			      &addr, chunks[i].len, dma_size)) {
+			ret = -EFAULT;
+			i++;
+			goto exit_unmap_prev;
+		}
+	}
+
+	desc_sz = struct_size(desc, descs, nseg);
+	desc = kmalloc(desc_sz, GFP_KERNEL);
+	if (!desc) {
+		ret = -ENOMEM;
+		goto exit_unmap_all;
+	}
+
+	desc->chunk_cnt = cpu_to_le32(nseg);
+	for (i = 0; i < nseg; i++) {
+		desc->descs[i].size = cpu_to_le32(chunks[i].len);
+		desc->descs[i].addr = cpu_to_le64(chunks[i].dma_addr);
+	}
+
+	dma_desc = dma_map_single(dev, desc, desc_sz, DMA_TO_DEVICE);
+	ret = dma_mapping_error(dev, dma_desc);
+	if (ret)
+		goto exit_free_desc;
+
+	arg->arg.dma = true;
+	arg->arg.buf = dma_desc;
+	ret = __nfp_nsp_command(nsp, &arg->arg);
+	if (ret < 0)
+		goto exit_unmap_desc;
+
+	i = 0;
+	off = 0;
+	while (off < arg->out_size) {
+		unsigned int len;
+
+		len = min_t(u64, chunks[i].len, arg->out_size - off);
+		memcpy(arg->out_buf + off, chunks[i].chunk, len);
+		off += len;
+		i++;
+	}
+
+exit_unmap_desc:
+	dma_unmap_single(dev, dma_desc, desc_sz, DMA_TO_DEVICE);
+exit_free_desc:
+	kfree(desc);
+exit_unmap_all:
+	i = nseg;
+exit_unmap_prev:
+	while (--i >= 0)
+		dma_unmap_single(dev, chunks[i].dma_addr, chunks[i].len,
+				 DMA_BIDIRECTIONAL);
+	i = nseg;
+exit_free_prev:
+	while (--i >= 0)
+		kfree(chunks[i].chunk);
+	kfree(chunks);
+	if (ret < 0)
+		nfp_err(cpp, "NSP: SG DMA failed for command 0x%04x: %d (sz:%d cord:%d)\n",
+			arg->arg.code, ret, max_size, chunk_order);
+	return ret;
+}
+
+static int
+nfp_nsp_command_buf_dma(struct nfp_nsp *nsp,
+			struct nfp_nsp_command_buf_arg *arg,
+			unsigned int max_size, unsigned int dma_order)
+{
+	unsigned int chunk_order, buf_order;
+	struct nfp_cpp *cpp = nsp->cpp;
+	bool sg_ok;
+	u64 reg;
+	int err;
+
+	buf_order = order_base_2(roundup_pow_of_two(max_size));
+
+	err = nfp_cpp_readq(cpp, nfp_resource_cpp_id(nsp->res),
+			    nfp_resource_address(nsp->res) + NFP_CAP_CMD_DMA_SG,
+			    &reg);
+	if (err < 0)
+		return err;
+	sg_ok = reg & BIT_ULL(arg->arg.code - 1);
+
+	if (!sg_ok) {
+		if (buf_order > dma_order) {
+			nfp_err(cpp, "NSP: can't service non-SG DMA for command 0x%04x\n",
+				arg->arg.code);
+			return -ENOMEM;
+		}
+		chunk_order = buf_order;
+	} else {
+		chunk_order = min_t(unsigned int, dma_order, PAGE_SHIFT);
+	}
+
+	return nfp_nsp_command_buf_dma_sg(nsp, arg, max_size, chunk_order,
+					  dma_order);
+}
+
 static int
 nfp_nsp_command_buf(struct nfp_nsp *nsp, struct nfp_nsp_command_buf_arg *arg)
 {
-	unsigned int def_size, max_size;
+	unsigned int dma_order, def_size, max_size;
 	struct nfp_cpp *cpp = nsp->cpp;
 	u64 reg;
 	int err;
@@ -484,12 +666,16 @@ nfp_nsp_command_buf(struct nfp_nsp *nsp, struct nfp_nsp_command_buf_arg *arg)
 	max_size = max(arg->in_size, arg->out_size);
 	def_size = FIELD_GET(NSP_DFLT_BUFFER_SIZE_MB, reg) * SZ_1M +
 		   FIELD_GET(NSP_DFLT_BUFFER_SIZE_4KB, reg) * SZ_4K;
-	if (def_size >= max_size)
+	dma_order = FIELD_GET(NSP_DFLT_BUFFER_DMA_CHUNK_ORDER, reg);
+	if (def_size >= max_size) {
 		return nfp_nsp_command_buf_def(nsp, arg);
+	} else if (!dma_order) {
+		nfp_err(cpp, "NSP: default buffer too small for command 0x%04x (%u < %u)\n",
+			arg->arg.code, def_size, max_size);
+		return -EINVAL;
+	}
 
-	nfp_err(cpp, "NSP: default buffer too small for command 0x%04x (%u < %u)\n",
-		arg->arg.code, def_size, max_size);
-	return -EINVAL;
+	return nfp_nsp_command_buf_dma(nsp, arg, max_size, dma_order);
 }
 
 int nfp_nsp_wait(struct nfp_nsp *state)
-- 
2.19.2


^ permalink raw reply related

* [PATCH net-next 5/5] nfp: nsp: set higher timeout for flash bundle
From: Jakub Kicinski @ 2019-02-27  4:19 UTC (permalink / raw)
  To: davem; +Cc: netdev, oss-drivers, Dirk van der Merwe
In-Reply-To: <20190227041933.15194-1-jakub.kicinski@netronome.com>

From: Dirk van der Merwe <dirk.vandermerwe@netronome.com>

The management firmware now supports being passed a bundle with
multiple components to be stored in flash at once. This makes it
easier to update all components to a known state with a single
user command, however, this also has the potential to increase
the time required to perform the update significantly.

The management firmware only updates the components out of a bundle
which are outdated, however, we need to make sure we can handle
the absolute worst case where a CPLD update can take a long time
to perform.

We set a very conservative total timeout of 900s which already
adds a contingency.

Signed-off-by: Dirk van der Merwe <dirk.vandermerwe@netronome.com>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
 drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
index dd6256841a37..3a4e224a64b7 100644
--- a/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
+++ b/drivers/net/ethernet/netronome/nfp/nfpcore/nfp_nsp.c
@@ -799,10 +799,7 @@ int nfp_nsp_write_flash(struct nfp_nsp *state, const struct firmware *fw)
 		{
 			.code		= SPCODE_NSP_WRITE_FLASH,
 			.option		= fw->size,
-			/* The flash time is specified to take a maximum of 70s
-			 * so we add an additional factor to this spec time.
-			 */
-			.timeout_sec	= 2.5 * 70,
+			.timeout_sec	= 900,
 		},
 		.in_buf		= fw->data,
 		.in_size	= fw->size,
-- 
2.19.2


^ permalink raw reply related

* [PATCH 0/8] net: ethernet: reduce calls to memset(,0)
From: Robert Eshleman @ 2019-02-27  6:04 UTC (permalink / raw)
  To: bobbyeshleman; +Cc: David S. Miller, netdev, linux-kernel

This patch series removes calls to memset(,0) that are
redundant when used in conjunction with a zalloc call or
by simple zero-assignment of structs.

Robert Eshleman (8):
  net/mlx4: use kzalloc instead of kmalloc
  net/mlxsw: use pci_zalloc_consistent instead of pci_alloc_consistent
  tlan: use pci_zalloc instead of pci_alloc
  qed: remove unnecessary memsets
  at12: use pci_zalloc instead of pci_alloc
  netxen: remove unnecessary memset(,0) calls
  net: seeq: replace kmalloc / memset(,0) with kzalloc
  net: ethernet: ixp4xx_eth: remove memset(,0) with zalloc

 drivers/net/ethernet/atheros/atlx/atl2.c      |  5 +-
 drivers/net/ethernet/mellanox/mlx4/cmd.c      |  1 -
 drivers/net/ethernet/mellanox/mlx4/en_rx.c    |  3 +-
 drivers/net/ethernet/mellanox/mlxsw/pci.c     |  7 +-
 .../ethernet/qlogic/netxen/netxen_nic_ctx.c   | 36 ++++------
 drivers/net/ethernet/qlogic/qed/qed_cxt.c     |  4 +-
 drivers/net/ethernet/qlogic/qed/qed_hw.c      |  3 +-
 drivers/net/ethernet/qlogic/qed/qed_mcp.c     | 70 ++++++-------------
 drivers/net/ethernet/seeq/ether3.c            |  3 +-
 drivers/net/ethernet/ti/tlan.c                |  9 ++-
 drivers/net/ethernet/xscale/ixp4xx_eth.c      |  7 +-
 11 files changed, 52 insertions(+), 96 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH 4/8] qed: remove unnecessary memsets
From: Robert Eshleman @ 2019-02-27  6:09 UTC (permalink / raw)
  To: bobbyeshleman
  Cc: Tariq Toukan, David S. Miller, Ariel Elior, GR-everest-linux-l2,
	netdev, linux-rdma, linux-kernel
In-Reply-To: <cover.1551246708.git.bobbyeshleman@gmail.com>

This patch replaces unnecessary memset(,0) calls with
simply assigning structs to zero.

Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
 drivers/net/ethernet/qlogic/qed/qed_cxt.c |  4 +-
 drivers/net/ethernet/qlogic/qed/qed_hw.c  |  3 +-
 drivers/net/ethernet/qlogic/qed/qed_mcp.c | 70 ++++++++---------------
 3 files changed, 25 insertions(+), 52 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qed/qed_cxt.c b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
index c2ad405b2f50..0452ef2fdf1d 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_cxt.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_cxt.c
@@ -902,12 +902,10 @@ static int qed_cxt_src_t2_alloc(struct qed_hwfn *p_hwfn)
 	struct qed_cxt_mngr *p_mngr = p_hwfn->p_cxt_mngr;
 	u32 conn_num, total_size, ent_per_page, psz, i;
 	struct qed_ilt_client_cfg *p_src;
-	struct qed_src_iids src_iids;
+	struct qed_src_iids src_iids = {0};
 	struct qed_dma_mem *p_t2;
 	int rc;
 
-	memset(&src_iids, 0, sizeof(src_iids));
-
 	/* if the SRC ILT client is inactive - there are no connection
 	 * requiring the searcer, leave.
 	 */
diff --git a/drivers/net/ethernet/qlogic/qed/qed_hw.c b/drivers/net/ethernet/qlogic/qed/qed_hw.c
index 70504dcf4087..b8ca3a31409b 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_hw.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_hw.c
@@ -831,7 +831,7 @@ int qed_dmae_sanity(struct qed_hwfn *p_hwfn,
 		    struct qed_ptt *p_ptt, const char *phase)
 {
 	u32 size = PAGE_SIZE / 2, val;
-	struct qed_dmae_params params;
+	struct qed_dmae_params params = {0};
 	int rc = 0;
 	dma_addr_t p_phys;
 	void *p_virt;
@@ -864,7 +864,6 @@ int qed_dmae_sanity(struct qed_hwfn *p_hwfn,
 		   (u64)p_phys,
 		   p_virt, (u64)(p_phys + size), (u8 *)p_virt + size, size);
 
-	memset(&params, 0, sizeof(params));
 	rc = qed_dmae_host2host(p_hwfn, p_ptt, p_phys, p_phys + size,
 				size / 4 /* size_in_dwords */, &params);
 	if (rc) {
diff --git a/drivers/net/ethernet/qlogic/qed/qed_mcp.c b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
index e7f18e34ff0d..e1b72fc819a9 100644
--- a/drivers/net/ethernet/qlogic/qed/qed_mcp.c
+++ b/drivers/net/ethernet/qlogic/qed/qed_mcp.c
@@ -642,10 +642,9 @@ int qed_mcp_cmd(struct qed_hwfn *p_hwfn,
 		u32 *o_mcp_resp,
 		u32 *o_mcp_param)
 {
-	struct qed_mcp_mb_params mb_params;
+	struct qed_mcp_mb_params mb_params = {0};
 	int rc;
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = cmd;
 	mb_params.param = param;
 
@@ -667,10 +666,9 @@ qed_mcp_nvm_wr_cmd(struct qed_hwfn *p_hwfn,
 		   u32 *o_mcp_resp,
 		   u32 *o_mcp_param, u32 i_txn_size, u32 *i_buf)
 {
-	struct qed_mcp_mb_params mb_params;
+	struct qed_mcp_mb_params mb_params = {0};
 	int rc;
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = cmd;
 	mb_params.param = param;
 	mb_params.p_data_src = i_buf;
@@ -695,11 +693,10 @@ int qed_mcp_nvm_rd_cmd(struct qed_hwfn *p_hwfn,
 		       u32 *o_mcp_resp,
 		       u32 *o_mcp_param, u32 *o_txn_size, u32 *o_buf)
 {
-	struct qed_mcp_mb_params mb_params;
+	struct qed_mcp_mb_params mb_params = {0};
 	u8 raw_data[MCP_DRV_NVM_BUF_LEN];
 	int rc;
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = cmd;
 	mb_params.param = param;
 	mb_params.p_data_dst = raw_data;
@@ -821,13 +818,12 @@ __qed_mcp_load_req(struct qed_hwfn *p_hwfn,
 		   struct qed_load_req_in_params *p_in_params,
 		   struct qed_load_req_out_params *p_out_params)
 {
-	struct qed_mcp_mb_params mb_params;
-	struct load_req_stc load_req;
+	struct qed_mcp_mb_params mb_params = {0};
+	struct load_req_stc load_req = {0};
 	struct load_rsp_stc load_rsp;
 	u32 hsi_ver;
 	int rc;
 
-	memset(&load_req, 0, sizeof(load_req));
 	load_req.drv_ver_0 = p_in_params->drv_ver_0;
 	load_req.drv_ver_1 = p_in_params->drv_ver_1;
 	load_req.fw_ver = p_in_params->fw_ver;
@@ -843,7 +839,6 @@ __qed_mcp_load_req(struct qed_hwfn *p_hwfn,
 		  DRV_ID_MCP_HSI_VER_CURRENT :
 		  (p_in_params->hsi_ver << DRV_ID_MCP_HSI_VER_SHIFT);
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = DRV_MSG_CODE_LOAD_REQ;
 	mb_params.param = PDA_COMP | hsi_ver | p_hwfn->cdev->drv_type;
 	mb_params.p_data_src = &load_req;
@@ -959,12 +954,11 @@ int qed_mcp_load_req(struct qed_hwfn *p_hwfn,
 		     struct qed_ptt *p_ptt,
 		     struct qed_load_req_params *p_params)
 {
-	struct qed_load_req_out_params out_params;
-	struct qed_load_req_in_params in_params;
+	struct qed_load_req_out_params out_params = {0};
+	struct qed_load_req_in_params in_params = {0};
 	u8 mfw_drv_role, mfw_force_cmd;
 	int rc;
 
-	memset(&in_params, 0, sizeof(in_params));
 	in_params.hsi_ver = QED_LOAD_REQ_HSI_VER_DEFAULT;
 	in_params.drv_ver_0 = QED_VERSION;
 	in_params.drv_ver_1 = qed_get_config_bitmap();
@@ -981,7 +975,6 @@ int qed_mcp_load_req(struct qed_hwfn *p_hwfn,
 	in_params.force_cmd = mfw_force_cmd;
 	in_params.avoid_eng_reset = p_params->avoid_eng_reset;
 
-	memset(&out_params, 0, sizeof(out_params));
 	rc = __qed_mcp_load_req(p_hwfn, p_ptt, &in_params, &out_params);
 	if (rc)
 		return rc;
@@ -1072,7 +1065,7 @@ int qed_mcp_load_req(struct qed_hwfn *p_hwfn,
 
 int qed_mcp_unload_req(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 {
-	struct qed_mcp_mb_params mb_params;
+	struct qed_mcp_mb_params mb_params = {0};
 	u32 wol_param;
 
 	switch (p_hwfn->cdev->wol_config) {
@@ -1091,7 +1084,6 @@ int qed_mcp_unload_req(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 		wol_param = DRV_MB_PARAM_UNLOAD_WOL_MCP;
 	}
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = DRV_MSG_CODE_UNLOAD_REQ;
 	mb_params.param = wol_param;
 	mb_params.flags = QED_MB_FLAG_CAN_SLEEP | QED_MB_FLAG_AVOID_BLOCK;
@@ -1101,17 +1093,15 @@ int qed_mcp_unload_req(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 
 int qed_mcp_unload_done(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt)
 {
-	struct qed_mcp_mb_params mb_params;
-	struct mcp_mac wol_mac;
+	struct qed_mcp_mb_params mb_params = {0};
+	struct mcp_mac wol_mac = {0};
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = DRV_MSG_CODE_UNLOAD_DONE;
 
 	/* Set the primary MAC if WoL is enabled */
 	if (p_hwfn->cdev->wol_config == QED_OV_WOL_ENABLED) {
 		u8 *p_mac = p_hwfn->cdev->wol_mac;
 
-		memset(&wol_mac, 0, sizeof(wol_mac));
 		wol_mac.mac_upper = p_mac[0] << 8 | p_mac[1];
 		wol_mac.mac_lower = p_mac[2] << 24 | p_mac[3] << 16 |
 				    p_mac[4] << 8 | p_mac[5];
@@ -1167,7 +1157,7 @@ int qed_mcp_ack_vf_flr(struct qed_hwfn *p_hwfn,
 	u32 mfw_func_offsize = qed_rd(p_hwfn, p_ptt, addr);
 	u32 func_addr = SECTION_ADDR(mfw_func_offsize,
 				     MCP_PF_ID(p_hwfn));
-	struct qed_mcp_mb_params mb_params;
+	struct qed_mcp_mb_params mb_params = {0};
 	int rc;
 	int i;
 
@@ -1176,7 +1166,6 @@ int qed_mcp_ack_vf_flr(struct qed_hwfn *p_hwfn,
 			   "Acking VFs [%08x,...,%08x] - %08x\n",
 			   i * 32, (i + 1) * 32 - 1, vfs_to_ack[i]);
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = DRV_MSG_CODE_VF_DISABLED_DONE;
 	mb_params.p_data_src = vfs_to_ack;
 	mb_params.data_src_size = VF_MAX_STATIC / 8;
@@ -1455,13 +1444,12 @@ static void qed_mcp_handle_link_change(struct qed_hwfn *p_hwfn,
 int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool b_up)
 {
 	struct qed_mcp_link_params *params = &p_hwfn->mcp_info->link_input;
-	struct qed_mcp_mb_params mb_params;
-	struct eth_phy_cfg phy_cfg;
+	struct qed_mcp_mb_params mb_params = {0};
+	struct eth_phy_cfg phy_cfg = {0};
 	int rc = 0;
 	u32 cmd;
 
 	/* Set the shmem configuration according to params */
-	memset(&phy_cfg, 0, sizeof(phy_cfg));
 	cmd = b_up ? DRV_MSG_CODE_INIT_PHY : DRV_MSG_CODE_LINK_RESET;
 	if (!params->speed.autoneg)
 		phy_cfg.speed = params->speed.forced_speed;
@@ -1505,7 +1493,6 @@ int qed_mcp_set_link(struct qed_hwfn *p_hwfn, struct qed_ptt *p_ptt, bool b_up)
 			   "Resetting link\n");
 	}
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = cmd;
 	mb_params.p_data_src = &phy_cfg;
 	mb_params.data_src_size = sizeof(phy_cfg);
@@ -1534,7 +1521,7 @@ static void qed_mcp_send_protocol_stats(struct qed_hwfn *p_hwfn,
 {
 	enum qed_mcp_protocol_type stats_type;
 	union qed_mcp_protocol_stats stats;
-	struct qed_mcp_mb_params mb_params;
+	struct qed_mcp_mb_params mb_params = {0};
 	u32 hsi_param;
 
 	switch (type) {
@@ -1561,7 +1548,6 @@ static void qed_mcp_send_protocol_stats(struct qed_hwfn *p_hwfn,
 
 	qed_get_protocol_stats(p_hwfn->cdev, stats_type, &stats);
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = DRV_MSG_CODE_GET_STATS;
 	mb_params.param = hsi_param;
 	mb_params.p_data_src = &stats;
@@ -2370,20 +2356,18 @@ qed_mcp_send_drv_version(struct qed_hwfn *p_hwfn,
 			 struct qed_ptt *p_ptt,
 			 struct qed_mcp_drv_version *p_ver)
 {
-	struct qed_mcp_mb_params mb_params;
-	struct drv_version_stc drv_version;
+	struct qed_mcp_mb_params mb_params = {0};
+	struct drv_version_stc drv_version = {0};
 	__be32 val;
 	u32 i;
 	int rc;
 
-	memset(&drv_version, 0, sizeof(drv_version));
 	drv_version.version = p_ver->version;
 	for (i = 0; i < (MCP_DRV_VER_STR_SIZE - 4) / sizeof(u32); i++) {
 		val = cpu_to_be32(*((u32 *)&p_ver->name[i * sizeof(u32)]));
 		*(__be32 *)&drv_version.name[i * sizeof(u32)] = val;
 	}
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = DRV_MSG_CODE_SET_VERSION;
 	mb_params.p_data_src = &drv_version;
 	mb_params.data_src_size = sizeof(drv_version);
@@ -2536,11 +2520,10 @@ int qed_mcp_ov_update_mtu(struct qed_hwfn *p_hwfn,
 int qed_mcp_ov_update_mac(struct qed_hwfn *p_hwfn,
 			  struct qed_ptt *p_ptt, u8 *mac)
 {
-	struct qed_mcp_mb_params mb_params;
+	struct qed_mcp_mb_params mb_params = {0};
 	u32 mfw_mac[2];
 	int rc;
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = DRV_MSG_CODE_SET_VMAC;
 	mb_params.param = DRV_MSG_CODE_VMAC_TYPE_MAC <<
 			  DRV_MSG_CODE_VMAC_TYPE_SHIFT;
@@ -3197,12 +3180,10 @@ qed_mcp_resc_allocation_msg(struct qed_hwfn *p_hwfn,
 			    struct qed_resc_alloc_in_params *p_in_params,
 			    struct qed_resc_alloc_out_params *p_out_params)
 {
-	struct qed_mcp_mb_params mb_params;
-	struct resource_info mfw_resc_info;
+	struct qed_mcp_mb_params mb_params = {0};
+	struct resource_info mfw_resc_info = {0};
 	int rc;
 
-	memset(&mfw_resc_info, 0, sizeof(mfw_resc_info));
-
 	mfw_resc_info.res_id = qed_mcp_get_mfw_res_id(p_in_params->res_id);
 	if (mfw_resc_info.res_id == RESOURCE_NUM_INVALID) {
 		DP_ERR(p_hwfn,
@@ -3224,7 +3205,6 @@ qed_mcp_resc_allocation_msg(struct qed_hwfn *p_hwfn,
 		return -EINVAL;
 	}
 
-	memset(&mb_params, 0, sizeof(mb_params));
 	mb_params.cmd = p_in_params->cmd;
 	mb_params.param = QED_RESC_ALLOC_VERSION;
 	mb_params.p_data_src = &mfw_resc_info;
@@ -3277,15 +3257,13 @@ qed_mcp_set_resc_max_val(struct qed_hwfn *p_hwfn,
 			 enum qed_resources res_id,
 			 u32 resc_max_val, u32 *p_mcp_resp)
 {
-	struct qed_resc_alloc_out_params out_params;
-	struct qed_resc_alloc_in_params in_params;
+	struct qed_resc_alloc_out_params out_params = {0};
+	struct qed_resc_alloc_in_params in_params = {0};
 	int rc;
 
-	memset(&in_params, 0, sizeof(in_params));
 	in_params.cmd = DRV_MSG_SET_RESOURCE_VALUE_MSG;
 	in_params.res_id = res_id;
 	in_params.resc_max_val = resc_max_val;
-	memset(&out_params, 0, sizeof(out_params));
 	rc = qed_mcp_resc_allocation_msg(p_hwfn, p_ptt, &in_params,
 					 &out_params);
 	if (rc)
@@ -3302,14 +3280,12 @@ qed_mcp_get_resc_info(struct qed_hwfn *p_hwfn,
 		      enum qed_resources res_id,
 		      u32 *p_mcp_resp, u32 *p_resc_num, u32 *p_resc_start)
 {
-	struct qed_resc_alloc_out_params out_params;
-	struct qed_resc_alloc_in_params in_params;
+	struct qed_resc_alloc_out_params out_params = {0};
+	struct qed_resc_alloc_in_params in_params = {0};
 	int rc;
 
-	memset(&in_params, 0, sizeof(in_params));
 	in_params.cmd = DRV_MSG_GET_RESOURCE_ALLOC_MSG;
 	in_params.res_id = res_id;
-	memset(&out_params, 0, sizeof(out_params));
 	rc = qed_mcp_resc_allocation_msg(p_hwfn, p_ptt, &in_params,
 					 &out_params);
 	if (rc)
-- 
2.20.1


^ permalink raw reply related

* [PATCH 8/8] net: ethernet: ixp4xx_eth: remove memset(,0) with zalloc
From: Robert Eshleman @ 2019-02-27  6:09 UTC (permalink / raw)
  To: bobbyeshleman
  Cc: Tariq Toukan, David S. Miller, Krzysztof Halasa, netdev,
	linux-rdma, linux-kernel
In-Reply-To: <cover.1551246708.git.bobbyeshleman@gmail.com>

This patch removes a call to memset(,0) by replacing the
prior call to dma_pool_alloc with a call to dma_pool_zalloc.

Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
 drivers/net/ethernet/xscale/ixp4xx_eth.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/xscale/ixp4xx_eth.c b/drivers/net/ethernet/xscale/ixp4xx_eth.c
index aee55c03def0..8471e1857d53 100644
--- a/drivers/net/ethernet/xscale/ixp4xx_eth.c
+++ b/drivers/net/ethernet/xscale/ixp4xx_eth.c
@@ -1107,11 +1107,10 @@ static int init_queues(struct port *port)
 		if (!dma_pool)
 			return -ENOMEM;
 	}
-
-	if (!(port->desc_tab = dma_pool_alloc(dma_pool, GFP_KERNEL,
-					      &port->desc_tab_phys)))
+	port->desc_tab = dma_pool_zalloc(dma_pool, GFP_KERNEL,
+					 &port->desc_tab_phys);
+	if (!port->desc_tab)
 		return -ENOMEM;
-	memset(port->desc_tab, 0, POOL_ALLOC_SIZE);
 	memset(port->rx_buff_tab, 0, sizeof(port->rx_buff_tab)); /* tables */
 	memset(port->tx_buff_tab, 0, sizeof(port->tx_buff_tab));
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH 1/8] net/mlx4: use kzalloc instead of kmalloc
From: Robert Eshleman @ 2019-02-27  6:09 UTC (permalink / raw)
  To: bobbyeshleman
  Cc: Tariq Toukan, David S. Miller, netdev, linux-rdma, linux-kernel
In-Reply-To: <cover.1551246708.git.bobbyeshleman@gmail.com>

This patch replaces a kmalloc/memset(,0) call with a call to kzalloc.
It also removes a memset(,0) call that always follows a *zalloc call.

Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx4/cmd.c   | 1 -
 drivers/net/ethernet/mellanox/mlx4/en_rx.c | 3 +--
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/cmd.c b/drivers/net/ethernet/mellanox/mlx4/cmd.c
index e65bc3c95630..7bfa6e850e5f 100644
--- a/drivers/net/ethernet/mellanox/mlx4/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx4/cmd.c
@@ -3307,7 +3307,6 @@ int mlx4_get_counter_stats(struct mlx4_dev *dev, int counter_index,
 	if (IS_ERR(mailbox))
 		return PTR_ERR(mailbox);
 
-	memset(mailbox->buf, 0, sizeof(struct mlx4_counter));
 	if_stat_in_mod = counter_index;
 	if (reset)
 		if_stat_in_mod |= MLX4_QUERY_IF_STAT_RESET;
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index 9a0881cb7f51..f55805d206ef 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -1044,7 +1044,7 @@ static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv, int qpn,
 	struct mlx4_qp_context *context;
 	int err = 0;
 
-	context = kmalloc(sizeof(*context), GFP_KERNEL);
+	context = kzalloc(sizeof(*context), GFP_KERNEL);
 	if (!context)
 		return -ENOMEM;
 
@@ -1055,7 +1055,6 @@ static int mlx4_en_config_rss_qp(struct mlx4_en_priv *priv, int qpn,
 	}
 	qp->event = mlx4_en_sqp_event;
 
-	memset(context, 0, sizeof(*context));
 	mlx4_en_fill_qp_context(priv, ring->actual_size, ring->stride, 0, 0,
 				qpn, ring->cqn, -1, context);
 	context->db_rec_addr = cpu_to_be64(ring->wqres.db.dma);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 7/8] net: seeq: replace kmalloc / memset(,0) with kzalloc
From: Robert Eshleman @ 2019-02-27  6:09 UTC (permalink / raw)
  To: bobbyeshleman
  Cc: Tariq Toukan, David S. Miller, Russell King, netdev, linux-rdma,
	linux-kernel, linux-arm-kernel
In-Reply-To: <cover.1551246708.git.bobbyeshleman@gmail.com>

This patch reduces a call to memset(,0) by replacing
a kmalloc call with a kzalloc call.

Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
 drivers/net/ethernet/seeq/ether3.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/seeq/ether3.c b/drivers/net/ethernet/seeq/ether3.c
index d1bb73bf9914..7456cf08a48f 100644
--- a/drivers/net/ethernet/seeq/ether3.c
+++ b/drivers/net/ethernet/seeq/ether3.c
@@ -223,7 +223,7 @@ ether3_addr(char *addr, struct expansion_card *ec)
 static int
 ether3_ramtest(struct net_device *dev, unsigned char byte)
 {
-	unsigned char *buffer = kmalloc(RX_END, GFP_KERNEL);
+	unsigned char *buffer = kzalloc(RX_END, GFP_KERNEL);
 	int i,ret = 0;
 	int max_errors = 4;
 	int bad = -1;
@@ -231,7 +231,6 @@ ether3_ramtest(struct net_device *dev, unsigned char byte)
 	if (!buffer)
 		return 1;
 
-	memset(buffer, byte, RX_END);
 	ether3_setbuffer(dev, buffer_write, 0);
 	ether3_writebuffer(dev, buffer, TX_END);
 	ether3_setbuffer(dev, buffer_write, RX_START);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 6/8] netxen: remove unnecessary memset(,0) calls
From: Robert Eshleman @ 2019-02-27  6:09 UTC (permalink / raw)
  To: bobbyeshleman
  Cc: Tariq Toukan, David S. Miller, Manish Chopra, Rahul Verma,
	GR-Linux-NIC-Dev, netdev, linux-rdma, linux-kernel
In-Reply-To: <cover.1551246708.git.bobbyeshleman@gmail.com>

This patch reduces calls to memset(,0) by replacing memset(,0)
calls that only zero-out newly declared structs with simply
assigning those structs to zero structs.

It also removes a pci_alloc_consistent call followed by a memset(,0)
call by simply using pci_zalloc_consistent.

Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
 .../ethernet/qlogic/netxen/netxen_nic_ctx.c   | 36 +++++++------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c
index 7503aa222392..f2010c032361 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_ctx.c
@@ -99,8 +99,7 @@ netxen_issue_cmd(struct netxen_adapter *adapter, struct netxen_cmd_args *cmd)
 static int
 netxen_get_minidump_template_size(struct netxen_adapter *adapter)
 {
-	struct netxen_cmd_args cmd;
-	memset(&cmd, 0, sizeof(cmd));
+	struct netxen_cmd_args cmd = {0};
 	cmd.req.cmd = NX_CDRP_CMD_TEMP_SIZE;
 	memset(&cmd.rsp, 1, sizeof(struct _cdrp_cmd));
 	netxen_issue_cmd(adapter, &cmd);
@@ -120,7 +119,7 @@ netxen_get_minidump_template(struct netxen_adapter *adapter)
 	dma_addr_t md_template_addr;
 	void *addr;
 	u32 size;
-	struct netxen_cmd_args cmd;
+	struct netxen_cmd_args cmd = {0};
 	size = adapter->mdump.md_template_size;
 
 	if (size == 0) {
@@ -135,7 +134,6 @@ netxen_get_minidump_template(struct netxen_adapter *adapter)
 		return -ENOMEM;
 	}
 
-	memset(&cmd, 0, sizeof(cmd));
 	memset(&cmd.rsp, 1, sizeof(struct _cdrp_cmd));
 	cmd.req.cmd = NX_CDRP_CMD_GET_TEMP_HDR;
 	cmd.req.arg1 = LSD(md_template_addr);
@@ -233,9 +231,8 @@ nx_fw_cmd_set_mtu(struct netxen_adapter *adapter, int mtu)
 {
 	u32 rcode = NX_RCODE_SUCCESS;
 	struct netxen_recv_context *recv_ctx = &adapter->recv_ctx;
-	struct netxen_cmd_args cmd;
+	struct netxen_cmd_args cmd = {0};
 
-	memset(&cmd, 0, sizeof(cmd));
 	cmd.req.cmd = NX_CDRP_CMD_SET_MTU;
 	cmd.req.arg1 = recv_ctx->context_id;
 	cmd.req.arg2 = mtu;
@@ -254,9 +251,8 @@ int
 nx_fw_cmd_set_gbe_port(struct netxen_adapter *adapter,
 			u32 speed, u32 duplex, u32 autoneg)
 {
-	struct netxen_cmd_args cmd;
+	struct netxen_cmd_args cmd = {0};
 
-	memset(&cmd, 0, sizeof(cmd));
 	cmd.req.cmd = NX_CDRP_CMD_CONFIG_GBE_PORT;
 	cmd.req.arg1 = speed;
 	cmd.req.arg2 = duplex;
@@ -276,7 +272,7 @@ nx_fw_cmd_create_rx_ctx(struct netxen_adapter *adapter)
 	nx_cardrsp_sds_ring_t *prsp_sds;
 	struct nx_host_rds_ring *rds_ring;
 	struct nx_host_sds_ring *sds_ring;
-	struct netxen_cmd_args cmd;
+	struct netxen_cmd_args cmd = {0};
 
 	dma_addr_t hostrq_phys_addr, cardrsp_phys_addr;
 	u64 phys_addr;
@@ -359,7 +355,6 @@ nx_fw_cmd_create_rx_ctx(struct netxen_adapter *adapter)
 	}
 
 	phys_addr = hostrq_phys_addr;
-	memset(&cmd, 0, sizeof(cmd));
 	cmd.req.arg1 = (u32)(phys_addr >> 32);
 	cmd.req.arg2 = (u32)(phys_addr & 0xffffffff);
 	cmd.req.arg3 = rq_size;
@@ -413,9 +408,8 @@ static void
 nx_fw_cmd_destroy_rx_ctx(struct netxen_adapter *adapter)
 {
 	struct netxen_recv_context *recv_ctx = &adapter->recv_ctx;
-	struct netxen_cmd_args cmd;
+	struct netxen_cmd_args cmd = {0};
 
-	memset(&cmd, 0, sizeof(cmd));
 	cmd.req.arg1 = recv_ctx->context_id;
 	cmd.req.arg2 = NX_DESTROY_CTX_RESET;
 	cmd.req.arg3 = 0;
@@ -520,9 +514,8 @@ nx_fw_cmd_create_tx_ctx(struct netxen_adapter *adapter)
 static void
 nx_fw_cmd_destroy_tx_ctx(struct netxen_adapter *adapter)
 {
-	struct netxen_cmd_args cmd;
+	struct netxen_cmd_args cmd = {0};
 
-	memset(&cmd, 0, sizeof(cmd));
 	cmd.req.arg1 = adapter->tx_context_id;
 	cmd.req.arg2 = NX_DESTROY_CTX_RESET;
 	cmd.req.arg3 = 0;
@@ -538,9 +531,8 @@ int
 nx_fw_cmd_query_phy(struct netxen_adapter *adapter, u32 reg, u32 *val)
 {
 	u32 rcode;
-	struct netxen_cmd_args cmd;
+	struct netxen_cmd_args cmd = {0};
 
-	memset(&cmd, 0, sizeof(cmd));
 	cmd.req.arg1 = reg;
 	cmd.req.arg2 = 0;
 	cmd.req.arg3 = 0;
@@ -561,9 +553,8 @@ int
 nx_fw_cmd_set_phy(struct netxen_adapter *adapter, u32 reg, u32 val)
 {
 	u32 rcode;
-	struct netxen_cmd_args cmd;
+	struct netxen_cmd_args cmd = {0};
 
-	memset(&cmd, 0, sizeof(cmd));
 	cmd.req.arg1 = reg;
 	cmd.req.arg2 = val;
 	cmd.req.arg3 = 0;
@@ -763,15 +754,14 @@ int netxen_alloc_hw_resources(struct netxen_adapter *adapter)
 	recv_ctx = &adapter->recv_ctx;
 	tx_ring = adapter->tx_ring;
 
-	addr = pci_alloc_consistent(pdev,
-			sizeof(struct netxen_ring_ctx) + sizeof(uint32_t),
-			&recv_ctx->phys_addr);
-	if (addr == NULL) {
+	addr = pci_zalloc_consistent(pdev,
+				     sizeof(struct netxen_ring_ctx) + sizeof(uint32_t),
+				     &recv_ctx->phys_addr);
+	if (!addr) {
 		dev_err(&pdev->dev, "failed to allocate hw context\n");
 		return -ENOMEM;
 	}
 
-	memset(addr, 0, sizeof(struct netxen_ring_ctx));
 	recv_ctx->hwctx = addr;
 	recv_ctx->hwctx->ctx_id = cpu_to_le32(port);
 	recv_ctx->hwctx->cmd_consumer_offset =
-- 
2.20.1


^ permalink raw reply related

* [PATCH 5/8] at12: use pci_zalloc instead of pci_alloc
From: Robert Eshleman @ 2019-02-27  6:09 UTC (permalink / raw)
  To: bobbyeshleman
  Cc: Jay Cliburn, Chris Snook, David S. Miller, Tariq Toukan, netdev,
	linux-kernel, linux-rdma
In-Reply-To: <cover.1551246708.git.bobbyeshleman@gmail.com>

This patch replaces a pci_alloc and memset(,0) call
with a single call to pci_zalloc.

Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
 drivers/net/ethernet/atheros/atlx/atl2.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/atheros/atlx/atl2.c b/drivers/net/ethernet/atheros/atlx/atl2.c
index bb41becb6609..a145c2d1b1d2 100644
--- a/drivers/net/ethernet/atheros/atlx/atl2.c
+++ b/drivers/net/ethernet/atheros/atlx/atl2.c
@@ -300,11 +300,10 @@ static s32 atl2_setup_ring_resources(struct atl2_adapter *adapter)
 		adapter->txs_ring_size * 4 + 7 +	/* dword align */
 		adapter->rxd_ring_size * 1536 + 127;	/* 128bytes align */
 
-	adapter->ring_vir_addr = pci_alloc_consistent(pdev, size,
-		&adapter->ring_dma);
+	adapter->ring_vir_addr = pci_zalloc_consistent(pdev, size,
+						       &adapter->ring_dma);
 	if (!adapter->ring_vir_addr)
 		return -ENOMEM;
-	memset(adapter->ring_vir_addr, 0, adapter->ring_size);
 
 	/* Init TXD Ring */
 	adapter->txd_dma = adapter->ring_dma ;
-- 
2.20.1


^ permalink raw reply related

* [PATCH 3/8] tlan: use pci_zalloc instead of pci_alloc
From: Robert Eshleman @ 2019-02-27  6:09 UTC (permalink / raw)
  To: bobbyeshleman
  Cc: Tariq Toukan, David S. Miller, Samuel Chessman, netdev,
	linux-rdma, linux-kernel
In-Reply-To: <cover.1551246708.git.bobbyeshleman@gmail.com>

This patch replaces a pci_alloc_consistent and memset(,0) call
with a single call to pci_zalloc_consistent.

Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
 drivers/net/ethernet/ti/tlan.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/ti/tlan.c b/drivers/net/ethernet/ti/tlan.c
index b4ab1a5f6cd0..e1f7e71c3b21 100644
--- a/drivers/net/ethernet/ti/tlan.c
+++ b/drivers/net/ethernet/ti/tlan.c
@@ -845,17 +845,16 @@ static int tlan_init(struct net_device *dev)
 
 	dma_size = (TLAN_NUM_RX_LISTS + TLAN_NUM_TX_LISTS)
 		* (sizeof(struct tlan_list));
-	priv->dma_storage = pci_alloc_consistent(priv->pci_dev,
-						 dma_size,
-						 &priv->dma_storage_dma);
+	priv->dma_storage = pci_zalloc_consistent(priv->pci_dev,
+						  dma_size,
+						  &priv->dma_storage_dma);
 	priv->dma_size = dma_size;
 
-	if (priv->dma_storage == NULL) {
+	if (!priv->dma_storage) {
 		pr_err("Could not allocate lists and buffers for %s\n",
 		       dev->name);
 		return -ENOMEM;
 	}
-	memset(priv->dma_storage, 0, dma_size);
 	priv->rx_list = (struct tlan_list *)
 		ALIGN((unsigned long)priv->dma_storage, 8);
 	priv->rx_list_dma = ALIGN(priv->dma_storage_dma, 8);
-- 
2.20.1


^ permalink raw reply related

* [PATCH 2/8] net/mlxsw: use pci_zalloc_consistent instead of pci_alloc_consistent
From: Robert Eshleman @ 2019-02-27  6:09 UTC (permalink / raw)
  To: bobbyeshleman
  Cc: Tariq Toukan, David S. Miller, Jiri Pirko, Ido Schimmel, netdev,
	linux-rdma, linux-kernel
In-Reply-To: <cover.1551246708.git.bobbyeshleman@gmail.com>

This patch replaces a call to pci_alloc_consistent and then
memset(0,) with a single call to pci_zalloc_consistent.

Signed-off-by: Robert Eshleman <bobbyeshleman@gmail.com>
---
 drivers/net/ethernet/mellanox/mlxsw/pci.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlxsw/pci.c b/drivers/net/ethernet/mellanox/mlxsw/pci.c
index a2321fe8d6a0..388f349573f3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/pci.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/pci.c
@@ -830,12 +830,11 @@ static int mlxsw_pci_queue_init(struct mlxsw_pci *mlxsw_pci, char *mbox,
 		tasklet_init(&q->tasklet, q_ops->tasklet, (unsigned long) q);
 
 	mem_item->size = MLXSW_PCI_AQ_SIZE;
-	mem_item->buf = pci_alloc_consistent(mlxsw_pci->pdev,
-					     mem_item->size,
-					     &mem_item->mapaddr);
+	mem_item->buf = pci_zalloc_consistent(mlxsw_pci->pdev,
+					      mem_item->size,
+					      &mem_item->mapaddr);
 	if (!mem_item->buf)
 		return -ENOMEM;
-	memset(mem_item->buf, 0, mem_item->size);
 
 	q->elem_info = kcalloc(q->count, sizeof(*q->elem_info), GFP_KERNEL);
 	if (!q->elem_info) {
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH 3/8] tlan: use pci_zalloc instead of pci_alloc
From: Joe Perches @ 2019-02-27  6:22 UTC (permalink / raw)
  To: Robert Eshleman
  Cc: Tariq Toukan, David S. Miller, Samuel Chessman, netdev,
	linux-rdma, linux-kernel
In-Reply-To: <51125a3daac49f7b5bb360a422663aee26e6776b.1551246708.git.bobbyeshleman@gmail.com>

On Tue, 2019-02-26 at 22:09 -0800, Robert Eshleman wrote:
> This patch replaces a pci_alloc_consistent and memset(,0) call
> with a single call to pci_zalloc_consistent.
[]
> diff --git a/drivers/net/ethernet/ti/tlan.c b/drivers/net/ethernet/ti/tlan.c
[]
> @@ -845,17 +845,16 @@ static int tlan_init(struct net_device *dev)
>  
>  	dma_size = (TLAN_NUM_RX_LISTS + TLAN_NUM_TX_LISTS)
>  		* (sizeof(struct tlan_list));
> -	priv->dma_storage = pci_alloc_consistent(priv->pci_dev,
> -						 dma_size,
> -						 &priv->dma_storage_dma);
> +	priv->dma_storage = pci_zalloc_consistent(priv->pci_dev,
> +						  dma_size,
> +						  &priv->dma_storage_dma);
>  	priv->dma_size = dma_size;
>  
> -	if (priv->dma_storage == NULL) {
> +	if (!priv->dma_storage) {
>  		pr_err("Could not allocate lists and buffers for %s\n",
>  		       dev->name);

unrelated trivia:

This pr_err (and likely others in this file)
could be replace by netdev_err



^ permalink raw reply

* Re: [PATCH bpf-next 1/3] bpf: add bpf_progenyof helper
From: Martin Lau @ 2019-02-27  6:26 UTC (permalink / raw)
  To: Javier Honduvilla Coto; +Cc: netdev@vger.kernel.org, Yonghong Song, Kernel Team
In-Reply-To: <20190226223651.3166820-2-javierhonduco@fb.com>

On Tue, Feb 26, 2019 at 02:36:49PM -0800, Javier Honduvilla Coto wrote:
> This patch adds the bpf_progenyof helper which receives a PID and returns
What is progenof?

> 1 if the process currently being executed is in the process hierarchy
> including itself or 0 if not.
> 
> This is very useful in tracing programs when we want to filter by a
> given PID and all the children it might spawn. The current workarounds
> most people implement for this purpose have issues:
> 
> - Attaching to process spawning syscalls and dynamically add those PIDs
>   to some bpf map that would be used to filter is cumbersome and
> potentially racy.
> - Unrolling some loop to perform what this helper is doing consumes lots
>   of instructions. That and the impossibility to jump backwards makes it
> really hard to be correct in really large process chains.
> 
> Signed-off-by: Javier Honduvilla Coto <javierhonduco@fb.com>
> ---
>  include/linux/bpf.h      |  1 +
>  include/uapi/linux/bpf.h |  3 ++-
>  kernel/bpf/core.c        |  1 +
>  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
>  kernel/trace/bpf_trace.c |  2 ++
>  5 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index de18227b3d95..447395ba202b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
>  extern const struct bpf_func_proto bpf_spin_lock_proto;
>  extern const struct bpf_func_proto bpf_spin_unlock_proto;
>  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> +extern const struct bpf_func_proto bpf_progenyof_proto;
It seems only used in bpf_trace.c.  Does it have to be here?

>  
>  /* Shared helpers among cBPF and eBPF. */
>  void bpf_user_rnd_init_once(void);
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index bcdd2474eee7..804e4218eb28 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2457,7 +2457,8 @@ union bpf_attr {
>  	FN(spin_lock),			\
>  	FN(spin_unlock),		\
>  	FN(sk_fullsock),		\
> -	FN(tcp_sock),
> +	FN(tcp_sock),			\
> +	FN(progenyof),
Please add doc like other helpers do.

>  
>  /* integer value in 'imm' field of BPF_CALL instruction selects which helper
>   * function eBPF program intends to call
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index ef88b167959d..69e209fbd128 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -2015,6 +2015,7 @@ const struct bpf_func_proto bpf_get_current_uid_gid_proto __weak;
>  const struct bpf_func_proto bpf_get_current_comm_proto __weak;
>  const struct bpf_func_proto bpf_get_current_cgroup_id_proto __weak;
>  const struct bpf_func_proto bpf_get_local_storage_proto __weak;
> +const struct bpf_func_proto bpf_progenyof_proto __weak;
>  
>  const struct bpf_func_proto * __weak bpf_get_trace_printk_proto(void)
>  {
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index a411fc17d265..3899787e8dbf 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -18,6 +18,7 @@
>  #include <linux/sched.h>
>  #include <linux/uidgid.h>
>  #include <linux/filter.h>
> +#include <linux/init_task.h>
>  
>  /* If kernel subsystem is allowing eBPF programs to call this function,
>   * inside its own verifier_ops->get_func_proto() callback it should return
> @@ -364,3 +365,31 @@ const struct bpf_func_proto bpf_get_local_storage_proto = {
>  };
>  #endif
>  #endif
> +
> +BPF_CALL_1(bpf_progenyof, int, pid)
> +{
> +	int result = 0;
> +	struct task_struct *task = current;
> +
> +	if (unlikely(!task))
hmm.... Could current be NULL?

> +		return -EINVAL;
> +
> +	rcu_read_lock();
> +	while (task != &init_task) {
I don't know the details of init_task, so qq:
Could the passed in "pid" be the init_task->pid?
If possible, what is the expected "result"?

> +		if (task->pid == pid) {
> +			result = 1;
> +			break;
> +		}
> +		task = rcu_dereference(task->real_parent);
> +	}
> +	rcu_read_unlock();
> +
> +	return result;
> +}
> +
> +const struct bpf_func_proto bpf_progenyof_proto = {
> +	.func		= bpf_progenyof,
> +	.gpl_only	= false,
> +	.ret_type	= RET_INTEGER,
> +	.arg1_type	= ARG_ANYTHING,
> +};
> diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> index f1a86a0d881d..8602ae83c799 100644
> --- a/kernel/trace/bpf_trace.c
> +++ b/kernel/trace/bpf_trace.c
> @@ -600,6 +600,8 @@ tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>  		return &bpf_get_prandom_u32_proto;
>  	case BPF_FUNC_probe_read_str:
>  		return &bpf_probe_read_str_proto;
> +	case BPF_FUNC_progenyof:
> +		return &bpf_progenyof_proto;
>  #ifdef CONFIG_CGROUPS
>  	case BPF_FUNC_get_current_cgroup_id:
>  		return &bpf_get_current_cgroup_id_proto;
> -- 
> 2.17.1
> 

^ permalink raw reply

* Re: [PATCH] samples: bpf: fix: broken sample regarding removed function
From: Martin Lau @ 2019-02-27  6:40 UTC (permalink / raw)
  To: Daniel T. Lee; +Cc: Daniel Borkmann, Alexei Starovoitov, netdev@vger.kernel.org
In-Reply-To: <20190227030559.19672-1-danieltimlee@gmail.com>

On Tue, Feb 26, 2019 at 10:05:59PM -0500, Daniel T. Lee wrote:
> Currently, running sample "task_fd_query" and "tracex3" occurs the
> following error. On kernel v5.0-rc* this sample will be unavailable
> due to the removal of function 'blk_start_request' at commit "a1ce35f".
> (function removed, as "Single Queue IO scheduler" no longer exists)
> 
> $ sudo ./task_fd_query
> failed to create kprobe 'blk_start_request' error 'No such file or
> directory'
> 
> This commit will change the function 'blk_start_request' to
> 'blk_mq_start_request' to fix the broken sample.
> 
> Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com>
> ---
>  samples/bpf/task_fd_query_kern.c | 2 +-
>  samples/bpf/task_fd_query_user.c | 2 +-
>  samples/bpf/tracex3_kern.c       | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/samples/bpf/task_fd_query_kern.c b/samples/bpf/task_fd_query_kern.c
> index f4b0a9ea674d..5f1b2cdababd 100644
> --- a/samples/bpf/task_fd_query_kern.c
> +++ b/samples/bpf/task_fd_query_kern.c
> @@ -4,7 +4,7 @@
>  #include <uapi/linux/bpf.h>
>  #include "bpf_helpers.h"
>  
> -SEC("kprobe/blk_start_request")
> +SEC("kprobe/blk_mq_start_requset")
I suspect it does not run.  Have you tested it?

There is a typo:
blk_mq_start_requSEt
blk_mq_start_requESt

^ permalink raw reply

* [PATCH iproute2-next] rdma: Add the prefix for driver attributes
From: Leon Romanovsky @ 2019-02-27  6:41 UTC (permalink / raw)
  To: David Ahern, Lijun Ou, Steve Wise
  Cc: Leon Romanovsky, netdev, RDMA mailing list, Stephen Hemminger

From: Leon Romanovsky <leonro@mellanox.com>

There is a need to distinguish between driver vs. general exposed
attributes. The most common use case is to expose some internal
garbage under extremely common and sexy name, e.g. pi, ci e.t.c

In order to achieve that, we will add "drv_" prefix to all strings
which were received through RDMA_NLDEV_ATTR_DRIVER_* attributes.

Signed-off-by: Leon Romanovsky <leonro@mellanox.com>a
---
 rdma/utils.c | 34 ++++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 12 deletions(-)

diff --git a/rdma/utils.c b/rdma/utils.c
index 6bc14cd5..1f6bf330 100644
--- a/rdma/utils.c
+++ b/rdma/utils.c
@@ -829,27 +829,37 @@ static int print_driver_entry(struct rd *rd, struct nlattr *key_attr,
 				struct nlattr *val_attr,
 				enum rdma_nldev_print_type print_type)
 {
-	const char *key_str = mnl_attr_get_str(key_attr);
 	int attr_type = nla_type(val_attr);
+	int ret = -EINVAL;
+	char *key_str;
+
+	if (asprintf(&key_str, "drv_%s", mnl_attr_get_str(key_attr)) == -1)
+		return -ENOMEM;

 	switch (attr_type) {
 	case RDMA_NLDEV_ATTR_DRIVER_STRING:
-		return print_driver_string(rd, key_str,
-				mnl_attr_get_str(val_attr));
+		ret = print_driver_string(rd, key_str,
+					  mnl_attr_get_str(val_attr));
+		break;
 	case RDMA_NLDEV_ATTR_DRIVER_S32:
-		return print_driver_s32(rd, key_str,
-				mnl_attr_get_u32(val_attr), print_type);
+		ret = print_driver_s32(rd, key_str, mnl_attr_get_u32(val_attr),
+				       print_type);
+		break;
 	case RDMA_NLDEV_ATTR_DRIVER_U32:
-		return print_driver_u32(rd, key_str,
-				mnl_attr_get_u32(val_attr), print_type);
+		ret = print_driver_u32(rd, key_str, mnl_attr_get_u32(val_attr),
+				       print_type);
+		break;
 	case RDMA_NLDEV_ATTR_DRIVER_S64:
-		return print_driver_s64(rd, key_str,
-				mnl_attr_get_u64(val_attr), print_type);
+		ret = print_driver_s64(rd, key_str, mnl_attr_get_u64(val_attr),
+				       print_type);
+		break;
 	case RDMA_NLDEV_ATTR_DRIVER_U64:
-		return print_driver_u64(rd, key_str,
-				mnl_attr_get_u64(val_attr), print_type);
+		ret = print_driver_u64(rd, key_str, mnl_attr_get_u64(val_attr),
+				       print_type);
+		break;
 	}
-	return -EINVAL;
+	free(key_str);
+	return ret;
 }

 void print_driver_table(struct rd *rd, struct nlattr *tb)
--
2.19.1


^ permalink raw reply related

* Re: [PATCH net-next 3/3] net: dsa: microchip: add other KSZ9477 switch variants
From: kbuild test robot @ 2019-02-27  6:47 UTC (permalink / raw)
  To: Tristram.Ha
  Cc: kbuild-all, Andrew Lunn, Florian Fainelli, Pavel Machek,
	Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1551224265-9304-4-git-send-email-Tristram.Ha@microchip.com>

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

Hi Tristram,

Thank you for the patch! Yet something to improve:

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

url:    https://github.com/0day-ci/linux/commits/Tristram-Ha-microchip-com/net-dsa-microchip-add-KSZ9893-switch-support/20190227-141333
config: i386-randconfig-x009-201908 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/net/dsa/microchip/ksz9477.c: In function 'ksz9477_switch_detect':
>> drivers/net/dsa/microchip/ksz9477.c:1516:8: error: implicit declaration of function 'of_modalias_node'; did you mean 'of_match_node'? [-Werror=implicit-function-declaration]
      if (!of_modalias_node(dev->dev->of_node, name, sizeof(name))) {
           ^~~~~~~~~~~~~~~~
           of_match_node
   cc1: some warnings being treated as errors

vim +1516 drivers/net/dsa/microchip/ksz9477.c

  1433	
  1434	static int ksz9477_switch_detect(struct ksz_device *dev)
  1435	{
  1436		int chip = -1;
  1437		u8 data8;
  1438		u8 id_hi;
  1439		u8 id_lo;
  1440		u32 id32;
  1441		int ret;
  1442	
  1443		/* turn off SPI DO Edge select */
  1444		ret = ksz_read8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, &data8);
  1445		if (ret)
  1446			return ret;
  1447	
  1448		data8 &= ~SPI_AUTO_EDGE_DETECTION;
  1449		ret = ksz_write8(dev, REG_SW_GLOBAL_SERIAL_CTRL_0, data8);
  1450		if (ret)
  1451			return ret;
  1452	
  1453		/* read chip id */
  1454		ret = ksz_read32(dev, REG_CHIP_ID0__1, &id32);
  1455		if (ret)
  1456			return ret;
  1457		ret = ksz_read8(dev, REG_GLOBAL_OPTIONS, &data8);
  1458		if (ret)
  1459			return ret;
  1460	
  1461		/* Number of ports can be reduced depending on chip. */
  1462		dev->mib_port_cnt = TOTAL_PORT_NUM;
  1463		dev->phy_port_cnt = 5;
  1464	
  1465		/* Default capability is gigabit capable. */
  1466		dev->features = GBIT_SUPPORT;
  1467	
  1468		id_hi = (u8)(id32 >> 16);
  1469		id_lo = (u8)(id32 >> 8);
  1470		if ((id_lo & 0xf) == 3) {
  1471			/* Chip is from KSZ9893 design. */
  1472			dev->features |= IS_9893;
  1473	
  1474			/* Chip does not support gigabit. */
  1475			if (data8 & SW_QW_ABLE)
  1476				dev->features &= ~GBIT_SUPPORT;
  1477			dev->mib_port_cnt = 3;
  1478			dev->phy_port_cnt = 2;
  1479			if (!(data8 & SW_AVB_ABLE))
  1480				chip = KSZ9893_SW_CHIP;
  1481			else if (data8 & SW_QW_ABLE)
  1482				chip = KSZ8563_SW_CHIP;
  1483			else
  1484				chip = KSZ9563_SW_CHIP;
  1485		} else {
  1486			/* Chip uses new XMII register definitions. */
  1487			dev->features |= NEW_XMII;
  1488	
  1489			/* Chip does not support gigabit. */
  1490			if (!(data8 & SW_GIGABIT_ABLE))
  1491				dev->features &= ~GBIT_SUPPORT;
  1492			if ((id_lo & 0xf) == 6)
  1493				dev->mib_port_cnt = 6;
  1494			if (id_hi == FAMILY_ID_94)
  1495				chip = KSZ9477_SW_CHIP;
  1496			else if (id_hi == FAMILY_ID_98 && id_lo == CHIP_ID_97)
  1497				chip = KSZ9897_SW_CHIP;
  1498			else if (id_hi == FAMILY_ID_98 && id_lo == CHIP_ID_96)
  1499				chip = KSZ9896_SW_CHIP;
  1500			else if (id_hi == FAMILY_ID_95 && id_lo == CHIP_ID_67)
  1501				chip = KSZ9567_SW_CHIP;
  1502			else if (id_hi == FAMILY_ID_85 && id_lo == CHIP_ID_67)
  1503				chip = KSZ8567_SW_CHIP;
  1504			if (id_lo == CHIP_ID_67) {
  1505				id_hi = FAMILY_ID_98;
  1506				id_lo = CHIP_ID_97;
  1507			} else if (id_lo == CHIP_ID_66) {
  1508				id_hi = FAMILY_ID_98;
  1509				id_lo = CHIP_ID_96;
  1510			}
  1511		}
  1512		if (dev->dev->of_node) {
  1513			char name[80];
  1514	
  1515			/* KSZ8565 chip can only be set through device tree. */
> 1516			if (!of_modalias_node(dev->dev->of_node, name, sizeof(name))) {
  1517				if (!strcmp(name, "ksz8565")) {
  1518					chip = KSZ8565_SW_CHIP;
  1519					id_hi = FAMILY_ID_98;
  1520					id_lo = 0x95;
  1521				}
  1522			}
  1523		}
  1524	
  1525		/* Change chip id to known ones so it can be matched against them. */
  1526		id32 = (id_hi << 16) | (id_lo << 8);
  1527	
  1528		dev->chip_id = id32;
  1529	
  1530		/* Update switch device name to matched chip. */
  1531		if (chip >= 0)
  1532			dev->name = ksz9477_chip_names[chip];
  1533	
  1534		return 0;
  1535	}
  1536	

---
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: 31591 bytes --]

^ permalink raw reply

* Re: [RFC v1 13/19] RDMA/irdma: Add RoCEv2 UD OP support
From: Leon Romanovsky @ 2019-02-27  6:50 UTC (permalink / raw)
  To: Shiraz Saleem
  Cc: dledford, jgg, davem, linux-rdma, netdev, mustafa.ismail,
	jeffrey.t.kirsher
In-Reply-To: <20190215171107.6464-14-shiraz.saleem@intel.com>

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

On Fri, Feb 15, 2019 at 11:11:00AM -0600, Shiraz Saleem wrote:
> From: Mustafa Ismail <mustafa.ismail@intel.com>
>
> Add the header, data structures and functions
> to populate the WQE descriptors and issue the
> Control QP commands that support RoCEv2 UD operations.
>
> Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> ---

<...>

> +#define IRDMA_UDA_QPSQ_IMMDATA_M \
> +	((u64)0xffffffffffffffffULL << IRDMA_UDA_QPSQ_IMMDATA_S)

Both u64 and ULL?

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [PATCH internal pre-review] bpf: add missing entries to bpf_helpers.h
From: Martin Lau @ 2019-02-27  6:52 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: netdev@vger.kernel.org, sdf@google.com, soheil@google.com,
	Willem de Bruijn
In-Reply-To: <20190226225553.214360-1-willemdebruijn.kernel@gmail.com>

On Tue, Feb 26, 2019 at 05:55:53PM -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>  tools/testing/selftests/bpf/bpf_helpers.h | 29 +++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index d9999f1ed1d2a..fcd11839903c7 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -232,6 +232,35 @@ static int (*bpf_skb_change_head)(void *, int len, int flags) =
>  	(void *) BPF_FUNC_skb_change_head;
>  static int (*bpf_skb_pull_data)(void *, int len) =
>  	(void *) BPF_FUNC_skb_pull_data;
> +static unsigned long long (*bpf_get_cgroup_classid)(void *ctx) =
> +	(void *) BPF_FUNC_get_cgroup_classid;
> +static unsigned long long (*bpf_get_route_realm)(void *ctx) =
> +	(void *) BPF_FUNC_get_route_realm;
> +static int (*bpf_skb_change_proto)(void *ctx, __be16 proto, __u64 flags) =
> +	(void *) BPF_FUNC_skb_change_proto;
> +static int (*bpf_skb_change_type)(void *ctx, __u32 type) =
> +	(void *) BPF_FUNC_skb_change_type;
> +static unsigned long long (*bpf_get_hash_recalc)(void *ctx) =
> +	(void *) BPF_FUNC_get_hash_recalc;
> +static unsigned long long (*bpf_get_current_task)(void *ctx) =
> +	(void *) BPF_FUNC_get_current_task;
> +static int (*bpf_skb_change_tail)(void *ctx, __u32 len, __u64 flags) =
> +	(void *) BPF_FUNC_skb_change_tail;
> +static long long (*bpf_csum_update)(void *ctx, __u32 csum) =
> +	(void *) BPF_FUNC_csum_update;
> +static void (*bpf_set_hash_invalid)(void *ctx) =
> +	(void *) BPF_FUNC_set_hash_invalid;
> +static int (*bpf_get_numa_node_id)(void) =
> +	(void *) BPF_FUNC_get_numa_node_id;
> +static int (*bpf_probe_read_str)(void *ctx, int size, const void *unsafe_ptr) =
> +	(void *) BPF_FUNC_probe_read_str;
> +static unsigned int (*bpf_get_socket_uid)(void *ctx) =
> +	(void *) BPF_FUNC_get_socket_uid;
> +static unsigned int (*bpf_set_hash)(void *ctx, __u32 hash) =
> +	(void *) BPF_FUNC_set_hash;
> +static int (*bpf_skb_adjust_room)(void *ctx, __s32 len_diff, __u32 mode,
> +				  unsigned long long flags) =
> +	(void *) BPF_FUNC_skb_adjust_room;
>  
>  /* Scan the ARCH passed in from ARCH env variable (see Makefile) */
>  #if defined(__TARGET_ARCH_x86)
> -- 
> 2.21.0.rc2.261.ga7da99ff1b-goog
> 
LGTM.  Please respin with a commit message.

Acked-by: Martin KaFai Lau <kafai@fb.com>

^ permalink raw reply

* Re: [PATCH] net: dsa: read mac address from DT for slave device
From: xiaofeis @ 2019-02-27  6:57 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Vinod Koul, David S. Miller, linux-arm-msm, Bjorn Andersson,
	Andrew Lunn, Vivien Didelot, Niklas Cassel, netdev
In-Reply-To: <9962d7a13bb25c88cc42b3f4dd68cee7@codeaurora.org>

On 2019-02-27 10:04, xiaofeis@codeaurora.org wrote:
> On 2019-02-26 15:45, xiaofeis@codeaurora.org wrote:
>> On 2019-02-26 01:27, Florian Fainelli wrote:
>>> On 2/25/19 5:28 AM, xiaofeis@codeaurora.org wrote:
>>>> Hi Florian
>>>> 
>>>> We have two slave DSA interfaces, wan0 and lan0, one is for wan 
>>>> port,
>>>> and the other is for lan port. Customer has it's mac address pool, 
>>>> they
>>>> want
>>>> to assign the mac address from the pool on wan0, lan0, and other
>>>> interfaces like
>>>> wifi, bt. Coreboot/uboot will populate it to the DTS node, so the 
>>>> driver
>>>> can
>>>> get it from it's node. For DSA slave interface, it already has it's 
>>>> own
>>>> DTS node, it's
>>>> easy to just add one porperty "local-mac-address" there for the 
>>>> usage in
>>>> DSA driver.
>>>> 
>>>> If not use DSA framework, normally we will use eth0.x and eth0.y for 
>>>> wan
>>>> and lan.
>>>> On this case, customer usually also assign the MAC address on these
>>>> logical interface
>>>> from it's pool.
>>> 
>>> OK, but this is not necessary per my previous explanation: the CPU 
>>> <=>
>>> WAN pipe is a separate broadcast domain (otherwise it is a security 
>>> hole
>>> since you exposing LAN machines to the outside world), and so there 
>>> is
>>> no need for a separate MAC address. It might be convenient to have 
>>> one,
>>> especially for the provider, if they run a management software (e.g.:
>>> TR69), but it is not required per-se.
>>> 
>>> Let me ask a secondary question here, how many Ethernet MACs connect 
>>> to
>>> the switch in this configuration? Is there one that is supposed to be
>>> assigned all LAN traffic and one that is supposed to be assigned all 
>>> WAN
>>> traffic? If so, then what you are doing makes even less
>>> 
>> 
>> Only one MAC connected to switch cpu port, both lan0 and wan0 are on 
>> the top of
>> same interface(eth0).
>> 
> Customer doesn't care about the MAC controller's MAC address, just
> leave it as the driver
> randomly generated. They just want to assign the MAC address on wan
> and lan DSA logical
> interface.
> 
> Many customer doesn't use DSA, for example, they use eth0.1/eth0.2 for
> lan/wan with one MAC controller.
> They configure switch wan port in vlan2 group, and lan port in vlan1
> group, they usually assign mac address
> on the logical interface(eth0.1&eth0.2), i think this is similar with
> DSA slave interfaces.
> 

Unlike vlan logical interface, dsa port already has it's dts node, so 
it's
easy and natural to add the mac address property for DSA driver use.

On DSA configuration, we won't assign ip on mater eth0. The mac DA from 
wan port will be
the wan0's mac address, the mac DA from lan port will be the lan0's mac 
address, they are
separate. I think the this is in general on some customer's deploy.

>>>> 
>>>> On 2019-02-22 23:43, Florian Fainelli wrote:
>>>>> On 2/22/19 4:58 AM, Vinod Koul wrote:
>>>>>> From: Xiaofei Shen <xiaofeis@codeaurora.org>
>>>>>> 
>>>>>> Before creating a slave netdevice, get the mac address from DTS 
>>>>>> and
>>>>>> apply in case it is valid.
>>>>> 
>>>>> Can you explain your use case in details?
>>>>> 
>>>>> Assigning a MAC address to a network device that represents a 
>>>>> switch
>>>>> port does not quite make sense in general. The switch port is 
>>>>> really
>>>>> representing one end of a pipe, so one side you have stations and 
>>>>> on the
>>>>> other side, you have the CPU/management Ethernet MAC controller's 
>>>>> MAC
>>>>> address which constitutes a station as well. The DSA slave network
>>>>> devices are just software constructs meant to steer traffic towards
>>>>> specific ports of the switch, but they are all from the perpsective 
>>>>> of
>>>>> traffic reaching the CPU Port in the first place, therefore traffic 
>>>>> that
>>>>> is generally a known unicast Ethernet frame with the CPU's MAC 
>>>>> address
>>>>> as MAC DA (and of course all types of unknown MC, management 
>>>>> traffic
>>>>> etc.)
>>>>> 
>>>>> By default, DSA switch need to come up in a configuration where all
>>>>> ports (except CPU/management) must be strictly separate from every 
>>>>> other
>>>>> port such that we can achieve what a standalone Ethernet NIC would 
>>>>> do.
>>>>> This works because all ports are isolated from one another, so 
>>>>> there is
>>>>> no cross talk and so having the same MAC address (the one from the 
>>>>> CPU)
>>>>> on the DSA slave network devices just works, each port is a 
>>>>> separate
>>>>> broadcast domain.
>>>>> 
>>>>> Once you start bridging one or ore ports, the bridge root port will 
>>>>> have
>>>>> a MAC address, most likely the one the CPU/management Ethernet MAC, 
>>>>> but
>>>>> similarly, this is not an issue and that's exactly how a software 
>>>>> bridge
>>>>> would work as well.
>>>>> 
>>>>>> 
>>>>>> Signed-off-by: Xiaofei Shen <xiaofeis@codeaurora.org>
>>>>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>>>>> ---
>>>>>>  include/net/dsa.h | 1 +
>>>>>>  net/dsa/dsa2.c    | 1 +
>>>>>>  net/dsa/slave.c   | 5 ++++-
>>>>>>  3 files changed, 6 insertions(+), 1 deletion(-)
>>>>>> 
>>>>>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>>>>>> index b3eefe8e18fd..aa24ce756679 100644
>>>>>> --- a/include/net/dsa.h
>>>>>> +++ b/include/net/dsa.h
>>>>>> @@ -198,6 +198,7 @@ struct dsa_port {
>>>>>>      unsigned int        index;
>>>>>>      const char        *name;
>>>>>>      const struct dsa_port    *cpu_dp;
>>>>>> +    const char        *mac;
>>>>>>      struct device_node    *dn;
>>>>>>      unsigned int        ageing_time;
>>>>>>      u8            stp_state;
>>>>>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>>>>>> index a1917025e155..afb7d9fa42f6 100644
>>>>>> --- a/net/dsa/dsa2.c
>>>>>> +++ b/net/dsa/dsa2.c
>>>>>> @@ -261,6 +261,7 @@ static int dsa_port_setup(struct dsa_port *dp)
>>>>>>      int err = 0;
>>>>>> 
>>>>>>      memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
>>>>>> +    dp->mac = of_get_mac_address(dp->dn);
>>>>>> 
>>>>>>      if (dp->type != DSA_PORT_TYPE_UNUSED)
>>>>>>          err = devlink_port_register(ds->devlink, 
>>>>>> &dp->devlink_port,
>>>>>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>>>>>> index a3fcc1d01615..8e64c4e947c6 100644
>>>>>> --- a/net/dsa/slave.c
>>>>>> +++ b/net/dsa/slave.c
>>>>>> @@ -1308,7 +1308,10 @@ int dsa_slave_create(struct dsa_port *port)
>>>>>>      slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
>>>>>>      slave_dev->hw_features |= NETIF_F_HW_TC;
>>>>>>      slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
>>>>>> -    eth_hw_addr_inherit(slave_dev, master);
>>>>>> +    if (port->mac && is_valid_ether_addr(port->mac))
>>>>>> +        ether_addr_copy(slave_dev->dev_addr, port->mac);
>>>>>> +    else
>>>>>> +        eth_hw_addr_inherit(slave_dev, master);
>>>>>>      slave_dev->priv_flags |= IFF_NO_QUEUE;
>>>>>>      slave_dev->netdev_ops = &dsa_slave_netdev_ops;
>>>>>>      slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
>>>>>> 

^ permalink raw reply

* Re: [RFC v1 11/19] RDMA/irdma: Add PBLE resource manager
From: Leon Romanovsky @ 2019-02-27  6:58 UTC (permalink / raw)
  To: Shiraz Saleem
  Cc: dledford, jgg, davem, linux-rdma, netdev, mustafa.ismail,
	jeffrey.t.kirsher
In-Reply-To: <20190215171107.6464-12-shiraz.saleem@intel.com>

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

On Fri, Feb 15, 2019 at 11:10:58AM -0600, Shiraz Saleem wrote:
> From: Mustafa Ismail <mustafa.ismail@intel.com>
>
> Implement a Physical Buffer List Entry (PBLE) resource manager
> to manage a pool of PBLE HMC resource objects.
>
> Signed-off-by: Mustafa Ismail <mustafa.ismail@intel.com>
> Signed-off-by: Shiraz Saleem <shiraz.saleem@intel.com>
> ---
>  drivers/infiniband/hw/irdma/pble.c | 520 +++++++++++++++++++++++++++++++++++++
>  drivers/infiniband/hw/irdma/pble.h | 135 ++++++++++
>  2 files changed, 655 insertions(+)
>  create mode 100644 drivers/infiniband/hw/irdma/pble.c
>  create mode 100644 drivers/infiniband/hw/irdma/pble.h
>
> diff --git a/drivers/infiniband/hw/irdma/pble.c b/drivers/infiniband/hw/irdma/pble.c
> new file mode 100644
> index 0000000..66fab69
> --- /dev/null
> +++ b/drivers/infiniband/hw/irdma/pble.c
> @@ -0,0 +1,520 @@
> +// SPDX-License-Identifier: GPL-2.0 or Linux-OpenIB
> +/* Copyright (c) 2019, Intel Corporation. */
> +
> +#include "osdep.h"
> +#include "status.h"
> +#include "hmc.h"
> +#include "defs.h"
> +#include "type.h"
> +#include "protos.h"
> +#include "pble.h"
> +
> +static enum irdma_status_code add_pble_prm(struct irdma_hmc_pble_rsrc *pble_rsrc);
> +
> +/**
> + * irdma_destroy_pble_prm - destroy prm during module unload
> + * @pble_rsrc:	pble resources
> + */
> +void irdma_destroy_pble_prm(struct irdma_hmc_pble_rsrc *pble_rsrc)
> +{
> +	struct irdma_sc_dev *dev = pble_rsrc->dev;
> +	struct irdma_chunk *chunk;
> +	struct irdma_pble_prm *pinfo = &pble_rsrc->pinfo;
> +
> +	while (!list_empty(&pinfo->clist)) {
> +		chunk = (struct irdma_chunk *)pinfo->clist.next;
> +		list_del(&chunk->list);
> +		if (chunk->type == PBLE_SD_PAGED)
> +			irdma_pble_free_paged_mem(chunk);
> +		if (chunk->bitmapbuf)
> +			irdma_free_virt_mem(dev->hw, &chunk->bitmapmem);
> +		irdma_free_virt_mem(dev->hw, &chunk->chunkmem);
> +	}
> +}
> +
> +/**
> + * irdma_hmc_init_pble - Initialize pble resources during module load
> + * @dev: irdma_sc_dev struct
> + * @pble_rsrc:	pble resources
> + */
> +enum irdma_status_code
> +irdma_hmc_init_pble(struct irdma_sc_dev *dev,
> +		    struct irdma_hmc_pble_rsrc *pble_rsrc)
> +{
> +	struct irdma_hmc_info *hmc_info;
> +	u32 fpm_idx = 0;
> +	enum irdma_status_code status = 0;
> +
> +	hmc_info = dev->hmc_info;
> +	pble_rsrc->dev = dev;
> +	pble_rsrc->fpm_base_addr = hmc_info->hmc_obj[IRDMA_HMC_IW_PBLE].base;
> +	/* Start pble' on 4k boundary */
> +	if (pble_rsrc->fpm_base_addr & 0xfff)
> +		fpm_idx = (PAGE_SIZE - (pble_rsrc->fpm_base_addr & 0xfff)) >> 3;
> +	pble_rsrc->unallocated_pble =
> +		hmc_info->hmc_obj[IRDMA_HMC_IW_PBLE].cnt - fpm_idx;
> +	pble_rsrc->next_fpm_addr = pble_rsrc->fpm_base_addr + (fpm_idx << 3);
> +	pble_rsrc->pinfo.pble_shift = PBLE_SHIFT;
> +
> +	spin_lock_init(&pble_rsrc->pinfo.prm_lock);
> +	INIT_LIST_HEAD(&pble_rsrc->pinfo.clist);
> +	if (add_pble_prm(pble_rsrc)) {
> +		irdma_destroy_pble_prm(pble_rsrc);
> +		status = IRDMA_ERR_NO_MEMORY;
> +	}
> +
> +	return status;
> +}
> +
> +/**
> + * get_sd_pd_idx -  Returns sd index, pd index and rel_pd_idx from fpm address
> + * @ pble_rsrc:	structure containing fpm address
> + * @ idx: where to return indexes
> + */
> +static void get_sd_pd_idx(struct irdma_hmc_pble_rsrc *pble_rsrc,
> +			  struct sd_pd_idx *idx)
> +{
> +	idx->sd_idx = (u32)(pble_rsrc->next_fpm_addr) /
> +		      IRDMA_HMC_DIRECT_BP_SIZE;
> +	idx->pd_idx = (u32)(pble_rsrc->next_fpm_addr) / IRDMA_HMC_PAGED_BP_SIZE;
> +	idx->rel_pd_idx = (idx->pd_idx % IRDMA_HMC_PD_CNT_IN_SD);

The amount of type-casting in this driver is astonishing. It will be
better to declare all types to be aligned from the beginning.


> +}
> +
> +/**
> + * add_sd_direct - add sd direct for pble
> + * @pble_rsrc: pble resource ptr
> + * @info: page info for sd
> + */
> +static enum irdma_status_code
> +add_sd_direct(struct irdma_hmc_pble_rsrc *pble_rsrc,
> +	      struct irdma_add_page_info *info)
> +{
> +	struct irdma_sc_dev *dev = pble_rsrc->dev;
> +	enum irdma_status_code ret_code = 0;
> +	struct sd_pd_idx *idx = &info->idx;
> +	struct irdma_chunk *chunk = info->chunk;
> +	struct irdma_hmc_info *hmc_info = info->hmc_info;
> +	struct irdma_hmc_sd_entry *sd_entry = info->sd_entry;
> +	u32 offset = 0;
> +
> +	if (!sd_entry->valid) {
> +		ret_code = irdma_add_sd_table_entry(dev->hw, hmc_info,
> +						    info->idx.sd_idx,
> +						    IRDMA_SD_TYPE_DIRECT,
> +						    IRDMA_HMC_DIRECT_BP_SIZE);
> +		if (ret_code)
> +			return ret_code;
> +
> +		chunk->type = PBLE_SD_CONTIGOUS;
> +	}
> +
> +	offset = idx->rel_pd_idx << HMC_PAGED_BP_SHIFT;
> +	chunk->size = info->pages << HMC_PAGED_BP_SHIFT;
> +	chunk->vaddr = (u64)((u8 *)sd_entry->u.bp.addr.va + offset);
> +	chunk->fpm_addr = pble_rsrc->next_fpm_addr;
> +	irdma_debug(dev, IRDMA_DEBUG_PBLE,
> +		    "chunk_size[%lld] = 0x%llx vaddr=0x%llx fpm_addr = %llx\n",
> +		    chunk->size, chunk->size, chunk->vaddr, chunk->fpm_addr);
> +
> +	return 0;
> +}
> +
> +/**
> + * fpm_to_idx - given fpm address, get pble index
> + * @pble_rsrc: pble resource management
> + * @addr: fpm address for index
> + */
> +static u32 fpm_to_idx(struct irdma_hmc_pble_rsrc *pble_rsrc, u64 addr)
> +{
> +	u64 idx;
> +
> +	idx = (addr - (pble_rsrc->fpm_base_addr)) >> 3;
> +
> +	return (u32)idx;

lower_32_bits()

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply

* Re: [RFC v1 12/19] RDMA/irdma: Implement device supported verb APIs
From: Gal Pressman @ 2019-02-27  7:31 UTC (permalink / raw)
  To: Saleem, Shiraz, dledford@redhat.com, jgg@ziepe.ca,
	davem@davemloft.net
  Cc: linux-rdma@vger.kernel.org, netdev@vger.kernel.org,
	Ismail, Mustafa, Kirsher, Jeffrey T, Yossi Leybovich
In-Reply-To: <9DD61F30A802C4429A01CA4200E302A7A5A4FB85@fmsmsx124.amr.corp.intel.com>

On 26-Feb-19 23:09, Saleem, Shiraz wrote:
>> Subject: Re: [RFC v1 12/19] RDMA/irdma: Implement device supported verb APIs
>>
>> On 15-Feb-19 19:10, Shiraz Saleem wrote:
>>> /**
>>>  * irdma_dealloc_ucontext - deallocate the user context data structure
>>>  * @context: user context created during alloc  */ static int
>>> irdma_dealloc_ucontext(struct ib_ucontext *context) {
>>> 	struct irdma_ucontext *ucontext = to_ucontext(context);
>>> 	unsigned long flags;
>>>
>>> 	spin_lock_irqsave(&ucontext->cq_reg_mem_list_lock, flags);
>>> 	if (!list_empty(&ucontext->cq_reg_mem_list)) {
>>> 		spin_unlock_irqrestore(&ucontext->cq_reg_mem_list_lock, flags);
>>> 		return -EBUSY;
>>> 	}
>>> 	spin_unlock_irqrestore(&ucontext->cq_reg_mem_list_lock, flags);
>>>
>>> 	spin_lock_irqsave(&ucontext->qp_reg_mem_list_lock, flags);
>>> 	if (!list_empty(&ucontext->qp_reg_mem_list)) {
>>> 		spin_unlock_irqrestore(&ucontext->qp_reg_mem_list_lock, flags);
>>> 		return -EBUSY;
>>
>> Drivers are not permitted to fail dealloc_ucontext.
> 
> This is fixed in RFC v1 submission. Maybe this was pasted from the v0 ver?
> 
> [..]
> 
>>> +/**
>>> + * irdma_alloc_pd - allocate protection domain
>>> + * @pd: PD pointer
>>> + * @context: user context created during alloc
>>> + * @udata: user data
>>> + */
>>> +static int irdma_alloc_pd(struct ib_pd *pd,
>>> +			  struct ib_ucontext *context,
>>> +			  struct ib_udata *udata)
>>> +{
>>> +	struct irdma_pd *iwpd = to_iwpd(pd);
>>> +	struct irdma_device *iwdev = to_iwdev(pd->device);
>>> +	struct irdma_sc_dev *dev = &iwdev->rf->sc_dev;
>>> +	struct irdma_pci_f *rf = iwdev->rf;
>>> +	struct irdma_alloc_pd_resp uresp = {};
>>> +	struct irdma_sc_pd *sc_pd;
>>> +	struct irdma_ucontext *ucontext;
>>> +	u32 pd_id = 0;
>>> +	int err;
>>> +
>>> +	if (iwdev->closing)
>>> +		return -ENODEV;
>>> +
>>> +	err = irdma_alloc_rsrc(rf, rf->allocated_pds, rf->max_pd, &pd_id,
>>> +			       &rf->next_pd);
>>> +	if (err)
>>> +		return err;
>>> +
>>> +	sc_pd = &iwpd->sc_pd;
>>> +	if (context) {
>>
>> I think this should be 'if (udata)', this applies to many other places in this driver.
> 
> That’s right. Will fix it.
> 
>>
>>> +		ucontext = to_ucontext(context);
>>> +		dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, ucontext->abi_ver);
>>> +		uresp.pd_id = pd_id;
>>> +		if (ib_copy_to_udata(udata, &uresp, sizeof(uresp))) {
>>> +			err = -EFAULT;
>>> +			goto error;
>>> +		}
>>> +	} else {
>>> +		dev->iw_pd_ops->pd_init(dev, sc_pd, pd_id, -1);
>>> +	}
>>> +
>>> +	irdma_add_pdusecount(iwpd);
>>> +
>>> +	return 0;
>>> +error:
>>> +	irdma_free_rsrc(rf, rf->allocated_pds, pd_id);
>>> +
>>> +	return err;
>>> +}
>>> +/**
>>> + * irdma_create_qp - create qp
>>> + * @ibpd: ptr of pd
>>> + * @init_attr: attributes for qp
>>> + * @udata: user data for create qp
>>> + */
>>> +static struct ib_qp *irdma_create_qp(struct ib_pd *ibpd,
>>> +				     struct ib_qp_init_attr *init_attr,
>>> +				     struct ib_udata *udata)
>>> +{
>>> +	struct irdma_pd *iwpd = to_iwpd(ibpd);
>>> +	struct irdma_device *iwdev = to_iwdev(ibpd->device);
>>> +	struct irdma_pci_f *rf = iwdev->rf;
>>> +	struct irdma_cqp *iwcqp = &rf->cqp;
>>> +	struct irdma_qp *iwqp;
>>> +	struct irdma_ucontext *ucontext;
>>> +	struct irdma_create_qp_req req;
>>> +	struct irdma_create_qp_resp uresp = {};
>>> +	struct i40iw_create_qp_resp uresp_gen1 = {};
>>> +	u32 qp_num = 0;
>>> +	void *mem;
>>> +	enum irdma_status_code ret;
>>> +	int err_code = 0;
>>> +	int sq_size;
>>> +	int rq_size;
>>> +	struct irdma_sc_qp *qp;
>>> +	struct irdma_sc_dev *dev = &rf->sc_dev;
>>> +	struct irdma_qp_init_info init_info = {};
>>> +	struct irdma_create_qp_info *qp_info;
>>> +	struct irdma_cqp_request *cqp_request;
>>> +	struct cqp_cmds_info *cqp_info;
>>> +	struct irdma_qp_host_ctx_info *ctx_info;
>>> +	struct irdma_iwarp_offload_info *iwarp_info;
>>> +	struct irdma_roce_offload_info *roce_info;
>>> +	struct irdma_udp_offload_info *udp_info;
>>> +	unsigned long flags;
>>> +
>>> +	if (iwdev->closing)
>>> +		return ERR_PTR(-ENODEV);
>>> +
>>> +	if (init_attr->create_flags)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>> +	if (init_attr->cap.max_inline_data > dev->hw_attrs.max_hw_inline)
>>> +		init_attr->cap.max_inline_data = dev->hw_attrs.max_hw_inline;
>>> +
>>> +	if (init_attr->cap.max_send_sge > dev->hw_attrs.max_hw_wq_frags)
>>> +		init_attr->cap.max_send_sge = dev-
>>> hw_attrs.max_hw_wq_frags;
>>> +
>>> +	if (init_attr->cap.max_recv_sge > dev->hw_attrs.max_hw_wq_frags)
>>> +		init_attr->cap.max_recv_sge = dev->hw_attrs.max_hw_wq_frags;
>>
>> AFAIK, you can change the requested values to be greater than or equal to the
>> values requested. I don't think you can change them to something smaller.
> 
> Hmm...This is a sanity check to make sure we don’t exceed the device supported values.
> But we should fail the call.
> 
> [..]
> 
>>> +	mem = kzalloc(sizeof(*iwqp), GFP_KERNEL);
>>> +	if (!mem)
>>> +		return ERR_PTR(-ENOMEM);
>>> +
>>> +	iwqp = (struct irdma_qp *)mem;
>>> +	iwqp->allocated_buf = mem;
>>
>> 'allocated_buf' feels redundant. Why is iwqp not sufficient?
> 
> I agree.
> [..]
> 
>>> +	if (udata) {
>>> +		err_code = ib_copy_from_udata(&req, udata, sizeof(req));
>>
>> Perhaps ib_copy_from_udata(&req, udata, min(sizeof(req), udata->inlen)?
>> Applies to other call sites of ib_copy_from/to_udata as well.
>>
> 
> It’s a good idea.
> 
>>> + * irdma_query - query qp attributes
>>> + * @ibqp: qp pointer
>>> + * @attr: attributes pointer
>>> + * @attr_mask: Not used
>>> + * @init_attr: qp attributes to return  */ static int
>>> +irdma_query_qp(struct ib_qp *ibqp,
>>> +			  struct ib_qp_attr *attr,
>>> +			  int attr_mask,
>>> +			  struct ib_qp_init_attr *init_attr) {
>>> +	struct irdma_qp *iwqp = to_iwqp(ibqp);
>>> +	struct irdma_sc_qp *qp = &iwqp->sc_qp;
>>> +
>>> +	attr->qp_state = iwqp->ibqp_state;
>>> +	attr->cur_qp_state = iwqp->ibqp_state;
>>> +	attr->qp_access_flags = 0;
>>> +	attr->cap.max_send_wr = qp->qp_uk.sq_size - 1;
>>> +	attr->cap.max_recv_wr = qp->qp_uk.rq_size - 1;
>>
>> Why -1?
> 
> It's reserved for HW. But the equation should be 
> (sqdepth - I40IW_SQ_RSVD) >> sqshift.
> 
> [....]
>>
>>> +	attr->cap.max_inline_data = qp->qp_uk.max_inline_data;
>>> +	attr->cap.max_send_sge = qp->qp_uk.max_sq_frag_cnt;
>>> +	attr->cap.max_recv_sge = qp->qp_uk.max_rq_frag_cnt;
>>> +	attr->qkey = iwqp->roce_info.qkey;
>>> +
>>> +	init_attr->event_handler = iwqp->ibqp.event_handler;
>>> +	init_attr->qp_context = iwqp->ibqp.qp_context;
>>> +	init_attr->send_cq = iwqp->ibqp.send_cq;
>>> +	init_attr->recv_cq = iwqp->ibqp.recv_cq;
>>> +	init_attr->srq = iwqp->ibqp.srq;
>>> +	init_attr->cap = attr->cap;
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +/**
>>> + * irdma_destroy_cq - destroy cq
>>> + * @ib_cq: cq pointer
>>> + */
>>> +static int irdma_destroy_cq(struct ib_cq *ib_cq) {
>>> +	struct irdma_cq *iwcq;
>>> +	struct irdma_device *iwdev;
>>> +	struct irdma_sc_cq *cq;
>>> +
>>> +	if (!ib_cq) {
>>> +		irdma_pr_err("ib_cq == NULL\n");
>>> +		return 0;
>>> +	}
>>
>> Is this really needed? Which caller can pass NULL pointer?
> 
> Not needed.
> 
>>> +
>>> +/**
>>> + * board_id_show
>>> + */
>>> +static ssize_t board_id_show(struct device *dev,
>>> +			     struct device_attribute *attr,
>>> +			     char *buf)
>>> +{
>>> +	return sprintf(buf, "%.*s\n", 32, "IRDMA Board ID");
>>
>> That doesn't add much information.
> 
> Will fix.
> 
>>
>>> +}
>>> +
>>> +static DEVICE_ATTR_RO(hw_rev);
>>> +static DEVICE_ATTR_RO(hca_type);
>>> +static DEVICE_ATTR_RO(board_id);
>>> +
>>> +static struct attribute *irdma_dev_attributes[] = {
>>> +	&dev_attr_hw_rev.attr,
>>> +	&dev_attr_hca_type.attr,
>>> +	&dev_attr_board_id.attr,
>>> +	NULL
>>> +};
>>> +
>>> +static const struct attribute_group irdma_attr_group = {
>>> +	.attrs = irdma_dev_attributes,
>>> +};
>>> +
>>> +/**
>>> + * irdma_modify_port  Modify port properties
>>> + * @ibdev: device pointer from stack
>>> + * @port: port number
>>> + * @port_modify_mask: mask for port modifications
>>> + * @props: port properties
>>> + */
>>> +static int irdma_modify_port(struct ib_device *ibdev,
>>> +			     u8 port,
>>> +			     int port_modify_mask,
>>> +			     struct ib_port_modify *props) {
>>> +	return 0;
>>> +}
>>
>> Same question as disacossiate_ucontext.
> 
> This was likely added during early dev. and can be removed.
> 
>>
>>> +
>>> +/**
>>> + * irdma_query_gid_roce - Query port GID for Roce
>>> + * @ibdev: device pointer from stack
>>> + * @port: port number
>>> + * @index: Entry index
>>> + * @gid: Global ID
>>> + */
>>> +static int irdma_query_gid_roce(struct ib_device *ibdev,
>>> +				u8 port,
>>> +				int index,
>>> +				union ib_gid *gid)
>>> +{
>>> +	int ret;
>>> +
>>> +	ret = rdma_query_gid(ibdev, port, index, gid);
>>> +	if (ret == -EAGAIN) {
>>
>> I can't see a path where rdma_query_gid returns -EAGAIN.
> 
> This function can be removed now. It's only applicable to non-Roce providers.
> 
>>
>>> +		memcpy(gid, &zgid, sizeof(*gid));
>>> +		return 0;
>>> +	}
>>> +
>>> +	return ret;
>>> +}
>>> +
>>
>>> +/**
>>> + * irdma_create_ah - create address handle
>>> + * @ibpd: ptr to protection domain
>>> + * @ah_attr: address handle attributes
>>
>> 'ah_attr' -> 'attr', missing flags and udata.
> 
> Will fix all these hits in the driver.
> 
> [..]
>>> + */
>>> +static int irdma_destroy_ah(struct ib_ah *ibah, u32 flags) {
>>> +	struct irdma_device *iwdev = to_iwdev(ibah->device);
>>> +	struct irdma_ah *ah = to_iwah(ibah);
>>> +	int err;
>>> +
>>> +	if (!ah->sc_ah.ah_info.ah_valid)
>>> +		return -EINVAL;
>>> +
>>> +	err = irdma_ah_cqp_op(iwdev->rf, &ah->sc_ah,
>> IRDMA_OP_AH_DESTROY,
>>> +			      flags & RDMA_DESTROY_AH_SLEEPABLE,
>>> +			      irdma_destroy_ah_cb, ah);
>>> +	if (!err)
>>> +		return 0;
>>
>> Why are the rest of the cleanups only in case of error?
> 
> On success, the cleanup is done in the callback, irdma_destroy_ah_cb.
> 
> [...]
> 
> 
>>> +static __be64 irdma_mac_to_guid(struct net_device *ndev) {
>>> +	unsigned char *mac = ndev->dev_addr;
>>> +	__be64 guid;
>>> +	unsigned char *dst = (unsigned char *)&guid;
>>> +
>>> +	dst[0] = mac[0] ^ 2;
>>> +	dst[1] = mac[1];
>>> +	dst[2] = mac[2];
>>> +	dst[3] = 0xff;
>>> +	dst[4] = 0xfe;
>>> +	dst[5] = mac[3];
>>> +	dst[6] = mac[4];
>>> +	dst[7] = mac[5];
>>> +
>>> +	return guid;
>>> +}
>>
>> There's a variant of this function in irdma, bnxt_re, ocrdma and qedr.
>> Maybe it's time to provide it in common code?
> 
> Agreed.
> 

Other than that:
Reviewed-by: Gal Pressman <galpress@amazon.com>

^ permalink raw reply


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