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

Decode bytes before running string operations on the same #85

Open
wants to merge 2 commits into
base: mlnx_ofed
Choose a base branch
from
Open

Decode bytes before running string operations on the same #85

wants to merge 2 commits into from

Conversation

rohit-mundra
Copy link

@rohit-mundra rohit-mundra commented Mar 5, 2024 •
edited
Loading

Post execution of grep on vlan information for interface and when tc tool is executed get qdisc information for the device, the bytes must be converted to string to run split and regex.

resolve: #84

ntnx-alay
ntnx-alay approved these changes Mar 5, 2024
View reviewed changes
Copy link

@ntnx-alay ntnx-alay left a comment

Choose a reason for hiding this comment

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

Looks good to me. This would uncover in certain conditions where vlan is enabled. Otherwise code path is not exercised.

prasheshpatel
prasheshpatel approved these changes Mar 5, 2024
View reviewed changes
Copy link

@prasheshpatel prasheshpatel left a comment

Choose a reason for hiding this comment

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

The code change resolves #84 impacting Socket Priority to User Priority mapping for a parent PF device of the VLAN device.

@tzafrir-mellanox
Copy link
Collaborator

When removing spaces, you also removed useful spaces. Please fix that.

@rohit-mundra
Copy link
Author

rohit-mundra commented Mar 20, 2024 •
edited
Loading

When removing spaces, you also removed useful spaces. Please fix that.

Not able to find which space I have removed, as I have added just the decode calls to line variable.

[rohit.mundra@rohit-mundra mlnx-tools]$ git diff HEAD~2

diff --git a/python/tc_wrap.py b/python/tc_wrap.py
index b6fbe5e..54180bc 100755
--- a/python/tc_wrap.py
+++ b/python/tc_wrap.py
@@ -53,7 +53,7 @@ class skprio2up:
                                "* 2> /dev/null", shell=True, bufsize=4096,
                                stdout=PIPE).stdout
                for line in output:
-                       param, val=line.strip().split(":", 1)
+                       param, val=line.decode("utf-8").strip().split(":", 1)
                        vlan = param.split('.')[-1]
                        for item in val.split(":", 1)[1].split():
                                skprio, up = item.split(':')
@@ -150,7 +150,7 @@ class tcnum_mqprio(tcnum):

                for line in output:
                        empty=False
-                       m = re.search(r'tc (\d)', line)
+                       m = re.search(r'tc (\d)', line.decode("utf-8"))
                        if m:
                                self.tc_num = m.group(1)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prasheshpatel prasheshpatel prasheshpatel approved these changes

@ntnx-alay ntnx-alay ntnx-alay 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.

tc_wrap.py is failing due to use of string operation on bytes object
4 participants
@rohit-mundra @tzafrir-mellanox @ntnx-alay @prasheshpatel