Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history 
Signed-off-by: graysonwu <wgrayson@vmware.com>
  • Loading branch information
@GraysonWu
GraysonWu committed Oct 24, 2023
1 parent a2c64d5 commit bca28da
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 88 deletions.
2 changes: 1 addition & 1 deletion docs/egress.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ The `bandwidth` field enables traffic shaping for an Egress, by limiting the
bandwidth for all egress traffic belonging to this Egress. `rate` specifies
the maximum transmission rate. `burst` specifies the maximum burst size when
traffic exceeds the rate. The user-provided values for `rate` and `burst` must
follow the k8s [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/) format.
follow the k8s [Quantity](https://kubernetes.io/docs/reference/kubernetes-api/common-definitions/quantity/) format,
e.g. 300k, 100M, 2G. All backend workloads selected by a rate-limited Egress share the
same bandwidth while sending egress traffic via this Egress. If these limits are exceeded,
the traffic will be dropped.
Expand Down
11 changes: 11 additions & 0 deletions docs/feature-gates.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ edit the Agent configuration in the
| `SupportBundleCollection` | Agent + Controller | `false` | Alpha | v1.10 | N/A | N/A | Yes | |
| `L7NetworkPolicy` | Agent + Controller | `false` | Alpha | v1.10 | N/A | N/A | Yes | |
| `AdminNetworkPolicy` | Controller | `false` | Alpha | v1.13 | N/A | N/A | Yes | |
| `EgressTrafficShaping` | Agent | `false` | Alpha | v1.14 | N/A | N/A | Yes | OVS meters should be supported |

## Description and Requirements of Features

Expand Down Expand Up @@ -404,3 +405,13 @@ this [document](antrea-l7-network-policy.md#prerequisites) for more information

The `AdminNetworkPolicy` API (which currently includes the AdminNetworkPolicy and BaselineAdminNetworkPolicy objects)
complements the Antrea-native policies and help cluster administrators to set security postures in a portable manner.

### EgressTrafficShaping

The `EgressTrafficShaping` feature gate of Antrea Agent enables traffic shaping of Egress, which could limit the
bandwidth for all egress traffic belonging to an Egress. Refer to this [document](egress.md#trafficshaping)

#### Requirements for this Feature

This feature leverages OVS meters to do the actual rate-limiting, therefore this feature requires OVS meters
to be supported in the datapath.
12 changes: 10 additions & 2 deletions pkg/agent/controller/egress/egress_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,8 +536,16 @@ func bandwidthToRateLimitMeter(bandwidth *crdv1b1.Bandwidth, meterID uint32) *ra
if bandwidth == nil {
return nil
}
rate, _ := resource.ParseQuantity(bandwidth.Rate)
burst, _ := resource.ParseQuantity(bandwidth.Burst)
rate, err := resource.ParseQuantity(bandwidth.Rate)
if err != nil {
klog.ErrorS(err, "Invalid bandwidth rate configured for Egress", "rate", bandwidth.Rate)
return nil
}
burst, err := resource.ParseQuantity(bandwidth.Burst)
if err != nil {
klog.ErrorS(err, "Invalid bandwidth burst size configured for Egress", "burst", bandwidth.Burst)
return nil
}
return &rateLimitMeter{
MeterID: meterID,
Rate: uint32(rate.Value() / 1000),
Expand Down
70 changes: 29 additions & 41 deletions pkg/agent/openflow/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1615,9 +1615,7 @@ func Test_client_InstallSNATMarkFlows(t *testing.T) {
ctrl := gomock.NewController(t)
m := oftest.NewMockOFEntryOperations(ctrl)
var fc *client
if tc.trafficShapingEnabled {
// Windows doesn't support OVS meter, where EgressQoSTable won't be initialized.
skipTest(t, false, true)
if tc.trafficShapingEnabled && OVSMetersAreSupported() {
fc = newFakeClient(m, true, true, config.K8sNode, config.TrafficEncapModeEncap, enableEgressTrafficShaping)
fc.featureEgress.enableEgressTrafficShaping = tc.trafficShapingEnabled
} else {
Expand Down Expand Up @@ -1682,9 +1680,7 @@ func Test_client_InstallPodSNATFlows(t *testing.T) {
m := oftest.NewMockOFEntryOperations(ctrl)

var fc *client
if tc.trafficShapingEnabled {
// Windows doesn't support OVS meter, where EgressQoSTable won't be initialized.
skipTest(t, false, true)
if tc.trafficShapingEnabled && OVSMetersAreSupported() {
fc = newFakeClient(m, true, true, config.K8sNode, config.TrafficEncapModeEncap, enableEgressTrafficShaping)
fc.featureEgress.enableEgressTrafficShaping = tc.trafficShapingEnabled
} else {
Expand All @@ -1709,7 +1705,9 @@ func Test_client_InstallPodSNATFlows(t *testing.T) {
}

func Test_client_InstallEgressQoS(t *testing.T) {
skipTest(t, false, true)
if !OVSMetersAreSupported() {
t.Skipf("Skip test because OVS meters are not supported")
}
meterID := uint32(100)
meterRate := uint32(100)
meterBurst := uint32(200)
Expand Down Expand Up @@ -1737,15 +1735,15 @@ func Test_client_InstallEgressQoS(t *testing.T) {
meterBuilder.EXPECT().Done().Return(meter).Times(1)
meter.EXPECT().Add().Return(nil).Times(1)

assert.NoError(t, fc.InstallEgressQoS(meterID, meterRate, meterBurst))
require.NoError(t, fc.InstallEgressQoS(meterID, meterRate, meterBurst))

cacheKey := fmt.Sprintf("eq%x", meterID)
fCacheI, ok := fc.featureEgress.cachedFlows.Load(cacheKey)
require.True(t, ok)
assert.ElementsMatch(t, expectedFlows, getFlowStrings(fCacheI))

meter.EXPECT().Delete().Return(nil).Times(1)
assert.NoError(t, fc.UninstallEgressQoS(meterID))
require.NoError(t, fc.UninstallEgressQoS(meterID))
_, ok = fc.featureEgress.cachedFlows.Load(cacheKey)
require.False(t, ok)
}
Expand Down Expand Up @@ -2664,23 +2662,16 @@ func Test_client_RegisterPacketInHandler(t *testing.T) {
}

func Test_client_ReplayFlows(t *testing.T) {
testClientReplayFlows(t, false)
if !runtime.IsWindowsPlatform() {
testClientReplayFlows(t, true)
}
}

func testClientReplayFlows(t *testing.T, egressTrafficShaping bool) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
m := oftest.NewMockOFEntryOperations(ctrl)

var fc *client
if egressTrafficShaping {
fc = newFakeClient(m, true, false, config.K8sNode, config.TrafficEncapModeEncap, enableTrafficControl, enableMulticast, enableMulticluster, enableEgressTrafficShaping)
} else {
fc = newFakeClient(m, true, false, config.K8sNode, config.TrafficEncapModeEncap, enableTrafficControl, enableMulticast, enableMulticluster)
egressTrafficShaping := false
clientOptions := []clientOptionsFn{enableTrafficControl, enableMulticast, enableMulticluster}
if OVSMetersAreSupported() {
egressTrafficShaping = true
clientOptions = append(clientOptions, enableEgressTrafficShaping)
}
fc := newFakeClient(m, true, false, config.K8sNode, config.TrafficEncapModeEncap, clientOptions...)
defer resetPipelines()

expectedFlows := append(pipelineDefaultFlows(egressTrafficShaping, false, true, true), egressInitFlows(true)...)
Expand Down Expand Up @@ -2830,18 +2821,23 @@ func testClientReplayFlows(t *testing.T, egressTrafficShaping bool) {
egressMeterID := uint32(1)
egressMeterRate := uint32(100)
egressMeterBurst := uint32(200)
egressReplayMeter := ovsoftest.NewMockMeter(ctrl)
meterBuilder := ovsoftest.NewMockMeterBandBuilder(ctrl)
bridge.EXPECT().NewMeter(binding.MeterIDType(egressMeterID), ofctrl.MeterBurst|ofctrl.MeterKbps).Return(egressReplayMeter).Times(1)
egressReplayMeter.EXPECT().MeterBand().Return(meterBuilder).Times(1)
meterBuilder.EXPECT().MeterType(ofctrl.MeterDrop).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Rate(egressMeterRate).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Burst(egressMeterBurst).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Done().Return(egressReplayMeter).Times(1)
expectNewMeter := func(id, rate, burst uint32, unit ofctrl.MeterFlag, isCached bool) {
meter := ovsoftest.NewMockMeter(ctrl)
meterBuilder := ovsoftest.NewMockMeterBandBuilder(ctrl)
bridge.EXPECT().NewMeter(binding.MeterIDType(id), ofctrl.MeterBurst|unit).Return(meter).Times(1)
meter.EXPECT().MeterBand().Return(meterBuilder).Times(1)
meterBuilder.EXPECT().MeterType(ofctrl.MeterDrop).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Rate(rate).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Burst(burst).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Done().Return(meter).Times(1)
if isCached {
meter.EXPECT().Reset().Times(1)
}
meter.EXPECT().Add().Return(nil).Times(1)
}
expectNewMeter(egressMeterID, egressMeterRate, egressMeterBurst, ofctrl.MeterKbps, true)
egressMeter := fc.genOFMeter(binding.MeterIDType(egressMeterID), ofctrl.MeterBurst|ofctrl.MeterKbps, egressMeterRate, egressMeterBurst)
fc.featureEgress.cachedMeter.Store(egressMeterID, egressMeter)
egressReplayMeter.EXPECT().Reset().Times(1)
egressReplayMeter.EXPECT().Add().Return(nil).Times(1)
for _, meterCfg := range []struct {
id binding.MeterIDType
rate uint32
Expand All @@ -2850,15 +2846,7 @@ func testClientReplayFlows(t *testing.T, egressTrafficShaping bool) {
{id: PacketInMeterIDTF, rate: uint32(defaultPacketInRate)},
{id: PacketInMeterIDDNS, rate: uint32(defaultPacketInRate)},
} {
meter := ovsoftest.NewMockMeter(ctrl)
meterBuilder := ovsoftest.NewMockMeterBandBuilder(ctrl)
bridge.EXPECT().NewMeter(meterCfg.id, ofctrl.MeterBurst|ofctrl.MeterPktps).Return(meter).Times(1)
meter.EXPECT().MeterBand().Return(meterBuilder).Times(1)
meterBuilder.EXPECT().MeterType(ofctrl.MeterDrop).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Rate(meterCfg.rate).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Burst(2 * meterCfg.rate).Return(meterBuilder).Times(1)
meterBuilder.EXPECT().Done().Return(meter).Times(1)
meter.EXPECT().Add().Return(nil).Times(1)
expectNewMeter(uint32(meterCfg.id), meterCfg.rate, meterCfg.rate*2, ofctrl.MeterPktps, false)
}
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/agent/openflow/packetin.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ const (
// We use OpenFlow Meter for packetIn rate limiting on OVS side.
// Meter Entry ID.
// 1-255 are reserved for Egress QoS. The Egress QoS meterID leverage the same
// number as the mark allocated to the EgressIP and Antrea limits the number of
// Egress IPs pers Node to 255, hence the reserved meter ID range is 1-255.
// value as the mark allocated to the EgressIP and Antrea limits the number of
// Egress IPs per Node to 255, hence the reserved meter ID range is 1-255.
PacketInMeterIDNP = 256
PacketInMeterIDTF = 257
PacketInMeterIDDNS = 258
Expand Down
34 changes: 17 additions & 17 deletions pkg/agent/openflow/pipeline_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -167,14 +167,14 @@ func pipelineDefaultFlows(egressTrafficShapingEnabled, externalNodeEnabled, isEn

func Test_client_defaultFlows(t *testing.T) {
testCases := []struct {
name string
enableIPv4 bool
enableIPv6 bool
nodeType config.NodeType
trafficEncapMode config.TrafficEncapModeType
clientOptions []clientOptionsFn
skipWindows bool
expectedFlows []string
name string
enableIPv4 bool
enableIPv6 bool
nodeType config.NodeType
trafficEncapMode config.TrafficEncapModeType
clientOptions []clientOptionsFn
requireMeterSupport bool
expectedFlows []string
}{
{
name: "IPv4,Encap,K8s Node",
Expand All @@ -185,13 +185,13 @@ func Test_client_defaultFlows(t *testing.T) {
expectedFlows: pipelineDefaultFlows(false, false, true, true),
},
{
name: "IPv4,Encap,K8s Node,EgressTrafficShaping",
enableIPv4: true,
nodeType: config.K8sNode,
trafficEncapMode: config.TrafficEncapModeEncap,
clientOptions: []clientOptionsFn{enableTrafficControl, enableMulticast, enableMulticluster, enableEgressTrafficShaping},
skipWindows: true,
expectedFlows: pipelineDefaultFlows(true, false, true, true),
name: "IPv4,Encap,K8s Node,EgressTrafficShaping",
enableIPv4: true,
nodeType: config.K8sNode,
trafficEncapMode: config.TrafficEncapModeEncap,
clientOptions: []clientOptionsFn{enableTrafficControl, enableMulticast, enableMulticluster, enableEgressTrafficShaping},
requireMeterSupport: true,
expectedFlows: pipelineDefaultFlows(true, false, true, true),
},
{
name: "IPv4,NoEncap,K8s Node",
Expand All @@ -218,8 +218,8 @@ func Test_client_defaultFlows(t *testing.T) {
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
if tc.skipWindows {
skipTest(t, false, true)
if tc.requireMeterSupport && !OVSMetersAreSupported() {
t.Skipf("Skip test because OVS meters are not supported")
}
ctrl := gomock.NewController(t)
m := oftest.NewMockOFEntryOperations(ctrl)
Expand Down
30 changes: 5 additions & 25 deletions test/e2e/egress_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -670,19 +670,13 @@ func testEgressUpdateBandwidth(t *testing.T, data *TestData) {
skipIfEgressShapingDisabled(t)
skipIfNotIPv4Cluster(t)
skipIfHasWindowsNodes(t)
bandwidth1 := &v1beta1.Bandwidth{
bandwidth := &v1beta1.Bandwidth{
Rate: "100M",
Burst: "200M",
}
bandwidth2 := &v1beta1.Bandwidth{
Rate: "1G",
Burst: "2G",
}
transMap := map[string]int{
"100M": 100,
"200M": 200,
"1G": 1000,
"2G": 2000,
}

egressNode := nodeName(1)
Expand All @@ -706,7 +700,7 @@ func testEgressUpdateBandwidth(t *testing.T, data *TestData) {
err = data.podWaitForRunning(defaultTimeout, clientPodName, data.testNamespace)
require.NoError(t, err, "Error when waiting for the client Pod to be in the Running state")

egress := data.createEgress(t, "egress-qos-", nil, map[string]string{"antrea-e2e": clientPodName}, "", egressNodeIP, bandwidth1)
egress := data.createEgress(t, "egress-qos-", nil, map[string]string{"antrea-e2e": clientPodName}, "", egressNodeIP, bandwidth)
_, err = data.waitForEgressRealized(egress)
require.NoError(t, err, "Error when waiting for Egress to be realized")
defer data.crdClient.CrdV1beta1().Egresses().Delete(context.TODO(), egress.Name, metav1.DeleteOptions{})
Expand All @@ -721,25 +715,11 @@ func testEgressUpdateBandwidth(t *testing.T, data *TestData) {
actualBandwidth, _ := strconv.ParseFloat(strings.TrimSpace(stdout), 64)
t.Logf("Actual bandwidth: %v Mbits/sec", actualBandwidth)
// Allow a certain deviation.
assert.InEpsilon(t, actualBandwidth, expectedBandwidth, 0.1)
assert.InEpsilon(t, actualBandwidth, expectedBandwidth, 0.2)
}

runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -t 1|grep sender|awk '{print $7}'"}, transMap[bandwidth1.Rate]+transMap[bandwidth1.Burst])
runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 1|grep sender|awk '{print $7}'"}, transMap[bandwidth1.Rate])

toUpdate := egress.DeepCopy()
err = retry.RetryOnConflict(retry.DefaultRetry, func() error {
toUpdate.Spec.Bandwidth = bandwidth2
_, err := data.crdClient.CrdV1beta1().Egresses().Update(context.TODO(), toUpdate, metav1.UpdateOptions{})
if err != nil && errors.IsConflict(err) {
toUpdate, _ = data.crdClient.CrdV1beta1().Egresses().Get(context.TODO(), egress.Name, metav1.GetOptions{})
}
return err
})
require.NoError(t, err, "Failed to update Egress")

runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -t 1|grep sender|awk '{print $7}'"}, transMap[bandwidth2.Rate]+transMap[bandwidth2.Burst])
runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 1|grep sender|awk '{print $7}'"}, transMap[bandwidth2.Rate])
runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -t 1|grep sender|awk '{print $7}'"}, transMap[bandwidth.Rate]+transMap[bandwidth.Burst])
runIperf([]string{"bash", "-c", "iperf3 -c 1.1.1.1 -f m -O 1|grep sender|awk '{print $7}'"}, transMap[bandwidth.Rate])
}

func (data *TestData) checkEgressState(egressName, expectedIP, expectedNode, otherNode string, timeout time.Duration) (*v1beta1.Egress, error) {
Expand Down

0 comments on commit bca28da

Please sign in to comment.