Add options support for SocketDiagTCP #1127
Open
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
…s for querying TCP socket information
WalkthroughIntroduces an options-driven TCP socket diagnostics API enabling multi-family (AF_INET/AF_INET6) queries. Adds SocketDiagTCPOptions, new Handle and package-level SocketDiagTCPWithOptions functions, and refactors the single-family SocketDiagTCP to delegate to the new pathway. Implements per-family request iteration, defaults, validation, and specific error handling for ErrDumpInterrupted. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant API as SocketDiagTCPWithOptions (pkg/Handle)
participant Loop as For each Family
participant NL as Netlink
participant Proc as Response Processor
Caller->>API: SocketDiagTCPWithOptions(opts)
API->>API: Resolve Families (default to AF_INET + AF_INET6 if 0)
API->>API: Resolve States (default to all if 0)
loop For each enabled family
API->>NL: Send TCPDIAG request (Ext=0, States, Family)
NL-->>API: Dump responses (per-family)
API->>Proc: Parse responses into Socket objects
alt Non-ErrDumpInterrupted error
API-->>Caller: Return immediately with error
else ErrDumpInterrupted
Note right of API: Record interruption, continue
end
end
API-->>Caller: Sockets (+possible ErrDumpInterrupted)
rect rgba(230,245,255,0.5)
note right of Caller: Legacy path
Caller->>API: SocketDiagTCP(family)
API->>API: Build options (Families bit for family)
API->>API: Delegate to SocketDiagTCPWithOptions
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
socket_linux.go (4)
341-351: Clarify bitmask semantics to prevent misuse (document the 1<<family/state expectation).Make it explicit that callers must pass bitmasks like (1<<unix.AF_INET) and (1<<TCP_ESTABLISHED), not raw AF_/TCP_ enums.
-// SocketDiagTCPOptions represents the configuration options for socket diagnostics. +// SocketDiagTCPOptions represents the configuration options for TCP socket diagnostics. type SocketDiagTCPOptions struct { - // Families is a bitset representing the internet address families of interest to be used in the query. The bits - // are expected to be set according to the unix.AF_* constants. If Families is zero, both AF_INET and AF_INET6 - // address families will be retrieved by default. + // Families is a bitmask of internet address families to query. + // Set bits as (1<<unix.AF_*), e.g. (1<<unix.AF_INET)|(1<<unix.AF_INET6). + // If zero, both AF_INET and AF_INET6 are queried. Families uint32 - // States is a bitset that specifies the TCP state of the target sockets to be retrieved. The bits are - // expected to be set according to TCP_* state constants. If States is zero, all states will be retrieved. + // States is a bitmask of TCP states (inet_diag idiag_states). + // Set bits as (1<<TCP_*), e.g. (1<<TCP_ESTABLISHED). If zero, all states are queried. States uint32 }(Optional) Provide helpers to reduce call-site errors:
// add near top-level const ( SocketDiagFamilyIPv4 = uint32(1) << unix.AF_INET SocketDiagFamilyIPv6 = uint32(1) << unix.AF_INET6 SocketDiagStatesAll = uint32(0x0fff) )
362-365: Mask unexpected state bits (defensive).Kernel currently uses 12 state bits. Masking user input avoids accidental out-of-range bits.
if opts.States == 0 { - opts.States = uint32(0xfff) // all states + opts.States = uint32(0x0fff) // all states + } else { + opts.States &= uint32(0x0fff) }
366-371: More actionable error for unsupported families.Return the unsupported mask and hint on using (1<<family).
- // check if any unsupported families are set - supportedFamilies := uint32((1 << unix.AF_INET) | (1 << unix.AF_INET6)) - if opts.Families&^supportedFamilies != 0 { - return nil, fmt.Errorf("unsupported address families specified: only AF_INET and AF_INET6 are supported") - } + // check if any unsupported families are set + supportedFamilies := (uint32(1) << unix.AF_INET) | (uint32(1) << unix.AF_INET6) + if unsupported := opts.Families &^ supportedFamilies; unsupported != 0 { + return nil, fmt.Errorf("unsupported families mask 0x%x; only AF_INET (%d) and AF_INET6 (%d) are supported; use (1<<family)", + unsupported, unix.AF_INET, unix.AF_INET6) + }
360-361: Prefer typed shifts for clarity and portability.Make the masks explicitly uint32 to avoid implicit typing quirks and improve readability.
- opts.Families = (1 << unix.AF_INET) | (1 << unix.AF_INET6) // all IPv4 and IPv6 sockets + opts.Families = (uint32(1) << unix.AF_INET) | (uint32(1) << unix.AF_INET6) // all IPv4 and IPv6 sockets- supportedFamilies := uint32((1 << unix.AF_INET) | (1 << unix.AF_INET6)) + supportedFamilies := (uint32(1) << unix.AF_INET) | (uint32(1) << unix.AF_INET6)- if opts.Families&(1<<family) != 0 { + if opts.Families&(uint32(1)<<family) != 0 {Also applies to: 367-367, 375-376
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
socket_linux.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
socket_linux.go (6)
handle_linux.go (1)
Handle(24-27)socket.go (1)
Socket(16-27)nl/syscall.go (1)
SOCK_DIAG_BY_FAMILY(48-48)nl/tc_linux.go (1)
IPPROTO_TCP(1247-1247)nl/nl_linux.go (1)
ErrDumpInterrupted(51-51)netlink_linux.go (1)
ErrDumpInterrupted(14-14)
🔇 Additional comments (2)
socket_linux.go (2)
359-365: Sane defaults. LGTM.Defaults to both families and all states are reasonable and backward-friendly.
386-393: Parsing loop looks solid.Deserialization errors halt the current iteration as expected; successful messages append to result.
Comment on lines
+353
to
+358
| // SocketDiagTCPWithOptions requests INET_DIAG_INFO for TCP protocol for specified options and returns related sockets. | ||
| // Currently only AF_INET and AF_INET6 address families are supported. | ||
| // | ||
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent | ||
| // or incomplete. | ||
| func (h *Handle) SocketDiagTCP(family uint8) ([]*Socket, error) { | ||
| // Construct the request | ||
| req := h.newNetlinkRequest(nl.SOCK_DIAG_BY_FAMILY, unix.NLM_F_DUMP) | ||
| req.AddData(&socketRequest{ | ||
| Family: family, | ||
| Protocol: unix.IPPROTO_TCP, | ||
| Ext: (1 << (INET_DIAG_VEGASINFO - 1)) | (1 << (INET_DIAG_INFO - 1)), | ||
| States: uint32(0xfff), // all states | ||
| }) | ||
| func (h *Handle) SocketDiagTCPWithOptions(opts SocketDiagTCPOptions) ([]*Socket, error) { |
There was a problem hiding this comment.
Doc fix: this path does not request INET_DIAG_INFO extensions.
Ext is 0; the function returns base Socket messages only. Update the comment to avoid misleading users.
-// SocketDiagTCPWithOptions requests INET_DIAG_INFO for TCP protocol for specified options and returns related sockets.
-// Currently only AF_INET and AF_INET6 address families are supported.
+// SocketDiagTCPWithOptions performs TCP socket diagnostics (INET_DIAG) for the
+// specified options and returns base Socket records (no INET_DIAG_* extensions).
+// Currently only AF_INET and AF_INET6 address families are supported.
//
-// If the returned error is [ErrDumpInterrupted], results may be inconsistent
-// or incomplete.
+// If the returned error is [ErrDumpInterrupted], results may be inconsistent or incomplete.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // SocketDiagTCPWithOptions requests INET_DIAG_INFO for TCP protocol for specified options and returns related sockets. | |
| // Currently only AF_INET and AF_INET6 address families are supported. | |
| // | |
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent | |
| // or incomplete. | |
| func (h *Handle) SocketDiagTCP(family uint8) ([]*Socket, error) { | |
| // Construct the request | |
| req := h.newNetlinkRequest(nl.SOCK_DIAG_BY_FAMILY, unix.NLM_F_DUMP) | |
| req.AddData(&socketRequest{ | |
| Family: family, | |
| Protocol: unix.IPPROTO_TCP, | |
| Ext: (1 << (INET_DIAG_VEGASINFO - 1)) | (1 << (INET_DIAG_INFO - 1)), | |
| States: uint32(0xfff), // all states | |
| }) | |
| func (h *Handle) SocketDiagTCPWithOptions(opts SocketDiagTCPOptions) ([]*Socket, error) { | |
| // SocketDiagTCPWithOptions performs TCP socket diagnostics (INET_DIAG) for the | |
| // specified options and returns base Socket records (no INET_DIAG_* extensions). | |
| // Currently only AF_INET and AF_INET6 address families are supported. | |
| // | |
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent or incomplete. | |
| func (h *Handle) SocketDiagTCPWithOptions(opts SocketDiagTCPOptions) ([]*Socket, error) { |
🤖 Prompt for AI Agents
In socket_linux.go around lines 353 to 358, the function comment incorrectly
states it "requests INET_DIAG_INFO" — update the docstring to accurately state
that Ext is 0 and the function does not request INET_DIAG_INFO extensions, and
that it returns only base Socket messages (not extended info); keep the note
about ErrDumpInterrupted unchanged.
Comment on lines
+406
to
+414
| // SocketDiagTCPWithOptions requests INET_DIAG_INFO for TCP protocol for specified options and returns related sockets. | ||
| // Currently only AF_INET and AF_INET6 address families are supported. | ||
| // | ||
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent | ||
| // or incomplete. | ||
| func SocketDiagTCPWithOptions(opts SocketDiagTCPOptions) ([]*Socket, error) { | ||
| return pkgHandle.SocketDiagTCPWithOptions(opts) | ||
| } | ||
|
|
There was a problem hiding this comment.
Doc fix: wrapper also does not request INET_DIAG_INFO.
Ext is 0 in the callee; adjust the comment.
-// SocketDiagTCPWithOptions requests INET_DIAG_INFO for TCP protocol for specified options and returns related sockets.
-// Currently only AF_INET and AF_INET6 address families are supported.
+// SocketDiagTCPWithOptions performs TCP socket diagnostics (INET_DIAG) for the
+// specified options and returns base Socket records (no INET_DIAG_* extensions).
+// Currently only AF_INET and AF_INET6 address families are supported.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // SocketDiagTCPWithOptions requests INET_DIAG_INFO for TCP protocol for specified options and returns related sockets. | |
| // Currently only AF_INET and AF_INET6 address families are supported. | |
| // | |
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent | |
| // or incomplete. | |
| func SocketDiagTCPWithOptions(opts SocketDiagTCPOptions) ([]*Socket, error) { | |
| return pkgHandle.SocketDiagTCPWithOptions(opts) | |
| } | |
| // SocketDiagTCPWithOptions performs TCP socket diagnostics (INET_DIAG) for the | |
| // specified options and returns base Socket records (no INET_DIAG_* extensions). | |
| // Currently only AF_INET and AF_INET6 address families are supported. | |
| // | |
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent | |
| // or incomplete. | |
| func SocketDiagTCPWithOptions(opts SocketDiagTCPOptions) ([]*Socket, error) { | |
| return pkgHandle.SocketDiagTCPWithOptions(opts) | |
| } |
🤖 Prompt for AI Agents
In socket_linux.go around lines 406 to 414, the function comment incorrectly
states that the wrapper requests INET_DIAG_INFO; the callee currently uses ext=0
so INET_DIAG_INFO is not requested — update the doc comment to remove the claim
about requesting INET_DIAG_INFO, note that ext is 0 in the callee (so no
INET_DIAG_INFO returned), and keep the rest of the description (supported
families AF_INET/AF_INET6 and ErrDumpInterrupted caveat) accurate.
Comment on lines
+415
to
424
| // SocketDiagTCP requests INET_DIAG_INFO for TCP protocol for specified family type and returns related sockets. | ||
| // | ||
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent | ||
| // or incomplete. | ||
| func (h *Handle) SocketDiagTCP(family uint8) ([]*Socket, error) { | ||
| opts := SocketDiagTCPOptions{ | ||
| Families: 1 << family, | ||
| } | ||
| return h.SocketDiagTCPWithOptions(opts) | ||
| } |
There was a problem hiding this comment.
Doc fix: this wrapper returns base sockets, not INET_DIAG_INFO.
Also hint at SocketDiagTCPInfo for extended info.
-// SocketDiagTCP requests INET_DIAG_INFO for TCP protocol for specified family type and returns related sockets.
+// SocketDiagTCP performs TCP socket diagnostics (INET_DIAG) for the specified family
+// and returns base Socket records. For extended TCPInfo, use SocketDiagTCPInfo.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // SocketDiagTCP requests INET_DIAG_INFO for TCP protocol for specified family type and returns related sockets. | |
| // | |
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent | |
| // or incomplete. | |
| func (h *Handle) SocketDiagTCP(family uint8) ([]*Socket, error) { | |
| opts := SocketDiagTCPOptions{ | |
| Families: 1 << family, | |
| } | |
| return h.SocketDiagTCPWithOptions(opts) | |
| } | |
| // SocketDiagTCP performs TCP socket diagnostics (INET_DIAG) for the specified family | |
| // and returns base Socket records. For extended TCPInfo, use SocketDiagTCPInfo. | |
| // | |
| // If the returned error is [ErrDumpInterrupted], results may be inconsistent | |
| // or incomplete. | |
| func (h *Handle) SocketDiagTCP(family uint8) ([]*Socket, error) { | |
| opts := SocketDiagTCPOptions{ | |
| Families: 1 << family, | |
| } | |
| return h.SocketDiagTCPWithOptions(opts) | |
| } |
🤖 Prompt for AI Agents
In socket_linux.go around lines 415 to 424, the function comment incorrectly
states it "requests INET_DIAG_INFO" and should instead document that this
wrapper returns base Socket structures (not INET_DIAG_INFO); update the
docstring to say it returns base sockets and add a brief hint referencing
SocketDiagTCPInfo for callers who need extended INET_DIAG_INFO details or richer
diagnostics.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
introduced SocketDiagTCPOptions for specifying different filters for querying TCP socket information.
Summary by CodeRabbit