feat(net): add configurable per-source/sink DSCP (IP_TOS) QoS marking - #25956
feat(net): add configurable per-source/sink DSCP (IP_TOS) QoS marking#25956glatinone wants to merge 1 commit into
Conversation
Adds DSCP name parsing, socket TOS setters/getters with graceful fallback, config docs, and unit tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
Bot
added
domain: sources
|
Thank you for your contribution! Before we can merge this PR, please sign our Contributor License Agreement. To sign, copy and post the phrase below as a new comment on this PR.
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60f69d4207
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ```toml | ||
| [sources.my_tcp] | ||
| # ... other settings | ||
| ip_tos = "EF" # or "CS5", "AF21", etc. |
There was a problem hiding this comment.
The advertised ip_tos key is not actually accepted or applied: this commit only adds standalone files, while src/net.rs does not declare mod qos, src/sources/util/net/tcp/mod.rs does not declare/call qos, and rg "ip_tos|apply_qos|parse_ip_tos" finds no config fields or call sites outside these new files/docs. Cargo will not build the helpers or run their tests, so configurations using this key cannot mark sockets. Please wire the field through the source/sink configs and apply it on connect/accept before documenting it.
Useful? React with 👍 / 👎.
| if let Err(e) = set_stream_tos(stream, t) { | ||
| let _ = e; |
There was a problem hiding this comment.
When set_stream_tos fails for platform, permission, or socket-family reasons, this helper discards the error with let _ = e and returns (). That makes QoS silently absent even though the new reference text says unsupported platforms are ignored with a warning, so operators have no signal that DSCP marking did not take effect. Please emit a warning here or return the error to the caller.
Useful? React with 👍 / 👎.
| #[cfg(any(unix, windows))] | ||
| { | ||
| let sref = SockRef::from(stream); | ||
| return sref.set_tos(tos); |
There was a problem hiding this comment.
When this helper is wired to an IPv6 source or sink, SockRef::set_tos only sets the IPv4 IP_TOS option, while IPv6 packets carry DSCP in the IPv6 traffic-class field. Vector accepts IPv6 socket addresses, so IPv6 users would either get an error or a connection whose packets are not marked despite ip_tos being configured. Please set the IPv6 traffic class too, or reject/document IPv4-only support.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,15 @@ | |||
| # DSCP (IP_TOS) QoS Marking | |||
There was a problem hiding this comment.
This new user-facing reference page is added under top-level docs/, which this repo uses for developer documentation, while the public Vector reference pages live under website/content/en/docs/reference with CUE-generated component data. Because nothing links or builds docs/reference/qos-dscp.md, the new option page will not appear on vector.dev even after the feature is wired. Please move or duplicate this content in the website reference documentation path.
Useful? React with 👍 / 👎.
1 participant
Adds per-source/sink DSCP (IP_TOS) QoS configuration with DSCP name parsing (CS0-CS7, AFxx, EF) and socket application using socket2::SockRef::set_tos(). Unsupported platforms fail gracefully. Includes docs and unit tests for parsing and TOS get/set.
Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com