Hi Albert,
Thank you for posting your patch again!
May I have some comments on this patch?
> diff -uprN ryu_git/ryu/services/protocols/bgp/peer.py
> ryu_allowasin/ryu/services/protocols/bgp/peer.py
> --- ryu_git/ryu/services/protocols/bgp/peer.py 2017-01-16
> 18:05:25.376542037 +0100
> +++ ryu_allowasin/ryu/services/protocols/bgp/peer.py 2017-02-01
> 15:54:35.986179005 +0100
> @@ -1623,9 +1623,12 @@ class Peer(Source, Sink, NeighborConfLis
> aspath = umsg_pattrs.get(BGP_ATTR_TYPE_AS_PATH)
> # Check if AS_PATH has loops.
> if aspath.has_local_as(self.local_as):
> - LOG.error('Update message AS_PATH has loops. Ignoring this'
> - ' UPDATE. %s', update_msg)
> - return
> + if self._common_conf.allow_local_as_in:
> + LOG.info('Allowing update message AS_PATH with own AS
> {}'.format(update_msg))
> + else:
> + LOG.error('Update message AS_PATH has loops. Ignoring this'
> + ' UPDATE. %s', update_msg)
> + return
>
> next_hop = update_msg.get_path_attr(BGP_ATTR_TYPE_NEXT_HOP).value
> # Nothing to do if we do not have any new NLRIs in this message.
The above sill has pep8 warnings (an invalid indentation and too long lines).
BTW, we are using Git for indexing versions.
Would you send patches in the git format?
e.g.)
$ git clone https://github.com/osrg/ryu.git
$ cd ryu
Create a branch, if needed.
$ git checkout -b <your branch>
<... some modifications ...>
$ git add <modified files>
$ git commit -s
Format patches. ("N" means the number of commits to be included)
$ git format-patch HEAD~N
Then, you can use *.patch for submitting!
By Mailer or "git send-email" command like:
$ git send-email --to="[email protected]" *.patch
I attached the patch created like the above.
Could you review and test my patch?
FYI, if you can use Git, applying patches is easy:
e.g.)
Save *.patch files and
$ git am *.patch
Thanks,
Iwase
On 2017年02月02日 00:09, Albert Siersema wrote:
> Hello,
>
> See attached patch to support updates with local ASN in the path.
> The use case for this patch is auto RD/RT and simplified leaf/spine
> architectures, see below.
>
> Cisco/Juniper/Cumulus and undoubtedly more vendors offer a
> simplification/generalization of the EVPN configuration by not having to
> specify RD/RT's, e.g. in Cisco NX-OS syntax:
>
> evpn
> vni 10311 l2
> rd auto
> route-target import auto
> route-target export auto
>
> See:
> http://www.cisco.com/c/en/us/products/collateral/switches/nexus-9000-series-switches/guide-c07-734107.html
> search for "MP-eBGP Design with VTEP Leaf Nodes in the Same BGP Autonomous
> System"
>
> In short:
> To further simplify configuration (and facilitate auto RD) all physical leaf
> switches share the same ASN (e.g. 65511). All spine switches in turn share an
> ASN as well (e.g. 65510).
> This means an AS path has to be accepted with the local ASN in it. To this
> end, Cisco includes an 'allowas-in' configuration statement.
>
> Currently, my patch is a simple boolean value and a check on this boolean to
> allow the local ASN in the loop check.
> A better approach might be test for a maximum number of occurrences of the
> ASN and/or check if there is a non-local ASN in the path as well.
> What do you guys think ?
>
> Cheers,
> 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
>
>From 1f3e9fb4bc0ced53321041e9db0f91679962d977 Mon Sep 17 00:00:00 2001
From: IWASE Yusuke <[email protected]>
Date: Thu, 2 Feb 2017 11:52:49 +0900
Subject: [PATCH] BGPSpeaker: Option for allowing local AS in AS_PATH
For the leaf/spine architecture with the shared AS number, for example,
Cisco has the option for allowing own local AS in AS_PATH.
This patch adds the similar option for this feature into BGPSpeaker.
Suggested-by: Albert Siersema <[email protected]>
Signed-off-by: IWASE Yusuke <[email protected]>
---
ryu/services/protocols/bgp/bgpspeaker.py | 9 ++++++++-
ryu/services/protocols/bgp/peer.py | 24 ++++++++++++++++++------
ryu/services/protocols/bgp/rtconf/common.py | 15 ++++++++++++++-
3 files changed, 40 insertions(+), 8 deletions(-)
diff --git a/ryu/services/protocols/bgp/bgpspeaker.py b/ryu/services/protocols/bgp/bgpspeaker.py
index d82be62..6226fc5 100644
--- a/ryu/services/protocols/bgp/bgpspeaker.py
+++ b/ryu/services/protocols/bgp/bgpspeaker.py
@@ -63,6 +63,7 @@ from ryu.services.protocols.bgp.rtconf.common import DEFAULT_LABEL_RANGE
from ryu.services.protocols.bgp.rtconf.common import REFRESH_MAX_EOR_TIME
from ryu.services.protocols.bgp.rtconf.common import REFRESH_STALEPATH_TIME
from ryu.services.protocols.bgp.rtconf.common import LABEL_RANGE
+from ryu.services.protocols.bgp.rtconf.common import ALLOW_LOCAL_AS_IN
from ryu.services.protocols.bgp.rtconf import neighbors
from ryu.services.protocols.bgp.rtconf import vrfs
from ryu.services.protocols.bgp.rtconf.base import CAP_MBGP_IPV4
@@ -174,7 +175,8 @@ class BGPSpeaker(object):
peer_up_handler=None,
ssh_console=False,
ssh_port=None, ssh_host=None, ssh_host_key=None,
- label_range=DEFAULT_LABEL_RANGE):
+ label_range=DEFAULT_LABEL_RANGE,
+ allow_local_as_in=False):
"""Create a new BGPSpeaker object with as_number and router_id to
listen on bgp_server_port.
@@ -222,6 +224,10 @@ class BGPSpeaker(object):
``label_range`` specifies the range of MPLS labels generated
automatically.
+
+ ``allow_local_as_in`` allows the local AS number in AS_PATH.
+ This option is useful for the leaf/spine architecture with the
+ shared AS number, for example.
"""
super(BGPSpeaker, self).__init__()
@@ -232,6 +238,7 @@ class BGPSpeaker(object):
REFRESH_STALEPATH_TIME: refresh_stalepath_time,
REFRESH_MAX_EOR_TIME: refresh_max_eor_time,
LABEL_RANGE: label_range,
+ ALLOW_LOCAL_AS_IN: allow_local_as_in,
}
self._core_start(settings)
self._init_signal_listeners()
diff --git a/ryu/services/protocols/bgp/peer.py b/ryu/services/protocols/bgp/peer.py
index 8bf96d6..f52b1d2 100644
--- a/ryu/services/protocols/bgp/peer.py
+++ b/ryu/services/protocols/bgp/peer.py
@@ -1623,9 +1623,15 @@ class Peer(Source, Sink, NeighborConfListener, Activity):
aspath = umsg_pattrs.get(BGP_ATTR_TYPE_AS_PATH)
# Check if AS_PATH has loops.
if aspath.has_local_as(self.local_as):
- LOG.error('Update message AS_PATH has loops. Ignoring this'
- ' UPDATE. %s', update_msg)
- return
+ if self._common_conf.allow_local_as_in:
+ LOG.debug(
+ 'Allowing own local AS in AS_PATH on UPDATE message: %s',
+ update_msg)
+ else:
+ LOG.error(
+ 'Looped AS_PATH detected. Ignoring UPDATE message: %s',
+ update_msg)
+ return
next_hop = update_msg.get_path_attr(BGP_ATTR_TYPE_NEXT_HOP).value
# Nothing to do if we do not have any new NLRIs in this message.
@@ -1751,9 +1757,15 @@ class Peer(Source, Sink, NeighborConfListener, Activity):
aspath = umsg_pattrs.get(BGP_ATTR_TYPE_AS_PATH)
# Check if AS_PATH has loops.
if aspath.has_local_as(self.local_as):
- LOG.error('Update message AS_PATH has loops. Ignoring this'
- ' UPDATE. %s', update_msg)
- return
+ if self._common_conf.allow_local_as_in:
+ LOG.debug(
+ 'Allowing own local AS in AS_PATH on UPDATE message: %s',
+ update_msg)
+ else:
+ LOG.error(
+ 'Looped AS_PATH detected. Ignoring UPDATE message: %s',
+ update_msg)
+ return
if msg_rf in (RF_IPv4_VPN, RF_IPv6_VPN):
# Check if we have Extended Communities Attribute.
diff --git a/ryu/services/protocols/bgp/rtconf/common.py b/ryu/services/protocols/bgp/rtconf/common.py
index acf4634..cf009a0 100644
--- a/ryu/services/protocols/bgp/rtconf/common.py
+++ b/ryu/services/protocols/bgp/rtconf/common.py
@@ -40,6 +40,7 @@ ROUTER_ID = 'router_id'
LABEL_RANGE = 'label_range'
LABEL_RANGE_MAX = 'max'
LABEL_RANGE_MIN = 'min'
+ALLOW_LOCAL_AS_IN = 'allow_local_as_in'
# Configuration that can be set at global level as well as per context
# (session/vrf) level
@@ -140,6 +141,11 @@ def validate_label_range(label_range):
return label_range
+@validate(name=ALLOW_LOCAL_AS_IN)
+def validate_allow_local_as_in(allow):
+ return allow
+
+
@validate(name=BGP_SERVER_PORT)
def validate_bgp_server_port(server_port):
if not isinstance(server_port, numbers.Integral):
@@ -208,13 +214,16 @@ class CommonConf(BaseConf):
LABEL_RANGE, BGP_SERVER_PORT,
TCP_CONN_TIMEOUT,
BGP_CONN_RETRY_TIME,
- MAX_PATH_EXT_RTFILTER_ALL])
+ MAX_PATH_EXT_RTFILTER_ALL,
+ ALLOW_LOCAL_AS_IN])
def __init__(self, **kwargs):
super(CommonConf, self).__init__(**kwargs)
def _init_opt_settings(self, **kwargs):
super(CommonConf, self)._init_opt_settings(**kwargs)
+ self._settings[ALLOW_LOCAL_AS_IN] = compute_optional_conf(
+ ALLOW_LOCAL_AS_IN, False, **kwargs)
self._settings[LABEL_RANGE] = compute_optional_conf(
LABEL_RANGE, DEFAULT_LABEL_RANGE, **kwargs)
self._settings[REFRESH_STALEPATH_TIME] = compute_optional_conf(
@@ -268,6 +277,10 @@ class CommonConf(BaseConf):
return self._settings[LABEL_RANGE]
@property
+ def allow_local_as_in(self):
+ return self._settings[ALLOW_LOCAL_AS_IN]
+
+ @property
def bgp_server_port(self):
return self._settings[BGP_SERVER_PORT]
--
2.7.4
------------------------------------------------------------------------------
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
