Hi,

Thank you for posting your patch!

A few remarks:

> --- a/ryu/lib/packet/bgp.py
> +++ b/ryu/lib/packet/bgp.py
> @@ -1405,7 +1405,7 @@ class EvpnNLRI(StringifyMixin, TypeDisp):
>  
>      @staticmethod
>      def _ip_addr_to_bin(ip_addr):
> -        return ip.text_to_bin(ip_addr)
> +        return bytes() if not ip_addr else ip.text_to_bin(ip_addr) # [AJS] 
> allow empty ip_addr (len 0), e.g. L2VPN MAC advertisement Cisco NX-OS

We already add ip_addr_len check for the case with empty ip_addr at the 
following.
Is this not enough?
  https://github.com/osrg/ryu/blob/master/ryu/lib/packet/bgp.py#L1684


> @@ -1748,13 +1748,14 @@ class EvpnInclusiveMulticastEthernetTagNLRI(EvpnNLRI):
>      }
>  
>      def __init__(self, route_dist, ethernet_tag_id, ip_addr,
> -                 ip_addr_len=None, type_=None, length=None):
> +                 ip_addr_len=None, type_=None, length=None, vni=None): # 
> [AJS] vni
>          super(EvpnInclusiveMulticastEthernetTagNLRI,
>                self).__init__(type_, length)
>          self.route_dist = route_dist
>          self.ethernet_tag_id = ethernet_tag_id
>          self.ip_addr_len = ip_addr_len
>          self.ip_addr = ip_addr
> +        self.vni = vni # [AJS] vni
>  
>      @classmethod
>      def parse_value(cls, buf):

According RFC7432, "7.3.  Inclusive Multicast Ethernet Tag Route" has not vni
field. We should not add vni attribute.
I guess Cisco NX-OS send vni value as ethernet_tag_id, because Cisco NX-OS
provides "6.1.  VLAN-Based Service Interface", I guess.


Also, some comments like "# [AJS]" breaks pep8 check...
I don't recommend you to add name into source comment...


Thanks,
Iwase


On 2017年01月26日 19:25, Albert Siersema wrote:
> 
> Hello,
> 
> I've tried connecting Ryu to Cisco NX-OS 7.x switches (Nexus 3172, Nexus 
> 9372PX, Nexus C92160YC-X) and ran into two issues which I solved with the 
> attached patch.
> 
> 
> This patch adds interoperability with other MP BGP EVPN VXLAN 
> implementations, e.g.  Cisco NX-OS.
> 
> 
> Issue #1
> Interoperability with at least Cisco NX-OS requires an empty ip_addr in the
> EVPN_MAC_IP_ADV_ROUTE prefix announcement.
> 
> Example usage:
> 
>          speaker.evpn_prefix_add(
>              route_type=EVPN_MAC_IP_ADV_ROUTE,
>              tunnel_type=TUNNEL_TYPE_VXLAN,
>              route_dist=evpn_route_dist,
>              ethernet_tag_id=0,
>              mac_addr=evpn_mac,
>              ip_addr=None, # Cisco: None (make sure ip_addr_len=0 , 
> ip_addr="0.0.0.0" != None)
>              next_hop=loopback_IPv4,
>              vni=0,
>          )
> 
> 
> 
> Issue #2
> A working MP-BGP EVPN VXLAN fabric needs to announce the local NVE/VTEP IP
> address to all other NVE/VTEP peers through an Inclusive Multicast Ethernet
> Tag Route (IMET) with the VNI in a BGPPathAttributePmsiTunnel.
> 
> Interoperability with at least Cisco NX-OS requires an IMET announcement .
> Only NVE/VTEP IP addresses received through these announcements are valid
> VTEP's and will thus receive BUM traffic as well. Without this
> announcement, the L2VPN is not working.
> 
> On NX-OS one can check with:
>    show l2route evpn imet all [detail]    <VNI_VLAN>  <VNI>  BGP 
> <remote_NVE_VTEP_IP>
>                     e.g. 311    10311  BGP  10.1..126
>    sh bgp l2vpn evpn received-paths    look for type 3, e.g. : 
> [3]:[0]:[32]:[10.1.1.126]/88)
> 
> See also:
> https://tools.ietf.org/html/rfc7432#section-7.3
> https://tools.ietf.org/html/rfc7432#section-11
> 
> 
> Example usage:
> 
>          speaker.evpn_prefix_add(
>              route_type=EVPN_MULTICAST_ETAG_ROUTE,
>              tunnel_type=TUNNEL_TYPE_VXLAN,
>              pmsi_tunnel_type=PMSI_TYPE_INGRESS_REP,
>              vni=vni, # send through BGPPathAttributePmsiTunnel
>              #
>              route_dist=evpn_route_dist,
>              ethernet_tag_id=0,
>              ip_addr=loopback_IPv4,
>              next_hop=loopback_IPv4,
>          )
> 
> 
> 
> Regards,
> Albert Siersema
> 
> 
> 
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> 
> 
> 
> _______________________________________________
> Ryu-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/ryu-devel
> 

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to