Add NetworkLimiter with bidirectional bandwidth limiting via tc integration#4
Add NetworkLimiter with bidirectional bandwidth limiting via tc integration#4
Conversation
Co-authored-by: pmarchini <[email protected]>
Co-authored-by: pmarchini <[email protected]>
…erface support Co-authored-by: pmarchini <[email protected]>
pmarchini
left a comment
There was a problem hiding this comment.
I want to be able to set a defined max bandwidth
I'd like to clarify the requirement. The cgroups v2 network controller doesn't directly support bandwidth limiting - it only provides classID (for packet classification) and priority settings. Network bandwidth limiting in Linux is typically achieved through:
Are you looking for:
Could you provide more details on your use case and preferred implementation approach? |
pmarchini
left a comment
There was a problem hiding this comment.
What I’m looking for is integration with Traffic Control (tc) rather than a new kernel-level bandwidth controller.
Concretely, my requirement is:
assign processes/services to a cgroup
use the cgroup’s classID / packet classification
automatically attach those packets to a tc HTB/TBF class
enforce a configurable max bandwidth (rate/ceil)
So the expected behavior would be something like:
cgroup → automatic tc class → bandwidth limit enforced
- Add --network-max-bandwidth flag for setting bandwidth limits - Implement tc (traffic control) HTB setup and cleanup - Integrate tc lifecycle management in executor - Add comprehensive tests for bandwidth parsing - Update README with bandwidth limiting documentation Co-authored-by: pmarchini <[email protected]>
- Export GetDefaultInterface to avoid duplication - Use os.Stderr for warning messages in executor - Return cleanup errors instead of silently logging - Remove duplicate getDefaultInterface function Co-authored-by: pmarchini <[email protected]>
Implemented! The feature now provides automatic tc integration exactly as requested: cgroup → automatic tc class → bandwidth limit enforced Added in commit 5ed595e:
Example: sudo giogo --network-class-id=100 --network-max-bandwidth=1m -- your_appThe implementation:
All tests pass with no security vulnerabilities detected. |
internal/executor/executor.go
Outdated
| executor := &Executor{ | ||
| Limiters: limiters, | ||
| } | ||
|
|
||
| // Extract NetworkLimiter if present for special handling | ||
| for _, l := range limiters { | ||
| if netLimiter, ok := l.(*limiter.NetworkLimiter); ok { | ||
| executor.NetworkLimiter = netLimiter | ||
| break | ||
| } | ||
| } | ||
|
|
||
| return executor |
There was a problem hiding this comment.
We should not break the design having special handling here
There was a problem hiding this comment.
Fixed in commit 595e495. Refactored to use LifecycleLimiter interface instead of special handling:
- Added optional
LifecycleLimiterinterface with Setup/Cleanup methods - Executor uses type assertion to check for the interface
- No special NetworkLimiter field or hardcoded logic
- Any limiter can implement LifecycleLimiter if needed
The design now maintains clean separation of concerns with interface-based patterns.
internal/executor/executor.go
Outdated
| } | ||
|
|
||
| // Set up traffic control if network limiter with bandwidth is configured | ||
| if e.NetworkLimiter != nil && e.NetworkLimiter.MaxBandwidth > 0 { |
There was a problem hiding this comment.
Fixed in commit 595e495. Removed all special handling by using the LifecycleLimiter interface pattern. The executor now treats all limiters uniformly through interfaces.
…Limiter handling - Add LifecycleLimiter interface with Setup/Cleanup methods - Implement Setup/Cleanup on NetworkLimiter - Remove special NetworkLimiter field from Executor - Use interface-based design for lifecycle management - Maintains clean separation of concerns Co-authored-by: pmarchini <[email protected]>
- Track successfully setup limiters separately - Cleanup only limiters that were successfully initialized - Cleanup in reverse order on error - Add interface name validation in Setup method Co-authored-by: pmarchini <[email protected]>
pmarchini
left a comment
There was a problem hiding this comment.
I want now to handle the incoming traffic too.
We can achieve it following this schema:
Redirect ingress → IFB → tc rules
- Add --network-max-bandwidth-ingress flag - Implement setupIngressHTB with IFB device redirection - Pattern: Redirect ingress → IFB → tc rules - Support bidirectional bandwidth control (egress + ingress) - Add comprehensive tests for ingress bandwidth - Update README with ingress examples Co-authored-by: pmarchini <[email protected]>
Implemented in commit 0584d41! The ingress traffic handling now follows the IFB pattern exactly as requested: Redirect ingress → IFB → tc rules Added
Example usage: # Ingress only
sudo giogo --network-class-id=100 --network-max-bandwidth-ingress=500k -- app
# Bidirectional (egress + ingress)
sudo giogo --network-class-id=100 --network-max-bandwidth=1m --network-max-bandwidth-ingress=500k -- appBoth egress and ingress can be limited independently for full bidirectional bandwidth control. |
|
|
||
| // Helper function to create a pointer to uint32 | ||
| func uint32Ptr(v uint32) *uint32 { | ||
| return &v | ||
| } | ||
|
|
||
| func TestNewNetworkLimiterWithBandwidth(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| init *limiter.NetworkLimiterInitializer | ||
| wantMaxBandwidth uint64 | ||
| wantErr bool | ||
| }{ | ||
| { | ||
| name: "valid bandwidth in kilobytes", | ||
| init: &limiter.NetworkLimiterInitializer{ | ||
| ClassID: "100", | ||
| MaxBandwidth: "500k", | ||
| }, | ||
| wantMaxBandwidth: 500 * 1024, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "valid bandwidth in megabytes", | ||
| init: &limiter.NetworkLimiterInitializer{ | ||
| ClassID: "100", | ||
| MaxBandwidth: "1m", | ||
| }, | ||
| wantMaxBandwidth: 1024 * 1024, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "valid bandwidth in gigabytes", | ||
| init: &limiter.NetworkLimiterInitializer{ | ||
| ClassID: "100", | ||
| MaxBandwidth: "2g", | ||
| }, | ||
| wantMaxBandwidth: 2 * 1024 * 1024 * 1024, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "invalid bandwidth", | ||
| init: &limiter.NetworkLimiterInitializer{ | ||
| ClassID: "100", | ||
| MaxBandwidth: "invalid", | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| { | ||
| name: "bandwidth without classID should work", | ||
| init: &limiter.NetworkLimiterInitializer{ | ||
| MaxBandwidth: "1m", | ||
| }, | ||
| wantMaxBandwidth: 1024 * 1024, | ||
| wantErr: false, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| netLimiter, err := limiter.NewNetworkLimiter(tt.init) | ||
| if (err != nil) != tt.wantErr { | ||
| t.Errorf("NewNetworkLimiter() error = %v, wantErr %v", err, tt.wantErr) | ||
| return | ||
| } | ||
| if tt.wantErr { | ||
| return | ||
| } | ||
|
|
||
| if netLimiter.MaxBandwidth != tt.wantMaxBandwidth { | ||
| t.Errorf("MaxBandwidth = %v, expected %v", netLimiter.MaxBandwidth, tt.wantMaxBandwidth) | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func TestNewNetworkLimiterWithIngressBandwidth(t *testing.T) { | ||
| tests := []struct { | ||
| name string | ||
| init *limiter.NetworkLimiterInitializer | ||
| wantMaxBandwidthIngress uint64 | ||
| wantErr bool | ||
| }{ | ||
| { | ||
| name: "valid ingress bandwidth", | ||
| init: &limiter.NetworkLimiterInitializer{ | ||
| ClassID: "100", | ||
| MaxBandwidthIngress: "1m", | ||
| }, | ||
| wantMaxBandwidthIngress: 1024 * 1024, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "both egress and ingress", | ||
| init: &limiter.NetworkLimiterInitializer{ | ||
| ClassID: "100", | ||
| MaxBandwidth: "500k", | ||
| MaxBandwidthIngress: "1m", | ||
| }, | ||
| wantMaxBandwidthIngress: 1024 * 1024, | ||
| wantErr: false, | ||
| }, | ||
| { | ||
| name: "invalid ingress bandwidth", | ||
| init: &limiter.NetworkLimiterInitializer{ | ||
| ClassID: "100", | ||
| MaxBandwidthIngress: "invalid", | ||
| }, | ||
| wantErr: true, | ||
| }, | ||
| } | ||
|
|
||
| for _, tt := range tests { | ||
| t.Run(tt.name, func(t *testing.T) { | ||
| netLimiter, err := limiter.NewNetworkLimiter(tt.init) | ||
| if (err != nil) != tt.wantErr { | ||
| t.Errorf("NewNetworkLimiter() error = %v, wantErr %v", err, tt.wantErr) | ||
| return | ||
| } | ||
| if tt.wantErr { | ||
| return | ||
| } | ||
|
|
||
| if netLimiter.MaxBandwidthIngress != tt.wantMaxBandwidthIngress { | ||
| t.Errorf("MaxBandwidthIngress = %v, expected %v", netLimiter.MaxBandwidthIngress, tt.wantMaxBandwidthIngress) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Formatted in commit 0af5d28. Test tables now use proper indentation and alignment.
|
|
||
| // setupHTB sets up HTB (Hierarchical Token Bucket) qdisc for bandwidth limiting | ||
| // It creates a root qdisc and a class with the specified rate limit | ||
| func setupHTB(interfaceName string, classID uint32, rateBytesPerSec uint64) error { |
There was a problem hiding this comment.
Create a separated documentation markdown file for this specific limiter in order to explain how it works (both in ingress and egress)
There was a problem hiding this comment.
Created in commit 0af5d28. Added docs/network-limiter.md with comprehensive documentation covering:
- Architecture for egress and ingress patterns
- Detailed explanations of how both work
- tc command references
- Usage examples and requirements
- Implementation details and lifecycle
- Limitations, debugging, and troubleshooting
- Format TestNewNetworkLimiterWithBandwidth and TestNewNetworkLimiterWithIngressBandwidth - Create docs/network-limiter.md with detailed documentation - Document egress and ingress traffic control patterns - Include architecture, usage examples, and implementation details - Add tc command references and troubleshooting guide Co-authored-by: pmarchini <[email protected]>
Implementation Plan for NetworkLimiter - COMPLETED ✓
network.gofile with NetworkLimiter implementationnetwork_test.gofile with comprehensive testsSummary
Successfully implemented bidirectional network bandwidth limiting with clean interface-based design, robust error handling, and comprehensive documentation:
✓ LifecycleLimiter interface - Optional Setup/Cleanup methods for limiters needing lifecycle management
✓ No special handling - Executor treats all limiters uniformly through interfaces
✓ Proper cleanup tracking - Only cleans up successfully initialized limiters
✓ Error recovery - Cleans up on setup failure in reverse order
✓ Input validation - Validates interface name before tc setup
✓ Egress bandwidth limiting - Direct HTB on main interface for outgoing traffic
✓ Ingress bandwidth limiting - IFB device redirection pattern for incoming traffic
✓ Bidirectional control - Both egress and ingress can be limited independently
✓ Formatted code - All test code properly formatted
✓ Comprehensive documentation - Detailed docs/network-limiter.md file
Documentation Highlights (docs/network-limiter.md):
Example usage:
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.