netdev - Re: [PATCH net] bnxt_en: Fix VF mac address regression.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADGSJ22a-oi2REKJnXSVyX3Y+y_Nu+o0cwx9zcdJVdeaHrEdXg@mail.gmail.com>
Date:   Fri, 14 Sep 2018 13:14:53 -0700
From:   Siwei Liu <loseweigh@...il.com>
To:     Michael Chan <michael.chan@...adcom.com>
Cc:     David Miller <davem@...emloft.net>,
        Netdev <netdev@...r.kernel.org>, seth.forshee@...onical.com,
        si-wei liu <si-wei.liu@...cle.com>
Subject: Re: [PATCH net] bnxt_en: Fix VF mac address regression.

Ack. Looks fine to me.

-Siwei

On Fri, Sep 14, 2018 at 12:41 PM, Michael Chan
<michael.chan@...adcom.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@...onical.com>
> Reported-by: Siwei Liu <loseweigh@...il.com>
> Signed-off-by: Michael Chan <michael.chan@...adcom.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
>