xddxdd / bird-lg-go

BIRD looking glass in Go, for better maintainability, easier deployment & smaller memory footprint
GNU General Public License v3.0
141 stars 27 forks source link

bgpmap shows the wrong protocol name as preferred for iBGP routes #75

Closed jlu5 closed 1 year ago

jlu5 commented 1 year ago

I noticed this when updating to v1.2.0.

Example: https://lg.highdef.network/route_all/las/172.20.0.53 shows

Table master4:
172.20.0.53/32       unicast [ibgp_sjc2 2023-04-29 from fd86:bad:11b7:22::1] * (100/39) [AS4242423914i]
    via 169.254.108.122 on igp-sjc2
    Type: BGP univ
    BGP.origin: IGP
    BGP.as_path: 4242423914
    BGP.next_hop: 172.20.229.122
    BGP.med: 50
    BGP.local_pref: 100
    BGP.community: (64511,1) (64511,24) (64511,34)
    BGP.large_community: (4242421080, 101, 44) (4242421080, 103, 122) (4242421080, 104, 1)
                     unicast [miaotony_2688 2023-04-29 from fe80::2688] (100) [AS4242423914i]
    via 172.23.6.6 on dn42las-miaoton
    Type: BGP univ
    BGP.origin: IGP
    BGP.as_path: 4242422688 4242423914
    BGP.next_hop: 172.23.6.6
    BGP.med: 50
    BGP.local_pref: 100
    BGP.community: (64511,3) (64511,24) (64511,34)
    BGP.large_community: (4242421080, 104, 1) (4242421080, 101, 44) (4242421080, 103, 126)
-snip-
                     unicast [yura_2464 2023-04-29] (100) [AS4242423914i]
    via fe80::2464 on dn42las-yura
    Type: BGP univ
    BGP.origin: IGP
    BGP.as_path: 4242422464 4242423914
    BGP.next_hop: :: fe80::2464
    BGP.med: 50
    BGP.local_pref: 100
    BGP.community: (64511,1) (64511,24) (64511,34)
    BGP.large_community: (4242422464, 2, 4242423914) (4242422464, 64511, 44) (4242422464, 64511, 1840) (4242421080, 104, 1) (4242421080, 101, 44) (4242421080, 103, 126)
                     unicast [ibgp_fra 2023-04-29 from fd86:bad:11b7:117::1] (100/187) [AS4242423914i]
    via 169.254.108.113 on igp-chi
    Type: BGP univ
    BGP.origin: IGP
    BGP.as_path: 4242423914
    BGP.next_hop: 172.20.229.117
    BGP.med: 50
    BGP.local_pref: 100
    BGP.community: (64511,1) (64511,24) (64511,34)
    BGP.large_community: (4242421080, 101, 41) (4242421080, 103, 117) (4242421080, 104, 3)
                     unicast [ibgp_sgp 2023-04-29 from fd86:bad:11b7:239::1] (100/200) [AS4242423914i]
    via 169.254.108.39 on igp-sgp
    Type: BGP univ
    BGP.origin: IGP
    BGP.as_path: 4242423914
    BGP.next_hop: 172.22.108.39
    BGP.med: 50
    BGP.local_pref: 100
    BGP.community: (64511,4) (64511,24) (64511,34)
    BGP.large_community: (4242421080, 101, 51) (4242421080, 103, 39) (4242421080, 104, 4)
                     unicast [ibgp_ymq 2023-04-30 from fd86:bad:11b7:23::1] (100/106) [AS4242423914i]
    via 169.254.108.113 on igp-chi
    Type: BGP univ
    BGP.origin: IGP
    BGP.as_path: 4242423914
    BGP.next_hop: 172.20.229.123
    BGP.med: 50
    BGP.local_pref: 100
    BGP.community: (64511,3) (64511,24) (64511,34)
    BGP.large_community: (4242421080, 101, 42) (4242421080, 103, 123) (4242421080, 104, 2)
                     unicast [cola_3391 08:28:23.795 from fe80::3391] (100) [AS4242423914i]
    via 172.22.96.65 on dn42-cola
    Type: BGP univ
    BGP.origin: IGP
    BGP.as_path: 4242423391 4242420604 4242423914
    BGP.next_hop: 172.22.96.65
    BGP.med: 50
    BGP.local_pref: 100
    BGP.community: (64511,4) (64511,34) (64511,24)
    BGP.large_community: (4242420604, 2, 50) (4242420604, 501, 4242423914) (4242420604, 502, 44) (4242420604, 504, 4) (4242421080, 104, 1) (4242421080, 101, 44) (4242421080, 103, 126)

such that the preferred route comes from ibgp_sjc2

But bgpmap on the same query incorrectly shows that the preferred route comes from ibgp_ymq

example

xddxdd commented 1 year ago

The edges in graph were deduplicated using (src, dst) pair. When multiple edges (routes) with the same path exist, the later occurrence will overwrite the label of the previous occurrence.

Fixed by refactoring graphing code to deduplicate based on (src, dst, label).

jlu5 commented 1 year ago

I tried the latest commit on my Bird instance and it seems to have made the rendering worse :(

bird-lg-go-new

xddxdd commented 1 year ago

Can you check if your whois server is working? Looks like whois lookup failed, and the name of every intermediate ASN is replaced with an empty string.

xddxdd commented 1 year ago

I added a test case based on your bird output for this query, and verified that the BGP routes are parsed correctly.

My current guess is that something went wrong in your ASN whois lookup process, and the name for every node was resolved into an empty string. In the latest commit, I refactored the graphing code to use plain ASNs as internal node identifiers, instead of whois results. If my theory is correct, you will get the correct graph topology, but with empty names for all the nodes.

(I also changed the behavior of handling multiple edges between same src/dst pair. Now the labels for these edges will be combined, and the result graph should be less cluttered.)

jlu5 commented 1 year ago

Hmm, with latest master the graph renders, but I'm not sure why the whois info is broken. I haven't changed any settings, and that part loads fine with the v1.2.0 release binary.

I use:

BIRDLG_WHOIS=172.22.0.43

For edge display, it probably makes more sense to hide the interface info for nodes after the immediate next hop?

jlu5 commented 1 year ago

bird-lg-go-new

xddxdd commented 1 year ago

Try the latest commit, the whois lookup bug is caused by a mistake during the initial refactoring.

jlu5 commented 1 year ago

Yep, it works nicely now. Thanks for the quick follow ups! bird-lg-go-new