Skip to content

Minor clarification on handling of fragmented packets #64

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Minor clarification on handling of fragmented packets #64

wants to merge 3 commits into from

Conversation

jafingerhut
Copy link
Contributor

No description provided.

@jafingerhut
Copy link
Contributor Author

Created in response to discussion on this issue: #61

@chrispsommers
Copy link
Collaborator

I think #63 and #64 are both trying to address the same issue, somewhat differently.

@chrispsommers chrispsommers mentioned this pull request Feb 24, 2022
Merged
@jfingerh
Copy link
Collaborator

Everything in PR #63 looks reasonable to me, but I do not see how #63 addresses the issue that non-first fragments must be dropped if there is no matching temporal flow state that matches it.

@chrispsommers
Copy link
Collaborator

chrispsommers commented Feb 24, 2022 •
edited
Loading

It seems like PR64 is a superset. Perhaps the two authors @jafingerhut and @mhanif can agree on a single unified PR? Thanks, it's nice to have a surplus of enthusiasm!

@jfingerh
Copy link
Collaborator

If I understood correctly, in the meeting today Gerald suggested I create a PR for Microsoft engineers to look at and think about when they are doing their careful review of IP fragment handling, and likely this PR will be closed without being merged, as they will make their own changes. It is fine with me either way.

@jfingerh jfingerh mentioned this pull request Feb 27, 2022
Open
@KrisNey-MSFT KrisNey-MSFT added engineering Fragments We have many PRs and Issues around Fragments, creating a Label to pull them together labels Feb 28, 2022
@KrisNey-MSFT
Copy link
Collaborator

Hi Andy, I merged and closed PR 63, would you mind resyncing and submit 64 again so I can view the changes?
Thank you very much, Kristina

@jfingerh
Copy link
Collaborator

jfingerh commented Mar 1, 2022

@KrisNey-MSFT I have merged the latest changes in the main branch with the changes proposed in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
engineering Fragments We have many PRs and Issues around Fragments, creating a Label to pull them together
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jafingerhut @chrispsommers @jfingerh @KrisNey-MSFT