Details

Reviewers

cy
• hselasky
philip
emaste
delphij
sunpoet
koue_chaosophia.net

Commits

rG6d1c946e9168: libpcap: Make pcap/bpf.h a wrapper around system net/bpf.h

Summary

In the past, we modified contrib/libpcap/pcap/pcap.h to include
net/bpf.h rather than libpcap's own bpf.h. Revert that change, because
libpcap's cut-down version of bpf.h only includes components necessary
for the code generator, userland BPF interpreter, and the libpcap APIs
for setting filters, etc.. pcap-bpf.c includes the native OS version.

This is in preparation for a libpcap update to 1.10.3 where pcap/pcap.h
includes headers within the pcap source tree. Continuing to directly
include our net/bpf.h would require more local changes.

Test Plan

Build tested libpcap and tcpdump on 12.4, 13.1 and 14.0-CURRENT main-n259729-69542f26820b

@hselasky, in the commit log for 5fd1ea0810fa7811886c51740e0758f2dd36d940, you say "Restore local change to include <net/bpf.h> inside pcap.h. This fixes ports build problems." Do you recall which ports were affected?

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jrm created this revision.Feb 13 2023, 11:21 PM

Herald added a subscriber: imp. · View Herald TranscriptFeb 13 2023, 11:21 PM

jrm requested review of this revision.Feb 13 2023, 11:21 PM

Harbormaster completed remote builds in B49757: Diff 117160.Feb 13 2023, 11:21 PM

emaste added subscribers: antoine, delphij, np.Feb 14 2023, 1:53 AM

I think this makes sense. I am also generally in favour of anything that reduces our diffs to upstream.

I would like to know how this affected ports. I searched the mailing list archives from mid- to late-May 2018 and couldn't find any reference to this, other than the commit message linked.

This revision is now accepted and ready to land.Feb 14 2023, 2:12 AM

delphij accepted this revision.Feb 14 2023, 6:35 AM

Hi,
In may 2018 I reported that some ports were broken due to the upgrade (non-exhaustive list):

packetdrill
spamd
vde2
xprobe2

jrm added reviewers: sunpoet, koue_chaosophia.net.Feb 14 2023, 8:38 PM

In D38575#877791, @antoine wrote:

Hi,
In may 2018 I reported that some ports were broken due to the upgrade (non-exhaustive list):

packetdrill

spamd

vde2

xprobe2

Thanks Antoine. On 13.1-amd64, with the change proposed here, packetdrill and vde2 still build successfully, but mail/spamd and net/xprobe have problems. I'm including logs for the two build failures for reference. The last upstream release for spamd was over 11 years ago and the last upstream release for xprobe2 was over 17 years ago, so it's likely that each has fallen behind on some libpcap changes. It would be good to move ahead with the change here, so that we can update libpcap before 13.2 (if that's still possible). There's a vulnderabilty that's affecting the version of libpcap we have in the tree now [1].

@sunpoet and @koue_chaosophia.net, are you good if we move forward here and deal with the fallout of your ports before 13.2 is realeased?

[1] https://www.mend.io/vulnerability-database/WS-2019-0573

mail/spamd build log:

===>  License BSD2CLAUSE accepted by the user
===>   spamd-4.9.1_6 depends on file: /usr/local/sbin/pkg - found
===> Fetching all distfiles required by spamd-4.9.1_6 for building
===>  Extracting for spamd-4.9.1_6
=> SHA256 Checksum OK for spamd-4.9.1.tar.gz.
===>  Patching for spamd-4.9.1_6
===>  Applying FreeBSD patches for spamd-4.9.1_6 from /usr/ports/mail/spamd/files
===>  Configuring for spamd-4.9.1_6
===>  Building for spamd-4.9.1_6
--- all_subdir_spamd ---
===> spamd (all)
--- objwarn ---
--- .depend ---
--- objwarn ---
Warning: Object directory not changed from original /usr/ports/mail/spamd/work/spamd-4.9.1/spamd
--- .depend ---
echo spamd: /usr/lib/libc.a /usr/lib/libcrypto.a /usr/lib/libmd.a >> .depend
--- spamd.o ---
--- sdl.o ---
--- grey.o ---
--- sync.o ---
--- spamd.8.gz ---
--- spamd.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes   -fPIE -MD  -MF.depend.spamd.o -MTspamd.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c spamd.c -o spamd.o
--- sdl.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes   -fPIE -MD  -MF.depend.sdl.o -MTsdl.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c sdl.c -o sdl.o
--- grey.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes   -fPIE -MD  -MF.depend.grey.o -MTgrey.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c grey.c -o grey.o
--- sync.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes   -fPIE -MD  -MF.depend.sync.o -MTsync.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c sync.c -o sync.o
--- spamd.8.gz ---
gzip -cn spamd.8 > spamd.8.gz
--- spamd ---
cc -O2 -pipe -I/usr/include -fstack-protector-strong -fno-strict-aliasing -Wall -Wstrict-prototypes -fPIE -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong -Qunused-arguments  -L/usr/lib  -fstack-protector-strong  -pie   -o spamd spamd.o sdl.o grey.o sync.o  -lcrypto -lmd
--- all_subdir_spamd-setup ---
===> spamd-setup (all)
--- objwarn ---
--- .depend ---
--- objwarn ---
Warning: Object directory not changed from original /usr/ports/mail/spamd/work/spamd-4.9.1/spamd-setup
--- .depend ---
echo spamd-setup: /usr/lib/libc.a /usr/lib/libz.a >> .depend
--- spamd-setup.o ---
--- spamd-setup.8.gz ---
--- spamd-setup.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes   -fPIE -MD  -MF.depend.spamd-setup.o -MTspamd-setup.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c spamd-setup.c -o spamd-setup.o
--- spamd-setup.8.gz ---
gzip -cn spamd-setup.8 > spamd-setup.8.gz
--- spamd-setup ---
cc -O2 -pipe -I/usr/include -fstack-protector-strong -fno-strict-aliasing -Wall -Wstrict-prototypes -fPIE -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong -Qunused-arguments  -L/usr/lib  -fstack-protector-strong  -pie   -o spamd-setup spamd-setup.o  -lz
--- all_subdir_spamdb ---
===> spamdb (all)
--- objwarn ---
--- .depend ---
--- objwarn ---
Warning: Object directory not changed from original /usr/ports/mail/spamd/work/spamd-4.9.1/spamdb
--- .depend ---
echo spamdb: /usr/lib/libc.a /usr/lib/libcrypto.a /usr/lib/libmd.a >> .depend
--- spamdb.o ---
--- sync.o ---
--- spamdb.8.gz ---
--- spamdb.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes -I/usr/ports/mail/spamd/work/spamd-4.9.1/spamdb/../spamd   -fPIE -MD  -MF.depend.spamdb.o -MTspamdb.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c spamdb.c -o spamdb.o
--- sync.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes -I/usr/ports/mail/spamd/work/spamd-4.9.1/spamdb/../spamd   -fPIE -MD  -MF.depend.sync.o -MTsync.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c /usr/ports/mail/spamd/work/spamd-4.9.1/spamdb/../spamd/sync.c -o sync.o
--- spamdb.8.gz ---
gzip -cn spamdb.8 > spamdb.8.gz
--- spamdb ---
cc -O2 -pipe -I/usr/include -fstack-protector-strong -fno-strict-aliasing -Wall -Wstrict-prototypes -I/usr/ports/mail/spamd/work/spamd-4.9.1/spamdb/../spamd -fPIE -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong -Qunused-arguments  -L/usr/lib  -fstack-protector-strong  -pie   -o spamdb spamdb.o sync.o  -lcrypto -lmd
--- all_subdir_spamlogd ---
===> spamlogd (all)
--- objwarn ---
--- .depend ---
--- objwarn ---
Warning: Object directory not changed from original /usr/ports/mail/spamd/work/spamd-4.9.1/spamlogd
--- .depend ---
echo spamlogd: /usr/lib/libc.a /usr/lib/libpcap.a /usr/lib/libmd.a >> .depend
--- spamlogd.o ---
--- sync.o ---
--- spamlogd.8.gz ---
--- spamlogd.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes -I/usr/ports/mail/spamd/work/spamd-4.9.1/spamlogd/../spamd   -fPIE -MD  -MF.depend.spamlogd.o -MTspamlogd.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c spamlogd.c -o spamlogd.o
--- sync.o ---
cc  -O2 -pipe  -I/usr/include -fstack-protector-strong -fno-strict-aliasing  -Wall -Wstrict-prototypes -I/usr/ports/mail/spamd/work/spamd-4.9.1/spamlogd/../spamd   -fPIE -MD  -MF.depend.sync.o -MTsync.o -std=gnu99 -Wno-format-zero-length -nobuiltininc -idirafter /usr/lib/clang/13.0.0/include -fstack-protector-strong    -Qunused-arguments    -c /usr/ports/mail/spamd/work/spamd-4.9.1/spamlogd/../spamd/sync.c -o sync.o
--- spamlogd.8.gz ---
gzip -cn spamlogd.8 > spamlogd.8.gz
--- spamlogd.o ---
spamlogd.c:169:32: error: use of undeclared identifier 'BIOCLOCK'
        if (ioctl(pcap_fileno(hpcap), BIOCLOCK) < 0) {
                                      ^
1 error generated.
*** [spamlogd.o] Error code 1

make[3]: stopped in /usr/ports/mail/spamd/work/spamd-4.9.1/spamlogd
1 error

make[3]: stopped in /usr/ports/mail/spamd/work/spamd-4.9.1/spamlogd

make[2]: stopped in /usr/ports/mail/spamd/work/spamd-4.9.1
===> Compilation failed unexpectedly.
Try to set MAKE_JOBS_UNSAFE=yes and rebuild before reporting the failure to
the maintainer.
*** Error code 1

Stop.
make[1]: stopped in /usr/ports/mail/spamd
*** Error code 1

Stop.
make: stopped in /usr/ports/mail/spamd
#

net/xprobe build log:

===>  Building for xprobe2-0.3_1
--- all ---
(cd libs-external/USI++/src; /usr/bin/make)
--- icmp.o ---
--- datalink.o ---
--- ip.o ---
--- misc.o ---
--- udp.o ---
--- tcp.o ---
--- TX_IP.o ---
--- Layer2.o ---
--- arp.o ---
--- icmp.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 icmp.cc
--- datalink.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 datalink.cc
--- ip.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 ip.cc
--- misc.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 misc.cc
--- udp.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 udp.cc
--- tcp.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 tcp.cc
--- TX_IP.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 TX_IP.cc
--- Layer2.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 Layer2.cc
--- arp.o ---
c++ -DIMMEDIATE -O2 -pipe -fstack-protector-strong -fno-strict-aliasing  -I/usr/include/pcap -I. -Wall -c -O2 arp.cc
--- icmp.o ---
In file included from icmp.cc:14:
In file included from ./usi++/icmp.h:16:
./usi++/ip.h:182:14: warning: 'usipp::IP::sendpack' hides overloaded virtual function [-Woverloaded-virtual]
        virtual int sendpack(void *payload, size_t paylen);
                    ^
./usi++/Layer2.h:44:14: note: hidden overloaded virtual function 'usipp::Layer2::sendpack' declared here: different number of parameters (3 vs 2)
        virtual int sendpack(void *buf, size_t len, struct sockaddr *);
                    ^
In file included from icmp.cc:14:
In file included from ./usi++/icmp.h:16:
./usi++/ip.h:185:14: warning: 'usipp::IP::sendpack' hides overloaded virtual function [-Woverloaded-virtual]
        virtual int sendpack(char *pay_string);
                    ^
./usi++/Layer2.h:44:14: note: hidden overloaded virtual function 'usipp::Layer2::sendpack' declared here: different number of parameters (3 vs 1)
        virtual int sendpack(void *buf, size_t len, struct sockaddr *);
                    ^
--- udp.o ---
In file included from udp.cc:13:
In file included from ./usi++/udp.h:17:
./usi++/ip.h:182:14: warning: 'usipp::IP::sendpack' hides overloaded virtual function [-Woverloaded-virtual]
        virtual int sendpack(void *payload, size_t paylen);
                    ^
./usi++/Layer2.h:44:14: note: hidden overloaded virtual function 'usipp::Layer2::sendpack' declared here: different number of parameters (3 vs 2)
        virtual int sendpack(void *buf, size_t len, struct sockaddr *);
                    ^
In file included from udp.cc:13:
In file included from ./usi++/udp.h:17:
./usi++/ip.h:185:14: warning: 'usipp::IP::sendpack' hides overloaded virtual function [-Woverloaded-virtual]
        virtual int sendpack(char *pay_string);
                    ^
./usi++/Layer2.h:44:14: note: hidden overloaded virtual function 'usipp::Layer2::sendpack' declared here: different number of parameters (3 vs 1)
        virtual int sendpack(void *buf, size_t len, struct sockaddr *);
                    ^
udp.cc:217:24: warning: ISO C++11 does not allow conversion from string literal to 'char *' [-Wwritable-strings]
        r = Layer2::setfilter("udp");
                              ^
--- misc.o ---
misc.cc:21:3: warning: 'register' storage class specifier is deprecated and incompatible with C++17 [-Wdeprecated-register]
  register long sum;            /* assumes long == 32 bits */
  ^~~~~~~~~
misc.cc:23:3: warning: 'register' storage class specifier is deprecated and incompatible with C++17 [-Wdeprecated-register]
  register u_short answer;      /* assumes u_short == 16 bits */
  ^~~~~~~~~
--- udp.o ---
3 warnings generated.
--- misc.o ---
2 warnings generated.
--- icmp.o ---
icmp.cc:296:24: warning: ISO C++11 does not allow conversion from string literal to 'char *' [-Wwritable-strings]
        r = Layer2::setfilter("icmp");
                              ^
--- TX_IP.o ---
TX_IP.cc:37:2: warning: misleading indentation; statement is not part of the previous 'if' [-Wmisleading-indentation]
        int r;
        ^
TX_IP.cc:33:5: note: previous statement is here
    if (tx_timeout != false)
    ^
--- ip.o ---
In file included from ip.cc:13:
./usi++/ip.h:182:14: warning: 'usipp::IP::sendpack' hides overloaded virtual function [-Woverloaded-virtual]
--- tcp.o ---
In file included from tcp.cc:14:
In file included from ./usi++/tcp.h:19:
./usi++/ip.h:182:14: warning: 'usipp::IP::sendpack' hides overloaded virtual function [-Woverloaded-virtual]
--- ip.o ---
        virtual int sendpack(void *payload, size_t paylen);
                    ^
./usi++/Layer2.h:44:14: note: hidden overloaded virtual function 'usipp::Layer2::sendpack' declared here: different number of parameters (3 vs 2)
--- tcp.o ---
        virtual int sendpack(void *payload, size_t paylen);
                    ^
./usi++/Layer2.h:44:14: note: hidden overloaded virtual function 'usipp::Layer2::sendpack' declared here: different number of parameters (3 vs 2)
--- ip.o ---
        virtual int sendpack(void *buf, size_t len, struct sockaddr *);
                    ^
In file included from ip.cc:13:
./usi++/ip.h:185:14: warning: 'usipp::IP::sendpack' hides overloaded virtual function [-Woverloaded-virtual]
--- tcp.o ---
        virtual int sendpack(void *buf, size_t len, struct sockaddr *);
                    ^
In file included from tcp.cc:14:
In file included from ./usi++/tcp.h:19:
--- ip.o ---
        virtual int sendpack(char *pay_string);
                    ^
--- tcp.o ---
./usi++/ip.h:185:14: warning: 'usipp::IP::sendpack' hides overloaded virtual function [-Woverloaded-virtual]
--- ip.o ---
./usi++/Layer2.h:44:14: note: hidden overloaded virtual function 'usipp::Layer2::sendpack' declared here: different number of parameters (3 vs 1)
--- tcp.o ---
        virtual int sendpack(char *pay_string);
                    ^
--- ip.o ---
        virtual int sendpack(void *buf, size_t len, struct sockaddr *);
                    ^
--- tcp.o ---
./usi++/Layer2.h:44:14: note: hidden overloaded virtual function 'usipp::Layer2::sendpack' declared here: different number of parameters (3 vs 1)
        virtual int sendpack(void *buf, size_t len, struct sockaddr *);
                    ^
--- arp.o ---
In file included from arp.cc:13:
./usi++/arp.h:66:21: warning: 'usipp::ARP::sniffpack' hides overloaded virtual function [-Woverloaded-virtual]
        virtual int sniffpack();
                    ^
./usi++/Layer2.h:41:14: note: hidden overloaded virtual function 'usipp::Layer2::sniffpack' declared here: different number of parameters (2 vs 0)
        virtual int sniffpack(void *, size_t);
                    ^
--- icmp.o ---
3 warnings generated.
--- datalink.o ---
In file included from datalink.cc:21:
/usr/include/net/bpf.h:71:8: error: redefinition of 'bpf_program'
struct bpf_program {
       ^
/usr/include/pcap/bpf.h:104:8: note: previous definition is here
struct bpf_program {
       ^
In file included from datalink.cc:21:
/usr/include/net/bpf.h:364:8: error: redefinition of 'bpf_insn'
struct bpf_insn {
       ^
/usr/include/pcap/bpf.h:234:8: note: previous definition is here
struct bpf_insn {
       ^
--- tcp.o ---
tcp.cc:321:24: warning: ISO C++11 does not allow conversion from string literal to 'char *' [-Wwritable-strings]
        r = Layer2::setfilter("tcp");
                              ^
--- datalink.o ---
datalink.cc:274:12: warning: explicitly assigning value of variable of type 'size_t' (aka 'unsigned long') to itself [-Wself-assign]
        d_snaplen = d_snaplen;
        ~~~~~~~~~ ^ ~~~~~~~~~
1 warning and 2 errors generated.
*** [datalink.o] Error code 1

make[2]: stopped in /usr/ports/net/xprobe/work/xprobe2-0.3/libs-external/USI++/src
--- arp.o ---
1 warning generated.
--- TX_IP.o ---
1 warning generated.
--- tcp.o ---
3 warnings generated.
--- ip.o ---
ip.cc:559:24: warning: ISO C++11 does not allow conversion from string literal to 'char *' [-Wwritable-strings]
        r = Layer2::setfilter("ip");
                              ^
3 warnings generated.
1 error

make[2]: stopped in /usr/ports/net/xprobe/work/xprobe2-0.3/libs-external/USI++/src
*** [all] Error code 2

make[1]: stopped in /usr/ports/net/xprobe/work/xprobe2-0.3
1 error

make[1]: stopped in /usr/ports/net/xprobe/work/xprobe2-0.3
===> Compilation failed unexpectedly.
Try to set MAKE_JOBS_UNSAFE=yes and rebuild before reporting the failure to
the maintainer.
*** Error code 1

Stop.
make: stopped in /usr/ports/net/xprobe

The affected ports should be able to use ports/net/libpcap?

In D38575#878410, @cy wrote:

The affected ports should be able to use ports/net/libpcap?

That won't help for mail/spamd. It needs the definition of BIOCLOCK from net/bpf.h. This highlights a problem that I originally missed. The log message above says, "...libpcap's cut-down version of bpf.h includes [only essential parts] ... pcap-bpf.c includes the native OS version." That means with the change proposed in this review, code with #include <pcap.h> now won't have net/bpf.h pulled in. Without the change proposed in this review, pcap 1.10.3 fails to build, because it requires a few lines from pcap/bpf.h.

PCAP_AVAILABLE_0_4
PCAP_API u_int	bpf_filter(const struct bpf_insn *, const u_char *, u_int, u_int);

PCAP_AVAILABLE_0_6
PCAP_API int	bpf_validate(const struct bpf_insn *f, int len);

PCAP_AVAILABLE_0_4
PCAP_API char	*bpf_image(const struct bpf_insn *, int);

PCAP_AVAILABLE_0_6
PCAP_API void	bpf_dump(const struct bpf_program *, int);

where PCAP_API is #define PCAP_API PCAP_API_DEF extern.

There is quite a bit of overlap in pcap/bpf.h and net/bpf.h. What we could do is cut out the duplication from pcap/bpf.h then in the cut-down file add #include <net/bpf.h>. The downside is that it adds a bit of local change under contrib/libpcap, but I don't see a better solution.

libpcap: Make pcap/bpf.h a minimal wrapper around net/bpf.h

In the past, we modified contrib/libpcap/pcap/pcap.h to include
net/bpf.h rather than libpcap's own bpf.h. Revert that local change and
make libpcap's bpf.h a minimal wrapper around net/bpf.h.

This is in preparation for libpcap 1.10.3, where a few extern function
definitions in pcap/bpf.h are required.

This revision now requires review to proceed.Feb 15 2023, 5:16 PM

Harbormaster completed remote builds in B49807: Diff 117311.Feb 15 2023, 5:16 PM

I'm okay with this in principle (though I've not tested it).

Would it make sense to reduce the diff to upstream by sticking the #include <net/bpf.h> under #ifdef __FreeBSD__ and the rest of the file under #else?

Or is it not worth bothering?

jrm retitled this revision from libpcap: Revert local modification to directly include net/bpf.h to libpcap: Make pcap/bpf.h a minimal wrapper around net/bpf.h.Feb 15 2023, 5:29 PM

In D38575#878653, @philip wrote:

I'm okay with this in principle (though I've not tested it).

Would it make sense to reduce the diff to upstream by sticking the #include <net/bpf.h> under #ifdef __FreeBSD__ and the rest of the file under #else?

Or is it not worth bothering?

Yeah, I like that idea. I'll update the diff in a few hours.

I tested by building both libpcap 1.9.1 and 1.10.3 as well as tcpdump. I also tested Antoine's list of ports and didn't encounter any problems.

Incorporate Philip's suggestion to wrap things with #if defined(FreeBSD)

I must have missed this in the last round, but net/xprobe still fails to build, because it doesn't like.

#ifdef __cplusplus
extern "C" {
#endif

We can hide that under the #else as well, but I have little insight about the wider implications. Thoughts?

Harbormaster completed remote builds in B49811: Diff 117321.Feb 15 2023, 7:33 PM

Correct an error with the location of #endif /* defined(FreeBSD) */ in the update.

That's why net/xprobe suddenly wasn't building.

Harbormaster completed remote builds in B49812: Diff 117322.Feb 15 2023, 7:47 PM

emaste accepted this revision.Feb 15 2023, 9:23 PM

emaste added inline comments.

contrib/libpcap/pcap/bpf.h
78	perhaps a comment explaining why we do this (referencing example ports)?

This revision is now accepted and ready to land.Feb 15 2023, 9:23 PM

Incorporate Ed's suggesting for a comment
Tweak the location of the #ifdefs to hopefully make the future merge cleaner

This revision now requires review to proceed.Feb 15 2023, 11:19 PM

Harbormaster completed remote builds in B49825: Diff 117344.Feb 15 2023, 11:19 PM

jrm marked an inline comment as done.Feb 15 2023, 11:20 PM

LGTM assuming this is good for ports, and left a small comment about the comment.

contrib/libpcap/pcap/bpf.h
78	This file gets installed as /usr/include/pcap/bpf.h, and the one mentioned in the comment there as /usr/include/pcap/pcap.h, so we may want to omit the 'contrib/libpcap' prefix?
84	maybe around the system net/bpf.h

This revision is now accepted and ready to land.Feb 15 2023, 11:38 PM

Excellent work! Thank you.

cy accepted this revision.Feb 16 2023, 4:23 AM

Closed by commit rG6d1c946e9168: libpcap: Make pcap/bpf.h a wrapper around system net/bpf.h (authored by jrm). · Explain WhyFeb 16 2023, 3:42 PM

This revision was automatically updated to reflect the committed changes.

jrm added a commit: rG6d1c946e9168: libpcap: Make pcap/bpf.h a wrapper around system net/bpf.h.

libpcap: Make pcap/bpf.h a minimal wrapper around net/bpf.h
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 117451

contrib/libpcap/pcap/bpf.h

contrib/libpcap/pcap/pcap.h

libpcap: Make pcap/bpf.h a minimal wrapper around net/bpf.hClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 117451

contrib/libpcap/pcap/bpf.h

contrib/libpcap/pcap/pcap.h

libpcap: Make pcap/bpf.h a minimal wrapper around net/bpf.h
ClosedPublic
Actions

Revision Contents
Changeset List