rdma-core in Dabian

rdma-core in Dabian

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 178 bytes --]

Hi Benjamin,

It looks like, we fixed all outstanding reviews comments
for inclusion of rdma-core into Debian.

How can we move forward and see rdma-core part of Debian?

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: rdma-core in Dabian

[Benjamin Drung]

[-- Attachment #1: Type: text/plain, Size: 3400 bytes --]

Hi Leon,

Am Sonntag, den 07.05.2017, 09:43 +0300 schrieb Leon Romanovsky:
> Hi Benjamin,
> 
> It looks like, we fixed all outstanding reviews comments
> for inclusion of rdma-core into Debian.
> 
> How can we move forward and see rdma-core part of Debian?

I found some time to continue the review of the source package merge. I
finally reviewed all changes from the 11 source packages to rdma-core.
The review is done except for the debian/copyright file regarding the
upstream part (i.e. the copyright for all files outside of debian/).
The resulting changes from the review can be found in the pull request 
https://github.com/linux-rdma/rdma-core/pull/128

Remaining topics to address:

* review copyright (by me)

* ibacm: Required-Start on openibd (see separate post)

* rdma-ndd shipped by infiniband-diags and rdma-core (see separate
post)

* srp_daemon: Disallow all targets if not explicitly allowed by
default (see separate post)

* Can we upstream some redhat files, i.e. move them out of the redhat
directory and maintain them in their corresponding code? Following
files fall in this category:
** ibacm.service
** srp_daemon.service
** rdma service (see next point)

* Can we provide an upstream rdma-core "package" that contains the rdma
service and the following files from the redhat directory?
** rdma.conf
** rdma.kernel-init
** rdma.service
** rdma.udev-rules

* Fix lintian issues:

  I: rdma-core: extended-description-is-probably-too-short
  I: iwpmd: extended-description-is-probably-too-short

Patches for improved descriptions are welcome.

  W: iwpmd: init.d-script-missing-start etc/init.d/iwpmd 2 4

Any objections to add 2 and 4?

  I: srptools: init.d-script-does-not-implement-optional-
     option etc/init.d/srptools status

I can write that if you decide not to consolidate the srp daemon (see
bonus point below)

  W: ibverbs-providers: package-name-doesnt-match-sonames libmlx5-1

I think we should just ignore this warning since using libmlx5-1 is
just one part of ibverbs-providers that shouldn't be use alone, should
it?

  W: ibverbs-providers: non-dev-pkg-with-shlib-symlink usr/lib/x86_64-
     linux-gnu/libmlx5.so.1.1.14 usr/lib/x86_64-linux-gnu/libmlx5.so

This libmlx5.so symlink should be part of a development package. Should
I add a new binary libmlx5-dev package or should it be moved to
libibverbs-dev (where already the header files are)?

* Bonus points: consolidate the srp daemon. Debian ships a different
service file than upstream, but I am against an additional layer
introduced by srp_daemon.sh. It would also be nice to have a systemd
service shipped by upstream (and not just in the redhat directory)


Once these points are addressed (and in case I found no new stuff), I
will upload the package to Debian experimental since Debian is in
freeze. And no, we are months too late for Debian 9 (stretch). The
packaging doesn't use the latest stuff to allow a no-change backport to
Debian 8 and 9.

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
Web: https://www.profitbricks.com

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Achim Weiss.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: rdma-core in Dabian

[Bart Van Assche]

On Tue, 2017-05-09 at 19:43 +0200, Benjamin Drung wrote:
> Am Sonntag, den 07.05.2017, 09:43 +0300 schrieb Leon Romanovsky:
> * Can we upstream some redhat files, i.e. move them out of the redhat
> directory and maintain them in their corresponding code? Following
> files fall in this category:
> ** ibacm.service
> ** srp_daemon.service
> ** rdma service (see next point)

One of the design goals of the systemd project was to avoid that different
service files would be needed for different distro's. So I think it's weird
that these service files exist in the redhat/ directory.

>   I: srptools: init.d-script-does-not-implement-optional-
>      option etc/init.d/srptools status
> 
> I can write that if you decide not to consolidate the srp daemon (see
> bonus point below)

I will submit a patch.

> * Bonus points: consolidate the srp daemon. Debian ships a different
> service file than upstream, but I am against an additional layer
> introduced by srp_daemon.sh. It would also be nice to have a systemd
> service shipped by upstream (and not just in the redhat directory)

I will have a look at this too.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Jason Gunthorpe]

On Tue, May 09, 2017 at 07:43:09PM +0200, Benjamin Drung wrote:

> * Can we upstream some redhat files, i.e. move them out of the redhat
> directory and maintain them in their corresponding code? Following
> files fall in this category:

Anything in the redhat directory could be moved out if it is going to
be used by another distro. It is there waiting for other distros to
look at it.

> * Can we provide an upstream rdma-core "package" that contains the rdma
> service and the following files from the redhat directory?
> ** rdma.conf
> ** rdma.kernel-init
> ** rdma.service
> ** rdma.udev-rules

Do you mean adding a 'rdma-core' package to debian/?

We could move the relevant stuff out of redhat/ and into, say,
kernel-boot/ or something.

IMHO there is no reason not to do that..

> Patches for improved descriptions are welcome.
> 
>   W: iwpmd: init.d-script-missing-start etc/init.d/iwpmd 2 4
> 
> Any objections to add 2 and 4?

Debian and RH6 are the only places I know of that use init.d scripts,
so I think most changes Debian needs would be fine.

>   W: ibverbs-providers: package-name-doesnt-match-sonames libmlx5-1
> 
> I think we should just ignore this warning since using libmlx5-1 is
> just one part of ibverbs-providers that shouldn't be use alone, should
> it?

Probably.. It is has become a bit odd with mlx5 being directly
linkable now..

>   W: ibverbs-providers: non-dev-pkg-with-shlib-symlink usr/lib/x86_64-
>      linux-gnu/libmlx5.so.1.1.14 usr/lib/x86_64-linux-gnu/libmlx5.so
> 
> This libmlx5.so symlink should be part of a development package. Should
> I add a new binary libmlx5-dev package or should it be moved to
> libibverbs-dev (where already the header files are)?

It should be with the headers, so I suggest libibverbs-dev

> * Bonus points: consolidate the srp daemon. Debian ships a different
> service file than upstream, but I am against an additional layer
> introduced by srp_daemon.sh. It would also be nice to have a systemd
> service shipped by upstream (and not just in the redhat directory)

The srp daemon needs proper native systemd support, not via a shell
script wrapper. More like rdma-ndd. This is probably a bigger
project..

I didn't think debian had a .service file for it at all?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Jason Gunthorpe]

On Tue, May 09, 2017 at 05:54:33PM +0000, Bart Van Assche wrote:
> > * Bonus points: consolidate the srp daemon. Debian ships a different
> > service file than upstream, but I am against an additional layer
> > introduced by srp_daemon.sh. It would also be nice to have a systemd
> > service shipped by upstream (and not just in the redhat directory)
> 
> I will have a look at this too.

My thoughts..

It looked to me like srp_daemon needed one process per port.

The best path looked to me like using systemd templates (eg
srp_daemon@mlx4_0/0) to allow systemd to directly manage the per port
srp_daemon.

Then the question is how to request the right templates are
created.. Perhaps udev rules can do this directly, but I'm not sure
about how to get the port #, perhaps an udev triggered script or
something can do it.

Perhaps there could be an inbetween progarm that took the udev events
and asked systemd to make the right units.

Another option is to have a srp_daemon 'runner' that monitors udev and
actively manages a set of fork'd childern so that the children cover
all required ports. But this is actually somewhat hard to do well..

The big issue with the redhat script is that it is not hotplug safe,
and needs special boot ordering, which we really need to get away from
in general for robustness.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Benjamin Drung]

Am Dienstag, den 09.05.2017, 12:05 -0600 schrieb Jason Gunthorpe:
> On Tue, May 09, 2017 at 07:43:09PM +0200, Benjamin Drung wrote:
> 
> > * Can we upstream some redhat files, i.e. move them out of the
> > redhat
> > directory and maintain them in their corresponding code? Following
> > files fall in this category:
> 
> Anything in the redhat directory could be moved out if it is going to
> be used by another distro. It is there waiting for other distros to
> look at it.

* The ifdown-ib and ifup-ib scripts are redhat-specific. It won't work
with ifupdown from Debian.

* The files related to mlx4 look like a special handling for mlx4 to me
and they do not following the KISS principle.

* What's the purpose of the cxgb{3,4} modprobe files?

* rdma.modules-setup.sh is not needed since Debian does not use dracut.

Everything else should be moved.

> > * Can we provide an upstream rdma-core "package" that contains the
> > rdma
> > service and the following files from the redhat directory?
> > ** rdma.conf
> > ** rdma.kernel-init
> > ** rdma.service
> > ** rdma.udev-rules
> 
> Do you mean adding a 'rdma-core' package to debian/?
> 
> We could move the relevant stuff out of redhat/ and into, say,
> kernel-boot/ or something.

I meant a directory ('kernel-boot' sounds good) where these files live
and a corresponding binary package where these files are installed
into.

Jarod already created a rdma-core package in debian/control in commit
8df5873b9. Currently we do not have a rdma-core package in Debian and
no package with a kernel-boot service file. I know that the openibd and
 mlnx-ofed-kernel-utils packages shipped a openibd service that does
similar job.

Do we want to call the binary package that contains the kernel-boot
service 'rdma-core' or simplify that to 'rdma'? We should use the same
binary package name across the distributions (unless it's conflicting
with a naming convention).

Can you come up with a patch set? I will be happy to review it. Note
that paths like /usr/libexec/rdma-init-kernel (in rdma.service) needs
to be made configurable. In Debian, we would install these helper
scripts in /usr/lib/$packagename/$filename.

> IMHO there is no reason not to do that..
> 
> > Patches for improved descriptions are welcome.
> > 
> >   W: iwpmd: init.d-script-missing-start etc/init.d/iwpmd 2 4
> > 
> > Any objections to add 2 and 4?
> 
> Debian and RH6 are the only places I know of that use init.d scripts,
> so I think most changes Debian needs would be fine.

Thanks. I will create a patch then.

> >   W: ibverbs-providers: package-name-doesnt-match-sonames libmlx5-1
> > 
> > I think we should just ignore this warning since using libmlx5-1 is
> > just one part of ibverbs-providers that shouldn't be use alone,
> > should
> > it?
> 
> Probably.. It is has become a bit odd with mlx5 being directly
> linkable now..
> 
> >   W: ibverbs-providers: non-dev-pkg-with-shlib-symlink
> > usr/lib/x86_64-
> >      linux-gnu/libmlx5.so.1.1.14 usr/lib/x86_64-linux-
> > gnu/libmlx5.so
> > 
> > This libmlx5.so symlink should be part of a development package.
> > Should
> > I add a new binary libmlx5-dev package or should it be moved to
> > libibverbs-dev (where already the header files are)?
> 
> It should be with the headers, so I suggest libibverbs-dev

Okay. I will put it there.

> > * Bonus points: consolidate the srp daemon. Debian ships a
> > different
> > service file than upstream, but I am against an additional layer
> > introduced by srp_daemon.sh. It would also be nice to have a
> > systemd
> > service shipped by upstream (and not just in the redhat directory)
> 
> The srp daemon needs proper native systemd support, not via a shell
> script wrapper. More like rdma-ndd. This is probably a bigger
> project..
> 
> I didn't think debian had a .service file for it at all?

Yes. I referred to the sysvinit script when I wrote "service file" (and
not to a .service systemd file). ;) It would be nice if Debian ships
both: a sysvinit script (legacy support and users that do not want to
use systemd) and a systemd .service file.

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
Web: https://www.profitbricks.com

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Achim Weiss.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Jason Gunthorpe]

On Wed, May 10, 2017 at 11:35:47AM +0200, Benjamin Drung wrote:
> Am Dienstag, den 09.05.2017, 12:05 -0600 schrieb Jason Gunthorpe:
> > On Tue, May 09, 2017 at 07:43:09PM +0200, Benjamin Drung wrote:
> > 
> > > * Can we upstream some redhat files, i.e. move them out of the
> > > redhat
> > > directory and maintain them in their corresponding code? Following
> > > files fall in this category:
> > 
> > Anything in the redhat directory could be moved out if it is going to
> > be used by another distro. It is there waiting for other distros to
> > look at it.
> 
> * The ifdown-ib and ifup-ib scripts are redhat-specific. It won't work
> with ifupdown from Debian.
> 
> * The files related to mlx4 look like a special handling for mlx4 to me
> and they do not following the KISS principle.

One part is managing the need to setup multi-protocol ports on
boot. This seems like a general requirement for mlx4/5 so maybe it
should get some more common solution..

> * What's the purpose of the cxgb{3,4} modprobe files?

cxgb is a multi-kernel module driver, those scripts are an attempt to
load the other parts when the main driver is loaded. I'm not sure why
cxgb is special enough to get these files but mlx/etc are not.

I suspect it is old cruft because redhat/rdma.kernel-init handles
things uniformly on systemd environments.

I've been thinking we should patch the kernel to solve this problem
instead of using this ugly userspace solution..

> Jarod already created a rdma-core package in debian/control in commit
> 8df5873b9. Currently we do not have a rdma-core package in Debian and
> no package with a kernel-boot service file. I know that the openibd and
>  mlnx-ofed-kernel-utils packages shipped a openibd service that does
> similar job.

Yes, pulling something like openibd into rdma-core is the idea here..

> Do we want to call the binary package that contains the kernel-boot
> service 'rdma-core' or simplify that to 'rdma'? We should use the same
> binary package name across the distributions (unless it's conflicting
> with a naming convention).

I think rdma-core is fine..

> Can you come up with a patch set? I will be happy to review it. Note
> that paths like /usr/libexec/rdma-init-kernel (in rdma.service) needs
> to be made configurable. In Debian, we would install these helper
> scripts in /usr/lib/$packagename/$filename.

Maybe Leon's team will try to tackle some of this? It is complicated,
the RH scripts do quite a bit stuff :\

But this doesn't seem particularly urgent for Debian packaging, right?

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2918 bytes --]

On Wed, May 10, 2017 at 09:50:05AM -0600, Jason Gunthorpe wrote:
> On Wed, May 10, 2017 at 11:35:47AM +0200, Benjamin Drung wrote:
> > Am Dienstag, den 09.05.2017, 12:05 -0600 schrieb Jason Gunthorpe:
> > > On Tue, May 09, 2017 at 07:43:09PM +0200, Benjamin Drung wrote:
> > >
> > > > * Can we upstream some redhat files, i.e. move them out of the
> > > > redhat
> > > > directory and maintain them in their corresponding code? Following
> > > > files fall in this category:
> > >
> > > Anything in the redhat directory could be moved out if it is going to
> > > be used by another distro. It is there waiting for other distros to
> > > look at it.
> >
> > * The ifdown-ib and ifup-ib scripts are redhat-specific. It won't work
> > with ifupdown from Debian.
> >
> > * The files related to mlx4 look like a special handling for mlx4 to me
> > and they do not following the KISS principle.
>
> One part is managing the need to setup multi-protocol ports on
> boot. This seems like a general requirement for mlx4/5 so maybe it
> should get some more common solution..
>
> > * What's the purpose of the cxgb{3,4} modprobe files?
>
> cxgb is a multi-kernel module driver, those scripts are an attempt to
> load the other parts when the main driver is loaded. I'm not sure why
> cxgb is special enough to get these files but mlx/etc are not.
>
> I suspect it is old cruft because redhat/rdma.kernel-init handles
> things uniformly on systemd environments.
>
> I've been thinking we should patch the kernel to solve this problem
> instead of using this ugly userspace solution.

Can it be related to the fact that mlx5 core calls to
request_module_nowait for mlx5_ib and chelsio doesn't do it?

>
> > Jarod already created a rdma-core package in debian/control in commit
> > 8df5873b9. Currently we do not have a rdma-core package in Debian and
> > no package with a kernel-boot service file. I know that the openibd and
> >  mlnx-ofed-kernel-utils packages shipped a openibd service that does
> > similar job.
>
> Yes, pulling something like openibd into rdma-core is the idea here..
>
> > Do we want to call the binary package that contains the kernel-boot
> > service 'rdma-core' or simplify that to 'rdma'? We should use the same
> > binary package name across the distributions (unless it's conflicting
> > with a naming convention).
>
> I think rdma-core is fine..
>
> > Can you come up with a patch set? I will be happy to review it. Note
> > that paths like /usr/libexec/rdma-init-kernel (in rdma.service) needs
> > to be made configurable. In Debian, we would install these helper
> > scripts in /usr/lib/$packagename/$filename.
>
> Maybe Leon's team will try to tackle some of this? It is complicated,
> the RH scripts do quite a bit stuff :\

I would be happy to help, but I'm not sure that I understand what should
be done :(

>
> But this doesn't seem particularly urgent for Debian packaging, right?
>
> Jason

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: rdma-core in Dabian

[Benjamin Drung]

Am Mittwoch, den 10.05.2017, 09:50 -0600 schrieb Jason Gunthorpe:
> On Wed, May 10, 2017 at 11:35:47AM +0200, Benjamin Drung wrote:
> > Am Dienstag, den 09.05.2017, 12:05 -0600 schrieb Jason Gunthorpe:
> > > On Tue, May 09, 2017 at 07:43:09PM +0200, Benjamin Drung wrote:
> > > 
> > > > * Can we upstream some redhat files, i.e. move them out of the
> > > > redhat
> > > > directory and maintain them in their corresponding code?
> > > > Following
> > > > files fall in this category:
> > > 
> > > Anything in the redhat directory could be moved out if it is
> > > going to
> > > be used by another distro. It is there waiting for other distros
> > > to
> > > look at it.
> > 
> > * The ifdown-ib and ifup-ib scripts are redhat-specific. It won't
> > work
> > with ifupdown from Debian.
> > 
> > * The files related to mlx4 look like a special handling for mlx4
> > to me
> > and they do not following the KISS principle.
> 
> One part is managing the need to setup multi-protocol ports on
> boot. This seems like a general requirement for mlx4/5 so maybe it
> should get some more common solution..

A common solution would be better.

> > * What's the purpose of the cxgb{3,4} modprobe files?
> 
> cxgb is a multi-kernel module driver, those scripts are an attempt to
> load the other parts when the main driver is loaded. I'm not sure why
> cxgb is special enough to get these files but mlx/etc are not.
> 
> I suspect it is old cruft because redhat/rdma.kernel-init handles
> things uniformly on systemd environments.
>
> I've been thinking we should patch the kernel to solve this problem
> instead of using this ugly userspace solution..

Yes, better fix it in the kernel.

> > Jarod already created a rdma-core package in debian/control in
> > commit
> > 8df5873b9. Currently we do not have a rdma-core package in Debian
> > and
> > no package with a kernel-boot service file. I know that the openibd
> > and
> >  mlnx-ofed-kernel-utils packages shipped a openibd service that
> > does
> > similar job.
> 
> Yes, pulling something like openibd into rdma-core is the idea here..
> 
> > Do we want to call the binary package that contains the kernel-boot
> > service 'rdma-core' or simplify that to 'rdma'? We should use the
> > same
> > binary package name across the distributions (unless it's
> > conflicting
> > with a naming convention).
> 
> I think rdma-core is fine..

Okay. I am fine with this name.

> > Can you come up with a patch set? I will be happy to review it.
> > Note
> > that paths like /usr/libexec/rdma-init-kernel (in rdma.service)
> > needs
> > to be made configurable. In Debian, we would install these helper
> > scripts in /usr/lib/$packagename/$filename.
> 
> Maybe Leon's team will try to tackle some of this? It is complicated,
> the RH scripts do quite a bit stuff :\
> 
> But this doesn't seem particularly urgent for Debian packaging,
> right?

I would love to have it in the Debian package. Currently the rdma-core
binary package contains only rdma-ndd. If we defer the kernel-init
part, I have to rename rdma-core or remove it.

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
Web: https://www.profitbricks.com

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Achim Weiss.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Benjamin Drung]

[-- Attachment #1: Type: text/plain, Size: 2158 bytes --]

Am Mittwoch, den 10.05.2017, 19:02 +0300 schrieb Leon Romanovsky:
> On Wed, May 10, 2017 at 09:50:05AM -0600, Jason Gunthorpe wrote:
> > On Wed, May 10, 2017 at 11:35:47AM +0200, Benjamin Drung wrote:
> > > Jarod already created a rdma-core package in debian/control in
> > > commit
> > > 8df5873b9. Currently we do not have a rdma-core package in Debian
> > > and
> > > no package with a kernel-boot service file. I know that the
> > > openibd and
> > >  mlnx-ofed-kernel-utils packages shipped a openibd service that
> > > does
> > > similar job.
> > 
> > Yes, pulling something like openibd into rdma-core is the idea
> > here..
> > 
> > > Do we want to call the binary package that contains the kernel-
> > > boot
> > > service 'rdma-core' or simplify that to 'rdma'? We should use the
> > > same
> > > binary package name across the distributions (unless it's
> > > conflicting
> > > with a naming convention).
> > 
> > I think rdma-core is fine..
> > 
> > > Can you come up with a patch set? I will be happy to review it.
> > > Note
> > > that paths like /usr/libexec/rdma-init-kernel (in rdma.service)
> > > needs
> > > to be made configurable. In Debian, we would install these helper
> > > scripts in /usr/lib/$packagename/$filename.
> > 
> > Maybe Leon's team will try to tackle some of this? It is
> > complicated,
> > the RH scripts do quite a bit stuff :\
> 
> I would be happy to help, but I'm not sure that I understand what
> should
> be done :(

Move redhat/rdma.service and everything that is required by it to a new
'kernel-boot' directory. Add a CMakeLists.txt file that installs the
service. Make sure that the hard-coded paths are set by cmake. Also
check the moved scripts for old, legacy stuff that might not work
somewhere else.

-- 
Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: benjamin.drung-EIkl63zCoXaH+58JC4qpiA@public.gmane.org
Web: https://www.profitbricks.com

Sitz der Gesellschaft: Berlin.
Registergericht: Amtsgericht Charlottenburg, HRB 125506B.
Geschäftsführer: Achim Weiss.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

Re: rdma-core in Dabian

[Jason Gunthorpe]

On Wed, May 10, 2017 at 07:02:01PM +0300, Leon Romanovsky wrote:

> > I've been thinking we should patch the kernel to solve this problem
> > instead of using this ugly userspace solution.
> 
> Can it be related to the fact that mlx5 core calls to
> request_module_nowait for mlx5_ib and chelsio doesn't do it?

Yeah, maybe we should update chelsio kernel drivers instead of using
these strange modprobe files...

Steve?

> > Maybe Leon's team will try to tackle some of this? It is complicated,
> > the RH scripts do quite a bit stuff :\
> 
> I would be happy to help, but I'm not sure that I understand what should
> be done :(

I'm not totally clear either, TBH. The overall topic is to move files
out of redhat/ and into the common directory. This requires removing
the redhat-isms and making sure the redhat solution is what we really
want upstream.

eg it seems rdma.cxgb[3]4.sys.modprobe is probably best done by patching
the kernel instead, not copying the modprobe files.

rdma.kernel-init does:
 - Load extra ULP modules
    * /etc/rdma/rdma.conf is not appropriate for Debian
    * 'TECH_PREVIEW' is a RedHat concept
 - Obsolete mttr/errata stuff. This is in the kernel now (delete it)
 - Yet another go at loading extra driver kernel modules (move to
   kernel)
 - Something about SRIOV (?)

For ULP modules, I think we are loading too many by hand, again, the
kernel really needs fixing to autoload our stuff on first use. So, I
suggest a quick pass over that idea to see how much kernel code can be
fixed up.

So the stuff to move to common user space code might just be the port
configuration and sriov? But someone needs to study this entire topic
with a broad view including the kernel.
 
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: rdma-core in Dabian

[Steve Wise]

> From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
> 
> On Wed, May 10, 2017 at 07:02:01PM +0300, Leon Romanovsky wrote:
> 
> > > I've been thinking we should patch the kernel to solve this problem
> > > instead of using this ugly userspace solution.
> >
> > Can it be related to the fact that mlx5 core calls to
> > request_module_nowait for mlx5_ib and chelsio doesn't do it?
> 
> Yeah, maybe we should update chelsio kernel drivers instead of using
> these strange modprobe files...
> 
> Steve?

Under what criteria would cxgb4 want to request its Upper Layer Drivers (ULDs)
to be loaded?  You wouldn't want to load the iwarp and iscsi drivers if they
aren't going to be used, would you?



> 
> > > Maybe Leon's team will try to tackle some of this? It is complicated,
> > > the RH scripts do quite a bit stuff :\
> >
> > I would be happy to help, but I'm not sure that I understand what should
> > be done :(
> 
> I'm not totally clear either, TBH. The overall topic is to move files
> out of redhat/ and into the common directory. This requires removing
> the redhat-isms and making sure the redhat solution is what we really
> want upstream.
> 
> eg it seems rdma.cxgb[3]4.sys.modprobe is probably best done by patching
> the kernel instead, not copying the modprobe files.
> 
> rdma.kernel-init does:
>  - Load extra ULP modules
>     * /etc/rdma/rdma.conf is not appropriate for Debian
>     * 'TECH_PREVIEW' is a RedHat concept
>  - Obsolete mttr/errata stuff. This is in the kernel now (delete it)
>  - Yet another go at loading extra driver kernel modules (move to
>    kernel)
>  - Something about SRIOV (?)
> 
> For ULP modules, I think we are loading too many by hand, again, the
> kernel really needs fixing to autoload our stuff on first use. So, I
> suggest a quick pass over that idea to see how much kernel code can be
> fixed up.
> 
> So the stuff to move to common user space code might just be the port
> configuration and sriov? But someone needs to study this entire topic
> with a broad view including the kernel.
> 
> Jason

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Jason Gunthorpe]

On Wed, May 10, 2017 at 11:50:06AM -0500, Steve Wise wrote:
> > From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
> > 
> > On Wed, May 10, 2017 at 07:02:01PM +0300, Leon Romanovsky wrote:
> > 
> > > > I've been thinking we should patch the kernel to solve this problem
> > > > instead of using this ugly userspace solution.
> > >
> > > Can it be related to the fact that mlx5 core calls to
> > > request_module_nowait for mlx5_ib and chelsio doesn't do it?
> > 
> > Yeah, maybe we should update chelsio kernel drivers instead of using
> > these strange modprobe files...
> > 
> > Steve?
> 
> Under what criteria would cxgb4 want to request its Upper Layer Drivers (ULDs)
> to be loaded?  You wouldn't want to load the iwarp and iscsi drivers if they
> aren't going to be used, would you?

Sounds like Mellanox is loading rocee drivers unconditionally, so why
not load iwarp ones too? People who really don't want that can opt out
with some modprobe configuration to block the modules.

iscsi should probably be demand loaded when userspace uses iscsi that
crosses a cxgb chip..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 2353 bytes --]

On Wed, May 10, 2017 at 11:50:06AM -0500, Steve Wise wrote:
> > From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
> >
> > On Wed, May 10, 2017 at 07:02:01PM +0300, Leon Romanovsky wrote:
> >
> > > > I've been thinking we should patch the kernel to solve this problem
> > > > instead of using this ugly userspace solution.
> > >
> > > Can it be related to the fact that mlx5 core calls to
> > > request_module_nowait for mlx5_ib and chelsio doesn't do it?
> >
> > Yeah, maybe we should update chelsio kernel drivers instead of using
> > these strange modprobe files...
> >
> > Steve?
>
> Under what criteria would cxgb4 want to request its Upper Layer Drivers (ULDs)
> to be loaded?  You wouldn't want to load the iwarp and iscsi drivers if they
> aren't going to be used, would you?

If you don't want them, you will blacklist them. There is nothing wrong
with ready-to-use approach.

>
>
>
> >
> > > > Maybe Leon's team will try to tackle some of this? It is complicated,
> > > > the RH scripts do quite a bit stuff :\
> > >
> > > I would be happy to help, but I'm not sure that I understand what should
> > > be done :(
> >
> > I'm not totally clear either, TBH. The overall topic is to move files
> > out of redhat/ and into the common directory. This requires removing
> > the redhat-isms and making sure the redhat solution is what we really
> > want upstream.
> >
> > eg it seems rdma.cxgb[3]4.sys.modprobe is probably best done by patching
> > the kernel instead, not copying the modprobe files.
> >
> > rdma.kernel-init does:
> >  - Load extra ULP modules
> >     * /etc/rdma/rdma.conf is not appropriate for Debian
> >     * 'TECH_PREVIEW' is a RedHat concept
> >  - Obsolete mttr/errata stuff. This is in the kernel now (delete it)
> >  - Yet another go at loading extra driver kernel modules (move to
> >    kernel)
> >  - Something about SRIOV (?)
> >
> > For ULP modules, I think we are loading too many by hand, again, the
> > kernel really needs fixing to autoload our stuff on first use. So, I
> > suggest a quick pass over that idea to see how much kernel code can be
> > fixed up.
> >
> > So the stuff to move to common user space code might just be the port
> > configuration and sriov? But someone needs to study this entire topic
> > with a broad view including the kernel.
> >
> > Jason
>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: rdma-core in Dabian

[Steve Wise]

> On Wed, May 10, 2017 at 11:50:06AM -0500, Steve Wise wrote:
> > > From: Jason Gunthorpe [mailto:jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org]
> > >
> > > On Wed, May 10, 2017 at 07:02:01PM +0300, Leon Romanovsky wrote:
> > >
> > > > > I've been thinking we should patch the kernel to solve this problem
> > > > > instead of using this ugly userspace solution.
> > > >
> > > > Can it be related to the fact that mlx5 core calls to
> > > > request_module_nowait for mlx5_ib and chelsio doesn't do it?
> > >
> > > Yeah, maybe we should update chelsio kernel drivers instead of using
> > > these strange modprobe files...
> > >
> > > Steve?
> >
> > Under what criteria would cxgb4 want to request its Upper Layer Drivers
(ULDs)
> > to be loaded?  You wouldn't want to load the iwarp and iscsi drivers if they
> > aren't going to be used, would you?
> 
> If you don't want them, you will blacklist them. There is nothing wrong
> with ready-to-use approach.

Ok...

> 
> >
> >
> >
> > >
> > > > > Maybe Leon's team will try to tackle some of this? It is complicated,
> > > > > the RH scripts do quite a bit stuff :\
> > > >
> > > > I would be happy to help, but I'm not sure that I understand what should
> > > > be done :(
> > >
> > > I'm not totally clear either, TBH. The overall topic is to move files
> > > out of redhat/ and into the common directory. This requires removing
> > > the redhat-isms and making sure the redhat solution is what we really
> > > want upstream.
> > >
> > > eg it seems rdma.cxgb[3]4.sys.modprobe is probably best done by patching
> > > the kernel instead, not copying the modprobe files.
> > >
> > > rdma.kernel-init does:
> > >  - Load extra ULP modules
> > >     * /etc/rdma/rdma.conf is not appropriate for Debian
> > >     * 'TECH_PREVIEW' is a RedHat concept
> > >  - Obsolete mttr/errata stuff. This is in the kernel now (delete it)
> > >  - Yet another go at loading extra driver kernel modules (move to
> > >    kernel)
> > >  - Something about SRIOV (?)
> > >
> > > For ULP modules, I think we are loading too many by hand, again, the
> > > kernel really needs fixing to autoload our stuff on first use. So, I
> > > suggest a quick pass over that idea to see how much kernel code can be
> > > fixed up.
> > >
> > > So the stuff to move to common user space code might just be the port
> > > configuration and sriov? But someone needs to study this entire topic
> > > with a broad view including the kernel.
> > >
> > > Jason
> >

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: rdma-core in Dabian

[Bart Van Assche]

[-- Attachment #1: Type: text/plain, Size: 2132 bytes --]

On Tue, 2017-05-09 at 12:18 -0600, Jason Gunthorpe wrote:
> On Tue, May 09, 2017 at 05:54:33PM +0000, Bart Van Assche wrote:
> > > * Bonus points: consolidate the srp daemon. Debian ships a different
> > > service file than upstream, but I am against an additional layer
> > > introduced by srp_daemon.sh. It would also be nice to have a systemd
> > > service shipped by upstream (and not just in the redhat directory)
> > 
> > I will have a look at this too.
> 
> My thoughts..
> 
> It looked to me like srp_daemon needed one process per port.
> 
> The best path looked to me like using systemd templates (eg
> srp_daemon@mlx4_0/0) to allow systemd to directly manage the per port
> srp_daemon.
> 
> Then the question is how to request the right templates are
> created.. Perhaps udev rules can do this directly, but I'm not sure
> about how to get the port #, perhaps an udev triggered script or
> something can do it.
> 
> Perhaps there could be an inbetween progarm that took the udev events
> and asked systemd to make the right units.
> 
> Another option is to have a srp_daemon 'runner' that monitors udev and
> actively manages a set of fork'd childern so that the children cover
> all required ports. But this is actually somewhat hard to do well..
> 
> The big issue with the redhat script is that it is not hotplug safe,
> and needs special boot ordering, which we really need to get away from
> in general for robustness.

Hello Jason,

Thanks for having shared your thoughts.

When using systemd templates instead of the current approach it would
become cumbersome for users to stop and start the srp_daemon on all ports
simultaneously. Additionally, if an RDMA adapter is hot-added, should the
srp_daemon be started or should it not be started for the newly added ports?

An in-between program could indeed do the job of listening for udev events
and actively managing srp_daemon children. Since we already have an
in-between program, namely srp_daemon.sh, the simplest approach is to modify
that shell script. Can you have a look at the attached patch?

Thanks,

Bart.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-srp_daemon.service-Add-support-for-hotplugging.patch --]
[-- Type: text/x-patch; name="0001-srp_daemon.service-Add-support-for-hotplugging.patch", Size: 2836 bytes --]

From bec3884e36298ab2b6fb09c533b575fa95bd3378 Mon Sep 17 00:00:00 2001
From: Bart Van Assche <bart.vanassche@sandisk.com>
Date: Fri, 12 May 2017 10:24:44 -0700
Subject: [PATCH] srp_daemon.service: Add support for hotplugging

Instead of only starting srp_daemon.sh after the RDMA subsystem has
been initialized, make srp_daemon.sh scan periodically for RDMA ports.
An advantage of the new approach is that it supports hot-add of RDMA
adapters.

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
---
 redhat/srp_daemon.service   |  4 ----
 srp_daemon/srp_daemon.sh.in | 41 ++++++++++++++++++++++++++---------------
 2 files changed, 26 insertions(+), 19 deletions(-)

diff --git a/redhat/srp_daemon.service b/redhat/srp_daemon.service
index 9510f5fb..cf98aa74 100644
--- a/redhat/srp_daemon.service
+++ b/redhat/srp_daemon.service
@@ -3,10 +3,6 @@ Description=Start or stop the daemon that attaches to SRP devices
 Documentation=man:srp_daemon file:/etc/rdma/rdma.conf file:/etc/srp_daemon.conf
 DefaultDependencies=false
 Conflicts=emergency.target emergency.service
-Requires=rdma.service
-Wants=opensm.service
-After=rdma.service opensm.service
-After=network.target
 Before=remote-fs-pre.target
 
 [Service]
diff --git a/srp_daemon/srp_daemon.sh.in b/srp_daemon/srp_daemon.sh.in
index 75e8a31b..74c08d7a 100755
--- a/srp_daemon/srp_daemon.sh.in
+++ b/srp_daemon/srp_daemon.sh.in
@@ -37,6 +37,7 @@ rescan_interval=60
 pids=()
 pidfile="@CMAKE_INSTALL_FULL_RUNDIR@/srp_daemon.sh.pid"
 mypid=$$
+umad_devs=()
 
 trap_handler()
 {
@@ -49,6 +50,17 @@ trap_handler()
     exit 0
 }
 
+# Check whether $1 is equal to one of $2..${$#}
+contains()
+{
+    local v
+
+    for v in "${@:2}"; do
+	[ "$v" = "$1" ] && return 0
+    done
+    return 1
+}
+
 # Check if there is another copy running of srp_daemon.sh
 if [ -f "$pidfile" ]; then
     if [ -e "/proc/$(cat "$pidfile" 2>/dev/null)/status" ]; then
@@ -66,19 +78,18 @@ fi
 
 trap 'trap_handler' 2 15
 
-while [ ! -d ${ibdir} ]
-do
-    sleep 30
+while :; do
+    for d in ${ibdir}_mad/umad*; do
+	[ -e "$d" ] || continue
+	contains "$d" "${umad_devs[@]}" && continue
+	hca_id="$(<"$d/ibdev")"
+	port="$(<"$d/port")"
+	add_target="${ibdir}_srp/srp-${hca_id}-${port}/add_target"
+	if [ -e "${add_target}" ]; then
+            ${prog} -e -c -n -i "${hca_id}" -p "${port}" -R "${rescan_interval}" "${params[@]}" >/dev/null 2>&1 &
+            pids+=($!)
+	    umad_dev+=($d)
+	fi
+    done
+    sleep $rescan_interval
 done
-
-for d in ${ibdir}_mad/umad*; do
-    hca_id="$(<"$d/ibdev")"
-    port="$(<"$d/port")"
-    add_target="${ibdir}_srp/srp-${hca_id}-${port}/add_target"
-    if [ -e "${add_target}" ]; then
-        ${prog} -e -c -n -i "${hca_id}" -p "${port}" -R "${rescan_interval}" "${params[@]}" >/dev/null 2>&1 &
-        pids+=($!)
-    fi
-done
-
-wait
-- 
2.12.2

Re: rdma-core in Dabian

[Jason Gunthorpe]

On Fri, May 12, 2017 at 06:24:35PM +0000, Bart Van Assche wrote:

> When using systemd templates instead of the current approach it would
> become cumbersome for users to stop and start the srp_daemon on all ports
> simultaneously.

I think systemd PartOf dependencies is intended to solve that
problem..

       PartOf=

           Configures dependencies similar to Requires=, but limited
           to stopping and restarting of units. When systemd stops or restarts
           the units listed here, the action is propagated to this
           unit. Note that this is a one-way dependency -- changes to this unit
           do not affect the listed units.

So there could still be a service name that covered all of the
children (eg srp_daemon.service and srp_daemon_port@.service)

> Additionally, if an RDMA adapter is hot-added, should the srp_daemon
> be started or should it not be started for the newly added ports?

I'm not sure what is best for that policy..

Today it looks like srp_daemon defaults to explicitly opt-in to
running on ports. To continue that policy the admin would have to
explicitly opt in to each port. Eg perhaps something like:

 systemctl enable srp_daemon@mlx4_0/0

Instead of editing a /etc/ config file.

systemd has a '.device' system, which as I understand it, would allow
srp_daemon@mlx4_0/0 to depend on mlx4_0/0.device, which is created by
udev on hotplug, and would trigger running the service. 

(perhaps for this reason the template value would be umad0, not mlx4_0/0)

> An in-between program could indeed do the job of listening for udev events
> and actively managing srp_daemon children. Since we already have an
> in-between program, namely srp_daemon.sh, the simplest approach is to modify
> that shell script. Can you have a look at the attached patch?

This seems like a reasonable improvement, but as I said, the inbetween
program is a bit more complex that you probably want to do with a
script:

- Sites do not like things running on timers due to the introduced
  jitter, so the trivial sleep loop is not nice.
- Failure of a child should result in a sensible log message and a
  restart protocol like systemd has. Failure to SIGTERM the child should
  escalate to SIGKILL to prevent lockups during shutdown
- Dumping stderr/etc into /dev/null is a bad idea (eg glibc internal
  asserts are lost now). In a systemd world each child should get its
  own stderrr logger connection so this stuff works right.
- We still need to be able to configure which ports srp_daemon runs
  on and to 'reload' that configuration into the script
- Ideally hot removal of an adaptor requires halting just those
  daemons without triggering restart

Since so much of that overlaps with systemd capabilities it does make
some sense to have systemd manage the child processes directly.

For instance you could have the srp_daemon script like in the patch
but replace this:

  ${prog} -e -c -n [..]
with
  systemctl start srp_daemon_port@${hca_id}/${port}

Using PartOf dependencies.. The user experience would be very similar
to the script directly forking, but many more of the above areas are
covered off..

But.. in that case we don't even need the script. If the admin wishes
to run srp_daemon on all IB ports then a simple udev rule will do the
job:

SUBSYSTEM=="infiniband", TAG+="systemd", ATTRS{node_desc}=="*", ENV{SYSTEMD_WANTS}+="srp_daemon_port@$name.service"

[more or less]

And now the timer is gone too.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html