PF: Handle fragmented IPv6 packets

[kp]

Summary

Roughly based on the OpenBSD work by Alexander Bluhm.

[glebius]

Kristof, big thanks for working on this. See my comments.

sys/netpfil/pf/pf.c

366

This function can also be used not only for fragment rbtree, but can also substitute the PF_ANEQ, PF_AEQ and could be considered to substitute even pf_match_addr(). So, lots of code can be generalized using this function. That's good.

I'd suggest not to inline it, and rename it to pf_addr_cmp. "cmp" is a widely used abbreviation clear to anyone.

396

Please add a panic() for default switch case.

sys/netpfil/pf/pf_norm.c

64

Please use C99 types in new code instead of 80-ish BSD types: uint32_t, uint16_t, uint8_t.

72

When hacking pf, I strongly dislike this struct foo and struct foo_cmp, that must be kept synchronized. I skipped fixing that in 2012 and now I regret that. Let's now do it better in the new code. You can just embed pf_fragment_cmp into pf_fragment. Alternatively, you can do this trick:

#define fr_cmp_offset fr_entry

and in code you do:

bcmp(a, b, offsetof(struct pf_fragment, fr_cmp_offset))

I prefer the trick over embedding, but that's up to you.

340

Empty line here is style(9) requirement, shouldn't be removed.

403

Let's put PF_FRAG_ASSERT() at the beginning of this function. Kinda documenting its locking requirements.

459

Dots in end of comments throught the function, please. Thanks :)

735

Please use KASSERT:

m = m_getptr(m, hdrlen + offsetof(struct ip6_frag, ip6f_nxt), &off);
KASSERT(m, ("%s: short mbuf chain", func));

757

Same here.

[glebius]

Thanks. Now let me complete universe with anonymous structs/unions on.

sys/netpfil/pf/pf_norm.c

88

This line looks like cut-n-paste typo. Don't hurry to fix, I am about to improve the code to use anonymous structure, which, I hope, will be enabled for the whole kernel build soon.

[kp]

Submitted as r278831 - head/sys/netpfil/pf