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

add connection establish check for resource pool elements #258

Merged

Conversation

pperiyasamy
Copy link
Member

Signed-off-by: Periyasamy Palanisamy periyasamy.palanisamy@est.tech

@@ -65,11 +65,13 @@ func (i *resourcePoolClient) Request(ctx context.Context, request *networkservic
oldPCIAddress := request.GetConnection().GetMechanism().GetParameters()[common.PCIAddressKey]
oldTokenID := request.GetConnection().GetMechanism().GetParameters()[TokenIDKey]

isEstablished := request.GetConnection().GetNextPathSegment() != nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fine for the restart scenario?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@denis-tingaikin this is a good point, this check is just to provide do-nothing op for refresh/heal connection requests. yes, the connection state must be recovered for forwarder restart scenario, is there a way nsm mgr set isRecovery flag in connection object which would be parsed in forwarder chain elements to recover the state ? does sdk-vpp somehow do it already ?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that in restart scenario we will have Connection.Path cleaned up on NSMgr.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually for checking to see if a particular chain element has already done its work for a connection I will check some metadata for the connection that gets set after the work has been done... since the real question is "Do I have to do work or not" that may be the best choice.

Copy link
Member Author

@pperiyasamy pperiyasamy Sep 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@edwarnicke as discussed resource pool chain elements are now made to use vfconfig metadata cache (this is populated by same chain element) for isEstablished check. this would cover both restart and refresh scenarios.

Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@est.tech>
@pperiyasamy pperiyasamy force-pushed the resource-reestablish-check branch from b0a816f to 12c08ff Compare September 15, 2021 07:28
Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@est.tech>
@pperiyasamy pperiyasamy force-pushed the resource-reestablish-check branch from 12c08ff to a392587 Compare September 15, 2021 07:33
@denis-tingaikin denis-tingaikin merged commit ea35b0f into networkservicemesh:main Sep 15, 2021
@pperiyasamy pperiyasamy deleted the resource-reestablish-check branch September 15, 2021 08:37
nsmbot pushed a commit to networkservicemesh/cmd-nsc-init that referenced this pull request Sep 15, 2021
…k-sriov@main

PR link: networkservicemesh/sdk-sriov#258

Commit: ea35b0f
Author: Denis Tingaikin
Date: 2021-09-15 11:36:26 +0300
Message:
  - Merge pull request #258 from Nordix/resource-reestablish-check
Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-nsc that referenced this pull request Sep 15, 2021
…k-sriov@main

PR link: networkservicemesh/sdk-sriov#258

Commit: ea35b0f
Author: Denis Tingaikin
Date: 2021-09-15 11:36:26 +0300
Message:
  - Merge pull request #258 from Nordix/resource-reestablish-check
Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/cmd-forwarder-sriov that referenced this pull request Sep 15, 2021
…k-sriov@main

PR link: networkservicemesh/sdk-sriov#258

Commit: ea35b0f
Author: Denis Tingaikin
Date: 2021-09-15 11:36:26 +0300
Message:
  - Merge pull request #258 from Nordix/resource-reestablish-check
Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-ovs that referenced this pull request Sep 15, 2021
…k-sriov@main

PR link: networkservicemesh/sdk-sriov#258

Commit: ea35b0f
Author: Denis Tingaikin
Date: 2021-09-15 11:36:26 +0300
Message:
  - Merge pull request #258 from Nordix/resource-reestablish-check
Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants