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

[Feature] qsfpinfo: New tool to print QSFP EEPROM information #3119

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

Conversation

rchriste1
Copy link
Contributor

[Feature]- qsfpinfo: New tool to print QSFP EEPROM information for modules in cards with qsfp feature (id 0x13).

Layout is imported from ethertool repo.
Imported c-files are wrapped to undefine HAVE_CONFIG_H to avoid importing more files.

------------- Keep everything below this line -------------------------

Description

Describe the issue, update, change or fix and why

Collateral (docs, reports, design examples, case IDs):

  • Document Update Required? (Specify FIM/AFU/Scripts)

Tests added:

Tests run:

@rchriste1 rchriste1 requested review from a team as code owners April 15, 2024 11:20
@coveralls
Copy link

coveralls commented Apr 15, 2024

Pull Request Test Coverage Report for Build 8851240764

Details

  • 0 of 164 (0.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.4%) to 64.22%

Changes Missing Coverage Covered Lines Changed/Added Lines %
binaries/qsfpinfo/main.c 0 164 0.0%
Totals Coverage Status
Change from base Build 8818231667: -0.4%
Covered Lines: 15825
Relevant Lines: 24642

💛 - Coveralls

Comment on lines 44 to 56
include(ExternalProject)

ExternalProject_Add(
ethtool_project
GIT_REPOSITORY "https://git.kernel.org/pub/scm/network/ethtool/ethtool.git"
GIT_TAG "v6.7"
CONFIGURE_COMMAND ""
BUILD_COMMAND ""
INSTALL_COMMAND ""
UPDATE_COMMAND ""
)

ExternalProject_Get_Property(ethtool_project SOURCE_DIR)
Copy link
Contributor

Choose a reason for hiding this comment

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

This will fail in a build sandbox without network access, e.g., when building for Fedora or Yocto. Is there any way to obtain the required functionality from a library?

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 code used is normally compiled into the ethertool binary. The qsfpinfo program calls a static function internally in the code. So there is no library providing that interface. Since the dfl drivers do not create a Linux network interface we cannot use ethertool directly.

ExternalProject_Add() is also used in other places in opae-sdk.

Copy link
Contributor

Choose a reason for hiding this comment

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

The code used is normally compiled into the ethertool binary. The qsfpinfo program calls a static function internally in the code. So there is no library providing that interface. Since the dfl drivers do not create a Linux network interface we cannot use ethertool directly.

Do you know why the DFL drivers do not create a Linux netlink interface that could be consumed by regular ethtool or libnl? Are there any fundamental blockers to implementing this or has it simply not been considered necessary so far?

ExternalProject_Add() is also used in other places in opae-sdk.

Please correct me if I am wrong, but OPAE SDK does not have any mandatory external dependencies.

opae-sdk$ rg ExternalProject_Add
CMakeLists.txt
110:    ExternalProject_Add(uuid
378:        ExternalProject_Add(libedit
492:        ExternalProject_Add(hwloc
598:        ExternalProject_Add(numa

Those are libraries provided by the distribution, hence are not needed to build in an isolated sandbox. In the worst case, the qsfpinfo tool could be disabled by default, but I am wondering if there is a better solution that allows using rather than forking ethtool.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@matthew-gerlach may no more about why the drivers do not implement "a Linux netlink interface". The qsfp-mem driver only implements QSFP handling. I know that the HSSI could not be accepted by Linux community as a network interface because you cannot send and receive packets. So HSSI is handled in user space.

Copy link
Contributor

Choose a reason for hiding this comment

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

To be a valid, accepted "Linux netlink interface", the device must transmit and receive packets between the network connection and memory. The HSSI and QSFP subsystems are just pieces of the pipeline of full netlink device.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have made the printing of QSFP information, thus importing from Ethertool, dependent on cmake option OPAE_WITH_QSFPINFO_QSFPPRINT, which is OFF by default. qsfpinfo will thus by default only print if the QSFP modules are present or not. In this way the qsfpinfo binary is always part of the packages built.

…fp feature (id 0x13).

Signed-off-by: Roger Christensen <rc@silicom.dk>
… ethertool) if OPAE-SDK is compiled with option OPAE_WITH_QSFPINFO_QSFPPRINT

Signed-off-by: Roger Christensen <rc@silicom.dk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants