Hi,
Thank you for updating your patch quickly!
On 2017年01月27日 19:26, Albert Siersema wrote:
> Hello,
>
>> some comments like "# [AJS]" breaks pep8 check...
>> I don't recommend you to add name into source comment...
>
> My apologies, I meant to remove them before submitting the patch, but kinda
> forgot about it :(
> Removed an unnecessary 'self.vni = vni' line as well.
> Fixed it all, now run_tests.sh runs flawlessly.
> See attachment for an updated patch.
> I did leave in some comments with regard to why the vni parameter is
> accepted, but these can be removed of course.
Thanks!
But... the packet library should be follow the "on-wire"(RFC) structure of
packet,
I guess.
How about omitting "vni" key from "kwargs" before being passed to EVPN NLIR
class?
$ git diff ryu/services/protocols/bgp/core_managers/table_manager.py
diff --git a/ryu/services/protocols/bgp/core_managers/table_manager.py
b/ryu/services/protocols/bgp/core_managers/table_manager.py
index 0e084b8..7a4658b 100644
--- a/ryu/services/protocols/bgp/core_managers/table_manager.py
+++ b/ryu/services/protocols/bgp/core_managers/table_manager.py
@@ -36,6 +36,7 @@ from ryu.lib.packet.bgp import EvpnEsi
from ryu.lib.packet.bgp import EvpnArbitraryEsi
from ryu.lib.packet.bgp import EvpnNLRI
from ryu.lib.packet.bgp import EvpnMacIPAdvertisementNLRI
+from ryu.lib.packet.bgp import EvpnInclusiveMulticastEthernetTagNLRI
from ryu.lib.packet.bgp import IPAddrPrefix
from ryu.lib.packet.bgp import IP6AddrPrefix
@@ -543,6 +544,10 @@ class TableCoreManager(object):
if route_type == EvpnMacIPAdvertisementNLRI.ROUTE_TYPE_NAME:
# MPLS labels will be assigned automatically
kwargs['mpls_labels'] = []
+ if route_type ==
EvpnInclusiveMulticastEthernetTagNLRI.ROUTE_TYPE_NAME:
+ # Inclusive Multicast Ethernet Tag Route does not have "vni"
+ # field, omit "vni" from "kwargs" here.
+ vni = kwargs.pop('vni', None)
subclass = EvpnNLRI._lookup_type_name(route_type)
kwargs['route_dist'] = route_dist
esi = kwargs.get('esi', None)
>
>> 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.
>
> That's what I thought at first, but the "Ethernet Tag ID" field of the
> EvpnInclusiveMulticastEthernetTagNLRI packet remains 0. The 'short' answer,
> see:
> https://tools.ietf.org/html/rfc7432#section-11.2
> https://tools.ietf.org/html/rfc7432#section-12.2
> I read through the RFC's and the Ryu code plus I experimented a lot,
> including sniffing the traffic. The sniff also shows a Path Attribute
> PMSI_TUNNEL_ATTRIBUTE with a bogus MPLS Label Stack (644), the VNI is incoded
> in the MPLS LABEL (e.g. 10310644).
> i.e. the VNI gets passed as an RFC 6514 MPLS Label in the PathAttribute
> BGP_ATTR_TYEP_PMSI_TUNNEL_ATTRIBUTE.
> See class BGPPathAttributePmsiTunnel in ryu/lib/packet/bgp.py
> Most of the code was already there, I justed made it possible to pass it
> through to the path attribute :-)
Thank you for letting me know.
From draft-ietf-bess-evpn-overlay-07:
https://tools.ietf.org/html/draft-ietf-bess-evpn-overlay-07
using VNI implies the data plane encapsulation type [RFC5512] is VXLAN or
NVGRE, right?
https://tools.ietf.org/html/rfc5512#section-4.5
If Cisco NX-OS advertises "Inclusive Multicast Ethernet Tag Route" with the BGP
Encapsulation extended community as VXLAN encapsulation, how about setting
"tunnel_type"
with "vni" at BGPSpeaker?
$ git diff ryu/services/protocols/bgp/bgpspeaker.py
diff --git a/ryu/services/protocols/bgp/bgpspeaker.py
b/ryu/services/protocols/bgp/bgpspeaker.py
index 6a0025c..a85b0bf 100644
--- a/ryu/services/protocols/bgp/bgpspeaker.py
+++ b/ryu/services/protocols/bgp/bgpspeaker.py
@@ -638,6 +638,9 @@ class BGPSpeaker(object):
EVPN_ETHERNET_TAG_ID: ethernet_tag_id,
IP_ADDR: ip_addr,
})
+ # Set tunnel type specific arguments
+ if tunnel_type in [TUNNEL_TYPE_VXLAN, TUNNEL_TYPE_NVGRE]:
+ kwargs[EVPN_VNI] = vni
# Set PMSI Tunnel Attribute arguments
if pmsi_tunnel_type in [
PMSI_TYPE_NO_TUNNEL_INFO,
>
>> 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
>
> Without my changes, passing ip_addr=None fails, e.g. :
>
> 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,
> next_hop=loopback_IPv4,
> vni=0,
> )
>
> Traceback (most recent call last):
> File
> "/usr/local/lib/python2.7/dist-packages/ryu/services/protocols/bgp/api/base.py",
> line 205, in call
> return call(**kwargs)
> File
> "/usr/local/lib/python2.7/dist-packages/ryu/services/protocols/bgp/api/base.py",
> line 169, in wrapped_fun
> validator(opt_value)
> File
> "/usr/local/lib/python2.7/dist-packages/ryu/services/protocols/bgp/api/prefix.py",
> line 196, in is_valid_ip_addr
> conf_value=addr)
> ConfigValueError: 200.4 - Incorrect Value for configuration: ip_addr
>
> Traceback (most recent call last):
> File "./bgp-ryu.py", line 198, in <module>
> bgp_speaker = myBGP()
> File "./bgp-ryu.py", line 51, in __init__
> self.start_BGP()
> File "./bgp-ryu.py", line 191, in start_BGP
> vni=0,
> File
> "/usr/local/lib/python2.7/dist-packages/ryu/services/protocols/bgp/bgpspeaker.py",
> line 671, in evpn_prefix_add
> call(func_name, **kwargs)
> File
> "/usr/local/lib/python2.7/dist-packages/ryu/services/protocols/bgp/api/base.py",
> line 208, in call
> raise r
> ryu.services.protocols.bgp.rtconf.base.ConfigValueError: 200.4 - Incorrect
> Value for configuration: ip_addr
Thank you for pointing out!
Yes, you are right.
We need "is_valid_ip_addr" in prefix.py.
Thanks,
Iwase
>
>
> Regards,
> Albert
------------------------------------------------------------------------------
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
