On 19 August 2016 at 16:34, Richard Procter <richard.n.proc...@gmail.com> wrote: > Hi Mike, > > On Fri, 19 Aug 2016, Mike Belopuhov wrote: > >> I've looked through it and couldn't find anything wrong with it. > > Thanks. > >> I do however find pacthing of values in pf_translate_icmp_af >> unneccessary since we'll be throwing away the original header >> anyway. > > Do you mean e.g. circa line 2602, pf.c:pf_translate_icmp_af() > (HEAD + complete diff)? > > pf_patch_8(pd, &icmp4->icmp_type, type, PF_HI); > pf_patch_8(pd, &icmp4->icmp_code, code, PF_LO); > pf_patch_16(pd, &icmp4->icmp_nextmtu, htons(mtu)); > if (ptr >= 0) > pf_patch_32(pd, &icmp4->icmp_void, htonl(ptr)); > > My understanding is that the ICMP v4 and v6 headers are so similar that pf > af-to hacks the one into the other; the code above is filling an ICMPv4 > header with v6 values; the updates are not redundant. It muddled me a bit > when I was converting it. >
I see now that you're not recalculating checksums after pf_translate_af, but rather update them in place. As long as it works, it's fine with me :) >> >> OK mikeb for the diff. > > best, > Richard.
