Skip to content
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.

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

RX depacketizer (QUIC) #18610

Closed
wants to merge 9 commits into from
Closed

Conversation

levitte
Copy link
Member

@levitte levitte commented Jun 20, 2022

The same-ish module as the TX packetizer, handling the opposite direction.

Ref to companions: #18564 #18570

@levitte
Copy link
Member Author

levitte commented Jun 20, 2022

This is a very early draft with a bunch of missing pieces

@levitte levitte marked this pull request as ready for review June 22, 2022 07:27
@levitte
Copy link
Member Author

levitte commented Jun 22, 2022

This PR is ready for comments. What needs more detail? What did I forget?

Mind you, I'm going on vacation June 23rd -- July 4th, so will not respond much to comments during that period.

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Jun 22, 2022
@levitte
Copy link
Member Author

levitte commented Jun 22, 2022

(why the hell did this trigger the fips-change label??????)

@t8m t8m closed this Jun 22, 2022
@t8m t8m reopened this Jun 22, 2022
@github-actions github-actions bot removed the severity: fips change The pull request changes FIPS provider sources label Jun 22, 2022
@levitte
Copy link
Member Author

levitte commented Jun 22, 2022

Thanks, @t8m

@t8m
Copy link
Member

t8m commented Jun 22, 2022

I do not yet know the exact conditions when this happens but it has something to do with changes on the to-be-merged-in branch in time of the pushes or in between the pushes.

doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
@levitte levitte mentioned this pull request Jul 11, 2022
2 tasks
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
@t8m t8m added triaged: design The issue/pr deals with a design document branch: master Merge to master branch labels Jul 27, 2022
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
doc/designs/quic-design/rx-depacketizer.md Outdated Show resolved Hide resolved
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we discuss this at OTC yet? We probably should do.

Copy link
Contributor

@paulidale paulidale left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Subject to the overview diagram receiving the appropriate renaming.

doc/designs/quic-design/rx-depacketizer.md Show resolved Hide resolved
@paulidale
Copy link
Contributor

IMO: OTC discussion can happen after this is merged and any changes made in a separate PR. Until OTC review, this should be considered as a draft (perhaps add a comment at the top indicating this, lest this draft be pointed at as canonical).

There are too many design documents lingering in an unmerged state.

@paulidale paulidale added the approval: done This pull request has the required number of approvals label Aug 17, 2022
@levitte
Copy link
Member Author

levitte commented Aug 17, 2022

There are too many design documents lingering in an unmerged state.

Agreed, it makes it very difficult to figure out what's what...

@mattcaswell
Copy link
Member

IMO: OTC discussion can happen after this is merged and any changes made in a separate PR. Until OTC review, this should be considered as a draft (perhaps add a comment at the top indicating this, lest this draft be pointed at as canonical).

There are too many design documents lingering in an unmerged state.

OTOH if we merge them without OTC review they will probably never receive OTC review because we will forget to do it.

@levitte
Copy link
Member Author

levitte commented Aug 17, 2022

OTOH if we merge them without OTC review they will probably never receive OTC review because we will forget to do it.

Then maybe we need to start by having these much higher on the priority list of the OTC meetings.

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@levitte
Copy link
Member Author

levitte commented Aug 18, 2022

Merged

d7fed97 RX depacketizer (QUIC)
42a0817 Rename "RX Frame Handler" to "RX Depacketizer" in the overview

@levitte levitte closed this Aug 18, 2022
@levitte levitte deleted the rx-depacketizer branch August 18, 2022 05:41
openssl-machine pushed a commit that referenced this pull request Aug 18, 2022
The same-ish module as the TX packetizer, handling the opposite direction.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #18610)
openssl-machine pushed a commit that referenced this pull request Aug 18, 2022
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #18610)
sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
The same-ish module as the TX packetizer, handling the opposite direction.

Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#18610)
sftcd pushed a commit to sftcd/openssl that referenced this pull request Sep 24, 2022
Reviewed-by: Paul Dale <pauli@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from openssl#18610)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch triaged: design The issue/pr deals with a design document
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants