From 3653e74dc69338eea23287556279d7c9eb34358e Mon Sep 17 00:00:00 2001 From: Karol Szwaj Date: Mon, 9 Sep 2024 07:48:33 +0200 Subject: [PATCH] fix: support ptp networks with ipv4 /31 netmask and ipv6 /127 netmask (#4425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Karol Szwaj Co-authored-by: 张祖建 --- pkg/ipam/subnet_test.go | 56 ++++++++++++++++++++++++++++++++++++++ pkg/util/net.go | 44 +++++++++++++++++++++--------- pkg/util/net_test.go | 54 +++++++++++++++++++++++++++++++++++- pkg/util/validator.go | 4 +++ pkg/util/validator_test.go | 50 ++++++++++++++++++++++++++++++++++ test/unittest/util/net.go | 8 +++--- 6 files changed, 198 insertions(+), 18 deletions(-) diff --git a/pkg/ipam/subnet_test.go b/pkg/ipam/subnet_test.go index 6b183ca5367..04dbf052b82 100644 --- a/pkg/ipam/subnet_test.go +++ b/pkg/ipam/subnet_test.go @@ -208,6 +208,38 @@ func TestGetV4StaticAddress(t *testing.T) { require.Equal(t, "pod1.default", usingPod) } +func TestGetV4StaticAddressPTP(t *testing.T) { + excludeIps := []string{ + "10.0.0.0", + } + subnet, err := NewSubnet("v4Subnet", "10.0.0.0/31", excludeIps) + require.NoError(t, err) + require.NotNil(t, subnet) + // 1. pod1 has v4 ip but no mac, should get specified ip and mac + podName := "pod1.default" + nicName := "pod1.default" + var mac *string + v4 := "10.0.0.1" + v4IP, err := NewIP(v4) + require.NoError(t, err) + ip1, macStr1, err := subnet.GetStaticAddress(podName, nicName, v4IP, mac, false, true) + require.NoError(t, err) + require.Equal(t, v4, ip1.String()) + require.NotEmpty(t, macStr1) + v4IP, v6IP, m, protocol := subnet.GetPodAddress(nicName) + require.Equal(t, v4, v4IP.String()) + require.Nil(t, v6IP) + require.Equal(t, macStr1, m) + require.Equal(t, apiv1.ProtocolIPv4, protocol) + + // 2. ip is assigned to pod1, should get error + podName = "pod5.default" + v4 = "10.0.0.1" + usingPod, using := subnet.isIPAssignedToOtherPod(v4, podName) + require.True(t, using) + require.Equal(t, "pod1.default", usingPod) +} + func TestGetV6StaticAddress(t *testing.T) { excludeIps := []string{ "2001:db8::2", "2001:db8::4", "2001:db8::100", @@ -402,6 +434,30 @@ func TestGetGetV4RandomAddress(t *testing.T) { require.NotEqual(t, mac1, mac2) } +func TestGetGetV4RandomAddressPTP(t *testing.T) { + excludeIps := []string{ + "10.0.0.0", + } + subnet, err := NewSubnet("randomAddressV4Subnet1", "10.0.0.0/31", excludeIps) + require.NoError(t, err) + require.NotNil(t, subnet) + // 1. no mac, get v4 address for pod1 + podName := "pod1.default" + nicName := "pod1.default" + v4IP1, v6IP1, mac1, err := subnet.GetRandomAddress("", podName, nicName, nil, nil, false) + require.NoError(t, err) + require.NotEmpty(t, v4IP1.String()) + require.Nil(t, v6IP1) + require.NotEmpty(t, mac1) + + // 2. ip is assigned to pod1, should get error + podName = "pod5.default" + v4 := "10.0.0.1" + usingPod, using := subnet.isIPAssignedToOtherPod(v4, podName) + require.True(t, using) + require.Equal(t, "pod1.default", usingPod) +} + func TestGetGetV6RandomAddress(t *testing.T) { excludeIps := []string{ "2001:db8::2", "2001:db8::4", "2001:db8::100", diff --git a/pkg/util/net.go b/pkg/util/net.go index 947ff5e3f28..802efe79b94 100644 --- a/pkg/util/net.go +++ b/pkg/util/net.go @@ -69,51 +69,66 @@ func BigInt2Ip(ipInt *big.Int) string { func SubnetNumber(subnet string) string { _, cidr, _ := net.ParseCIDR(subnet) + maskLength, length := cidr.Mask.Size() + if maskLength+1 == length { + return "" + } return cidr.IP.String() } func SubnetBroadcast(subnet string) string { _, cidr, _ := net.ParseCIDR(subnet) - var length uint - if CheckProtocol(subnet) == kubeovnv1.ProtocolIPv4 { - length = 32 - } else { - length = 128 + maskLength, length := cidr.Mask.Size() + if maskLength+1 == length { + return "" } - maskLength, _ := cidr.Mask.Size() ipInt := IP2BigInt(cidr.IP.String()) - size := big.NewInt(0).Lsh(big.NewInt(1), length-uint(maskLength)) + size := big.NewInt(0).Lsh(big.NewInt(1), uint(length-maskLength)) size = big.NewInt(0).Sub(size, big.NewInt(1)) return BigInt2Ip(ipInt.Add(ipInt, size)) } +// FirstIP returns first usable ip address in the subnet func FirstIP(subnet string) (string, error) { _, cidr, err := net.ParseCIDR(subnet) if err != nil { return "", fmt.Errorf("%s is not a valid cidr", subnet) } + // Handle ptp network case specially + if ones, bits := cidr.Mask.Size(); ones+1 == bits { + return cidr.IP.String(), nil + } ipInt := IP2BigInt(cidr.IP.String()) return BigInt2Ip(ipInt.Add(ipInt, big.NewInt(1))), nil } +// LastIP returns last usable ip address in the subnet func LastIP(subnet string) (string, error) { _, cidr, err := net.ParseCIDR(subnet) if err != nil { return "", fmt.Errorf("%s is not a valid cidr", subnet) } - var length uint - if CheckProtocol(subnet) == kubeovnv1.ProtocolIPv4 { + var length int + proto := CheckProtocol(subnet) + if proto == kubeovnv1.ProtocolIPv4 { length = 32 } else { length = 128 } maskLength, _ := cidr.Mask.Size() ipInt := IP2BigInt(cidr.IP.String()) - size := big.NewInt(0).Lsh(big.NewInt(1), length-uint(maskLength)) - size = big.NewInt(0).Sub(size, big.NewInt(2)) + size := getCIDRSize(length, maskLength) return BigInt2Ip(ipInt.Add(ipInt, size)), nil } +func getCIDRSize(length, maskLength int) *big.Int { + size := big.NewInt(0).Lsh(big.NewInt(1), uint(length-maskLength)) + if maskLength+1 == length { + return big.NewInt(0).Sub(size, big.NewInt(1)) + } + return big.NewInt(0).Sub(size, big.NewInt(2)) +} + func CIDRContainIP(cidrStr, ipStr string) bool { cidrs := strings.Split(cidrStr, ",") ips := strings.Split(ipStr, ",") @@ -188,8 +203,11 @@ func CheckProtocol(address string) string { func AddressCount(network *net.IPNet) float64 { prefixLen, bits := network.Mask.Size() - if bits-prefixLen < 2 { - return 0 + // Special case handling for /31 and /32 subnets + if bits-prefixLen == 1 { + return 2 // /31 subnet + } else if bits-prefixLen == 0 { + return 1 // /32 subnet } return math.Pow(2, float64(bits-prefixLen)) - 2 } diff --git a/pkg/util/net_test.go b/pkg/util/net_test.go index c7580ce4cc2..bbc9fd55f16 100644 --- a/pkg/util/net_test.go +++ b/pkg/util/net_test.go @@ -205,6 +205,11 @@ func TestSubnetBroadcast(t *testing.T) { subnet: "192.128.23.1/15", expect: "192.129.255.255", }, + { + name: "v4/31", + subnet: "192.128.23.0/31", + expect: "", + }, { name: "v6", subnet: "ffff:ffff:ffff:ffff:ffff:0:ffff:ffff/96", @@ -238,6 +243,17 @@ func TestFirstIP(t *testing.T) { name: "controversy", subnet: "192.168.0.23/32", expect: "192.168.0.24", + }, + { + name: "base31netmask", + subnet: "192.168.0.23/31", + expect: "192.168.0.22", + err: "", + }, + { + name: "base31netmask", + subnet: "192.168.0.0/31", + expect: "192.168.0.0", err: "", }, { @@ -252,6 +268,12 @@ func TestFirstIP(t *testing.T) { expect: "ffff:ffff:ffff:ffff:ffff::1", err: "", }, + { + name: "v6127netmask", + subnet: "ffff:ffff:ffff:ffff:ffff:0:ffff:0/127", + expect: "ffff:ffff:ffff:ffff:ffff:0:ffff:0", + err: "", + }, } for _, c := range tests { t.Run(c.name, func(t *testing.T) { @@ -271,6 +293,12 @@ func TestLastIP(t *testing.T) { expect string err string }{ + { + name: "base31netmask", + subnet: "192.168.0.2/31", + expect: "192.168.0.3", + err: "", + }, { name: "base", subnet: "192.168.0.23/24", @@ -289,6 +317,12 @@ func TestLastIP(t *testing.T) { expect: "ffff:ffff:ffff:ffff:ffff:0:ffff:fffe", err: "", }, + { + name: "v6127netmask", + subnet: "ffff:ffff:ffff:ffff:ffff:0:ffff:0/127", + expect: "ffff:ffff:ffff:ffff:ffff:0:ffff:1", + err: "", + }, } for _, c := range tests { t.Run(c.name, func(t *testing.T) { @@ -309,6 +343,12 @@ func TestCIDRContainIP(t *testing.T) { ipStr string want bool }{ + { + name: "base", + cidrStr: "192.168.0.23/31", + ipStr: "192.168.0.23,192.168.0.22", + want: true, + }, { name: "base", cidrStr: "192.168.0.23/24", @@ -327,6 +367,12 @@ func TestCIDRContainIP(t *testing.T) { ipStr: "ffff:ffff:ffff:ffff:ffff:0:ffff:fffe", want: true, }, + { + name: "v6", + cidrStr: "ffff:ffff:ffff:ffff:ffff:0:ffff:4/127", + ipStr: "ffff:ffff:ffff:ffff:ffff:0:ffff:4,ffff:ffff:ffff:ffff:ffff:0:ffff:5", + want: true, + }, } for _, c := range tests { t.Run(c.name, func(t *testing.T) { @@ -395,7 +441,7 @@ func TestAddressCount(t *testing.T) { IP: net.ParseIP("192.168.1.0"), Mask: net.IPMask{255, 255, 255, 254}, }, - want: 0, + want: 2, }, } for _, c := range tests { @@ -671,6 +717,12 @@ func TestGetGwByCidr(t *testing.T) { want: "10.16.0.1", err: "", }, + { + name: "v4", + cidr: "10.16.0.112/31", + want: "10.16.0.112", + err: "", + }, { name: "dual", cidr: "10.16.0.112/24,ffff:ffff:ffff:ffff:ffff:0:ffff:0/96", diff --git a/pkg/util/validator.go b/pkg/util/validator.go index c5af196e2b3..b4d2d25d350 100644 --- a/pkg/util/validator.go +++ b/pkg/util/validator.go @@ -334,6 +334,10 @@ func ValidateNetworkBroadcast(cidr, ip string) error { if CheckProtocol(cidrBlock) != CheckProtocol(ipAddr) { continue } + _, network, _ := net.ParseCIDR(cidrBlock) + if AddressCount(network) == 1 { + return fmt.Errorf("subnet %s is configured with /32 netmask", cidrBlock) + } ipStr := IPToString(ipAddr) if SubnetBroadcast(cidrBlock) == ipStr { diff --git a/pkg/util/validator_test.go b/pkg/util/validator_test.go index 2d3eda1b963..4124d1b36ad 100644 --- a/pkg/util/validator_test.go +++ b/pkg/util/validator_test.go @@ -476,6 +476,50 @@ func TestValidateSubnet(t *testing.T) { }, err: "ip 10.16.1 in excludeIps is not a valid address", }, + { + name: "ValidPTPSubnet", + asubnet: kubeovnv1.Subnet{ + TypeMeta: metav1.TypeMeta{Kind: "Subnet", APIVersion: "kubeovn.io/v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "utest-ptpsubnet", + }, + Spec: kubeovnv1.SubnetSpec{ + Default: true, + Vpc: "ovn-cluster", + Protocol: kubeovnv1.ProtocolIPv4, + Namespaces: nil, + CIDRBlock: "10.16.0.0/31", + Gateway: "10.16.0.0", + ExcludeIps: []string{"10.16.0.0"}, + Provider: OvnProvider, + GatewayType: kubeovnv1.GWDistributedType, + }, + Status: kubeovnv1.SubnetStatus{}, + }, + err: "", + }, + { + name: "Invalid/32Subnet", + asubnet: kubeovnv1.Subnet{ + TypeMeta: metav1.TypeMeta{Kind: "Subnet", APIVersion: "kubeovn.io/v1"}, + ObjectMeta: metav1.ObjectMeta{ + Name: "utest-ptpsubnet", + }, + Spec: kubeovnv1.SubnetSpec{ + Default: true, + Vpc: "ovn-cluster", + Protocol: kubeovnv1.ProtocolIPv4, + Namespaces: nil, + CIDRBlock: "10.16.0.0/32", + Gateway: "10.16.0.0", + ExcludeIps: []string{"10.16.0.0"}, + Provider: OvnProvider, + GatewayType: kubeovnv1.GWDistributedType, + }, + Status: kubeovnv1.SubnetStatus{}, + }, + err: "validate gateway 10.16.0.0 for cidr 10.16.0.0/32 failed: subnet 10.16.0.0/32 is configured with /32 netmask", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -653,6 +697,12 @@ func TestValidateNetworkBroadcast(t *testing.T) { ip: "10.16.0.0", err: "10.16.0.0 is the network number ip in cidr 10.16.0.0/16", }, + { + name: "boardV4/31subnet", + cidr: "10.16.0.0/31", + ip: "", + err: "", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/test/unittest/util/net.go b/test/unittest/util/net.go index ed804e19258..e5d838907f2 100644 --- a/test/unittest/util/net.go +++ b/test/unittest/util/net.go @@ -18,8 +18,8 @@ var _ = ginkgo.Describe("[Net]", func() { {IP: net.ParseIP("10.0.0.0"), Mask: net.CIDRMask(24, 32)}, } wants := []float64{ - 0, - 0, + 1, + 2, 2, 254, } @@ -112,7 +112,7 @@ var _ = ginkgo.Describe("[Net]", func() { {"10.0.1.1", "10.0.1.101..10.0.1.105"}, {"10.0.1.1", "10.0.1.101..10.0.1.105", "10.0.1.111..10.0.1.120"}, {"10.0.1.1"}, - {}, + {"10.0.1.1"}, {}, {}, @@ -130,7 +130,7 @@ var _ = ginkgo.Describe("[Net]", func() { {"fe00::101", "fe00::1a1..fe00::1a5"}, {"fe00::101", "fe00::1a1..fe00::1a5", "fe00::1b1..fe00::1c0"}, {"fe00::101"}, - {}, + {"fe00::101"}, {}, {"10.0.1.1", "10.0.1.101..10.0.1.105"},