NLNOG / lg.ring.nlnog.net

Source code for the NLNOG Looking Glass front end
https://lg.ring.nlnog.net
ISC License
43 stars 64 forks source link

AS paths with { } fail to create a map #87

Closed tbaschak closed 1 year ago

tbaschak commented 1 year ago

Was looking at some routes, and I noticed that ones with { } in the path don't make maps. I assume that part needs to be stripped out before being sent to the dot map creation code, or ignored in the map creation code.

Example URL: https://lg.ring.nlnog.net/prefix/map?saved=8uKX6LIG8M (2001:410::/32)

tbaschak commented 1 year ago

It looks like the lines where this logic would need to be would be: https://github.com/NLNOG/lg.ring.nlnog.net/blob/main/nlnog_lg.py#L425

athompson-merlin commented 1 year ago

Stripping the braces will result in a misleading graph, e.g. http://lg2.merlin.ca/route_bgpmap/lg2/2001:410::/32 shows the BGP confederation as a path, not a collection.

Here's what I think the graph ought to look like:

strict digraph {
lg2 [color=blue,shape=box];
target [ label="Target: 2001:410::/32" color=red, shape=diamond ];

AS271 [ label="AS271\n[CC]\n[RIR]\n[date]\n[NETNAME]" ]
AS577 [ label="AS577\nCA\narin\n1999-03-03\nBACOM, CA" ]
AS1299 [ label="AS1299\nSE\nripencc\n1993-09-01\nTWELVE99 Arelion, fka Telia Carrier, SE" ]
AS2603 [ label="AS2603\nDK\nripencc\n1993-09-30\nNORDUNET, DK" color=red];
AS6327 [ label="AS6327\nCA\narin\n1996-03-08\nSHAW, CA" color=red ]
AS6509 [ label="AS6509\nCA\narin\n1996-05-10\nCANARIE-NTN, CA" ];
AS6939 [ label="AS6939\nUS\narin\n1996-06-28\nHURRICANE, US" ];
AS7122 [ label="AS7122\nCA\narin\n1996-09-11\nMTS-ASN, CA" ];
AS7860 [ label="AS7860\nCA\narin\n1997-01-30\nUPEI-AS, CA" ];
AS8111 [ label="AS8111\nCA\narin\n1998-08-19\nDALUNIV, CA" ];
AS10972 [ label="AS10972\nCA\narin\n1998-02-18\nNF-CANET2, CA" ];
AS53904 [ label="AS53904\n[CC]\n[RIR]\n[date]\n[NETNAME]" ];

subgraph cluster1 { "AS271", "AS7860", "AS8111", "AS10972", "AS53904" } [color=red];
lg2 -> AS6327 -> AS2603 -> AS6509 -> { AS271 AS7860 AS8111 AS10972 AS53904 } -> target [color=red];

AS2603 [color=red];
AS6509 [color=red];
AS271 [color=red];
AS7860 [color=red];
AS8111 [color=red];
AS10972 [color=red];
AS53904 [color=red];

lg2 -> AS7122 -> AS577 -> AS6939;
lg2 -> AS6939 -> AS1299 -> AS2603;

lg2 -> AS6327 [fontsize=12.0,label="uwpg_rtr1_v6*\n2620:b0:0:a::4",color=red];
lg2 -> AS6939 [fontsize=12.0,label="umrtr1_v6\n2620:b0:0:f::3"];
lg2 -> AS7122 [fontsize=12.0,label="umrtr1_v6\n2620:b0:0:f::3"];     
}

paste that graph "code" into either Viz.JS or Graphviz Visual Editor to see what what I think it should look like.

A few things about dot source:

  1. Graphviz doesn't care what order the statements arrive in, it essentially compiles an internal graph representation and only then emits the output.

  2. As a direct consequence, you can also redefine and/or add attributes to existing nodes whenever you want, wherever happens to be handy. e.g. AS6509 [color=red]; adds the red color to a pre-existing node AS6509.

  3. By adding the keyword strict in front of the leading digraph term, graphviz takes care of deduplicating everything for you including edges.

  4. Strictly speaking, the subgraph cluster1 { ... } is completely unnecessary. All that does is make the layout engine try really hard to keep those nodes in the same bounding box and on the same page. It does draw a box, though, which gives a visual clue that something special is going on here.

  5. It's really too bad you can't establish relationships to the entire subgraph, it would be much nicer both generation-wise and visually, if you could do, say,

    subgraph cluster1 { A B C};
    X -> cluster1;
    cluster1 -> Y;

    but you can't.

  6. graphviz allows you to chain connections, this suggests a more straightforward way of generating DOT links from the bgpctl output. Stepwise, consider this (using UNIX commands, 'cuz that's what I know off the top of my head):

    echo 'I*>     N 2001:410::/32        2620:b0:0:a::4    100     0 6327 6327 6327 2603 6509 { 271 7860 8111 10972 53904 } i' | \
    tr -s ' ' ' '  | \                    # compress spaces, since cut doesn't understand padded fields
    cut -d' ' -f7.99 | \                  # remove all the garbage at the front
    sed -e 's/ [a-z]$//' | \              # get rid of that pesky 'i'
    sed -e 's/ \([0-9]\)/ AS\1/' | \      # put the letters "AS" in front of each number
    sed -e 's/{ [0-9 ]*}/CLUS1/' | \      # replace the first cluster with a temporary tag (in prod, we'd have to store the replaced cluster value, and it's also possible to have more than one in the path)
    sed -e 's/ / -> /g' | \               # insert the DOT links between these handily valid DOT nodes
    sed -e 's/CLUS1/{ 271 7860 8111 10972 53904 }/' | \      # put the cluster back in WITHOUT links
    sed -e 's/^/lg2 ->/' |\               # prepend ourselves
    sed -e 's/$/ -> Target [color=red];'  # append the target and the color to apply to the entire set of edges

    gives you lg2 -> 6327 -> 6327 -> 6327 -> 2603 -> 6509 -> { 271 7860 8111 10972 53904 } -> Target [color=red]; which happens to be perfectly valid and complete DOT.

teunvink commented 1 year ago

This has been fixed.