Skip to content

quic: remove duplicate code in QUIC encodeData(), encodeMetadata() and encodeTrailers() implementations#32570

Merged
mattklein123 merged 11 commits intoenvoyproxy:mainfrom
RyanTheOptimist:encodeData
Feb 28, 2024
Merged

quic: remove duplicate code in QUIC encodeData(), encodeMetadata() and encodeTrailers() implementations#32570
mattklein123 merged 11 commits intoenvoyproxy:mainfrom
RyanTheOptimist:encodeData

Conversation

@RyanTheOptimist
Copy link
Copy Markdown
Contributor

@RyanTheOptimist RyanTheOptimist commented Feb 26, 2024 •
edited
Loading

quic: remove duplicate code in QUIC encodeData(), encodeMetadata() and encodeTrailers() implementations

Add a QuicSpdyStream& member to EnvoyQuicStream so that the encodeData(), encodeMetadata() and encodeTrailers() implementation can be moved to EnvoyQuicStream and out of the subclasses.

Risk Level: Low
Testing: Existing
Docs Changes: N/A
Release Notes: N/A

@RyanTheOptimist
quic: remove duplciate code in QUIC encodeData() implementations
906b525 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Cleanup
99b61c0 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
More cleanup
81ea8fc 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Format
775b6fd 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Add
c9ce16e 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Datagrams
2afb849 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Fomat
67906d5 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Formatter
54559d0 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Format
048ba50 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
Copy link
Copy Markdown
Contributor Author

RyanTheOptimist commented Feb 26, 2024

/assign @danzh2010

@RyanTheOptimist
encodeMetadata
0b8f0da 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist
encodeTrailers
702170e 
Signed-off-by: Ryan Hamilton <rch@google.com>
@RyanTheOptimist RyanTheOptimist changed the title quic: remove duplciate code in QUIC encodeData() implementations quic: remove duplciate code in QUIC encodeData(), encodeMetadata() and encodeTrailers() implementations Feb 27, 2024
@RyanTheOptimist RyanTheOptimist changed the title quic: remove duplciate code in QUIC encodeData(), encodeMetadata() and encodeTrailers() implementations quic: remove duplicate code in QUIC encodeData(), encodeMetadata() and encodeTrailers() implementations Feb 27, 2024
@RyanTheOptimist
Copy link
Copy Markdown
Contributor Author

RyanTheOptimist commented Feb 27, 2024

@danzh2010 friendly ping

@mattklein123 mattklein123 self-assigned this Feb 27, 2024
@mattklein123
Copy link
Copy Markdown
Member

mattklein123 commented Feb 27, 2024

/wait-any

danzh2010
danzh2010 reviewed Feb 27, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@danzh2010 danzh2010 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This is a good movement! Thanks for doing it!

Just one question, is it possible to also move part of encodeHeaders(), i.e. ScopedWatermarkBufferUpdater, byte tracker and local_end_stream_ to EnvoyQuicStream?

@danzh2010
Copy link
Copy Markdown
Contributor

danzh2010 commented Feb 27, 2024

This is a good movement! Thanks for doing it!

Just one question, is it possible to also move part of encodeHeaders(), i.e. ScopedWatermarkBufferUpdater, byte tracker and local_end_stream_ to EnvoyQuicStream?

Discussed offline, this can be done as a follow up.

@RyanTheOptimist
Copy link
Copy Markdown
Contributor Author

RyanTheOptimist commented Feb 27, 2024

This is a good movement! Thanks for doing it!
Just one question, is it possible to also move part of encodeHeaders(), i.e. ScopedWatermarkBufferUpdater, byte tracker and local_end_stream_ to EnvoyQuicStream?

Discussed offline, this can be done as a follow up.

Great, thanks! Over to you, @mattklein123

@RyanTheOptimist
Copy link
Copy Markdown
Contributor Author

RyanTheOptimist commented Feb 28, 2024

/retest

@mattklein123 mattklein123 merged commit c7ce585 into envoyproxy:main Feb 28, 2024
steveWang added a commit to steveWang/envoy that referenced this pull request Mar 6, 2024
@steveWang
Delete unused runtime flag.
9ea4fa3 
envoy.reloadable_features.quiche_use_mem_slice_releasor_api is no
longer used as of envoyproxy#32570.

Signed-off-by: Steve Wang <wangsteve@google.com>
@steveWang steveWang mentioned this pull request Mar 6, 2024
Merged
RyanTheOptimist pushed a commit that referenced this pull request Mar 6, 2024
@steveWang
Delete unused runtime flag. (#32739)
90846c6 
envoy.reloadable_features.quiche_use_mem_slice_releasor_api is no
longer used as of #32570.

Signed-off-by: Steve Wang <wangsteve@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mattklein123 mattklein123 mattklein123 approved these changes

@danzh2010 danzh2010 danzh2010 approved these changes

Assignees

@mattklein123 mattklein123

@danzh2010 danzh2010

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@RyanTheOptimist @mattklein123 @danzh2010 @KBaichoo