mbox series

[v1,0/1,SRU,F:linux-bluefield,1/1] UBUNTU: SAUCE: Fix the rshim logging buffer full/wraparound issue

Message ID cover.1616024156.git.limings@nvidia.com
Headers show
Series UBUNTU: SAUCE: Fix the rshim logging buffer full/wraparound issue | expand

Message

Liming Sun March 17, 2021, 11:44 p.m. UTC 
Buglink: https://bugs.launchpad.net/bugs/1919847

SRU Justification:

[Impact]
* When 'cat /dev/rshim0/misc' with DISPLAY_LEVEL set to 2, the logging
  buffer could be potentially displayed as empty when it's full.

[Fix]
* Add the check properly to make sure the buffer won't wrap-around.

[Test Case]
* Run "bfrshlog <some-message>" repeatedly and check the
  /dev/rshim0/misc with DISPLAY_LEVEL set to 2 in this file. The
  buffer shouldn't be overwritten or wrap-around.

[Regression Potential]
* Yes, it should be easy to add regression by using the bfrshlog to
  add logs on BlueField side, and check the logging buffer 
  (/dev/rshim0/misc) from external rshim host.

[Other]
* N/A

Comments

Stefan Bader March 18, 2021, 8:53 a.m. UTC | #1 
On 18.03.21 00:44, Liming Sun wrote:
> Buglink: https://bugs.launchpad.net/bugs/1919847

   ^ The correct tag is BugLink (capital B and L)
> 
> SRU Justification:
> 
> [Impact]
> * When 'cat /dev/rshim0/misc' with DISPLAY_LEVEL set to 2, the logging
>    buffer could be potentially displayed as empty when it's full.
> 
> [Fix]
> * Add the check properly to make sure the buffer won't wrap-around.
> 
> [Test Case]
> * Run "bfrshlog <some-message>" repeatedly and check the
>    /dev/rshim0/misc with DISPLAY_LEVEL set to 2 in this file. The
>    buffer shouldn't be overwritten or wrap-around.
> 
> [Regression Potential]
> * Yes, it should be easy to add regression by using the bfrshlog to
>    add logs on BlueField side, and check the logging buffer
>    (/dev/rshim0/misc) from external rshim host.

The regression potential is there to describe how regressions might manifest if 
they happen. IOW when the attempted fix is incorrect or has side effects, which 
area(s) would show this? Intention is to help regression testing to not only 
check that the bug has gone away but also that nothing new has been introduced 
by it.

The BugLink, we can fix when applying. Please fix the regression potential in 
the bug report (that seems to have even less useful information that this 
submission).

Acked-by: Stefan Bader <stefan.bader@canonical.com>

-Stefan
> 
> [Other]
> * N/A
>
Tim Gardner March 18, 2021, 11:34 a.m. UTC | #2 
Acked-by: Tim Gardner <tim.gardner@canonical.com>

On 3/17/21 5:44 PM, Liming Sun wrote:
> Buglink: https://bugs.launchpad.net/bugs/1919847
> 
> SRU Justification:
> 
> [Impact]
> * When 'cat /dev/rshim0/misc' with DISPLAY_LEVEL set to 2, the logging
>    buffer could be potentially displayed as empty when it's full.
> 
> [Fix]
> * Add the check properly to make sure the buffer won't wrap-around.
> 
> [Test Case]
> * Run "bfrshlog <some-message>" repeatedly and check the
>    /dev/rshim0/misc with DISPLAY_LEVEL set to 2 in this file. The
>    buffer shouldn't be overwritten or wrap-around.
> 
> [Regression Potential]
> * Yes, it should be easy to add regression by using the bfrshlog to
>    add logs on BlueField side, and check the logging buffer
>    (/dev/rshim0/misc) from external rshim host.
> 
> [Other]
> * N/A
>
Liming Sun March 18, 2021, 12:59 p.m. UTC | #3 
> -----Original Message-----
> From: Stefan Bader <stefan.bader@canonical.com>
> Sent: Thursday, March 18, 2021 4:53 AM
> To: Liming Sun <limings@nvidia.com>; kernel-team@lists.ubuntu.com
> Subject: ACK/Cmnt: [PATCH v1 0/1] [SRU][F:linux-bluefield][PATCH 1/1]
> UBUNTU: SAUCE: Fix the rshim logging buffer full/wraparound issue
> 
> On 18.03.21 00:44, Liming Sun wrote:
> > Buglink: https://bugs.launchpad.net/bugs/1919847
> 
>    ^ The correct tag is BugLink (capital B and L)
> >
> > SRU Justification:
> >
> > [Impact]
> > * When 'cat /dev/rshim0/misc' with DISPLAY_LEVEL set to 2, the logging
> >    buffer could be potentially displayed as empty when it's full.
> >
> > [Fix]
> > * Add the check properly to make sure the buffer won't wrap-around.
> >
> > [Test Case]
> > * Run "bfrshlog <some-message>" repeatedly and check the
> >    /dev/rshim0/misc with DISPLAY_LEVEL set to 2 in this file. The
> >    buffer shouldn't be overwritten or wrap-around.
> >
> > [Regression Potential]
> > * Yes, it should be easy to add regression by using the bfrshlog to
> >    add logs on BlueField side, and check the logging buffer
> >    (/dev/rshim0/misc) from external rshim host.
> 
> The regression potential is there to describe how regressions might manifest if
> they happen. IOW when the attempted fix is incorrect or has side effects,
> which
> area(s) would show this? Intention is to help regression testing to not only
> check that the bug has gone away but also that nothing new has been
> introduced
> by it.
> 
> The BugLink, we can fix when applying. Please fix the regression potential in
> the bug report (that seems to have even less useful information that this
> submission).

Thanks!
Updated the  regression potential in  https://bugs.launchpad.net/bugs/1919847.

> 
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
> 
> -Stefan
> >
> > [Other]
> > * N/A
> >
>
Tim Gardner March 19, 2021, 7:05 p.m. UTC | #4 
Applied to focal/linux-bluefield-master-next. Thanks.

Fixed BugLink.

-rtg

On 3/17/21 5:44 PM, Liming Sun wrote:
> Buglink: https://bugs.launchpad.net/bugs/1919847
> 
> SRU Justification:
> 
> [Impact]
> * When 'cat /dev/rshim0/misc' with DISPLAY_LEVEL set to 2, the logging
>    buffer could be potentially displayed as empty when it's full.
> 
> [Fix]
> * Add the check properly to make sure the buffer won't wrap-around.
> 
> [Test Case]
> * Run "bfrshlog <some-message>" repeatedly and check the
>    /dev/rshim0/misc with DISPLAY_LEVEL set to 2 in this file. The
>    buffer shouldn't be overwritten or wrap-around.
> 
> [Regression Potential]
> * Yes, it should be easy to add regression by using the bfrshlog to
>    add logs on BlueField side, and check the logging buffer
>    (/dev/rshim0/misc) from external rshim host.
> 
> [Other]
> * N/A
>