From 7d53d0f203953efe1eb92436107d87305c30e6c6 Mon Sep 17 00:00:00 2001
From: Quan Tian <qtian@vmware.com>
Date: Fri, 22 Jan 2021 12:54:20 +0800
Subject: [PATCH] Normalize non standard CIDRs to fix OVS error (#1767)

K8s allows non standard CIDRs to be specified (e.g. 10.0.1.1/16,
fe80::7015:efff:fe9a:146b/64). However, OVS will report
OFPBMC_BAD_WILDCARDS error if using them in the OpenFlow messages.
antrea-agent has normalized IPv6 CIDRs before installing Openflows but
hasn't done it for IPv4 CIDRs. This patch fixes the IPv4 case.
---
 pkg/util/ip/ip.go      | 27 +++++++++---------------
 pkg/util/ip/ip_test.go | 48 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 17 deletions(-)

diff --git a/pkg/util/ip/ip.go b/pkg/util/ip/ip.go
index b25888082f6..78c16c4fd06 100644
--- a/pkg/util/ip/ip.go
+++ b/pkg/util/ip/ip.go
@@ -16,7 +16,6 @@ package ip
 
 import (
 	"bytes"
-	"fmt"
 	"net"
 	"sort"
 
@@ -129,23 +128,17 @@ func mergeCIDRs(cidrBlocks []*net.IPNet) []*net.IPNet {
 	return cidrBlocks
 }
 
-// Function to transform Antrea IPNet to net.IPNet
+// IPNetToNetIPNet converts Antrea IPNet to *net.IPNet.
+// Note that K8s allows non-standard CIDRs to be specified (e.g. 10.0.1.1/16, fe80::7015:efff:fe9a:146b/64). However,
+// OVS will report OFPBMC_BAD_WILDCARDS error if using them in the OpenFlow messages. The function will normalize the
+// CIDR if it's non-standard.
 func IPNetToNetIPNet(ipNet *v1beta2.IPNet) *net.IPNet {
 	ip := net.IP(ipNet.IP)
-	var bits int
-	if ip.To4() != nil {
-		bits = v4BitLen
-		return &net.IPNet{IP: ip, Mask: net.CIDRMask(int(ipNet.PrefixLength), bits)}
-	} else {
-		// Since OVS will report error when using an IPv6 CIDR in the OpenFlow messages if the bit is not zero in the
-		// address but it is zero in the mask. Here using the function in "net" library to ensure the result is valid.
-		_, ipNet, _ := net.ParseCIDR(fmt.Sprintf("%s/%d", ip.String(), ipNet.PrefixLength))
-		return ipNet
+	ipLen := net.IPv4len
+	if ip.To4() == nil {
+		ipLen = net.IPv6len
 	}
-}
-
-// NetIPNetToIPNet transforms net.IPNet to Antrea IPNet
-func NetIPNetToIPNet(ipNet *net.IPNet) *v1beta2.IPNet {
-	prefix, _ := ipNet.Mask.Size()
-	return &v1beta2.IPNet{IP: v1beta2.IPAddress(ipNet.IP), PrefixLength: int32(prefix)}
+	mask := net.CIDRMask(int(ipNet.PrefixLength), 8*ipLen)
+	maskedIP := ip.Mask(mask)
+	return &net.IPNet{IP: maskedIP, Mask: mask}
 }
diff --git a/pkg/util/ip/ip_test.go b/pkg/util/ip/ip_test.go
index 20412de0d89..3bff1a0a4da 100644
--- a/pkg/util/ip/ip_test.go
+++ b/pkg/util/ip/ip_test.go
@@ -19,6 +19,8 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+
+	"github.com/vmware-tanzu/antrea/pkg/apis/controlplane/v1beta2"
 )
 
 func newCIDR(cidrStr string) *net.IPNet {
@@ -108,3 +110,49 @@ func TestMergeCIDRs(t *testing.T) {
 	ipNetList4 = mergeCIDRs(ipNetList4)
 	assert.ElementsMatch(t, correctList4, ipNetList4)
 }
+
+func TestIPNetToNetIPNet(t *testing.T) {
+	tests := []struct {
+		name  string
+		ipNet *v1beta2.IPNet
+		want  *net.IPNet
+	}{
+		{
+			name: "valid IPv4 CIDR",
+			ipNet: &v1beta2.IPNet{
+				IP:           v1beta2.IPAddress(net.ParseIP("10.10.0.0")),
+				PrefixLength: 16,
+			},
+			want: newCIDR("10.10.0.0/16"),
+		},
+		{
+			name: "valid IPv6 CIDR",
+			ipNet: &v1beta2.IPNet{
+				IP:           v1beta2.IPAddress(net.ParseIP("2001:ab03:cd04:55ef::")),
+				PrefixLength: 64,
+			},
+			want: newCIDR("2001:ab03:cd04:55ef::/64"),
+		},
+		{
+			name: "non standard IPv4 CIDR",
+			ipNet: &v1beta2.IPNet{
+				IP:           v1beta2.IPAddress(net.ParseIP("10.10.10.10")),
+				PrefixLength: 16,
+			},
+			want: newCIDR("10.10.0.0/16"),
+		},
+		{
+			name: "non standard IPv6 CIDR",
+			ipNet: &v1beta2.IPNet{
+				IP:           v1beta2.IPAddress(net.ParseIP("fe80::7015:efff:fe9a:146b")),
+				PrefixLength: 64,
+			},
+			want: newCIDR("fe80::/64"),
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, IPNetToNetIPNet(tt.ipNet))
+		})
+	}
+}