Remove path attributes in an UPDATE message that only contains MP_UNREACH_NLRI

[subsecond]

Summary

Following the principle "be liberal in what you accept and conservative in what you send" we should remove the path attributes ORIGIN and AS_PATH from UPDATE messages that only contain the MP_UNREACH_NLRI because RFC4760 states:

An UPDATE message that contains the MP_UNREACH_NLRI is not required
to carry any other path attributes.

Currently, when sending a BGP withdraw, here is what that update message looks like on the wire:

21:12:32.103664 fa:16:3e:22:0a:8b > fa:16:3e:4f:6f:0b, ethertype IPv6 (0x86dd), length 153: (flowlabel 0x491a8, hlim 64, next-header TCP (6) payload length: 99) 2001:db8::200.41291 > 2001:db8::100.179: Flags [P.], cksum 0xc11d (correct), seq 77:136, ack 1, win 507, options [nop,nop,md5 valid], length 59: BGP
	Update Message (2), length: 59
	  Origin (1), length: 1, Flags [T]: IGP
	    0x0000:  00
	  AS Path (2), length: 6, Flags [T]: 65200 
	    0x0000:  0201 0000 feb0
	  Multi-Protocol Unreach NLRI (15), length: 20, Flags [O]: 
	    AFI: IPv6 (2), SAFI: Unicast (1)
	      2001:db8:dead:beef::1/128
	    0x0000:  0002 0180 2001 0db8 dead beef 0000 0000
	    0x0010:  0000 0001

Both ORIGIN and AS_PATH are path attributes that, according to RFC4760, are not required to be sent in the update message (I am fully aware that this is not a "must not" but more of a "should not").

OS

Debian Linux

root@exabgp-test:~# uname -a
Linux exabgp-test 4.19.0-8-cloud-amd64 #1 SMP Debian 4.19.98-1 (2020-01-26) x86_64 GNU/Linux

Version

root@exabgp-test:~# exabgp --version
ExaBGP : 4.2.6
Python : 2.7.16 (default, Oct 10 2019, 22:02:15)  [GCC 8.3.0]
Uname  : Linux exabgp-test 4.19.0-8-cloud-amd64 #1 SMP Debian 4.19.98-1 (2020-01-26) x86_64
Root   : /usr/local

Installation

pip

Environment

root@exabgp-test:~# exabgp --di

[exabgp.daemon]
user = 'root'

[exabgp.log]
reactor = false

Configuration

root@exabgp-test:~# cat /usr/local/etc/exabgp/exabgp.conf 
template {
  neighbor default {
    router-id 192.0.2.200;
    local-as 65200;
    hold-time 30;
    group-updates true;
  }
}

neighbor 192.0.2.100 {
  inherit default;
  description "eBGP session with frr-test";
  local-address 192.0.2.200;
  peer-as 65100;
  md5-password foobar123;
  md5-base64 false;
  family {
    ipv4 unicast;
  }
}
neighbor 2001:db8::100 {
  inherit default;
  description "eBGP session with frr-test";
  local-address 2001:db8::200;
  peer-as 65100;
  md5-password foobar123;
  md5-base64 false;
  family {
    ipv6 unicast;
  }
}

Steps to reproduce

exabgp-cli neighbor 2001:db8::100 announce route 2001:db8:dead:beef::1/128 next-hop 2001:db8:cafe::1
exabgp-cli neighbor 2001:db8::100 withdraw route 2001:db8:dead:beef::1/128 next-hop 2001:db8:cafe::1

Importance

After the update to ExaBGP 4.2.6 (from 3.x), IPv6 withdraws are tearing down the IPv6 BGP session with the FRR neighbor. That is because older versions of FRR expect either no path attributes at all or the nexthop attribute to be present as well when sending MP_UNREACH_NLRI. FRRouting/frr@404c82d fixes this for FRR.

Credits

Many thanks to @donaldsharp for pointing out that ExaBGP is sending superfluous path attributes (ORIGIN and AS_PATH) when the update message contains the MP_UNREACH_NLRI.

References

Could be related to:
#573
#954

[thomas-mangin]

I believe we currently send both reach and unreach in the same message. I need to see what the impact would be on the code but in principle it makes sense.

[subsecond]

Fully agreed: We should only remove all path attributes in case we do not send MP_REACH_NLRI in the same update.

[subsecond]

Thanks for the patch, @href.
I can confirm that the patch indeed resolves our issue in older versions of FRR.