srp_daemon : ​Disallow ​all targets ​if ​not ​explicitly ​allowed by default

Hi,

srptools 1.0.3-2 in Debian disallows all targets if not explicitly allowed by default. Motivation (taken from debian/changelog):

* Don't activate any targets per default. (Closes: #740945) This is more sensible than the previous default of bringing up all targets in the IB fabric upon boot. In a larger fabric with many storage targets available, most of the times only one or a few targets are wanted on a particular machine.

Do you agree and could you change the default upstream? I prefer not to carry a different behavior in Debian alone.

Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: [email protected]
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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hello Benjamin,

What I expect is that users will hate this change. They will notice that after they have installed and enabled srp_daemon that no targets are discovered without having any clue why no automatic login to SRP targets occurs.

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

----- Original Message -----

Hello

Indeed, I agree with Bart here, this change will lead to a lot of confusion about why users no longer see device discovery. For me I would prefer its not changed.

Regards
Laurence
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

According to the Benjamin's description, this is already behavior of Debian and their derivatives. So this change won't change for these users anything. I think that the best solution will be to keep in sync distros and upstream. Or enable by default on all systems or disable by default on all systems.

Thanks

Users of large setups know how to edit the srp_daemon configuration file but most users of small setups don't know how to edit that configuration file. So if consistency is required my advice is to change the behavior on Debian to activating all targets by default.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

I'm fine with any option as long as Debian and upstream are in sync.

Thanks

I thought the default in Debain was to have a null list of devices anyhow, so if you just install srp_daemon nothing actually happens until the .default config file is edited..

Thus it seems really strange to have to edit two config files to make it work, particularly if there is no security concern here.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

----- Original Message -----

On Red Hat I have the following default set for the customers I deal with.

# cat srp_daemon.conf
a      queue_size=128,max_cmd_per_lun=32,max_sect=32768

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Am Dienstag, den 09.05.2017, 21:01 +0300 schrieb Leon Romanovsky:

This change was uploaded to Debian unstable two days ago. So it is a recent change done by Roland Fehrenbacher (CCed) which still can be reverted. I'm fine with any option as long as Debian and upstream are in sync.

Bart's objection (that users of large setups know how to edit the srp_daemon configuration file but most users of small setups don't know how to edit that configuration file) seem valid to me. Roland, your input please.

Benjamin Drung
System Developer
Debian & Ubuntu Developer

ProfitBricks GmbH
Greifswalder Str. 207
D - 10405 Berlin

Email: [email protected]
Web: https://www.profitbricks.com

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

Hi all,

BD> Am Dienstag, den 09.05.2017, 21:01 +0300 schrieb Leon
BD> Romanovsky:
>> On Tue, May 09, 2017 at 01:41:46PM -0400, Laurence Oberman wrote:
>> >
>> >
>> > ----- Original Message -----
>> > > From: "Bart Van Assche" <[email protected]> To:
>> > > "Benjamin Drung" <[email protected]>, linux-rdm >> > > [email protected] Sent: Tuesday, May 9, 2017 1:11:06 PM
>> > > Subject: Re: srp_daemon : Disallow all targets if not
>> > > explicitly allowed by default
>> > >
>> > > On 05/09/17 10:07, Benjamin Drung wrote:
>> > > > srptools 1.0.3-2 in Debian disallows all targets if not
>> > > > explicitly allowed by default. Motivation (taken from
>> > > > debian/changelog):
>> > > >
>> > > > * Don't activate any targets per default. (Closes: #740945)
>> > > >   This is more sensible than the previous default of
>> > > > bringing   up all targets in the IB fabric upon boot. In a
>> > > > larger fabric   with many storage targets available, most
>> > > > of the times only   one or a few targets are wanted on a
>> > > > particular machine.
>> > > >
>> > > > Do you agree and could you change the default upstream? I
>> > > > prefer not to carry a different behavior in Debian alone.
>> > >
>> > > Hello Benjamin,
>> > >
>> > > What I expect is that users will hate this change. They will
>> > > notice that after they have installed and enabled srp_daemon
>> > > that no targets are discovered without having any clue why no
>> > > automatic login to SRP targets occurs.
>> > >
>> > > Bart.
>> > >
>> > > -- To unsubscribe from this list: send the line "unsubscribe
>> > > linux- rdma" in the body of a message to
>> > > [email protected] More majordomo info >> > > at  http://vger.kernel.org/majordomo-info.htm l
>> > >
>> >
>> > Hello
>> >
>> > Indeed, I agree with Bart here, this change will lead to a lot
>> > of confusion about why users no longer see device discovery.
>> > For me I would prefer its not changed.
>>
>> According to the Benjamin's description, this is already behavior
>> of Debian and their derivatives. So this change won't change for
>> these users anything. I think that the best solution will be to
>> keep in sync distros and upstream. Or enable by default on all
>> systems or disable by default on all systems.

BD> This change was uploaded to Debian unstable two days ago. So it BD> is a recent change done by Roland Fehrenbacher (CCed) which BD> still can be reverted. I'm fine with any option as long as BD> Debian and upstream are in sync.

BD> Bart's objection (that users of large setups know how to edit BD> the srp_daemon configuration file but most users of small setups BD> don't know how to edit that configuration file) seem valid to BD> me. Roland, your input please.

it's hard to judge what is the most wanted default setup. The reason why this was changed was the cited Debian bug report. I'm also a friend of well-defined configs, so I agree with that opinion. I believe, adding a '#' in front of a line of a config file shouldn't overburden an admin who deals with IB storage under Linux, if she/he prefers the most simple config. The change in Debian is also clearly communicated (NEWS file), so I don't really see a problem. Finally: There are many packages in Debian that have a different default config as compared to upstream.

Cheers,

Roland--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

There are number of reasons to keep upstream and distros in sync. Main reasons are simplify testing (test once for all distros), packaging (part of our CI checks) and developing (new features are added to all distros automatically).

Thanks

Hello Roland,

Thanks for chiming in. Are you perhaps referring to
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740945 ? If so, are you aware that changing /etc/srp_daemon.conf was not necessary because of the PORTS=NONE setting in /etc/default/srptools? BTW, I just reinstalled srptools on an Ubuntu 17.04 system (apt-get remove -y srptools && dpkg --purge srptools && rm -f /etc/default/srptools /etc/srp_daemon.conf && rm -rf /var/cache/apt/archives && apt-get install -y srptools). From what I see in the installed configuration files it looks like the change you mentioned has either not been applied or it has been reverted?

# head -n 99 /etc/default/srptools /etc/srp_daemon.conf   
==> /etc/default/srptools <==
#How often should srpdeamon  rescan the fabric (seconds)
RETRIES=60

#Where should srp-deamon log to
LOG=/var/log/srp_daemon.log

# What ports should srp-deamon be started on.
# Format is CA:port
# ALL or NONE will run on all ports on none

# respectively

PORTS=NONE
#PORTS=ALL
#PORTS="mthca0:1 mlx4_0:2"

==> /etc/srp_daemon.conf <==
## This is an example rules configuration file for srp_daemon. ##
#This is a comment
## disallow the following dgid
#d       dgid=fe800000000000000002c90200402bd5
## allow target with the following ioc_guid
#a       ioc_guid=00a0b80200402bd7
## allow target with the following pkey
#a       pkey=ffff
## allow target with the following id_ext and ioc_guid #a       id_ext=200500A0B81146A1,ioc_guid=00a0b80200402bef ## disallow all the rest
#d
##
## Here is another example:
##
## Allow all targets and set queue size to 128.
# a      queue_size=128,max_cmd_per_lun=128

Thanks,

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Bart,

B> On Wed, 2017-05-10 at 16:02 +0200, Roland Fehrenbacher wrote:
>> it's hard to judge what is the most wanted default setup. The >> reason why this was changed was the cited Debian bug report. I'm >> also a friend of well-defined configs, so I agree with that >> opinion. I believe, adding a '#' in front of a line of a config >> file shouldn't overburden an admin who deals with IB storage >> under Linux, if she/he prefers the most simple config. The change >> in Debian is also clearly communicated (NEWS file), so I don't >> really see a problem. Finally: There are many packages in Debian >> that have a different default config as compared to upstream.

B> Hello Roland,

B> Thanks for chiming in. Are you perhaps referring to B> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=740945 ?

yes.

B> If so, are you aware that changing /etc/srp_daemon.conf was not B> necessary because of the PORTS=NONE setting in
B> /etc/default/srptools?

That setting has a different effect: srp_daemon won't even be started if PORTS=NONE. The PORTS variable just controls for which ports of the HCA(s) a daemon is started. On the other hand, in the config file you have additional control over the targets that will be added to the system.

B> BTW, I just reinstalled srptools on an Ubuntu 17.04 system B> (apt-get remove -y srptools && dpkg --purge srptools && rm -f B> /etc/default/srptools /etc/srp_daemon.conf && rm -rf B> /var/cache/apt/archives && apt-get install -y srptools). From B> what I see in the installed configuration files it looks like the B> change you mentioned has either not been applied or it has been B> reverted?

The new package is very new, sits in debian unstable and most likely hasn't made it to Ubuntu 17.04.

Cheers,

Roland

B> # head -n 99 /etc/default/srptools /etc/srp_daemon.conf B> ==> /etc/default/srptools <==
B> #How often should srpdeamon  rescan the fabric (seconds) B> RETRIES=60

B> #Where should srp-deamon log to
B> LOG=/var/log/srp_daemon.log

B> # What ports should srp-deamon be started on. Format is CA:port B> # ALL or NONE will run on all ports on none respectively

B> PORTS=NONE
B> #PORTS=ALL PORTS="mthca0:1 mlx4_0:2"

B> ==> /etc/srp_daemon.conf <==
B> ## This is an example rules configuration file for srp_daemon. B> ##
B> #This is a comment
B> ## disallow the following dgid
B> #d       dgid=fe800000000000000002c90200402bd5
B> ## allow target with the following ioc_guid
B> #a       ioc_guid=00a0b80200402bd7
B> ## allow target with the following pkey
B> #a       pkey=ffff
B> ## allow target with the following id_ext and ioc_guid B> #a       id_ext=200500A0B81146A1,ioc_guid=00a0b80200402bef B> ## disallow all the rest
B> #d
B> ##
B> ## Here is another example:
B> ##
B> ## Allow all targets and set queue size to 128.
B> # a      queue_size=128,max_cmd_per_lun=128

B> Thanks,

B> Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hello Roland,

Does that mean that most Debian users do not yet have that change in their /etc/srp_daemon.conf and when they upgrade and the packaged version of /etc/srp_daemon.conf gets installed that SRP login to remote systems will not occur anymore after a reboot? Since you wrote in a previous message that such changes are mentioned in the NEWS file: how many people do you think read that file before or after installing an upgrade?

Thanks,

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

B> On Wed, 2017-05-10 at 18:39 +0200, Roland Fehrenbacher wrote:
>> The new package is very new, sits in debian unstable and most >> likely hasn't made it to Ubuntu 17.04.

B> Hello Roland,

B> Does that mean that most Debian users do not yet have that change B> in their /etc/srp_daemon.conf and when they upgrade and the B> packaged version of /etc/srp_daemon.conf gets installed that SRP B> login to remote systems will not occur anymore after a reboot?

The pre/postinst files of the new package version are written, such that the new config file/behavior has to be explicitly enabled by the admin when the package is updated. Only new installations get the new config file automatically.

B> Since you wrote in a previous message that such changes are B> mentioned in the NEWS file: how many people do you think read B> that file before or after installing an upgrade?

The NEWS file is displayed automatically if the apt-listchanges package is installed.

Roland
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Ohh, this explains why I never saw them :)

Hello Roland,

Please revert this change such that the Debian srptools package is again in sync with the upstream srptools package and such that SRP users are saved a few hours of frustration by trying to figure out why SRP login doesn't work if they would install a version of srptools in which /etc/srp_daemon.conf disables login by default.

In case you would not be aware of this: I am the maintainer of the upstream srptools package.

Bart.--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Hi Bart,

B> On Wed, 2017-05-10 at 19:13 +0200, Roland Fehrenbacher wrote:
>> > > > > > "B" == Bart Van Assche <[email protected]> >> > > > > > writes:
>>
B> On Wed, 2017-05-10 at 18:39 +0200, Roland Fehrenbacher wrote:
>> >> The new package is very new, sits in debian unstable and most
>> >> likely hasn't made it to Ubuntu 17.04.
>>
B> Hello Roland,
>>
B> Does that mean that most Debian users do not yet have that change
B> in their /etc/srp_daemon.conf and when they upgrade and the
B> packaged version of /etc/srp_daemon.conf gets installed that SRP
B> login to remote systems will not occur anymore after a reboot?
>>
>> The pre/postinst files of the new package version are written,
>> such that the new config file/behavior has to be explicitly
>> enabled by the admin when the package is updated. Only new
>> installations get the new config file automatically.
>>
B> Since you wrote in a previous message that such changes are
B> mentioned in the NEWS file: how many people do you think read
B> that file before or after installing an upgrade?
>>
>> The NEWS file is displayed automatically if the apt-listchanges
>> package is installed.

B> Hello Roland,

B> Please revert this change such that the Debian srptools package B> is again in sync with the upstream srptools package and such that B> SRP users are saved a few hours of frustration by trying to B> figure out why SRP login doesn't work if they would install a B> version of srptools in which /etc/srp_daemon.conf disables login B> by default.

B> In case you would not be aware of this: I am the maintainer of B> the upstream srptools package.

I didn't find any of the arguments presented against this change really convincing. You make an assumption about SRP users whereas the motivation of the change came from a bug report by a real user who was frustrated with the previous config. Understandably in my opinion.

Anyway, since I'm not a DD, I can't revert it. Benjamin, since you're a DD and if you also believe the change should be reverted, please feel free to do so.

Roland

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html