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
