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

Inadvertent change to unnumbered bgp next-hop with "set ip next-hop" route-map #3922

Open
dslicenc opened this issue Mar 7, 2019 · 0 comments
Assignees

Comments

@dslicenc
Copy link
Member

dslicenc commented Mar 7, 2019

  • [X ] Did you check if this is a duplicate issue?
  • [X ] Did you test it on the latest FRRouting/frr master branch?

Description

Found that if a route-map with "set ip address x.x.x.x" is applied outbound to an unnumbered peer, the update is sent with a single link-local ipv6 address (16 bytes) instead of the 32 byte link-local previously sent. This change was the result of a change made to allow setting ip next-hop for mpls-vpn routes (per the comment) but has the side-effect of modifying behavior not pertaining to that use-case.

Steps to Reproduce

  1. Bring up bgp unnumbered peering between two routers
  2. Define a route-map with "set ip next-hop x.x.x.x"
  3. Apply the route-map to outbound updates to the unnumbered peer
  4. Display the received bgp table entry for the change

Configuration on sender

router bgp 10
 bgp router-id 90.0.0.1
 no bgp default ipv4-unicast
 neighbor swp6 interface remote-as 80
 neighbor swp6 description r8
 neighbor swp6 advertisement-interval 0
address-family ipv4 unicast
  neighbor swp6 activate
  neighbor swp6 route-map FOO out
!
route-map FOO permit 10
 set ip next-hop 9.9.9.9

Expected behavior:
Before commit commit d54600f, the received table entry looked like this:

r8# sh ip bgp 10.0.0.4
BGP routing table entry for 10.0.0.4/32
Paths: (1 available, best #1, table default)
  Advertised to non peer-group peers:
  r1(swp1)
  10 40
    fe80::202:ff:fe00:6 from r1(swp1) (90.0.0.1) 
    (fe80::202:ff:fe00:6) (used)                         
      Origin IGP, valid, external, bestpath-from-AS 10, best
      Last update: Wed Mar  6 13:51:05 2019

Pcap file snippet of the above update:

Path attributes
    Path Attribute - MP_REACH_NLRI
        Flags: 0x90, Optional, Extended-Length, Non-transitive, Complete
        Type Code: MP_REACH_NLRI (14)
        Length: 49
        Address family identifier (AFI): IPv4 (1)
        Subsequent address family identifier (SAFI): Unicast (1)
        Next hop network address (32 bytes)
            Next Hop: Unknown address
        Number of Subnetwork points of attachment (SNPA): 0
        Network layer reachability information (12 bytes)
            10.0.2.0/24
                MP Reach NLRI prefix length: 24
                MP Reach NLRI IPv4 prefix: 10.0.2.0
            172.16.10.0/24
                MP Reach NLRI prefix length: 24
                MP Reach NLRI IPv4 prefix: 172.16.10.0
            172.16.1.0/24
                MP Reach NLRI prefix length: 24
                MP Reach NLRI IPv4 prefix: 172.16.1.0
    Path Attribute - ORIGIN: INCOMPLETE
        Flags: 0x40, Transitive, Well-known, Complete
        Type Code: ORIGIN (1)
        Length: 1
        Origin: INCOMPLETE (2)
    Path Attribute - AS_PATH: 65000 65002 
        Flags: 0x50, Transitive, Extended-Length, Well-known, Complete
        Type Code: AS_PATH (2)
        Length: 10
        AS Path segment: 65000 65002

Actual behavior:
After the above commit, the following bgp table entry on the receiver:

r8# sh ip bgp 10.0.0.4
BGP routing table entry for 10.0.0.4/32
Paths: (1 available, best #1, table default)
  Advertised to non peer-group peers:
  r1(swp1)
  10 40
    fe80::202:ff:fe00:6 from r1(swp1) (90.0.0.1)
      Origin IGP, valid, external, bestpath-from-AS 10, best
      Last update: Wed Mar  6 20:05:11 2019

And pcap file snippet

Border Gateway Protocol - UPDATE Message
    Marker: ffffffffffffffffffffffffffffffff
    Length: 71
    Type: UPDATE Message (2)
    Withdrawn Routes Length: 0
    Total Path Attribute Length: 48
    Path attributes
        Path Attribute - MP_REACH_NLRI
            Flags: 0x90, Optional, Extended-Length, Non-transitive, Complete
            Type Code: MP_REACH_NLRI (14)
            Length: 26
            Address family identifier (AFI): IPv4 (1)
            Subsequent address family identifier (SAFI): Unicast (1)
            Next hop network address (16 bytes)
                Next Hop: fe80::202:ff:fe00:6
            Number of Subnetwork points of attachment (SNPA): 0
            Network layer reachability information (5 bytes)
                10.0.0.4/32
                    MP Reach NLRI prefix length: 32
                    MP Reach NLRI IPv4 prefix: 10.0.0.4
        Path Attribute - ORIGIN: IGP
            Flags: 0x40, Transitive, Well-known, Complete
            Type Code: ORIGIN (1)
            Length: 1
            Origin: IGP (0)
        Path Attribute - AS_PATH: 10 40 
            Flags: 0x50, Transitive, Extended-Length, Well-known, Complete
            Type Code: AS_PATH (2)
            Length: 10
            AS Path segment: 10 40

Portion of commit causing the changed behavior

diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index f7c79f873..f9f5142cd 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -1298,6 +1298,9 @@ static route_map_result_t route_set_ip_nexthop(void *rule,
                        bgp_info->attr->nexthop = *rins->address;
                        SET_FLAG(bgp_info->attr->rmap_change_flags,
                                 BATTR_RMAP_IPV4_NHOP_CHANGED);
+                       /* case for MP-BGP : MPLS VPN */
+                       bgp_info->attr->mp_nexthop_global_in = *rins->address;
+                       bgp_info->attr->mp_nexthop_len = sizeof(*rins->address);
                }
        }

Components

bgpd

Versions

  • FRR: frr_7.1-dev_amd64.deb
@eqvinox eqvinox self-assigned this Mar 12, 2019
@qlyoung qlyoung added bug and removed bug labels Mar 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants