fix golint errors

Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@est.tech>
networkservicemesh · May 31, 2021 · 994e27d · 994e27d
1 parent 8ae1a4a
commit 994e27d
Show file tree

Hide file tree

Showing 14 changed files with 154 additions and 114 deletions.
diff --git a/pkg/networkservice/chains/xconnectns/server.go b/pkg/networkservice/chains/xconnectns/server.go
@@ -14,6 +14,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package xconnectns provides interpose endpoint implementation for ovs forwarder
+// which provides kernel and smartnic endpoints
 package xconnectns
 
 import (
@@ -41,16 +43,13 @@ import (
 	"github.com/networkservicemesh/sdk/pkg/networkservice/common/mechanismtranslation"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/chain"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/metadata"
-	"github.com/networkservicemesh/sdk/pkg/tools/log"
 	"github.com/networkservicemesh/sdk/pkg/tools/token"
-	ovsutil "github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 	"google.golang.org/grpc"
-	kexec "k8s.io/utils/exec"
 
 	"github.com/networkservicemesh/sdk-ovs/pkg/networkservice/l2ovsconnect"
 	"github.com/networkservicemesh/sdk-ovs/pkg/networkservice/mechanisms/kernel"
 	"github.com/networkservicemesh/sdk-ovs/pkg/networkservice/mechanisms/vxlan"
-	"github.com/networkservicemesh/sdk-ovs/pkg/tools/util"
+	"github.com/networkservicemesh/sdk-ovs/pkg/tools/utils"
 )
 
 type ovsConnectNSServer struct {
@@ -59,14 +58,15 @@ type ovsConnectNSServer struct {
 
 // NewSriovServer - returns sriov implementation of the ovsconnectns network service
 func NewSriovServer(ctx context.Context, name string, authzServer networkservice.NetworkServiceServer,
-	tokenGenerator token.GeneratorFunc, clientURL *url.URL, bridgeName string, tunnelIpCidr net.IP,
+	tokenGenerator token.GeneratorFunc, clientURL *url.URL, bridgeName string, tunnelIPCidr net.IP,
 	pciPool resourcepool.PCIPool, resourcePool resourcepool.ResourcePool, sriovConfig *config.Config,
 	clientDialOptions ...grpc.DialOption) (endpoint.Endpoint, error) {
 	resourceLock := &sync.Mutex{}
-	tunnelIP, err := util.ParseTunnelIP(tunnelIpCidr)
+	tunnelIP, err := utils.ParseTunnelIP(tunnelIPCidr)
 	if err != nil {
 		return nil, err
 	}
+	utils.ConfigureOvS(ctx, bridgeName)
 	vxlanInterfacesMutex := &sync.Mutex{}
 	vxlanInterfaces := make(map[string]int)
 	rv := &ovsConnectNSServer{}
@@ -85,11 +85,11 @@ func NewSriovServer(ctx context.Context, name string, authzServer networkservice
 						mechanismtranslation.NewClient(),
 						connectioncontextkernel.NewClient(),
 						// TODO: uncomment once inject chain element has NewClient
-						//inject.NewClient(),
+						// inject.NewClient(),
 						// mechanisms
 						kernel.NewClient(bridgeName),
 						// TODO: uncomment once resourcepool chain element has NewClient
-						//resourcepool.NewClient(sriov.KernelDriver, resourceLock, pciPool, resourcePool, sriovConfig),
+						// resourcepool.NewClient(sriov.KernelDriver, resourceLock, pciPool, resourcePool, sriovConfig),
 						vxlan.NewClient(tunnelIP, bridgeName, vxlanInterfacesMutex, vxlanInterfaces),
 						recvfd.NewClient(),
 						sendfd.NewClient(),
@@ -122,12 +122,13 @@ func NewSriovServer(ctx context.Context, name string, authzServer networkservice
 
 // NewKernelServer - returns kernel implementation of the ovsconnectns network service
 func NewKernelServer(ctx context.Context, name string, authzServer networkservice.NetworkServiceServer,
-	tokenGenerator token.GeneratorFunc, clientURL *url.URL, bridgeName string, tunnelIpCidr net.IP,
+	tokenGenerator token.GeneratorFunc, clientURL *url.URL, bridgeName string, tunnelIPCidr net.IP,
 	clientDialOptions ...grpc.DialOption) (endpoint.Endpoint, error) {
-	tunnelIP, err := util.ParseTunnelIP(tunnelIpCidr)
+	tunnelIP, err := utils.ParseTunnelIP(tunnelIPCidr)
 	if err != nil {
 		return nil, err
 	}
+	utils.ConfigureOvS(ctx, bridgeName)
 	vxlanInterfacesMutex := &sync.Mutex{}
 	vxlanInterfaces := make(map[string]int)
 	rv := &ovsConnectNSServer{}
@@ -146,7 +147,7 @@ func NewKernelServer(ctx context.Context, name string, authzServer networkservic
 						mechanismtranslation.NewClient(),
 						connectioncontextkernel.NewClient(),
 						// TODO: uncomment once inject chain element has NewClient
-						//inject.NewClient(),
+						// inject.NewClient(),
 						// mechanisms
 						kernel.NewClient(bridgeName),
 						vxlan.NewClient(tunnelIP, bridgeName, vxlanInterfacesMutex, vxlanInterfaces),
@@ -177,24 +178,3 @@ func NewKernelServer(ctx context.Context, name string, authzServer networkservic
 
 	return rv, nil
 }
-
-func configureOvS(ctx context.Context, bridgeName string) {
-	// Initialize the ovs utility wrapper.
-	exec := kexec.New()
-	if err := ovsutil.SetExec(exec); err != nil {
-		log.FromContext(ctx).Warnf("failed to initialize ovs exec helper: %v", err)
-	}
-
-	// Create ovs bridge for client and endpoint connections
-	stdout, stderr, err := ovsutil.RunOVSVsctl("--", "--may-exist", "add-br", bridgeName)
-	if err != nil {
-		log.FromContext(ctx).Warnf("Failed to add bridge %s, stdout: %q, stderr: %q, error: %v", bridgeName, stdout, stderr, err)
-	}
-
-	// Clean the flows from the above created ovs bridge
-	stdout, stderr, err = ovsutil.RunOVSOfctl("del-flows", bridgeName)
-	if err != nil {
-		log.FromContext(ctx).Warnf("Failed to cleanup flows on %s "+
-			"stdout: %q, stderr: %q, error: %v", bridgeName, stdout, stderr, err)
-	}
-}
diff --git a/pkg/networkservice/l2ovsconnect/local.go b/pkg/networkservice/l2ovsconnect/local.go
@@ -17,7 +17,6 @@
 package l2ovsconnect
 
 import (
-	"context"
 	"fmt"
 
 	"github.com/networkservicemesh/sdk/pkg/tools/log"
@@ -26,7 +25,7 @@ import (
 	"github.com/networkservicemesh/sdk-ovs/pkg/tools/ifnames"
 )
 
-func createLocalCrossConnect(ctx context.Context, logger log.Logger, bridgeName string, endpointOvsPortInfo,
+func createLocalCrossConnect(logger log.Logger, bridgeName string, endpointOvsPortInfo,
 	clientOvsPortInfo ifnames.OvsPortInfo) error {
 	stdout, stderr, err := util.RunOVSOfctl("add-flow", bridgeName, fmt.Sprintf("priority=100, in_port=%d,"+
 		" actions=output:%d", endpointOvsPortInfo.PortNo, clientOvsPortInfo.PortNo))
@@ -55,7 +54,7 @@ func createLocalCrossConnect(ctx context.Context, logger log.Logger, bridgeName
 	return nil
 }
 
-func deleteLocalCrossConnect(ctx context.Context, logger log.Logger, bridgeName string, endpointOvsPortInfo,
+func deleteLocalCrossConnect(logger log.Logger, bridgeName string, endpointOvsPortInfo,
 	clientOvsPortInfo ifnames.OvsPortInfo) error {
 	stdout, stderr, err := util.RunOVSOfctl("del-flows", bridgeName, fmt.Sprintf("in_port=%d",
 		endpointOvsPortInfo.PortNo))

diff --git a/pkg/networkservice/l2ovsconnect/remote.go b/pkg/networkservice/l2ovsconnect/remote.go
@@ -17,7 +17,6 @@
 package l2ovsconnect
 
 import (
-	"context"
 	"fmt"
 
 	"github.com/networkservicemesh/sdk/pkg/tools/log"
@@ -26,8 +25,7 @@ import (
 	"github.com/networkservicemesh/sdk-ovs/pkg/tools/ifnames"
 )
 
-func createRemoteCrossConnect(ctx context.Context, logger log.Logger, bridgeName string, endpointOvsPortInfo,
-	clientOvsPortInfo ifnames.OvsPortInfo) error {
+func createRemoteCrossConnect(logger log.Logger, bridgeName string, endpointOvsPortInfo, clientOvsPortInfo ifnames.OvsPortInfo) error {
 	var (
 		ovsLocalPortNum, ovsTunnelPortNum int
 		ovsLocalPort, ovsTunnelPort       string
@@ -76,8 +74,7 @@ func createRemoteCrossConnect(ctx context.Context, logger log.Logger, bridgeName
 	return nil
 }
 
-func deleteRemoteCrossConnect(ctx context.Context, logger log.Logger, bridgeName string, endpointOvsPortInfo,
-	clientOvsPortInfo ifnames.OvsPortInfo) error {
+func deleteRemoteCrossConnect(logger log.Logger, bridgeName string, endpointOvsPortInfo, clientOvsPortInfo ifnames.OvsPortInfo) error {
 	var (
 		ovsLocalPortNum, ovsTunnelPortNum int
 		ovsLocalPort, ovsTunnelPort       string

diff --git a/pkg/networkservice/l2ovsconnect/server.go b/pkg/networkservice/l2ovsconnect/server.go
@@ -14,6 +14,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package l2ovsconnect chain element which cross connects both client and endpoint.
+// This suppports both local and remote (vxlan) cross connections.
 package l2ovsconnect
 
 import (
@@ -31,6 +33,7 @@ type l2ConnectServer struct {
 	bridgeName string
 }
 
+// NewServer creates l2 connect server
 func NewServer(bridgeName string) networkservice.NetworkServiceServer {
 	return &l2ConnectServer{bridgeName}
 }
@@ -67,15 +70,12 @@ func addDel(ctx context.Context, logger log.Logger, bridgeName string, addDel bo
 	}
 	if !endpointOvsPortInfo.IsTunnelPort && !clientOvsPortInfo.IsTunnelPort {
 		if addDel {
-			return createLocalCrossConnect(ctx, logger, bridgeName, endpointOvsPortInfo, clientOvsPortInfo)
-		} else {
-			return deleteLocalCrossConnect(ctx, logger, bridgeName, endpointOvsPortInfo, clientOvsPortInfo)
-		}
-	} else {
-		if addDel {
-			return createRemoteCrossConnect(ctx, logger, bridgeName, endpointOvsPortInfo, clientOvsPortInfo)
-		} else {
-			return deleteRemoteCrossConnect(ctx, logger, bridgeName, endpointOvsPortInfo, clientOvsPortInfo)
+			return createLocalCrossConnect(logger, bridgeName, endpointOvsPortInfo, clientOvsPortInfo)
 		}
+		return deleteLocalCrossConnect(logger, bridgeName, endpointOvsPortInfo, clientOvsPortInfo)
+	}
+	if addDel {
+		return createRemoteCrossConnect(logger, bridgeName, endpointOvsPortInfo, clientOvsPortInfo)
 	}
+	return deleteRemoteCrossConnect(logger, bridgeName, endpointOvsPortInfo, clientOvsPortInfo)
 }
diff --git a/pkg/networkservice/mechanisms/kernel/client.go b/pkg/networkservice/mechanisms/kernel/client.go
@@ -14,6 +14,8 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package kernel implements client and server kernel mechanism chain element supports
+// both kernel and smartvf datapath
 package kernel
 
 import (
@@ -23,18 +25,20 @@ import (
 	"github.com/networkservicemesh/api/pkg/api/networkservice"
 	"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/cls"
 	"github.com/networkservicemesh/api/pkg/api/networkservice/mechanisms/kernel"
-	"github.com/networkservicemesh/sdk-ovs/pkg/tools/ifnames"
 	"github.com/networkservicemesh/sdk-sriov/pkg/networkservice/common/resourcepool"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/core/next"
 	"github.com/networkservicemesh/sdk/pkg/networkservice/utils/metadata"
 	"github.com/networkservicemesh/sdk/pkg/tools/log"
 	"google.golang.org/grpc"
+
+	"github.com/networkservicemesh/sdk-ovs/pkg/tools/ifnames"
 )
 
 type kernelClient struct {
 	bridgeName string
 }
 
+// NewClient returns a client chain element implementing kernel mechanism with veth pair or smartvf
 func NewClient(bridgeName string) networkservice.NetworkServiceClient {
 	return &kernelClient{bridgeName}
 }
@@ -72,7 +76,7 @@ func (c *kernelClient) Close(ctx context.Context, conn *networkservice.Connectio
 	ovsPortInfo, exists := ifnames.Load(ctx, true)
 	if exists {
 		if !ovsPortInfo.IsVfRepresentor {
-			if err := resetVeth(ctx, logger, conn, c.bridgeName, metadata.IsClient(c)); err != nil {
+			if err := resetVeth(logger, conn, c.bridgeName, metadata.IsClient(c)); err != nil {
 				return nil, err
 			}
 		} else {

diff --git a/pkg/networkservice/mechanisms/kernel/common.go b/pkg/networkservice/mechanisms/kernel/common.go
@@ -28,7 +28,7 @@ import (
 	"github.com/vishvananda/netlink"
 
 	"github.com/networkservicemesh/sdk-ovs/pkg/tools/ifnames"
-	ovsutil "github.com/networkservicemesh/sdk-ovs/pkg/tools/util"
+	ovsutil "github.com/networkservicemesh/sdk-ovs/pkg/tools/utils"
 )
 
 const (
@@ -68,7 +68,7 @@ func setupVeth(ctx context.Context, logger log.Logger, conn *networkservice.Conn
 	return nil
 }
 
-func resetVeth(ctx context.Context, logger log.Logger, conn *networkservice.Connection, bridgeName string, isClient bool) error {
+func resetVeth(logger log.Logger, conn *networkservice.Connection, bridgeName string, isClient bool) error {
 	ifaceName := GetOvsInterfaceName(conn, isClient)
 	/* delete the port from ovs bridge */
 	stdout, stderr, err := util.RunOVSVsctl("del-port", bridgeName, ifaceName)
@@ -127,6 +127,8 @@ func newVETH(srcName, dstName string) *netlink.Veth {
 	}
 }
 
+// GetInterfaceName get kernel interface name for the given connection.
+// TODO: consume from sdk once this method is moved
 func GetInterfaceName(conn *networkservice.Connection, isClient bool) string {
 	namingConn := conn.Clone()
 	namingConn.Id = namingConn.GetPrevPathSegment().GetId()
@@ -136,6 +138,7 @@ func GetInterfaceName(conn *networkservice.Connection, isClient bool) string {
 	return kernel.ToMechanism(conn.GetMechanism()).GetInterfaceName(namingConn)
 }
 
+// GetOvsInterfaceName get ovs interface name for the given connection.
 func GetOvsInterfaceName(conn *networkservice.Connection, isClient bool) string {
 	namingConn := conn.Clone()
 	prefix := srcPrefix

diff --git a/pkg/networkservice/mechanisms/kernel/server.go b/pkg/networkservice/mechanisms/kernel/server.go
@@ -32,24 +32,25 @@ type kernelServer struct {
 	bridgeName string
 }
 
-// NewServer - return a new Server chain element implementing the kernel mechanism with vpp using a veth pair
+// NewServer - return a new Server chain element implementing the kernel mechanism with veth pair or smartvf
 func NewServer(bridgeName string) networkservice.NetworkServiceServer {
 	return &kernelServer{bridgeName}
-
 }
 
+// NewClient create a kernel server chain element which would be useful to do network plumbing
+// for service client container
 func (k *kernelServer) Request(ctx context.Context, request *networkservice.NetworkServiceRequest) (*networkservice.Connection, error) {
 	logger := log.FromContext(ctx).WithField("kernelServer", "Request")
 	_, exists := request.GetConnection().GetMechanism().GetParameters()[resourcepool.TokenIDKey]
 	if !exists {
 		if err := setupVeth(ctx, logger, request.Connection, k.bridgeName, false); err != nil {
-			_ = resetVeth(ctx, logger, request.Connection, k.bridgeName, false)
+			_ = resetVeth(logger, request.Connection, k.bridgeName, false)
 			return nil, err
 		}
 	}
 	conn, err := next.Server(ctx).Request(ctx, request)
 	if err != nil && err.Error() != "no token ID provided" && !exists {
-		_ = resetVeth(ctx, logger, request.Connection, k.bridgeName, false)
+		_ = resetVeth(logger, request.Connection, k.bridgeName, false)
 		return nil, err
 	}
 	if exists {
@@ -65,7 +66,7 @@ func (k *kernelServer) Close(ctx context.Context, conn *networkservice.Connectio
 	ovsPortInfo, exists := ifnames.Load(ctx, true)
 	if exists {
 		if !ovsPortInfo.IsVfRepresentor {
-			if err := resetVeth(ctx, logger, conn, k.bridgeName, false); err != nil {
+			if err := resetVeth(logger, conn, k.bridgeName, false); err != nil {
 				return nil, err
 			}
 		} else {

diff --git a/pkg/networkservice/mechanisms/kernel/sriov.go b/pkg/networkservice/mechanisms/kernel/sriov.go
@@ -25,7 +25,7 @@ import (
 	"github.com/ovn-org/ovn-kubernetes/go-controller/pkg/util"
 
 	"github.com/networkservicemesh/sdk-ovs/pkg/tools/ifnames"
-	ovsutil "github.com/networkservicemesh/sdk-ovs/pkg/tools/util"
+	ovsutil "github.com/networkservicemesh/sdk-ovs/pkg/tools/utils"
 )
 
 func setupVF(ctx context.Context, logger log.Logger, bridgeName string, isClient bool) error {
@@ -60,6 +60,7 @@ func resetVF(logger log.Logger, portInfo ifnames.OvsPortInfo, bridgeName string)
 	if err != nil {
 		logger.Errorf("Failed to delete port %s from %s, stdout: %q, stderr: %q,"+
 			" error: %v", portInfo.PortName, bridgeName, stdout, stderr, err)
+		return err
 	}
 	return nil
 }
diff --git a/pkg/networkservice/mechanisms/vxlan/client.go b/pkg/networkservice/mechanisms/vxlan/client.go
@@ -14,6 +14,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package vxlan implements vxlan remote mechanism client and server chain element
 package vxlan
 
 import (
@@ -34,17 +35,16 @@ import (
 
 type vxlanClient struct {
 	bridgeName           string
-	vxlanInterfacesMutex *sync.Mutex
+	vxlanInterfacesMutex sync.Locker
 	vxlanInterfacesMap   map[string]int
 }
 
 // NewClient returns a Vxlan client chain element
-func NewClient(tunnelIP net.IP, bridgeName string, mutex *sync.Mutex, vxlanRefCountMap map[string]int) networkservice.NetworkServiceClient {
+func NewClient(tunnelIP net.IP, bridgeName string, mutex sync.Locker, vxlanRefCountMap map[string]int) networkservice.NetworkServiceClient {
 	return chain.NewNetworkServiceClient(
 		&vxlanClient{
 			bridgeName: bridgeName, vxlanInterfacesMutex: mutex, vxlanInterfacesMap: vxlanRefCountMap,
 		},
-		// TODO: vni client is from sdk-vpp, work with nsm community to host this into sdk repo
 		vni.NewClient(tunnelIP),
 	)
 }
@@ -60,17 +60,19 @@ func (c *vxlanClient) Request(ctx context.Context, request *networkservice.Netwo
 		return nil, err
 	}
 	if err := add(ctx, logger, request.Connection, c.bridgeName, c.vxlanInterfacesMutex, c.vxlanInterfacesMap, true); err != nil {
-		_ = remove(ctx, logger, request.Connection, c.bridgeName, c.vxlanInterfacesMutex, c.vxlanInterfacesMap, true)
+		_ = remove(request.Connection, c.bridgeName, c.vxlanInterfacesMutex, c.vxlanInterfacesMap, true)
 		return nil, err
 	}
 	return conn, nil
 }
 
 func (c *vxlanClient) Close(ctx context.Context, conn *networkservice.Connection, opts ...grpc.CallOption) (*empty.Empty, error) {
-	logger := log.FromContext(ctx).WithField("vxlanClient", "Close")
 	rv, err := next.Client(ctx).Close(ctx, conn, opts...)
-	if err := remove(ctx, logger, conn, c.bridgeName, c.vxlanInterfacesMutex, c.vxlanInterfacesMap, true); err != nil {
+	if err != nil {
+		return nil, err
+	}
+	if err := remove(conn, c.bridgeName, c.vxlanInterfacesMutex, c.vxlanInterfacesMap, true); err != nil {
 		return nil, err
 	}
-	return rv, err
+	return rv, nil
 }