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

[sdk-vpp#314] Update tests to new VPP forwarder #2413

Merged

Conversation

Bolodya1997
Copy link

@Bolodya1997 Bolodya1997 commented Aug 3, 2021

Description

Use VPP Forwarder with SR-IOV capabilities instead of SR-IOV Forwarder in all tests.

Issue link

Needed for networkservicemesh/sdk-vpp#314.

How Has This Been Tested?

  • Added unit testing to cover
  • Tested manually
  • Tested by integration testing
  • Have not tested

Types of changes

  • Bug fix
  • New functionallity
  • Documentation
  • Refactoring
  • CI

- name: forwarder-vpp
env:
- name: NSM_SRIOV_CONFIG_FILE
value: /dev/null
Copy link
Member

Choose a reason for hiding this comment

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

Question... why does this have to be set... if you don't configure NSM_SRIOV ... shouldn't it just not be used?

Copy link
Author

Choose a reason for hiding this comment

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

You are totally right, fixed cmd-forwarder-vpp just not to use SR-IOV if there is no config - networkservicemesh/cmd-forwarder-vpp#343.

@Bolodya1997 Bolodya1997 force-pushed the sdk-vpp#314/update-tests branch from c8165a8 to 00b824b Compare September 23, 2021 09:19
Signed-off-by: Vladimir Popov <vladimir.popov@xored.com>
@Bolodya1997 Bolodya1997 force-pushed the sdk-vpp#314/update-tests branch from 00b824b to 7d4149b Compare September 23, 2021 09:21
@Bolodya1997 Bolodya1997 marked this pull request as ready for review September 23, 2021 09:21
Copy link
Member

@denis-tingaikin denis-tingaikin left a comment

Choose a reason for hiding this comment

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

LGTM

@edwarnicke Do you have any additional comments on this?

@@ -1,42 +0,0 @@
## Requires
Copy link
Member

Choose a reason for hiding this comment

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

Can we just disable this example?

Copy link
Member

Choose a reason for hiding this comment

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

@pperiyasamy How are you on getting the cmd-forwarder-ovs into a place we could put it in the examples?

Copy link
Member

Choose a reason for hiding this comment

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

@denis-tingaikin One option is to simply use the cmd-forwarder-vpp and not provide it a SRIOV config (and thus it won't handle SRIOV case).

Copy link
Member

Choose a reason for hiding this comment

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

@edwarnicke The cmd-forwarder-ovs runs only in kernel datapath if no SRIOV config provided. when sriov config exists, then it supports both kernel and smart nic, the connection type would be chosen based on client service request URL.

@edwarnicke edwarnicke merged commit fa9dc94 into networkservicemesh:main Sep 27, 2021
nsmbot pushed a commit to networkservicemesh/integration-tests that referenced this pull request Sep 27, 2021
…ployments-k8s@main

PR link: networkservicemesh/deployments-k8s#2413

Commit: e5f2ac6
Author: Network Service Mesh Bot
Date: 2021-09-27 05:44:34 -0500
Message:
  - Update go.mod and go.sum to latest version from networkservicemesh/de…
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