netdev - Re: [PATCH net-next 03/14] bnxt_en: Refactor bnxt_sriov

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]

Date:   Sun, 25 Aug 2019 23:06:34 -0700
From:   Michael Chan <michael.chan@...adcom.com>
To:     David Miller <davem@...emloft.net>
Cc:     Netdev <netdev@...r.kernel.org>,
        Vasundhara Volam <vasundhara-v.volam@...adcom.com>,
        Jiri Pirko <jiri@...lanox.com>, Ray Jui <ray.jui@...adcom.com>
Subject: Re: [PATCH net-next 03/14] bnxt_en: Refactor bnxt_sriov_enable().

On Sun, Aug 25, 2019 at 10:36 PM David Miller <davem@...emloft.net> wrote:
>
> From: Michael Chan <michael.chan@...adcom.com>
> Date: Sun, 25 Aug 2019 23:54:54 -0400
>
> > @@ -687,6 +687,32 @@ static int bnxt_func_cfg(struct bnxt *bp, int num_vfs)
> >               return bnxt_hwrm_func_cfg(bp, num_vfs);
> >  }
> >
> > +int bnxt_cfg_hw_sriov(struct bnxt *bp, int *num_vfs)
> > +{
> > +     int rc;
> > +
> > +     /* Register buffers for VFs */
> > +     rc = bnxt_hwrm_func_buf_rgtr(bp);
> > +     if (rc)
> > +             return rc;
> > +
> > +     /* Reserve resources for VFs */
> > +     rc = bnxt_func_cfg(bp, *num_vfs);
> > +     if (rc != *num_vfs) {
>
> I notice that these two operations are reversed here from where they were in the
> bnxt_sriov_enable() function.  Does the BUF_RGTR operation have to be undone if
> the bnxt_func_cfg() fails?
>
> When it's not a straight extraction of code into a helper function one really
> should do one of two things in my opinion:
>
> 1) Explain the differences in the commit message.
>
> 2) Do a straight extration in one commit, change the ordering in another.

OK.  Will break it up into 2 commits with explanations.  The reason is
that during normal init, the PF is always initialized first and the
exact bring-up sequence does not matter too much.  During error
recovery, the PF and VFs can be all trying to recover at about the
same time and the sequence matters more.  Will explain all of this
again in v2.  Thanks.