fix handling large packets and sending icmp-too-big fragmentation bug #206
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…th 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
approved these changes
Nov 13, 2023
|
@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? |
|
@frankfeir has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@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
<#206 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AC6NHVFMT3LRFW5N3HHSCH3YEKE6PAVCNFSM6AAAAAA7E4I45SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBZGEZTOOBWHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@frankfeir merged this pull request in 06b5b4a. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
bpf(icmp-too-big): This commit fixes a bug that katran cannot deal with large packets.
Katran should generate
ICMP_TOO_BIGpacket ('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 = 0option to fix bytes replacements and fragmentation affects.