Skip to content

augment hash to consider icmp ids - #1676

Open
Fredi-raspall wants to merge 1 commit into
mainfrom
pr/fredi/packet_hash_icmp
Open

augment hash to consider icmp ids#1676
Fredi-raspall wants to merge 1 commit into
mainfrom
pr/fredi/packet_hash_icmp

Conversation

@Fredi-raspall

Copy link
Copy Markdown
Contributor

Augment the hash method for ip packets to consider the ICMP id field (if present). This adds entropy and allows spraying ICMP traffic over ECMP legs.

Augment the hash method for ip packets to consider the ICMP id
field (if present). This adds entropy and allows spraying ICMP
traffic over ECMP legs.

Signed-off-by: Fredi Raspall <fredi@githedgehog.com>
@Fredi-raspall Fredi-raspall self-assigned this Jul 31, 2026
Copilot AI review requested due to automatic review settings July 31, 2026 11:25
@Fredi-raspall
Fredi-raspall requested a review from a team as a code owner July 31, 2026 11:25
@Fredi-raspall
Fredi-raspall removed the request for review from a team July 31, 2026 11:25
@Fredi-raspall Fredi-raspall added the ci:+release Enable VLAB release tests label Jul 31, 2026
@Fredi-raspall Fredi-raspall added the ci:+vlab Enable VLAB tests label Jul 31, 2026
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 78f2a82c-e986-405c-b50f-560a023dff06

📥 Commits

Reviewing files that changed from the base of the PR and between ce05789 and ed2685a.

📒 Files selected for processing (1)
  • net/src/packet/hash.rs

📝 Walkthrough

Walkthrough

Changes

ICMP hashing

Layer / File(s) Summary
Hash ICMP transport identifiers
net/src/packet/hash.rs
hash_ip now includes identifiers from ICMPv4 and ICMPv6 transports when present.

Suggested reviewers: daniel-noland

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the change to include ICMP identifiers in packet hashing.
Description check ✅ Passed The description directly explains the ICMP identifier hashing change and its ECMP distribution purpose.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the IP packet hashing logic to incorporate the ICMP identifier (when present) so that ICMP query traffic (e.g., Echo) gains additional entropy for improved ECMP spreading.

Changes:

  • Extend Packet::hash_ip to hash the ICMPv4 identifier when available.
  • Extend Packet::hash_ip to hash the ICMPv6 identifier when available.

Comment thread net/src/packet/hash.rs
Comment on lines +41 to +50
Transport::Icmp4(icmp4) => {
if let Some(id) = icmp4.identifier() {
id.hash(state);
}
}
Transport::Icmp6(icmp6) => {
if let Some(id) = icmp6.identifier() {
id.hash(state);
}
}
@Fredi-raspall
Fredi-raspall added this pull request to the merge queue Aug 1, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:+release Enable VLAB release tests ci:+vlab Enable VLAB tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants