Fix FreeBSD build issues

[kprovost]

Recent FreeBSD versions started including <net/bpf.h> from <net/if_pflog.h>, which triggered two distinct build issues.

[guyharris]

The change that did that was:

commit 6d4baa0d011cb3e78b4b08415568e71c0aab00fe
Author: Kristof Provost <kp@FreeBSD.org>
Date:   Thu Dec 2 08:22:34 2021 +0100

    if_pflog: fix packet length
    
    There were two issues with the new pflog packet length.
    The first is that the length is expected to be a multiple of
    sizeof(long), but we'd assumed it had to be a multiple of
    sizeof(uint32_t).
    
    The second is that there's some broken software out there (such as
    Wireshark) that makes incorrect assumptions about the amount of padding.
    That is, Wireshark assumes there's always three bytes of padding, rather
    than however much is needed to get to a multiple of sizeof(long).
    
    Fix this by adding extra padding, and a fake field to maintain
    Wireshark's assumption.
    
    Reported by:    Ozkan KIRIK <ozkan.kirik@gmail.com>
    Tested by:      Ozkan KIRIK <ozkan.kirik@gmail.com>
    MFC after:      1 week
    Sponsored by:   Rubicon Communications, LLC ("Netgate")
    Differential Revision:  https://reviews.freebsd.org/D33236

Note that what Wireshark is doing wrong is bothering to even try to dissect LINKTYPE_PFLOG; it's not a good LINKTYPE_ value, as it doesn't have a single specification, it's "whatever the OS that wrote the file decided to use, subject to change without notice". If it were up to me, at this point, I'd rip the code for it out of tcpdump and Wireshark and leave it up to particular OSes to add in dissection appropriate for the version of the OS with which they're packaging Wireshark in their packaging system.

Note: any file format or packet format that includes a long is broken unless it contains sufficient information to indicate how long a long is. This includes packet formats in capture files.

(THIS is why, when somebody proposes a new LINKTYPE_ value, I try to get them to nail down all the details; I want to give them as little freedom as possible to change things. Every time you require a packet dissector to use a heuristic to dissect a link-layer type, God kills a kitten. Please, think of the kittens.)

So why did this change have to include net/bpf.h?

[infrastation]

(See also the-tcpdump-group/tcpdump#587.)

[kprovost]

6d4baa0d011cb3e78b4b08415568e71c0aab00fe includes net/bpf.h because it now uses BPF_WORDALIGN() in PFLOG_HDRLEN.

[guyharris]

6d4baa0d011cb3e78b4b08415568e71c0aab00fe includes net/bpf.h because it now uses BPF_WORDALIGN() in PFLOG_HDRLEN.

Given that struct pfloghdr is written to files that may be read on other machines, the only sane definition of it is one in which the size and layout of the structure is the same on all machines.

None of the data types in the structure are long, so none of them either 1) require alignment greater than 32-bit alignment or 2) have a size that depends on whether your programming environment is ILP32 or LP64 (FreeBSD doesn't use other environments such as LLP64).

Thus, the only way in which the size and layout of the structure would differ between ILP32 and LP64 environments would be if it were to be aligned on a 32-bit boundary in an ILP32 environment and a 64-bit boundary in an LP64 environment, as that would require its size to be a multiple of 8 in an LP64 environment but only a multiple of 4 in an ILP32 environment.

As long as 1) neither IF_NAMESIZE nor PFLOG_RULESET_NAME_SIZE change and 2) neither uid_t nor pid_t are extended to be 64-bit, the size and layout of the structure will be fixed, and the padding require to align it on an 8-byte boundary on all platforms will always be 4 bytes.

So:

1. It's a good thing that Wireshark forced you to add the padding, to force the structure to be the same on ILP32 and LP64 platforms, so that it's a sane header for use in files such as pcap and pcapng files.
2. sizeof(struct pfloghdr) should be sufficient, as the size should be the same on all platforms.

Furthermore:

1. BPF_WORDALIGN() rounds the size up to a multipel fo 4; given that the structure is not "packed", and has uint32_t members, is there any reason why the size would ever not be a multiple of 4?
2. Also,BPF_WORDALIGN() doesn't round it up to a multiple of 8, so I'm not sure what purpose it's serving here.

So I don't see that it's necessary.

If there's still anything compiler-dependent or ILP32 vs. LP64-dependent about the structure, that needs to be fixed, so that all LINKTYPE_PFLOG files using the structure look Exactly. The. Same. Reardless. Of Whether. They're. Written. By. An. ILP32. Host. Or. An. LP64. Host.

While we're at it, if there will ever be a reason why that structure might change, whether it's a structure member being added or removed, a structure member having its size change (including array-size changes), a change to the numerical values of AF_ definitions (probably unlikely), or a change to the numerical values for action or reason, and it cannot be ensured that the structure layout can always be determined based on the length field value, then, unless every such change will result in a new LINKTYPE_ value being assigned, there should be a field in the structure specifying a version number for the structure.

If no changes to the numerical values in question, and all changes to the structure result in an increase in the structure size, the header field will suffice.

And, given that struct pfloghdr in current FreeBSD is different from struct pfloghdr in:

• current DragonFly BSD;
• current NetBSD;
• current OpenBSD;
• macOS as of xnu-7195.50.7.100.1;

how about, when we finally have a FreeBSD struct pfloghdr that will not change in ways that require either 1) a new LINKTYPE_ or 2) heuristics to allow the structure format to be determined, we add a new LINKTYPE_FREEBSD_PFLOG, and we can have all software that reads pcap or pcapng files with LINKTYPE_FREEBSD_PFLOG able to read all such files from all current or past versions of software that generates them and able to be modified to handle all future versions - and make it possible to write programs to read those formats without having to include if_pflog.h from the OS.

(Then we can work on handling the other versions, and ensuring that they don't require any more than one LINKTYPE_ value per OS or any heuristics - and not require including if_pflog.h.)

[guyharris]

This should be fixed by 2757ddc and 80dbee1.