[dpdk-dev] fm10k: fix VF cannot receive broadcast traffic

Message ID 1465203647-18717-1-git-send-email-xiao.w.wang@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Bruce Richardson
Headers

Commit Message

Xiao Wang June 6, 2016, 9 a.m. UTC
  When app tries promisc/allmulti setting, fm10k will check if a valid glort
is acquired, if not then exit without doing anything. It's a long journey
for VF to acquire glort info from VF to PF mailbox, PF to switch mailbox.
It could be a long interval that's out of DPDK's control. Thus, app may
fail on promisc/allmulti setting in VF. In fact, we don't need a valid
glort value in VF, so this patch just skips the glort check for VF.

Fixes: df02ba864695 ("fm10k: support promiscuous mode")

Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

Comments

Chen, Jing D June 14, 2016, 5:03 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: Wang, Xiao W
> Sent: Monday, June 06, 2016 5:01 PM
> To: Chen, Jing D <jing.d.chen@intel.com>
> Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.wang@intel.com>
> Subject: [PATCH] fm10k: fix VF cannot receive broadcast traffic
> 
> When app tries promisc/allmulti setting, fm10k will check if a valid glort
> is acquired, if not then exit without doing anything. It's a long journey
> for VF to acquire glort info from VF to PF mailbox, PF to switch mailbox.
> It could be a long interval that's out of DPDK's control. Thus, app may
> fail on promisc/allmulti setting in VF. In fact, we don't need a valid
> glort value in VF, so this patch just skips the glort check for VF.
> 
> Fixes: df02ba864695 ("fm10k: support promiscuous mode")
> 
> Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
  
Bruce Richardson June 17, 2016, 10:18 a.m. UTC | #2
On Mon, Jun 06, 2016 at 05:00:47PM +0800, Wang Xiao W wrote:
> When app tries promisc/allmulti setting, fm10k will check if a valid glort
> is acquired, if not then exit without doing anything. It's a long journey
> for VF to acquire glort info from VF to PF mailbox, PF to switch mailbox.
> It could be a long interval that's out of DPDK's control. Thus, app may

I think the use of "thus" here is wrong, as I suspect that the failure is not
due to the "long interval that's out of DPDK's control", but instead due to
not having a valid glort.

> fail on promisc/allmulti setting in VF. In fact, we don't need a valid
> glort value in VF, so this patch just skips the glort check for VF.
> 
> Fixes: df02ba864695 ("fm10k: support promiscuous mode")
> 
> Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>

I rework this commit message for you on apply. Please check the updated version
when done.

/Bruce
  
Bruce Richardson June 17, 2016, 10:32 a.m. UTC | #3
On Tue, Jun 14, 2016 at 05:03:57AM +0000, Chen, Jing D wrote:
> Hi,
> 
> > -----Original Message-----
> > From: Wang, Xiao W
> > Sent: Monday, June 06, 2016 5:01 PM
> > To: Chen, Jing D <jing.d.chen@intel.com>
> > Cc: dev@dpdk.org; Wang, Xiao W <xiao.w.wang@intel.com>
> > Subject: [PATCH] fm10k: fix VF cannot receive broadcast traffic
> > 
> > When app tries promisc/allmulti setting, fm10k will check if a valid glort
> > is acquired, if not then exit without doing anything. It's a long journey
> > for VF to acquire glort info from VF to PF mailbox, PF to switch mailbox.
> > It could be a long interval that's out of DPDK's control. Thus, app may
> > fail on promisc/allmulti setting in VF. In fact, we don't need a valid
> > glort value in VF, so this patch just skips the glort check for VF.
> > 
> > Fixes: df02ba864695 ("fm10k: support promiscuous mode")
> > 
> > Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
> Acked-by: Jing Chen <jing.d.chen@intel.com>

Applied to dpdk-next-net/rel_16_07

/Bruce
  
Chen, Jing D June 19, 2016, 1:47 p.m. UTC | #4
Hi, Bruce,

> -----Original Message-----
> From: Richardson, Bruce
> Sent: Friday, June 17, 2016 6:19 PM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: Chen, Jing D <jing.d.chen@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] fm10k: fix VF cannot receive broadcast
> traffic
> 
> On Mon, Jun 06, 2016 at 05:00:47PM +0800, Wang Xiao W wrote:
> > When app tries promisc/allmulti setting, fm10k will check if a valid
> > glort is acquired, if not then exit without doing anything. It's a
> > long journey for VF to acquire glort info from VF to PF mailbox, PF to switch
> mailbox.
> > It could be a long interval that's out of DPDK's control. Thus, app
> > may
> 
> I think the use of "thus" here is wrong, as I suspect that the failure is not due
> to the "long interval that's out of DPDK's control", but instead due to not
> having a valid glort.

The logic in VF is glort ID is invalid at beginning. When VF port is enabled by
sending mailbox to PF, PF will send a message back to VF without carrying valid
info. Then, VF will fake a glort ID. 
In this case, it's useless to do sanity check of VALID glort ID.  Besides that, VF didn't
use glort ID to do functional call at all.
	
> 
> > fail on promisc/allmulti setting in VF. In fact, we don't need a valid
> > glort value in VF, so this patch just skips the glort check for VF.
> >
> > Fixes: df02ba864695 ("fm10k: support promiscuous mode")
> >
> > Signed-off-by: Wang Xiao W <xiao.w.wang@intel.com>
> 
> I rework this commit message for you on apply. Please check the updated
> version when done.
> 
> /Bruce
  

Patch

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index c2d377f..b3aefdb 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -947,7 +947,7 @@  fm10k_dev_promiscuous_enable(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 
 	/* Return if it didn't acquire valid glort range */
-	if (!fm10k_glort_valid(hw))
+	if ((hw->mac.type == fm10k_mac_pf) && !fm10k_glort_valid(hw))
 		return;
 
 	fm10k_mbx_lock(hw);
@@ -969,7 +969,7 @@  fm10k_dev_promiscuous_disable(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 
 	/* Return if it didn't acquire valid glort range */
-	if (!fm10k_glort_valid(hw))
+	if ((hw->mac.type == fm10k_mac_pf) && !fm10k_glort_valid(hw))
 		return;
 
 	if (dev->data->all_multicast == 1)
@@ -995,7 +995,7 @@  fm10k_dev_allmulticast_enable(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 
 	/* Return if it didn't acquire valid glort range */
-	if (!fm10k_glort_valid(hw))
+	if ((hw->mac.type == fm10k_mac_pf) && !fm10k_glort_valid(hw))
 		return;
 
 	/* If promiscuous mode is enabled, it doesn't make sense to enable
@@ -1026,7 +1026,7 @@  fm10k_dev_allmulticast_disable(struct rte_eth_dev *dev)
 	PMD_INIT_FUNC_TRACE();
 
 	/* Return if it didn't acquire valid glort range */
-	if (!fm10k_glort_valid(hw))
+	if ((hw->mac.type == fm10k_mac_pf) && !fm10k_glort_valid(hw))
 		return;
 
 	if (dev->data->promiscuous) {