Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped - Build Status
Buildable 71142 Build 68025: arc lint + arc unit
Event Timeline
I now see there are a few things I missed. Please give me a bit more time before reviewing.
What a coincidence, I am resurrecting a major consumer of libpcap.
Full disclosure: I am the former maintainer of vendorized tcpdump and libcap in FreeBSD from the 2000s, as well as the ports for some time.
If you're dealing with tcpdump as well, vendorizing ILNP support would be very useful to @rgrimes and myself, if you can spare the cycles.
I don't feel myself expert enough to neither endorse or veto the update. In general I am of course in favor of being up to date with this library.
My changes related to new BPF ioctls and obsoleting USB-ifnet were fully upstreamed and followup fixes upstream were brought back to our tree. There should be no difference to upstream left.
The key changes to review are to lib/libpcap/{config.h,Makefile} and sys/net/dlt.h.
- I made config.h as close as possible to what is generated by upstream's configure script, but if we prefer to remove comments, I can.
- Upstream now expects SIZEOF_TIME_T to be defined. Is what I have ok?
- When synchronizing sys/net/dlt.h, a few DLT_* were removed, but they don't appear to be used.
% rg 'DLT_CLASS|DLT_CLASS_NETBSD_RAWAF|DLT_NETBSD_RAWAF|DLT_IS_NETBSD_RAWAF|DLT_MATCHING_MIN|DLT_MATCHING_MAX' /usr/src %
The DLT deltas just look like they're in the noise. config.h generation is probably OK.
Yeah, I am out of touch with modern libpcap itself mostly, apart from furtively porting the ILNP additions for RFC6740+ from my legacy branch to bleeding edge and raising the GitHub Pull for it this time last week, but am willing to eyeball stuff as I've spent most of yesterday rototilling the PCS pcap bindings originally written by Dug Song.
There is very little left to roll custom wrappers for in PCS at the moment in its pcap_ex.c file; most of it can now be merged to the pcap.pyx Cython file natively, and I think that file just existed from Dug's additions for Windows at the time (2005 onwards as used in dpkt and libdnet).
There's a wrapper function in PCS for pcap_set_immediate_mode() (Windows provides only pcap_setmintocopy()) and that's about it, although I will need to implement pcap_set_tstamp_precision() whilst modernising the rest of the Python wrapper.
I have used non-blocking I/O for years of course, I am just out of touch with Python 3's async/await keywords and how they are used, but of course I have C++23 coroutines to catch up on now, also.
I see both libpcap and tcpdump are gradually migrating to CMake. I don't plan to use Autoconf/Automake at all for any greenfield work, either, and I think it would be beneficial to FreeBSD to eventually follow that.
Full disclosure: I was the primary autopoo wrangler at ICSI and XORP, Inc. for quite some time in the 00s. I've met Dug personally (ironically, a client of mine had moved into Duo Security's old offices in London) and know him a little from the w00w00 security crew whom I used to hang with online in the 00s.
This is probably best pushed towards @garga (garga@FreeBSD.org) assuming s/he is still maintaining the libpcap port out of src tree.
I would up doing exactly this this morning in PCS to get closer to Python 3.x async/await support. Most of Dug's old C code in the pcap wrapper got nuked in the process.
I'm in a move fast and break things phase with PCS at the moment, so there is no guarantee anything actually works. If you are interested in dogfooding it you can find it here, it might provide a good means of exercising the libpcap vendorised import in base, and it would be valuable to make sure the include file detection in its setup.py is doing the right thing on FreeBSD base.
Recreating RFC 6740+ support in PCS for wire-level protocol conversation testing is more important, probably, to both myself and @rgrimes ; I had this in my set of artefacts that got lost during my personal crisis in 2018, and it produced some valuable PhD results, so there is a case for doing it again, and I can probably do that from memory, much as I did for the tcpdump dissector.
I don't plan to implement nanosecond precision yet; I don't really need it just now. Windows support is also most likely broken.
I support keeping the base libpcap up to date with the vendor branch and would like to see this review move forward, though it is rather large to review, so I must defer to the creater that this just does the right things for FreeBSD.
I don't expect anyone to wade through all of upstream's code. See my comment above starting with "The key changes to review are...".
Incorporate feedback (via IRC) from kevans about how to define SIZEOF_TIME_T. It's also 64-bit on all 32-bit platforms except i386.
#ifdef __i386__ #define SIZEOF_TIME_T 4 #else #define SIZEOF_TIME_T 8 #endif
I don't think so because contrib/libpcap/pcap-int.h has:
#if SIZEOF_TIME_T == 8 #define PCAP_SIZEOF_TIME_T_BITS_STRING "64" #elif SIZEOF_TIME_T == 4 #define PCAP_SIZEOF_TIME_T_BITS_STRING "32" #else #error Unknown time_t size #endif
Commited in 16cef5f7a65588def71db4fdfa961f959847e3b6.
It seems Phabricator can't handle two Differential Revision lines in a commit log. This was committed with:
Differential Revision: https://reviews.freebsd.org/D55545 Differential Revision: https://reviews.freebsd.org/D55858
Closing manually.