tests: Guard against testing without scapy

[jlduran]

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

[jlduran]

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]

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]

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]

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

[ngie]

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]

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 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]

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!

tests/sys/netpfil/pf/utils.subr

 #ifdef _KERNEL
 typedef struct {
         struct iso_node *inop;
         int             fields;         /* interesting fields in this analysis */
         daddr_t         iso_ce_blk;     /* block of continuation area */
         off_t           iso_ce_off;     /* offset of continuation area */
         int             iso_ce_len;     /* length of continuation area */
         struct iso_mnt        *imp;                /* mount structure */
-        cd_ino_t        *inump;                /* inode number pointer */
+        ino_t                *inump;                /* inode number pointer */
         char            *outbuf;        /* name/symbolic link output area */
         u_short         *outlen;        /* length of above */
         u_short         maxlen;         /* maximum length of above */
         int             cont;           /* continuation of above */
 } ISO_RRIP_ANALYZE;
 
 struct iso_directory_record;
 
 int cd9660_rrip_analyze(struct iso_directory_record *isodir,
                             struct iso_node *inop, struct iso_mnt *imp);
 int cd9660_rrip_getname(struct iso_directory_record *isodir,
                             char *outbuf, u_short *outlen,
-                            cd_ino_t *inump, struct iso_mnt *imp);
+                            ino_t *inump, struct iso_mnt *imp);
 int cd9660_rrip_getsymname(struct iso_directory_record *isodir,
                                char *outbuf, u_short *outlen,
                                struct iso_mnt *imp);
 int cd9660_rrip_offset(struct iso_directory_record *isodir,
                            struct iso_mnt *imp);
 #endif /* _KERNEL */
 
 #endif /* _ISOFS_CD9660_ISO_RRIP_H_ */