feat(socket): Socket::set_dscp()/get_dscp() typed option setters - #736
Merged
Conversation
The SocketReactor was reaching for the raw native handle and calling ::setsockopt directly to apply a DSCP marking. Move that onto the socket itself, alongside the existing typed option setters: - Socket::set_dscp(espp::Dscp): applies IP_TOS via the existing set_option() wrapper; rejects out-of-range (static_cast'd) code points with a log rather than silently masking them. - Socket::get_dscp(): reads the code point back from IP_TOS (ECN bits discarded), following the get_receive_buffer_size() pattern. - SocketReactor::add_udp_receiver now just calls socket.set_dscp() - no native handle, no platform ifdefs. - pc test dogfoods the new getter for the read-back checks and adds a direct set/get round-trip incl. rejection leaving the previous value in place (socket_reactor suite 49/49). - python: set_dscp/get_dscp bound on Socket; UdpSocket/TcpSocket bindings now declare Socket as their base (pre-existing gap - no base Socket method was reachable from python subclass instances), preserved on regeneration via a new _fix_base_classes postprocess in autogenerate_bindings.py; stub updated (TcpSocket/UdpSocket now inherit Socket). - docs: socket README lists the typed option setters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
✅Static analysis result - no issues found! ✅ |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR moves DSCP (DiffServ) marking into the espp::Socket API via typed set_dscp(espp::Dscp) / get_dscp() helpers, updates SocketReactor to use the new API instead of raw setsockopt, and wires the functionality through the Python bindings (including fixing missing base-class inheritance for UdpSocket/TcpSocket in Python).
Changes:
- Add
Socket::set_dscp()/Socket::get_dscp()implemented viaIP_TOSand the existingset_option()wrapper. - Update
SocketReactor::add_udp_receiver()to callsocket.set_dscp(...)and update host tests to useget_dscp()and validate out-of-range rejection behavior. - Expose DSCP APIs in Python and fix Python inheritance so derived sockets inherit base
Socketmethods.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
components/socket/include/socket.hpp |
Adds public typed DSCP setter/getter declarations and includes dscp.hpp. |
components/socket/src/socket.cpp |
Implements DSCP set/get using IP_TOS, with out-of-range rejection. |
components/socket/src/socket_reactor.cpp |
Replaces direct setsockopt(IP_TOS) with Socket::set_dscp() usage. |
components/socket/README.md |
Documents the typed option setters including DSCP helpers. |
pc/tests/socket_reactor.cpp |
Updates DSCP verification to use get_dscp() and adds round-trip/rejection tests. |
lib/python_bindings/pybind_espp.cpp |
Binds set_dscp/get_dscp and fixes pybind base classes for TcpSocket/UdpSocket. |
lib/python_bindings/espp/__init__.pyi |
Updates stubs to include DSCP APIs and correct Python inheritance. |
lib/autogenerate_bindings.py |
Adds a postprocess pass to enforce socket base classes in generated pybind code. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…g cleanly _fix_base_classes() now counts occurrences like _fix_implicit_default_ctors and prints a loud warning when a pattern applies != 1 time, so a litgen output drift cannot silently regress python-side UdpSocket/TcpSocket inheritance on regeneration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eserves stub fixes autogenerate() overwrites espp/__init__.pyi but only postprocessed pybind_espp.cpp, so a regeneration would have reverted the committed TcpSocket(Socket)/UdpSocket(Socket) stub inheritance. Add _postprocess_generated_stub() with the equivalent base-class replacements (same exactly-once count check + loud warning as _fix_base_classes) and apply it to the generated stub; the _STUB_FIX map is the extension point for future developer-facing .pyi improvements. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Follow-up to #735: the
SocketReactorwas reaching for the raw native handle and calling::setsockoptdirectly to apply a DSCP marking. This moves that onto the socket itself, alongside the existing typed option setters (set_receive_timeout,set_receive_buffer_size,set_reuse_address, …), so callers never need the native handle or platform#ifdefs for this.espp::Socketset_dscp(espp::Dscp)— appliesIP_TOSvia the existingset_option()wrapper (DSCP in the upper 6 bits, RFC 2474). Best-effort, documented as network/driver treatment only. Out-of-range (static_cast'd) code points are rejected with a log rather than silently masked to a different code point.get_dscp()— reads the code point back fromIP_TOS(ECN bits discarded), following theget_receive_buffer_size()pattern.espp::SocketReactoradd_udp_receiver()now just callssocket.set_dscp(...)— the rawsetsockoptblock is gone.Python
set_dscp/get_dscpbound onSocket.UdpSocket/TcpSocketwere bound without declaringespp::Socketas their base, so no baseSocketmethod (set_receive_timeout,native_handle, …) was reachable from Python. They now declare the base (preserved across regeneration via a new_fix_base_classespostprocess inautogenerate_bindings.py), and the stub declares the inheritance too.Testing
pc/tests/socket_reactor.cpp: read-back checks now dogfoodget_dscp(), plus a directset_dscp/get_dscpround-trip including out-of-range rejection leaving the previous code point in place — 49/49, repeated runs.set_dscp/get_dscpon bothUdpSocketandTcpSocket, inherited base methods (set_receive_timeout) now reachable,__mro__showsSocketas base.socketexample builds; lib/pybind builds; cppcheck clean.🤖 Generated with Claude Code