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

make endpoint to honor sriov token #286

Merged
merged 3 commits into from
Sep 21, 2021

Conversation

pperiyasamy
Copy link
Member

aims to use sriov device for client connection using kernel mechanism.

Fixes #281 and Depends on this PR

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

@pperiyasamy
Copy link
Member Author

main.go Outdated
endpoint.WithAdditionalFunctionality(
sriovTokens := tokens.FromEnv(os.Environ())

var additionalFunc endpoint.Option

Choose a reason for hiding this comment

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

Suggested change
var additionalFunc endpoint.Option
var tokenServer networkservice.NetworkServiceServer
switch len(sriovTokens) {
case 0:
tokenServer = null.NewServer()
case 1:
var tokenKey string
for tokenKey = range sriovTokens {
break
}
tokenServer = token.NewServer(tokenKey)
default:
log.FromContext(ctx).Fatalf("endpoint must be configured with none or only one sriov resource")
}

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@denis-tingaikin
Copy link
Member

@pperiyasamy Could you rebase this PR? :)

@pperiyasamy pperiyasamy force-pushed the sriov-support branch 2 times, most recently from d6caa1e to 484b69b Compare September 1, 2021 07:49
@pperiyasamy
Copy link
Member Author

@pperiyasamy Could you rebase this PR? :)

done.

@denis-tingaikin
Copy link
Member

denis-tingaikin commented Sep 3, 2021

I wondered if we should create a separate cmd repository for sriov supported endpoint.

Benefits of this directions are:

  1. cmd-nse-icmp-responder still extremely simple.
  2. cmd-nse-sriov-icmp-responder will use this inline in the main https://github.com/networkservicemesh/cmd-nse-icmp-responder/pull/286/files#diff-2873f79a86c0d8b3335cd7731b0ecf7dd4301eb19a82ef7a1cba7589b5252261R274-R289 that also will look more simple.

@edwarnicke What do you think?

@pperiyasamy
Copy link
Member Author

I wondered if we should create a separate cmd repository for sriov supported endpoint.

Benefits of this directions are:

  1. cmd-nse-icmp-responder still extremely simple.
  2. cmd-nse-sriov-icmp-responder will use this inline in the main https://github.com/networkservicemesh/cmd-nse-icmp-responder/pull/286/files#diff-2873f79a86c0d8b3335cd7731b0ecf7dd4301eb19a82ef7a1cba7589b5252261R274-R289 that also will look more simple.

@edwarnicke What do you think?

@denis-tingaikin we just add this sriov token server chain element conditionally and this can be decided with pod spec.

cat > patch-nse.yaml <<EOF
---
apiVersion: apps/v1
kind: Deployment
metadata:
  name: nse-kernel
spec:
  template:
    spec:
      containers:
        - name: nse
          env:
            - name: NSE_LABELS
              value: serviceDomain:worker.domain
            - name: NSE_CIDR_PREFIX
              value: 172.16.1.100/31
          resources:
            limits:
              worker.domain/100G: 1
EOF

so do you really think we need another repo just for this ?

@denis-tingaikin
Copy link
Member

denis-tingaikin commented Sep 13, 2021

@pperiyasamy Yes, as I said #286 (comment) separate repo makes sense to me, but I'm fine if @edwarnicke agree to merge this PR :)

@denis-tingaikin denis-tingaikin added stability The problem is related to system stability and removed stability The problem is related to system stability labels Sep 21, 2021
Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@est.tech>
Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@est.tech>
Signed-off-by: Periyasamy Palanisamy <periyasamy.palanisamy@est.tech>
@edwarnicke edwarnicke merged commit 7751049 into networkservicemesh:main Sep 21, 2021
@pperiyasamy pperiyasamy deleted the sriov-support branch September 22, 2021 06:56
nsmbot pushed a commit that referenced this pull request Oct 19, 2021
…k-sriov@main

PR link: networkservicemesh/sdk-sriov#286

Commit: e344c25
Author: Ed Warnicke
Date: 2021-10-19 12:50:08 -0500
Message:
  - Merge pull request #286 from networkservicemesh/update/networkservicemesh/sdk-kernel
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.

add support for VF attachment
4 participants