dhclient: remove the need to patch static values

[franco_opnsense.org]

Summary

Imported into FreeBSD from OpenBSD 3.7. Imported in 2004 from
ISC DHCP... Looks like the patching was part of templating for
different things, sort of see:

https://gitlab.isc.org/isc-projects/dhcp/-/blob/master/common/bpf.c

At least we know the BPF filter values should be given in host
order and apparently the whole thing keeps working.

While here enable the IP_MF bit during receiving which seems to be
more consistent.

Test Plan

Running in production in OPNsense 21.7 for two weeks. Requires
another dhclient(1) test on a CURRENT build.

[markj]

Seems ok to me overall.

At least we know the BPF filter values should be given in host
order and apparently the whole thing keeps working.

Are you sure it worked before? I see that BPF_LD will unconditionally byte-swap the loaded value into A, so on little-endian systems will convert it to host order. Then a subsequent JMP will compare the immediate value with A. So passing in host order is correct, and on big-endian systems BPF filters look to be somewhat broken...

sbin/dhclient/bpf.c

108

Did you consider making these tables const? You would have to write:

p.bf_insns = __DECONST(struct bpf_insn *, dhcp_bpf_wfilter);

below, but that is worth it IMO.

[franco_opnsense.org]

constify filters and avoid static length variables

[franco_opnsense.org]

OpenBSD added the single htons() while adding write filter, but nowhere else. I suspect the fragmentation check is mostly correct so this doesn't matter in the real world.

From the ISC bpf.c I can see no htons() but two ntohs() calls for port definitions that are initialised as:

local_port = htons(67);

If there is an issue with bpf on big endian it should probably be fixed closer to bpf code, but it doesn't seem this way (port filters likely work on big endian as a fundamental assumption for lack of reports)?

sbin/dhclient/bpf.c

108

Agreed, went a little further by inlining the length vars.

[markj]

In D31502#712599, @franco_opnsense.org wrote:

OpenBSD added the single htons() while adding write filter, but nowhere else. I suspect the fragmentation check is mostly correct so this doesn't matter in the real world.

From the ISC bpf.c I can see no htons() but two ntohs() calls for port definitions that are initialised as:

local_port = htons(67);

If there is an issue with bpf on big endian it should probably be fixed closer to bpf code, but it doesn't seem this way (port filters likely work on big endian as a fundamental assumption for lack of reports)?

Indeed, BPF_LD doesn't byte-swap, I just misread.