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

T6231: MFT debs #777

Closed
wants to merge 1 commit into from
Closed

Conversation

sempervictus
Copy link

Per comment by Nicolas Vollmar - MFT userspace tools are needed by some of the MLX functionality.

Add download and unpack directives to the OFED build script which should pull and extract the Debian package from MLX/NV's site into the current working directory which will contain the built DEBs after completion of the script for inclusion into the image build.

Testing:
None yet, quickly put together to address user comment, would
apperciate QA by someone who has time as i wont be able to do this until the weekend at the earliest.

Change Summary

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes)
  • Migration from an old Vyatta component to vyos-1x, please link to related PR inside obsoleted component
  • Other (please describe):

Related Task(s)

Component(s) name

Proposed changes

How to test

  1. Build from source
  2. Deploy on system w/ CX5+ cards
  3. Verify MFT tools work on the Connect-X cards.

Checklist:

  • I have read the CONTRIBUTING document
  • I have linked this PR to one or more Phabricator Task(s)
  • My commit headlines contain a valid Task id
  • My change requires a change to the documentation
  • I have updated the documentation accordingly

Per comment by Nicolas Vollmar - MFT userspace tools are needed
by some of the MLX functionality.

Add download and unpack directives to the OFED build script which
should pull and extract the Debian package from MLX/NV's site into
the current working directory which will contain the built DEBs
after completion of the script for inclusion into the image build.

Testing:
  None yet, quickly put together to address user comment, would
apperciate QA by someone who has time as i wont be able to do this
until the weekend at the earliest.
Copy link

github-actions bot commented Oct 1, 2024

👍
No issues in PR Title / Commit Title

@nvollmar
Copy link
Contributor

nvollmar commented Oct 1, 2024

I'm doing a test run with those changes

@nvollmar
Copy link
Contributor

nvollmar commented Oct 2, 2024

The code added works, but ends up with the deps in mft-4.29.0-131-x86_64-deb/DEBS/, also the file owner seems wrong.

Currently I fail building the image at

N: Download is performed unsandboxed as root as file '/root/packages/./mlnx-ofed-kernel-utils_24.07.OFED.24.07.0.6.1.1-1.kver.6.6.32-amd64-vyos_amd64.deb' couldn't be accessed by user '_apt'. - pkgAcquire::Run (13: Permission denied)
E: Sub-process /usr/bin/dpkg returned an error code (1)
E: An unexpected failure occurred, exiting...
P: Begin unmounting filesystems...
P: Saving caches...

Copy link
Member

@sever-sever sever-sever left a comment

Choose a reason for hiding this comment

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

Copy link

github-actions bot commented Oct 2, 2024

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@sempervictus
Copy link
Author

Jenkins is deprecated, use please this path for changes https://github.com/vyos/vyos-build/blob/current/scripts/package-build/linux-kernel/build-mellanox-ofed.sh

Humbug, I'm lagging on reviews; apologies.

Any idea what I should be using for owner and permissions on the deb?

@nvollmar
Copy link
Contributor

nvollmar commented Oct 2, 2024

the other built packages belong to root:root.
after extracting I see this:

ls -alh mft-4.29.0-131-x86_64-deb/DEBS
total 57M
drwxr-xr-x 2 70705 dip  4.0K Aug  7 17:30 .
drwxr-xr-x 4 root  root 4.0K Oct  2 17:28 ..
-rw-r--r-- 1 70705 dip  9.1K Aug  7 17:30 mft-autocomplete_4.29.0-131_amd64.deb
-rw-r--r-- 1 70705 dip  5.9M Aug  7 17:30 mft-oem_4.29.0-131_amd64.deb
-rw-r--r-- 1 70705 dip   54K Aug  7 17:30 mft-pcap_4.29.0-131_amd64.deb
-rw-r--r-- 1 70705 dip   51M Aug  7 17:30 mft_4.29.0-131_amd64.deb

Copy link
Member

@dmbaturin dmbaturin left a comment

Choose a reason for hiding this comment

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

Folks, it looks like we simply cannot include that package in our images — that would violate the Mellanox EULA!

Here is the relevant part of LICENSE.txt included in the tarball:

1. General
   Each copy of the Software is intended for use only in conjunction 
   with Mellanox’s hardware products (“Mellanox Products”) 
   and is subject to the terms of this Agreement.

2. Grant of License
   Subject to the terms and conditions of this Agreement, Mellanox 
   grants you a personal, non-exclusive, non-transferable license 
   to use, view, copy, print, and distribute software and 
   accompanying documentation subject to the following conditions:
   (i) The Software and any accompanying documentation may 
       be used solely to support Mellanox Products;     
   (ii)You may distribute the Software and accompanying documentation 
       solely as integrated with or installed on Your products that 
       incorporate the Mellanox Products.

After "resolving macros", that's:

You may distribute the Software and accompanying documentation
solely as integrated with or installed on Your products that incorporate Mellanox's hardware products.

As I understand it, it's only legal to redistribute the package in an OS preinstalled (or at least meant for) hardware boxes that include Mellanox cards in them. Distributing them in a general-purpose OS that is meant to run on boxes that don't have any Mellanox hardware in them would violate those conditions.

@dmbaturin
Copy link
Member

I got a confirmation from the legal team about the concerns — this PR would indeed violate the Mellanox EULA and cannot be merged.

@Apachez-
Copy link
Contributor

What does Nvidia themselves say about that?

I interpret that as the software from Nvidia can only be used with Nvidia products which is kind of obvious since a Intel driver for lets say igb doesnt work with any other NIC.

Another workaround could perhaps be to add this as a VyOS module/package that could be installed afterwards by the admin in case this cannot be part of the ISO itself?

@nvollmar
Copy link
Contributor

Currently they can't even be manually installed because the ISO is lacking some dependencies needed by the OFED driver packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants