Introduce a messageHandler object for bgp

[uablrek]

The messageHandler will be used for ipv6 support while leaving
the existing and working ipv4 code untouched.

What this PR does / why we need it:

This PR addresses a code separation problem when support for ipv6 in the speaker is introduced. The message handling should be handled by an object messageHandler rather than a set of functions. A ipv6 messageHandler can be introduced while leaving ipv4 code as-is.

The current ipv4 code for bgp must not be altered when support for ipv6 is introduced. This would cause maintenance problems and likely bugs.

Which issue(s) this PR fixes

This is yet another step on the way for #7

Special notes for your reviewer:

The PR is not complete and contains only updates for the setup and the sendUpdate function. The intention with this PR is to propose a way of adding ipv6 support in a non-intrusive way and come to an agreement about how to add ipv6 support.

In the existing messages.go only object variables are added in front of selected (every?) function. The rest of the code is not altered.

A skeleton messages-ipv6.go is added to show the concept. It return ok (nil) to prevent re-start of the session while developing. For ipv6 it is stable but not working. The messages-ipv6.go will later contain the ipv6 support for bgp.

If the proposed approach is acceptable this PR can be merged as-is, or it can be filled with more code to be more complete.

[uablrek]

This PR is updated with support for update messages for announce/withdraw for ipv6. The speaker is now functional for ipv6 but with a limitation; all addresses are assumed to be /128. I.e. no ranges are announced. This is the normal case for load-balancer addresses I think but needs to be addresses anyway in the future.

Testing is done manually on xcluster.

The failed CI tests does not seem related to this PR.

[danderson]

I've looked at the code, and I have one problem: it segments BGP sessions into "ipv4 or ipv6", but the BGP protocol itself allows any combination of {ipv4, ipv6} announcements over {ipv4, ipv6} BGP sessions, based on the advertised capabilities of the peer.

So, instead of segmenting this codebase into one message handler for v4 and one for v6, I would prefer if we track the capabilities the peer sent us, and when we're sending update messages filter based on which address families the peer wants. This would reduce a bunch of code duplication (the messages are the same except for one enum value and some field lengths), and support more types of BGP deployments. This is especially important because k8s is finally working on dual-stack support, so some day we might have completely hybrid environments at the IP layer, and we want the BGP speaker to handle it.

WDYT?

[uablrek]

You are right. I thought of it but when I alter somebody elses code I try very hard to leave the existing (presumably working) code as intact as I possibly can. I'll try a more itrusive approach. To be able to use the same code for ipv4 and ipv6 the current ipv4 implementation must be altered to use bgp-mp. I have not an array of 3pp routers to test against (same as yourself I think) so I am a bit worried to mess things up.

[uablrek]

Here is my new plan;

• Keep the "messageHandler" object but for the current bgp-non-mp implementation
• Add a parameter/env-var for enabling bgb-mp, including both ipv4 and ipv6
• Add a bgp-mp protocol in the configuration

Then it is possible to keep the current implementation while developing support for bgp-mp. Maybe a bit cowardly...

[uablrek]

Here is a trace (narrowed tshark) of ipv4 with bgp-mp;

Frame 9: 115 bytes on wire (920 bits), 115 bytes captured (920 bits)
Ethernet II, Src: 00:00:00:01:01:02, Dst: 00:00:00:01:01:c9
Internet Protocol Version 4, Src: 192.168.1.2, Dst: 192.168.1.201
Transmission Control Protocol, Src Port: 52115, Dst Port: 179, Seq: 1, Ack: 1, Len: 49
Border Gateway Protocol - OPEN Message
    Marker: ffffffffffffffffffffffffffffffff
    Length: 49
    Type: OPEN Message (1)
    Version: 4
    My AS: 65001
    Hold Time: 90
    BGP Identifier: 192.168.1.2
    Optional Parameters Length: 20
    Optional Parameters
        Optional Parameter: Capability
            Parameter Type: Capability (2)
            Parameter Length: 18
            Capability: Multiprotocol extensions capability
                Type: Multiprotocol extensions capability (1)
                Length: 4
                AFI: IPv4 (1)
                Reserved: 00
                SAFI: Unicast (1)
            Capability: Multiprotocol extensions capability
                Type: Multiprotocol extensions capability (1)
                Length: 4
                AFI: IPv6 (2)
                Reserved: 00
                SAFI: Unicast (1)
            Capability: Support for 4-octet AS number capability
                Type: Support for 4-octet AS number capability (65)
                Length: 4
                AS Number: 65001


Frame 56: 111 bytes on wire (888 bits), 111 bytes captured (888 bits)
Ethernet II, Src: 00:00:00:01:01:c9, Dst: 00:00:00:01:01:02
Internet Protocol Version 4, Src: 192.168.1.201, Dst: 192.168.1.2
Transmission Control Protocol, Src Port: 179, Dst Port: 52115, Seq: 1, Ack: 50, Len: 45
Border Gateway Protocol - OPEN Message
    Marker: ffffffffffffffffffffffffffffffff
    Length: 45
    Type: OPEN Message (1)
    Version: 4
    My AS: 65002
    Hold Time: 90
    BGP Identifier: 192.168.1.201
    Optional Parameters Length: 16
    Optional Parameters
        Optional Parameter: Capability
            Parameter Type: Capability (2)
            Parameter Length: 14
            Capability: Route refresh capability
                Type: Route refresh capability (2)
                Length: 0
            Capability: Multiprotocol extensions capability
                Type: Multiprotocol extensions capability (1)
                Length: 4
                AFI: IPv4 (1)
                Reserved: 00
                SAFI: Unicast (1)
            Capability: Support for 4-octet AS number capability
                Type: Support for 4-octet AS number capability (65)
                Length: 4
                AS Number: 65002

Frame 95: 119 bytes on wire (952 bits), 119 bytes captured (952 bits)
Ethernet II, Src: 00:00:00:01:01:02, Dst: 00:00:00:01:01:c9
Internet Protocol Version 4, Src: 192.168.1.2, Dst: 192.168.1.201
Transmission Control Protocol, Src Port: 52115, Dst Port: 179, Seq: 69, Ack: 65, Len: 53
Border Gateway Protocol - UPDATE Message
    Marker: ffffffffffffffffffffffffffffffff
    Length: 53
    Type: UPDATE Message (2)
    Withdrawn Routes Length: 0
    Total Path Attribute Length: 30
    Path attributes
        Path Attribute - ORIGIN: INCOMPLETE
            Flags: 0x40, Transitive, Well-known, Complete
                0... .... = Optional: Not set
                .1.. .... = Transitive: Set
                ..0. .... = Partial: Not set
                ...0 .... = Extended-Length: Not set
                .... 0000 = Unused: 0x0
            Type Code: ORIGIN (1)
            Length: 1
            Origin: INCOMPLETE (2)
        Path Attribute - AS_PATH: 65001 
            Flags: 0x40, Transitive, Well-known, Complete
                0... .... = Optional: Not set
                .1.. .... = Transitive: Set
                ..0. .... = Partial: Not set
                ...0 .... = Extended-Length: Not set
                .... 0000 = Unused: 0x0
            Type Code: AS_PATH (2)
            Length: 6
            AS Path segment: 65001
                Segment type: AS_SEQUENCE (2)
                Segment length (number of ASN): 1
                AS4: 65001
        Path Attribute - MP_REACH_NLRI
            Flags: 0x80, Optional, Non-transitive, Complete
                1... .... = Optional: Set
                .0.. .... = Transitive: Not set
                ..0. .... = Partial: Not set
                ...0 .... = Extended-Length: Not set
                .... 0000 = Unused: 0x0
            Type Code: MP_REACH_NLRI (14)
            Length: 14
            Address family identifier (AFI): IPv4 (1)
            Subsequent address family identifier (SAFI): Unicast (1)
            Next hop network address (4 bytes)
                Next Hop: 192.168.1.2
            Number of Subnetwork points of attachment (SNPA): 0
            Network layer reachability information (5 bytes)
                10.0.0.2/32
                    MP Reach NLRI prefix length: 32
                    MP Reach NLRI IPv4 prefix: 10.0.0.2

The peer router is gpbgp. I have no idea why it says is does not have ipv6. It has!

[uablrek]

To enable bgb-mp in the manifest specify;

        env:
          - name: METALLB_NODE_NAME
            valueFrom:
              fieldRef:
                fieldPath: spec.nodeName
          - name: METALLB_BGP_MP
            value: "yes"

This is a temporary solution for testing the bgp-mp function.

[uablrek]

The mask when using bgp-mp is hardcoded to /128 for ipv6 and /32 for ipv4.

[uablrek]

@danderson

Is there anything I can do to improve this PR?

The failing tests does not seem related. I make local ipv6 tests for basic function and externalTrafficPolicy:Local (does not work due to kubernetes/kubernetes#71596).

The received OPEN message is not checked for AFI-ipv6. Do you think it should?

[danderson]

Sorry, this change is fairly complex, and I simply no longer have enough time to review this kind of change :(. I do still want to make IPv6 work at some point, but right now for my own sanity I need to close this PR because realistically, I'm never going to get to it.

I'm very sorry for the effort you wasted. I wish I could spend more time and energy on MetalLB, but it's simply no longer possible for me :(