All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] net/qede: fix to get vendor/device id info
@ 2016-12-03  6:35 Rasesh Mody
  2016-12-03  6:35 ` [PATCH 2/8] net/qede/base: fix sriov printouts Rasesh Mody
                   ` (7 more replies)
  0 siblings, 8 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-03  6:35 UTC (permalink / raw)
  To: dev; +Cc: Dept-EngDPDKDev, Rasesh Mody

Fixes: ec94dbc5 ("qede: add base driver")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/qede_ethdev.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 9c2a5eae..b9a325df 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -2071,6 +2071,10 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 
+	/* @DPDK */
+	edev->vendor_id = pci_dev->id.vendor_id;
+	edev->device_id = pci_dev->id.device_id;
+
 	qed_ops = qed_get_eth_ops();
 	if (!qed_ops) {
 		DP_ERR(edev, "Failed to get qed_eth_ops_pass\n");
-- 
2.11.0.rc1

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

* [PATCH 2/8] net/qede/base: fix sriov printouts
  2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
@ 2016-12-03  6:35 ` Rasesh Mody
  2016-12-03  6:35 ` [PATCH 3/8] net/qede/base: fix multiple acquisition requests by VF Rasesh Mody
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-03  6:35 UTC (permalink / raw)
  To: dev; +Cc: Dept-EngDPDKDev, Rasesh Mody

Remove unmeaningful value in print.

Don't print the number of Multicast filters as part of Acquire response,
as this is an obsolete field which isn't enforced by PF.

Fixes: 86a2265e ("qede: add SRIOV support")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_sriov.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/qede/base/ecore_sriov.c b/drivers/net/qede/base/ecore_sriov.c
index b28d7281..de54b9a0 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -317,10 +317,9 @@ static enum _ecore_status_t ecore_iov_pci_cfg_info(struct ecore_dev *p_dev)
 
 	OSAL_PCI_READ_CONFIG_BYTE(p_dev, pos + PCI_SRIOV_FUNC_LINK, &iov->link);
 
-	DP_VERBOSE(p_dev, ECORE_MSG_IOV, "IOV info[%d]: nres %d, cap 0x%x,"
+	DP_VERBOSE(p_dev, ECORE_MSG_IOV, "IOV info: nres %d, cap 0x%x,"
 		   "ctrl 0x%x, total %d, initial %d, num vfs %d, offset %d,"
-		   " stride %d, page size 0x%x\n", 0,
-		   /* @@@TBD MichalK - function id */
+		   " stride %d, page size 0x%x\n",
 		   iov->nres, iov->cap, iov->ctrl,
 		   iov->total_vfs, iov->initial_vfs, iov->nr_virtfn,
 		   iov->offset, iov->stride, iov->pgsz);
@@ -1575,12 +1574,12 @@ static void ecore_iov_vf_mbx_acquire(struct ecore_hwfn       *p_hwfn,
 		   "VF[%d] ACQUIRE_RESPONSE: pfdev_info- chip_num=0x%x,"
 		   " db_size=%d, idx_per_sb=%d, pf_cap=0x%lx\n"
 		   "resources- n_rxq-%d, n_txq-%d, n_sbs-%d, n_macs-%d,"
-		   " n_vlans-%d, n_mcs-%d\n",
+		   " n_vlans-%d\n",
 		   vf->abs_vf_id, resp->pfdev_info.chip_num,
 		   resp->pfdev_info.db_size, resp->pfdev_info.indices_per_sb,
 		   (unsigned long)resp->pfdev_info.capabilities, resc->num_rxqs,
 		   resc->num_txqs, resc->num_sbs, resc->num_mac_filters,
-		   resc->num_vlan_filters, resc->num_mc_filters);
+		   resc->num_vlan_filters);
 
 	vf->state = VF_ACQUIRED;
 
-- 
2.11.0.rc1

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

* [PATCH 3/8] net/qede/base: fix multiple acquisition requests by VF
  2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
  2016-12-03  6:35 ` [PATCH 2/8] net/qede/base: fix sriov printouts Rasesh Mody
@ 2016-12-03  6:35 ` Rasesh Mody
  2016-12-03  6:35 ` [PATCH 4/8] net/qede/base: fix error return code in resc alloc Rasesh Mody
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-03  6:35 UTC (permalink / raw)
  To: dev; +Cc: Dept-EngDPDKDev, Rasesh Mody

There are certain conditions under which VF would infinitely send
ACQUIRE messages, as it will fail to understand that PF has rejected
the ACQUIRE request. Fix to reject multiple acquisition requests by VF.

Fixes: 22d07d93 ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_vf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/qede/base/ecore_vf.c b/drivers/net/qede/base/ecore_vf.c
index be8b1ec4..161b3176 100644
--- a/drivers/net/qede/base/ecore_vf.c
+++ b/drivers/net/qede/base/ecore_vf.c
@@ -296,6 +296,14 @@ static enum _ecore_status_t ecore_vf_pf_acquire(struct ecore_hwfn *p_hwfn)
 						VFPF_ACQUIRE_CAP_PRE_FP_HSI;
 				}
 			}
+
+			/* If PF/VF are using same Major, PF must have had
+			 * it's reasons. Simply fail.
+			 */
+			DP_NOTICE(p_hwfn, false,
+				  "PF rejected acquisition by VF\n");
+			rc = ECORE_INVAL;
+			goto exit;
 		} else {
 			DP_ERR(p_hwfn,
 			       "PF returned err %d to VF acquisition request\n",
-- 
2.11.0.rc1

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

* [PATCH 4/8] net/qede/base: fix error return code in resc alloc
  2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
  2016-12-03  6:35 ` [PATCH 2/8] net/qede/base: fix sriov printouts Rasesh Mody
  2016-12-03  6:35 ` [PATCH 3/8] net/qede/base: fix multiple acquisition requests by VF Rasesh Mody
@ 2016-12-03  6:35 ` Rasesh Mody
  2016-12-03  6:35 ` [PATCH 5/8] net/qede/base: fix mutex in freeing context manager Rasesh Mody
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-03  6:35 UTC (permalink / raw)
  To: dev; +Cc: Dept-EngDPDKDev, Rasesh Mody

Fix to return error code ECORE_INVAL instead of 0 when EQ elements
is too large as done elsewhere in this function.

Fixes: 22d07d93 ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_dev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/qede/base/ecore_dev.c b/drivers/net/qede/base/ecore_dev.c
index 6060f9ee..58b93877 100644
--- a/drivers/net/qede/base/ecore_dev.c
+++ b/drivers/net/qede/base/ecore_dev.c
@@ -667,6 +667,7 @@ enum _ecore_status_t ecore_resc_alloc(struct ecore_dev *p_dev)
 			DP_ERR(p_hwfn, "Cannot allocate 0x%x EQ elements."
 				       "The maximum of a u16 chain is 0x%x\n",
 			       n_eqes, 0xFFFF);
+			rc = ECORE_INVAL;
 			goto alloc_err;
 		}
 
-- 
2.11.0.rc1

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

* [PATCH 5/8] net/qede/base: fix mutex in freeing context manager
  2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
                   ` (2 preceding siblings ...)
  2016-12-03  6:35 ` [PATCH 4/8] net/qede/base: fix error return code in resc alloc Rasesh Mody
@ 2016-12-03  6:35 ` Rasesh Mody
  2016-12-03  6:35 ` [PATCH 6/8] net/qede/base: fix Rx queue access by malicious VFs Rasesh Mody
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-03  6:35 UTC (permalink / raw)
  To: dev; +Cc: Dept-EngDPDKDev, Rasesh Mody

Fix OSAL_MUTEX_DEALLOC() in freeing the context manager.

Fixes: 22d07d93 ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_cxt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/qede/base/ecore_cxt.c b/drivers/net/qede/base/ecore_cxt.c
index 3dd953d9..5ea4f5cd 100644
--- a/drivers/net/qede/base/ecore_cxt.c
+++ b/drivers/net/qede/base/ecore_cxt.c
@@ -1155,7 +1155,7 @@ void ecore_cxt_mngr_free(struct ecore_hwfn *p_hwfn)
 	ecore_cid_map_free(p_hwfn);
 	ecore_cxt_src_t2_free(p_hwfn);
 	ecore_ilt_shadow_free(p_hwfn);
-	OSAL_MUTEX_DEALLOC(&p_mngr->mutex);
+	OSAL_MUTEX_DEALLOC(&p_hwfn->p_cxt_mngr->mutex);
 	OSAL_FREE(p_hwfn->p_dev, p_hwfn->p_cxt_mngr);
 
 	p_hwfn->p_cxt_mngr = OSAL_NULL;
-- 
2.11.0.rc1

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

* [PATCH 6/8] net/qede/base: fix Rx queue access by malicious VFs
  2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
                   ` (3 preceding siblings ...)
  2016-12-03  6:35 ` [PATCH 5/8] net/qede/base: fix mutex in freeing context manager Rasesh Mody
@ 2016-12-03  6:35 ` Rasesh Mody
  2016-12-03  6:35 ` [PATCH 7/8] net/qede/base: fix to handle acquire request from VF Rasesh Mody
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-03  6:35 UTC (permalink / raw)
  To: dev; +Cc: Dept-EngDPDKDev, Rasesh Mody

8.10.x.x was supposed to prevent malicious VFs from using out-of-bound
queue indices, but apparently Rx queue access is still done prior to the
index being validated by PF.

Fixes: 98bc693e ("net/qede/base: change queue start")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_sriov.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/qede/base/ecore_sriov.c b/drivers/net/qede/base/ecore_sriov.c
index de54b9a0..12552966 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -1968,6 +1968,11 @@ static void ecore_iov_vf_mbx_start_rxq(struct ecore_hwfn *p_hwfn,
 	enum _ecore_status_t rc;
 
 	req = &mbx->req_virt->start_rxq;
+
+	if (!ecore_iov_validate_rxq(p_hwfn, vf, req->rx_qid) ||
+	    !ecore_iov_validate_sb(p_hwfn, vf, req->hw_sb))
+		goto out;
+
 	OSAL_MEMSET(&p_params, 0, sizeof(p_params));
 	p_params.queue_id = (u8)vf->vf_queues[req->rx_qid].fw_rx_qid;
 	p_params.vf_qid = req->rx_qid;
@@ -1976,10 +1981,6 @@ static void ecore_iov_vf_mbx_start_rxq(struct ecore_hwfn *p_hwfn,
 	p_params.sb = req->hw_sb;
 	p_params.sb_idx = req->sb_index;
 
-	if (!ecore_iov_validate_rxq(p_hwfn, vf, req->rx_qid) ||
-	    !ecore_iov_validate_sb(p_hwfn, vf, req->hw_sb))
-		goto out;
-
 	/* Legacy VFs have their Producers in a different location, which they
 	 * calculate on their own and clean the producer prior to this.
 	 */
-- 
2.11.0.rc1

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

* [PATCH 7/8] net/qede/base: fix to handle acquire request from VF
  2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
                   ` (4 preceding siblings ...)
  2016-12-03  6:35 ` [PATCH 6/8] net/qede/base: fix Rx queue access by malicious VFs Rasesh Mody
@ 2016-12-03  6:35 ` Rasesh Mody
  2016-12-03  6:35 ` [PATCH 8/8] net/qede/base: fix to check FW override after rejection Rasesh Mody
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
  7 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-03  6:35 UTC (permalink / raw)
  To: dev; +Cc: Dept-EngDPDKDev, Rasesh Mody

Add a check and fail the VF's probe request if VF is already in
VF_ACQUIRED state.

Fixes: 22d07d93 ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_sriov.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/qede/base/ecore_sriov.c b/drivers/net/qede/base/ecore_sriov.c
index 12552966..c2fbee87 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -1459,6 +1459,18 @@ static void ecore_iov_vf_mbx_acquire(struct ecore_hwfn       *p_hwfn,
 	pfdev_info->major_fp_hsi = ETH_HSI_VER_MAJOR;
 	pfdev_info->minor_fp_hsi = ETH_HSI_VER_MINOR;
 
+	/* TODO - not doing anything is bad since we'll assert, but this isn't
+	 * necessarily the right behavior - perhaps we should have allowed some
+	 * versatility here.
+	 */
+	if (vf->state != VF_FREE &&
+	    vf->state != VF_STOPPED) {
+		DP_VERBOSE(p_hwfn, ECORE_MSG_IOV,
+			   "VF[%d] sent ACQUIRE but is already in state %d - fail request\n",
+			   vf->abs_vf_id, vf->state);
+		goto out;
+	}
+
 	/* Validate FW compatibility */
 	if (req->vfdev_info.eth_fp_hsi_major != ETH_HSI_VER_MAJOR) {
 		if (req->vfdev_info.capabilities &
-- 
2.11.0.rc1

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

* [PATCH 8/8] net/qede/base: fix to check FW override after rejection
  2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
                   ` (5 preceding siblings ...)
  2016-12-03  6:35 ` [PATCH 7/8] net/qede/base: fix to handle acquire request from VF Rasesh Mody
@ 2016-12-03  6:35 ` Rasesh Mody
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
  7 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-03  6:35 UTC (permalink / raw)
  To: dev; +Cc: Dept-EngDPDKDev, Rasesh Mody

Apparently VF over Legacy PF doesn't work, as VF would fail after
getting the initial rejection message [instead of sending an additional
one where it asks for a FW override and see if it works].

Fixes: 22d07d93 ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_vf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/qede/base/ecore_vf.c b/drivers/net/qede/base/ecore_vf.c
index 161b3176..c26b602b 100644
--- a/drivers/net/qede/base/ecore_vf.c
+++ b/drivers/net/qede/base/ecore_vf.c
@@ -294,6 +294,7 @@ static enum _ecore_status_t ecore_vf_pf_acquire(struct ecore_hwfn *p_hwfn)
 						" override\n");
 					req->vfdev_info.capabilities |=
 						VFPF_ACQUIRE_CAP_PRE_FP_HSI;
+					continue;
 				}
 			}
 
-- 
2.11.0.rc1

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

* Re: [PATCH 1/8] net/qede: fix to get vendor/device id info
  2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
                   ` (6 preceding siblings ...)
  2016-12-03  6:35 ` [PATCH 8/8] net/qede/base: fix to check FW override after rejection Rasesh Mody
@ 2016-12-09 13:35 ` Ferruh Yigit
  2016-12-20 23:25   ` Mody, Rasesh
                     ` (8 more replies)
  7 siblings, 9 replies; 19+ messages in thread
From: Ferruh Yigit @ 2016-12-09 13:35 UTC (permalink / raw)
  To: Rasesh Mody, dev; +Cc: Dept-EngDPDKDev

Hi Rasesh,

On 12/3/2016 6:35 AM, Rasesh Mody wrote:
> Fixes: ec94dbc5 ("qede: add base driver")
> 
> Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>

Related to the commit logs of this patchset. Many people, including me,
won't know technical details of your driver as you do.

>From below patch, I also can see vendor_id and device_id get, which were
not there before. But I have no clue why? Or what happens when you don't
have them, or what works fine when you have them.
Overall a little context helps a lot to understand what is really fixed,
and what happens if not fixed.

For example in patch 8/8, it is a one line easy modification :), but
there is no way that I can understand what really it does, but commit
log comes to help there, and describes it really fixes VF over legacy
PF, by VF asking a FW overwrite instead of failing on PF reject msg. So
patch subject can be here: fix VF over legacy PF.

Please trying to provide more context on fixes.


Also you need to add CC:stable@dpdk.org to commit log, so that git
send-email ensures this fixes also sent to stable trees.


Thanks,

> ---
>  drivers/net/qede/qede_ethdev.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index 9c2a5eae..b9a325df 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -2071,6 +2071,10 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
>  
>  	rte_eth_copy_pci_info(eth_dev, pci_dev);
>  
> +	/* @DPDK */
> +	edev->vendor_id = pci_dev->id.vendor_id;
> +	edev->device_id = pci_dev->id.device_id;
> +
>  	qed_ops = qed_get_eth_ops();
>  	if (!qed_ops) {
>  		DP_ERR(edev, "Failed to get qed_eth_ops_pass\n");
> 

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

* Re: [PATCH 1/8] net/qede: fix to get vendor/device id info
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
@ 2016-12-20 23:25   ` Mody, Rasesh
  2016-12-23  0:49   ` [PATCH v2 " Rasesh Mody
                     ` (7 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Mody, Rasesh @ 2016-12-20 23:25 UTC (permalink / raw)
  To: Ferruh Yigit, dev; +Cc: Dept-Eng DPDK Dev

Hi Ferruh,

> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Friday, December 09, 2016 5:36 AM
> 
> Hi Rasesh,
> 
> On 12/3/2016 6:35 AM, Rasesh Mody wrote:
> > Fixes: ec94dbc5 ("qede: add base driver")
> >
> > Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
> 
> Related to the commit logs of this patchset. Many people, including me,
> won't know technical details of your driver as you do.
> 
> From below patch, I also can see vendor_id and device_id get, which were
> not there before. But I have no clue why? Or what happens when you don't
> have them, or what works fine when you have them.
> Overall a little context helps a lot to understand what is really fixed, and what
> happens if not fixed.

The vendor_id and device_id are used to determine device type. If you don't have them, then check for determining device type fails and is always set to default device type.

Hope this helps.

> For example in patch 8/8, it is a one line easy modification :), but there is no
> way that I can understand what really it does, but commit log comes to help
> there, and describes it really fixes VF over legacy PF, by VF asking a FW
> overwrite instead of failing on PF reject msg. So patch subject can be here:
> fix VF over legacy PF.

Sounds good, will reword the subject.

> Please trying to provide more context on fixes.
> 
> 
> Also you need to add CC:stable@dpdk.org to commit log, so that git send-
> email ensures this fixes also sent to stable trees.

Will take care of this in v2 submission.

Thanks!
Rasesh

> 
> 
> Thanks,
> 
> > ---
> >  drivers/net/qede/qede_ethdev.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/net/qede/qede_ethdev.c
> > b/drivers/net/qede/qede_ethdev.c index 9c2a5eae..b9a325df 100644
> > --- a/drivers/net/qede/qede_ethdev.c
> > +++ b/drivers/net/qede/qede_ethdev.c
> > @@ -2071,6 +2071,10 @@ static int qede_common_dev_init(struct
> > rte_eth_dev *eth_dev, bool is_vf)
> >
> >  	rte_eth_copy_pci_info(eth_dev, pci_dev);
> >
> > +	/* @DPDK */
> > +	edev->vendor_id = pci_dev->id.vendor_id;
> > +	edev->device_id = pci_dev->id.device_id;
> > +
> >  	qed_ops = qed_get_eth_ops();
> >  	if (!qed_ops) {
> >  		DP_ERR(edev, "Failed to get qed_eth_ops_pass\n");
> >

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

* [PATCH v2 1/8] net/qede: fix to get vendor/device id info
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
  2016-12-20 23:25   ` Mody, Rasesh
@ 2016-12-23  0:49   ` Rasesh Mody
  2016-12-23 15:26     ` Ferruh Yigit
  2016-12-23  0:49   ` [PATCH v2 2/8] net/qede/base: fix sriov printouts Rasesh Mody
                     ` (6 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Rasesh Mody @ 2016-12-23  0:49 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: Rasesh Mody, dev, stable, Dept-EngDPDKDev

The vendor_id and device_id are used to determine device type. If you
don't have them, then check for determining device type fails and is
always set to default device type.

Fixes: ec94dbc57362 ("qede: add base driver")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/qede_ethdev.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 2c600c1..4943358 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1422,6 +1422,10 @@ static int qede_common_dev_init(struct rte_eth_dev *eth_dev, bool is_vf)
 
 	rte_eth_copy_pci_info(eth_dev, pci_dev);
 
+	/* @DPDK */
+	edev->vendor_id = pci_dev->id.vendor_id;
+	edev->device_id = pci_dev->id.device_id;
+
 	qed_ops = qed_get_eth_ops();
 	if (!qed_ops) {
 		DP_ERR(edev, "Failed to get qed_eth_ops_pass\n");
-- 
1.7.10.3

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

* [PATCH v2 2/8] net/qede/base: fix sriov printouts
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
  2016-12-20 23:25   ` Mody, Rasesh
  2016-12-23  0:49   ` [PATCH v2 " Rasesh Mody
@ 2016-12-23  0:49   ` Rasesh Mody
  2016-12-23  0:49   ` [PATCH v2 3/8] net/qede/base: fix multiple acquisition requests by VF Rasesh Mody
                     ` (5 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-23  0:49 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: Rasesh Mody, dev, stable, Dept-EngDPDKDev

Remove unmeaningful function ID value in print.

Don't print the number of Multicast filters as part of Acquire response,
as this is an obsolete field which isn't enforced by PF.

Fixes: 86a2265e59d7 ("qede: add SRIOV support")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_sriov.c |    9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/qede/base/ecore_sriov.c b/drivers/net/qede/base/ecore_sriov.c
index b28d728..de54b9a 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -317,10 +317,9 @@ static enum _ecore_status_t ecore_iov_pci_cfg_info(struct ecore_dev *p_dev)
 
 	OSAL_PCI_READ_CONFIG_BYTE(p_dev, pos + PCI_SRIOV_FUNC_LINK, &iov->link);
 
-	DP_VERBOSE(p_dev, ECORE_MSG_IOV, "IOV info[%d]: nres %d, cap 0x%x,"
+	DP_VERBOSE(p_dev, ECORE_MSG_IOV, "IOV info: nres %d, cap 0x%x,"
 		   "ctrl 0x%x, total %d, initial %d, num vfs %d, offset %d,"
-		   " stride %d, page size 0x%x\n", 0,
-		   /* @@@TBD MichalK - function id */
+		   " stride %d, page size 0x%x\n",
 		   iov->nres, iov->cap, iov->ctrl,
 		   iov->total_vfs, iov->initial_vfs, iov->nr_virtfn,
 		   iov->offset, iov->stride, iov->pgsz);
@@ -1575,12 +1574,12 @@ static void ecore_iov_vf_mbx_acquire(struct ecore_hwfn       *p_hwfn,
 		   "VF[%d] ACQUIRE_RESPONSE: pfdev_info- chip_num=0x%x,"
 		   " db_size=%d, idx_per_sb=%d, pf_cap=0x%lx\n"
 		   "resources- n_rxq-%d, n_txq-%d, n_sbs-%d, n_macs-%d,"
-		   " n_vlans-%d, n_mcs-%d\n",
+		   " n_vlans-%d\n",
 		   vf->abs_vf_id, resp->pfdev_info.chip_num,
 		   resp->pfdev_info.db_size, resp->pfdev_info.indices_per_sb,
 		   (unsigned long)resp->pfdev_info.capabilities, resc->num_rxqs,
 		   resc->num_txqs, resc->num_sbs, resc->num_mac_filters,
-		   resc->num_vlan_filters, resc->num_mc_filters);
+		   resc->num_vlan_filters);
 
 	vf->state = VF_ACQUIRED;
 
-- 
1.7.10.3

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

* [PATCH v2 3/8] net/qede/base: fix multiple acquisition requests by VF
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
                     ` (2 preceding siblings ...)
  2016-12-23  0:49   ` [PATCH v2 2/8] net/qede/base: fix sriov printouts Rasesh Mody
@ 2016-12-23  0:49   ` Rasesh Mody
  2016-12-23  0:50   ` [PATCH v2 4/8] net/qede/base: fix error return code in resc alloc Rasesh Mody
                     ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-23  0:49 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: Rasesh Mody, dev, stable, Dept-EngDPDKDev

There are certain conditions under which VF would infinitely send
ACQUIRE messages, as it will fail to understand that PF has rejected
the ACQUIRE request. Fix to reject multiple acquisition requests by VF.

Fixes: 22d07d939c3c ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_vf.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/qede/base/ecore_vf.c b/drivers/net/qede/base/ecore_vf.c
index be8b1ec..161b317 100644
--- a/drivers/net/qede/base/ecore_vf.c
+++ b/drivers/net/qede/base/ecore_vf.c
@@ -296,6 +296,14 @@ static enum _ecore_status_t ecore_vf_pf_acquire(struct ecore_hwfn *p_hwfn)
 						VFPF_ACQUIRE_CAP_PRE_FP_HSI;
 				}
 			}
+
+			/* If PF/VF are using same Major, PF must have had
+			 * it's reasons. Simply fail.
+			 */
+			DP_NOTICE(p_hwfn, false,
+				  "PF rejected acquisition by VF\n");
+			rc = ECORE_INVAL;
+			goto exit;
 		} else {
 			DP_ERR(p_hwfn,
 			       "PF returned err %d to VF acquisition request\n",
-- 
1.7.10.3

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

* [PATCH v2 4/8] net/qede/base: fix error return code in resc alloc
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
                     ` (3 preceding siblings ...)
  2016-12-23  0:49   ` [PATCH v2 3/8] net/qede/base: fix multiple acquisition requests by VF Rasesh Mody
@ 2016-12-23  0:50   ` Rasesh Mody
  2016-12-23  0:50   ` [PATCH v2 5/8] net/qede/base: fix mutex in freeing context manager Rasesh Mody
                     ` (3 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-23  0:50 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: Rasesh Mody, dev, stable, Dept-EngDPDKDev

Fix to return error code ECORE_INVAL instead of 0 when EQ elements
is too large as done elsewhere in this function.

Fixes: 22d07d939c3c ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_dev.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/qede/base/ecore_dev.c b/drivers/net/qede/base/ecore_dev.c
index 6060f9e..58b9387 100644
--- a/drivers/net/qede/base/ecore_dev.c
+++ b/drivers/net/qede/base/ecore_dev.c
@@ -667,6 +667,7 @@ enum _ecore_status_t ecore_resc_alloc(struct ecore_dev *p_dev)
 			DP_ERR(p_hwfn, "Cannot allocate 0x%x EQ elements."
 				       "The maximum of a u16 chain is 0x%x\n",
 			       n_eqes, 0xFFFF);
+			rc = ECORE_INVAL;
 			goto alloc_err;
 		}
 
-- 
1.7.10.3

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

* [PATCH v2 5/8] net/qede/base: fix mutex in freeing context manager
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
                     ` (4 preceding siblings ...)
  2016-12-23  0:50   ` [PATCH v2 4/8] net/qede/base: fix error return code in resc alloc Rasesh Mody
@ 2016-12-23  0:50   ` Rasesh Mody
  2016-12-23  0:50   ` [PATCH v2 6/8] net/qede/base: fix Rx queue access by malicious VFs Rasesh Mody
                     ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-23  0:50 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: Rasesh Mody, dev, stable, Dept-EngDPDKDev

Fix OSAL_MUTEX_DEALLOC() in freeing the context manager.

Fixes: 22d07d939c3c ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_cxt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/qede/base/ecore_cxt.c b/drivers/net/qede/base/ecore_cxt.c
index 3dd953d..5ea4f5c 100644
--- a/drivers/net/qede/base/ecore_cxt.c
+++ b/drivers/net/qede/base/ecore_cxt.c
@@ -1155,7 +1155,7 @@ void ecore_cxt_mngr_free(struct ecore_hwfn *p_hwfn)
 	ecore_cid_map_free(p_hwfn);
 	ecore_cxt_src_t2_free(p_hwfn);
 	ecore_ilt_shadow_free(p_hwfn);
-	OSAL_MUTEX_DEALLOC(&p_mngr->mutex);
+	OSAL_MUTEX_DEALLOC(&p_hwfn->p_cxt_mngr->mutex);
 	OSAL_FREE(p_hwfn->p_dev, p_hwfn->p_cxt_mngr);
 
 	p_hwfn->p_cxt_mngr = OSAL_NULL;
-- 
1.7.10.3

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

* [PATCH v2 6/8] net/qede/base: fix Rx queue access by malicious VFs
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
                     ` (5 preceding siblings ...)
  2016-12-23  0:50   ` [PATCH v2 5/8] net/qede/base: fix mutex in freeing context manager Rasesh Mody
@ 2016-12-23  0:50   ` Rasesh Mody
  2016-12-23  0:50   ` [PATCH v2 7/8] net/qede/base: fix to handle acquire request from VF Rasesh Mody
  2016-12-23  0:50   ` [PATCH v2 8/8] net/qede/base: fix VF over legacy PF Rasesh Mody
  8 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-23  0:50 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: Rasesh Mody, dev, stable, Dept-EngDPDKDev

Rx queue access is still done prior to the index being validated by PF.
Hence move Rx queue and status block validation check before accessing
Rx queue to prevent malicious VFs from using out-of-bound queue indices.

Fixes: 98bc693e1938 ("net/qede/base: change queue start")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_sriov.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/net/qede/base/ecore_sriov.c b/drivers/net/qede/base/ecore_sriov.c
index de54b9a..1255296 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -1968,6 +1968,11 @@ static void ecore_iov_vf_mbx_start_rxq(struct ecore_hwfn *p_hwfn,
 	enum _ecore_status_t rc;
 
 	req = &mbx->req_virt->start_rxq;
+
+	if (!ecore_iov_validate_rxq(p_hwfn, vf, req->rx_qid) ||
+	    !ecore_iov_validate_sb(p_hwfn, vf, req->hw_sb))
+		goto out;
+
 	OSAL_MEMSET(&p_params, 0, sizeof(p_params));
 	p_params.queue_id = (u8)vf->vf_queues[req->rx_qid].fw_rx_qid;
 	p_params.vf_qid = req->rx_qid;
@@ -1976,10 +1981,6 @@ static void ecore_iov_vf_mbx_start_rxq(struct ecore_hwfn *p_hwfn,
 	p_params.sb = req->hw_sb;
 	p_params.sb_idx = req->sb_index;
 
-	if (!ecore_iov_validate_rxq(p_hwfn, vf, req->rx_qid) ||
-	    !ecore_iov_validate_sb(p_hwfn, vf, req->hw_sb))
-		goto out;
-
 	/* Legacy VFs have their Producers in a different location, which they
 	 * calculate on their own and clean the producer prior to this.
 	 */
-- 
1.7.10.3

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

* [PATCH v2 7/8] net/qede/base: fix to handle acquire request from VF
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
                     ` (6 preceding siblings ...)
  2016-12-23  0:50   ` [PATCH v2 6/8] net/qede/base: fix Rx queue access by malicious VFs Rasesh Mody
@ 2016-12-23  0:50   ` Rasesh Mody
  2016-12-23  0:50   ` [PATCH v2 8/8] net/qede/base: fix VF over legacy PF Rasesh Mody
  8 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-23  0:50 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: Rasesh Mody, dev, stable, Dept-EngDPDKDev

Add a check and fail the VF's probe request if VF is already in
VF_ACQUIRED state.

Fixes: 22d07d939c3c ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_sriov.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/qede/base/ecore_sriov.c b/drivers/net/qede/base/ecore_sriov.c
index 1255296..c2fbee8 100644
--- a/drivers/net/qede/base/ecore_sriov.c
+++ b/drivers/net/qede/base/ecore_sriov.c
@@ -1459,6 +1459,18 @@ static void ecore_iov_vf_mbx_acquire(struct ecore_hwfn       *p_hwfn,
 	pfdev_info->major_fp_hsi = ETH_HSI_VER_MAJOR;
 	pfdev_info->minor_fp_hsi = ETH_HSI_VER_MINOR;
 
+	/* TODO - not doing anything is bad since we'll assert, but this isn't
+	 * necessarily the right behavior - perhaps we should have allowed some
+	 * versatility here.
+	 */
+	if (vf->state != VF_FREE &&
+	    vf->state != VF_STOPPED) {
+		DP_VERBOSE(p_hwfn, ECORE_MSG_IOV,
+			   "VF[%d] sent ACQUIRE but is already in state %d - fail request\n",
+			   vf->abs_vf_id, vf->state);
+		goto out;
+	}
+
 	/* Validate FW compatibility */
 	if (req->vfdev_info.eth_fp_hsi_major != ETH_HSI_VER_MAJOR) {
 		if (req->vfdev_info.capabilities &
-- 
1.7.10.3

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

* [PATCH v2 8/8] net/qede/base: fix VF over legacy PF
  2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
                     ` (7 preceding siblings ...)
  2016-12-23  0:50   ` [PATCH v2 7/8] net/qede/base: fix to handle acquire request from VF Rasesh Mody
@ 2016-12-23  0:50   ` Rasesh Mody
  8 siblings, 0 replies; 19+ messages in thread
From: Rasesh Mody @ 2016-12-23  0:50 UTC (permalink / raw)
  To: ferruh.yigit; +Cc: Rasesh Mody, dev, stable, Dept-EngDPDKDev

Apparently VF over Legacy PF doesn't work, as VF would fail after
getting the initial rejection message [instead of sending an additional
one where it asks for a FW override and see if it works].

Fixes: 22d07d939c3c ("net/qede/base: update")

Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>
---
 drivers/net/qede/base/ecore_vf.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/qede/base/ecore_vf.c b/drivers/net/qede/base/ecore_vf.c
index 161b317..c26b602 100644
--- a/drivers/net/qede/base/ecore_vf.c
+++ b/drivers/net/qede/base/ecore_vf.c
@@ -294,6 +294,7 @@ static enum _ecore_status_t ecore_vf_pf_acquire(struct ecore_hwfn *p_hwfn)
 						" override\n");
 					req->vfdev_info.capabilities |=
 						VFPF_ACQUIRE_CAP_PRE_FP_HSI;
+					continue;
 				}
 			}
 
-- 
1.7.10.3

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

* Re: [PATCH v2 1/8] net/qede: fix to get vendor/device id info
  2016-12-23  0:49   ` [PATCH v2 " Rasesh Mody
@ 2016-12-23 15:26     ` Ferruh Yigit
  0 siblings, 0 replies; 19+ messages in thread
From: Ferruh Yigit @ 2016-12-23 15:26 UTC (permalink / raw)
  To: Rasesh Mody; +Cc: dev, stable, Dept-EngDPDKDev

On 12/23/2016 12:49 AM, Rasesh Mody wrote:
> The vendor_id and device_id are used to determine device type. If you
> don't have them, then check for determining device type fails and is
> always set to default device type.
> 
> Fixes: ec94dbc57362 ("qede: add base driver")
> 
> Signed-off-by: Rasesh Mody <Rasesh.Mody@cavium.com>

Series applied to dpdk-next-net/master, thanks.

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

end of thread, other threads:[~2016-12-23 15:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-03  6:35 [PATCH 1/8] net/qede: fix to get vendor/device id info Rasesh Mody
2016-12-03  6:35 ` [PATCH 2/8] net/qede/base: fix sriov printouts Rasesh Mody
2016-12-03  6:35 ` [PATCH 3/8] net/qede/base: fix multiple acquisition requests by VF Rasesh Mody
2016-12-03  6:35 ` [PATCH 4/8] net/qede/base: fix error return code in resc alloc Rasesh Mody
2016-12-03  6:35 ` [PATCH 5/8] net/qede/base: fix mutex in freeing context manager Rasesh Mody
2016-12-03  6:35 ` [PATCH 6/8] net/qede/base: fix Rx queue access by malicious VFs Rasesh Mody
2016-12-03  6:35 ` [PATCH 7/8] net/qede/base: fix to handle acquire request from VF Rasesh Mody
2016-12-03  6:35 ` [PATCH 8/8] net/qede/base: fix to check FW override after rejection Rasesh Mody
2016-12-09 13:35 ` [PATCH 1/8] net/qede: fix to get vendor/device id info Ferruh Yigit
2016-12-20 23:25   ` Mody, Rasesh
2016-12-23  0:49   ` [PATCH v2 " Rasesh Mody
2016-12-23 15:26     ` Ferruh Yigit
2016-12-23  0:49   ` [PATCH v2 2/8] net/qede/base: fix sriov printouts Rasesh Mody
2016-12-23  0:49   ` [PATCH v2 3/8] net/qede/base: fix multiple acquisition requests by VF Rasesh Mody
2016-12-23  0:50   ` [PATCH v2 4/8] net/qede/base: fix error return code in resc alloc Rasesh Mody
2016-12-23  0:50   ` [PATCH v2 5/8] net/qede/base: fix mutex in freeing context manager Rasesh Mody
2016-12-23  0:50   ` [PATCH v2 6/8] net/qede/base: fix Rx queue access by malicious VFs Rasesh Mody
2016-12-23  0:50   ` [PATCH v2 7/8] net/qede/base: fix to handle acquire request from VF Rasesh Mody
2016-12-23  0:50   ` [PATCH v2 8/8] net/qede/base: fix VF over legacy PF Rasesh Mody

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.