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

use metadata map for storing vfconfig #307

Merged
merged 2 commits into from
Aug 29, 2021

Conversation

pperiyasamy
Copy link
Member

stores vfconfig into metadata map service to accommodate vfconfig for both client and endpoint connections.

an alternate and proper fix as per feedback in the PR

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

@pperiyasamy pperiyasamy requested a review from Bolodya1997 August 3, 2021 15:23
// LoadOrStore returns the existing VFConfig stored in per Connection.Id metadata if present.
// Otherwise, it stores and returns the given nterface_types.InterfaceIndex.
// The loaded result is true if the value was loaded, false if stored.
func LoadOrStore(ctx context.Context, isClient bool, config VFConfig) (value VFConfig, ok bool) {

Choose a reason for hiding this comment

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

Please return *VFConfig instead of VFConfig, I suppose that the most common usage for altering the data would be:

vfConfig, _ := vfconfig.LoadOrStore(ctx, ...)
...
vfConfig.xxx = yyy

Copy link
Member Author

Choose a reason for hiding this comment

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

sure, done.

Choose a reason for hiding this comment

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

So actually I have meant replacing all VFConfig with *VFConfig, both for input and for output values :)
It should be really confusing if Load and LoadOrStore return different types or if input and output parameters for LoadOrStore have different types.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, changed it now.

Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@est.tech>
Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@est.tech>
@denis-tingaikin denis-tingaikin merged commit eba0791 into networkservicemesh:main Aug 29, 2021
nsmbot pushed a commit to networkservicemesh/sdk-vpp that referenced this pull request Aug 29, 2021
…k-kernel@main

PR link: networkservicemesh/sdk-kernel#307

Commit: eba0791
Author: Denis Tingaikin
Date: 2021-08-29 23:55:39 +0300
Message:
  - Merge pull request #307 from Nordix/metadata-vfconfig
Signed-off-by: NSMBot <nsmbot@networkservicmesh.io>
nsmbot pushed a commit to networkservicemesh/sdk-sriov that referenced this pull request Aug 29, 2021
…k-kernel@main

PR link: networkservicemesh/sdk-kernel#307

Commit: eba0791
Author: Denis Tingaikin
Date: 2021-08-29 23:55:39 +0300
Message:
  - Merge pull request #307 from Nordix/metadata-vfconfig
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.

3 participants