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

babeld ubus output contains duplicate keys (was: luci-app-babeld: Missing neighbour) #966

Open
marek22k opened this issue Apr 11, 2023 · 8 comments

Comments

@marek22k
Copy link
Contributor

marek22k commented Apr 11, 2023

Steps to reproduce:

  1. go to: Status → Babeld tab
  2. look at the status

Actual behavior:

One neighbor is displayed.

Expected behavior:

Two neighbors are displayed as with UBUS binding:

# ubus call babeld get_neighbours
{
	"IPv4": {
		
	},
	"IPv6": {
		"fe80::2923": {
			"dev": "crxnbandura2",
			"hello-reach": 65535,
			"uhello-reach": 0,
			"rxcost": 40,
			"txcost": 40,
			"rtt": "0.000",
			"channel": -2,
			"if_up": true
		},
		"fe80::2923": {
			"dev": "crxnbandura1",
			"hello-reach": 65535,
			"uhello-reach": 0,
			"rxcost": 40,
			"txcost": 40,
			"rtt": "0.000",
			"channel": -2,
			"if_up": true
		}
	}
}

Additional Information:

I have attached a picture.
Screen Shot

OpenWrt version information from system /etc/openwrt_release

DISTRIB_ID='OpenWrt'
DISTRIB_RELEASE='22.03.3'
DISTRIB_REVISION='r20028-43d71ad93e'
DISTRIB_TARGET='ramips/mt76x8'
DISTRIB_ARCH='mipsel_24kc'
DISTRIB_DESCRIPTION='OpenWrt 22.03.3 r20028-43d71ad93e'
DISTRIB_TAINTS=''

Ping on maintainers: @jonnytischbein @PolynomialDivision @aparcar
(I am sorry if I am pinging you and you are not the maintainers, however there is nothing in the files about this and you are in the commit history).

@jow-
Copy link
Contributor

jow- commented Apr 28, 2023

It's a bug in the JSON output produced by https://github.com/openwrt/routing/blob/master/babeld/src/ubus.c - note the duplicate dictionary key fe80::2923 - we can't fix this on the LuCI side.

@jow- jow- transferred this issue from openwrt/luci Apr 28, 2023
@jow- jow- changed the title luci-app-babeld: Missing neighbour babeld ubus output contains duplicate keys (was: luci-app-babeld: Missing neighbour) Apr 28, 2023
@jow-
Copy link
Contributor

jow- commented Apr 28, 2023

@PolynomialDivision

@PolynomialDivision
Copy link
Member

PolynomialDivision commented Apr 28, 2023

It's a bug in the JSON output produced by https://github.com/openwrt/routing/blob/master/babeld/src/ubus.c - note the duplicate dictionary key fe80::2923 - we can't fix this on the LuCI side.

Yes. You are right. We could change it to [IP]%[DEV] to fix duplicate addressing? Or we sort it by the device first?

@jow-
Copy link
Contributor

jow- commented May 11, 2023

We could change it to [IP]%[DEV] to fix duplicate addressing

Sounds sensible to me! Just make sure to only do that for fe80... link local addresses, others are guaranteed to be unique on the local system.

@PolynomialDivision
Copy link
Member

PolynomialDivision commented May 13, 2023

I think it is possible that this still brings some duplicate entries with it. Maybe @jech can say something to it, how we can do the sorting in the best manner? Or how we can arrange the json output so we don't have any duplicate keys.

@marek22k
Copy link
Contributor Author

Actually, a link-local IP address must not occur twice on an interface. The DAD should exist for this purpose. Alternatively, the whole thing could perhaps be represented as an array?

@jech
Copy link

jech commented May 13, 2023

Right. Neighbours are indexed by (adress, interface). It is perfectly normal to see the same neighbour on two interfaces: if a router A has two radios tuned at the same frequency, or two Ethernet interfaces connected to a single switch, then it will see all neighbors twice, once over each interface. The protocol is designed to handle this case gracefully.

I see two solutions to your problem:

  1. Use a string of the form address%interface as the key, as @PolynomialDivision suggested.
  2. Use the address of the struct neighbor as the key. This is the approach used by Babeld's native dumping code:
    https://github.com/jech/babeld/blob/master/local.c#L163

@marek22k
Copy link
Contributor Author

marek22k commented Oct 4, 2023

Personally, I think the former suggestion is nicer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants