Hi Iwamoto-San,
On 2016年12月12日 11:21, IWAMOTO Toshihiro wrote: > At Thu, 8 Dec 2016 15:28:42 +0900, > Iwase Yusuke wrote: >> >> Hi Kakuma-San, >> >> >> On 2016年12月08日 15:06, fumihiko kakuma wrote: >>> On Mon, 7 Nov 2016 16:04:23 +0900 >>> IWASE Yusuke <iwase.yusu...@gmail.com> wrote: >>> >>>> This patch enables BGPSpeaker to set the capability for IPv6 unicast >>>> address family. >>>> >>>> Signed-off-by: IWASE Yusuke <iwase.yusu...@gmail.com> >>>> --- >>>> ryu/services/protocols/bgp/bgp_sample_conf.py | 32 >>>> ++++++++++++++++++++++----- >>>> ryu/services/protocols/bgp/bgpspeaker.py | 26 +++++++++------------- >>>> ryu/tests/integrated/common/ryubgp.py | 11 +++++---- >>>> 3 files changed, 42 insertions(+), 27 deletions(-) >>>> >>> >>> snip >>> >>>> diff --git a/ryu/services/protocols/bgp/bgpspeaker.py >>>> b/ryu/services/protocols/bgp/bgpspeaker.py >>>> index 4934e41..9e30d3e 100644 >>>> --- a/ryu/services/protocols/bgp/bgpspeaker.py >>>> +++ b/ryu/services/protocols/bgp/bgpspeaker.py >>>> @@ -64,6 +64,7 @@ from ryu.services.protocols.bgp.rtconf.base import >>>> CAP_FOUR_OCTET_AS_NUMBER >>>> from ryu.services.protocols.bgp.rtconf.base import MULTI_EXIT_DISC >>>> from ryu.services.protocols.bgp.rtconf.base import SITE_OF_ORIGINS >>>> from ryu.services.protocols.bgp.rtconf.neighbors import >>>> DEFAULT_CAP_MBGP_IPV4 >>>> +from ryu.services.protocols.bgp.rtconf.neighbors import >>>> DEFAULT_CAP_MBGP_IPV6 >>>> from ryu.services.protocols.bgp.rtconf.neighbors import >>>> DEFAULT_CAP_MBGP_VPNV4 >>>> from ryu.services.protocols.bgp.rtconf.neighbors import >>>> DEFAULT_CAP_MBGP_VPNV6 >>>> from ryu.services.protocols.bgp.rtconf.neighbors import >>>> DEFAULT_CAP_MBGP_EVPN >>>> @@ -289,6 +290,7 @@ class BGPSpeaker(object): >>>> >>>> def neighbor_add(self, address, remote_as, >>>> enable_ipv4=DEFAULT_CAP_MBGP_IPV4, >>>> + enable_ipv6=DEFAULT_CAP_MBGP_IPV6, >>>> enable_vpnv4=DEFAULT_CAP_MBGP_VPNV4, >>>> enable_vpnv6=DEFAULT_CAP_MBGP_VPNV6, >>>> enable_evpn=DEFAULT_CAP_MBGP_EVPN, >>>> @@ -313,6 +315,9 @@ class BGPSpeaker(object): >>>> ``enable_ipv4`` enables IPv4 address family for this >>>> neighbor. The default is True. >>>> >>>> + ``enable_ipv6`` enables IPv6 address family for this >>>> + neighbor. The default is False. >>>> + >>>> ``enable_vpnv4`` enables VPNv4 address family for this >>>> neighbor. The default is False. >>>> >>>> @@ -371,23 +376,12 @@ class BGPSpeaker(object): >>>> CONNECT_MODE: connect_mode, >>>> CAP_ENHANCED_REFRESH: enable_enhanced_refresh, >>>> CAP_FOUR_OCTET_AS_NUMBER: enable_four_octet_as_number, >>>> + CAP_MBGP_IPV4: enable_ipv4, >>>> + CAP_MBGP_IPV6: enable_ipv6, >>>> + CAP_MBGP_VPNV4: enable_vpnv4, >>>> + CAP_MBGP_VPNV6: enable_vpnv6, >>>> + CAP_MBGP_EVPN: enable_evpn, >>>> } >>>> - # v6 advertisement is available with only v6 peering >>>> - if netaddr.valid_ipv4(address): >>>> - bgp_neighbor[CAP_MBGP_IPV4] = enable_ipv4 >>>> - bgp_neighbor[CAP_MBGP_IPV6] = False >>>> - bgp_neighbor[CAP_MBGP_VPNV4] = enable_vpnv4 >>>> - bgp_neighbor[CAP_MBGP_VPNV6] = enable_vpnv6 >>>> - bgp_neighbor[CAP_MBGP_EVPN] = enable_evpn >>>> - elif netaddr.valid_ipv6(address): >>>> - bgp_neighbor[CAP_MBGP_IPV4] = False >>>> - bgp_neighbor[CAP_MBGP_IPV6] = True >>>> - bgp_neighbor[CAP_MBGP_VPNV4] = False >>>> - bgp_neighbor[CAP_MBGP_VPNV6] = False >>>> - bgp_neighbor[CAP_MBGP_EVPN] = enable_evpn >>>> - else: >>>> - # FIXME: should raise an exception >>>> - pass >>>> >>>> if multi_exit_disc: >>>> bgp_neighbor[MULTI_EXIT_DISC] = multi_exit_disc >>> >>> Prior to this patch, CAP_MBGP_IPV6 capability is advertised to BGP >>> peer when ipv6 transport is used for a BGP connection. So, v6 BGP on >>> v6 transport worked automagically. >>> Now the CAP_MBGP_IPV6 advertisement is controlled by the new >>> enable_ipv6 argument, existing applications[1], which don't know the new >>> argument, fail to advertise IPv6 routes. >>> >>> I think this patch is in the right direction, but I wish I could >>> receive some warning of such an API change. >> >> Thank you for pointing out. >> >> How have we better to notify changes? >> Describing NOTE more explicitly on commit logs? >> Or reporting on OpenStack lauchpad? > > The commit message just says "enable to set ipv6 capability". > It is very difficult to know that one needs to explicitly set ipv6 > capability from this commit. IMO, the commit message should have said > "you need to explicitly set enable_ipv6 when using ipv6 peers" or > somesuch. > > For maintaining API compatibility, having one or two release cycles of > deprecation period, which raises warning messages if deprecated APIs > are used, will help users. OTOH, deprecation cycles slow down > depelopment and I am not aware of which policy ryu should take. Thank you for your advice. I'm sorry commit message is my fault. Especially in this case, I could implement the backward compatibility, I should do this. > >> Then, for quick-fix, how about setting IPv6 capability enable when >> neighbor IP address is valid IPv6 address automatically? >> >> $ git diff >> diff --git a/ryu/services/protocols/bgp/bgpspeaker.py >> b/ryu/services/protocols/bgp/bgpspeaker.py >> index c56f8dd..fed47bf 100644 >> --- a/ryu/services/protocols/bgp/bgpspeaker.py >> +++ b/ryu/services/protocols/bgp/bgpspeaker.py >> @@ -372,6 +372,9 @@ class BGPSpeaker(object): >> CONNECT_MODE_BOTH use both methods. >> The default is CONNECT_MODE_BOTH. >> """ >> + # For the backward compatibility >> + if netaddr.valid_ipv6(address): >> + enable_ipv6 = True >> bgp_neighbor = { >> neighbors.IP_ADDRESS: address, >> neighbors.REMOTE_AS: remote_as, >> >> >> Thanks, >> Iwase >> >>> >>> >>> [1] e.g. neutron-dynamic-routing on openstack >>> https://github.com/openstack/neutron-dynamic-routing/blob/master/neutron_dynamic_routing/services/bgp/agent/driver/ryu/driver.py#L126 >>> >>> Thanks, >>> kakuma >>> >> >> ------------------------------------------------------------------------------ >> Developer Access Program for Intel Xeon Phi Processors >> Access to Intel Xeon Phi processor-based developer platforms. >> With one year of Intel Parallel Studio XE. >> Training and support from Colfax. >> Order your platform today.http://sdm.link/xeonphi >> _______________________________________________ >> Ryu-devel mailing list >> Ryu-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/ryu-devel > > ------------------------------------------------------------------------------ > Developer Access Program for Intel Xeon Phi Processors > Access to Intel Xeon Phi processor-based developer platforms. > With one year of Intel Parallel Studio XE. > Training and support from Colfax. > Order your platform today.http://sdm.link/xeonphi > _______________________________________________ > Ryu-devel mailing list > Ryu-devel@lists.sourceforge.net > 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 Ryu-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/ryu-devel