feat: generalize bgp server to allow any net.Listener

[tobikris]

This allows to implement in-memory bgp servers using the same code.
They can be used for testing bio-routing as well as some other use cases.

[codecov-commenter]

Codecov Report

Merging #398 (b554c27) into master (91dfd06) will increase coverage by 3.40%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##           master     #398      +/-   ##
==========================================
+ Coverage   55.14%   58.55%   +3.40%     
==========================================
  Files         147      147              
  Lines        7672     7702      +30     
==========================================
+ Hits         4231     4510     +279     
+ Misses       3245     2967     -278     
- Partials      196      225      +29     

Flag	Coverage Δ
unittests	58.55% <50.00%> (+3.40%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
protocols/bgp/server/tcplistener.go	0.00% <0.00%> (ø)
protocols/bgp/server/server.go	54.96% <60.00%> (+37.68%)	⬆️
protocols/bgp/server/peer.go	41.23% <71.42%> (+39.10%)	⬆️
protocols/bgp/server/fsm.go	67.59% <100.00%> (+39.66%)	⬆️
protocols/bgp/server/fsm_established.go	45.80% <0.00%> (+2.29%)	⬆️
protocols/bgp/server/fsm_address_family.go	78.04% <0.00%> (+2.43%)	⬆️
protocols/bgp/server/fsm_open_sent.go	52.09% <0.00%> (+11.37%)	⬆️
protocols/bgp/server/fsm_open_confirm.go	16.90% <0.00%> (+14.08%)	⬆️
protocols/bgp/server/peer_role.go	28.57% <0.00%> (+28.57%)	⬆️
protocols/bgp/server/fsm_active.go	28.57% <0.00%> (+28.57%)	⬆️
... and 6 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[tobikris]

I would like to add the router ID to all log lines originating from a BGP server instance. The direct calls are trivial, do you think I should do something similar for the deeper calls like the FSM logging?

go run ./examples/bgp-in-memory
INFO[0000] Starting memory server: 172.17.0.3           
INFO[0000] Starting memory server: 169.254.200.0        
DEBU[0000] setTCPMD5 called on dummyListener, ignoring...  router_id=172.17.0.3
INFO[0000] Added BGP peer                                local_address=172.17.0.3 local_as=65100 peer_address=169.254.200.0 peer_as=65101 router_id=172.17.0.3
DEBU[0000] setTCPMD5 called on dummyListener, ignoring...  router_id=169.254.200.0
INFO[0000] Added BGP peer                                local_address=169.254.200.0 local_as=65101 peer_address=172.17.0.3 peer_as=65100 router_id=169.254.200.0
DEBU[0000] AddPath to locRIB                             Prefix=10.0.0.0/8 Route="Local Pref: 3, Origin: IGP, AS Path: , BGP type: external, NEXT HOP: 172.17.0.3, MED: 0, Path ID: 10, Source: 172.17.0.3, "
INFO[0003] FSM: Neighbor state change                    last_state=idle new_state=connect peer=169.254.200.0 reason="Received AutomaticStart event"
INFO[0003] Incoming TCP connection                       router_id=169.254.200.0 source="172.17.0.3:0"
DEBU[0003] Sending incoming TCP connection to fsm for peer  peer=172.17.0.3 router_id=169.254.200.0
INFO[0003] FSM: Neighbor state change                    last_state=active new_state=openSent peer=172.17.0.3 reason="Sent OPEN message"
INFO[0003] FSM: Neighbor state change                    last_state=connect new_state=openSent peer=169.254.200.0 reason="TCP connection succeeded"
INFO[0003] FSM: Neighbor state change                    last_state=openSent new_state=openConfirm peer=172.17.0.3 reason="Received OPEN message"
INFO[0003] FSM: Neighbor state change                    last_state=openConfirm new_state=established peer=172.17.0.3 reason="Received KEEPALIVE"
INFO[0003] FSM: Neighbor state change                    last_state=openSent new_state=openConfirm peer=169.254.200.0 reason="Received OPEN message"
INFO[0003] FSM: Neighbor state change                    last_state=openConfirm new_state=established peer=169.254.200.0 reason="Received KEEPALIVE"
DEBU[0003] AddPath to locRIB                             Prefix=10.0.0.0/8 Route="Local Pref: 0, Origin: IGP, AS Path: 65100, BGP type: external, NEXT HOP: 172.17.0.3, MED: 0, Path ID: 0, Source: 172.17.0.3, "

I would probably also like to be able to override the logger for each BGP server manually from the caller side.
This would allow for example to mute server logging for some servers completely or redirect them to separate files.

[tobikris]

I have added a testcase to verify the correctness of multiple BGP speakers in one process.

The test case is currently failing:

--- FAIL: TestInMemoryServerMinimalPaths (2.10s)
    server_test.go:122: 
                Error Trace:    /home/tobikris/dev/git.exaring.net/network/bio-rd/protocols/bgp/server/server_test.go:122
                                                        /home/tobikris/dev/git.exaring.net/network/bio-rd/protocols/bgp/server/asm_amd64.s:1594
                Error:          Not equal: 
                                expected: 1
                                actual  : 2
                Test:           TestInMemoryServerMinimalPaths
                Messages:       Expected 1 path, got 2
    server_test.go:123: [Local Pref: 0, Origin: IGP, AS Path: 65101, BGP type: external, NEXT HOP: 172.16.0.2, MED: 0, Path ID: 0, Source: 172.16.0.1,  Local Pref: 0, Origin: IGP, AS Path: 65101, BGP type: external, NEXT HOP: 172.16.0.1, MED: 0, Path ID: 0, Source: 172.16.0.1, ]
    server_test.go:126: 
                Error Trace:    /home/tobikris/dev/git.exaring.net/network/bio-rd/protocols/bgp/server/server_test.go:126
                                                        /home/tobikris/dev/git.exaring.net/network/bio-rd/protocols/bgp/server/asm_amd64.s:1594
                Error:          Should be true
                Test:           TestInMemoryServerMinimalPaths
                Messages:       Expected next hop to be 172.16.0.1, got 172.16.0.2
    server_test.go:133: 
                Error Trace:    /home/tobikris/dev/git.exaring.net/network/bio-rd/protocols/bgp/server/server_test.go:133
                                                        /home/tobikris/dev/git.exaring.net/network/bio-rd/protocols/bgp/server/asm_amd64.s:1594
                Error:          Not equal: 
                                expected: 1
                                actual  : 2
                Test:           TestInMemoryServerMinimalPaths
                Messages:       Expected 1 path, got 2
    server_test.go:134: [Local Pref: 0, Origin: IGP, AS Path: 65102 65101, BGP type: external, NEXT HOP: 172.16.0.2, MED: 0, Path ID: 0, Source: 172.16.0.2,  Local Pref: 0, Origin: IGP, AS Path: 65102 65101, BGP type: external, NEXT HOP: 172.16.0.2, MED: 0, Path ID: 0, Source: 172.16.0.2, ]
    server_test.go:137: 
                Error Trace:    /home/tobikris/dev/git.exaring.net/network/bio-rd/protocols/bgp/server/server_test.go:137
                                                        /home/tobikris/dev/git.exaring.net/network/bio-rd/protocols/bgp/server/asm_amd64.s:1594
                Error:          Should be true
                Test:           TestInMemoryServerMinimalPaths
                Messages:       Expected next hop to be 172.16.0.1, got 172.16.0.2
FAIL
FAIL    github.com/bio-routing/bio-rd/protocols/bgp/server      3.208s
FAIL

However, I do not find the relevant code path responsible for this.

@taktv6 @BarbarossaTM: Is my test premise valid in the first place? Only one path is expected, correct? The incorrect next-hop is probably caused by the singular cache structure being reused by multiple servers?

[taktv6]

Looking into this I'm wondering how this is caused. Even with just two instances it happens.

[taktv6]

Could it be both instance 1 and 2 initiate connections to each other and don't clean up the collision?