search

osrg / gobgp

BGP implemented in the Go Programming Language
https://osrg.github.io/gobgp/
Apache License 2.0
3.6k stars 685 forks source link

Encoding bug with EVPN Type 2 message for MPLS #2134

Open yicwang opened 5 years ago

yicwang commented 5 years ago

According to RFC 7432, EVPN Type 2 message allows to carry two MPLS Labels, and every label is allocating with 3 octets (24 bits). This works perfectly fine when Type 2 is for VxLAN (bgp.TUNNEL_TYPE_VXLAN), but there is a bug when tunnel type is MPLS (bgp.TUNNEL_TYPE_MPLS).

In Section 9.2.1, "The MPLS Label1 field is encoded as 3 octets, where the high-order 20 bits contain the label value." This basically means, When tunnel type is VxLAN, all 24 bits are used; when tunnel type if MPLS, only the high-order 20 bits are used to represent the label value. Today's code doesn't consider this part, which will result an error value being encoded when tunnel type is MPLS. The fix would just shift the values 4 bits to the left and everything will work.

I workaround this from application API, and verified against Cisco ASR9K, that works fine.

fujita commented 5 years ago

Thanks, can you create a pull request to fix it?

yicwang commented 5 years ago

@fujita I find it is a bit of hard with today's framework. From EVPNMacIPAdvertisementRoute, there is a labelSerialize() function. However, in the entire data structure, I didn't see any visibility or reference that I can use to get the tunnel type. This is like a "inter-attribute" dependency, which I don't know what is the best way to do.

What I did is just in the application API, I will shift left or right myself manually when I know the tunnel type is VxLAN or MPLS...

fujita commented 5 years ago

the tunnel type is in an extended attribute so it's necessary to parse attributes first then an NLRI, right?

yicwang commented 5 years ago

@fujita Yes, I believe so. I did more checks with the BGP expert here, how Cisco ASR9K does is, all the labels will be treated as MPLS label by default, which means only 20bits are used. It will only treat it as VxLAN VNIs when bgp.TUNNEL_TYPE_VXLAN is explicitly specified in ExtendedCommunities. The same logic applies to both EVPN Type-2, and EVPN PMSI Attribute (the one together with EVPN Type-3), and maybe other EVPN types as well.