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

Add firmware reset, up, and version to dpu-tools #25

Merged
merged 9 commits into from
Oct 21, 2024

Conversation

SamD2021
Copy link
Contributor

@SamD2021 SamD2021 commented Oct 8, 2024

This PR introduces new commands to manage IPU firmware, including:

Firmware Reset: Reset firmware versions.
Firmware Upgrade: Upgrade the firmware using specific version flags.
Firmware Version: Query the current firmware version.
Additionally, it consolidates shared logic into a new common module for reusability and implements a fwutils-based module to interact with the firmware.

@SamD2021 SamD2021 force-pushed the add_fwutil_ipu branch 2 times, most recently from 8a3edad to 11b744f Compare October 8, 2024 21:45
@SamD2021 SamD2021 changed the title Implement fwutil to dpu-tools Add firmware reset, up, and version to dpu-tools Oct 8, 2024
@bn222
Copy link
Owner

bn222 commented Oct 9, 2024

Can you create a uniform way to handle all DPUs? Some high level commands that branch into the right tool for the right DPU that you detect

@SamD2021
Copy link
Contributor Author

Hi Balazs, would you want the changes make a uniform way to handle all dpus in a seperate pr or would you want it here?
Sal and I were thinking that to do that it would be useful to reorganize the tools to build from the same dockerfile since we want to unify them under the dpu-tools script.

@bn222
Copy link
Owner

bn222 commented Oct 11, 2024

Agreed. Let's get this feature PR in, and then create a refactoring PR.

Here are my thoughts when I ran throught this as I was working on it: you will have a command to list all DPUs (similar to what I've added for the IPU). Then, since the console command for the IPU is a bit tricky, you will have a mandatory arg: acc/imc. You won't need that for the Marvell.

Also, please make your scripts work outside of the container too (write/clean the config file for minicom before/after as I'm doing for the IPU). iow: running in a container should be detail, not a requirement.

If you fix the style check errors, I will merge pr 25.

@SalDaniele looping in

This minicom module is useful for getting IMC connectivity in Ipu MeV
1.2
- added minicom_cmd to return the correct minicom command based on if
its an imc or the acc.
This can later be used for parts of the dpu-tools like in the console
command

- added pexpect_child_wait to be used in command functions that require
waiting for a specific pattern like enabling connectivity in the IMC,
this function is borrowed by the pxeboot module.

- added configure_minicom to fix the problem where the minicom can't
write any input because of Hardware Flow Control
- Modified and moved run into common_ipu:
This run command is able to both capture output and stream the output to
the screen since long commands like with flashing the ipu with dd, feed
back is essential to help the user not interrupt its task.
After a lot of trial and error this function needs threads since reading
from both stdout and stderr is inherently a blocking operation which
would halt the output until the end of the command. Likewise, passing
the shell=True argument to Popen worked better than parsing the command
with shlex since many commands run in fwutils requires piping commands
and ssh which with a shell, it's handled easily.

- Added a Version list constant to track the supported version which is
useful for the flags choices

- Moved the Result structure to common_ipu to be shared between modules

- Added get_current_version that ssh into the IMC and runs cat
/etc/issue.net and parses the version, returning it in a Result

- Added a minicom library for dpu-tools

Since minicom functionality seems to be useful to multiples components
in dpu-tools for mev 1.2, keeping it in the minicom.py helps share some
code.

- Added a download files function

It would be useful to be able to download a file in a temp location that
can be used to then flash onto the ipu, a nice way was using the
requests library, since we can have easier error handling, chunck large
downloads and etc.

- Added an extracting compressed tar function

Since we have a server already to serve the ISOs, hosting the IPU
firmware images on that server is a straight forward way to have a
repository. In that server there a good way to transferring the images
quickly is to have it compressed; so this function can take that
downloaded compressed tar and extract its contents to be used in the
flashing steps

- Added a function to find the ipu firmware images needed for each
flashing step

The ipu firmware files follow a specific structure, so this function
allows us to automate the process of getting the necessary bin files
depending on the version needed. Essentially we can use this file in
get_images to allow reset to infer what the files it needs depending on
the version given by get_version in fwutils

- Added a function to setup logging based on verbosity

Keeps the supporting modules in sync with fwutils to only display useful
output depending on the users needs.

- Added get_version where it ssh into the IMC and runs cat
  /etc/issue.net to parse its contents and return its result.

- Added a minicom version of get_version where it runs cat
  /etc/issue.net and parses it to find the current version that can be
run
  before IMC connectivity is enabled.
- Added a IPUFirmware class to encapsulate the fields that are often
shared between methods. There are fields necessary fields like the
imc_address, along with optional fields that can be interacted with
using flags such as the repo_url where the IPU firmware would be
downloaded from.

- Added should_run to track what steps will be performed to
conform with multiple versions being supported, such as enabling IMC
connectivity only being required in MeV 1.2

- Added reflash_ipu that detects the version it's currently running,
depending on the version, enables IMC connectivity, then gets the proper
images and flashes them.

- Added get_images to download and extract images and find the bin files
for the ssd and firmware recovery. This utilizes the functions in
common_ipu.
- added python3-pexpect to the image since to enable network
connectivity, minicom usage is required in MEV < 1.6

- added openssh-clients since fwutil realies heavily on ssh which is not
available in the base image

- added procps-ng so we can use pkill inside the container
- Moved Result class into common_ipu and imported Result from common_ipu

- Changed Run implementation and moved it to common_ipu and import it

- Imported get_current_version to be used with firmware reset

- Added logging to make the printing experience uniform based on
verbosity, also required for get_current_version

- Added the firmware subcommand "dpu-tools firmware" with the following
flags:
  + --imc-address
  + --repo-url
  + --verbose
  + --dry-run

- Added firmware_reset function to be used with the subcommand
"dpu-tools firmware reset"
- Added a firmware up subcommand "dpu-tools firmware up" with the
following flags:
  + --version
- This subcommand calls firmware up which passes the version to the
class and reflashes the ipu to that specific version.
- Added firmware_version which is a wrapper around get_current_version
and prints the result to the screen
- Added a firmware version subcommand "dpu-tools firmware version" that
calls the firmware_version function
…missing imports

Since the ipu specific things are in a subdirectory the current mypy.ini
doesn't automatically track its module.
Like with pxexpect, requests will also cause mypy to complain that there
is type-stub for requests.
… the parent dir

It currently doesn't know the new modules and library, and some of the
imports we already ignore since its in its own directory
@SamD2021
Copy link
Contributor Author

Just fixed the style errors, It should be passing the check now!

@SalDaniele
Copy link
Contributor

LGTM

@SalDaniele
Copy link
Contributor

@bn222 This is all set to merge.

@SamD2021 will follow up with the refactor to consolidate the tools

@bn222 bn222 merged commit b7715cb into bn222:main Oct 21, 2024
1 check passed
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