-
Notifications
You must be signed in to change notification settings - Fork 791
add more srv6 encap mode consts #1146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughAdded three new SEG6 encapsulation mode constants (L2ENCAP, ENCAP_RED, L2ENCAP_RED) and introduced the EncodeSEG6Srh function to encode Segment Routing Headers for IP segments. Extended SEG6EncapModeString to handle the new modes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
nl/seg6_linux.go (1)
130-149: Consider refactoring to eliminate code duplication.The function logic is correct and consistent with
EncodeSEG6Encap. However, the SRH encoding logic (lines 137-148) is duplicated fromEncodeSEG6Encap(lines 69-79). Consider refactoringEncodeSEG6Encapto callEncodeSEG6Srhand prepend the mode bytes, reducing maintenance burden.Example refactor for
EncodeSEG6Encap:func EncodeSEG6Encap(mode int, segments []net.IP) ([]byte, error) { - nsegs := len(segments) // nsegs: number of segments - if nsegs == 0 { + srh, err := EncodeSEG6Srh(segments) + if err != nil { - return nil, errors.New("EncodeSEG6Encap: No Segment in srh") + return nil, err } - b := make([]byte, 12, 12+len(segments)*16) + b := make([]byte, 4, 4+len(srh)) native := NativeEndian() native.PutUint32(b, uint32(mode)) - b[4] = 0 // srh.nextHdr (0 when calling netlink) - b[5] = uint8(16 * nsegs >> 3) // srh.hdrLen (in 8-octets unit) - b[6] = IPV6_SRCRT_TYPE_4 // srh.routingType (assigned by IANA) - b[7] = uint8(nsegs - 1) // srh.segmentsLeft - b[8] = uint8(nsegs - 1) // srh.firstSegment - b[9] = 0 // srh.flags (SR6_FLAG1_HMAC for srh_hmac) - // srh.reserved: Defined as "Tag" in draft-ietf-6man-segment-routing-header-07 - native.PutUint16(b[10:], 0) // srh.reserved - for _, netIP := range segments { - b = append(b, netIP...) // srh.Segments - } + b = append(b, srh...) return b, nil }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
nl/seg6_linux.go(4 hunks)
🔇 Additional comments (2)
nl/seg6_linux.go (2)
158-163: Implementation verified and correct.The string representations ("l2encap", "encap.red", "l2encap.red") match the iproute2 format for SEG6 encapsulation modes, confirming the implementation is correct.
40-47: Based on my verification through the Linux kernel source code, the kernel'snet/ipv6/seg6_iptunnel.cuses all five modes in switch statements:SEG6_IPTUN_MODE_INLINE,SEG6_IPTUN_MODE_ENCAP,SEG6_IPTUN_MODE_ENCAP_RED,SEG6_IPTUN_MODE_L2ENCAP, andSEG6_IPTUN_MODE_L2ENCAP_RED. This confirms that all the constants you've defined are valid and exist in the kernel.Your Go code correctly defines these constants using iota continuation, which assigns sequential values 0, 1, 2, 3, and 4 respectively. This matches the expected enumeration pattern for these tunnel modes.
No changes are needed to the constant definitions.
akerouanton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| case SEG6_IPTUN_MODE_L2ENCAP: | ||
| return "l2encap" | ||
| case SEG6_IPTUN_MODE_ENCAP_RED: | ||
| return "encap.red" | ||
| case SEG6_IPTUN_MODE_L2ENCAP_RED: | ||
| return "l2encap.red" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
static const char *seg6_mode_types[] = {
[SEG6_IPTUN_MODE_INLINE] = "inline",
[SEG6_IPTUN_MODE_ENCAP] = "encap",
[SEG6_IPTUN_MODE_L2ENCAP] = "l2encap",
[SEG6_IPTUN_MODE_ENCAP_RED] = "encap.red",
[SEG6_IPTUN_MODE_L2ENCAP_RED] = "l2encap.red",
};
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.