[dpdk-dev] [PATCH] fm10k: fix PF/VF MAC address register and clean up bug

[dpdk-dev] [PATCH] fm10k: fix PF/VF MAC address register and clean up bug

[Shaopeng He]

MAC address with fixed VLAN 0 was removed. VF MAC/VLAN filter was enabled
for the default value. Removed all VLAN and MAC address table entries when
the system(e.g. testpmd) was closed.

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

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 9b198a7..587debc 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -58,6 +58,8 @@ static int
 fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on);
 static void
 fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add);
+static void
+fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev);
 
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)
@@ -703,6 +705,8 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	fm10k_MACVLAN_remove_all(dev);
+
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
@@ -830,10 +834,6 @@ fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 	hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
 
-	/* @todo - add support for the VF */
-	if (hw->mac.type != fm10k_mac_pf)
-		return -ENOTSUP;
-
 	if (vlan_id > ETH_VLAN_ID_MAX) {
 		PMD_INIT_LOG(ERR, "Invalid vlan_id: must be < 4096");
 		return (-EINVAL);
@@ -916,10 +916,6 @@ fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add)
 	hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
 
-	/* @todo - add support for the VF */
-	if (hw->mac.type != fm10k_mac_pf)
-		return;
-
 	fm10k_mbx_lock(hw);
 	i = 0;
 	for (j = 0; j < FM10K_VFTA_SIZE; j++) {
@@ -970,6 +966,25 @@ fm10k_macaddr_remove(struct rte_eth_dev *dev, uint32_t index)
 				FALSE);
 }
 
+/* Remove all VLAN and MAC address table entries */
+static void
+fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev)
+{
+	uint32_t j, k;
+	struct fm10k_macvlan_filter_info *macvlan;
+
+	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
+	for (j = 0; j < FM10K_VFTA_SIZE; j++) {
+		if (macvlan->vfta[j]) {
+			for (k = 0; k < FM10K_UINT32_BIT_SIZE; k++) {
+				if (macvlan->vfta[j] & (1 << k))
+					fm10k_vlan_filter_set(dev,
+						j * FM10K_UINT32_BIT_SIZE + k, false);
+			}
+		}
+	}
+}
+
 static inline int
 check_nb_desc(uint16_t min, uint16_t max, uint16_t mult, uint16_t request)
 {
@@ -1997,13 +2012,6 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	/* Enable port first */
 	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
 
-	/*
-	 * Add default mac. glort is assigned by SM for PF, while is
-	 * unused for VF. PF will assign correct glort for VF.
-	 */
-	hw->mac.ops.update_uc_addr(hw, hw->mac.dglort_map, hw->mac.addr,
-				0, 1, 0);
-
 	/* Set unicast mode by default. App can change to other mode in other
 	 * API func.
 	 */
-- 
1.9.3

Re: [dpdk-dev] [PATCH] fm10k: fix PF/VF MAC address register and clean up bug

[Thomas Monjalon]

Hi,

2015-06-05 11:24, Shaopeng He:
> MAC address with fixed VLAN 0 was removed. VF MAC/VLAN filter was enabled
> for the default value. Removed all VLAN and MAC address table entries when
> the system(e.g. testpmd) was closed.
> 
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>

Sorry, it's difficult to understand what was the issue in this description.
Could you try to add more context and reword it, please?

Re: [dpdk-dev] [PATCH] fm10k: fix PF/VF MAC address register and clean up bug

[He, Shaopeng]

Hi Thomas,

Thanks for the comments. I will add more context to make it more clear.

BR,
--Shaopeng
-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
Sent: Friday, June 5, 2015 4:56 PM
To: He, Shaopeng
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] fm10k: fix PF/VF MAC address register and clean up bug

Hi,

2015-06-05 11:24, Shaopeng He:
> MAC address with fixed VLAN 0 was removed. VF MAC/VLAN filter was 
> enabled for the default value. Removed all VLAN and MAC address table 
> entries when the system(e.g. testpmd) was closed.
> 
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>

Sorry, it's difficult to understand what was the issue in this description.
Could you try to add more context and reword it, please?

[dpdk-dev] [PATCH v2] fm10k: fix PF/VF MAC address register and clean up bug

[Shaopeng He]

v2 changes:
 - rebase this patch onto the HEAD
 - add more description for this patch

This patch includes 3 changes related to MAC/VLAN address table
when the system(e.g. testpmd) is started and closed:
 - remove default MAC address with fixed VLAN 0 which was for the
   debug purpose before the MAC/VLAN filter function was implemented.
 - enable VF MAC/VLAN filter for the first valid MAC address
   and first valid VLAN ID. This is needed for system(e.g. testpmd)
   to setup default MAC address and default VLAN for VF.
   Later attempt to change these default value will be refused by
   under layer shared code and PF host functions.
 - un-register any combination of VLAN and MAC address from fm10k
   switch side MAC table when the system(e.g. testpmd) is closed.

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

diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 802a930..625fd31 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -58,6 +58,8 @@ static int
 fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on);
 static void
 fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add);
+static void
+fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev);
 
 static void
 fm10k_mbx_initlock(struct fm10k_hw *hw)
@@ -703,6 +705,8 @@ fm10k_dev_close(struct rte_eth_dev *dev)
 
 	PMD_INIT_FUNC_TRACE();
 
+	fm10k_MACVLAN_remove_all(dev);
+
 	/* Stop mailbox service first */
 	fm10k_close_mbx_service(hw);
 	fm10k_dev_stop(dev);
@@ -832,12 +836,6 @@ fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
 	hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
 
-	/* @todo - add support for the VF */
-	if (hw->mac.type != fm10k_mac_pf) {
-		PMD_INIT_LOG(ERR, "VLAN filter not available on VF");
-		return -ENOTSUP;
-	}
-
 	if (vlan_id > ETH_VLAN_ID_MAX) {
 		PMD_INIT_LOG(ERR, "Invalid vlan_id: must be < 4096");
 		return (-EINVAL);
@@ -926,12 +924,6 @@ fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add)
 	hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
 
-	/* @todo - add support for the VF */
-	if (hw->mac.type != fm10k_mac_pf) {
-		PMD_INIT_LOG(ERR, "MAC filter not available on VF");
-		return;
-	}
-
 	i = 0;
 	for (j = 0; j < FM10K_VFTA_SIZE; j++) {
 		if (macvlan->vfta[j]) {
@@ -981,6 +973,25 @@ fm10k_macaddr_remove(struct rte_eth_dev *dev, uint32_t index)
 				FALSE);
 }
 
+/* Remove all VLAN and MAC address table entries */
+static void
+fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev)
+{
+	uint32_t j, k;
+	struct fm10k_macvlan_filter_info *macvlan;
+
+	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
+	for (j = 0; j < FM10K_VFTA_SIZE; j++) {
+		if (macvlan->vfta[j]) {
+			for (k = 0; k < FM10K_UINT32_BIT_SIZE; k++) {
+				if (macvlan->vfta[j] & (1 << k))
+					fm10k_vlan_filter_set(dev,
+						j * FM10K_UINT32_BIT_SIZE + k, false);
+			}
+		}
+	}
+}
+
 static inline int
 check_nb_desc(uint16_t min, uint16_t max, uint16_t mult, uint16_t request)
 {
@@ -2008,13 +2019,6 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
 	/* Enable port first */
 	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
 
-	/*
-	 * Add default mac. glort is assigned by SM for PF, while is
-	 * unused for VF. PF will assign correct glort for VF.
-	 */
-	hw->mac.ops.update_uc_addr(hw, hw->mac.dglort_map, hw->mac.addr,
-				0, 1, 0);
-
 	/* Set unicast mode by default. App can change to other mode in other
 	 * API func.
 	 */
-- 
1.9.3

Re: [dpdk-dev] [PATCH v2] fm10k: fix PF/VF MAC address register and clean up bug

[Chen, Jing D]

Hi,

> -----Original Message-----
> From: He, Shaopeng
> Sent: Monday, June 15, 2015 9:26 AM
> To: dev@dpdk.org
> Cc: Chen, Jing D; Qiu, Michael; He, Shaopeng
> Subject: [PATCH v2] fm10k: fix PF/VF MAC address register and clean up bug
> 
> v2 changes:
>  - rebase this patch onto the HEAD
>  - add more description for this patch
> 
> This patch includes 3 changes related to MAC/VLAN address table
> when the system(e.g. testpmd) is started and closed:
>  - remove default MAC address with fixed VLAN 0 which was for the
>    debug purpose before the MAC/VLAN filter function was implemented.
>  - enable VF MAC/VLAN filter for the first valid MAC address
>    and first valid VLAN ID. This is needed for system(e.g. testpmd)
>    to setup default MAC address and default VLAN for VF.
>    Later attempt to change these default value will be refused by
>    under layer shared code and PF host functions.
>  - un-register any combination of VLAN and MAC address from fm10k
>    switch side MAC table when the system(e.g. testpmd) is closed.
> 
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c | 42 ++++++++++++++++++++++--------
> ----------
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c
> b/drivers/net/fm10k/fm10k_ethdev.c
> index 802a930..625fd31 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -58,6 +58,8 @@ static int
>  fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on);
>  static void
>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add);
> +static void
> +fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev);
> 
>  static void
>  fm10k_mbx_initlock(struct fm10k_hw *hw)
> @@ -703,6 +705,8 @@ fm10k_dev_close(struct rte_eth_dev *dev)
> 
>  	PMD_INIT_FUNC_TRACE();
> 
> +	fm10k_MACVLAN_remove_all(dev);
> +
>  	/* Stop mailbox service first */
>  	fm10k_close_mbx_service(hw);
>  	fm10k_dev_stop(dev);
> @@ -832,12 +836,6 @@ fm10k_vlan_filter_set(struct rte_eth_dev *dev,
> uint16_t vlan_id, int on)
>  	hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data-
> >dev_private);
> 
> -	/* @todo - add support for the VF */
> -	if (hw->mac.type != fm10k_mac_pf) {
> -		PMD_INIT_LOG(ERR, "VLAN filter not available on VF");
> -		return -ENOTSUP;
> -	}
> -
>  	if (vlan_id > ETH_VLAN_ID_MAX) {
>  		PMD_INIT_LOG(ERR, "Invalid vlan_id: must be < 4096");
>  		return (-EINVAL);
> @@ -926,12 +924,6 @@ fm10k_MAC_filter_set(struct rte_eth_dev *dev,
> const u8 *mac, bool add)
>  	hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data-
> >dev_private);
> 
> -	/* @todo - add support for the VF */
> -	if (hw->mac.type != fm10k_mac_pf) {
> -		PMD_INIT_LOG(ERR, "MAC filter not available on VF");
> -		return;
> -	}
> -
>  	i = 0;
>  	for (j = 0; j < FM10K_VFTA_SIZE; j++) {
>  		if (macvlan->vfta[j]) {
> @@ -981,6 +973,25 @@ fm10k_macaddr_remove(struct rte_eth_dev *dev,
> uint32_t index)
>  				FALSE);
>  }
> 
> +/* Remove all VLAN and MAC address table entries */
> +static void
> +fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev)
> +{
> +	uint32_t j, k;
> +	struct fm10k_macvlan_filter_info *macvlan;
> +
> +	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data-
> >dev_private);
> +	for (j = 0; j < FM10K_VFTA_SIZE; j++) {
> +		if (macvlan->vfta[j]) {
> +			for (k = 0; k < FM10K_UINT32_BIT_SIZE; k++) {
> +				if (macvlan->vfta[j] & (1 << k))
> +					fm10k_vlan_filter_set(dev,
> +						j * FM10K_UINT32_BIT_SIZE
> + k, false);
> +			}
> +		}
> +	}
> +}
> +
>  static inline int
>  check_nb_desc(uint16_t min, uint16_t max, uint16_t mult, uint16_t request)
>  {
> @@ -2008,13 +2019,6 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  	/* Enable port first */
>  	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
> 
> -	/*
> -	 * Add default mac. glort is assigned by SM for PF, while is
> -	 * unused for VF. PF will assign correct glort for VF.
> -	 */
> -	hw->mac.ops.update_uc_addr(hw, hw->mac.dglort_map, hw-
> >mac.addr,
> -				0, 1, 0);
> -
>  	/* Set unicast mode by default. App can change to other mode in
> other
>  	 * API func.
>  	 */
> --
> 1.9.3

Acked-by : Jing Chen <jing.d.chen@intel.com>

Re: [dpdk-dev] [PATCH v2] fm10k: fix PF/VF MAC address register and clean up bug

[Thomas Monjalon]

> > v2 changes:
> >  - rebase this patch onto the HEAD
> >  - add more description for this patch
> > 
> > This patch includes 3 changes related to MAC/VLAN address table
> > when the system(e.g. testpmd) is started and closed:
> >  - remove default MAC address with fixed VLAN 0 which was for the
> >    debug purpose before the MAC/VLAN filter function was implemented.
> >  - enable VF MAC/VLAN filter for the first valid MAC address
> >    and first valid VLAN ID. This is needed for system(e.g. testpmd)
> >    to setup default MAC address and default VLAN for VF.
> >    Later attempt to change these default value will be refused by
> >    under layer shared code and PF host functions.
> >  - un-register any combination of VLAN and MAC address from fm10k
> >    switch side MAC table when the system(e.g. testpmd) is closed.
> > 
> > Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> 
> Acked-by : Jing Chen <jing.d.chen@intel.com>

Applied, thanks

Re: [dpdk-dev] [PATCH v2] fm10k: fix PF/VF MAC address register and clean up bug

[Qiu, Michael]

Tested-by: Michael Qiu <michael.qiu@intel.com>

- OS: Fedora20  3.11.10-301
- GCC: gcc version 4.8.3 2014911
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Ethernet controller: Intel Corporation Device 15a4 (rev 01)
- Default x86_64-native-linuxapp-gcc configuration
- Total 1 cases, 1 passed, 0 failed


On 6/15/2015 9:26 AM, He, Shaopeng wrote:
> v2 changes:
>  - rebase this patch onto the HEAD
>  - add more description for this patch
>
> This patch includes 3 changes related to MAC/VLAN address table
> when the system(e.g. testpmd) is started and closed:
>  - remove default MAC address with fixed VLAN 0 which was for the
>    debug purpose before the MAC/VLAN filter function was implemented.
>  - enable VF MAC/VLAN filter for the first valid MAC address
>    and first valid VLAN ID. This is needed for system(e.g. testpmd)
>    to setup default MAC address and default VLAN for VF.
>    Later attempt to change these default value will be refused by
>    under layer shared code and PF host functions.
>  - un-register any combination of VLAN and MAC address from fm10k
>    switch side MAC table when the system(e.g. testpmd) is closed.
>
> Signed-off-by: Shaopeng He <shaopeng.he@intel.com>
> ---
>  drivers/net/fm10k/fm10k_ethdev.c | 42 ++++++++++++++++++++++------------------
>  1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
> index 802a930..625fd31 100644
> --- a/drivers/net/fm10k/fm10k_ethdev.c
> +++ b/drivers/net/fm10k/fm10k_ethdev.c
> @@ -58,6 +58,8 @@ static int
>  fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on);
>  static void
>  fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add);
> +static void
> +fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev);
>  
>  static void
>  fm10k_mbx_initlock(struct fm10k_hw *hw)
> @@ -703,6 +705,8 @@ fm10k_dev_close(struct rte_eth_dev *dev)
>  
>  	PMD_INIT_FUNC_TRACE();
>  
> +	fm10k_MACVLAN_remove_all(dev);
> +
>  	/* Stop mailbox service first */
>  	fm10k_close_mbx_service(hw);
>  	fm10k_dev_stop(dev);
> @@ -832,12 +836,6 @@ fm10k_vlan_filter_set(struct rte_eth_dev *dev, uint16_t vlan_id, int on)
>  	hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
>  
> -	/* @todo - add support for the VF */
> -	if (hw->mac.type != fm10k_mac_pf) {
> -		PMD_INIT_LOG(ERR, "VLAN filter not available on VF");
> -		return -ENOTSUP;
> -	}
> -
>  	if (vlan_id > ETH_VLAN_ID_MAX) {
>  		PMD_INIT_LOG(ERR, "Invalid vlan_id: must be < 4096");
>  		return (-EINVAL);
> @@ -926,12 +924,6 @@ fm10k_MAC_filter_set(struct rte_eth_dev *dev, const u8 *mac, bool add)
>  	hw = FM10K_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>  	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
>  
> -	/* @todo - add support for the VF */
> -	if (hw->mac.type != fm10k_mac_pf) {
> -		PMD_INIT_LOG(ERR, "MAC filter not available on VF");
> -		return;
> -	}
> -
>  	i = 0;
>  	for (j = 0; j < FM10K_VFTA_SIZE; j++) {
>  		if (macvlan->vfta[j]) {
> @@ -981,6 +973,25 @@ fm10k_macaddr_remove(struct rte_eth_dev *dev, uint32_t index)
>  				FALSE);
>  }
>  
> +/* Remove all VLAN and MAC address table entries */
> +static void
> +fm10k_MACVLAN_remove_all(struct rte_eth_dev *dev)
> +{
> +	uint32_t j, k;
> +	struct fm10k_macvlan_filter_info *macvlan;
> +
> +	macvlan = FM10K_DEV_PRIVATE_TO_MACVLAN(dev->data->dev_private);
> +	for (j = 0; j < FM10K_VFTA_SIZE; j++) {
> +		if (macvlan->vfta[j]) {
> +			for (k = 0; k < FM10K_UINT32_BIT_SIZE; k++) {
> +				if (macvlan->vfta[j] & (1 << k))
> +					fm10k_vlan_filter_set(dev,
> +						j * FM10K_UINT32_BIT_SIZE + k, false);
> +			}
> +		}
> +	}
> +}
> +
>  static inline int
>  check_nb_desc(uint16_t min, uint16_t max, uint16_t mult, uint16_t request)
>  {
> @@ -2008,13 +2019,6 @@ eth_fm10k_dev_init(struct rte_eth_dev *dev)
>  	/* Enable port first */
>  	hw->mac.ops.update_lport_state(hw, hw->mac.dglort_map, 1, 1);
>  
> -	/*
> -	 * Add default mac. glort is assigned by SM for PF, while is
> -	 * unused for VF. PF will assign correct glort for VF.
> -	 */
> -	hw->mac.ops.update_uc_addr(hw, hw->mac.dglort_map, hw->mac.addr,
> -				0, 1, 0);
> -
>  	/* Set unicast mode by default. App can change to other mode in other
>  	 * API func.
>  	 */