Skip to content

Commit

Permalink
Fix for Policy Base Routing failure; Possible table ID collision.
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
ljkiraly committed May 4, 2023
1 parent bfd86b5 commit 5ab87dd
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -25,6 +27,7 @@ import (
"context"
"fmt"
"strconv"
"sync"
"time"

"golang.org/x/sys/unix"
Expand All @@ -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())
Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
//
Expand All @@ -25,6 +25,7 @@ package iprule

import (
"context"
"sync"

"github.com/golang/protobuf/ptypes/empty"
"github.com/networkservicemesh/api/pkg/api/networkservice"
Expand All @@ -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) {
Expand All @@ -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()

Expand Down

0 comments on commit 5ab87dd

Please sign in to comment.