netdev - Re: [PATCH net-next v2 1/9] bnxt_en: add support for storing crash dump into host memory

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: <CACKFLi=2oWO86Z_Sa77Q047cq=FVLysoRVdxERyij=gmk_ypCQ@mail.gmail.com>
Date: Tue, 20 Aug 2024 11:09:33 -0700
From: Michael Chan <michael.chan@...adcom.com>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: netdev@...r.kernel.org, edumazet@...gle.com, davem@...emloft.net, 
	kuba@...nel.org, pabeni@...hat.com, pavan.chebbi@...adcom.com, 
	andrew.gospodarek@...adcom.com, horms@...nel.org, helgaas@...nel.org, 
	Vikas Gupta <vikas.gupta@...adcom.com>, Somnath Kotur <somnath.kotur@...adcom.com>
Subject: Re: [PATCH net-next v2 1/9] bnxt_en: add support for storing crash
 dump into host memory

On Mon, Aug 19, 2024 at 3:00 AM Przemek Kitszel
<przemyslaw.kitszel@...el.com> wrote:
>
> On 8/16/24 23:28, Michael Chan wrote:
> > From: Vikas Gupta <vikas.gupta@...adcom.com>
> > +static int bnxt_alloc_crash_dump_mem(struct bnxt *bp)
> > +{
> > +     u32 mem_size = 0;
> > +     int rc;
> > +
> > +     if (!(bp->fw_dbg_cap & DBG_QCAPS_RESP_FLAGS_CRASHDUMP_HOST_DDR))
> > +             return 0;
> > +
> > +     rc = bnxt_hwrm_get_dump_len(bp, BNXT_DUMP_CRASH, &mem_size);
> > +     if (rc)
> > +             return rc;
> > +
> > +     mem_size = round_up(mem_size, 4);
> > +
> > +     if (bp->fw_crash_mem && mem_size == bp->fw_crash_len)
> > +             return 0;
> > +
> > +     bnxt_free_crash_dump_mem(bp);
>
> I would say it would be better to have the old buffer still allocated
> in case of an allocation failure for the new one (like krealloc()).
> Especially in case of shrink request.

Thanks for the review.  The original author Vikas is no longer working
for our group.  The FW crash dump memory is not just one buffer, but a
series of pages.  To resize these pages, we'll have to add a lot of
new code especially to cover the case that the page level can change.
A change in the memory size is rare.  It only happens when FW has
reset and the new running version requires a different crash dump
length.  So freeing it all and allocating again is a lot easier.  I'll
take a closer look to see if some small optimizations can be done
here.

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