From af5c232074a1360c3e159ce66f43b2960afff2cd Mon Sep 17 00:00:00 2001
From: Artem Glazychev <artem.glazychev@xored.com>
Date: Tue, 24 Oct 2023 17:10:56 +0700
Subject: [PATCH] pinhole: fix behavior in case of creation error (#762)

Signed-off-by: Artem Glazychev <artem.glazychev@xored.com>
---
 pkg/networkservice/pinhole/client.go | 18 +++++++++++++-----
 pkg/networkservice/pinhole/server.go | 18 +++++++++++++-----
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/pkg/networkservice/pinhole/client.go b/pkg/networkservice/pinhole/client.go
index fad645d0..a4d7add4 100644
--- a/pkg/networkservice/pinhole/client.go
+++ b/pkg/networkservice/pinhole/client.go
@@ -74,20 +74,28 @@ func (v *pinholeClient) Request(ctx context.Context, request *networkservice.Net
 		if key == nil {
 			continue
 		}
-		if _, ok := v.ipPortMap.LoadOrStore(*key, struct{}{}); !ok {
+		// Check if this ACL rule has been added
+		if _, ok := v.ipPortMap.Load(*key); !ok {
+			var err error
+
 			v.mutex.Lock()
-			if err := create(ctx, v.vppConn, key.IP(), key.Port(), fmt.Sprintf("%s port %d", aclTag, key.port)); err != nil {
+			// Double check after mutex
+			if _, ok := v.ipPortMap.Load(*key); !ok {
+				if err = create(ctx, v.vppConn, key.IP(), key.Port(), fmt.Sprintf("%s port %d", aclTag, key.port)); err == nil {
+					v.ipPortMap.Store(*key, struct{}{})
+				}
+			}
+			v.mutex.Unlock()
+
+			if err != nil {
 				closeCtx, cancelClose := postponeCtxFunc()
 				defer cancelClose()
 
 				if _, closeErr := v.Close(closeCtx, conn, opts...); closeErr != nil {
 					err = errors.Wrapf(err, "connection closed with error: %s", closeErr.Error())
 				}
-
-				v.mutex.Unlock()
 				return nil, err
 			}
-			v.mutex.Unlock()
 		}
 	}
 
diff --git a/pkg/networkservice/pinhole/server.go b/pkg/networkservice/pinhole/server.go
index 939f389e..34104099 100644
--- a/pkg/networkservice/pinhole/server.go
+++ b/pkg/networkservice/pinhole/server.go
@@ -73,20 +73,28 @@ func (v *pinholeServer) Request(ctx context.Context, request *networkservice.Net
 		if key == nil {
 			continue
 		}
-		if _, ok := v.ipPortMap.LoadOrStore(*key, struct{}{}); !ok {
+		// Check if this ACL rule has been added
+		if _, ok := v.ipPortMap.Load(*key); !ok {
+			var err error
+
 			v.mutex.Lock()
-			if err := create(ctx, v.vppConn, key.IP(), key.Port(), fmt.Sprintf("%s port %d", aclTag, key.port)); err != nil {
+			// Double check after mutex
+			if _, ok := v.ipPortMap.Load(*key); !ok {
+				if err = create(ctx, v.vppConn, key.IP(), key.Port(), fmt.Sprintf("%s port %d", aclTag, key.port)); err == nil {
+					v.ipPortMap.Store(*key, struct{}{})
+				}
+			}
+			v.mutex.Unlock()
+
+			if err != nil {
 				closeCtx, cancelClose := postponeCtxFunc()
 				defer cancelClose()
 
 				if _, closeErr := v.Close(closeCtx, conn); closeErr != nil {
 					err = errors.Wrapf(err, "connection closed with error: %s", closeErr.Error())
 				}
-
-				v.mutex.Unlock()
 				return nil, err
 			}
-			v.mutex.Unlock()
 		}
 	}