Bug 189824 - [new port] net-mgmt/lldpd
Summary: [new port] net-mgmt/lldpd
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: Normal Affects Only Me
Assignee: Rodrigo Osorio
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-05-15 01:40 UTC by User &
Modified: 2015-02-05 18:45 UTC (History)
3 users (show)

See Also:


Attachments
lldpd.shar (4.18 KB, text/plain)
2014-05-15 01:40 UTC, User &
no flags Details
GIDs.patch (240 bytes, patch)
2014-05-15 01:40 UTC, User &
no flags Details | Diff
lldp port staged (5.92 KB, text/plain)
2014-12-28 10:25 UTC, Rodrigo Osorio
no flags Details
UID and GID changes (913 bytes, patch)
2014-12-28 10:35 UTC, Rodrigo Osorio
no flags Details | Diff
lldpd 0.7.13 (6.46 KB, patch)
2014-12-30 22:21 UTC, Mathieu Simon
no flags Details | Diff
lldpd 0.7.13 (6.33 KB, patch)
2014-12-31 13:06 UTC, Mathieu Simon
no flags Details | Diff
lldpd 0.7.13 (6.34 KB, patch)
2015-01-05 13:09 UTC, Mathieu Simon
no flags Details | Diff
lldpd 0.7.13 (rev. 4) (6.21 KB, text/plain)
2015-01-06 17:15 UTC, Mathieu Simon
no flags Details
updated diff for lldpd (6.21 KB, patch)
2015-01-16 16:37 UTC, Rodrigo Osorio
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description User & 2014-05-15 01:40:00 UTC
LLDP is an industry standard protocol designed to supplant
proprietary Link-Layer protocols such as Extreme's EDP (Extreme
Discovery Protocol) and CDP (Cisco Discovery Protocol). The goal of
LLDP is to provide an inter-vendor compatible mechanism to deliver
Link-Layer notifications to adjacent network devices.

This implementation provides LLDP sending and reception, supports
VLAN and includes an SNMP subagent that can interface to an SNMP
agent through AgentX protocol. It is more modern than openlldp 
and supports JSON and XML output.

This daemon is also able to deal with CDP, SONMP, FDP and EDP
protocol. It also handles LLDP-MED extension.

Fix: --- /usr/ports/UIDs	2014-04-11 20:51:04.000000000 +0200
+++ /usr/ports/UIDs	2014-05-15 02:08:42.401175712 +0200
@@ -205,6 +205,7 @@
 riak:*:667:667::0:0:Riak user:/usr/local/lib/riak:/bin/sh
 bnetd:*:700:700::0:0:Bnetd user:/nonexistent:/usr/sbin/nologin
 bopm:*:717:717::0:0:Blitzed Open Proxy Monitor:/nonexistent:/bin/sh
+_lldpd:*:720:720::0:0:lldpd user:/nonexistent:/usr/sbin/nologin
 openxpki:*:777:777::0:0:OpenXPKI Owner:/nonexistent:/usr/sbin/nologin
 foreman_proxy:*:812:812::0:0:Foreman Smart Proxy:/usr/local/share/foreman-proxy:/usr/sbin/nologin
 puppet:*:814:814::0:0:Puppet Daemon:/nonexistent:/usr/sbin/nologin
--- UIDs.patch ends here ---
Comment 1 John Marino freebsd_committer 2014-08-07 15:00:25 UTC
Hi, if you are still interested in having this port in FreeBSD, it may (or may not) need to be reworked to support stage, and it may need updating to other newer conventions such as "USES" which is expanding all time.
For staging, see http://lists.freebsd.org/pipermail/freebsd-ports-announce/2014-May/000080.html


Additionally, you need to provide some sort of quality assurance.    
In order of preference, we are looking for:

1) "poudriere testport" or "poudriere bulk -t" logs
2) Redports or tinderbox logs
3) at least this: https://www.freebsd.org/doc/en/books/porters-handbook/porting-testing.html

Please provide an updated shar file and attach a test log.  Alternatively, please indicate if you are no longer interested in having this software in the Ports Collection and that we can close the PR.

Thanks!
Comment 2 John Marino freebsd_committer 2014-08-24 19:33:12 UTC
this port seems staged and modern.  A test log would fast-track it into the tree.
Comment 3 Rodrigo Osorio freebsd_committer 2014-12-28 10:25:18 UTC
Created attachment 151028 [details]
lldp port staged

Fix staging issues
Makes portlint & poudriere happy
Comment 4 Rodrigo Osorio freebsd_committer 2014-12-28 10:35:11 UTC
Created attachment 151029 [details]
UID and GID changes

Add user and group
Comment 5 Rodrigo Osorio freebsd_committer 2014-12-28 10:37:19 UTC
Added a new shar fixing staging issues
and a patch with the new user an group

@wartung: please let me know if it's ok or make the required changes
Comment 6 Mathieu Simon 2014-12-30 22:19:22 UTC
*dou* Just when you want to open a bugreport to submit your work, you realize that someone else has been working on that too. ;-) Anyhow it's already been a good exercise up to this point for getting a better idea of making ports.

I see that the proposed port would add lldpd 0.7.8, however I'd have it ready for 0.7.13. This latest release by the upstream developer allows portlint-cleanness without having to add cruft to the port.

I'm happy to do whatever is needed to make go into the ports tree and am forward to feedback.

Some data on my variant:
- Passes "portlint -AC"
- Builds and passes "poudriere testport" on 8.4, 9.1-9.3 and 10.0-10.1
- Has been actually tested with FreeBSD 10.1 amd64 with a an LLDP (lldpd on Debian jessie) and to a lesser degree with CDP peer (Cisco 3550).

Best regards
Mathieu
Comment 7 Mathieu Simon 2014-12-30 22:21:21 UTC
Created attachment 151133 [details]
lldpd 0.7.13

As independently (without the knowledge of the work for wartung@) written for lldpd 0.7.13
Comment 8 Rodrigo Osorio freebsd_committer 2014-12-30 23:19:36 UTC
(In reply to mathieu.sim from comment #7)
> Created attachment 151133 [details]
> lldpd 0.7.13
> 
> As independently (without the knowledge of the work for wartung@) written
> for lldpd 0.7.13

Good job, poudriere agrees :)
Comment 9 Mathieu Simon 2014-12-31 08:02:36 UTC
Thanks for the feedback, only now I've spotted a slight bug in the rc script which should have been 'command=%%PREFIX%%/sbin/${name}'.

Quick rationale on the options and differences:
- PROPRIETARY is offered for paranoid folks who fear vendor' protocol-detector  
  vans. Even Debian folks ship lldpd with compile-time enabled CDP support.
- lldpd normally puts its chroot, pid file and ctl-socket to 
  /usr/local/var/run. I was told on IRC that /var/run is preferred on FreeBSD
  that implies the 3 switches. If directories miss, lldpd won't launch
- with-pkgconfigdir has been added by upstream after to make our portlint 
  happy for pkgconf (lldpd #87)
- pkg-descr was obtained from OpenBSD's lldpd DESCR (author: sthen@OpenBSD)
Comment 10 Rodrigo Osorio freebsd_committer 2014-12-31 09:16:42 UTC
(In reply to mathieu.sim from comment #9)
> Thanks for the feedback, only now I've spotted a slight bug in the rc script
> which should have been 'command=%%PREFIX%%/sbin/${name}'.

Fixed.

> 
> Quick rationale on the options and differences:

regarding the options, I think most of them can be set by default.
Take in mind we uses binary packages most of the time, and only few peoples
rebuild the ports for options.

I suggest at least JSON PROPRIETARY SNMP XML BASH
Comment 11 Mathieu Simon 2014-12-31 10:12:45 UTC
Hi Rodrigo

lldpd itself is pretty small (< 1MB), unfortunately something such as sub-packages like Debian isn't (as of writing) possible so your proposed defaults draw quite some dependencies:

- BASH: We'd have to drop the dependency on bash-completion (depends on bash) as otherwise installing lldpd draws in full-blown bash. - bapt@ on IRC told me so when I asked (why not add ZSH to defaults BTW ?)
- SNMP draws in a full Perl as net-snmp depends on it, rather huge one (i.e. Debian's lldpd depends on smaller libsnmp30)

Anyhow I'm fine with doing so, however as soon as sub-packages get possible I'd want to rework the list of dependencies to limit the drawn-in dependencies. I'll update the patch, I've also spotted potential for simplification as the upstream author recommends using /var/empty for the chroot (as did wartung@)

-- Mathieu
Comment 12 Mathieu Simon 2014-12-31 13:06:02 UTC
Created attachment 151156 [details]
lldpd 0.7.13

Update the patch to add lldpd, changes are:

- files/lldpd.in: /usr/local -> %%PREFIX%%
- Modify options as suggested by rodrigo@
  - Removes dependency on bash-completion
- Fix SNMP option
- Improves pkg-message

If you want to get the commit history and comments, see my github:
https://github.com/matsimon/freebsd-ports-dev/commits/lldpd-proposal

Note: I'm not fully certain the SNMP option fully works as it yields some errors lldpd_flags contains '-x' that enables SNMP subagent at runtime - a feature I never used so far and am not particularly familiar with snmpd:

Dec 31 14:00:14 freebsd101-testbox lldpd[16522]: Warning: Failed to connect to the agentx master agent ([NIL]):

-- Mathieu
Comment 13 Rodrigo Osorio freebsd_committer 2015-01-01 22:08:51 UTC
(In reply to mathieu.sim from comment #12)
> Created attachment 151156 [details]
> lldpd 0.7.13
> 
> Update the patch to add lldpd, changes are:


Thanks I'll take a look
Comment 14 Rodrigo Osorio freebsd_committer 2015-01-02 14:23:43 UTC
Many problems in your last version preventing the port compilation
The files with my fixes : http://files.bebik.net/patches/lldpd-0.7.13.tgz

Next time use poudriere or redports before the submition
Comment 15 Mathieu Simon 2015-01-05 13:09:27 UTC
Created attachment 151353 [details]
lldpd 0.7.13

Hi

I'm really sorry Rodrigo, yes indeed I did not go back to poudriere testport after the modifications. BTW: redports is likely dead for quite some time now and hasn't been resurrected (mailing list archives confirms so).

Regarding your changes - let me say thanks - however:
- *_LIB_DEPENDS: Didn't see that in the docs until now, it simplifies 
  the port as it allows dropping explicit build dependencies, cool.
  However concerning the net-snmp dependency I'm not 100% certain it 
  it's just the library dependency but more (as previously written, I haven't 
  really tested the SNMP part). If you are certain about the change however, 
  I'm OK with that - then again it then depends on 2 additional 
  libnetsnmp libraries.

- JSON_CONFIGURE_WITH vs. _CONFIGURE_ENABLE: I've checked back with the
  configure script where it clearly says "--with-json". If I take your
  modification it builds, but doesn't enable JSON as it doesn't look 
  for the presence of libjansson.

- JSON_BUILD_DEPENDS on pkgconf: Unfortunately IMHO that one is still 
  required for the build to pass. It needs it to find the jansson library 
  properly. Likely due to the previous CONFIGURE_ENABLE issue it seemed 
  as if you could drop that one.

- Removal of the >=2 dependency on jansson: It's the only library the 
  configure script explicitely checks for a version. If you think that 
  it might never happen to have such an old jansson in the ports tree, I'm fine.
  (seems *_lib_depends can't do version dependency, that's why I then chose 
  to depend on jansson>=2)

Attached is a new patch, poudriere logs can be found here:
https://gist.github.com/matsimon/215f1794a9b3c91bbf46

Looking forward to hearing back from you
Mathieu
Comment 16 Rodrigo Osorio freebsd_committer 2015-01-05 14:05:13 UTC
> I'm really sorry Rodrigo, yes indeed I did not go back to poudriere testport 
No problem :). It's just an advice for your future submissions.

> Regarding your changes - let me say thanks - however:
> - *_LIB_DEPENDS: Didn't see that in the docs until now, it simplifies 
>   the port as it allows dropping explicit build dependencies, cool.
>   However concerning the net-snmp dependency I'm not 100% certain it 
>   it's just the library dependency but more (as previously written, I haven't 
>   really tested the SNMP part). If you are certain about the change however, 
>   I'm OK with that - then again it then depends on 2 additional 
>   libnetsnmp libraries.
ya, LIB_DEPEND means the package is checked during build and install.
About SNMP there is a misunderstanding, we don't install part of a package.
In XXX_DEPEND= a:b, 'a' is the minimal dependency, a way to check if the
package has to be installed. 'b' is the package to install if 'a' is not found.
test 'libnetsnmp' availability is enough to determine if the snmp package must be installed, with all the binaries. No extra check needed.

> - JSON_CONFIGURE_WITH vs. _CONFIGURE_ENABLE: I've checked back with the
>   configure script where it clearly says "--with-json". If I take your
>   modification it builds, but doesn't enable JSON as it doesn't look 
>   for the presence of libjansson.
JSON_CONFIGURE_WITH breaks build in my poudriere when JSON_CONFIGURE_ENABLE
works all the time. But I didn't investigate too much this problem.

> - JSON_BUILD_DEPENDS on pkgconf: Unfortunately IMHO that one is still 
>   required for the build to pass. It needs it to find the jansson library 
>   properly. Likely due to the previous CONFIGURE_ENABLE issue it seemed 
>   as if you could drop that one.
See remark in #1, LIB_DEPEND means the dependency is checked at build and at install (cf Porter Handbook 5.8.1)

> - Removal of the >=2 dependency on jansson: It's the only library the 
It was usefull in the past or if we have multiples jansson versions in the port,
but pkg make it a little bit useless. But I agree this can be reintroduce.
Comment 17 Mathieu Simon 2015-01-06 17:15:53 UTC
Created attachment 151403 [details]
lldpd 0.7.13 (rev. 4)

Hi Rodrigo

I hope this patch adresses the your concerns :-)

- Replace JSON_BUILD_DEPENDS= by JSON_USES= pkgconfig as the macro induces a build dependency on pkgconfig, thank you. It needs pkgconf to properly detect the presence of libjanson.so.
- Drop unnessesary dependencies on net-snmp libraries as you explained, I know we don't have sub-packages or something like Debian Linux has. I happen to fall into thinking a bit this way. *duck*
- Re-ran poudriere testport

Concerning your issues with jansson: For short time I also had issues building (fetching) jansson during testport. For these tests I pointed it to a fresh ports tree and copied the lldpd port into it, then it worked reliably. - At least for me.

Testport logs are updated, same link as previously [1], additionally I've added a log where I removed the pkgconfig dependency so it's visible where it fails. [2]

Thanks and looking forward,
Mathieu

[1] https://gist.github.com/matsimon/215f1794a9b3c91bbf46
[2] https://gist.github.com/matsimon/215f1794a9b3c91bbf46#file-10-1-without-pkgconfig-dependencies
Comment 18 Rodrigo Osorio freebsd_committer 2015-01-16 14:21:03 UTC
Thanks. The patch looks good and work as intended,
Give me an extra time to look in detail.
Comment 19 Rodrigo Osorio freebsd_committer 2015-01-16 16:37:39 UTC
Created attachment 151740 [details]
updated diff for lldpd

New diff Mathieu submit by email on Mon, January 12, 2015
Comment 20 commit-hook freebsd_committer 2015-02-05 17:03:22 UTC
A commit references this bug:

Author: rodrigo
Date: Thu Feb  5 17:03:18 UTC 2015
New revision: 378477
URL: https://svnweb.freebsd.org/changeset/ports/378477

Log:
  Add lldp, an implementation of the LLDP industry protocol

  PR:		189824
  Submitted by:	freebsd@simweb.ch
  Reviewed by:	danfe

Changes:
  head/GIDs
  head/UIDs
  head/net-mgmt/Makefile
  head/net-mgmt/lldpd/
  head/net-mgmt/lldpd/Makefile
  head/net-mgmt/lldpd/distinfo
  head/net-mgmt/lldpd/files/
  head/net-mgmt/lldpd/files/README.bsd
  head/net-mgmt/lldpd/files/lldpd.in
  head/net-mgmt/lldpd/pkg-descr
  head/net-mgmt/lldpd/pkg-message
  head/net-mgmt/lldpd/pkg-plist
Comment 21 Rodrigo Osorio freebsd_committer 2015-02-05 17:03:56 UTC
committed, thanks.
Comment 22 Mathieu Simon 2015-02-05 18:45:08 UTC
Hi

The maintainer adress is now active, thank you for your the review and Feedback.

-- Mathieu