mirage / qubes-mirage-firewall

A Mirage firewall VM for QubesOS
BSD 2-Clause "Simplified" License
210 stars 28 forks source link

IP checksums #153

Closed hannesm closed 11 months ago

hannesm commented 2 years ago

From the #OCaml IRC channel, it may be worth to just drop any IP checksum computations (IPv6 doesn't have checksums anyways, and most of the IPv4 checksums on the Internet are wrong). This may help in respect to speedups.

haesbaert commented 2 years ago

I don't see why, I also never heard about having wrong checksums in the wild. OpenBSD stack just drops on bad checksum, and I assume all other as well.

On Sun, Oct 16, 2022, 12:49 Hannes Mehnert @.***> wrote:

From the #OCaml IRC channel, it may be worth to just drop any IP checksum computations (IPv6 doesn't have checksums anyways, and most of the IPv4 checksums on the Internet are wrong). This may help in respect to speedups.

— Reply to this email directly, view it on GitHub https://github.com/mirage/qubes-mirage-firewall/issues/153, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABR2EHH5TMZZOFZVTZBJXDWDPMS3ANCNFSM6AAAAAARGJS3GI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

hannesm commented 2 years ago

Yes, @haesbaert -- likely you're right, I still think we should take a look into the bottlenecks, and checksum code is C and could benefit from a check whether this is the hot path...

haesbaert commented 2 years ago

Ack, just have in mind that the IP checksum is nothing compared to the TCP one, since it's just the header, and in my humble experience (at least on a BSD stack), the cycles for checksuming are not significant (even the TCP).

hannesm commented 11 months ago

Let's close this, performance evaluation is important, but best not be done on a speculative basis. So, better to benchmark and figure out numbers before trying to propose fixes.