mbox series

[SRU,Kinetic,0/3] Fix udpgro_frglist.sh kernel selftest

Message ID 20230208181338.476272-1-andrei.gherzan@canonical.com
Headers show
Series Fix udpgro_frglist.sh kernel selftest | expand

Message

Andrei Gherzan Feb. 8, 2023, 6:13 p.m. UTC
Buglink: https://bugs.launchpad.net/bugs/1996536
Buglink: https://bugs.launchpad.net/bugs/2000708

SRU Justification:

[Impact]

* The udpgro_frglist.sh was failing due to not being able to find a
dependency: nat6to4. This dependency was broken upstream due to
where it gets compiled but also disabled by us in lp:1996536 for lack of
CC support.

[Fix]

* This series backports a couple of patches fixing the expected location
of nat6to4 for udpgro_frglist.sh and also adding CC support for nat6to4.
* All patches apply cleanly (with some fuzz) except
3c107f36db061603bee7564fbd6388b1f1879fd3 which requires minimum context
changes due to tests changes in the net Makefile.

[Test Plan]

* These patches were tested by building the kernel, the tests
required for udpgro_frglist.sh and running the test successfully in a
clean VM.

[Where problems could occur]

* The regression can be considered as low, since it would affect only
some tests.

Björn Töpel (2):
  selftests: net: Add cross-compilation support for BPF programs
  selftests: net: Fix O=dir builds

Hangbin Liu (1):
  selftests/net: mv bpf/nat6to4.c to net folder

 tools/testing/selftests/net/Makefile          | 50 ++++++++++++++++++-
 tools/testing/selftests/net/bpf/Makefile      | 15 ------
 .../testing/selftests/net/{bpf => }/nat6to4.c |  0
 tools/testing/selftests/net/udpgro_frglist.sh |  8 +--
 4 files changed, 52 insertions(+), 21 deletions(-)
 delete mode 100644 tools/testing/selftests/net/bpf/Makefile
 rename tools/testing/selftests/net/{bpf => }/nat6to4.c (100%)

Comments

Tim Gardner Feb. 8, 2023, 6:27 p.m. UTC | #1
On 2/8/23 11:13 AM, Andrei Gherzan wrote:
> Buglink: https://bugs.launchpad.net/bugs/1996536
> Buglink: https://bugs.launchpad.net/bugs/2000708
> 
> SRU Justification:
> 
> [Impact]
> 
> * The udpgro_frglist.sh was failing due to not being able to find a
> dependency: nat6to4. This dependency was broken upstream due to
> where it gets compiled but also disabled by us in lp:1996536 for lack of
> CC support.
> 
> [Fix]
> 
> * This series backports a couple of patches fixing the expected location
> of nat6to4 for udpgro_frglist.sh and also adding CC support for nat6to4.
> * All patches apply cleanly (with some fuzz) except
> 3c107f36db061603bee7564fbd6388b1f1879fd3 which requires minimum context
> changes due to tests changes in the net Makefile.
> 
> [Test Plan]
> 
> * These patches were tested by building the kernel, the tests
> required for udpgro_frglist.sh and running the test successfully in a
> clean VM.
> 
> [Where problems could occur]
> 
> * The regression can be considered as low, since it would affect only
> some tests.
> 
> Björn Töpel (2):
>    selftests: net: Add cross-compilation support for BPF programs
>    selftests: net: Fix O=dir builds
> 
> Hangbin Liu (1):
>    selftests/net: mv bpf/nat6to4.c to net folder
> 
>   tools/testing/selftests/net/Makefile          | 50 ++++++++++++++++++-
>   tools/testing/selftests/net/bpf/Makefile      | 15 ------
>   .../testing/selftests/net/{bpf => }/nat6to4.c |  0
>   tools/testing/selftests/net/udpgro_frglist.sh |  8 +--
>   4 files changed, 52 insertions(+), 21 deletions(-)
>   delete mode 100644 tools/testing/selftests/net/bpf/Makefile
>   rename tools/testing/selftests/net/{bpf => }/nat6to4.c (100%)
> 
Acked-by: Tim Gardner <tim.gardner@canonical.com>

Patch 3 is now in linux-next
Kamal Mostafa Feb. 8, 2023, 8:07 p.m. UTC | #2
LGTM.  Thanks for addressing this, Andrei.

Acked-by: Kamal Mostafa <kamal@canonical.com>

 -Kamal

On Wed, Feb 8, 2023 at 10:14 AM Andrei Gherzan <andrei.gherzan@canonical.com>
wrote:

> Buglink: https://bugs.launchpad.net/bugs/1996536
> Buglink: https://bugs.launchpad.net/bugs/2000708
>
> SRU Justification:
>
> [Impact]
>
> * The udpgro_frglist.sh was failing due to not being able to find a
> dependency: nat6to4. This dependency was broken upstream due to
> where it gets compiled but also disabled by us in lp:1996536 for lack of
> CC support.
>
> [Fix]
>
> * This series backports a couple of patches fixing the expected location
> of nat6to4 for udpgro_frglist.sh and also adding CC support for nat6to4.
> * All patches apply cleanly (with some fuzz) except
> 3c107f36db061603bee7564fbd6388b1f1879fd3 which requires minimum context
> changes due to tests changes in the net Makefile.
>
> [Test Plan]
>
> * These patches were tested by building the kernel, the tests
> required for udpgro_frglist.sh and running the test successfully in a
> clean VM.
>
> [Where problems could occur]
>
> * The regression can be considered as low, since it would affect only
> some tests.
>
> Björn Töpel (2):
>   selftests: net: Add cross-compilation support for BPF programs
>   selftests: net: Fix O=dir builds
>
> Hangbin Liu (1):
>   selftests/net: mv bpf/nat6to4.c to net folder
>
>  tools/testing/selftests/net/Makefile          | 50 ++++++++++++++++++-
>  tools/testing/selftests/net/bpf/Makefile      | 15 ------
>  .../testing/selftests/net/{bpf => }/nat6to4.c |  0
>  tools/testing/selftests/net/udpgro_frglist.sh |  8 +--
>  4 files changed, 52 insertions(+), 21 deletions(-)
>  delete mode 100644 tools/testing/selftests/net/bpf/Makefile
>  rename tools/testing/selftests/net/{bpf => }/nat6to4.c (100%)
>
> --
> 2.34.1
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>
Stefan Bader Feb. 10, 2023, 8:44 a.m. UTC | #3
Patch #2 is missing the BugLink, which of the 2 should be used when 
applying? Generally, it would be better to either duplicate bug reports 
or submit individually. One thing to fix per submission is simpler to 
manage.

-Stefan

On 08.02.23 19:13, Andrei Gherzan wrote:
> Buglink: https://bugs.launchpad.net/bugs/1996536
> Buglink: https://bugs.launchpad.net/bugs/2000708
>
> SRU Justification:
>
> [Impact]
>
> * The udpgro_frglist.sh was failing due to not being able to find a
> dependency: nat6to4. This dependency was broken upstream due to
> where it gets compiled but also disabled by us in lp:1996536 for lack of
> CC support.
>
> [Fix]
>
> * This series backports a couple of patches fixing the expected location
> of nat6to4 for udpgro_frglist.sh and also adding CC support for nat6to4.
> * All patches apply cleanly (with some fuzz) except
> 3c107f36db061603bee7564fbd6388b1f1879fd3 which requires minimum context
> changes due to tests changes in the net Makefile.
>
> [Test Plan]
>
> * These patches were tested by building the kernel, the tests
> required for udpgro_frglist.sh and running the test successfully in a
> clean VM.
>
> [Where problems could occur]
>
> * The regression can be considered as low, since it would affect only
> some tests.
>
> Björn Töpel (2):
>    selftests: net: Add cross-compilation support for BPF programs
>    selftests: net: Fix O=dir builds
>
> Hangbin Liu (1):
>    selftests/net: mv bpf/nat6to4.c to net folder
>
>   tools/testing/selftests/net/Makefile          | 50 ++++++++++++++++++-
>   tools/testing/selftests/net/bpf/Makefile      | 15 ------
>   .../testing/selftests/net/{bpf => }/nat6to4.c |  0
>   tools/testing/selftests/net/udpgro_frglist.sh |  8 +--
>   4 files changed, 52 insertions(+), 21 deletions(-)
>   delete mode 100644 tools/testing/selftests/net/bpf/Makefile
>   rename tools/testing/selftests/net/{bpf => }/nat6to4.c (100%)
>
Andrei Gherzan Feb. 10, 2023, 9:38 a.m. UTC | #4
Hi Stefan,

Thanks for the review.

On Fri, 10 Feb 2023, 08:44 Stefan Bader, <stefan.bader@canonical.com> wrote:

> Patch #2 is missing the BugLink, which of the 2 should be used when
> applying? Generally, it would be better to either duplicate bug reports
> or submit individually. One thing to fix per submission is simpler to
> manage.
>

I avoided a bug link on that patch because it is a fix of a fix. It is a
fix of the CC support for net bpf progs. Now that I think about it, I
reckon we should go ahead and use the same BugLink with the CC support -
https://bugs.launchpad.net/bugs/1996536. I'm happy to resubmit a V2 with
this change.

I understand and support the maintenance advantage of talking one bug per
submission but in this case there was a tight relationship between these
two entries. One failed because of a dependency disabled by another. So
being able to fix and test the former required work on the latter. I hoped
that this was an acceptable balance of submission and maintenance/review
overhead. Similarly though, I'm happy to split and resubmit if deemed
otherwise.

agh
Stefan Bader Feb. 13, 2023, 10:59 a.m. UTC | #5
On 10.02.23 10:38, Andrei Gherzan wrote:
> Hi Stefan,
> 
> Thanks for the review.
> 
> On Fri, 10 Feb 2023, 08:44 Stefan Bader, <stefan.bader@canonical.com 
> <mailto:stefan.bader@canonical.com>> wrote:
> 
>     Patch #2 is missing the BugLink, which of the 2 should be used when
>     applying? Generally, it would be better to either duplicate bug reports
>     or submit individually. One thing to fix per submission is simpler to
>     manage.
> 
> 
> I avoided a bug link on that patch because it is a fix of a fix. It is a 
> fix of the CC support for net bpf progs. Now that I think about it, I 
> reckon we should go ahead and use the same BugLink with the CC support - 
> https://bugs.launchpad.net/bugs/1996536 
> <https://bugs.launchpad.net/bugs/1996536>. I'm happy to resubmit a V2 
> with this change.

Since it is clarified now, there is no need to re-submit. We just fix it 
up when applying. We always have to use some reference since that is 
what puts things together in the changelog. And we also need it as SRU 
rules say that no change should be made without a bug report.

> 
> I understand and support the maintenance advantage of talking one bug 
> per submission but in this case there was a tight relationship between 
> these two entries. One failed because of a dependency disabled by 
> another. So being able to fix and test the former required work on the 
> latter. I hoped that this was an acceptable balance of submission and 
> maintenance/review overhead. Similarly though, I'm happy to split and 
> resubmit if deemed otherwise.

Again it is ok as it now. And in this case there is good reason to have 
done so. Just for the general case it is hard to double check on the bug 
references.

> 
> agh

- Stefan
Luke Nowakowski-Krijger Feb. 16, 2023, 10:23 p.m. UTC | #6
Applied to kinetic linux master-next

I applied the "Buglink: https://bugs.launchpad.net/bugs/1996536" to the
second patch and adjusted the buglink in the first patch to the standard
buglink format.

Thanks,
-Luke

On Wed, Feb 8, 2023 at 10:14 AM Andrei Gherzan <andrei.gherzan@canonical.com>
wrote:

> Buglink: https://bugs.launchpad.net/bugs/1996536
> Buglink: https://bugs.launchpad.net/bugs/2000708
>
> SRU Justification:
>
> [Impact]
>
> * The udpgro_frglist.sh was failing due to not being able to find a
> dependency: nat6to4. This dependency was broken upstream due to
> where it gets compiled but also disabled by us in lp:1996536 for lack of
> CC support.
>
> [Fix]
>
> * This series backports a couple of patches fixing the expected location
> of nat6to4 for udpgro_frglist.sh and also adding CC support for nat6to4.
> * All patches apply cleanly (with some fuzz) except
> 3c107f36db061603bee7564fbd6388b1f1879fd3 which requires minimum context
> changes due to tests changes in the net Makefile.
>
> [Test Plan]
>
> * These patches were tested by building the kernel, the tests
> required for udpgro_frglist.sh and running the test successfully in a
> clean VM.
>
> [Where problems could occur]
>
> * The regression can be considered as low, since it would affect only
> some tests.
>
> Björn Töpel (2):
>   selftests: net: Add cross-compilation support for BPF programs
>   selftests: net: Fix O=dir builds
>
> Hangbin Liu (1):
>   selftests/net: mv bpf/nat6to4.c to net folder
>
>  tools/testing/selftests/net/Makefile          | 50 ++++++++++++++++++-
>  tools/testing/selftests/net/bpf/Makefile      | 15 ------
>  .../testing/selftests/net/{bpf => }/nat6to4.c |  0
>  tools/testing/selftests/net/udpgro_frglist.sh |  8 +--
>  4 files changed, 52 insertions(+), 21 deletions(-)
>  delete mode 100644 tools/testing/selftests/net/bpf/Makefile
>  rename tools/testing/selftests/net/{bpf => }/nat6to4.c (100%)
>
> --
> 2.34.1
>
>
> --
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>