Fix veth-pair creation (#820)

Signed-off-by: Artem Glazychev <[email protected]>

go.mod

@@ -15,7 +15,6 @@ require (
 	github.com/networkservicemesh/sdk-kernel v0.0.0-20240405103539-cf3b1676a8b2
 	github.com/pkg/errors v0.9.1
 	github.com/stretchr/testify v1.8.4
-	github.com/thanhpk/randstr v1.0.4
 	github.com/vishvananda/netlink v1.2.1-beta.2.0.20220630165224-c591ada0fb2b
 	github.com/vishvananda/netns v0.0.0-20211101163701-50045581ed74
 	go.fd.io/govpp v0.10.0-alpha.0.20240110141843-761adec77524

go.sum

@@ -155,8 +155,6 @@ github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcU
 github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo=
 github.com/tchap/go-patricia/v2 v2.3.1 h1:6rQp39lgIYZ+MHmdEq4xzuk1t7OdC35z/xm0BGhTkes=
 github.com/tchap/go-patricia/v2 v2.3.1/go.mod h1:VZRHKAb53DLaG+nA9EaYYiaEx6YztwDlLElMsnSHD4k=
-github.com/thanhpk/randstr v1.0.4 h1:IN78qu/bR+My+gHCvMEXhR/i5oriVHcTB/BJJIRTsNo=
-github.com/thanhpk/randstr v1.0.4/go.mod h1:M/H2P1eNLZzlDwAzpkkkUvoyNNMbzRGhESZuEQk3r0U=
 github.com/vishvananda/netlink v1.2.1-beta.2.0.20220630165224-c591ada0fb2b h1:CyMWBGvc1ZOvUBxW51DVTSIIAeJWWJJs+Ko3ouM/AVI=
 github.com/vishvananda/netlink v1.2.1-beta.2.0.20220630165224-c591ada0fb2b/go.mod h1:twkDnbuQxJYemMlGd4JFIcuhgX83tXhKS2B/PRMpOho=
 github.com/vishvananda/netns v0.0.0-20200728191858-db3c7e526aae/go.mod h1:DD4vA1DwXk04H54A1oHXtwZmA0grkVMdPxx/VGLCah0=

pkg/networkservice/mechanisms/kernel/kernelvethpair/common.go

@@ -1,4 +1,4 @@
-// Copyright (c) 2020-2023 Cisco and/or its affiliates.
+// Copyright (c) 2020-2024 Cisco and/or its affiliates.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -25,7 +25,6 @@ import (
 	"time"
 
 	"github.com/pkg/errors"
-	"github.com/thanhpk/randstr"
 	"github.com/vishvananda/netlink"
 
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
@@ -36,8 +35,9 @@ import (
 	"github.com/networkservicemesh/sdk-kernel/pkg/kernel/tools/peer"
 	"github.com/networkservicemesh/sdk/pkg/tools/log"
 
+	"github.com/networkservicemesh/sdk/pkg/tools/nanoid"
+
 	"github.com/networkservicemesh/sdk-vpp/pkg/tools/ethtool"
-	"github.com/networkservicemesh/sdk-vpp/pkg/tools/ifindex"
 	"github.com/networkservicemesh/sdk-vpp/pkg/tools/link"
 	"github.com/networkservicemesh/sdk-vpp/pkg/tools/mechutils"
 )
@@ -52,14 +52,17 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool)
 		defer handle.Close()
 
 		if _, ok := link.Load(ctx, isClient); ok {
-			if _, err = handle.LinkByName(mechanism.GetInterfaceName()); err == nil {
-				return nil
-			}
+			return nil
 		}
 
-		// Delete the previous kernel interface if there is one in the target namespace
+		// In the forwarder context:
+		// link is on the NSC/NSE side, peer is on the forwarder side.
+		linkAlias := mechutils.ToAlias(conn, isClient)
+		peerAlias := fmt.Sprintf("veth-%s", linkAlias)
+
+		// Delete the previous link if there is one in the target namespace
 		var prevLink netlink.Link
-		if prevLink, err = handle.LinkByName(mechanism.GetInterfaceName()); err == nil {
+		if prevLink, err = handle.LinkByAlias(linkAlias); err == nil {
 			now := time.Now()
 			if err = handle.LinkDel(prevLink); err != nil {
 				return errors.Wrapf(err, "failed to delete link device %v", prevLink)
@@ -69,18 +72,24 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool)
 				WithField("duration", time.Since(now)).
 				WithField("netlink", "LinkDel").Debug("completed")
 		}
-		ifindex.Delete(ctx, isClient)
 
-		alias := mechutils.ToAlias(conn, isClient)
+		// Create the veth pair
 		la := netlink.NewLinkAttrs()
-		la.Name = randstr.Hex(7)
+		la.Name, err = nanoid.GenerateLinuxInterfaceName()
+		if err != nil {
+			return err
+		}
+		peerName, err := nanoid.GenerateLinuxInterfaceName()
+		if err != nil {
+			return err
+		}
 
-		// Create the veth pair
 		now := time.Now()
 		veth := &netlink.Veth{
 			LinkAttrs: la,
-			PeerName:  linuxIfaceName(alias),
+			PeerName:  peerName,
 		}
+
 		var l netlink.Link = veth
 		if addErr := netlink.LinkAdd(l); addErr != nil {
 			return errors.Wrapf(addErr, "failed to add new link device %v", l)
@@ -150,12 +159,12 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool)
 
 		// Set the Link Alias
 		now = time.Now()
-		if err = handle.LinkSetAlias(l, alias); err != nil {
-			return errors.Wrapf(err, "failed to set the alias(%s) of the link device(%v)", alias, l)
+		if err = handle.LinkSetAlias(l, linkAlias); err != nil {
+			return errors.Wrapf(err, "failed to set the alias(%s) of the link device(%v)", linkAlias, l)
 		}
 		log.FromContext(ctx).
 			WithField("link.Name", l.Attrs().Name).
-			WithField("alias", alias).
+			WithField("alias", linkAlias).
 			WithField("duration", time.Since(now)).
 			WithField("netlink", "LinkSetAlias").Debug("completed")
 
@@ -187,14 +196,14 @@ func create(ctx context.Context, conn *networkservice.Connection, isClient bool)
 
 		// Set Alias of peerLink
 		now = time.Now()
-		if err = netlink.LinkSetAlias(peerLink, fmt.Sprintf("veth-%s", alias)); err != nil {
+		if err = netlink.LinkSetAlias(peerLink, peerAlias); err != nil {
 			_ = netlink.LinkDel(l)
 			_ = netlink.LinkDel(peerLink)
-			return errors.Wrapf(err, "failed to set the alias(%s) of the link device(%v)", fmt.Sprintf("veth-%s", alias), peerLink)
+			return errors.Wrapf(err, "failed to set the alias(%s) of the link device(%v)", peerAlias, peerLink)
 		}
 		log.FromContext(ctx).
 			WithField("link.Name", peerLink.Attrs().Name).
-			WithField("peerLink", fmt.Sprintf("veth-%s", alias)).
+			WithField("peerLink", peerAlias).
 			WithField("duration", time.Since(now)).
 			WithField("netlink", "LinkSetAlias").Debug("completed")
 
@@ -235,10 +244,3 @@ func del(ctx context.Context, conn *networkservice.Connection, isClient bool) er
 	}
 	return nil
 }
-
-func linuxIfaceName(ifaceName string) string {
-	if len(ifaceName) <= kernel.LinuxIfMaxLength {
-		return ifaceName
-	}
-	return ifaceName[:kernel.LinuxIfMaxLength]
-}

pkg/tools/mechutils/mechutils.go

@@ -1,6 +1,6 @@
 // Copyright (c) 2021-2022 Nordix Foundation.
 //
-// Copyright (c) 2020-2023 Cisco and/or its affiliates.
+// Copyright (c) 2020-2024 Cisco and/or its affiliates.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -50,15 +50,13 @@ func ToNSFilename(mechanism *kernel.Mechanism) (string, error) {
 //
 //	Note: Don't use this in a non-forwarder context
 func ToAlias(conn *networkservice.Connection, isClient bool) string {
-	// Naming is tricky.  We want to name based on either the next or prev connection id depending on whether we
-	// are on the client or server side.  Since this chain element is designed for use in a Forwarder,
-	// if we are on the client side, we want to name based on the connection id from the NSE that is Next
-	// if we are not the client, we want to name for the connection of of the client addressing us, which is Prev
+	// Naming is tricky. For the same connection, the aliases must always be the same.
+	// For consistency when using healing when there are multiple restarts, we can only rely on the first PathSegment.
 	namingConn := conn.Clone()
-	namingConn.Id = namingConn.GetPrevPathSegment().GetId()
+	namingConn.Id = namingConn.GetPath().GetPathSegments()[0].GetId()
 	alias := fmt.Sprintf("server-%s", namingConn.GetId())
 	if isClient {
-		namingConn.Id = namingConn.GetNextPathSegment().GetId()
+		namingConn.Id = namingConn.GetPath().GetPathSegments()[0].GetId()
 		alias = fmt.Sprintf("client-%s", namingConn.GetId())
 	}
 	return alias