Thank you for comments and code.

As you mentioned, I should use "set" for comparing BGP capabilities
to simplify the code.

But in order to use it, it is neccesary to override __hash__ and __cmp__ method
at BGPOptParamCapabilityEnhancedRouteRefresh class or the parent class.

So I implemented __hash__ and __cmp__ at _OptParamCapability class on trial
and I'm testing now.
Currently it seems to work without errors,
but I'm not sure that there is no side effect associated with this override.

I am going to investigate codes that are using BGP capability classes.

Thank you

(2014/07/18 16:01), FUJITA Tomonori wrote:
> On Fri, 18 Jul 2014 15:09:12 +0900
> Hiroshi Yokoi <[email protected]> wrote:
>
>> I changed codes to check ENHANCED_ROUTE_REFRESH capability for simplicity.
>>
>> Signed-off-by: Hiroshi Yokoi <[email protected]>
>> ---
>>   ryu/services/protocols/bgp/speaker.py | 29 ++++++++++++++---------------
>>   1 file changed, 14 insertions(+), 15 deletions(-)
>>
>> diff --git a/ryu/services/protocols/bgp/speaker.py 
>> b/ryu/services/protocols/bgp/speaker.py
>> index d079d6f..faaf8e4 100644
>> --- a/ryu/services/protocols/bgp/speaker.py
>> +++ b/ryu/services/protocols/bgp/speaker.py
>> @@ -173,16 +173,12 @@ class BgpProtocol(Protocol, Activity):
>>           if not self.recv_open_msg:
>>               raise ValueError('Did not yet receive peers open message.')
>>
>> -        err_cap_enabled = False
>> -        local_cap = self.sent_open_msg.caps
>> -        peer_cap = self.recv_open_msg.caps
>> -        # Both local and peer should advertise ERR capability for it to be
>> -        # enabled.
>> -        if (local_cap.get(BGP_CAP_ENHANCED_ROUTE_REFRESH) and
>> -                peer_cap.get(BGP_CAP_ENHANCED_ROUTE_REFRESH)):
>> -            err_cap_enabled = True
>
> Let's avoid a virtual long line.
>
>> +        for cap in set(self.sent_open_msg.opt_param) \
>> +                & set(self.recv_open_msg.opt_param):
>> +            if cap.cap_code == BGP_CAP_ENHANCED_ROUTE_REFRESH:
>> +                return True
>>
>> -        return err_cap_enabled
>> +        return False
>>
>>       def _check_route_fmly_adv(self, open_msg, route_family):
>>           match_found = False
>> @@ -218,19 +214,22 @@ class BgpProtocol(Protocol, Activity):
>>
>>       @property
>>       def negotiated_afs(self):
>> -        local_caps = self.sent_open_msg.caps
>> -        remote_caps = self.recv_open_msg.caps
>> +        local_caps = self.sent_open_msg.opt_param
>> +        remote_caps = self.recv_open_msg.opt_param
>> +
>> +        local_mbgp_cap = [cap for cap in local_caps
>> +                          if cap.cap_code == BGP_CAP_MULTIPROTOCOL]
>> +        remote_mbgp_cap = [cap for cap in remote_caps
>> +                           if cap.cap_code == BGP_CAP_MULTIPROTOCOL]
>
> Could we do like the above?
>
>> -        local_mbgp_cap = local_caps.get(BGP_CAP_MULTIPROTOCOL)
>> -        remote_mbgp_cap = remote_caps.get(BGP_CAP_MULTIPROTOCOL)
>>           # Check MP_BGP capabilities were advertised.
>>           if local_mbgp_cap and remote_mbgp_cap:
>>               local_families = {
>> -                (peer_cap.route_family.afi, peer_cap.route_family.safi)
>> +                (peer_cap.afi, peer_cap.safi)
>>                   for peer_cap in local_mbgp_cap
>>               }
>>               remote_families = {
>> -                (peer_cap.route_family.afi, peer_cap.route_family.safi)
>> +                (peer_cap.afi, peer_cap.safi)
>>                   for peer_cap in remote_mbgp_cap
>>               }
>>               afi_safi = local_families.intersection(remote_families)
>> --
>
> How about this?
>
> diff --git a/ryu/services/protocols/bgp/speaker.py 
> b/ryu/services/protocols/bgp/speaker.py
> index d079d6f..4471909 100644
> --- a/ryu/services/protocols/bgp/speaker.py
> +++ b/ryu/services/protocols/bgp/speaker.py
> @@ -173,16 +173,12 @@ class BgpProtocol(Protocol, Activity):
>           if not self.recv_open_msg:
>               raise ValueError('Did not yet receive peers open message.')
>
> -        err_cap_enabled = False
> -        local_cap = self.sent_open_msg.caps
> -        peer_cap = self.recv_open_msg.caps
> -        # Both local and peer should advertise ERR capability for it to be
> -        # enabled.
> -        if (local_cap.get(BGP_CAP_ENHANCED_ROUTE_REFRESH) and
> -                peer_cap.get(BGP_CAP_ENHANCED_ROUTE_REFRESH)):
> -            err_cap_enabled = True
> -
> -        return err_cap_enabled
> +        local_caps = self.sent_open_msg.caps
> +        remote_caps = self.recv_open_msg.caps
> +        for cap in set(local_caps) & set(remote_caps):
> +            if cap.cap_code == BGP_CAP_ENHANCED_ROUTE_REFRESH:
> +                return True
> +        return False
>
>       def _check_route_fmly_adv(self, open_msg, route_family):
>           match_found = False
> @@ -218,19 +214,20 @@ class BgpProtocol(Protocol, Activity):
>
>       @property
>       def negotiated_afs(self):
> -        local_caps = self.sent_open_msg.caps
> -        remote_caps = self.recv_open_msg.caps
> +        local_caps = self.sent_open_msg.opt_param
> +        remote_caps = self.recv_open_msg.opt_param
> +
> +        mbgp_cap = [cap for cap in set(local_caps) & set(remote_caps)
> +                    if cap.cap_code == BGP_CAP_MULTIPROTOCOL]
>
> -        local_mbgp_cap = local_caps.get(BGP_CAP_MULTIPROTOCOL)
> -        remote_mbgp_cap = remote_caps.get(BGP_CAP_MULTIPROTOCOL)
>           # Check MP_BGP capabilities were advertised.
> -        if local_mbgp_cap and remote_mbgp_cap:
> +        if mbgp_cap:
>               local_families = {
> -                (peer_cap.route_family.afi, peer_cap.route_family.safi)
> +                (peer_cap.afi, peer_cap.safi)
>                   for peer_cap in local_mbgp_cap
>               }
>               remote_families = {
> -                (peer_cap.route_family.afi, peer_cap.route_family.safi)
> +                (peer_cap.afi, peer_cap.safi)
>                   for peer_cap in remote_mbgp_cap
>               }
>               afi_safi = local_families.intersection(remote_families)
>

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

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


------------------------------------------------------------------------------
Want fast and easy access to all the code in your enterprise? Index and
search up to 200,000 lines of code with a free copy of Black Duck
Code Sight - the same software that powers the world's largest code
search on Ohloh, the Black Duck Open Hub! Try it now.
http://p.sf.net/sfu/bds
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to