[Pkg-ofed-devel] Infiniband Kernel Module Initializer and port config tool
Talat Batheesh
talatb at mellanox.com
Tue May 31 08:56:30 UTC 2016
Hi Ana,
Thank you for your effort and assist, I finished all the points below.
The code hosted in the debian git ssh://git.debian.org/git/pkg-ofed/rdma-utils.git .
Could you please direct me to the next step (building a debian package and share it).
Many thanks.
Yours,
Talat
-----Original Message-----
From: Ana Guerrero Lopez [mailto:ana at debian.org]
Sent: Thursday, May 05, 2016 4:58 PM
To: Talat Batheesh <talatb at mellanox.com>
Cc: pkg-ofed-devel at lists.alioth.debian.org
Subject: Re: Infiniband Kernel Module Initializer and port config tool
Hi Talat,
On Mon, May 02, 2016 at 10:41:02AM +0000, Talat Batheesh wrote:
> Hi Ana
> Thank you for your detailed response.
> I'm sorry, our mail server is not working with plain text.
> please see below my response marked with [TB].
Could you please address all the points from my initial mail before asking for review? The licensing part is very important, copying the debian/copyright file over is not the way to go.
The ITP is fine, just make clear what would be the long description of the package and leave the rest as a longer explanation for those interested.
Ana
> -----Original Message-----
> From: Ana Guerrero Lopez [mailto:ana at debian.org]
> Sent: Wednesday, March 23, 2016 10:41 PM
> To: Talat Batheesh <talatb at mellanox.com>
> Cc: pkg-ofed-devel at lists.alioth.debian.org; Tal Alon
> <talal at mellanox.com>; Vladimir Sokolovsky <vlad at mellanox.com>; Brian
> Fromme <brian.fromme at canonical.com>; Hadar Hen Zion
> <hadarh at mellanox.com>; Yishai Hadas <yishaih at mellanox.com>; Alaa
> Hleihel <alaa at mellanox.com>
> Subject: Re: Infiniband Kernel Module Initializer and port config tool
>
>
> Hi Talat,
>
> Sorry for taking so much to reply but I wanted to take the time to reply thoroughly as there were plenty of things to point to. Also, before I continue, thank you for packaging this for Debian, something like this was definitively missing.
>
> On Sun, Jan 17, 2016 at 01:26:07PM +0200, Talat mellanox wrote:
> > Hi Ana and OFED and Debian Developement and Discussion team,
> >
> >
> > Till now, in Ubuntu and Debian Operation systems, rdma and
> > Infiniband modules loaded manually by the user, compared with other
> > operation systems there is a service that control them (load, unload, status, autoload .. ).
> >
> > This services is required and needed by Infiniband and RoCE users,
> > in order to load the Infiniband kernel modules/ configure ports in
> > VPI mode, the service already integrated in other operation system named rdma.
> >
> > We prepare the package that contain this tools in Debian format – attached .
> >
> > What do you think about adding this package to debian under the
> > umbrella of “OFED and Debian Development and Discussion team”
>
> > Could you please review and share it with the relevant reviewer ?
>
> So here goes a first review.
>
> I'm making my comments in two groups, one related to Debian processes about things that should be done independently of the package and another about the packaging itself (as in the files in debian/).
>
> # Processes
>
> * Code source: the source should be published somewhere. E.g. github or some Mellanox's *public* git repository. The URL later goes in the "Homepage" field in debian/control and in debian/copyright as well.
> [TB] Now we host the rdma-utils at Mellanox github
> https://github.com/Mellanox/rdma-utils
>
> I know this is packaged in RedHat already but in Debian we must specify clearly the origin.
> Some files have Copyright lines from RedHat, are they the original upstream?
> [TB] Right, it's the original.
>
> I see you distributed the same content that rdma-7.2_4.1_rc6-1.el7.src.rpm plus a new file rdma-initramfs-tools-hook that has no authorship or licensing.
> Is this file from Mellanox?
> [TB] it is from Mellanox, Added copyright.
>
>
> * License: this point is related with the point above. There is no doubt this is meant to be licensed under the GPL but it's not properly documented.
> It's missing a file with the full license. Only the files rdma.ifdown-ib and rdma.ifup-ib say they're under the GPL v2 only, and the spec says GPLv2+, this is usually interpreted as GPL version 2 or any later version, but it would be nice to have something more explicit.
>
> * Git: please, use pkg-ofed's git for hosting the packaging. This is independent to where you publish your source code. Notice we use some branches specially for integration with git-buildpackage.
> The repositories are at:
> http://anonscm.debian.org/gitweb/?a=project_list;pf=pkg-ofed
> Alioth's git help:
> https://wiki.debian.org/Alioth/Git
>
> I can create a empty git repository for you if needed.
> [TB] Once we finish the ITP, we will use the pkg-ofed host it.
[TB] Created a new empty git repository with the new name 'rdma-utils.git', could you please remove the old one.
>
>
> * Before packaging something for Debian, you should send an ITP. More information at:
> https://wiki.debian.org/ITP
> https://www.debian.org/devel/wnpp/
> [TB] Attached ITP draft.
[TB] ITP sent and waiting for a replay https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=823901.
>
>
> * Package name: Finally something that maybe is a bit nitpicking. I'm not sure that "rdma" is a good name for your package. I'm aware this is the RedHat name and this is certainly something subjective, but it makes more sense to me something like rdma-scripts or rdma-modules-scripts. I would interpret 'rdma'
> like a meta-package that installs everything related to rdma, which
> isn't the case. I would welcome more opinions on this from other lurkers in the list :)
[TB] Agree with you, changed to rdma-utils.
>
>
> # Packaging
>
> * debian/changelog:
> It closes bug #676403 ( https://bugs.debian.org/676403 ) that's
> unrelated to this package or OFED. It should close the bug of the ITP mentioned earlier.
> [TB] will remove this lines when the ITP accepted.
>
> * debian/compat:
> It says 8, these days we're in debhelper compatibility level 9.
> [TB] Will change to 9
>
> * debian/control:
> To work, your package needs a set of packages installed. Maybe some of
> them should be in Depends, Recommends or Suggests. Please take a look at this.
> Basically, you should make sure your package shouldn't fail if no
> other ofed packages are installed.
[TB] fortunately, there is no dependences.
> The long description is too short, it should be quite more longer and
> explain clearly what this package does.
> [TB] attached a ITP .
>
> * debian/copyright:
> The source is missing, it can't point to a RedHat package, it must be
> at the very least a git repository.
> [TB] Added.
>
> The copyright is incomplete, e.g. this is missing:
> rdma.ifdown-ib:# Copyright (c) 1996-2013 Red Hat, Inc. all rights reserved.
> rdma.ifup-ib:# Copyright (c) 1996-2013 Red Hat, Inc. all rights reserved
> [TB] Added.
> All the boilerplate from the template (those line starting with #)
> should be removed.
>
> Finally, see the licensing issue I mentioned above.
>
> * debian/postinst debian/postrm debian/prerm
>
> You won't need those files if you install the systemd unit using the
> dh systemd helper.
[TB] removed the postinst and postrm, the prerem still due to package not support stop option.
> And if you were to use those files, they're missing #DEBHELPER# in the
> end of the file
>
> * debian/rules
>
> You define pversion but you don't use it later.
>
> Have you tried installing the files using a makefile or a
> debian/install file instead of doing it by hand in the debian/rules?
[TB] yes we test it.
> For installing the systemd unit there is also a special dh helper,
> don't do it by hand.
> Read: https://wiki.debian.org/Teams/pkg-systemd/Packaging
[TB] Done.
> Why are you overriding the conffiles installation?
> Files in /etc must be marked conffiles if they are included in a
> package. Otherwise they should be created by maintainer scripts.
> Refer to Debian Policy Manual section 10.7 (Configuration files) for details.
They are defined as conffiles see below
# apt-cache show rdma-utils
Package: rdma-utils
Status: install ok installed
Priority: optional
Section: utils
Installed-Size: 67
Maintainer: Talat Batheesh <talatb at mellanox.com>
Architecture: amd64
Version: 4.1-1
Conffiles:
/etc/rdma/rdma.conf cc042504f04ade59635be71f92e1388a
/etc/rdma/mlx4.conf a06176747bd47a53781f1b53e00b8171
/etc/rdma/sriov-vfs 169118e0e1ebe9c6b0f936a8b0847ac6
> * debian/patches
> It would be good if the patches have the authorship and a description
> of why they're needed.
>
[TB] agree, will add the patches from now on.
> * files installed
>
> Very important, your package shouldn't modify files installed by
> another package in any way, it's forbidden by the Debian policy.
[TB] the package didn't modify any file that installed another one.
> The package ships a udev rule and installs it under /etc/udev/rules.d,
> which is reserved for user-installed files. The correct directory for
> system rules is /lib/udev/rules.d.
>
[TB] Done.
> The *.sh files the package installs shouldn't include the extension ".sh"
[TB]Done.
> The scripts you're installing are also missing a manpage.
>
> Finally, a read of the maintainer's guide:
> https://www.debian.org/doc/manuals/maint-guide/
> is strongly encouraged. It would help to understand better the
> mentioned issues and have pointers to further documentation when needed.
>
> Please, don't hurry up, and take time to go through this list before
> pushing a new version of the package!
>
> I didn't build your package or test it, so there will be probably more
> issues.
>
> What do you mean with the relevant reviever?
>
>
> > - The package is based on rdma package from other Operation system.
>
> Yes, I have seen it's from RedHat.
>
> > - Currently only systemd is supported.
>
> That's ok
>
> > - Do we need to support upstart as well ?
>
> No.
>
> > - How do we publish the code git tree somewhere ?
>
> You can use github. Your company seems to be there https://github.com/Mellanox ?
> There is also http://git.openfabrics.org/ And for the packaging,
> debian's git.
>
[TB] Done.
> > - [very important AR] The conf file “/etc/modprobe.d/mlx4.conf”
> > (that comes with
> > “kmod” package) is not needed anymore, and must be removed.
> > This conf file conflicts with “/lib/modprobe.d/libmlx4.conf”
> > that will come with
> > “rdma” package, and prevents running a script (/sbin/mlx4-setup.sh) that
> > configures ConnectX-3 devices port types (IB vs. ETH).
> >
>
> Which kmod package? Debian's kmod doesn't include a file
> /etc/modprobe.d/mlx4.conf If your package conflicts with another package, it should have a Conflicts:
> line.
>
> I hope all this helps.
> Ana
>
>
> Subject: ITP: rdma-utils -- Infiniband Kernel Module Initializer and
> port config tool
> Package: wnpp
> Owner: Talat Batheesh <talatb at mellanox.com>
> Severity: wishlist
>
> * Package name : rdma-utils
> Version : 4.1.1
> Upstream Author : Doug Ledford <dledford at redhat.com>
> * URL : https://github.com/Mellanox/rdma-utils
> * License : GPLv2+
> Programming Lang: Bourne Shell, Bash
> Description : Infiniband Kernel Module Initializer and port config tool
>
> Overview
> rdma-utils package allows InfiniBand users to manage user-level RDMA
> protocols that want to be loaded by default, change port type to
> Ethernet or InfiniBand and manage the SR-IOV for Mellanox ConnectX family adapret cards.
> The rdma service is user space initialization scripts for the kernel
> InfiniBand /iWARP drivers.
> It reads the /etc/rdma/rdma.conf file to find out which kernel-level
> and user-level RDMA protocols the administrator wants to be loaded by
> default. This file can be modified to turn various drivers ON or OFF.
> The various drivers that can be enabled and disabled are:
> ?? IPoIB - an IP network emulation layer that allows IP applications to run
> over InfiniBand networks.
> ?? SRP - the SCSI Request Protocol. It allows a machine to mount a remot
> drive or drive array that is exported via the SRP protocol on the
> machine as though it were a local hard disk.
> ?? SRPT - the target mode, or server mode, of the SRP potocol. This load
> the kernel support necessary for exporting a drive or drive array for
> other machines to mount as though it were local on their machine.
> Further configuration of the target mode support is required before any
> devices will actually be exported.
> ?? iSER - a low-level driver for the general iSCSI layer of the Linux kerne
> that provides transport over InfiniBand networks for iSCSI devices.
> ?? RDS - the Reliable Datagram Service in the Linux kernel.
>
> Starting up the rdma service is done automatically. When RDMA capable
> hardware, whether InfiniBand or iWARP or RoCE/IBoE is detected, udev
> instructs systemd to start the rdma service. The rdma service can be
> enabled to be set as ON all the time but it is not mandatory to do so.
> The rdma package provides the
> /etc/udev.d/rules.d/70-persistent-ipoib.rules file which is used to
> rename IPoIB devices default names (such as ib0 and ib1) to a more
> descriptive names. Users must edit this file to change how their devices are named.
> This package will be maintained under the umbrella of "OFED and Debian
> Development and Discussion team", project name "OFED Infiniband Distribution".
>
>
> * Configuring Mellanox Cards Ports for Ethernet Operation
>
> Certain Mellanox ConnectX adapter cards (which use the mlx4 driver)
> can be configured to run in either InfiniBand or Ethernet mode (default InfiniBand).
> To set the mode:
> 1. Find the right PCI device ID of your adapter card. For information
> on how to, follow the instruction in the file /etc/rdma/mlx4.conf.
> 2.Create a line in the file using that device ID and the requested port
> type.
> 3.Rebuild the initramfs to make sure the updated port settings are
> copied into the initramfs.
>
> * RDMA Configuration File(s)
>
> The rdma service loads the configuration from /etc/rdma/rdma.conf file
> which controls which modules will be loaded during the service startup
> and some attributes about the RDMA modules. The following parameters are supported:
> +------------------+----------------------------------+-----------------------+
> | Parameter name | Description | Supported values |
> +-----------------------------------------------------------------------------+
> |IPOIB_LOAD | Load IPoIB module | yes/no |
> +-----------------------------------------------------------------------------+
> |SRP_LOAD | Load SRP initiator module | yes/no |
> +-----------------------------------------------------------------------------+
> |SRPT_LOAD | Load SRP target module | yes/no |
> +-----------------------------------------------------------------------------+
> |ISER_LOAD | Load ISER initiator module | yes/no |
> +-----------------------------------------------------------------------------+
> |RDS_LOAD | Load RDS module | yes/no |
> +-----------------------------------------------------------------------------+
> |FIXUP_MTRR_REGS | Modify the system mtrr registers | yes/no |
> +------------------+----------------------------------+-----------------------+
>
>
> * Starting the RDMA Services
>
> Loading/unloading the RDMA drivers is done using the Infiniband Kernel
> Module Initializer and port config tool.
> - To load/unload the rdma service:
> [root at localhost]# systemctl start/stop rdma.service
> - To start the rdma service automatically when the operating system is
> loaded:
> [root at localhost]# systemctl enable rdma
>
More information about the Pkg-ofed-devel
mailing list