Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

luci-app-lldpd: Create based on TanoWrt's app #6456

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

marek22k
Copy link
Contributor

@marek22k marek22k commented Jul 4, 2023

This is a customized copy of TanoWrt's fork, which you can find at https://github.com/tano-systems/luci-app-tn-lldpd, which is under the MIT license. For the app to work, openwrt/openwrt#14193 must be accepted.

@marek22k marek22k requested a review from yichya July 17, 2023 09:40
@systemcrash
Copy link
Contributor

This is... nice. @jow-

it has prerequisites for lldp tho.

@marek22k - those filltervals for the uninitiated: those are just the most common presets, right?

@systemcrash
Copy link
Contributor

Regarding the 'presets' - would it be possible for the user to choose independently of those?

@jow-
Copy link
Contributor

jow- commented Jul 28, 2023

Please test the following ucode replacement for the Lua rpcd plugin:

'use strict';

import { popen } from 'fs';

function lldpcli_json(section) {
	return json(popen(`lldpcli -f json0 show ${section}`, 'r'));
}

return {
	lldpd: {
		getStatus: {
			call: function() {
				return {
					statistics: lldpcli_json("statistics"),
					neighbors:  lldpcli_json("neighbors details"),
					interfaces: lldpcli_json("interfaces"),
					chassis:    lldpcli_json("chassis")
				};
			}
		}
	}
};

It needs to be placed in /usr/share/rpcd/ucode/lldpd. If it works, please include it in your PR instead of the Lua plugin and remove the dependency on luci-lua-runtime (and add rpcd-mod-ucode).

@marek22k
Copy link
Contributor Author

@marek22k - those filltervals for the uninitiated: those are just the most common presets, right?

Do you mean the neighbor filter? There are all possibilities. They were taken from the matrix in the man page.

@marek22k
Copy link
Contributor Author

Please test the following ucode replacement for the Lua rpcd plugin:

Is there any particular reason to prefer ucode over Lua?

@marek22k
Copy link
Contributor Author

marek22k commented Jul 30, 2023

root@OpenWrt:~# ls -l /usr/share/rpcd/ucode/
-rwxr-xr-x    1 root     root           416 Jul 30 11:51 lldpd
Uncaught (in promise) RPCError: RPC call to lldpd/getStatus failed with error -32000: Object not found
  at handleCallReply (http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:11:3)
    raise http://192.168.122.244/luci-static/resources/luci.js?v=git-23.119.80898-65ef406:155
    handleCallReply http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:11
[luci.js:155:3](http://192.168.122.244/luci-static/resources/luci.js?v=git-23.119.80898-65ef406)
RPCError: RPC call to lldpd/getStatus failed with error -32000: Object not found
  at handleCallReply (http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:11:3)
  at promise callback*parseCallReply (http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:9:70)
  at promise callback*call (http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:5:110)
  at declare/</< (http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:23:320)
  at declare/< (http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:20:436)
  at startPolling/< (http://192.168.122.244/luci-static/resources/view/lldpd/status.js?v=git-23.119.80898-65ef406:585:11)
  at step (http://192.168.122.244/luci-static/resources/luci.js?v=git-23.119.80898-65ef406:92:38)
    raise http://192.168.122.244/luci-static/resources/luci.js?v=git-23.119.80898-65ef406:155
    handleCallReply http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:11
    promise callback*parseCallReply http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:9
    promise callback*call http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:5
    declare http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:23
    declare http://192.168.122.244/luci-static/resources/rpc.js?v=git-23.119.80898-65ef406:20
    startPolling http://192.168.122.244/luci-static/resources/view/lldpd/status.js?v=git-23.119.80898-65ef406:585
    step http://192.168.122.244/luci-static/resources/luci.js?v=git-23.119.80898-65ef406:92

Mhh, am I missing something here?

EDIT: Works after installing rpcd-mod-ucode.

@marek22k
Copy link
Contributor Author

One other thing: Since I minimally adjusted the source code in some places, the line numbers in the translation file are no longer correct. How do you regenerate them?

@marek22k
Copy link
Contributor Author

EDIT: Works after installing rpcd-mod-ucode.

Is there something like luci-ucode-runtime?

@systemcrash
Copy link
Contributor

Is there any particular reason to prefer ucode over Lua?

It's the future in openwrt. Moving away from lua.

One other thing: Since I minimally adjusted the source code in some places, the line numbers in the translation file are no longer correct. How do you regenerate them?

I think recompiling.

Do you mean the neighbor filter? There are all possibilities. They were taken from the matrix in the man page.

OK - I suppose they're common then. User being able to choose and pick themselves would be a plus.

@marek22k
Copy link
Contributor Author

marek22k commented Jul 30, 2023

I think recompiling.

Do you know how to recompile the translation files?

OK - I suppose they're common then. User being able to choose and pick themselves would be a plus.

Why? The user can choose it itself. There are no more options than are listed there. There are no more than the filters 0-19 or 1-20.
pic

@marek22k marek22k force-pushed the master branch 3 times, most recently from 1cb4c0e to 1f93194 Compare July 30, 2023 13:26
@systemcrash
Copy link
Contributor

Do you know how to recompile the translation files?

Just focus on putting your new strings in, then I think there is a build string step which will build the .po files.

Why? The user can choose it itself. There are no more options than are listed there. There are no more than the filters 0-19 or 1-20.

I see.

@howels
Copy link

howels commented Aug 18, 2023

@marek22k Would it be possible to obtain opkg files to test this? The code should be architecture-independent I think.

@systemcrash
Copy link
Contributor

systemcrash commented Aug 18, 2023 via email

@hnyman
Copy link
Contributor

hnyman commented Aug 18, 2023

If someone chooses to write up how to make an opkg of a PR, it might happen

Creating a .ipk file for opkg is the normal end result of compiling a package. Applying PRs is pretty straightforward

  • download the PR as a patch (wget https://github.com/openwrt/luci/pull/6456.patch )
  • apply the patch / commit into feeds/packages repo
  • update package info with feeds update & install, and enable the new package in menuconfig
  • compile package with make package/luci-app-lldpd/compile

And then transfer the compiled .ipk to the router.

@marek22k
Copy link
Contributor Author

@hnyman Thanks for the tutorial!

@howels If you tell me for which device I should compile it, I can try to create an IPK package.

@howels
Copy link

howels commented Aug 18, 2023

@hnyman Thanks for the tutorial!

@howels If you tell me for which device I should compile it, I can try to create an IPK package.

I am testing on these devices:
Gl.inet AR750: https://openwrt.org/toh/gl.inet/gl-ar750
Zyxel GS1900-48: https://openwrt.org/toh/hwdata/zyxel/zyxel_gs1900-48

Thanks very much, excited to try it out.

@marek22k
Copy link
Contributor Author

marek22k commented Aug 19, 2023

I used the following "instructions" to create the IPK files.
Unfortunately, this did not work for the ZyXEL device. Does anyone know what could be the reason for this?
EDIT: It seems that both GL.iNet GL-AR750 and ZyXEL GS1900-48 are "mips_24kc". Therefore the IPK packages for GL.iNet GL-AR750 are probably also compatible with those for ZyXEL GS1900-48.

# Download OpenWrt sources
git clone -b openwrt-22.03 https://github.com/openwrt/openwrt.git openwrt-22.03
cd openwrt-22.03
git checkout v22.03.5
./scripts/feeds update
./scripts/feeds install -a

# Download configuration file
# VM image
wget https://downloads.openwrt.org/releases/22.03.5/targets/x86/generic/config.buildinfo -O .config

# Gl.iNet GL-MT300N-V2
wget https://downloads.openwrt.org/releases/22.03.5/targets/ramips/mt76x8/config.buildinfo -O .config

# GL.iNet GL-AR750
wget https://downloads.openwrt.org/releases/22.03.5/targets/ath79/generic/config.buildinfo -O .config

# ZyXEL GS1900-48
wget https://downloads.openwrt.org/releases/22.03.5/targets/realtek/rtl839x/config.buildinfo -O .config

# Download the patches
wget https://github.com/openwrt/luci/pull/6456.patch -O luci-app-lldpd.patch
wget https://github.com/openwrt/openwrt/pull/13018.patch -O lldpd.patch

# Apply the patches
git -C feeds/luci am ../../luci-app-lldpd.patch
git am lldpd.patch
# Reflect changes in symlinks
./scripts/feeds install -a

make defconfig

# Enable building of packages
echo "CONFIG_PACKAGE_lldpd=m" >> .config
echo "CONFIG_PACKAGE_luci-app-lldpd=m" >> .config

make defconfig

# Compile the packages
make -j$(nproc) tools/install
make -j$(nproc) toolchain/install
make -j$(nproc) package/lldpd/compile
make -j$(nproc) package/luci-app-lldpd/compile

(Please remove the .txt extension. GitHub unfortunately does not allow .7z files)
ipks.7z.asc.txt
ipks.7z.txt

@howels
Copy link

howels commented Aug 20, 2023

I used the following "instructions" to create the IPK files. Unfortunately, this did not work for the ZyXEL device. Does anyone know what could be the reason for this? EDIT: It seems that both GL.iNet GL-AR750 and ZyXEL GS1900-48 are "mips_24kc". Therefore the IPK packages for GL.iNet GL-AR750 are probably also compatible with those for ZyXEL GS1900-48.

# Download OpenWrt sources
git clone -b openwrt-22.03 https://github.com/openwrt/openwrt.git openwrt-22.03
cd openwrt-22.03
git checkout v22.03.5
./scripts/feeds update
./scripts/feeds install -a

# Download configuration file
# VM image
wget https://downloads.openwrt.org/releases/22.03.5/targets/x86/generic/config.buildinfo -O .config

# Gl.iNet GL-MT300N-V2
wget https://downloads.openwrt.org/releases/22.03.5/targets/ramips/mt76x8/config.buildinfo -O .config

# GL.iNet GL-AR750
wget https://downloads.openwrt.org/releases/22.03.5/targets/ath79/generic/config.buildinfo -O .config

# ZyXEL GS1900-48
wget https://downloads.openwrt.org/releases/22.03.5/targets/realtek/rtl839x/config.buildinfo -O .config

# Download the patches
wget https://github.com/openwrt/luci/pull/6456.patch -O luci-app-lldpd.patch
wget https://github.com/openwrt/openwrt/pull/13018.patch -O lldpd.patch

# Apply the patches
git -C feeds/luci am ../../luci-app-lldpd.patch
git am lldpd.patch
# Reflect changes in symlinks
./scripts/feeds install -a

make defconfig

# Enable building of packages
echo "CONFIG_PACKAGE_lldpd=m" >> .config
echo "CONFIG_PACKAGE_luci-app-lldpd=m" >> .config

make defconfig

# Compile the packages
make -j$(nproc) tools/install
make -j$(nproc) toolchain/install
make -j$(nproc) package/lldpd/compile
make -j$(nproc) package/luci-app-lldpd/compile

(Please remove the .txt extension. GitHub unfortunately does not allow .7z files) ipks.7z.asc.txt ipks.7z.txt

Working really well here, the protocol initially didn't come up but I noticed that no interfaces were added in settings. After adding the correct interfaces via Luci it worked perfectly and /etc/config/lldpd is updated and read properly to set lldpd interfaces.

@howels
Copy link

howels commented Aug 21, 2023

Tried to compile this on 23.05.0-rc3 but getting errors on the new luci-app-lldpd package:

$ make -j1 V=sc  package/luci-app-lldpd/compile
make[2]: Entering directory '/home/whatever/git/openwrt/scripts/config'
make[2]: 'conf' is up to date.
make[2]: Leaving directory '/home/whatever/git/openwrt/scripts/config'
make[1]: Entering directory '/home/whatever/git/openwrt'
cd "/home/whatever/git/openwrt"; git log --format=%h -1 toolchain > /home/whatever/git/openwrt/tmp/.ver_check
cmp -s /home/whatever/git/openwrt/tmp/.ver_check /home/whatever/git/openwrt/staging_dir/toolchain-mips_24kc_gcc-12.3.0_musl/stamp/.ver_check || { \
	rm -rf /home/whatever/git/openwrt/build_dir/target-mips_24kc_musl /home/whatever/git/openwrt/staging_dir/target-mips_24kc_musl /home/whatever/git/openwrt/staging_dir/toolchain-mips_24kc_gcc-12.3.0_musl /home/whatever/git/openwrt/build_dir/toolchain-mips_24kc_gcc-12.3.0_musl; \
	mkdir -p /home/whatever/git/openwrt/staging_dir/toolchain-mips_24kc_gcc-12.3.0_musl/stamp; \
	mv /home/whatever/git/openwrt/tmp/.ver_check /home/whatever/git/openwrt/staging_dir/toolchain-mips_24kc_gcc-12.3.0_musl/stamp/.ver_check; \
}
make[1]: *** No rule to make target 'package/luci-app-lldpd/compile'.  Stop.
make[1]: Leaving directory '/home/whatever/git/openwrt'
make: *** [/home/whatever/git/openwrt/include/toplevel.mk:232: package/luci-app-lldpd/compile] Error 2

LLDPD compiles fine as do other Luci app packages, so something is missing for this new package?

I need to build a version for my GS1900 because this is running snapshot due to an error in the 22.03 release on this product which can cause bootloops.

@marek22k
Copy link
Contributor Author

Mhh, I'm not an expert in building OpenWrt packages either. I would suggest to try the following instructions after applying the patch.

./scripts/feeds install -a

echo "CONFIG_PACKAGE_luci-app-lldpd=m" >> .config

make defconfig

@howels
Copy link

howels commented Aug 22, 2023

Mhh, I'm not an expert in building OpenWrt packages either. I would suggest to try the following instructions after applying the patch.

./scripts/feeds install -a

echo "CONFIG_PACKAGE_luci-app-lldpd=m" >> .config

make defconfig

thanks. I had followed the commands but somehow repeating those 3 made the build work. appreciate the pointers.

@howels
Copy link

howels commented Aug 22, 2023

Unfortunately the resulting packages give some errors on 23.05.0

root@GS1900-48:/tmp# opkg install lldpd_1.0.17-1_mips_24kc.ipk 
Installing lldpd (1.0.17-1) to root...
Configuring lldpd.
Collected errors:
 * resolve_conffiles: Existing conffile /etc/config/lldpd is different from the conffile in the new package. The new conffile will be placed at /etc/config/lldpd-opkg.
root@GS1900-48:/tmp# opkg install luci-app-lldpd_git-23.233.35287-d022eae_all.ipk 
Installing luci-app-lldpd (git-23.233.35287-d022eae) to root...
Configuring luci-app-lldpd.
uci: Parse error (invalid command) at line 20, byte 1

The page does not show up in Luci after this error. Tried RC2 and RC3 codedbases but get the same error.

@marek22k
Copy link
Contributor Author

Mhh, the only thing I could do would be to try the whole thing myself in a VM. Maybe someone else here knows how to fix the bug? What I also wonder is which line 20 of uci is meant?

I could also imagine that it is somehow because the whole it is snaphot and RC?!

Actually I thought the Luci Web App should be quite version independent.

This is a copy of https://github.com/tano-systems/luci-app-tn-lldpd, which is licensed under the MIT License.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Rename luci-app-tn-lldpd to luci-app-lldpd

The original version of Tanosystem has a naming scheme which does not correspond to the standard naming scheme in OpenWrt LuCi. Therefore the renaming.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Fix bug not getting the current lldpd status

Specifying the arguments in the wrong order can (and has in my tests) resulted in errors. So you can see in the man page that flags like -f come first and then the command.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Disable option to enable SNMP agent

SNMP agent support is not enabled by default in lldpd. This can (and has in my tests) cause LLDP to stop working. See comment in source code.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Add option to disable sending sensitive information

Added an option that sets the "-k" flag, which results in less sensitive information being sent. See man pages and description in source code.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Add license information to Makefile

All of Tano Systems source code for the app is licensed under the MIT License. This has now been indicated accordingly in the Makefile.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Remove old footer from Tano Systems

The app from Tano Systems appears to include a footer. Since most LuCi apps do not, I have removed it.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Bug fix caused that the management IP address could not be set.

The TanoWrt fork calls the option to set the management IP addresses "lldp_sys_mgmt_ip". However, in OpenWrt it is called "lldp_mgmt_ip".

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Improve style

Remove double space

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Adding the Lua dependency

The rpcd script used by the Luci app is written in Lua. Therefore, runtime dependencies for Lua must exist when using the Luci app.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Remove old files

Since the app is called luci-app-lldpd and not luci-app-tn-lldpd, the files for the TanoWrt app are no longer necessary.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: RPCd backend change from Lua to ucode

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Update filter matrix

The old filter matrix was based on the man pages, but the selected value in the range 1-20 was also assigned to the configuration and thus lldpd was configured incorrectly. lldpd expects a value in the range 0-19. The webapp matrix has now been changed so that the values 0-19 also appear there.
This leads to a unification of configuration and webapp.

Signed-off-by: Marek Küthe <m.k@mk16.de>

luci-app-lldpd: Add location parameter

Signed-off-by: Marek Küthe <m.k@mk16.de>
@marek22k
Copy link
Contributor Author

If someone wants to test the new changes:
Packages.zip
Packages.zip.asc.txt (Remove the .txt extension)

@systemcrash
Copy link
Contributor

systemcrash commented Oct 22, 2023 via email

@marek22k
Copy link
Contributor Author

OpenWrt 22.03 with the latest changes from me to lldpd

@systemcrash systemcrash added depends on other pr pull request depends on another pull request feature pull request adding a new feature WIP pull request the author is still working on labels Dec 5, 2023
@systemcrash
Copy link
Contributor

@marek22k would be good if you re-test this GUI with what goes in via openwrt/openwrt#14193 to ensure they're synched up.

@systemcrash systemcrash added depends on PR in other repo PR depends on PR in sister repo e.g. openwrt/packags and removed depends on other pr pull request depends on another pull request labels Jan 30, 2024
@howels
Copy link

howels commented Feb 8, 2024

openwrt/openwrt#14193 is now in master, so this Luci App can be worked on @marek22k

@howels
Copy link

howels commented Feb 8, 2024

If anyone is interested here is a build I did for 23.05
luci-app-lldpd.zip

@howels
Copy link

howels commented Feb 8, 2024

I noticed an off-by-one error on the filter selection list. You select one version, apply and then it shows the next row up as selected.

@systemcrash systemcrash removed the depends on PR in other repo PR depends on PR in sister repo e.g. openwrt/packags label Feb 8, 2024
@systemcrash
Copy link
Contributor

systemcrash commented Feb 8, 2024

I noticed an off-by-one error on the filter selection list. You select one version, apply and then it shows the next row up as selected.

Tested also, and it seems to be this row:

		var selected = parseInt(cfgvalue) - 1;

Which causes off by one :)

@systemcrash systemcrash added the depends on PR in other repo PR depends on PR in sister repo e.g. openwrt/packags label Feb 8, 2024
@systemcrash
Copy link
Contributor

This PR now needs the following PRs:
openwrt/openwrt#14583
And optionally:
openwrt/openwrt#14584

@systemcrash
Copy link
Contributor

systemcrash commented Mar 11, 2024

openwrt/openwrt#14583 PR down, and optional openwrt/openwrt#14584 down. Note to self: I have a number of fixes that must go in on top of this PR (there are a few quirks and bugs present).

@systemcrash systemcrash added depends on PR in other repo PR depends on PR in sister repo e.g. openwrt/packags and removed WIP pull request the author is still working on depends on PR in other repo PR depends on PR in sister repo e.g. openwrt/packags labels Mar 11, 2024
@systemcrash
Copy link
Contributor

systemcrash commented Mar 12, 2024

Well, shit. The lldpd init script it turns out leaves a few things to be desired:

openwrt/openwrt#14850 - merged
openwrt/openwrt#14867 - merged

More bugs 🙄 Once these are in, the init script should have parity with this GUI.

@howels
Copy link

howels commented Apr 11, 2024

Well, shit. The lldpd init script it turns out leaves a few things to be desired:

openwrt/openwrt#14850 - merged openwrt/openwrt#14867

More bugs 🙄 Once these are in, the init script should have parity with this GUI.

Looks like this is working already, only issues are parsing complex interface patterns. Could this be merged so we can expose it to a wider audience and get more testers using it?

@howels
Copy link

howels commented Apr 22, 2024

Last obstacle merged - please have a look at this PR!

@systemcrash systemcrash merged commit 49207b4 into openwrt:master Apr 22, 2024
@systemcrash
Copy link
Contributor

Done. Finally. Take it for a spin. I can't cherry-pick to 23 or 22 just yet, since those parts that are in openwrt:master need to get cherry-picked first to 23/22. That's largely straight-forward because I tested all additions on both.

A few more PRs should go in before the CPs start. Namely:

openwrt/openwrt#15234 - waiting
openwrt/openwrt#14872 - still draft. Want to elicit some more feedback for how best to structure TLV handling in UCI config.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depends on PR in other repo PR depends on PR in sister repo e.g. openwrt/packags feature pull request adding a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants