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

[ovs offload] script name change ovs_offload_lnv.py to ovs_offload_lnw.py #2

Merged
merged 8 commits into from
Sep 11, 2024

Conversation

naveenashok211
Copy link
Collaborator

Script names updated to
ovs_offload_lnv.py -> ovs_offload_lnw.py
ovs_offload_lnv2.py -> ovs_offload_lnw_v2.py
ovs_offload_lnv3.py -> ovs_offload_lnw_v3.py

The README is updated with the name change

Copy link
Collaborator

@5abeel 5abeel left a comment

Choose a reason for hiding this comment

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

LGTM

@naveenashok211
Copy link
Collaborator Author

Updated the README with suggested change .
-1. This is a python script : ovs_offload/ovs_offload_lnw.py that can be used with P4: fxp-net_linux-networking.p4 for release 1.7.0
+1. This is a python script : ovs_offload/ovs_offload_lnw.py that can be used with P4: fxp-net_linux-networking.p4 for release 1.7.0 and later

Copy link
Collaborator

@5abeel 5abeel left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ffoulkes ffoulkes left a comment

Choose a reason for hiding this comment

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

See suggested changes

I think I've marked all the incorrect uses of "setup."

ovs_offload/README.md Outdated Show resolved Hide resolved
ovs_offload/README.md Outdated Show resolved Hide resolved

## Test Environment Setup:
## Test Environment Setup

- Before running script check if ssh as a root user is enabled and root user password is setup for ssh.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Before running script check if ssh as a root user is enabled and root user password is setup for ssh.
Before running script check if ssh as a root user is enabled and root user password is set up for ssh.
  1. Bullets are used to form an unordered list. There's only one item here -- a sentence that sets up an example -- so you don't use a bullet.

  2. This sequence of words doesn't make sense. Please rewrite it. Perhaps you mean:

    Before running the script, make sure that /etc/ssh/sshd_config contains the line

    PermitRootLogin yes
  3. The bash command example doesn't help. I would recommend deleting it and focusing instead on describing clearly what you want the customer to accomplish.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Addressed this in the latest commit .

ovs_offload/README.md Outdated Show resolved Hide resolved
ovs_offload/README.md Outdated Show resolved Hide resolved
ovs_offload/README.md Outdated Show resolved Hide resolved
ovs_offload/README.md Outdated Show resolved Hide resolved
ovs_offload/README.md Outdated Show resolved Hide resolved
ovs_offload/README.md Outdated Show resolved Hide resolved
@@ -563,7 +561,7 @@ def build_p4rt_config(vf_list=[], acc_pr_list=[],vm_ip_list=[], host_idpf_intf='

def build_args():
# Create the top-level parser
parser = argparse.ArgumentParser(description='Run Linux networking with OVS Offload')
parser = argparse.ArgumentParser(description='Run Linux networking V2 with OVS Offload')
Copy link
Contributor

Choose a reason for hiding this comment

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

We've dropped support for LNW V2.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have 3 versions of the script to support the OVS offload recipe on all releases from 1.4 to 1.7 as we had customers using the older versions like 1.4 for the POC hence I have retained the 3 versions , but we can remove it in future once customers move to a release 1.7 and above

ovs_offload_lnw.py : (P4:fxp-net_linux-networking.p4, IPU SDK Release >= 1.7.0)
ovs_offload_lnw_v3.py : (P4:fxp-net_linux-networking_v3.p4, IPU SDK Release 1.6.0, 1.6.1)
ovs_offload_lnw_v2.py: (P4:fxp-net_linux-networking_v2.p4, IPU SDK Release 1.4.0)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah. Got it.

Copy link
Contributor

@ffoulkes ffoulkes left a comment

Choose a reason for hiding this comment

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

LGTM

@ffoulkes ffoulkes merged commit dbfdaa8 into ipdk-io:main Sep 11, 2024
4 checks passed
naveenashok211 added a commit that referenced this pull request Jan 9, 2025
Pull changes from ipdk-io main to fork
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