[feat] bandwidth plugin: exclude some subnets from traffic shapping

* Add the possibility to exclude some subnets from traffic shapping
  * subnets to be provided in config or runtimeconfig
  * switch from a classless qdisc to a classful qdisc

* Adapt unit tests

* Adapt integration tests

* Fix tests: some tests were skipped due to ginkgo Measure deprecation

Signed-off-by: Raphael <oOraph@users.noreply.github.com>

integration/integration_linux_test.go

@@ -17,7 +17,6 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"log"
 	"math/rand"
 	"net"
 	"os"
@@ -73,7 +72,6 @@ var _ = Describe("Basic PTP using cnitool", func() {
 				"NETCONFPATH=" + netConfPath,
 				"PATH=" + os.Getenv("PATH"),
 			})
-			log.Printf("Env PATH=%s, CNI_PATH=%s, NETCONFPATH=%s", os.Getenv("PATH"), cniPath, netConfPath)
 
 			hostNS = Namespace(fmt.Sprintf("cni-test-host-%x", rand.Int31()))
 			hostNS.Add()
@@ -88,27 +86,10 @@ var _ = Describe("Basic PTP using cnitool", func() {
 		})
 
 		basicAssertion := func(netName, expectedIPPrefix string) {
-			log.Printf("Test infos, host ns %s, container ns %s", hostNS, contNS)
-
-			o := env.runInNS(contNS, "ip", "addr")
-			log.Printf("container ns %s, network before any setup [%s]", contNS, o)
-
-			o = env.runInNS(hostNS, "ip", "addr")
-			log.Printf("host ns %s, network before any setup [%s]", hostNS, o)
-
-			log.Printf("Host ns %s, '%s add %s %s'", hostNS, cnitoolBin, netName, contNS.LongName())
-
-			// time.Sleep(1200 * time.Second)
-
 			env.runInNS(hostNS, cnitoolBin, "add", netName, contNS.LongName())
 
-			log.Printf("Cnitool bin %s ok, sleeping !!!", cnitoolBin)
-
-			// time.Sleep(600 * time.Second)
-
 			addrOutput := env.runInNS(contNS, "ip", "addr")
 
-			log.Printf("Container ns %s network after setup [%s]", contNS, addrOutput)
 			Expect(addrOutput).To(ContainSubstring(expectedIPPrefix))
 
 			env.runInNS(hostNS, cnitoolBin, "del", netName, contNS.LongName())
@@ -118,14 +99,11 @@ var _ = Describe("Basic PTP using cnitool", func() {
 			basicAssertion("basic-ptp", "10.1.2.")
 		})
 
-		log.Printf("Sleeping, check that both ns %s and %s do not exist", hostNS, contNS)
-
 		It("supports add and del with ptp + bandwidth", func() {
 			basicAssertion("chained-ptp-bandwidth", "10.9.2.")
 		})
 	})
 
-	// FIXME: measure deprecated, does not run anymore (and also, ginkgo/gomega is a non working overkill bazooka used to kill a fly)...
 	Context("when the bandwidth plugin is chained with a plugin that returns multiple adapters", func() {
 		var (
 			hostNS                                            Namespace

plugins/meta/bandwidth/bandwidth_linux_test.go

@@ -1732,7 +1732,6 @@ var _ = Describe("bandwidth test", func() {
 			})
 		})
 
-		// FIXME: all this is not run anymore... And it's the most important test in the file as it validates QoS is effective...
 		Describe(fmt.Sprintf("[%s] QoS effective", ver), func() {
 
 			Context(fmt.Sprintf("[%s] when chaining bandwidth plugin with PTP", ver), func() {
@@ -1755,10 +1754,9 @@ var _ = Describe("bandwidth test", func() {
 					rateInBytes := 1000
 					rateInBits = uint64(rateInBytes * 8)
 					burstInBits = rateInBits * 2
-					// NOTE: every test with QoS should last approximately the integer factor below in seconds but the ones with QoS take longer than expected
-					// (with both tbf or htb, it's not been introduced with the switch to htb...).
-					// TODO: clarify this, maybe there is some additionnal fragmentation due to the small mtu, or may be the client binary used does not
-					// send the amount of data it should. Iperf works ok though, when testing qos manually...
+
+					// NOTE: Traffic shapping is not that precise at low rates, would be better to use higher rates + simple time+netcat for data transfer, rather than the provided
+					// client/server bin (limited to small amount of data)
 					packetInBytes = rateInBytes * 3
 
 					var err error
@@ -1890,8 +1888,6 @@ var _ = Describe("bandwidth test", func() {
 					}
 				})
 
-				// Measure is deprecated and not run by ginkgo so we just measure time elapsed in a basic way
-				// https://onsi.github.io/ginkgo/MIGRATING_TO_V2#removed-measure
 				It("limits ingress traffic on veth device", func() {
 					var runtimeWithLimit time.Duration
 					var runtimeWithoutLimit time.Duration
@@ -1902,7 +1898,6 @@ var _ = Describe("bandwidth test", func() {
 						start := time.Now()
 						result, err := types100.GetResult(containerWithQoSRes)
 						Expect(err).NotTo(HaveOccurred())
-						log.Printf("Host ns %s, result %+v, port %d", hostNs.Path(), result, portServerWithQoS)
 						makeTCPClientInNS(hostNs.Path(), result.IPs[0].Address.IP.String(), portServerWithQoS, packetInBytes)
 						end := time.Now()
 						runtimeWithLimit = end.Sub(start)
@@ -1912,7 +1907,6 @@ var _ = Describe("bandwidth test", func() {
 					By("sending tcp traffic to the container that does not have traffic shaped", func() {
 						start := time.Now()
 						result, err := types100.GetResult(containerWithoutQoSRes)
-						log.Printf("Host ns %s, result %+v, port %d", hostNs.Path(), result, portServerWithoutQoS)
 						Expect(err).NotTo(HaveOccurred())
 						makeTCPClientInNS(hostNs.Path(), result.IPs[0].Address.IP.String(), portServerWithoutQoS, packetInBytes)
 						end := time.Now()

plugins/meta/bandwidth/bandwidth_suite_test.go

@@ -33,7 +33,7 @@ import (
 	"github.com/containernetworking/plugins/pkg/ns"
 )
 
-func TestTBF(t *testing.T) {
+func TestHTB(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "plugins/meta/bandwidth")
 }

plugins/meta/bandwidth/ifb_creator.go

@@ -173,7 +173,7 @@ func createHTB(rateInBits, burstInBits uint64, linkIndex int, excludeSubnets []s
 		return fmt.Errorf("error while creating htb default class: %s", err)
 	}
 
-	// The uncapped class for the excluded subnets (I did not know how to uncap so I capped it to very high)
+	// The uncapped class for the excluded subnets
 	// cmd = exec.Command("/usr/sbin/tc", "class", "add", "dev", interfaceName, "parent", "1:", "classid", "1:1", "htb",
 	// 	"rate", "100000000000")
 	bigRate := uint64(UncappedRate)
@@ -269,8 +269,6 @@ func createHTB(rateInBits, burstInBits uint64, linkIndex int, excludeSubnets []s
 
 		if len(keys) != cap(keys) {
 			// We need to shrink the keys capacity down to its length if we do not want additional "matchall" rules to be set in the filter
-			// Shrinking a slice down to its length seems a bit complicated...
-			// Nvm, may be I just do not have the correct way to do it :)
 			shrinkedKeys := make([]netlink.TcU32Key, len(keys), len(keys))
 			for i := 0; i < len(keys); i++ {
 				shrinkedKeys[i] = keys[i]

plugins/meta/bandwidth/main.go

@@ -40,7 +40,7 @@ const (
 // BandwidthEntry corresponds to a single entry in the bandwidth argument,
 // see CONVENTIONS.md
 type BandwidthEntry struct {
-	NonShapedSubnets []string `json:"nonShapedSubnets"` // Subnets to be excluded from traffic shaping
+	NonShapedSubnets []string `json:"nonShapedSubnets"` // Ipv4/ipv6 subnets to be excluded from traffic shaping
 	IngressRate      uint64   `json:"ingressRate"`      // Bandwidth rate in bps for traffic through container. 0 for no limit. If ingressRate is set, ingressBurst must also be set
 	IngressBurst     uint64   `json:"ingressBurst"`     // Bandwidth burst in bits for traffic through container. 0 for no limit. If ingressBurst is set, ingressRate must also be set
 
@@ -103,15 +103,12 @@ func getBandwidth(conf *PluginConf) *BandwidthEntry {
 		bw = conf.RuntimeConfig.Bandwidth
 	}
 
-	if bw.NonShapedSubnets == nil {
-		if conf.BandwidthEntry != nil {
-			bw.NonShapedSubnets = conf.BandwidthEntry.NonShapedSubnets
-		}
+	if (bw.NonShapedSubnets) == nil && (conf.BandwidthEntry != nil) {
+		bw.NonShapedSubnets = conf.BandwidthEntry.NonShapedSubnets
 	}
 
 	if bw.NonShapedSubnets == nil {
 		bw.NonShapedSubnets = make([]string, 0, 0)
-		// bw.NonShapedSubnets = append(bw.NonShapedSubnets, "10.0.0.0/8")
 	}
 
 	return bw
@@ -416,7 +413,7 @@ func checkHTB(link netlink.Link, rateInBytes uint64, bufferInBytes uint32) error
 
 		// TODO: check non shaped subnet filters
 		// if bandwidth.NonShapedSubnets {
-		// 	filters, err := netlink.FilterList(link, htb.Parent)
+		// 	filters, err := netlink.FilterList(link, htb.Handle)
 		// }
 	}