bgp: refine and fix filter code

- change the name of the filter class from PrefixList to PrefixFilter to
  make it easy to understand the role of this class.
- update docs
- fix bugs

Signed-off-by: ISHIDA Wataru <ishida.wataru@lab.ntt.co.jp>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

doc/source/library_bgp_speaker_ref.rst

@@ -11,5 +11,5 @@ BGPSpeaker class
 .. autoclass:: ryu.services.protocols.bgp.bgpspeaker.EventPrefix
    :members:
 
-.. autoclass:: ryu.services.protocols.bgp.bgpspeaker.PrefixList
+.. autoclass:: ryu.services.protocols.bgp.info_base.base.PrefixFilter
    :members:

ryu/services/protocols/bgp/bgpspeaker.py

@@ -55,13 +55,9 @@
 from ryu.services.protocols.bgp.rtconf.neighbors import PASSWORD
 from ryu.services.protocols.bgp.rtconf.neighbors import IN_FILTER
 from ryu.services.protocols.bgp.rtconf.neighbors import OUT_FILTER
-from ryu.lib.packet.bgp import RF_IPv4_UC, RF_IPv6_UC
+from ryu.services.protocols.bgp.info_base.base import Filter
 
 
-OUT_FILTER_RF_IPv4_UC = RF_IPv4_UC
-OUT_FILTER_RF_IPv6_UC = RF_IPv6_UC
-IN_FILTER_RF_IPv4_UC = RF_IPv4_UC
-IN_FILTER_RF_IPv6_UC = RF_IPv6_UC
 NEIGHBOR_CONF_MED = 'multi_exit_disc'
 
 
@@ -371,55 +367,59 @@ def rib_get(self, family='ipv4'):
         show['params'] = ['rib', family]
         return call('operator.show', **show)
 
-    def out_filter_set(self, address, prefix_lists,
-                       route_family=OUT_FILTER_RF_IPv4_UC):
-        """ This method sets out-filter to neighbor.
+    def _set_filter(self, filter_type, address, filters):
+        assert filter_type in ('in', 'out'),\
+            'filter type must be \'in\' or \'out\''
 
-        ``address`` specifies the IP address of the peer.
+        assert all(isinstance(f, Filter) for f in filters),\
+            'all the items in filters must be an instance of Filter sub-class'
 
-        ``prefix_lists`` specifies prefix list to filter path advertisement.
-         This parameter must be list that has PrefixList objects.
+        if filters is None:
+            filters = []
 
-        ``route_family`` specifies the route family for out-filter.
-        This parameter must be bgpspeaker.OUT_FILTER_RF_IPv4_UC or
-        bgpspeaker.OUT_FILTER_RF_IPv6_UC.
+        func_name = 'neighbor.' + filter_type + '_filter.set'
+        param = {}
+        param[neighbors.IP_ADDRESS] = address
+        if filter_type == 'in':
+            param[neighbors.IN_FILTER] = filters
+        else:
+            param[neighbors.OUT_FILTER] = filters
+        call(func_name, **param)
 
+    def out_filter_set(self, address, filters):
+        """ This method sets out-filter to neighbor.
+
+        ``address`` specifies the IP address of the peer.
+
+        ``filters`` specifies a filter list to filter the path advertisement.
+        The contents must be an instance of Filter sub-class
 
         If you want to define out-filter that send only a particular
-        prefix to neighbor, prefix_lists can be created as follows;
+        prefix to neighbor, filters can be created as follows;
 
-          p = PrefixList('10.5.111.0/24', policy=PrefixList.POLICY_PERMIT)
+          p = PrefixFilter('10.5.111.0/24',
+                           policy=PrefixFilter.POLICY_PERMIT)
 
-          all = PrefixList('0.0.0.0/0', policy=PrefixList.POLICY_DENY)
+          all = PrefixFilter('0.0.0.0/0',
+                             policy=PrefixFilter.POLICY_DENY)
 
           pList = [p, all]
 
           self.bgpspeaker.out_filter_set(neighbor_address, pList)
 
         NOTE:
-        out-filter evaluates prefixes in the order of PrefixList in the pList.
+        out-filter evaluates paths in the order of Filter in the pList.
 
         """
 
-        assert route_family in (OUT_FILTER_RF_IPv4_UC,
-                                OUT_FILTER_RF_IPv6_UC),\
-            "route family must be IPv4 or IPv6"
-
-        if prefix_lists is None:
-            prefix_lists = []
-
-        func_name = 'neighbor.out_filter.set'
-        param = {}
-        param[neighbors.IP_ADDRESS] = address
-        param[neighbors.OUT_FILTER] = prefix_lists
-        call(func_name, **param)
+        self._set_filter('out', address, filters)
 
     def out_filter_get(self, address):
         """ This method gets out-filter setting from the specified neighbor.
 
         ``address`` specifies the IP address of the peer.
 
-        Returns list object that has PrefixList objects.
+        Returns a list object containing an instance of Filter sub-class
 
         """
 
@@ -429,22 +429,28 @@ def out_filter_get(self, address):
         out_filter = call(func_name, **param)
         return out_filter
 
-    def in_filter_set(self, address, prefix_lists,
-                      route_family=IN_FILTER_RF_IPv4_UC):
-        assert route_family in (IN_FILTER_RF_IPv4_UC,
-                                IN_FILTER_RF_IPv6_UC),\
-            "route family must be IPv4 or IPv6"
+    def in_filter_set(self, address, filters):
+        """This method sets in-bound filters to a neighbor.
 
-        if prefix_lists is None:
-            prefix_lists = []
+        ``address`` specifies the IP address of the neighbor
 
-        func_name = 'neighbor.in_filter.set'
-        param = {}
-        param[neighbors.IP_ADDRESS] = address
-        param[neighbors.IN_FILTER] = prefix_lists
-        call(func_name, **param)
+        ``filters`` specifies filter list applied before advertised paths are
+        imported to the global rib. All the items in the list must be an
+        instance of Filter sub-class.
+
+        """
+
+        self._set_filter('in', address, filters)
 
     def in_filter_get(self, address):
+        """This method gets in-bound filters of the specified neighbor.
+
+        ``address`` specifies the IP address of the neighbor.
+
+        Returns a list object containing an instance of Filter sub-class
+
+        """
+
         func_name = 'neighbor.in_filter.get'
         param = {}
         param[neighbors.IP_ADDRESS] = address

ryu/services/protocols/bgp/info_base/base.py

@@ -833,53 +833,77 @@ def policy(self):
 
     @abstractmethod
     def evaluate(self, path):
+        """ This method evaluates the path.
+
+        Returns this object's policy and the result of matching.
+        If the specified prefix matches this object's prefix and
+        ge and le condition,
+        this method returns True as the matching result.
+
+        ``path`` specifies the path. prefix must be string.
+
+        """
+        raise NotImplementedError()
+
+    @abstractmethod
+    def clone(self):
+        """ This method clones Filter object.
+
+        Returns Filter object that has the same values with the original one.
+
+        """
         raise NotImplementedError()
 
 
-class PrefixList(Filter):
+class PrefixFilter(Filter):
     """
-    used to specify a prefix for out-filter.
+    used to specify a prefix for filter.
 
-    We can create PrefixList object as follows.
+    We can create PrefixFilter object as follows.
 
-    prefix_list = PrefixList('10.5.111.0/24', policy=PrefixList.POLICY_PERMIT)
+    prefix_filter = PrefixFilter('10.5.111.0/24',
+                                 policy=PrefixFilter.POLICY_PERMIT)
 
     ================ ==================================================
     Attribute        Description
     ================ ==================================================
-    prefix           A prefix used for out-filter
-    policy           PrefixList.POLICY.PERMIT or PrefixList.POLICY_DENY
-    ge               Prefix length that will be applied out-filter.
+    prefix           A prefix used for this filter
+    policy           PrefixFilter.POLICY.PERMIT or PrefixFilter.POLICY_DENY
+    ge               Prefix length that will be applied to this filter.
                      ge means greater than or equal.
-    le               Prefix length that will be applied out-filter.
+    le               Prefix length that will be applied to this filter.
                      le means less than or equal.
     ================ ==================================================
 
 
-    For example, when PrefixList object is created as follows:
+    For example, when PrefixFilter object is created as follows:
 
-    * p = PrefixList('10.5.111.0/24',
-                   policy=PrefixList.POLICY_DENY,
-                   ge=26, le=28)
+    * p = PrefixFilter('10.5.111.0/24',
+                       policy=PrefixFilter.POLICY_DENY,
+                       ge=26, le=28)
 
 
     prefixes which match 10.5.111.0/24 and its length matches
-    from 26 to 28 will be filtered and stopped to send to neighbor
-    because of POLICY_DENY. If you specify POLICY_PERMIT,
-    the path is sent to neighbor.
+    from 26 to 28 will be filtered.
+    When this filter is used as an out-filter, it will stop sending
+    the path to neighbor because of POLICY_DENY.
+    When this filter is used as in-filter, it will stop importing the path
+    to the global rib because of POLICY_DENY.
+    If you specify POLICY_PERMIT, the path is sent to neighbor or imported to
+    the global rib.
 
     If you don't want to send prefixes 10.5.111.64/26 and 10.5.111.32/27
     and 10.5.111.16/28, and allow to send other 10.5.111.0's prefixes,
     you can do it by specifying as follows;
 
-    * p = PrefixList('10.5.111.0/24',
-                   policy=PrefixList.POLICY_DENY,
-                   ge=26, le=28).
+    * p = PrefixFilter('10.5.111.0/24',
+                       policy=PrefixFilter.POLICY_DENY,
+                       ge=26, le=28).
 
     """
 
     def __init__(self, prefix, policy, ge=None, le=None):
-        super(PrefixList, self).__init__(policy)
+        super(PrefixFilter, self).__init__(policy)
         self._prefix = prefix
         self._network = netaddr.IPNetwork(prefix)
         self._ge = ge
@@ -892,7 +916,7 @@ def __repr__(self):
         policy = 'PERMIT' \
             if self._policy == self.POLICY_PERMIT else 'DENY'
 
-        return 'PrefixList(prefix=%s,policy=%s,ge=%s,le=%s)'\
+        return 'PrefixFilter(prefix=%s,policy=%s,ge=%s,le=%s)'\
                % (self._prefix, policy, self._ge, self._le)
 
     @property
@@ -947,14 +971,14 @@ def evaluate(self, path):
         return self.policy, result
 
     def clone(self):
-        """ This method clones PrefixList object.
+        """ This method clones PrefixFilter object.
 
-        Returns PrefixList object that has the same values with the
+        Returns PrefixFilter object that has the same values with the
         original one.
 
         """
 
-        return PrefixList(self.prefix,
-                          policy=self._policy,
-                          ge=self._ge,
-                          le=self._le)
+        return PrefixFilter(self.prefix,
+                            policy=self._policy,
+                            ge=self._ge,
+                            le=self._le)

ryu/services/protocols/bgp/peer.py

@@ -29,7 +29,7 @@
 from ryu.services.protocols.bgp import constants as const
 from ryu.services.protocols.bgp.model import OutgoingRoute
 from ryu.services.protocols.bgp.model import SentRoute
-from ryu.services.protocols.bgp.info_base.base import PrefixList
+from ryu.services.protocols.bgp.info_base.base import PrefixFilter
 from ryu.services.protocols.bgp.model import ReceivedRoute
 from ryu.services.protocols.bgp.net_ctrl import NET_CONTROLLER
 from ryu.services.protocols.bgp.rtconf.neighbors import NeighborConfListener
@@ -488,10 +488,10 @@ def _apply_filter(self, filters, path):
 
         for filter_ in filters:
             policy, is_matched = filter_.evaluate(path)
-            if policy == PrefixList.POLICY_PERMIT and is_matched:
+            if policy == PrefixFilter.POLICY_PERMIT and is_matched:
                 block = False
                 break
-            elif policy == PrefixList.POLICY_DENY and is_matched:
+            elif policy == PrefixFilter.POLICY_DENY and is_matched:
                 block = True
                 blocked_cause = filter_.prefix + ' - DENY'
                 break

ryu/services/protocols/bgp/rtconf/neighbors.py

@@ -62,7 +62,7 @@
 from ryu.services.protocols.bgp.utils.validation import is_valid_ipv4
 from ryu.services.protocols.bgp.utils.validation import is_valid_old_asn
 from ryu.services.protocols.bgp.info_base.base import Filter
-from ryu.services.protocols.bgp.info_base.base import PrefixList
+from ryu.services.protocols.bgp.info_base.base import PrefixFilter
 
 LOG = logging.getLogger('bgpspeaker.rtconf.neighbor')
 
@@ -173,13 +173,13 @@ def validate_remote_as(asn):
 def valid_prefix_filter(filter_):
     policy = filter_.get('policy', None)
     if policy == 'permit':
-        policy = PrefixList.POLICY_PERMIT
+        policy = PrefixFilter.POLICY_PERMIT
     else:
-        policy = PrefixList.POLICY_DENY
+        policy = PrefixFilter.POLICY_DENY
     prefix = filter_['prefix']
     ge = filter_.get('ge', None)
     le = filter_.get('le', None)
-    return PrefixList(prefix, policy, ge=ge, le=le)
+    return PrefixFilter(prefix, policy, ge=ge, le=le)
 
 PREFIX_FILTER = 'prefix_filter'