Refactor DPU Tools into a uniform interface #27
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
SamD2021
commented
Nov 8, 2024
SamD2021
commented
- All scripts have been either turned into modules, or been housed in existing modules.
- Merged Dockerfiles together.
- Added a new DPU Types enum class.
- Added DPU Hardware Detection.
- Refactored DPU Tools script into a class, since there were many variables that should be shared between functions
- Transformed pxeboot script into a module and refactored it into a class.
- Added firmware class for Bluefields into fwutils module.
- Made capture output the default for the run function in the common module.
- Most tools can be utilized with auto hardware detection
- Added Console check for DPU Hardware Type
This is in preparation for making a unified interface for dpu tools that is able to detected the hardware its running on an call its respective tools ipu: Remove IPU dir
Remove redundant fwversion Remove redundant fwup Remove redundant fwdefaults Remove listbf script The list bf script has been integrated into the list dpus command and function. Now we can use this information to auto detect hardware and have the tool behave accordingly depending on its hardware
Make the imports match the new structure
Many of these functions aren't necessarily tied to the ipu so moving it to the common module makes more sense
Matching imports to new structure
Since we want to unify the scripts behind dpu-tools we should also build the tools using the same Dockerfile
This enum keeps us with a structured way to track different DPU types. Previously we would just use hardcoded strings, but with this class we can even validate if the DPU Type is within our tracked types
Since DPU tools often relies passing the args to each function, it would be nice to have a data structure to bundle all of this in. Likewise it would be good if we could also keep track of what DPU type are working with.
This script is useful because it lets rshim run in the background, which the Bluefields DPUs need to console into them.
By making this class we can keep together related logic to reset, upgrade and version firmware across dpu types
Many of the Bluefield scripts requires capturing the output. It would be better to have it default to capturing output and have an option to turn it off when we think its not useful
Here we are using a single mode subcommand that can be used to retrieve the mode and set if passed the set-mode flag
SalDaniele
reviewed
Nov 12, 2024
There was a problem hiding this comment.
Overall, I am happy to merge if @bn222 approves.
From a user-perspective, I feel everything is working and intuitive (i.e. help for each subcommand provides good output).
From a maintainers persepctive, this could use a little more work to make it easy to extend this in the future for whatever DPUs we add support for moving forward.
|
I'm also happy with the changes. We will need to specify which dpu type we're using since we can't detect it. |
- Requires the user to provide the imc address, like with the firmware command, if the dpu-tools are in ipu mode. - The new reset command uses that imc address to call reboot on the imc.
- It either takes the dpu_type given by the user or auto detects it otherwise.
SamD2021
force-pushed
the
refactor_until_uniform
branch
from
c440661
to
178b02d
Compare
SamD2021
force-pushed
the
refactor_until_uniform
branch
from
b469ebb
to
ae6f8dc
Compare
|
Nice, I really like the changes, looks good to me. @bn222 can we merge this? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.