From 06b8bb0b3dbfc58178dfa5fee65d6e8bd99fc947 Mon Sep 17 00:00:00 2001 From: Nischal Sheth Date: Fri, 3 Jun 2016 09:41:17 -0700 Subject: [PATCH] Fix interop issue for EVPN w/ MPLS in GRE encapsulation Current version of EVPN overlay draft says to use MPLSoGRE codepoint, whereas Contrail uses GRE. This results in interop issues with other implementations. The justification for definining a new code point for MPLSoGRE is not clear. In the intrest of improving interoperability, advertise both GRE and MPLSoGRE for EVPN routes and map both GRE and MPLSoGRE to the same string value when advertising routes via xmpp. Change-Id: I6d01053de225c8058756f9d76682164b6499352e Closes-Bug: 1588554 --- src/bgp/bgp_xmpp_channel.cc | 10 +++++++ .../tunnel_encap/test/tunnel_encap_test.cc | 26 +++++++++++++++---- src/net/tunnel_encap_type.cc | 8 +++--- src/net/tunnel_encap_type.h | 3 ++- 4 files changed, 38 insertions(+), 9 deletions(-) diff --git a/src/bgp/bgp_xmpp_channel.cc b/src/bgp/bgp_xmpp_channel.cc index 69782f4c8ea..4a61d1716eb 100644 --- a/src/bgp/bgp_xmpp_channel.cc +++ b/src/bgp/bgp_xmpp_channel.cc @@ -1614,9 +1614,19 @@ bool BgpXmppChannel::ProcessEnetItem(string vrf_name, if (first_nh) { ext.communities.push_back( tun_encap.GetExtCommunityValue()); + if (tun_encap.tunnel_encap() == TunnelEncapType::GRE) { + TunnelEncap alt_tun_encap(TunnelEncapType::MPLS_O_GRE); + ext.communities.push_back( + alt_tun_encap.GetExtCommunityValue()); + } } nexthop.tunnel_encapsulations_.push_back( tun_encap.GetExtCommunity()); + if (tun_encap.tunnel_encap() == TunnelEncapType::GRE) { + TunnelEncap alt_tun_encap(TunnelEncapType::MPLS_O_GRE); + nexthop.tunnel_encapsulations_.push_back( + alt_tun_encap.GetExtCommunity()); + } } // Mark the path as infeasible if all tunnel encaps published diff --git a/src/bgp/tunnel_encap/test/tunnel_encap_test.cc b/src/bgp/tunnel_encap/test/tunnel_encap_test.cc index e1c2be866fd..db6e68b655b 100644 --- a/src/bgp/tunnel_encap/test/tunnel_encap_test.cc +++ b/src/bgp/tunnel_encap/test/tunnel_encap_test.cc @@ -13,7 +13,7 @@ class TunnelEncapTest : public ::testing::Test { TEST_F(TunnelEncapTest, String_1) { TunnelEncap tunnel_encap("gre"); - EXPECT_EQ(TunnelEncapType::MPLS_O_GRE, tunnel_encap.tunnel_encap()); + EXPECT_EQ(TunnelEncapType::GRE, tunnel_encap.tunnel_encap()); EXPECT_EQ("encapsulation:gre", tunnel_encap.ToString()); EXPECT_EQ("gre", tunnel_encap.ToXmppString()); } @@ -74,10 +74,17 @@ TEST_F(TunnelEncapTest, String_7) { EXPECT_EQ("vxlan-gpe", tunnel_encap.ToXmppString()); } -TEST_F(TunnelEncapTest, EncapType_1) { +TEST_F(TunnelEncapTest, EncapType_1a) { + TunnelEncap tunnel_encap(TunnelEncapType::GRE); + EXPECT_EQ(TunnelEncapType::GRE, tunnel_encap.tunnel_encap()); + EXPECT_EQ("encapsulation:gre", tunnel_encap.ToString()); + EXPECT_EQ("gre", tunnel_encap.ToXmppString()); +} + +TEST_F(TunnelEncapTest, EncapType_1b) { TunnelEncap tunnel_encap(TunnelEncapType::MPLS_O_GRE); EXPECT_EQ(TunnelEncapType::MPLS_O_GRE, tunnel_encap.tunnel_encap()); - EXPECT_EQ("encapsulation:gre", tunnel_encap.ToString()); + EXPECT_EQ("encapsulation:mpls-o-gre", tunnel_encap.ToString()); EXPECT_EQ("gre", tunnel_encap.ToXmppString()); } @@ -136,15 +143,24 @@ TEST_F(TunnelEncapTest, EncapType_7) { EXPECT_EQ("vxlan-gpe", tunnel_encap.ToXmppString()); } -TEST_F(TunnelEncapTest, ByteArray_1) { +TEST_F(TunnelEncapTest, ByteArray_1a) { TunnelEncap::bytes_type data = { { 0x03, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02 } }; TunnelEncap tunnel_encap(data); - EXPECT_EQ(TunnelEncapType::MPLS_O_GRE, tunnel_encap.tunnel_encap()); + EXPECT_EQ(TunnelEncapType::GRE, tunnel_encap.tunnel_encap()); EXPECT_EQ("encapsulation:gre", tunnel_encap.ToString()); EXPECT_EQ("gre", tunnel_encap.ToXmppString()); } +TEST_F(TunnelEncapTest, ByteArray_1b) { + TunnelEncap::bytes_type data = + { { 0x03, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0B } }; + TunnelEncap tunnel_encap(data); + EXPECT_EQ(TunnelEncapType::MPLS_O_GRE, tunnel_encap.tunnel_encap()); + EXPECT_EQ("encapsulation:mpls-o-gre", tunnel_encap.ToString()); + EXPECT_EQ("gre", tunnel_encap.ToXmppString()); +} + TEST_F(TunnelEncapTest, ByteArray_2a) { TunnelEncap::bytes_type data = { { 0x03, 0x0C, 0x00, 0x00, 0x00, 0x00, 0x00, 0x0d } }; diff --git a/src/net/tunnel_encap_type.cc b/src/net/tunnel_encap_type.cc index a960d5ffd37..6335a60c4f2 100644 --- a/src/net/tunnel_encap_type.cc +++ b/src/net/tunnel_encap_type.cc @@ -15,7 +15,7 @@ TunnelEncapType::TunnelEncapType() { static const map fromString = boost::assign::map_list_of ("unspecified", TunnelEncapType::UNSPEC) - ("gre", TunnelEncapType::MPLS_O_GRE) + ("gre", TunnelEncapType::GRE) ("vxlan", TunnelEncapType::VXLAN) ("nvgre", TunnelEncapType::NVGRE) ("mpls", TunnelEncapType::MPLS) @@ -27,10 +27,11 @@ static const map static const map toString = boost::assign::map_list_of (TunnelEncapType::UNSPEC, "unspecified") - (TunnelEncapType::MPLS_O_GRE, "gre") + (TunnelEncapType::GRE, "gre") (TunnelEncapType::VXLAN, "vxlan") (TunnelEncapType::NVGRE, "nvgre") (TunnelEncapType::MPLS, "mpls") + (TunnelEncapType::MPLS_O_GRE, "mpls-o-gre") (TunnelEncapType::VXLAN_GPE, "vxlan-gpe") (TunnelEncapType::MPLS_O_UDP, "udp") (TunnelEncapType::MPLS_O_UDP_CONTRAIL, "udp-contrail") @@ -39,10 +40,11 @@ static const map static const map toXmppString = boost::assign::map_list_of (TunnelEncapType::UNSPEC, "unspecified") - (TunnelEncapType::MPLS_O_GRE, "gre") + (TunnelEncapType::GRE, "gre") (TunnelEncapType::VXLAN, "vxlan") (TunnelEncapType::NVGRE, "nvgre") (TunnelEncapType::MPLS, "mpls") + (TunnelEncapType::MPLS_O_GRE, "gre") (TunnelEncapType::VXLAN_GPE, "vxlan-gpe") (TunnelEncapType::MPLS_O_UDP, "udp") (TunnelEncapType::MPLS_O_UDP_CONTRAIL, "udp") diff --git a/src/net/tunnel_encap_type.h b/src/net/tunnel_encap_type.h index df0e5c0ca0f..026248643fd 100644 --- a/src/net/tunnel_encap_type.h +++ b/src/net/tunnel_encap_type.h @@ -12,10 +12,11 @@ class TunnelEncapType { public: enum Encap { UNSPEC = 0, - MPLS_O_GRE = 2, + GRE = 2, VXLAN = 8, NVGRE = 9, MPLS = 10, + MPLS_O_GRE = 11, VXLAN_GPE = 12, MPLS_O_UDP = 13, MPLS_O_UDP_CONTRAIL = 37001,