Hi,

Thank you for your comments.
As you mentioned, cloning is too much for only withdraw_flag.
I also believe adding paramter to notify withdraw and other information in the 
future
is more suitable than cloning.

I applied your patch and confirmed that withdraw flag can be passed to my 
application.

Thank you,
Hiroshi Yokoi

(2014/06/29 23:22), FUJITA Tomonori wrote:
> On Thu, 26 Jun 2014 18:19:41 +0900
> Hiroshi Yokoi <[email protected]> wrote:
>
>> Set is_withdraw flag to True to notify _best_path_change_handler in 
>> BGPSpeaker
>> that best path is lost.
>>
>> Signed-off-by: Hiroshi Yokoi <[email protected]>
>> ---
>>   ryu/services/protocols/bgp/info_base/ipv4.py | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ryu/services/protocols/bgp/info_base/ipv4.py 
>> b/ryu/services/protocols/bgp/info_base/ipv4.py
>> index 13d7f62..a4acdfe 100644
>> --- a/ryu/services/protocols/bgp/info_base/ipv4.py
>> +++ b/ryu/services/protocols/bgp/info_base/ipv4.py
>> @@ -37,7 +37,7 @@ class IPv4Dest(Destination, NonVrfPathProcessingMixin):
>>       ROUTE_FAMILY = RF_IPv4_UC
>>
>>       def _best_path_lost(self):
>> -        old_best_path = self._best_path
>> +        old_best_path = self._best_path.clone(for_withdrawal=True)
>>           NonVrfPathProcessingMixin._best_path_lost(self)
>>           self._core_service._signal_bus.best_path_changed(old_best_path)
>>
>
> Thanks a lot for finding!
>
> I prefer to avoid clone() since cloning here is too much. The cloned
> object will be destroyed right after best_path_changed().
>
> How about the following? Like the existing signals, emit_signal
> can notify multiple parameters.
>
> =
>>From 4e94454ec6db649b5ee4cf18a9fe8c60e1594d68 Mon Sep 17 00:00:00 2001
> From: FUJITA Tomonori <[email protected]>
> Date: Sun, 29 Jun 2014 23:20:48 +0900
> Subject: [PATCH] bgp: fix withdraw in EventPrefix notification
>
> A lost path's is_withdraw is not True so we can't use it.
>
> Signed-off-by: FUJITA Tomonori <[email protected]>
> ---
>   ryu/services/protocols/bgp/bgpspeaker.py     | 8 +++++---
>   ryu/services/protocols/bgp/info_base/ipv4.py | 4 ++--
>   ryu/services/protocols/bgp/signals/emit.py   | 4 ++--
>   3 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/ryu/services/protocols/bgp/bgpspeaker.py 
> b/ryu/services/protocols/bgp/bgpspeaker.py
> index 8e33e85..09599ed 100644
> --- a/ryu/services/protocols/bgp/bgpspeaker.py
> +++ b/ryu/services/protocols/bgp/bgpspeaker.py
> @@ -129,21 +129,23 @@ class BGPSpeaker(object):
>               ssh_cli = app_mgr.instantiate(ssh.Cli)
>               ssh_cli.start()
>
> -    def _notify_best_path_changed(self, path):
> +    def _notify_best_path_changed(self, path, is_withdraw):
>           if not path.source:
>               # ours
>               return
>           ev = EventPrefix(remote_as=path.source.remote_as,
>                            route_dist=None,
>                            prefix=path.nlri.addr + '/' + 
> str(path.nlri.length),
> -                         nexthop=path.nexthop, is_withdraw=path.is_withdraw)
> +                         nexthop=path.nexthop, is_withdraw=is_withdraw)
>           if self._best_path_change_handler:
>               self._best_path_change_handler(ev)
>
>       def _init_signal_listeners(self):
>           CORE_MANAGER.get_core_service()._signal_bus.register_listener(
>               BgpSignalBus.BGP_BEST_PATH_CHANGED,
> -            lambda _, dest: self._notify_best_path_changed(dest)
> +            lambda _, info:
> +                self._notify_best_path_changed(info['path'],
> +                                               info['is_withdraw'])
>           )
>
>       def _core_start(self, settings):
> diff --git a/ryu/services/protocols/bgp/info_base/ipv4.py 
> b/ryu/services/protocols/bgp/info_base/ipv4.py
> index 13d7f62..cf18c23 100644
> --- a/ryu/services/protocols/bgp/info_base/ipv4.py
> +++ b/ryu/services/protocols/bgp/info_base/ipv4.py
> @@ -39,11 +39,11 @@ class IPv4Dest(Destination, NonVrfPathProcessingMixin):
>       def _best_path_lost(self):
>           old_best_path = self._best_path
>           NonVrfPathProcessingMixin._best_path_lost(self)
> -        self._core_service._signal_bus.best_path_changed(old_best_path)
> +        self._core_service._signal_bus.best_path_changed(old_best_path, True)
>
>       def _new_best_path(self, best_path):
>           NonVrfPathProcessingMixin._new_best_path(self, best_path)
> -        self._core_service._signal_bus.best_path_changed(best_path)
> +        self._core_service._signal_bus.best_path_changed(best_path, False)
>
>
>   class Ipv4Table(Table):
> diff --git a/ryu/services/protocols/bgp/signals/emit.py 
> b/ryu/services/protocols/bgp/signals/emit.py
> index 18a1287..890c1bc 100644
> --- a/ryu/services/protocols/bgp/signals/emit.py
> +++ b/ryu/services/protocols/bgp/signals/emit.py
> @@ -55,7 +55,7 @@ class BgpSignalBus(SignalBus):
>               vrf_conf
>           )
>
> -    def best_path_changed(self, best_path):
> +    def best_path_changed(self, path, is_withdraw):
>           return self.emit_signal(
>               self.BGP_BEST_PATH_CHANGED,
> -            best_path)
> +            {'path': path, 'is_withdraw': is_withdraw})
>

-- 
------------------------------------------------

Hiroshi Yokoi <[email protected]>
Cloud Computing Business Department
NTT Software Corporation
Tel: +81-45-212-7393
Fax: +81-45-662-7856


------------------------------------------------------------------------------
Open source business process management suite built on Java and Eclipse
Turn processes into business applications with Bonita BPM Community Edition
Quickly connect people, data, and systems into organized workflows
Winner of BOSSIE, CODIE, OW2 and Gartner awards
http://p.sf.net/sfu/Bonitasoft
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to