[Bug 171347] Review Request: l2tpd - Layer 2 Tunneling Protocol daemon

[bugzilla at redhat.com]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug report.

Summary: Review Request: l2tpd - Layer 2 Tunneling Protocol daemon


https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=171347





------- Additional Comments From dmitry at butskoy.name  2005-12-15 09:00 EST -------
Well.

- rm -f %{buildroot} must be at the beginning of %install section too.

> I did not add the chkconfig -add call, since I was told one shouldn't start a
> service upon install.
Here is some misunderstanding.
When we say "shouldn't start a service upon install", it means we should not do
"chkconfig l2tpd on" and "service l2tpd start" . But we must "chkconfig --add
l2tpd"!
See
http://fedoraproject.org/wiki/ScriptletSnippets#head-55b46ef483e6a08c24a8fc3b0b7e2ef7bfb84efd
, for this.
As l2tpd.init has "chkconfig: - 80 30", the "-" means that this service will be
"off" after install (try "chkconfig l2tpd --list" to make sure of it). So, don't
worry about it.

For this package, all the recommendations according to
http://fedoraproject.org/wiki/ScriptletSnippets#head-24ef9d59bda6032df14cf3cb433ce4ef09348f69
are acceptable.
Also, don't check .pid file at %preun etc., IMO it is not needed (l2tpd.init
works fine without this workaround), and it is not recommended way.

Nitpicks:
- defattr should be "%defattr(-,root,root,-)" (with the last ",-").

- l2tpd-chapsecrets.sample has 0600 permissions, which cause rpmlint to be
confused a little on the source rpm. As you explicitly "install -m600" this
file, too restrictive permissions of the source are not needed.
  I still suggest to get rid of chap-secrets.example at all. If you consider it
is needed, then just move it to %doc

- I would prefer to use "make" instead of "%{__make}", IMHO the last variant is
less clean and is not actually useful in Fedora.

- to make rpmlint more happy, remove DOS'ish '\r' from CREDITS file.

All suggested changes are in the following patch.


> I guess we can do 127.0.0.1 but I will need to add a comment to it that this
> is made clear to be the internal ip of the gateway.
Well, as user should set "ip-range" and "local-ip" to his/her specific values
anyway, it would be not much hard to set or comment-out the listen-address too.
At least, the user will be noticed about possible security risks (reading a
comment you will write), and will remember this notation (as he/she has been
compelled not only to read the comment, but also to change the listen-address
manually :)).


> As for your l2tpd/ppp problems, please give me a log of the l2tpd -D output,
> and if possible an 'ipsec barf' output.
This bug is more rare rather than repeated. I'll try to catch something later.







-- 
Configure bugmail: https://bugzilla.redhat.com/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.