Skip to content
This repository was archived by the owner on Sep 14, 2022. It is now read-only.

Comments

Crucible1#169

Draft
atheurer wants to merge 5 commits intomasterfrom
crucible1
Draft

Crucible1#169
atheurer wants to merge 5 commits intomasterfrom
crucible1

Conversation

@atheurer
Copy link
Owner

@atheurer atheurer commented Mar 3, 2021

changes to work with crucible

-should not be necessary if proper packages (lib-rdma) are installed
-may not have ofed installed
-some people may miss it
Copy link
Collaborator

@k-rister k-rister left a comment

Choose a reason for hiding this comment

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

This looks largely benign. I would probably want to get rid of the removal of a bunch of debug lines that are commented out. When trying to trace what is happening those are strategically placed based on previous experience and they aren't really hurting anything.

vlan_opt=""
fi
trex_server_cmd="./t-rex-64 -i -c ${trex_cpus} --checksum-offload --cfg ${yaml_file} --iom 0 -v 4 --prefix trafficgen_trex_ ${vlan_opt}"
trex_server_cmd="./t-rex-64 -i -c ${trex_cpus} --no-ofed-check --checksum-offload --cfg ${yaml_file} --iom 0 -v 4 --prefix trafficgen_trex_ ${vlan_opt}"
Copy link
Collaborator

Choose a reason for hiding this comment

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

So what is the side affect of this? I'm guessing Mellanox can't be used at all on the TRex side?

Copy link
Owner Author

Choose a reason for hiding this comment

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

No, this actually allows us to use Mellanox without TRex complaining.

ip = ip + "." + str(octet)
return ip


Copy link
Collaborator

Choose a reason for hiding this comment

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

You determined that this is backwards compatible, right?

Copy link
Owner Author

Choose a reason for hiding this comment

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

yes

ip_src = { "start": ip_to_int(ip_src) + flow_offset, "end": ip_to_int(ip_src) + tmp_num_flows + flow_offset }
ip_dst = { "start": ip_to_int(ip_dst) + flow_offset, "end": ip_to_int(ip_dst) + tmp_num_flows + flow_offset }
ip_src = { "start": int_to_ip(ip_to_int(ip_src) + flow_offset), "end": int_to_ip(ip_to_int(ip_src) + tmp_num_flows + flow_offset) }
ip_dst = { "start": int_to_ip(ip_to_int(ip_dst) + flow_offset), "end": int_to_ip(ip_to_int(ip_dst) + tmp_num_flows + flow_offset) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

As noted above, I believe you found this is backwards compatible.

Copy link
Owner Author

Choose a reason for hiding this comment

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

yes

@k-rister k-rister marked this pull request as draft March 3, 2021 18:19
@k-rister
Copy link
Collaborator

k-rister commented Mar 3, 2021

I'm a little confused by one aspect of this PR/branch. The first commit that I see (d18bbee) doesn't show up in the "Files Changed" diff -- or am I missing something? Make me wonder what else I may not be seeing...

@k-rister
Copy link
Collaborator

k-rister commented Mar 3, 2021

I'm a little confused by one aspect of this PR/branch. The first commit that I see (d18bbee) doesn't show up in the "Files Changed" diff -- or am I missing something? Make me wonder what else I may not be seeing...

Never mind. I see that you removed this change with the last commit.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants