Mstflint package - add RoCE disable support

fixed in focal

The problem is that mlxreg is not in 4.8 (18.04) nor 4.11 (19.10), so it's not a simple backport.. or am I missing something?

since this needs a bigger backport than originally expected, I'll unassign myself from the bug

Thanks Timo. We could argue for introducing mlxreg in an SRU as being hardware enablement (SRU team would be the judge), as long as we could clearly demonstrate a negligible regression risk in doing so. I took a stab at a backport to gauge that feasibility. Unfortunately, I found that this is non-trivial. I identified the following set of commits as touching mlxreg, or other code that mlxreg introduced (e.g. adb_parser):

e425ca0 Porting mlxreg - with bug fixes
844101e Fix build issues with expat and boost - bugs 1732270 and 1732354
349d693 Update Fix build issues with expat and boost - bugs 1732270 and 1732354
00fe460 Update Makefile.am
81ab091 Fix format security issue for ununtu
27762b9 Fix issues in catching exceptios and add mstreg in specs
5f78cec Fix issues in adb_parser and fw utilities - 1751157 and 1751189
015de1f Title: Porting from MFT to mstflint
bbc94a6 Title: mlxreg and fwtrace porting fix
60da93e Title: Removed openssl dependency from adb generic tools.
0b0e962 Porting from MFT to mstflint
4470808 Fixed mlxreg behaviour with autogenerated subnodes without access attr
d27d866 Fix mstreg access type field display
1d2cc0c Globally disable RoCE through mst

Some of these commits touch code all over the place w/o providing a technical explanation other than "porting from MFT". Some also add build and runtime dependencies (e.g. boost libraries) that would need to be vetted. While I'm not at a point where I would say "not possible", it would certainly require some engineering.

While trying to create a backport of mlxreg - without touching other parts of mstflint - I hit an incompatibility. See below. Does mlxreg require the new version of mget_max_reg_size(), or can it be made to work with the 4.8 version? The supports_reg_accces_gmp() interface will likely need to be added as well - I haven't checked to see how hairy that is. fyi, my working tree is at: https://git.launchpad.net/~dannf/+git/mstflint?h=master

libtool: compile: g++ -DHAVE_CONFIG_H -I. -I.. -I. -I.. -I../ -I -I../mft_utils -I../include/mtcr_ul -Wall -W -DMST_UL -g -MP -MD -pipe -Werror -g -O2 -DHAVE_TERMIOS_H -DMST_UL -Wno-implicit-fallthrough -Wno-format-overflow -MT mlxreg_lib.lo -MD -MP -MF .deps/mlxreg_lib.Tpo -c mlxreg_lib.cpp -fPIC -DPIC -o .libs/mlxreg_lib.o
mlxreg_lib.cpp: In member function ‘bool mlxreg::MlxRegLib::isRegSizeSupported(std::string)’:
mlxreg_lib.cpp:330:94: error: too many arguments to function ‘int mget_max_reg_size(mfile*)’
  330 | ) <= (u_int32_t)mget_max_reg_size(_mf, MACCESS_REG_METHOD_SET)) ||
      | ^

In file included from mlxreg_lib.h:39,
                 from mlxreg_lib.cpp:36:
../include/mtcr_ul/mtcr.h:152:5: note: declared here
  152 | int mget_max_reg_size(mfile *mf);
      | ^~~~~~~~~~~~~~~~~
mlxreg_lib.cpp:331:94: error: too many arguments to function ‘int mget_max_reg_size(mfile*)’
  331 | ) <= (u_int32_t)mget_max_reg_size(_mf, MACCESS_REG_METHOD_GET)));
      | ^

In file included from mlxreg_lib.h:39,
                 from mlxreg_lib.cpp:36:
../include/mtcr_ul/mtcr.h:152:5: note: declared here
  152 | int mget_max_reg_size(mfile *mf);
      | ^~~~~~~~~~~~~~~~~
mlxreg_lib.cpp: In member function ‘bool mlxreg::MlxRegLib::isAccessRegisterGMPSupported(maccess_reg_method_t)’:
mlxreg_lib.cpp:374:19: error: ‘supports_reg_access_gmp’ was not declared in this scope
  374 | return (bool)(supports_reg_access_gmp(_mf, reg_method));
      | ^~~~~~~~~~~~~~~~~~~~~~~
make[2]: *** [Makefile:607: mlxreg_lib.lo] Error 1

Actually, it turns out mstreg isn't built in the focal/eoan versions of mstflint either - it must be explicitly enabled at ./configure time.

This bug was fixed in the package mstflint - 4.13.3+2-1ubuntu1

---------------
mstflint (4.13.3+2-1ubuntu1) focal; urgency=medium

  * Enable mstreg command while continuing to not build mstlink due to
    libjson-c-dev incompatibility. LP: #1864475.
  * Remove unused libjson-c-dev build-dep.
  * Feature Freeze exception granted. LP: #1868174.

 -- dann frazier <email address hidden> Thu, 19 Mar 2020 11:14:35 -0600

While we are seeking to fix this in bionic/eoan by backporting a new upstream version (bug 1869441), note that it would have also been possible to backport the mstreg command (and supporting infrastructure) to bionic's 4.8 version. See:
  https://salsa.debian.org/dannf/mstflint/-/commit/261d0e650b711101d72eb2a651f3270a7cc55bb2

Hello Mohammad, or anyone else affected,

Accepted mstflint into eoan-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/mstflint/4.13.3+2-2~ubuntu19.10.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-eoan to verification-done-eoan. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-eoan. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

Hello Mohammad, or anyone else affected,

Accepted mstflint into bionic-proposed. The package will build now and be available at https://launchpad.net/ubuntu/+source/mstflint/4.13.3+2-2~ubuntu18.04.1 in a few hours, and then in the -proposed repository.

Please help us by testing this new package. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation on how to enable and use -proposed. Your feedback will aid us getting this update out to other Ubuntu users.

If this package fixes the bug for you, please add a comment to this bug, mentioning the version of the package you tested, what testing has been performed on the package and change the tag from verification-needed-bionic to verification-done-bionic. If it does not fix the bug for you, please add a comment stating that, and change the tag to verification-failed-bionic. In either case, without details of your testing we will not be able to proceed.

Further information regarding the verification process can be found at https://wiki.ubuntu.com/QATeam/PerformingSRUVerification . Thank you in advance for helping!

N.B. The updated package will be released to -updates after the bug(s) fixed by this package have been verified and the package has been in -proposed for a minimum of 7 days.

It has been brought to my attention the following by someone who tested mstflint package in bionic-proposed pocket:

"We have tested the install and function of the new package and have found no issues with it. The update performs as we require."

- Eric

Mellanox has tested the mstflint package and has successfully completed our QA test process without issue.

Our testing included:

- Manual and automation testing for mstflint package.
- 120 test cases were assigned and updated to check and test mstflint part of inbox.
- RoCE disable feature
- The following tools where tested :
 mstconfig mstflint mstfwreset mstfwtrace mstmcra mstmread mstmwrite mstregdump mstrecourcedump mstvpd mstreg

Thanks Jim,

Can you clarify if you perform the testing on both releases ?
Bionic/18.04LTS and Eoan/19.10.

- Eric

Eric,

Yes, we successfully tested both.

jim

-----Original Message-----
From: <email address hidden> <email address hidden> On Behalf Of Eric Desrochers
Sent: Friday, April 10, 2020 12:00 PM
To: Jim Michelson <email address hidden>
Subject: [Bug 1864475] Re: Mstflint package - add RoCE disable support

Thanks Jim,

Can you clarify if you perform the testing on both releases ?
Bionic/18.04LTS and Eoan/19.10.

- Eric

--
You received this bug notification because you are subscribed to the bug report.
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.launchpad.net%2Fbugs%2F1864475&amp;data=02%7C01%7Cjmichelson%40mellanox.com%7C05a2d414be8b4b7521dd08d7dd690bfd%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637221315551593918&amp;sdata=DkQWc0oUcHsDgkvjFbHjePI7OSbvwZzpX8MatfFHyL8%3D&amp;reserved=0

Title:
  Mstflint package - add RoCE disable support

Status in mstflint package in Ubuntu:
  Fix Released
Status in mstflint source package in Bionic:
  Fix Committed
Status in mstflint source package in Eoan:
  Fix Committed
Status in mstflint package in Debian:
  Fix Released

Bug description:
  [Impact]

  RoCE disable feature was added to the last upstream Mstflint package.
  This feature was requested by Mellanox customers that use Ubuntu 18.04,
  and it's a very high important to deliver this feature to the customers
  in one of ubuntu 18.04 SRU.

  [Test Case]

  use the feature requested above to enable/disable Roce.

  [Regression Potential]

  This feature shouldn't affect the regression because it's only adding support for
  RoCE enable/disable.
  Also,
  This feature was tested internally by Mellanox QA teams
  those tests logs/results are private unfortunately i can't share it here

  [Other Info]

  The feature support added to mstflint by the following Mstflint patch:

  1d2cc0ccbc8da9701078aeabac0454b8998f11b8 Globally disable RoCE through
  mst

  Link:
  https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FMellanox%2Fmstflint%2Fcommit%2F1d2cc0ccbc8da9701078aeabac0454b8998f11b8&amp;data=02%7C01%7Cjmichelson%40mellanox.com%7C05a2d414be8b4b7521dd08d7dd690bfd%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637221315551593918&amp;sdata=svqZgxXvG7HyIDOgA56Jt538RcBBggQivMfHCbWVZhA%3D&amp;reserved=0

To manage notifications about this bug go to:
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.launchpad.net%2Fubuntu%2F%2Bsource%2Fmstflint%2F%2Bbug%2F1864475%2F%2Bsubscriptions&amp;data=02%7C01%7Cjmichelson%40mellanox.com%7C05a2d414be8b4b7521dd08d7dd690bfd%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637221315551593918&amp;sdata=DCrX0Lx3Nfd6sv9Z92XFSzNhMSJFNEIB8EZI1E21uhc%3D&amp;reserved=0

The verification of the Stable Release Update for mstflint has completed successfully and the package is now being released to -updates. Subsequently, the Ubuntu Stable Release Updates Team is being unsubscribed and will not receive messages about this bug report. In the event that you encounter a regression using the package from -updates please report a new bug using ubuntu-bug and tag the bug report regression-update so we can easily find any regressions.

This bug was fixed in the package mstflint - 4.13.3+2-2~ubuntu18.04.1

---------------
mstflint (4.13.3+2-2~ubuntu18.04.1) bionic; urgency=medium

  * Backport to 18.04 (LP: #1869441):
    - Revert to debhelper-compat 11; no changes needed.
    - Adds mstreg command, which allows for disabling RoCE (LP: #1864475)

 -- dann frazier <email address hidden> Fri, 27 Mar 2020 16:18:02 -0600

This bug was fixed in the package mstflint - 4.13.3+2-2~ubuntu19.10.1

---------------
mstflint (4.13.3+2-2~ubuntu19.10.1) eoan; urgency=medium

  * Backport to 18.04 (LP: #1869441):
    - Adds mstreg command, which allows for disabling RoCE (LP: #1864475)

 -- dann frazier <email address hidden> Fri, 27 Mar 2020 16:26:24 -0600