tcp_proxy: added changes to support ppv2 tlvs to be merged when downstream proxy protocol values are present #42824
+342
−7
Conversation
This file contains hidden or 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
added 2 commits
December 30, 2025 16:46
…tocol values are present Signed-off-by: Prasad I V <[email protected]>
Signed-off-by: Prasad I V <[email protected]>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
added 3 commits
January 5, 2026 08:51
Signed-off-by: Prasad I V <[email protected]>
Signed-off-by: Prasad I V <[email protected]>
Signed-off-by: Prasad I V <[email protected]>
agrawroh
reviewed
Jan 6, 2026
Comment on lines
340
to
355
| // When set to true and the connection already contains ``PROXY`` protocol state | ||
| // (including TLVs) from a downstream proxy protocol listener filter, the TLVs | ||
| // specified in ``proxy_protocol_tlvs`` are merged with the downstream TLVs. | ||
| // | ||
| // **Precedence behavior**: | ||
| // | ||
| // - TLVs specified in ``proxy_protocol_tlvs`` take precedence over downstream TLVs | ||
| // with the same type. | ||
| // - This allows adding or overriding specific TLV values while preserving | ||
| // other downstream TLVs. | ||
| // - The source/destination addresses from the downstream ``PROXY`` protocol state | ||
| // are preserved (not modified). | ||
| // | ||
| // When set to false (default), the ``proxy_protocol_tlvs`` are ignored if | ||
| // downstream ``PROXY`` protocol state already exists. | ||
| bool merge_proxy_protocol_tlvs = 23; |
Member
There was a problem hiding this comment.
I would suggest calling it merge_with_downstream_tlvs to be more clear and also add more info to the docs.
Suggested change
| // When set to true and the connection already contains ``PROXY`` protocol state | |
| // (including TLVs) from a downstream proxy protocol listener filter, the TLVs | |
| // specified in ``proxy_protocol_tlvs`` are merged with the downstream TLVs. | |
| // | |
| // **Precedence behavior**: | |
| // | |
| // - TLVs specified in ``proxy_protocol_tlvs`` take precedence over downstream TLVs | |
| // with the same type. | |
| // - This allows adding or overriding specific TLV values while preserving | |
| // other downstream TLVs. | |
| // - The source/destination addresses from the downstream ``PROXY`` protocol state | |
| // are preserved (not modified). | |
| // | |
| // When set to false (default), the ``proxy_protocol_tlvs`` are ignored if | |
| // downstream ``PROXY`` protocol state already exists. | |
| bool merge_proxy_protocol_tlvs = 23; | |
| // Controls whether TLVs specified in ``proxy_protocol_tlvs`` are merged with downstream | |
| // ``PROXY`` protocol TLVs when downstream ``PROXY`` protocol state already exists. | |
| // | |
| // When set to ``true`` and the connection already contains ``PROXY`` protocol state | |
| // (including TLVs) from a downstream proxy protocol listener filter, the TLVs | |
| // specified in ``proxy_protocol_tlvs`` are merged with the downstream TLVs. | |
| // | |
| // When set to ``false`` (default), the TLVs specified in ``proxy_protocol_tlvs`` are | |
| // ignored if downstream ``PROXY`` protocol state already exists. In this case, only | |
| // the downstream TLVs are forwarded to the upstream. | |
| // | |
| // **Precedence behavior**: | |
| // | |
| // - TLVs specified in ``proxy_protocol_tlvs`` take precedence over downstream TLVs | |
| // with the same type. This allows adding or overriding specific TLV values while | |
| // preserving other downstream TLVs. | |
| // - The source/destination addresses from the downstream ``PROXY`` protocol state | |
| // are preserved (not modified). | |
| // - TLVs with types that do not conflict are combined, resulting in a merged set | |
| // of TLVs from both sources. | |
| // | |
| // .. note:: | |
| // This field only takes effect when downstream ``PROXY`` protocol state exists. | |
| // If the TCP proxy filter is creating new ``PROXY`` protocol state, this field | |
| // has no effect and the TLVs specified in ``proxy_protocol_tlvs`` are always | |
| // included. | |
| // | |
| // .. note:: | |
| // To ensure the merged TLVs are allowed in the upstream ``PROXY`` protocol header, | |
| // you must also configure passthrough TLVs on the upstream proxy protocol transport. | |
| // See :ref:`core.v3.ProxyProtocolConfig.pass_through_tlvs <envoy_v3_api_field_config.core.v3.ProxyProtocolConfig.pass_through_tlvs>` | |
| // for details. | |
| bool merge_with_downstream_tlvs = 23; |
Contributor
Author
There was a problem hiding this comment.
Makes sense. I've made the changes to rename the field along with the doc suggestions
Member
|
/assign @ggreenway |
…ith_downstream_tlvs Signed-off-by: Prasad I V <[email protected]>
ggreenway
requested changes
Jan 6, 2026
| // you must also configure passthrough TLVs on the upstream proxy protocol transport. | ||
| // See :ref:`core.v3.ProxyProtocolConfig.pass_through_tlvs <envoy_v3_api_field_config.core.v3.ProxyProtocolConfig.pass_through_tlvs>` | ||
| // for details. | ||
| bool merge_with_downstream_tlvs = 23; |
Member
There was a problem hiding this comment.
I think we need to be careful with how we handle merge vs override, when considering #42075. Would it ever make sense to add an additional TLV of the same key? Maybe we should use an enum instead of a bool here so that if different behaviors are needed in the future we can easily add it to the API.
|
|
||
| // Add downstream TLVs that don't conflict with tcp_proxy TLVs. | ||
| for (const auto& tlv : existing_data.tlv_vector_) { | ||
| if (!seen_types.contains(tlv.type)) { |
Member
There was a problem hiding this comment.
This will introduce the bug reported in #42075 to tcp_proxy.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Currently proxy_protocol_tlvs field in tcp_proxy ignores the TLVs specified when downstream has proxy protocol data. This changes adds support to merge the TLVs through an explicit field
merge_proxy_protocol_tlvs. When enabledproxy_protocol_tlvstake precedence over downstream TLVs with the same type.other downstream TLVs.
PROXYprotocol state are preserved.Commit Message: tcp_proxy: added changes to support ppv2 tlvs to be merged when downstream proxy protocol values are present
Additional Description: Currently proxy_protocol_tlvs field in tcp_proxy ignores the TLVs specified when downstream has proxy protocol data. This changes adds support to merge the TLVs through an explicit field
merge_proxy_protocol_tlvs.Risk Level: Low
Testing: Unit tests
Docs Changes: Added
Release Notes: Added
Platform Specific Features: N/A
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]