Page MenuHomeFreeBSD
Differential D48902

tests: Guard against testing without scapy
AbandonedPublic
Actions

Authored by jlduran on Feb 8 2025, 5:49 PM.
Tags
None
Referenced Files
Unknown Object (File)
Apr 7 2025, 10:19 AM
Unknown Object (File)
Apr 4 2025, 4:39 AM
Unknown Object (File)
Mar 5 2025, 9:18 AM
Unknown Object (File)
Feb 25 2025, 10:02 AM
Unknown Object (File)
Feb 20 2025, 3:23 PM
Unknown Object (File)
Feb 13 2025, 7:07 PM
Unknown Object (File)
Feb 13 2025, 1:15 AM
Unknown Object (File)
Feb 10 2025, 10:40 PM
Subscribers
farrokhi
glebius
imp
melifaro
ngie

Details

Reviewers
emaste
kp
vegeta_tuxpowered.net
Group Reviewers
tests
Summary

Add a guard that checks if a system has scapy installed, to avoid
reporting false positives in the testing framework.

This guard is added in the utilities subroutine instead of the
traditional:

atf_set "require.progs" "python3" "scapy"

in the header of each test using pft_ping.py.

The trade-off is that the test takes a little longer to be skipped.

Reported by: Jenkins

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 62316
Build 59200: arc lint + arc unit

Event Timeline

jlduran created this revision.Feb 8 2025, 5:49 PM
Herald added subscribers: glebius, melifaro, farrokhi, imp. · View Herald TranscriptFeb 8 2025, 5:49 PM
jlduran requested review of this revision.Feb 8 2025, 5:49 PM
Harbormaster completed remote builds in B62316: Diff 150721.Feb 8 2025, 5:50 PM
jlduran added a comment.Feb 8 2025, 5:50 PM

Current false positives (on architechtures other than amd64 or i386 [^1]):

  • sys.netpfil.pf.fragmentation_pass.dummynet_fragmented
  • sys.netpfil.pf.src_track.max_src_states_rule
  • sys.netpfil.pf.src_track.max_src_conn_rule
  • sys.netpfil.pf.max_states.max_states
  • sys.netpfil.pf.route_to.ifbound_reply_to_rdr_dummynet
  • sys.netpfil.pf.route_to.ifbound_reply_to
  • sys.netpfil.pf.route_to.ifbound_reply_to_v6
  • sys.netpfil.pf.fragmentation_pass.no_df
  • sys.netpfil.pf.fragmentation_pass.reassemble_slowpath

If tests strongly prefers to add atf_set "require.progs" "python3" "scapy", I'll change the revision. However, this review aims to avoid having this issue recurrently.

[^1]: See https://github.com/freebsd/freebsd-ci/blob/476c8808803a47344a47adfe9573345cceb14fce/scripts/build/build-test_image-head.sh#L63-L64 for more info.

kp added a comment.Feb 10 2025, 9:06 AM

I think there's a preference for expressing this in the head of the test, not the body. Igor is thinking about ways to teach kyua to satisfy these requirements automatically, which it won't be able to do if the test is done at run-time. This approach also leaves us with tests that execute part of the test and then potentially skip the rest of it.

Also, as the scapy package depends on the python one I don't see the need to also depend on python3.

glebius added a comment.Feb 10 2025, 7:22 PM

Thanks for working on this! Once you come to an agreement on how to properly handle that, can you please also look at other tests that use scapy, outside of pf? There are quite a lot in netinet, netinet6 and net tests.

ngie added a subscriber: ngie.Feb 10 2025, 7:37 PM

What is the benefit of doing it this way vs the other way using require.progs?

ngie added a comment.Feb 10 2025, 7:38 PM

Sidenote: you can require the program in the Kyuafile as well. That's generally the best way/least amount of code to ensure that the requirement is actually being met.

glebius added a comment.Feb 10 2025, 7:41 PM
In D48902#1115700, @ngie wrote:

Sidenote: you can require the program in the Kyuafile as well. That's generally the best way/least amount of code to ensure that the requirement is actually being met.

This seems a much better way to me. Without even invoking a test itself.

jlduran abandoned this revision.Feb 10 2025, 10:15 PM

Abandoned in favor of: D48917

In D48902#1115693, @glebius wrote:

Thanks for working on this! Once you come to an agreement on how to properly handle that, can you please also look at other tests that use scapy, outside of pf? There are quite a lot in netinet, netinet6 and net tests.

The only other failing test I can see is:
sys.net.routing.test_routing_l3.py.TestIfOps::test_change_prefix_route[inet], already addressed by D48650.
The others use the atf_python testing framework (based on pytest), that already takes care of this issue.

In D48902#1115699, @ngie wrote:

What is the benefit of doing it this way vs the other way using require.progs?

The idea was to avoid having to require.progs as this is often forgotten.

jlduran added a comment.Feb 11 2025, 5:44 PM
In D48902#1115693, @glebius wrote:

Thanks for working on this! Once you come to an agreement on how to properly handle that, can you please also look at other tests that use scapy, outside of pf? There are quite a lot in netinet, netinet6 and net tests.

I see them now. Will fix them. Thank you!

Revision Contents
Changeset List

PathSize
tests/
sys/
netpfil/
pf/
10 lines

Diff 150721

View Options

tests/sys/netpfil/pf/utils.subr

Loading...