From 5ab87dd990f73aded7394ddce8c10fbaca962b01 Mon Sep 17 00:00:00 2001
From: Laszlo Kiraly <laszlo.kiraly@est.tech>
Date: Thu, 4 May 2023 10:24:48 +0200
Subject: [PATCH] Fix for Policy Base Routing failure; Possible table ID
 collision.

Related issue: networkservicemesh/deployments-k8s/issues/9119

Added mutex lock to protect table ID selection from parallel runs.

Signed-off-by: Laszlo Kiraly <laszlo.kiraly@est.tech>
---
 .../ipcontext/iprule/common.go                | 46 ++++++++++++-------
 .../ipcontext/iprule/server.go                | 12 +++--
 2 files changed, 37 insertions(+), 21 deletions(-)

diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go
index 13b97c5c..573ca47e 100644
--- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go
+++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go
@@ -2,6 +2,8 @@
 //
 // Copyright (c) 2023 Cisco and/or its affiliates.
 //
+// Copyright (c) 2023 Nordix Foundation.
+//
 // SPDX-License-Identifier: Apache-2.0
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -25,6 +27,7 @@ import (
 	"context"
 	"fmt"
 	"strconv"
+	"sync"
 	"time"
 
 	"golang.org/x/sys/unix"
@@ -40,7 +43,7 @@ import (
 	link "github.com/networkservicemesh/sdk-kernel/pkg/kernel"
 )
 
-func create(ctx context.Context, conn *networkservice.Connection, tableIDs *Map) error {
+func create(ctx context.Context, conn *networkservice.Connection, tableIDs *Map, tIDLock sync.Locker) error {
 	if mechanism := kernel.ToMechanism(conn.GetMechanism()); mechanism != nil && mechanism.GetVLAN() == 0 {
 		// Construct the netlink handle for the target namespace for this kernel interface
 		netlinkHandle, err := link.GetNetlinkHandle(mechanism.GetNetNSURL())
@@ -75,28 +78,37 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *Map)
 			tableIDs.Store(conn.GetId(), ps)
 		}
 		// Add new policies
+		tIDLock.Lock()
+		defer tIDLock.Unlock()
 		for _, policy := range toAdd {
-			tableID, err := getFreeTableID(ctx, netlinkHandle)
-			if err != nil {
+			if err := addPolicy(ctx, netlinkHandle, policy, l, ps, tableIDs, conn); err != nil {
 				return err
 			}
-			// If policy doesn't contain any route - add default
-			if len(policy.Routes) == 0 {
-				policy.Routes = append(policy.Routes, defaultRoute())
-			}
+		}
+	}
+	return nil
+}
 
-			for _, route := range policy.Routes {
-				if err := routeAdd(ctx, netlinkHandle, l, route, tableID); err != nil {
-					return err
-				}
-			}
-			if err := ruleAdd(ctx, netlinkHandle, policy, tableID); err != nil {
-				return err
-			}
-			ps[tableID] = policy
-			tableIDs.Store(conn.GetId(), ps)
+func addPolicy(ctx context.Context, netlinkHandle *netlink.Handle, policy *networkservice.PolicyRoute, l netlink.Link, ps policies, tableIDs *Map, conn *networkservice.Connection) error {
+	tableID, err := getFreeTableID(ctx, netlinkHandle)
+	if err != nil {
+		return err
+	}
+	// If policy doesn't contain any route - add default
+	if len(policy.Routes) == 0 {
+		policy.Routes = append(policy.Routes, defaultRoute())
+	}
+
+	for _, route := range policy.Routes {
+		if err := routeAdd(ctx, netlinkHandle, l, route, tableID); err != nil {
+			return err
 		}
 	}
+	if err := ruleAdd(ctx, netlinkHandle, policy, tableID); err != nil {
+		return err
+	}
+	ps[tableID] = policy
+	tableIDs.Store(conn.GetId(), ps)
 	return nil
 }
 
diff --git a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/server.go b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/server.go
index f63e2cc3..79b3a408 100644
--- a/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/server.go
+++ b/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/server.go
@@ -2,7 +2,7 @@
 //
 // Copyright (c) 2022 Doc.ai and/or its affiliates.
 //
-// Copyright (c) 2021-2022 Nordix Foundation.
+// Copyright (c) 2023 Nordix Foundation.
 //
 // SPDX-License-Identifier: Apache-2.0
 //
@@ -25,6 +25,7 @@ package iprule
 
 import (
 	"context"
+	"sync"
 
 	"github.com/golang/protobuf/ptypes/empty"
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
@@ -34,12 +35,15 @@ import (
 )
 
 type ipruleServer struct {
-	tables Map
+	tables      Map
+	tableIDLock sync.Locker
 }
 
 // NewServer creates a new server chain element setting ip rules
 func NewServer() networkservice.NetworkServiceServer {
-	return &ipruleServer{}
+	return &ipruleServer{
+		tableIDLock: &sync.Mutex{},
+	}
 }
 
 func (i *ipruleServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) {
@@ -55,7 +59,7 @@ func (i *ipruleServer) Request(ctx context.Context, request *networkservice.Netw
 		return nil, err
 	}
 
-	if err := create(ctx, conn, &i.tables); err != nil {
+	if err := create(ctx, conn, &i.tables, i.tableIDLock); err != nil {
 		closeCtx, cancelClose := postponeCtxFunc()
 		defer cancelClose()