[dpdk-dev] [PATCH] fm10k: fix an error message when adding default VLAN

[dpdk-dev] [PATCH] fm10k: fix an error message when adding default VLAN

[Shaopeng He]

The default MAC address is directly copied to Device Ethernet
Link address array in the device initialize phase, which
bypasses fm10k MAC address number check mechanism, and will
cause an error message when adding default VLAN. Fix it by
moving default MAC address registration to device
initialize phase.

Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
---
 drivers/net/fm10k/fm10k_ethdev.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 406c350..df32665 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -791,14 +791,10 @@ fm10k_dev_start(struct rte_eth_dev *dev)
 		}
 	}
 
-	if (hw->mac.default_vid && hw->mac.default_vid <= ETHER_MAX_VLAN_ID) {
-		/* Update default vlan */
+	/* Update default vlan */
+	if (hw->mac.default_vid && hw->mac.default_vid <= ETHER_MAX_VLAN_ID)
 		fm10k_vlan_filter_set(dev, hw->mac.default_vid, true);
 
-		/* Add default mac/vlan filter to PF/Switch manager */
-		fm10k_MAC_filter_set(dev, hw->mac.addr, true);
-	}
-
 	return 0;
 }
 
@@ -2144,6 +2140,8 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 
 	fm10k_mbx_unlock(hw);
 
+	/* Add default mac address */
+	fm10k_MAC_filter_set(dev, hw->mac.addr, true);
 
 	return 0;
 }
-- 
1.9.3

Re: [dpdk-dev] [PATCH] fm10k: fix an error message when adding default VLAN

[Thomas Monjalon]

2015-06-26 10:37, Shaopeng He:
> The default MAC address is directly copied to Device Ethernet
> Link address array in the device initialize phase, which

Do you mean "device start phase" instead?

> bypasses fm10k MAC address number check mechanism, and will
> cause an error message when adding default VLAN. Fix it by

What is the error message?
Is it only an error message or a behaviour error?

> moving default MAC address registration to device
> initialize phase.

Yes it is moved from start to init.

> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -791,14 +791,10 @@ fm10k_dev_start(struct rte_eth_dev *dev)
>  		}
>  	}
>  
> -	if (hw->mac.default_vid && hw->mac.default_vid <= ETHER_MAX_VLAN_ID) {
> -		/* Update default vlan */
> +	/* Update default vlan */
> +	if (hw->mac.default_vid && hw->mac.default_vid <= ETHER_MAX_VLAN_ID)
>  		fm10k_vlan_filter_set(dev, hw->mac.default_vid, true);
>  
> -		/* Add default mac/vlan filter to PF/Switch manager */
> -		fm10k_MAC_filter_set(dev, hw->mac.addr, true);
> -	}
> -
>  	return 0;
>  }
>  
> @@ -2144,6 +2140,8 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  
>  	fm10k_mbx_unlock(hw);
>  
> +	/* Add default mac address */
> +	fm10k_MAC_filter_set(dev, hw->mac.addr, true);
>  
>  	return 0;
>  }
> 

Re: [dpdk-dev] [PATCH] fm10k: fix an error message when adding default VLAN

[He, Shaopeng]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, July 01, 2015 9:12 PM
> To: He, Shaopeng
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] fm10k: fix an error message when adding
> default VLAN
> 
> 2015-06-26 10:37, Shaopeng He:
> > The default MAC address is directly copied to Device Ethernet Link
> > address array in the device initialize phase, which
> 
> Do you mean "device start phase" instead?

Thanks for taking time to review this patch. The default MAC address is read from hardware
and copied to dev->data->mac_addrs in eth_fm10k_dev_init, but the fm10k_MAC_filter_set
previously was called in fm10k_dev_start, which caused this issue.

> 
> > bypasses fm10k MAC address number check mechanism, and will cause an
> > error message when adding default VLAN. Fix it by
> 
> What is the error message?
> Is it only an error message or a behaviour error?

The error message is "MAC address number not match", it is only an error message, because
fm10k_dev_start will eventually be called when default_vid was ready, and MAC/VLAN table
will be updated correctly. default_vid is necessary for fm10k to function correctly.

> 
> > moving default MAC address registration to device initialize phase.
> 
> Yes it is moved from start to init.

fm10k_MAC_filter_set is moved from eth_fm10k_dev_init to eth_fm10k_dev_init, aligned with 
the place where the default MAC address is actually read and copied.

> 
> > --- a/drivers/net/fm10k/fm10k_ethdev.c
> > +++ b/drivers/net/fm10k/fm10k_ethdev.c
> > @@ -791,14 +791,10 @@ fm10k_dev_start(struct rte_eth_dev *dev)
> >  		}
> >  	}
> >
> > -	if (hw->mac.default_vid && hw->mac.default_vid <=
> ETHER_MAX_VLAN_ID) {
> > -		/* Update default vlan */
> > +	/* Update default vlan */
> > +	if (hw->mac.default_vid && hw->mac.default_vid <=
> ETHER_MAX_VLAN_ID)
> >  		fm10k_vlan_filter_set(dev, hw->mac.default_vid, true);
> >
> > -		/* Add default mac/vlan filter to PF/Switch manager */
> > -		fm10k_MAC_filter_set(dev, hw->mac.addr, true);
> > -	}
> > -
> >  	return 0;
> >  }
> >
> > @@ -2144,6 +2140,8 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
> >
> >  	fm10k_mbx_unlock(hw);
> >
> > +	/* Add default mac address */
> > +	fm10k_MAC_filter_set(dev, hw->mac.addr, true);
> >
> >  	return 0;
> >  }
> >
> 

Re: [dpdk-dev] [PATCH] fm10k: fix an error message when adding default VLAN

[Qiu, Michael]

On 6/26/2015 10:37 AM, He, Shaopeng wrote:
> The default MAC address is directly copied to Device Ethernet
> Link address array in the device initialize phase, which
> bypasses fm10k MAC address number check mechanism, and will
> cause an error message when adding default VLAN. Fix it by
> moving default MAC address registration to device
> initialize phase.
>
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
>
Acked-by: Michael Qiu <michael.qiu@intel.com>

Re: [dpdk-dev] [PATCH] fm10k: fix an error message when adding default VLAN

[Thomas Monjalon]

2015-07-02 12:18, Qiu, Michael:
> On 6/26/2015 10:37 AM, He, Shaopeng wrote:
> > The default MAC address is directly copied to Device Ethernet
> > Link address array in the device initialize phase, which
> > bypasses fm10k MAC address number check mechanism, and will
> > cause an error message when adding default VLAN. Fix it by
> > moving default MAC address registration to device
> > initialize phase.
> >
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> >
> Acked-by: Michael Qiu <michael.qiu@intel.com>

Applied, thanks