1.Change the caller level from warn to debug when delete the qdisc

2.Implement a dynamic temporary cache that tracks network interfaces that do not support the ethtool operation.
3.Add new test cases for the new changings

pkg/plugin/linuxutil/ethtool_handle_linux.go

@@ -0,0 +1,50 @@
+package linuxutil
+
+import (
+	"strings"
+
+	"github.com/pkg/errors"
+
+	lru "github.com/hashicorp/golang-lru/v2"
+
+	"github.com/microsoft/retina/pkg/log"
+	"go.uber.org/zap"
+)
+
+type CachedEthtool struct {
+	EthtoolInterface
+	unsupported *lru.Cache[string, struct{}]
+	l           *log.ZapLogger
+}
+
+func NewCachedEthtool(ethHandle EthtoolInterface, opts *EthtoolOpts) *CachedEthtool {
+	cache, err := lru.New[string, struct{}](int(opts.limit))
+	if err != nil {
+		log.Logger().Error("failed to create LRU cache: ", zap.Error(err))
+	}
+
+	return &CachedEthtool{
+		EthtoolInterface: ethHandle,
+		unsupported:      cache,
+		l:                log.Logger().Named(string("EthtoolReader")),
+	}
+}
+
+var errskip = errors.New("skip interface")
+
+func (ce *CachedEthtool) Stats(intf string) (map[string]uint64, error) {
+	// Skip unsupported interfaces
+	if _, ok := ce.unsupported.Get(intf); ok {
+		return nil, errskip
+	}
+
+	ifaceStats, err := ce.EthtoolInterface.Stats(intf)
+	if err != nil {
+		if strings.Contains(err.Error(), "operation not supported") {
+			ce.unsupported.Add(intf, struct{}{})
+			return nil, errors.Wrap(err, "interface not supported while retrieving stats")
+		}
+		return nil, errors.Wrap(err, "failed to retrieve interface stats")
+	}
+	return ifaceStats, nil
+}

pkg/plugin/linuxutil/ethtool_stats_linux.go

@@ -3,6 +3,7 @@
 package linuxutil
 
 import (
+	"errors"
 	"net"
 	"strings"
 
@@ -28,11 +29,13 @@ func NewEthtoolReader(opts *EthtoolOpts, ethHandle EthtoolInterface) *EthtoolRea
 			return nil
 		}
 	}
+	// Construct a cached ethtool handle
+	CachedEthHandle := NewCachedEthtool(ethHandle, opts)
 	return &EthtoolReader{
 		l:         log.Logger().Named(string("EthtoolReader")),
 		opts:      opts,
 		data:      &EthtoolStats{},
-		ethHandle: ethHandle,
+		ethHandle: CachedEthHandle,
 	}
 }
 
@@ -48,8 +51,6 @@ func (er *EthtoolReader) readAndUpdate() error {
 }
 
 func (er *EthtoolReader) readInterfaceStats() error {
-	// ethtool section
-
 	ifaces, err := net.Interfaces()
 	if err != nil {
 		er.l.Error("Error while getting all interfaces: %v\n", zap.Error(err))
@@ -67,10 +68,15 @@ func (er *EthtoolReader) readInterfaceStats() error {
 		if strings.Contains(i.Name, "lo") || strings.Contains(i.Name, "cbr0") {
 			continue
 		}
+
 		// Retrieve tx from eth0
 		ifaceStats, err := er.ethHandle.Stats(i.Name)
 		if err != nil {
-			er.l.Error("Error while getting ethtool:", zap.String("ifacename", i.Name), zap.Error(err))
+			if errors.Is(err, errskip) {
+				er.l.Debug("Skipping unsupported interface", zap.String("ifacename", i.Name))
+			} else {
+				er.l.Error("Error while getting ethtool:", zap.String("ifacename", i.Name), zap.Error(err))
+			}
 			continue
 		}
 
@@ -79,6 +85,7 @@ func (er *EthtoolReader) readInterfaceStats() error {
 		er.data.stats[i.Name] = tempMap
 
 		er.l.Debug("Processed ethtool Stats ", zap.String("ifacename", i.Name))
+
 	}
 
 	return nil

pkg/plugin/linuxutil/ethtool_stats_linux_test.go

@@ -1,8 +1,11 @@
 package linuxutil
 
 import (
+	"errors"
 	"testing"
 
+	lru "github.com/hashicorp/golang-lru/v2"
+
 	"github.com/microsoft/retina/pkg/log"
 	"github.com/microsoft/retina/pkg/metrics"
 	"github.com/prometheus/client_golang/prometheus"
@@ -15,12 +18,18 @@ var (
 	MockCounterVec *metrics.MockICounterVec
 )
 
+var (
+	errInterfaceNotSupported = errors.New("operation not supported")
+	errOther                 = errors.New("other error")
+)
+
 func TestNewEthtool(t *testing.T) {
 	log.SetupZapLogger(log.GetDefaultLogOpts())
 
 	opts := &EthtoolOpts{
 		errOrDropKeysOnly: false,
 		addZeroVal:        false,
+		limit:             10,
 	}
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()
@@ -36,13 +45,19 @@ func TestNewEthtoolWithNil(t *testing.T) {
 	opts := &EthtoolOpts{
 		errOrDropKeysOnly: false,
 		addZeroVal:        false,
+		limit:             10,
 	}
 
 	ethReader := NewEthtoolReader(opts, nil)
 	assert.NotNil(t, ethReader)
 }
 
 func TestReadInterfaceStats(t *testing.T) {
+	globalCache, err := lru.New[string, struct{}](10)
+	if err != nil {
+		t.Fatal("failed to create LRU cache: ", err)
+	}
+
 	log.SetupZapLogger(log.GetDefaultLogOpts())
 	l := log.Logger().Named("ethtool test").Sugar()
 
@@ -59,6 +74,7 @@ func TestReadInterfaceStats(t *testing.T) {
 			opts: &EthtoolOpts{
 				errOrDropKeysOnly: false,
 				addZeroVal:        false,
+				limit:             10,
 			},
 			statsReturn: map[string]uint64{
 				"rx_packets": 1,
@@ -69,32 +85,88 @@ func TestReadInterfaceStats(t *testing.T) {
 			},
 			wantErr: false,
 		},
+		{
+			name: "test other error not added to cache",
+			opts: &EthtoolOpts{
+				errOrDropKeysOnly: false,
+				addZeroVal:        false,
+				limit:             10,
+			},
+			statsReturn: nil,
+			statErr:     errOther,
+			result:      nil,
+			wantErr:     true,
+		},
+		{
+			name: "test unsported interface",
+			opts: &EthtoolOpts{
+				errOrDropKeysOnly: false,
+				addZeroVal:        false,
+				limit:             10,
+			},
+			statsReturn: nil,
+			statErr:     errInterfaceNotSupported,
+
+			result:  nil,
+			wantErr: false,
+		},
+		{
+			name: "test skipped interface",
+			opts: &EthtoolOpts{
+				errOrDropKeysOnly: false,
+				addZeroVal:        false,
+				limit:             10,
+			},
+			statsReturn: nil,
+			statErr:     errInterfaceNotSupported,
+			result:      nil,
+			wantErr:     false,
+		},
 	}
 
 	for _, tt := range tests {
 		l.Infof("Running TestReadInterfaceStats %s", tt.name)
+
 		ctrl := gomock.NewController(t)
 		defer ctrl.Finish()
 
 		ethHandle := NewMockEthtoolInterface(ctrl)
-		ethReader := NewEthtoolReader(tt.opts, ethHandle)
+
+		cachedEthHandle := NewCachedEthtool(ethHandle, tt.opts)
+		cachedEthHandle.unsupported = globalCache
+
+		ethReader := NewEthtoolReader(tt.opts, cachedEthHandle)
+
 		assert.NotNil(t, ethReader)
 
-		ethHandle.EXPECT().Stats(gomock.Any()).Return(tt.statsReturn, nil).AnyTimes()
+		ethHandle.EXPECT().Stats(gomock.Any()).Return(tt.statsReturn, tt.statErr).AnyTimes()
 		ethHandle.EXPECT().Close().Times(1)
 		InitalizeMetricsForTesting(ctrl)
 
-		testmetric := prometheus.NewGauge(prometheus.GaugeOpts{
-			Name: "testmetric",
-			Help: "testmetric",
-		})
+		if tt.statErr == nil {
+			testmetric := prometheus.NewGauge(prometheus.GaugeOpts{
+				Name: "testmetric",
+				Help: "testmetric",
+			})
 
-		MockGaugeVec.EXPECT().WithLabelValues(gomock.Any()).Return(testmetric).AnyTimes()
+			MockGaugeVec.EXPECT().WithLabelValues(gomock.Any()).Return(testmetric).AnyTimes()
+		}
 
 		err := ethReader.readInterfaceStats()
 		assert.Nil(t, err)
 
-		ethReader.updateMetrics()
+		if tt.statErr == nil {
+			ethReader.updateMetrics()
+		}
+
+		if tt.statErr != nil && errors.Is(tt.statErr, errInterfaceNotSupported) {
+			assert.NotNil(t, cachedEthHandle.unsupported, "cache should not be nil")
+			assert.NotEqual(t, 0, cachedEthHandle.unsupported.Len(), "cache should contain interface")
+		} else if tt.statErr != nil && !errors.Is(tt.statErr, errInterfaceNotSupported) {
+			assert.Equal(t, 0, cachedEthHandle.unsupported.Len(), "cache should not add interface for other errors")
+		}
+
+		globalCache = cachedEthHandle.unsupported
 	}
 }
 

pkg/plugin/linuxutil/linuxutil_linux.go

@@ -18,6 +18,8 @@ import (
 	"go.uber.org/zap"
 )
 
+const defaultLimit = 2000
+
 // New creates a linuxutil plugin.
 func New(cfg *kcfg.Config) api.Plugin {
 	return &linuxUtil{
@@ -87,6 +89,7 @@ func (lu *linuxUtil) run(ctx context.Context) error {
 			ethtoolOpts := &EthtoolOpts{
 				errOrDropKeysOnly: false,
 				addZeroVal:        false,
+				limit:             defaultLimit,
 			}
 
 			ethHandle, err := ethtool.NewEthtool()

pkg/plugin/linuxutil/types_linux.go

@@ -102,6 +102,9 @@ type EthtoolOpts struct {
 
 	// when true will include all keys with value 0
 	addZeroVal bool
+
+	// Configurable limit for unsupported interfaces cache
+	limit uint
 }
 
 type EthtoolInterface interface {

pkg/plugin/packetparser/packetparser_linux.go

@@ -318,10 +318,10 @@ func (p *packetParser) clean(tcnl ITc, tcIngressObj *tc.Object, tcEgressObj *tc.
 	// Warning, not error. Clean is best effort.
 	if tcnl != nil {
 		if err := getQdisc(tcnl).Delete(tcEgressObj); err != nil && !errors.Is(err, tc.ErrNoArg) {
-			p.l.Warn("could not delete egress qdisc", zap.Error(err))
+			p.l.Debug("could not delete egress qdisc", zap.Error(err))
 		}
 		if err := getQdisc(tcnl).Delete(tcIngressObj); err != nil && !errors.Is(err, tc.ErrNoArg) {
-			p.l.Warn("could not delete ingress qdisc", zap.Error(err))
+			p.l.Debug("could not delete ingress qdisc", zap.Error(err))
 		}
 		if err := tcnl.Close(); err != nil {
 			p.l.Warn("could not close rtnetlink socket", zap.Error(err))