[Bug 261566] Padding of DLT_PFLOG packets should be done differently

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=261566

            Bug ID: 261566
           Summary: Padding of DLT_PFLOG packets should be done
                    differently
           Product: Base System
           Version: CURRENT
          Hardware: Any
                OS: Any
            Status: New
          Severity: Affects Some People
          Priority: ---
         Component: kern
          Assignee: bugs@FreeBSD.org
          Reporter: gharris@sonic.net

Currently, sys/net/if_pflog.h does

    #define PFLOG_HDRLEN            BPF_WORDALIGN(offsetof(struct pfloghdr,
pad2))

This is done only in FreeBSD; neither DragonFly BSD nor NetBSD nor OpenBSD nor
Darwin use BPF_WORDALIGN() here, and, as FreeBSD's BPF_WORDALIGN() aligns to
the size of a long, this means that the DLT_PFLOG packet format can differ
depending on whether the machine writing the packet is 32-bit (ILP32) or 64-bit
(LP64), which, given that neither the pcap file format nor the pcapng file
format give any indication of the size of a long on the platform on which the
file was written, means that the correct way to process a DLT_PFLOG packt in a
given file cannot be determined from the file.

The commit message for the commit that added BPF_WORDALIGN() was

    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).

but it gives not justification for the claim that "the length is expected to be
a multiple of sizeof(long)".  As indicated:

1) that's not the case on only *other* BSD-flavored OS with pflog

and

2) that creates a packet format that can only be processed correctly by
providing external information about the machine that wrote the file, which
requires not only extra code, but extra time and energy on the part of
whoever's trying to read the file, to determine the size of a long on the
platform on which it was written and to supply that to the program reading the
file.

The other OSes rely on the compiler to add the padding as a consequence of the
structure including some 32-bit integral values; that should suffice here,
although if you want to make *sure* it's padded to a 4-byte boundary, you could
use roundup2() from sys/param.h to do it rather than dragging in net/bpf.h.

Note that

1) it's not clear that aligning on an 8-byte boundary will provide any
performance improvement, as, even if you were to align the IP packet on an
8-byte boundary, there's no guarantee that this will put a significant number
of 8-byte fields on an 8-byte boundary

and

2) that will make a difference for two of the main packet analyzers that read
pcap and pcapng files (tcpdump and Wireshark) only on CPUs that 1) support
unaligned access but 2) impose a penalty for those accesses.

-- 
You are receiving this mail because:
You are the assignee for the bug.