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

add Xena functionality #153

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open

add Xena functionality #153

wants to merge 18 commits into from

Conversation

grdumas
Copy link
Contributor

@grdumas grdumas commented Aug 10, 2020

Adds argument parser option, allows user to specify a particular module of a Xena chassis.

(@ctrautma for reference)

Copy link

@ctrautma ctrautma left a comment

Choose a reason for hiding this comment

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

Looks OK to me.

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.

I would prefer if we were adding code in larger, more usable chunks. For example, this patch just adds a parameter and doesn't actually do anything with it.

@grdumas
Copy link
Contributor Author

grdumas commented Aug 10, 2020

Revised commit; adds Xena throughput test functionality.

@grdumas grdumas requested a review from k-rister August 10, 2020 19:26
@grdumas grdumas changed the title add parameter to specify a Xena module number add Xena functionality Aug 10, 2020
Copy link

@ctrautma ctrautma left a comment

Choose a reason for hiding this comment

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

Will test. Then give approval and other comments.

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.

I'm confused by the patches to binary-search.py. This looks like it is just calling Valkyrie and then exiting. Shouldn't this be happening inside the binary search loop?

@k-rister
Copy link
Collaborator

k-rister commented Sep 1, 2020

@ctrautma I think we can restart work on this. Obviously the first step we need to do is fix the merge conflicts.

@ctrautma
Copy link

ctrautma commented Sep 3, 2020 via email

Comment on lines +27 to +36
3. Obtain .x2544 config file & executable
* Create a test config inside xena2544 utility on the xenaweb system (ask @ctrautma for IP if necessary)
* Save config into the ftp folder off the c drive
* Use FTP commands to download the config and the latest copy of Valkyrie2544.exe
* Place both config and exe inside the trafficgen folder
```
ftp 11.11.111.11 [user]/[pwd]
bin
get valkyrie2544.exe
get <your config file>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there anything more generic we could say her that would be useful to other people who might try to use this?

Comment on lines +39 to +48
5. Install TRex
* Within trafficgen, open `install-trex.sh` for editing
* Make the following change:
* original: `if curl --output ${tarfile} ${trex_url} && tar zxf ${tarfile}; then`
* modified: ``if curl --insecure --output ${tarfile} ${trex_url} && tar zxf ${tarfile}; then``
* As root or using `sudo`, run: `./install-trex.sh`
* After install completes, replace the following files with copies from test ZIP `fix_trex_install` (or make changes listed in Known Issues):
* `trex_client.py` --> `/opt/trex/current/automation/trex_control_plane/interactive/trex/common/trex_client.py`
* `trex_conn.py` --> `/opt/trex/current/automation/trex_control_plane/interactive/trex/common/trex_conn.py`
* `trex_global_stats.py` --> `/opt/trex/current/automation/trex_control_plane/interactive/trex/common/stats/trex_global_stats.py`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is TRex stuff included here?

Also, lines 41-43 have been addressed directly in install-trex.sh.

Also a bit confused as to the relevance/reason why lines 45-48 exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The TRex inclusions are because of the following line from the beginning of binary-search.py:
from trex_tg_lib import *

Importing everything from trex_tg_lib in turn kicks off a chain of other imports - end result is that without TRex installed on a system, binary search crashes. For the sake of clarity I'll address the other questions in their own respective comments.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, that makes sense. I'll have to think if there is a way to work around this. Having to install TRex when you aren't using it just seems so broken...

Copy link
Collaborator

Choose a reason for hiding this comment

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

@ctrautma

I put together a patch that I think will solve these TRex dependency issues:

k-rister@fa7235c

Can you have somebody test if this patch allows the Valkrie2544 stuff (this PR) to work without having the TRex stuff present as is described in the proposed README files?

Comment on lines +120 to +138
## Known Issues
* SyntaxError from `trex_client.py`
* Open `/opt/trex/current/automation/trex_control_plane/interactive/trex/common/trex_client.py`
* Change line 1188 from: `self.conn.async.set_timeout_sec(timeout_sec)` to: `self.conn.async_.set_timeout_sec(timeout_sec)`

* SyntaxError from `trex_conn.py`
* Open `/opt/trex/current/automation/trex_control_plane/interactive/trex/common/trex_conn.py`
* Change line 36 from: `self.async = TRexSubscriber(self.ctx, self.rpc)` to: `self.async_ = TRexSubscriber(self.ctx, self.rpc)`
* Change line 60 from: `self.async.disconnect()` to: `self.async_.disconnect()`
* Change line 89 from: `self.async.barrier()` to: `self.async_.barrier()`
* Change line 98 from: `return self.async.barrier(baseline = True)` to: `return self.async_.barrier(baseline = True)`
* Change line 110 from: `self.async.set_as_zombie()` to: `self.async_.set_as_zombie()`
* Change line 143 from: `return ( self.async.last_data_recv_ts is not None and ((time.time() - self.async.last_data_recv_ts) <= 3) )` to `return ( self.async_.last_data_recv_ts is not None and ((time.time() - self.async_.last_data_recv_ts) <= 3) )`
* Change line 189 from: `rc = self.async.connect()` to `rc = self.async_.connect()`

* SyntaxError from `trex_global_stats.py`
* Open `/opt/trex/current/automation/trex_control_plane/interactive/trex/common/stats/trex_global_stats.py`
* Change line 81 from: `("async_util.", "{0}% / {1}".format( format_threshold(round_float(self.client.conn.async.monitor.get_cpu_util()), [85, 100], [0, 85]),` to: `("async_util.", "{0}% / {1}".format( format_threshold(round_float(self.client.conn.async_.monitor.get_cpu_util()), [85, 100], [0, 85]),`
* Change line 82 from: `format_num(self.client.conn.async.monitor.get_bps() / 8.0, suffix = "B/sec"))),` to: `format_num(self.client.conn.async_.monitor.get_bps() / 8.0, suffix = "B/sec"))),`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, what relevance does TRex have to Valkyrie2544?

We don't have problems like this when running TRex so I'm confused about why this stuff is here and why these problems are being seen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Egads, I thought I had commented on this already - my apologies! Back at the end of August I wrote this README as a "setup from scratch" sort of guide, which presumed the user had nothing already installed or configured. I walked through the process using a fresh Fedora 32 install, and wrote down the bare minimum steps necessary just to get the Xena functionality working.

(At the time of writing the patch) binary-search.py required that a user also install TRex in order to function (as mentioned in a previous comment). But even after installing TRex I still couldn't run binary-search.py due to several crash-inducing errors I'd never seen before. I still don't know why these problems suddenly popped up, but I eventually traced the errors back to the TRex library. Perhaps some update from Cisco unintentionally introduced a bug? I also can't rule out that the problem might have been self-inflicted from some flaw in my install/setup process.

Whatever the cause, I found that binary-search.py would stop crashing (at least enough so that the Xena bits worked) if I made a few tweaks to the TRex install. Hacky? Absolutely. But I lacked the necessary knowledge or skills to sort this problem out (or the time to develop such competencies). So I wrote those workarounds into the README in the hopes they'd be "good-enough" for folks needing to use Xena in the short term, at least while the TRex dependency issues persisted.

TLDR; the TRex changes were just duct tape and will hopefully become unneeded/irrelevant/obsolete and removed as soon as possible.

Comment on lines +1869 to +1872
if t_global.args.traffic_generator == 'valkyrie2544' and (t_global.args.num_flows < 1 or t_global.args.num_flows > 1000000):
bs_logger('Please enter a number of flows between 1 and 1000000')
bs_logger_cleanup(bs_logger_exit, bs_logger_thread)
return(1)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we shouldn't make this a generic error that isn't specific to Valkyrie2544...

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.

3 participants