netdev - Re: [PATCH v3 4/8] RDMA/bng_re: Allocate required memory resources for Firmware channel

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] [day] [month] [year] [list]

Message-ID: <CAMet4B55Usiq25oFOPMY6O_A9PUBgM3a8_pEWfbzt49DdaUtag@mail.gmail.com>
Date: Tue, 25 Nov 2025 16:05:25 +0530
From: Siva Reddy Kallam <siva.kallam@...adcom.com>
To: Simon Horman <horms@...nel.org>
Cc: leonro@...dia.com, jgg@...dia.com, linux-rdma@...r.kernel.org, 
	netdev@...r.kernel.org, vikas.gupta@...adcom.com, selvin.xavier@...adcom.com, 
	anand.subramanian@...adcom.com, usman.ansari@...adcom.com
Subject: Re: [PATCH v3 4/8] RDMA/bng_re: Allocate required memory resources
 for Firmware channel

On Mon, Nov 24, 2025 at 10:19 PM Simon Horman <horms@...nel.org> wrote:
>
> On Mon, Nov 17, 2025 at 05:11:22PM +0000, Siva Reddy Kallam wrote:
>
> ...
>
> > +static void bng_re_dev_uninit(struct bng_re_dev *rdev)
> > +{
> > +     bng_re_free_rcfw_channel(&rdev->rcfw);
> > +     bng_re_destroy_chip_ctx(rdev);
> > +     if (test_and_clear_bit(BNG_RE_FLAG_NETDEV_REGISTERED, &rdev->flags))
> > +             bnge_unregister_dev(rdev->aux_dev);
> > +}
> > +
> >  static int bng_re_dev_init(struct bng_re_dev *rdev)
> >  {
> >       int rc;
> > @@ -170,14 +184,18 @@ static int bng_re_dev_init(struct bng_re_dev *rdev)
> >
> >       bng_re_query_hwrm_version(rdev);
> >
> > +     rc = bng_re_alloc_fw_channel(&rdev->bng_res, &rdev->rcfw);
> > +     if (rc) {
> > +             ibdev_err(&rdev->ibdev,
> > +                       "Failed to allocate RCFW Channel: %#x\n", rc);
> > +             goto fail;
> > +     }
> > +
> >       return 0;
> > -}
> >
> > -static void bng_re_dev_uninit(struct bng_re_dev *rdev)
> > -{
> > -     bng_re_destroy_chip_ctx(rdev);
> > -     if (test_and_clear_bit(BNG_RE_FLAG_NETDEV_REGISTERED, &rdev->flags))
> > -             bnge_unregister_dev(rdev->aux_dev);
> > +fail:
> > +     bng_re_dev_uninit(rdev);
> > +     return rc;
>
> Hi Siva,
>
> IMHO, I think that it would best to handle unwind using a ladder
> of goto statements, that reverse the order of the incremental
> initialisation performed by this function.
>
> As is, this may not have much effect, other than seeming to duplicate
> bng_re_dev_uninit(). But I think that as bng_re_dev_init() grows,
> as it does in this patch-set, this will lead to clearer error handling
> (and ideally a lower chance of bugs later).
>
> I would also suggest that it would be best to name the label
> after what tit does, rather than somewhat general name 'fail'.

Thanks Simon. I will send a patch to fix this.
>
> >  }
> >
> >  static int bng_re_add_device(struct auxiliary_device *adev)
>
> ...

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5471 bytes)