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.

Sign up for GitHub

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

Jump to bottom

SR-IOV forwarder #18

Merged
merged 31 commits into from
Dec 29, 2020
Merged

SR-IOV forwarder #18

merged 31 commits into from
Dec 29, 2020

Conversation

Bolodya1997
Copy link

Add SR-IOV forwarder with kernel, VFIO mechanisms supported.

@Bolodya1997 Bolodya1997 marked this pull request as ready for review October 20, 2020 13:05
haiodo
haiodo reviewed Oct 20, 2020
View reviewed changes
Dockerfile Outdated Show resolved Hide resolved
@Bolodya1997 Bolodya1997 force-pushed the forwarder branch 2 times, most recently from 414841d to 72db92b Compare October 26, 2020 01:48
Vladimir Popov added 22 commits November 13, 2020 16:09
Rework deviceplugin server
d6d6a94 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Refactor project
bbc85f4 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Add resetmechanism server chain element
06373ec 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Add sriov server chain element
709f566 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Rework main.test
5189820 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix issues
4412289 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Add NOOP support
707cf2f 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix invalid replace check for CI
c898ae5 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Upgrade dependencies
0e22283 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Use token.Pool
c23fe8d 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix linter issues
1981482 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix issue with invalid device free
8a65a96 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Rework device plugin using tokens.ToEnv
e2647f6 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix resetmechanism
113d6b1 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Rework resetmechanism to use tailServer
4a4637f 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Upgrade dependencies
75ea60a 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix dockerfile for tests
d4ee098 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Upgrade spire, go version
bac8e54 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Fix dockerfile for go 1.15
8e54e27 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Make cmd-forwarder-sriov look like cmd-forwarder-vppagent
e25df2c 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Remove license job
886d287 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Small fixes
62b6499 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Vladimir Popov added 2 commits November 18, 2020 16:03
Use local listenOn, add tracing
30b939c 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
Update kubelet dependency and remove kubernetes dependency
dd1b3f9 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 force-pushed the forwarder branch 3 times, most recently from 89392f2 to f97e1a8 Compare December 22, 2020 14:00
Update dependencies
ccf7b5c 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 marked this pull request as ready for review December 22, 2020 15:27
haiodo
haiodo reviewed Dec 28, 2020
View reviewed changes
internal/k8s/deviceplugin/server.go Outdated
Endpoint: socket,
ResourceName: s.name,
}); err != nil {
logEntry.Errorf("error re registering server: %s %+v", s.name, err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we do a fatal here? Since it will not continue working properly and will not recover.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, fixed.

haiodo
haiodo reviewed Dec 28, 2020
View reviewed changes
internal/k8s/deviceplugin/server.go Outdated
}
}

func (s *devicePluginServer) respToDeviceIDs(resp *podresources.ListPodResourcesResponse) map[string]struct{} {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we unify with function respToDeviceIDs from line 121?

Copy link
Author

Choose a reason for hiding this comment

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

Nope, they are different:

  • 121: returns [resource name] -> [list of device IDs for this resource] mapping
  • 206: returns set of the device IDs for the given resource name

haiodo
haiodo reviewed Dec 28, 2020
View reviewed changes
internal/networkservice/chains/sriovns/server.go Outdated
return rv
}

func sriovChain(
Copy link
Contributor

Choose a reason for hiding this comment

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

I would prefer to have SR-IOV chain inside NewServer function, it is not so big.

Copy link
Author

@Bolodya1997 Bolodya1997 Dec 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Fixed in another PR: networkservicemesh/sdk-sriov#73
https://github.com/networkservicemesh/sdk-sriov/blob/54f053b5711978a72c56424a7aec60c08f54ac54/pkg/networkservice/chains/sriovns/server.go

haiodo
haiodo reviewed Dec 28, 2020
View reviewed changes
internal/networkservice/common/resetmechanism/server.go Outdated

conn, err := s.wrappedServer.Request(ctx, request)
if mech := conn.GetMechanism(); err == nil && mech != nil {
s.connMechanisms[connID] = mech.Clone()
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a synchronisation logic here? I suppose we need.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in another PR: networkservicemesh/sdk-sriov#73
https://github.com/networkservicemesh/sdk-sriov/blob/54f053b5711978a72c56424a7aec60c08f54ac54/pkg/networkservice/common/resetmechanism/server.go

haiodo
haiodo reviewed Dec 28, 2020
View reviewed changes
internal/networkservice/common/resetmechanism/server.go Outdated
}

func (s *resetMechanismServer) Close(ctx context.Context, conn *networkservice.Connection) (*empty.Empty, error) {
delete(s.connMechanisms, conn.GetId())
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Please add sync.

Copy link
Author

Choose a reason for hiding this comment

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

Fixed in another PR: networkservicemesh/sdk-sriov#73

haiodo
haiodo reviewed Dec 28, 2020
View reviewed changes
main.go Outdated
registryapi.NewNetworkServiceEndpointRegistryClient(registryCC),
)
// TODO - something smarter for expireTime
expireTime, err := ptypes.TimestampProto(time.Now().Add(config.MaxTokenLifetime))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think @denis-tingajkin already had some chain element for registry, to perform this automatically and it should include automatic re-register to prevent registration expire.

Copy link
Member

Choose a reason for hiding this comment

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

I feel refresh.NewNetworkServiceEndpointRegistryClient() should be fine here.

Copy link
Author

Choose a reason for hiding this comment

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

Yep, fixed.

haiodo
haiodo requested changes Dec 28, 2020
View reviewed changes
Copy link
Contributor

@haiodo haiodo left a comment

Choose a reason for hiding this comment

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

In general look good, but it seems it require a bit of more work. Thanks.

@Bolodya1997 Bolodya1997 marked this pull request as draft December 28, 2020 16:45
@Bolodya1997 Bolodya1997 force-pushed the forwarder branch 2 times, most recently from 902e105 to c1964ca Compare December 29, 2020 03:31
Move k8s, sriov files to sdk-k8s, sdk-sriov
eef9974 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 force-pushed the forwarder branch 2 times, most recently from 1bec9c0 to bfc5f0e Compare December 29, 2020 05:02
Use interpose registry client
f61a5f5 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 force-pushed the forwarder branch 2 times, most recently from f162372 to 52f2533 Compare December 29, 2020 08:29
Manually update to the latest cmd-template version
9681400 
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 marked this pull request as ready for review December 29, 2020 09:02
@haiodo haiodo merged commit 68f6ec3 into networkservicemesh:master Dec 29, 2020
This was referenced Jan 18, 2021
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@denis-tingaikin denis-tingaikin denis-tingaikin left review comments

@haiodo haiodo haiodo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Bolodya1997 @haiodo @denis-tingaikin