Bug 236842

Summary: check if_tap.ko for sys/netmap/ctrl-api-test test
Product: Base System Reporter: Olivier Cochard <olivier>
Component: testsAssignee: Olivier Cochard <olivier>
Status: Closed FIXED    
Severity: Affects Only Me CC: asomers, emaste, ngie, vmaffione
Priority: ---    
Version: 12.0-RELEASE   
Hardware: Any   
OS: Any   
Attachments:
Description Flags
patch to skip sys/netmap/ctrl-api-test if if_tap.ko not installed
none
patch using PLAIN_REQUIRE_KERNEL_MODULE none

Description Olivier Cochard freebsd_committer freebsd_triage 2019-03-27 23:38:48 UTC
Created attachment 203202 [details]
patch to skip sys/netmap/ctrl-api-test  if if_tap.ko not installed

On nanobsd, with a custom installation without if_tap, this test failed.
Little patch to check for /boot/kernel/if_tap.ko.
Notice this test will be wrongly skipped in this special condition:
1. Module if_tap.ko is not build&installed
2. and a custom kernel including if_tap

I don't know how to use 'advanced' (script or command line result) skip condition on kyua.
Comment 1 Alan Somers freebsd_committer freebsd_triage 2019-03-28 00:24:14 UTC
Comment on attachment 203202 [details]
patch to skip sys/netmap/ctrl-api-test  if if_tap.ko not installed

Instead of using "required_files", you should use PLAIN_REQUIRE_KERNEL_MODULE (from /usr/src/tests/freebsd_test_suite/macros.h).  Just stick that in the test's .c file.
Comment 2 Olivier Cochard freebsd_committer freebsd_triage 2019-03-28 11:10:24 UTC
Created attachment 203212 [details]
patch using PLAIN_REQUIRE_KERNEL_MODULE

Thanks for the tips: Here is a new patch.
Comment 3 Alan Somers freebsd_committer freebsd_triage 2019-03-28 14:33:21 UTC
Comment on attachment 203212 [details]
patch using PLAIN_REQUIRE_KERNEL_MODULE

LGTM.  You can consider this Reviewed by: me.  Do you need me to commit it as well?
Comment 4 Olivier Cochard freebsd_committer freebsd_triage 2019-03-28 14:48:23 UTC
I will commit it myself, but using "Approved by" because I've got a ports commit bit only, and "reviewed by" seems to be for mentor (according to https://www.freebsd.org/doc/en/articles/committers-guide/committer.types.html)
Comment 5 Alan Somers freebsd_committer freebsd_triage 2019-03-28 14:55:44 UTC
I can see how that section of the committer's guide is confusing.  Let me clarify:

A non-mentored src committer like can provide an "Approved by" or a "Reviewed by".  A mentored src committer like ngie can only provide a "Reviewed by".  "Reviewed by" is purely informational, but "Approved by" is needed for commits to certain areas, or for any commits by a mentored committer.

And yes I approve this commit.
Comment 6 commit-hook freebsd_committer freebsd_triage 2019-03-28 16:18:04 UTC
A commit references this bug:

Author: olivier
Date: Thu Mar 28 16:17:34 UTC 2019
New revision: 345644
URL: https://svnweb.freebsd.org/changeset/base/345644

Log:
  Skip this test if if_tap module is not available

  PR:		236842
  Approved by:	asomers
  MFC after:	1 month
  Sponsored by:	Netflix

Changes:
  head/tests/sys/netmap/Makefile
  head/tests/sys/netmap/ctrl-api-test.c
Comment 7 Enji Cooper freebsd_committer freebsd_triage 2019-03-28 16:30:56 UTC
(Adding Vincent as a heads up, since he will care about the change).
Comment 8 Olivier Cochard freebsd_committer freebsd_triage 2019-03-29 08:47:29 UTC
Thanks for your advice.
Comment 9 Enji Cooper freebsd_committer freebsd_triage 2019-03-29 14:57:52 UTC
(In reply to Olivier Cochard from comment #8)

Shouldn’t this bug remain open, but be changed to “In progress” until all MFCs are done?
Comment 10 Olivier Cochard freebsd_committer freebsd_triage 2019-03-29 15:05:29 UTC
ok, I was not aware of this. switched it back to "in progress".
Comment 11 Ed Maste freebsd_committer freebsd_triage 2019-04-01 13:42:33 UTC
(In reply to Enji Cooper from comment #9)
> Shouldn’t this bug remain open, but be changed to “In progress” until all MFCs
> are done?

Because we don't currently have tooling that tracks PR state against MFCs I don't think it matters much, and comes down to whichever approach makes most sense for the responsible committer.
Comment 12 commit-hook freebsd_committer freebsd_triage 2019-04-16 01:04:28 UTC
A commit references this bug:

Author: ngie
Date: Tue Apr 16 01:03:32 UTC 2019
New revision: 346256
URL: https://svnweb.freebsd.org/changeset/base/346256

Log:
  MFC r345644,r346061:

  r345644 (by olivier):

  Skip this test if if_tap module is not available

  PR:		236842

  r346061:

  Polish netmap(4) testcases a bit

  1. Not all kernels have netmap(4) support. Check for netmap(4) support before
     attempting to run the tests via the `PLAIN_REQUIRE_KERNEL_MODULE(..)` macro.
  2. Libraries shouldn't be added to LDFLAGS; they should be added to LIBADD
     instead. This allows the build system to evaluate dependencies for sanity.
  3. Sort some of the Makefile variables per bsd.README.

  1., in particular, will resolve failures when running this testcase on kernels
  lacking netmap(4) support, e.g., the i386 GENERIC kernels on ^/stable/11 and
  ^/stable/12.

  PR:		237129

Changes:
_U  stable/11/
  stable/11/tests/sys/netmap/Makefile
  stable/11/tests/sys/netmap/ctrl-api-test.c
Comment 13 commit-hook freebsd_committer freebsd_triage 2019-04-16 01:04:33 UTC
A commit references this bug:

Author: ngie
Date: Tue Apr 16 01:03:38 UTC 2019
New revision: 346257
URL: https://svnweb.freebsd.org/changeset/base/346257

Log:
  MFC r345644,r346061:

  r345644 (by olivier):

  Skip this test if if_tap module is not available

  PR:		236842

  r346061:

  Polish netmap(4) testcases a bit

  1. Not all kernels have netmap(4) support. Check for netmap(4) support before
     attempting to run the tests via the `PLAIN_REQUIRE_KERNEL_MODULE(..)` macro.
  2. Libraries shouldn't be added to LDFLAGS; they should be added to LIBADD
     instead. This allows the build system to evaluate dependencies for sanity.
  3. Sort some of the Makefile variables per bsd.README.

  1., in particular, will resolve failures when running this testcase on kernels
  lacking netmap(4) support, e.g., the i386 GENERIC kernels on ^/stable/11 and
  ^/stable/12.

  PR:		237129

Changes:
_U  stable/12/
  stable/12/tests/sys/netmap/Makefile
  stable/12/tests/sys/netmap/ctrl-api-test.c