Feat/pedit helper methods#331
Conversation
Add support for the tc pedit action, including: - Core types: Pedit, PeditSel, PeditKey, PeditKeyEx - PeditHeaderType and PeditCmd enums for extended key attributes - marshal/unmarshal for the pedit netlink message format High-level helpers that abstract raw byte offset calculations: - SetIPv4Src/SetIPv4Dst: rewrite IPv4 source/destination addresses - SetEthSrc/SetEthDst: rewrite Ethernet MAC addresses - SetSrcPort/SetDstPort/SetUDPSrc/SetUDPDst: rewrite L4 ports - AddIPv4TTL: increment or decrement the IPv4 TTL field
Add Pedit *Pedit field to Action, and handle "pedit" in marshalAction and extractActOptions so callers can use Kind: "pedit" alongside other action types.
|
Hi @Arshiya2005 |
florianl
left a comment
There was a problem hiding this comment.
Thanks for your work and contribution.
While I support merging the pedit module, I have reservations about the helper functions. They only cover a small subset of functionality and introduce maintenance overhead that falls outside the core scope of this package. Historically, we have avoided adding similar helpers for other TC modules for this exact reason.
| const ( | ||
| peditIPProtoTCP = 6 | ||
| peditIPProtoUDP = 17 | ||
| ) |
There was a problem hiding this comment.
TCP and UDP should not get redefined here.
| const ( | |
| peditIPProtoTCP = 6 | |
| peditIPProtoUDP = 17 | |
| ) |
| func (p *Pedit) SetUDPSrc(port uint16) { | ||
| p.SetSrcPort(port, peditIPProtoUDP) | ||
| } | ||
|
|
||
| func (p *Pedit) SetUDPDst(port uint16) { | ||
| p.SetDstPort(port, peditIPProtoUDP) | ||
| } |
There was a problem hiding this comment.
As these helpers are trivial and there are no TCP helpers, this should get removed.
| func (p *Pedit) SetUDPSrc(port uint16) { | |
| p.SetSrcPort(port, peditIPProtoUDP) | |
| } | |
| func (p *Pedit) SetUDPDst(port uint16) { | |
| p.SetDstPort(port, peditIPProtoUDP) | |
| } |
|
|
||
| func peditProtocolHeaderType(protocol uint8) (PeditHeaderType, bool) { | ||
| switch protocol { | ||
| case peditIPProtoTCP: |
There was a problem hiding this comment.
Add IPPROTO_TCP to internal/unix.
| case peditIPProtoTCP: | |
| case unix.IPPROTO_TCP: |
| switch protocol { | ||
| case peditIPProtoTCP: | ||
| return PeditHeaderTypeTCP, true | ||
| case peditIPProtoUDP: |
There was a problem hiding this comment.
Add IPPROTO_UDP to internal/unix.
| case peditIPProtoUDP: | |
| case unix.IPPROTO_UDP: |
| func peditSwap16(v uint16) uint16 { | ||
| return (v << 8) | (v >> 8) | ||
| } |
There was a problem hiding this comment.
Please use the existing function endianSwapUint16() instead.
| func peditSwap16(v uint16) uint16 { | |
| return (v << 8) | (v >> 8) | |
| } |
| err = unmarshalStruct(ad.Bytes(), tcft) | ||
| multiError = concatError(multiError, err) | ||
| info.Tm = tcft | ||
| case tcaPeditKeysEx, tcaPeditPad: |
There was a problem hiding this comment.
Please implement tcaPeditKeysEx.
| ip4 := ip.To4() | ||
| if ip4 == nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
Please use the existing helper functions.
| ip4 := ip.To4() | |
| if ip4 == nil { | |
| return | |
| } | |
| ip4, err := ipToUint32(ip) |
| ip4 := ip.To4() | ||
| if ip4 == nil { | ||
| return | ||
| } |
There was a problem hiding this comment.
Please use the existing helper functions.
| ip4 := ip.To4() | |
| if ip4 == nil { | |
| return | |
| } | |
| ip4, err := ipToUint32(ip) |
Problem
Working with the tc pedit action currently requires callers to manually
compute byte offsets, masks, and shifts for each packet field they want
to rewrite. This is error-prone and requires deep knowledge of packet
header layouts.
Changes
Add full support for the pedit action, including:
High-level helpers that abstract raw byte offset calculations:
Sample TC Rule
The helpers map directly to munge clauses — each call sets the correct offset, mask, and value internally:
Why
These helpers cover the most common stateless packet rewriting use cases,
following the same pattern established by other action types in this library.
Callers can now use p.SetIPv4Src(ip) instead of constructing raw
PeditKey structs with hardcoded offsets.