Use fixed-length paths in memifproxy

[d-uzlov]

In memifproxy we create unix sockets with path containing connection id.

sdk-vpp/pkg/networkservice/mechanisms/memif/memifproxy/client.go

Lines 82 to 84 in 7780191

	func listenSocketFilename(conn *networkservice.Connection) string {
	return filepath.Join(os.TempDir(), "memifproxy", conn.GetId(), "memif.socket")
	}

This is an easy way to ensure that every connection has its own unique socket, but it has drawbacks.
Unix sockets are limited in length. On linux the limit is 108.
Connection IDs are not limited in length, and paths that contain them can easily exceed this limit. Related issue: networkservicemesh/deployments-k8s#1826
To ensure reliable behavior we must not use any variable length strings when defining paths.
We have several possible solutions for this:

Solution 1

Use relative paths.
I don't think is an appropriate solution because in order to use relative path to a file in a certain directory we would need to change current working directory, and this is basically incompatible with multithreading.

Solution 2

Keep [connection id -> uuid] map in memifproxy, and use these uuids for socket paths.

Solution 3

Use short symlinks to sockets with potentially long paths.
This is basically identical to second solution, because we would still need to keep mapping from connection id to its symlink, so I believe it's overly complicated without any real benefits.

Conclusion

I think that the only suitable solution here is the second: use mapped uuids for socket paths.
It would also be pretty easy to implement.

There is still another unknown-length part of the path: os.TempDir(). Temp directory is usually just /tmp by default, but it is not guaranteed. For example, it seems like on MacOS default temp dir is relatively long (48 symbols). And, of course, users can set temp dir to be whatever they want.
We can probably say that very long temp dir path is just an invalid configuration for a program that uses memifproxy.

[denis-tingaikin]

Solution2 looks good to me.

@edwarnicke Can you also have a look at the issue and share your thoughts?

[edwarnicke]

Keep [connection id -> uuid] map in memifproxy, and use these uuids for socket paths.

Which is state we will lose when we heal :(

Use short symlinks to sockets with potentially long paths.
symlinks will break grpcfd in terms of passing them between Pods

Use relative paths.

You are correct about the issues for this within NSM. However, VPP's api runs in a single worker thread. It may very well be doable to upstream code to VPP that does the relative path trick.

[edwarnicke]

In the immediate term: why are we ever setting the connection id manually in any of our things like cmd-nsc ?

[denis-tingaikin]

Which is the state we will lose when we heal :(

Basically, memif chain element is using in next places

1. client
2. forwarder
3. endpoint

Let's consider death cases:

1. client died -- nothing to heal
2. forwarder/endpoint died -- we need to re-create memif folder with the socket on the forwarder/endpoint side.

it seems solution 2 can be used.

Did I miss something?

UPD: we also could store the map connId to sockerDir on the disk.

[edwarnicke]

In the immediate term: why are we ever setting the connection id manually in any of our things like cmd-nsc ?

[denis-tingaikin]

In the immediate term: why are we ever setting the connection id manually in any of our things like cmd-nsc ?

Note: we are setting id manually because we want to not see issues on NSC restarting.

And it is not manually, we are using the unique name of Pod.

See at networkservicemesh/cmd-nsc-vpp#180

[edwarnicke]

@denis-tingaikin What issues would we see on NSC restarting if we didn't manually set its connection ID?

[denis-tingaikin]

@edwarnicke

1. resources leaking on forwarder, manager, and etc. Resouces will be not disposed of until token expiration. This problem may be critical for restarting clients. (Imagine a ton of interfaces on the forwarder side that produces by the client restarting...)
2. Client may lose selected endpoint on restarting.
3. In the case of external clients we need also to identify the sidecar client as the init container client.

[NikitaSkrynnik]

Can't reproduce the problem. MemifProxy client has been deleted in this #470. MemifProxy server doesn't have this problem.