search

facebookincubator / katran

A high performance layer 4 load balancer
GNU General Public License v2.0
4.75k stars 504 forks source link

fix handling large packets and sending icmp-too-big fragmentation bug #206

Closed AlirezaKm closed 1 year ago

AlirezaKm commented 1 year ago

bpf(icmp-too-big): This commit fixes a bug that katran cannot deal with large packets. Katran should generate ICMP_TOO_BIG packet ('destination unreachable') for this situation but randomly packets dropped from processing in NIC due to checksum issue. So, we need to generate an IP header with frag_off = 0 option to fix bytes replacements and fragmentation affects.

frankfeir commented 1 year ago

@AlirezaKm , thanks for the fix. It looks good to me that frag_off should be reset to 0 for icmp packet generated by katran. However, I don't quite understand the checksum issue you mentioned. we do re-calculate the check sum in ip header in send_icmp4_too_big. Could you please elaborate a bit more on the checksum issue?

facebook-github-bot commented 1 year ago

@frankfeir has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

AlirezaKm commented 1 year ago

@frankfeir I mentioned checksum issue by mistake, sorry for that. Actualy we found icmp-too-big issue with checksum. We generate a large packet (with scapy and determined params) and sent it to the katran server, so we expected a specific icmp packet with determined checksum in our side which ran katran too. After our investigation we found the problem that its good to reset frag_off to 0. Because it can drop in process_l3_headers function on our side with katran which received icmp or other devices which not comfortable with fragmented packets.

On Tue, Nov 14, 2023, 00:47 frankfeir @.***> wrote:

@AlirezaKm https://github.com/AlirezaKm , thanks for the fix. It looks good to me that frag_off should be reset to 0 for icmp packet generated by katran. However, I don't quite understand the checksum issue you mentioned. we do re-calculate the check sum in ip header in send_icmp4_too_big. Could you please elaborate a bit more on the checksum issue?

— Reply to this email directly, view it on GitHub https://github.com/facebookincubator/katran/pull/206#issuecomment-1809137869, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6NHVFMT3LRFW5N3HHSCH3YEKE6PAVCNFSM6AAAAAA7E4I45SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBZGEZTOOBWHE . You are receiving this because you were mentioned.Message ID: @.***>

facebook-github-bot commented 1 year ago

@frankfeir merged this pull request in facebookincubator/katran@06b5b4ac25ce5324d401215b501846f842ee2edc.