Fix for Policy Base Routing failure; Possible table ID collision.

[ljkiraly]

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

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

[denis-tingaikin]

Question:

Is NSC from your usecase requests two or more connections?

[ljkiraly]

@denis-tingaikin In the use case described in the issue (networkservicemesh/deployments-k8s/issues/9119) the NSC has 2 connections and each nsc interface has 2 IP's. So that's why needs 2+2 source route and 4 tables.

[denis-tingaikin]

@ljkiraly Got it. I think we might need to solve the issue with multiple connections for NSC within begin chain element https://github.com/networkservicemesh/sdk/tree/main/pkg/networkservice/common/begin.

For now, could you try to use the same first path id for the client?

I meen something like this:

var req1, req2 networkservice.Request = ...

// use the same first path segement  id to enable sync on the server for both connections
req2.GetConnection().GetPath().GetPathSegemets()[0].Id = req1.GetConnection().GetPath().GetPathSegments()[0].Id



nsmClient := client.NewClient(...)


conn1, _ := nscClient.Request(ctx, req1)
conn2, _ := nscClient.Request(ctx, req2)

Then all operations for the client will be invoked in queue on the server. See at https://github.com/networkservicemesh/sdk/blob/main/pkg/networkservice/common/begin/doc.go#L18-L102

[ljkiraly]

@denis-tingaikin OK I see your point now. Other than that my proposal wasn't good enough in case of multiple NSCs on the same node. There is no sense to block the forwarder at the point where collecting the existing routing table IDs, This routing information is set to a given NSC namespace, so can not collide to another NSCs.

I can accept the answer provided by @NikitaSkrynnik (option 2: Two requests can go in parallel, but the first path segment should be the same in both of them)

[edwarnicke]

My suggestion would be that we keep a sync.Map for a key combining netns and tableid and a value of connection id. That sync.Map could be used with a LoadAndStore to determine whether in roughly https://github.com/Nordix/nsm-sdk-kernel/blob/5ab87dd990f73aded7394ddce8c10fbaca962b01/pkg/kernel/networkservice/connectioncontextkernel/ipcontext/iprule/common.go#L346 we have succeeded in getting the tableid or need to try the next free one.

Please note that such a sync.Map will need to have its (netns,tableid) entry cleaned up on Connection.Close