Date: Wed, 26 Aug 2015 14:50:33 +0200 From: =?UTF-8?Q?Ermal_Lu=C3=A7i?= <ermal.luci@gmail.com> To: Kristof Provost <kristof@sigsegv.be> Cc: freebsd-net <freebsd-net@freebsd.org>, "freebsd-pf@freebsd.org" <freebsd-pf@freebsd.org> Subject: Re: Near-term pf plans Message-ID: <CAPBZQG1gELWov6fOnJMHZyhux8W_pykjsXxwnJQdNjszJC_9-w@mail.gmail.com> In-Reply-To: <20150826114348.GA3433@vega.codepro.be> References: <20150823150957.GK48727@vega.codepro.be> <CAPBZQG2ERdcn-rqvKvtTm0%2B5fyk0_vjxMYdWd1YbgS02zAZ=bA@mail.gmail.com> <20150826114348.GA3433@vega.codepro.be>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Aug 26, 2015 at 1:43 PM, Kristof Provost <kristof@sigsegv.be> wrote= : > On 2015-08-25 19:56:59 (+0200), Ermal Lu=C3=A7i <ermal.luci@gmail.com> wr= ote: > > On Sun, Aug 23, 2015 at 5:09 PM, Kristof Provost <kp@freebsd.org> wrote= : > > > > > I'm inclined to say that ifgroups and interfaces should share a > > > namespace. That would certainly help pf, because it uses both > > > interchangeably, which just doesn't work unless they're in the sam= e > > > namespace. That affects more in the network stack though, so I'd > like > > > opinions for people with more experience with ifgroups. > > > > > > > > > The solution is easy the scenario of interface name and group name shou= ld > > not be allowed. > > I do not see the use case for this to be allowed at all its just > confusion > > in general in pf(4). > > > Agreed. I'll see about putting together a patch to do that. > Ok, keep me in the loop for the review. > > > > > > > - Removal of frag drop / frag crop support in pf. > ... > > While those provide more security protection they do not always work as > > advertised so i agree on their removal. > > > Okay, I'll get the patch committed soon. > > > > - PR 198868, 193579 (TSO issues) > > > pf has issues on certain network interfaces with TSO enabled. The > > > most important of these are amazon VMs. > > > I believe the problem is that pf tries to work with packets with > full > > > checksums. Usually output packets have pseudo-header checksums in > the IP > > > and TCP headers. pf doesn't know about those. It always tries to d= o > > > updates on checksums assuming there's a full checksum. (Which is t= he > > > case, pf always calculates a full checksum in pf_check_out()) > > > > > > I believe the fix for this issue is to have pf be aware of the > > > pseudo-header checksums. The type of checksum can be determined > from the > > > CSUM_TCP and CSUM_TCP_IPV6 flags. Whenever pf changes a packet > header it > > > will have to check for those flags to figure out if the checksum > should > > > be updated or not. > > > > > > > > I would be inclined to say that the real solution is not check packets > > generated from the host, > This also applies to forwarded packets. Think of things like NAT or port > rewriting. We have to change IP addresses for nat, and thus also update > checksums. Normally you would expect those with correct checksum AFAIK since the input path validates those as well! > > > otherwise you will have to go into complicated checksum handling which > > might not be worth it. > We should end up with slightly more complicated checksum code (because > sometimes we'll update and sometimes we won't), but I expect it to > actually be faster. Right now we always calculate a full checksum on > outbound packets. In the new case we'll either not touch the checksum, > or only calculate updates (depending on scenario). > > > I know Open has done some work on checksum handling altogether which > might > > be a good reason > > to go and look there first. > > > Right, I'll check. > > After looking at Open code, their solution is to not check anymore checksums on pf(4) code but only mark them to be recalculated if pf(NAT/RDR/BINAT) touches the packet and let the OS mechanics deal with them. Certainly they went to length on doing this allover but it seems a better choice, also same effort as making the checksum handling right, rather than duplicating whole portions of code which are difficult to get right and keep in sync. How do you say that we try to do the same in FreeBSD and seems a better path in maintainability rather than a complex code that can get out-of-date soon. Relying on the protocol routines deal with checksum handling, since they have already the code, is the better path and with so many offloading features on the NICs nowadays it does not make much sense to have it on pf(4), taking also in consideration the input path verifies them and output path handles them properly even during forwarding and even during host packet transmission. This also has the benefit to increase the performance! > > > - PR 172648 > > > This bug started out with an issue with TCP reassembly, but I've n= ot > > > been able to reproduce that. There is a problem with rdr targets f= or > > > ipv6 though. > > > ... > > I will try to look back at this but as a general rule look first at Ope= n > if > > they have already fixed this. > > IPv4 code has some security belts on such packets in pf(4) code to avoi= d > > doing the wrong thing. > > > I think their checks in ip6_output() are less strict than ours. > Regards, > Kristof > --=20 Ermal
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAPBZQG1gELWov6fOnJMHZyhux8W_pykjsXxwnJQdNjszJC_9-w>