All of lore.kernel.org
 help / color / mirror / Atom feed
* [pull request][net 0/6] Mellanox, mlx5 fixes 2017-11-08
@ 2017-11-08  7:21 Saeed Mahameed
  2017-11-08  7:21 ` [net 1/6] net/mlx5: Loop over temp list to release delay events Saeed Mahameed
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-08  7:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Saeed Mahameed

Hi Dave,

The follwoing series includes some fixes for mlx5 core and etherent
driver.

Sorry for the late submission but as you can see i have some very
critical fixes below that i would like them merged into this RC. 

Please pull and let me know if there is any problem.

For -stable:
('net/mlx5e: Set page to null in case dma mapping fails') kernels >= 4.13
('net/mlx5: FPGA, return -EINVAL if size is zero') kernels >= 4.13
('net/mlx5: Cancel health poll before sending panic teardown command') kernels >= 4.13

Thanks,
Saeed.

--- 

The following changes since commit 7fd078337201cf7468f53c3d9ef81ff78cb6df3b:

  net: qmi_wwan: fix divide by 0 on bad descriptors (2017-11-08 13:42:43 +0900)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-fixes-2017-11-08

for you to fetch changes up to a45ff49e52ba34e1607a6bf228bad56a5a9469b8:

  net/mlx5e: Increase Striding RQ minimum size limit to 4 multi-packet WQEs (2017-11-07 21:36:21 -0800)

----------------------------------------------------------------
mlx5-fixes-2017-11-08

This series includes some fixes for mlx5 core and ethernet driver.

----------------------------------------------------------------
Eugenia Emantayev (1):
      net/mlx5e: Increase Striding RQ minimum size limit to 4 multi-packet WQEs

Huy Nguyen (2):
      net/mlx5: Loop over temp list to release delay events
      net/mlx5: Cancel health poll before sending panic teardown command

Inbar Karmy (1):
      net/mlx5e: Set page to null in case dma mapping fails

Kamal Heib (1):
      net/mlx5: FPGA, return -EINVAL if size is zero

Saeed Mahameed (1):
      net/mlx5e: Fix napi poll with zero budget

 drivers/net/ethernet/mellanox/mlx5/core/dev.c      |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en.h       |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c    | 12 +++++-------
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c  | 10 ++++++----
 drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c |  6 ++++++
 drivers/net/ethernet/mellanox/mlx5/core/main.c     |  7 +++++++
 6 files changed, 26 insertions(+), 13 deletions(-)

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [net 1/6] net/mlx5: Loop over temp list to release delay events
  2017-11-08  7:21 [pull request][net 0/6] Mellanox, mlx5 fixes 2017-11-08 Saeed Mahameed
@ 2017-11-08  7:21 ` Saeed Mahameed
  2017-11-08  7:21 ` [net 2/6] net/mlx5: Cancel health poll before sending panic teardown command Saeed Mahameed
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-08  7:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Huy Nguyen, Feras Daoud, Saeed Mahameed

From: Huy Nguyen <huyn@mellanox.com>

list_splice_init initializing waiting_events_list after splicing it to
temp list, therefore we should loop over temp list to fire the events.

Fixes: 4ca637a20a52 ("net/mlx5: Delay events till mlx5 interface's add complete for pci resume")
Signed-off-by: Huy Nguyen <huyn@mellanox.com>
Signed-off-by: Feras Daoud <ferasda@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
index fc281712869b..17b723218b0c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/dev.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
@@ -93,7 +93,7 @@ static void delayed_event_release(struct mlx5_device_context *dev_ctx,
 	list_splice_init(&priv->waiting_events_list, &temp);
 	if (!dev_ctx->context)
 		goto out;
-	list_for_each_entry_safe(de, n, &priv->waiting_events_list, list)
+	list_for_each_entry_safe(de, n, &temp, list)
 		dev_ctx->intf->event(dev, dev_ctx->context, de->event, de->param);
 
 out:
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net 2/6] net/mlx5: Cancel health poll before sending panic teardown command
  2017-11-08  7:21 [pull request][net 0/6] Mellanox, mlx5 fixes 2017-11-08 Saeed Mahameed
  2017-11-08  7:21 ` [net 1/6] net/mlx5: Loop over temp list to release delay events Saeed Mahameed
@ 2017-11-08  7:21 ` Saeed Mahameed
  2017-11-08 14:28   ` Or Gerlitz
  2017-11-08  7:21 ` [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero Saeed Mahameed
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-08  7:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Huy Nguyen, Saeed Mahameed

From: Huy Nguyen <huyn@mellanox.com>

After the panic teardown firmware command, health_care detects the error
in PCI bus and calls the mlx5_pci_err_detected. This health_care flow is
no longer needed because the panic teardown firmware command will bring
down the PCI bus communication with the HCA.

The solution is to cancel the health care timer and its pending
workqueue request before sending panic teardown firmware command.

Kernel trace:
mlx5_core 0033:01:00.0: Shutdown was called
mlx5_core 0033:01:00.0: health_care:154:(pid 9304): handling bad device here
mlx5_core 0033:01:00.0: mlx5_handle_bad_state:114:(pid 9304): NIC state 1
mlx5_core 0033:01:00.0: mlx5_pci_err_detected was called
mlx5_core 0033:01:00.0: mlx5_enter_error_state:96:(pid 9304): start
mlx5_3:mlx5_ib_event:3061:(pid 9304): warning: event on port 0
mlx5_core 0033:01:00.0: mlx5_enter_error_state:104:(pid 9304): end
Unable to handle kernel paging request for data at address 0x0000003f
Faulting instruction address: 0xc0080000434b8c80

Fixes: 8812c24d28f4 ('net/mlx5: Add fast unload support in shutdown flow')
Signed-off-by: Huy Nguyen <huyn@mellanox.com>
Reviewed-by: Majd Dibbiny <majd@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 0d2c8dcd6eae..06562c9a6b9c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1482,9 +1482,16 @@ static int mlx5_try_fast_unload(struct mlx5_core_dev *dev)
 		return -EAGAIN;
 	}
 
+	/* Panic tear down fw command will stop the PCI bus communication
+	 * with the HCA, so the health polll is no longer needed.
+	 */
+	mlx5_drain_health_wq(dev);
+	mlx5_stop_health_poll(dev);
+
 	ret = mlx5_cmd_force_teardown_hca(dev);
 	if (ret) {
 		mlx5_core_dbg(dev, "Firmware couldn't do fast unload error: %d\n", ret);
+		mlx5_start_health_poll(dev);
 		return ret;
 	}
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero
  2017-11-08  7:21 [pull request][net 0/6] Mellanox, mlx5 fixes 2017-11-08 Saeed Mahameed
  2017-11-08  7:21 ` [net 1/6] net/mlx5: Loop over temp list to release delay events Saeed Mahameed
  2017-11-08  7:21 ` [net 2/6] net/mlx5: Cancel health poll before sending panic teardown command Saeed Mahameed
@ 2017-11-08  7:21 ` Saeed Mahameed
  2017-11-08 14:13   ` Or Gerlitz
  2017-11-08  7:21 ` [net 4/6] net/mlx5e: Fix napi poll with zero budget Saeed Mahameed
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-08  7:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Kamal Heib, Saeed Mahameed

From: Kamal Heib <kamalh@mellanox.com>

In the current code, if a size of zero is passed to
mlx5_fpga_mem_{read|write}_i2c() functions the "err"
return value will not initialized.

Fixes: a9956d35d199 ('net/mlx5: FPGA, Add SBU infrastructure')
Signed-off-by: Kamal Heib <kamalh@mellanox.com>
Reviewed-by: Yevgeny Kliteynik <kliteyn@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
index 3c11d6e2160a..14962969c5ba 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/sdk.c
@@ -66,6 +66,9 @@ static int mlx5_fpga_mem_read_i2c(struct mlx5_fpga_device *fdev, size_t size,
 	u8 actual_size;
 	int err;
 
+	if (!size)
+		return -EINVAL;
+
 	if (!fdev->mdev)
 		return -ENOTCONN;
 
@@ -95,6 +98,9 @@ static int mlx5_fpga_mem_write_i2c(struct mlx5_fpga_device *fdev, size_t size,
 	u8 actual_size;
 	int err;
 
+	if (!size)
+		return -EINVAL;
+
 	if (!fdev->mdev)
 		return -ENOTCONN;
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net 4/6] net/mlx5e: Fix napi poll with zero budget
  2017-11-08  7:21 [pull request][net 0/6] Mellanox, mlx5 fixes 2017-11-08 Saeed Mahameed
                   ` (2 preceding siblings ...)
  2017-11-08  7:21 ` [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero Saeed Mahameed
@ 2017-11-08  7:21 ` Saeed Mahameed
  2017-11-08  7:21 ` [net 5/6] net/mlx5e: Set page to null in case dma mapping fails Saeed Mahameed
  2017-11-08  7:21 ` [net 6/6] net/mlx5e: Increase Striding RQ minimum size limit to 4 multi-packet WQEs Saeed Mahameed
  5 siblings, 0 replies; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-08  7:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Saeed Mahameed, kernel-team

napi->poll can be called with budget 0, e.g. in netpoll scenarios
where the caller only wants to poll TX rings
(poll_one_napi@net/core/netpoll.c).

The below commit changed RX polling from "while" loop to "do {} while",
which caused to ignore the initial budget and handle at least one RX
packet.

This fixes the following warning:
[ 2852.049194] mlx5e_napi_poll+0x0/0x260 [mlx5_core] exceeded budget in poll
[ 2852.049195] ------------[ cut here ]------------
[ 2852.049195] WARNING: CPU: 0 PID: 25691 at net/core/netpoll.c:171 netpoll_poll_dev+0x18a/0x1a0

Fixes: 4b7dfc992514 ("net/mlx5e: Early-return on empty completion queues")
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Reported-by: Martin KaFai Lau <kafai@fb.com>
Tested-by: Martin KaFai Lau <kafai@fb.com>
Cc: kernel-team@fb.com
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
index e906b754415c..ab92298eafc3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_txrx.c
@@ -49,7 +49,7 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 	struct mlx5e_channel *c = container_of(napi, struct mlx5e_channel,
 					       napi);
 	bool busy = false;
-	int work_done;
+	int work_done = 0;
 	int i;
 
 	for (i = 0; i < c->num_tc; i++)
@@ -58,15 +58,17 @@ int mlx5e_napi_poll(struct napi_struct *napi, int budget)
 	if (c->xdp)
 		busy |= mlx5e_poll_xdpsq_cq(&c->rq.xdpsq.cq);
 
-	work_done = mlx5e_poll_rx_cq(&c->rq.cq, budget);
-	busy |= work_done == budget;
+	if (likely(budget)) { /* budget=0 means: don't poll rx rings */
+		work_done = mlx5e_poll_rx_cq(&c->rq.cq, budget);
+		busy |= work_done == budget;
+	}
 
 	busy |= c->rq.post_wqes(&c->rq);
 
 	if (busy) {
 		if (likely(mlx5e_channel_no_affinity_change(c)))
 			return budget;
-		if (work_done == budget)
+		if (budget && work_done == budget)
 			work_done--;
 	}
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net 5/6] net/mlx5e: Set page to null in case dma mapping fails
  2017-11-08  7:21 [pull request][net 0/6] Mellanox, mlx5 fixes 2017-11-08 Saeed Mahameed
                   ` (3 preceding siblings ...)
  2017-11-08  7:21 ` [net 4/6] net/mlx5e: Fix napi poll with zero budget Saeed Mahameed
@ 2017-11-08  7:21 ` Saeed Mahameed
  2017-11-08  7:21 ` [net 6/6] net/mlx5e: Increase Striding RQ minimum size limit to 4 multi-packet WQEs Saeed Mahameed
  5 siblings, 0 replies; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-08  7:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Inbar Karmy, kernel-team, Saeed Mahameed

From: Inbar Karmy <inbark@mellanox.com>

Currently, when dma mapping fails, put_page is called,
but the page is not set to null. Later, in the page_reuse treatment in
mlx5e_free_rx_descs(), mlx5e_page_release() is called for the second time,
improperly doing dma_unmap (for a non-mapped address) and an extra put_page.
Prevent this by nullifying the page pointer when dma_map fails.

Fixes: accd58833237 ("net/mlx5e: Introduce RX Page-Reuse")
Signed-off-by: Inbar Karmy <inbark@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Cc: kernel-team@fb.com
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
index 15a1687483cc..91b1b0938931 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
@@ -215,22 +215,20 @@ static inline bool mlx5e_rx_cache_get(struct mlx5e_rq *rq,
 static inline int mlx5e_page_alloc_mapped(struct mlx5e_rq *rq,
 					  struct mlx5e_dma_info *dma_info)
 {
-	struct page *page;
-
 	if (mlx5e_rx_cache_get(rq, dma_info))
 		return 0;
 
-	page = dev_alloc_pages(rq->buff.page_order);
-	if (unlikely(!page))
+	dma_info->page = dev_alloc_pages(rq->buff.page_order);
+	if (unlikely(!dma_info->page))
 		return -ENOMEM;
 
-	dma_info->addr = dma_map_page(rq->pdev, page, 0,
+	dma_info->addr = dma_map_page(rq->pdev, dma_info->page, 0,
 				      RQ_PAGE_SIZE(rq), rq->buff.map_dir);
 	if (unlikely(dma_mapping_error(rq->pdev, dma_info->addr))) {
-		put_page(page);
+		put_page(dma_info->page);
+		dma_info->page = NULL;
 		return -ENOMEM;
 	}
-	dma_info->page = page;
 
 	return 0;
 }
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [net 6/6] net/mlx5e: Increase Striding RQ minimum size limit to 4 multi-packet WQEs
  2017-11-08  7:21 [pull request][net 0/6] Mellanox, mlx5 fixes 2017-11-08 Saeed Mahameed
                   ` (4 preceding siblings ...)
  2017-11-08  7:21 ` [net 5/6] net/mlx5e: Set page to null in case dma mapping fails Saeed Mahameed
@ 2017-11-08  7:21 ` Saeed Mahameed
  5 siblings, 0 replies; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-08  7:21 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eugenia Emantayev, kernel-team, Saeed Mahameed

From: Eugenia Emantayev <eugenia@mellanox.com>

This is to prevent the case of working with a single MPWQE
(1 WQE is always reserved as RQ is linked-list).
When the WQE is fully consumed, HW should still have available buffer
in order not to drop packets.

Fixes: 461017cb006a ("net/mlx5e: Support RX multi-packet WQE (Striding RQ)")
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
Cc: kernel-team@fb.com
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index cc13d3dbd366..13b5ef9d8703 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -67,7 +67,7 @@
 #define MLX5E_PARAMS_DEFAULT_LOG_RQ_SIZE                0xa
 #define MLX5E_PARAMS_MAXIMUM_LOG_RQ_SIZE                0xd
 
-#define MLX5E_PARAMS_MINIMUM_LOG_RQ_SIZE_MPW            0x1
+#define MLX5E_PARAMS_MINIMUM_LOG_RQ_SIZE_MPW            0x2
 #define MLX5E_PARAMS_DEFAULT_LOG_RQ_SIZE_MPW            0x3
 #define MLX5E_PARAMS_MAXIMUM_LOG_RQ_SIZE_MPW            0x6
 
-- 
2.14.2

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero
  2017-11-08  7:21 ` [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero Saeed Mahameed
@ 2017-11-08 14:13   ` Or Gerlitz
  2017-11-09  7:43     ` Kamal Heib
  0 siblings, 1 reply; 15+ messages in thread
From: Or Gerlitz @ 2017-11-08 14:13 UTC (permalink / raw)
  To: Yevgeny Kliteynik, Kamal Heib
  Cc: Saeed Mahameed, Linux Netdev List, David Miller

On Wed, Nov 8, 2017 at 4:21 PM, Saeed Mahameed <saeedm@mellanox.com> wrote:
> From: Kamal Heib <kamalh@mellanox.com>
>
> In the current code, if a size of zero is passed to
> mlx5_fpga_mem_{read|write}_i2c() functions the "err"

Don't we need to fix the call site where zero size is provided and not
in called function?

Isn't sending down a zero size a sign for a bug which we are not fixing?

> return value will not initialized.
>
> Fixes: a9956d35d199 ('net/mlx5: FPGA, Add SBU infrastructure')
> Signed-off-by: Kamal Heib <kamalh@mellanox.com>
> Reviewed-by: Yevgeny Kliteynik <kliteyn@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net 2/6] net/mlx5: Cancel health poll before sending panic teardown command
  2017-11-08  7:21 ` [net 2/6] net/mlx5: Cancel health poll before sending panic teardown command Saeed Mahameed
@ 2017-11-08 14:28   ` Or Gerlitz
  0 siblings, 0 replies; 15+ messages in thread
From: Or Gerlitz @ 2017-11-08 14:28 UTC (permalink / raw)
  To: Saeed Mahameed, Majd Dibbiny
  Cc: David S. Miller, Linux Netdev List, Huy Nguyen

On Wed, Nov 8, 2017 at 4:21 PM, Saeed Mahameed <saeedm@mellanox.com> wrote:
> From: Huy Nguyen <huyn@mellanox.com>
>
> After the panic teardown firmware command, health_care detects the error
> in PCI bus and calls the mlx5_pci_err_detected. This health_care flow is
> no longer needed because the panic teardown firmware command will bring
> down the PCI bus communication with the HCA.
>
> The solution is to cancel the health care timer and its pending
> workqueue request before sending panic teardown firmware command.
>
> Kernel trace:
> mlx5_core 0033:01:00.0: Shutdown was called
> mlx5_core 0033:01:00.0: health_care:154:(pid 9304): handling bad device here
> mlx5_core 0033:01:00.0: mlx5_handle_bad_state:114:(pid 9304): NIC state 1
> mlx5_core 0033:01:00.0: mlx5_pci_err_detected was called
> mlx5_core 0033:01:00.0: mlx5_enter_error_state:96:(pid 9304): start
> mlx5_3:mlx5_ib_event:3061:(pid 9304): warning: event on port 0
> mlx5_core 0033:01:00.0: mlx5_enter_error_state:104:(pid 9304): end
> Unable to handle kernel paging request for data at address 0x0000003f
> Faulting instruction address: 0xc0080000434b8c80
>
> Fixes: 8812c24d28f4 ('net/mlx5: Add fast unload support in shutdown flow')
> Signed-off-by: Huy Nguyen <huyn@mellanox.com>
> Reviewed-by: Majd Dibbiny <majd@mellanox.com>

something might have went wrong here with the reviewer, we are
checking that internally

> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero
  2017-11-08 14:13   ` Or Gerlitz
@ 2017-11-09  7:43     ` Kamal Heib
  2017-11-09  9:12       ` Or Gerlitz
  2017-11-09  9:13       ` Or Gerlitz
  0 siblings, 2 replies; 15+ messages in thread
From: Kamal Heib @ 2017-11-09  7:43 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: Saeed Mahameed, Linux Netdev List, David Miller, Yevgeny Kliteynik

On Wed, 2017-11-08 at 23:13 +0900, Or Gerlitz wrote:
> On Wed, Nov 8, 2017 at 4:21 PM, Saeed Mahameed <saeedm@mellanox.com>
> wrote:
> > From: Kamal Heib <kamalh@mellanox.com>
> > 
> > In the current code, if a size of zero is passed to
> > mlx5_fpga_mem_{read|write}_i2c() functions the "err"
> 
> Don't we need to fix the call site where zero size is provided and
> not
> in called function?
> 
Isn't sending down a zero size a sign for a bug which we are not
fixing?
> 
Both functions are called from an exported symbols. so I think the size
validation should be within this two functions just like the case of
checking that mdev isn't set.


> > return value will not initialized.
> > 
> > Fixes: a9956d35d199 ('net/mlx5: FPGA, Add SBU infrastructure')
> > Signed-off-by: Kamal Heib <kamalh@mellanox.com>
> > Reviewed-by: Yevgeny Kliteynik <kliteyn@mellanox.com>
> > Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero
  2017-11-09  7:43     ` Kamal Heib
@ 2017-11-09  9:12       ` Or Gerlitz
  2017-11-10  6:13         ` Saeed Mahameed
  2017-11-09  9:13       ` Or Gerlitz
  1 sibling, 1 reply; 15+ messages in thread
From: Or Gerlitz @ 2017-11-09  9:12 UTC (permalink / raw)
  To: Kamal Heib
  Cc: Saeed Mahameed, Linux Netdev List, David Miller, Yevgeny Kliteynik

On Thu, Nov 9, 2017 at 4:43 PM, Kamal Heib <kamalh@mellanox.com> wrote:
> On Wed, 2017-11-08 at 23:13 +0900, Or Gerlitz wrote:
>> On Wed, Nov 8, 2017 at 4:21 PM, Saeed Mahameed <saeedm@mellanox.com>
>> wrote:
>> > From: Kamal Heib <kamalh@mellanox.com>
>> >
>> > In the current code, if a size of zero is passed to
>> > mlx5_fpga_mem_{read|write}_i2c() functions the "err"
>>
>> Don't we need to fix the call site where zero size is provided and
>> not
>> in called function?
>>
> Isn't sending down a zero size a sign for a bug which we are not
> fixing?
>>
> Both functions are called from an exported symbols. so I think the size
> validation should be within this two functions just like the case of
> checking that mdev isn't set.

mmm, I see exported to who exactly? how are they being called, by func pointer?
can you point to the call sites?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero
  2017-11-09  7:43     ` Kamal Heib
  2017-11-09  9:12       ` Or Gerlitz
@ 2017-11-09  9:13       ` Or Gerlitz
  1 sibling, 0 replies; 15+ messages in thread
From: Or Gerlitz @ 2017-11-09  9:13 UTC (permalink / raw)
  To: Kamal Heib
  Cc: Saeed Mahameed, Linux Netdev List, David Miller, Yevgeny Kliteynik

On Thu, Nov 9, 2017 at 4:43 PM, Kamal Heib <kamalh@mellanox.com> wrote:
> On Wed, 2017-11-08 at 23:13 +0900, Or Gerlitz wrote:
>> On Wed, Nov 8, 2017 at 4:21 PM, Saeed Mahameed <saeedm@mellanox.com>
>> wrote:
>> > From: Kamal Heib <kamalh@mellanox.com>
>> >
>> > In the current code, if a size of zero is passed to
>> > mlx5_fpga_mem_{read|write}_i2c() functions the "err"
>>
>> Don't we need to fix the call site where zero size is provided and
>> not
>> in called function?
>>
> Isn't sending down a zero size a sign for a bug which we are not
> fixing?
>>
> Both functions are called from an exported symbols. so I think the size
> validation should be within this two functions just like the case of
> checking that mdev isn't set.

Note that the kernel trust model doesn't enforce you to check
everything as you go.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero
  2017-11-09  9:12       ` Or Gerlitz
@ 2017-11-10  6:13         ` Saeed Mahameed
  2017-11-10  6:23           ` Or Gerlitz
  0 siblings, 1 reply; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-10  6:13 UTC (permalink / raw)
  To: Or Gerlitz, Kamal Heib; +Cc: Linux Netdev List, David Miller, Yevgeny Kliteynik

On Thu, 2017-11-09 at 18:12 +0900, Or Gerlitz wrote:
> On Thu, Nov 9, 2017 at 4:43 PM, Kamal Heib <kamalh@mellanox.com>
> wrote:
> > On Wed, 2017-11-08 at 23:13 +0900, Or Gerlitz wrote:
> > > On Wed, Nov 8, 2017 at 4:21 PM, Saeed Mahameed <saeedm@mellanox.c
> > > om>
> > > wrote:
> > > > From: Kamal Heib <kamalh@mellanox.com>
> > > > 
> > > > In the current code, if a size of zero is passed to
> > > > mlx5_fpga_mem_{read|write}_i2c() functions the "err"
> > > 
> > > Don't we need to fix the call site where zero size is provided
> > > and
> > > not
> > > in called function?
> > > 
> > 
> > Isn't sending down a zero size a sign for a bug which we are not
> > fixing?
> > > 
> > 
> > Both functions are called from an exported symbols. so I think the
> > size
> > validation should be within this two functions just like the case
> > of
> > checking that mdev isn't set.
> 
> mmm, I see exported to who exactly? how are they being called, by
> func pointer?
> can you point to the call sites?

Or, are you ok with this patch ? I would like to post V2 with the
reviewed-by tag fix.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero
  2017-11-10  6:13         ` Saeed Mahameed
@ 2017-11-10  6:23           ` Or Gerlitz
  2017-11-10  6:37             ` Saeed Mahameed
  0 siblings, 1 reply; 15+ messages in thread
From: Or Gerlitz @ 2017-11-10  6:23 UTC (permalink / raw)
  To: Saeed Mahameed
  Cc: Kamal Heib, Linux Netdev List, David Miller, Yevgeny Kliteynik

On Fri, Nov 10, 2017 at 3:13 PM, Saeed Mahameed <saeedm@mellanox.com> wrote:
> On Thu, 2017-11-09 at 18:12 +0900, Or Gerlitz wrote:
>> On Thu, Nov 9, 2017 at 4:43 PM, Kamal Heib <kamalh@mellanox.com>
>> wrote:
>> > On Wed, 2017-11-08 at 23:13 +0900, Or Gerlitz wrote:
>> > > On Wed, Nov 8, 2017 at 4:21 PM, Saeed Mahameed <saeedm@mellanox.c
>> > > om>
>> > > wrote:
>> > > > From: Kamal Heib <kamalh@mellanox.com>
>> > > >
>> > > > In the current code, if a size of zero is passed to
>> > > > mlx5_fpga_mem_{read|write}_i2c() functions the "err"
>> > >
>> > > Don't we need to fix the call site where zero size is provided
>> > > and
>> > > not
>> > > in called function?
>> > >
>> >
>> > Isn't sending down a zero size a sign for a bug which we are not
>> > fixing?
>> > >
>> >
>> > Both functions are called from an exported symbols. so I think the
>> > size
>> > validation should be within this two functions just like the case
>> > of
>> > checking that mdev isn't set.
>>
>> mmm, I see exported to who exactly? how are they being called, by
>> func pointer?
>> can you point to the call sites?
>
> Or, are you ok with this patch ? I would like to post V2 with the
> reviewed-by tag fix.

The RB tag issue was on another patch.. for this patch I realized after talking
to the author that it comes to fix a build warning. I would be happy
if we can clarify
that in the change log.

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero
  2017-11-10  6:23           ` Or Gerlitz
@ 2017-11-10  6:37             ` Saeed Mahameed
  0 siblings, 0 replies; 15+ messages in thread
From: Saeed Mahameed @ 2017-11-10  6:37 UTC (permalink / raw)
  To: Or Gerlitz; +Cc: Kamal Heib, Linux Netdev List, David Miller, Yevgeny Kliteynik

On Fri, 2017-11-10 at 15:23 +0900, Or Gerlitz wrote:
> On Fri, Nov 10, 2017 at 3:13 PM, Saeed Mahameed <saeedm@mellanox.com>
> wrote:
> > On Thu, 2017-11-09 at 18:12 +0900, Or Gerlitz wrote:
> > > On Thu, Nov 9, 2017 at 4:43 PM, Kamal Heib <kamalh@mellanox.com>
> > > wrote:
> > > > On Wed, 2017-11-08 at 23:13 +0900, Or Gerlitz wrote:
> > > > > On Wed, Nov 8, 2017 at 4:21 PM, Saeed Mahameed <saeedm@mellan
> > > > > ox.c
> > > > > om>
> > > > > wrote:
> > > > > > From: Kamal Heib <kamalh@mellanox.com>
> > > > > > 
> > > > > > In the current code, if a size of zero is passed to
> > > > > > mlx5_fpga_mem_{read|write}_i2c() functions the "err"
> > > > > 
> > > > > Don't we need to fix the call site where zero size is
> > > > > provided
> > > > > and
> > > > > not
> > > > > in called function?
> > > > > 
> > > > 
> > > > Isn't sending down a zero size a sign for a bug which we are
> > > > not
> > > > fixing?
> > > > > 
> > > > 
> > > > Both functions are called from an exported symbols. so I think
> > > > the
> > > > size
> > > > validation should be within this two functions just like the
> > > > case
> > > > of
> > > > checking that mdev isn't set.
> > > 
> > > mmm, I see exported to who exactly? how are they being called, by
> > > func pointer?
> > > can you point to the call sites?
> > 
> > Or, are you ok with this patch ? I would like to post V2 with the
> > reviewed-by tag fix.
> 
> The RB tag issue was on another patch.. for this patch I realized
> after talking
> to the author that it comes to fix a build warning. I would be happy
> if we can clarify
> that in the change log.


Ok I will drop this patch until the author provides the missing
information.

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-11-10  6:38 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-08  7:21 [pull request][net 0/6] Mellanox, mlx5 fixes 2017-11-08 Saeed Mahameed
2017-11-08  7:21 ` [net 1/6] net/mlx5: Loop over temp list to release delay events Saeed Mahameed
2017-11-08  7:21 ` [net 2/6] net/mlx5: Cancel health poll before sending panic teardown command Saeed Mahameed
2017-11-08 14:28   ` Or Gerlitz
2017-11-08  7:21 ` [net 3/6] net/mlx5: FPGA, return -EINVAL if size is zero Saeed Mahameed
2017-11-08 14:13   ` Or Gerlitz
2017-11-09  7:43     ` Kamal Heib
2017-11-09  9:12       ` Or Gerlitz
2017-11-10  6:13         ` Saeed Mahameed
2017-11-10  6:23           ` Or Gerlitz
2017-11-10  6:37             ` Saeed Mahameed
2017-11-09  9:13       ` Or Gerlitz
2017-11-08  7:21 ` [net 4/6] net/mlx5e: Fix napi poll with zero budget Saeed Mahameed
2017-11-08  7:21 ` [net 5/6] net/mlx5e: Set page to null in case dma mapping fails Saeed Mahameed
2017-11-08  7:21 ` [net 6/6] net/mlx5e: Increase Striding RQ minimum size limit to 4 multi-packet WQEs Saeed Mahameed

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.