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 25, 2023
1 parent bfd86b5 commit 04db164
Show file tree
Hide file tree
Showing 6 changed files with 152 additions and 25 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 Down Expand Up @@ -40,7 +42,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, nsRTableNextIdToConnId *NetnsRTableNextIdToConnMap) 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 All @@ -54,14 +56,14 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *Map)
if err != nil {
return errors.Wrapf(err, "failed to find link %s", ifName)
}

ps, ok := tableIDs.Load(conn.GetId())
connId := conn.GetId()
ps, ok := tableIDs.Load(connId)
if !ok {
if len(conn.Context.IpContext.Policies) == 0 {
return nil
}
ps = make(map[int]*networkservice.PolicyRoute)
tableIDs.Store(conn.GetId(), ps)
tableIDs.Store(connId, ps)
}
// Get policies to add and to remove
toAdd, toRemove := getPolicyDifferences(ps, conn.Context.IpContext.Policies)
Expand All @@ -72,34 +74,53 @@ func create(ctx context.Context, conn *networkservice.Connection, tableIDs *Map)
return err
}
delete(ps, tableID)
tableIDs.Store(conn.GetId(), ps)
tableIDs.Store(connId, ps)
}
// Add new policies
netNSInode := mechanism.GetNetNSInode()
for _, policy := range toAdd {
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 {
var tableID int
var nsrtid *NetnsRTableNextID
for {
tableID, err = getFreeTableID(ctx, netlinkHandle)
if err != nil {
return err
}
nsrtid = NewNetnsRTableNextID(netNSInode, tableID)
sconn, _ := nsRTableNextIdToConnId.LoadOrStore(*nsrtid, connId)
if connId == sconn {
break
}

}
if err := ruleAdd(ctx, netlinkHandle, policy, tableID); err != nil {
if err := addPolicy(ctx, netlinkHandle, policy, l, ps, tableIDs, tableID, connId, *nsrtid, nsRTableNextIdToConnId); err != nil {
return err
}
ps[tableID] = policy
tableIDs.Store(conn.GetId(), ps)
}
}
return nil
}

func addPolicy(ctx context.Context, netlinkHandle *netlink.Handle, policy *networkservice.PolicyRoute, l netlink.Link, ps policies, tableIDs *Map, tableID int, connID string, nsrtid NetnsRTableNextID, nsRTableNextIdToConnId *NetnsRTableNextIdToConnMap) error {
// 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(connID, ps)
nsRTableNextIdToConnId.Delete(nsrtid)
return nil
}

func getPolicyDifferences(current map[int]*networkservice.PolicyRoute, newPolicies []*networkservice.PolicyRoute) (toAdd []*networkservice.PolicyRoute, toRemove map[int]*networkservice.PolicyRoute) {
type table struct {
tableID int
Expand Down Expand Up @@ -338,7 +359,6 @@ func getFreeTableID(ctx context.Context, handle *netlink.Handle) (int, error) {
break
}
}

log.FromContext(ctx).
WithField("tableID", tableID).
WithField("netlink", "getFreeTableID").Debug("completed")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,12 @@ import (
)

//go:generate go-syncmap -output table_map.gen.go -type Map<string,policies>
//go:generate go-syncmap -output netns_rtable_conn_map.gen.go -type NetnsRTableNextIdToConnMap<NetnsRTableNextID,string>

type policies map[int]*networkservice.PolicyRoute

// Map - sync.Map with key == string (connID) and value == policies
type Map sync.Map

// NetnsRTableNextIdToConnMap - sync.Map with key NetnsRTableNextID value of string (connID)
type NetnsRTableNextIdToConnMap sync.Map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) 2023 Nordix and/or its affiliates.
//
// SPDX-License-Identifier: Apache-2.0
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at:
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package iprule

// NetnsRTableNextID stores Network namespace and Next Routing Table ID
type NetnsRTableNextID struct {
ns string
nrtid int
}

// NewNetnsRTableNextID returns *NetnsRTableNextID entry
func NewNetnsRTableNextID(ns string, nrtid int) *NetnsRTableNextID {
return &NetnsRTableNextID{
ns: ns,
nrtid: nrtid,
}
}
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 Down Expand Up @@ -34,7 +34,8 @@ import (
)

type ipruleServer struct {
tables Map
tables Map
nsRTableNextIdToConnId NetnsRTableNextIdToConnMap
}

// NewServer creates a new server chain element setting ip rules
Expand All @@ -55,7 +56,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.nsRTableNextIdToConnId); err != nil {
closeCtx, cancelClose := postponeCtxFunc()
defer cancelClose()

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 04db164

Please sign in to comment.