From f734da77542519870cea124710612ede2c0747a2 Mon Sep 17 00:00:00 2001 From: Laszlo Kiraly <75267280+ljkiraly@users.noreply.github.com> Date: Tue, 28 Feb 2023 16:45:22 +0100 Subject: [PATCH] remote-vlan-nse fix; Singlepoint IPAM should not allocate broadcast address (#1431) Signed-off-by: Laszlo Kiraly --- .../ipam/singlepointipam/README.md | 7 +++ .../ipam/singlepointipam/server.go | 8 ++++ .../ipam/singlepointipam/server_test.go | 45 ++++++++++++++----- 3 files changed, 49 insertions(+), 11 deletions(-) create mode 100644 pkg/networkservice/ipam/singlepointipam/README.md diff --git a/pkg/networkservice/ipam/singlepointipam/README.md b/pkg/networkservice/ipam/singlepointipam/README.md new file mode 100644 index 000000000..eb85f519d --- /dev/null +++ b/pkg/networkservice/ipam/singlepointipam/README.md @@ -0,0 +1,7 @@ +# Single point IPAM + +This chain element is used to provide IPAM functionality for the nse-remote-vlan. + +Per request allocates a single IP address in the given subnet and provides static routes. Request can set some exclude IP prefixes for the allocated IP. + +The first IP address from the specified IP range and the broadcast IPv4 address will be witheld. diff --git a/pkg/networkservice/ipam/singlepointipam/server.go b/pkg/networkservice/ipam/singlepointipam/server.go index eb60a6236..45e9b7c27 100644 --- a/pkg/networkservice/ipam/singlepointipam/server.go +++ b/pkg/networkservice/ipam/singlepointipam/server.go @@ -32,6 +32,7 @@ import ( "github.com/networkservicemesh/api/pkg/api/networkservice" "github.com/networkservicemesh/sdk/pkg/networkservice/core/next" + "github.com/networkservicemesh/sdk/pkg/tools/cidr" "github.com/networkservicemesh/sdk/pkg/tools/ippool" ) @@ -78,6 +79,13 @@ func (sipam *singlePIpam) init() { mask := fmt.Sprintf("/%d", ones) sipam.masks = append(sipam.masks, mask) ipPool := ippool.NewWithNet(prefix) + if prefix.IP.To4() != nil { + // Remove the broadcast address from the pool + _, sipam.initErr = ipPool.PullIP(cidr.BroadcastAddress(prefix)) + if sipam.initErr != nil { + return + } + } sipam.ipPools = append(sipam.ipPools, ipPool) } } diff --git a/pkg/networkservice/ipam/singlepointipam/server_test.go b/pkg/networkservice/ipam/singlepointipam/server_test.go index 581a578a3..566149bdf 100644 --- a/pkg/networkservice/ipam/singlepointipam/server_test.go +++ b/pkg/networkservice/ipam/singlepointipam/server_test.go @@ -1,5 +1,5 @@ // Copyright (c) 2020-2021 Doc.ai and/or its affiliates. -// Copyright (c) 2020-2022 Nordix and its affiliates. +// Copyright (c) 2020-2023 Nordix and its affiliates. // // SPDX-License-Identifier: Apache-2.0 // @@ -201,6 +201,34 @@ func TestExclude128PrefixIPv6(t *testing.T) { validateConn(t, conn3, "fe80::1:6/112") } +func TestBroadCastIP(t *testing.T) { + _, ipNet, err := net.ParseCIDR("10.143.248.96/28") + require.NoError(t, err) + + srv := newIpamServer(ipNet) + + conn1, err := srv.Request(context.Background(), newRequest()) + require.NoError(t, err) + validateConn(t, conn1, "10.143.248.97/28") + + conn2, err := srv.Request(context.Background(), newRequest()) + require.NoError(t, err) + validateConn(t, conn2, "10.143.248.98/28") + + for i := 0; i < 11; i++ { + _, err = srv.Request(context.Background(), newRequest()) + require.NoError(t, err) + } + + conn3, err := srv.Request(context.Background(), newRequest()) + require.NoError(t, err) + validateConn(t, conn3, "10.143.248.110/28") + + // Broadcast + _, err = srv.Request(context.Background(), newRequest()) + require.EqualError(t, err, "IPPool is empty") +} + func TestOutOfIPs(t *testing.T) { _, ipNet, err := net.ParseCIDR("192.168.1.2/31") require.NoError(t, err) @@ -208,13 +236,8 @@ func TestOutOfIPs(t *testing.T) { srv := newIpamServer(ipNet) req1 := newRequest() - conn1, err := srv.Request(context.Background(), req1) - require.NoError(t, err) - validateConn(t, conn1, "192.168.1.3/31") - - req2 := newRequest() - _, err = srv.Request(context.Background(), req2) - require.Error(t, err) + _, err = srv.Request(context.Background(), req1) + require.EqualError(t, err, "IPPool is empty") } func TestOutOfIPsIPv6(t *testing.T) { @@ -230,7 +253,7 @@ func TestOutOfIPsIPv6(t *testing.T) { req2 := newRequest() _, err = srv.Request(context.Background(), req2) - require.Error(t, err) + require.EqualError(t, err, "IPPool is empty") } func TestAllIPsExcluded(t *testing.T) { @@ -243,7 +266,7 @@ func TestAllIPsExcluded(t *testing.T) { req1.Connection.Context.IpContext.ExcludedPrefixes = []string{"192.168.1.2/30"} conn1, err := srv.Request(context.Background(), req1) require.Nil(t, conn1) - require.Error(t, err) + require.EqualError(t, err, "IPPool is empty") } func TestAllIPsExcludedIPv6(t *testing.T) { @@ -256,7 +279,7 @@ func TestAllIPsExcludedIPv6(t *testing.T) { req1.Connection.Context.IpContext.ExcludedPrefixes = []string{"fe80::1:2/126"} conn1, err := srv.Request(context.Background(), req1) require.Nil(t, conn1) - require.Error(t, err) + require.EqualError(t, err, "IPPool is empty") } //nolint:dupl