Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Calico/VPP NSM integration] memif socket ID generation is wrong for the single VPP scenario #361

Closed
Bolodya1997 opened this issue Sep 2, 2021 · 4 comments
Assignees
Labels
bug Something isn't working Planning Issue related to SOW

Comments

@Bolodya1997
Copy link

Bolodya1997 commented Sep 2, 2021

Issue

Currently memif generates socket IDs incrementing some per-application counter:

socketID = atomic.AddUint32(&lastSocketID, 1) // TODO - work out a solution that works long term

It is totally incorrect for the single VPP scenario (e.g. with Calico VPP) - Client, Forwarder and Endpoint start requesting VPP with the same socket IDs and fail.

Parent issue

networkservicemesh/integration-k8s-kind#325

Possible solution

Use some random generator for creating socket IDs + check for "already exists" error to retry:

var random *rand.Rand

func init() {
	random = rand.New(rand.NewSource(time.Now().UnixNano()))
}

func createMemifSocket(ctx context.Context, mechanism *memifMech.Mechanism, vppConn api.Connection, isClient bool) (socketID uint32, err error) {
	// Extract the socket filename
	u, err := url.Parse(mechanism.GetSocketFileURL())
	if err != nil {
		return 0, errors.Wrapf(err, "not a valid url %q", mechanism.GetSocketFileURL())
	}
	if u.Scheme != memifMech.SocketFileScheme {
		return 0, errors.Errorf("socket file url must have scheme %q, actual %q", memifMech.SocketFileScheme, u.Scheme)
	}

	now := time.Now()
	for {
		// Create the socketID
		socketID = random.Uint32()
		memifSocketAddDel := &memif.MemifSocketFilenameAddDel{
			IsAdd:          true,
			SocketID:       socketID,
			SocketFilename: u.Path,
		}

		if _, err = memif.NewServiceClient(vppConn).MemifSocketFilenameAddDel(ctx, memifSocketAddDel); err != nil {
			if err.(api.VPPApiError) == api.ENTRY_ALREADY_EXISTS {
				continue
			}
			return 0, errors.WithStack(err)
		}

		log.FromContext(ctx).
			WithField("SocketID", memifSocketAddDel.SocketID).
			WithField("SocketFilename", memifSocketAddDel.SocketFilename).
			WithField("IsAdd", memifSocketAddDel.IsAdd).
			WithField("duration", time.Since(now)).
			WithField("vppapi", "MemifSocketFilenameAddDel").Debug("completed")
		store(ctx, isClient, memifSocketAddDel)

		return socketID, nil
	}
}
@Bolodya1997
Copy link
Author

@denis-tingaikin @edwarnicke
cc

@Bolodya1997 Bolodya1997 changed the title memif socket ID generation is wrong for the single VPP scenario [Calico/VPP NSM integration] memif socket ID generation is wrong for the single VPP scenario Sep 2, 2021
@Bolodya1997 Bolodya1997 added Planning Issue related to SOW bug Something isn't working question Further information is requested and removed question Further information is requested labels Sep 2, 2021
@Bolodya1997
Copy link
Author

Possible solution with generating socket ID on the VPP side - https://gerrit.fd.io/r/c/vpp/+/32271/6/src/plugins/memif/memif.api#43.

@Bolodya1997
Copy link
Author

Should be closed with #370

@denis-tingaikin
Copy link
Member

It seems like all PRs have merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Planning Issue related to SOW
Projects
None yet
Development

No branches or pull requests

2 participants