[net] bnxt_en: Fix VF mac address regression.

Message ID 1536954089-6061-1-git-send-email-michael.chan@broadcom.com
State Accepted
Delegated to: David Miller
Headers show
Series
  • [net] bnxt_en: Fix VF mac address regression.
Related show

Commit Message

Michael Chan Sept. 14, 2018, 7:41 p.m.
The recent commit to always forward the VF MAC address to the PF for
approval may not work if the PF driver or the firmware is older.  This
will cause the VF driver to fail during probe:

  bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): hwrm req_type 0xf seq id 0x5 error 0xffff
  bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): VF MAC address 00:00:17:02:05:d0 not approved by the PF
  bnxt_en 0000:00:03.0: Unable to initialize mac address.
  bnxt_en: probe of 0000:00:03.0 failed with error -99

We fix it by treating the error as fatal only if the VF MAC address is
locally generated by the VF.

Fixes: 707e7e966026 ("bnxt_en: Always forward VF MAC address to the PF.")
Reported-by: Seth Forshee <seth.forshee@canonical.com>
Reported-by: Siwei Liu <loseweigh@gmail.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
Please queue this for stable as well.  Thanks.

 drivers/net/ethernet/broadcom/bnxt/bnxt.c       | 9 +++++++--
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 9 +++++----
 drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h | 2 +-
 3 files changed, 13 insertions(+), 7 deletions(-)

Comments

Siwei Liu Sept. 14, 2018, 8:14 p.m. | #1
Ack. Looks fine to me.

-Siwei

On Fri, Sep 14, 2018 at 12:41 PM, Michael Chan
<michael.chan@broadcom.com> wrote:
> The recent commit to always forward the VF MAC address to the PF for
> approval may not work if the PF driver or the firmware is older.  This
> will cause the VF driver to fail during probe:
>
>   bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): hwrm req_type 0xf seq id 0x5 error 0xffff
>   bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): VF MAC address 00:00:17:02:05:d0 not approved by the PF
>   bnxt_en 0000:00:03.0: Unable to initialize mac address.
>   bnxt_en: probe of 0000:00:03.0 failed with error -99
>
> We fix it by treating the error as fatal only if the VF MAC address is
> locally generated by the VF.
>
> Fixes: 707e7e966026 ("bnxt_en: Always forward VF MAC address to the PF.")
> Reported-by: Seth Forshee <seth.forshee@canonical.com>
> Reported-by: Siwei Liu <loseweigh@gmail.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> Please queue this for stable as well.  Thanks.
>
>  drivers/net/ethernet/broadcom/bnxt/bnxt.c       | 9 +++++++--
>  drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 9 +++++----
>  drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h | 2 +-
>  3 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index cecbb1d..177587f 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -8027,7 +8027,7 @@ static int bnxt_change_mac_addr(struct net_device *dev, void *p)
>         if (ether_addr_equal(addr->sa_data, dev->dev_addr))
>                 return 0;
>
> -       rc = bnxt_approve_mac(bp, addr->sa_data);
> +       rc = bnxt_approve_mac(bp, addr->sa_data, true);
>         if (rc)
>                 return rc;
>
> @@ -8827,14 +8827,19 @@ static int bnxt_init_mac_addr(struct bnxt *bp)
>         } else {
>  #ifdef CONFIG_BNXT_SRIOV
>                 struct bnxt_vf_info *vf = &bp->vf;
> +               bool strict_approval = true;
>
>                 if (is_valid_ether_addr(vf->mac_addr)) {
>                         /* overwrite netdev dev_addr with admin VF MAC */
>                         memcpy(bp->dev->dev_addr, vf->mac_addr, ETH_ALEN);
> +                       /* Older PF driver or firmware may not approve this
> +                        * correctly.
> +                        */
> +                       strict_approval = false;
>                 } else {
>                         eth_hw_addr_random(bp->dev);
>                 }
> -               rc = bnxt_approve_mac(bp, bp->dev->dev_addr);
> +               rc = bnxt_approve_mac(bp, bp->dev->dev_addr, strict_approval);
>  #endif
>         }
>         return rc;
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> index fcd085a..3962f6f 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
> @@ -1104,7 +1104,7 @@ void bnxt_update_vf_mac(struct bnxt *bp)
>         mutex_unlock(&bp->hwrm_cmd_lock);
>  }
>
> -int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
> +int bnxt_approve_mac(struct bnxt *bp, u8 *mac, bool strict)
>  {
>         struct hwrm_func_vf_cfg_input req = {0};
>         int rc = 0;
> @@ -1122,12 +1122,13 @@ int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
>         memcpy(req.dflt_mac_addr, mac, ETH_ALEN);
>         rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
>  mac_done:
> -       if (rc) {
> +       if (rc && strict) {
>                 rc = -EADDRNOTAVAIL;
>                 netdev_warn(bp->dev, "VF MAC address %pM not approved by the PF\n",
>                             mac);
> +               return rc;
>         }
> -       return rc;
> +       return 0;
>  }
>  #else
>
> @@ -1144,7 +1145,7 @@ void bnxt_update_vf_mac(struct bnxt *bp)
>  {
>  }
>
> -int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
> +int bnxt_approve_mac(struct bnxt *bp, u8 *mac, bool strict)
>  {
>         return 0;
>  }
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
> index e9b20cd..2eed9ed 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
> @@ -39,5 +39,5 @@ int bnxt_sriov_configure(struct pci_dev *pdev, int num_vfs);
>  void bnxt_sriov_disable(struct bnxt *);
>  void bnxt_hwrm_exec_fwd_req(struct bnxt *);
>  void bnxt_update_vf_mac(struct bnxt *);
> -int bnxt_approve_mac(struct bnxt *, u8 *);
> +int bnxt_approve_mac(struct bnxt *, u8 *, bool);
>  #endif
> --
> 2.5.1
>
David Miller Sept. 17, 2018, 2:58 p.m. | #2
From: Michael Chan <michael.chan@broadcom.com>
Date: Fri, 14 Sep 2018 15:41:29 -0400

> The recent commit to always forward the VF MAC address to the PF for
> approval may not work if the PF driver or the firmware is older.  This
> will cause the VF driver to fail during probe:
> 
>   bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): hwrm req_type 0xf seq id 0x5 error 0xffff
>   bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): VF MAC address 00:00:17:02:05:d0 not approved by the PF
>   bnxt_en 0000:00:03.0: Unable to initialize mac address.
>   bnxt_en: probe of 0000:00:03.0 failed with error -99
> 
> We fix it by treating the error as fatal only if the VF MAC address is
> locally generated by the VF.
> 
> Fixes: 707e7e966026 ("bnxt_en: Always forward VF MAC address to the PF.")
> Reported-by: Seth Forshee <seth.forshee@canonical.com>
> Reported-by: Siwei Liu <loseweigh@gmail.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> Please queue this for stable as well.  Thanks.

Applied and queued up for -stable.

Patch

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index cecbb1d..177587f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -8027,7 +8027,7 @@  static int bnxt_change_mac_addr(struct net_device *dev, void *p)
 	if (ether_addr_equal(addr->sa_data, dev->dev_addr))
 		return 0;
 
-	rc = bnxt_approve_mac(bp, addr->sa_data);
+	rc = bnxt_approve_mac(bp, addr->sa_data, true);
 	if (rc)
 		return rc;
 
@@ -8827,14 +8827,19 @@  static int bnxt_init_mac_addr(struct bnxt *bp)
 	} else {
 #ifdef CONFIG_BNXT_SRIOV
 		struct bnxt_vf_info *vf = &bp->vf;
+		bool strict_approval = true;
 
 		if (is_valid_ether_addr(vf->mac_addr)) {
 			/* overwrite netdev dev_addr with admin VF MAC */
 			memcpy(bp->dev->dev_addr, vf->mac_addr, ETH_ALEN);
+			/* Older PF driver or firmware may not approve this
+			 * correctly.
+			 */
+			strict_approval = false;
 		} else {
 			eth_hw_addr_random(bp->dev);
 		}
-		rc = bnxt_approve_mac(bp, bp->dev->dev_addr);
+		rc = bnxt_approve_mac(bp, bp->dev->dev_addr, strict_approval);
 #endif
 	}
 	return rc;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
index fcd085a..3962f6f 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
@@ -1104,7 +1104,7 @@  void bnxt_update_vf_mac(struct bnxt *bp)
 	mutex_unlock(&bp->hwrm_cmd_lock);
 }
 
-int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
+int bnxt_approve_mac(struct bnxt *bp, u8 *mac, bool strict)
 {
 	struct hwrm_func_vf_cfg_input req = {0};
 	int rc = 0;
@@ -1122,12 +1122,13 @@  int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
 	memcpy(req.dflt_mac_addr, mac, ETH_ALEN);
 	rc = hwrm_send_message(bp, &req, sizeof(req), HWRM_CMD_TIMEOUT);
 mac_done:
-	if (rc) {
+	if (rc && strict) {
 		rc = -EADDRNOTAVAIL;
 		netdev_warn(bp->dev, "VF MAC address %pM not approved by the PF\n",
 			    mac);
+		return rc;
 	}
-	return rc;
+	return 0;
 }
 #else
 
@@ -1144,7 +1145,7 @@  void bnxt_update_vf_mac(struct bnxt *bp)
 {
 }
 
-int bnxt_approve_mac(struct bnxt *bp, u8 *mac)
+int bnxt_approve_mac(struct bnxt *bp, u8 *mac, bool strict)
 {
 	return 0;
 }
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
index e9b20cd..2eed9ed 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.h
@@ -39,5 +39,5 @@  int bnxt_sriov_configure(struct pci_dev *pdev, int num_vfs);
 void bnxt_sriov_disable(struct bnxt *);
 void bnxt_hwrm_exec_fwd_req(struct bnxt *);
 void bnxt_update_vf_mac(struct bnxt *);
-int bnxt_approve_mac(struct bnxt *, u8 *);
+int bnxt_approve_mac(struct bnxt *, u8 *, bool);
 #endif