I've responded inlineĊ 

-Shaun

On 24/05/2013 20:09, "Isaku Yamahata" <[email protected]> wrote:

>On Fri, May 24, 2013 at 05:00:49PM +0000, Shaun Crampton wrote:
>> From: Shaun Crampton <[email protected]>
>> 
>> Various minor fixes to OF1.3 parser.
>> 
>> - Fix undefined variables and missing import.
>> - Add missing OFPP_NONE constant, which is equivalent to OFPP_ANY.
>> 
>> Signed-off-by: Shaun Crampton <[email protected]>
>> 
>> ---
>>  ryu/ofproto/ofproto_v1_3.py        |    3 ++-
>>  ryu/ofproto/ofproto_v1_3_parser.py |   12 ++++++------
>>  2 files changed, 8 insertions(+), 7 deletions(-)
>> 
>> diff --git a/ryu/ofproto/ofproto_v1_3.py b/ryu/ofproto/ofproto_v1_3.py
>> index 3a40eef..39d7dff 100644
>> --- a/ryu/ofproto/ofproto_v1_3.py
>> +++ b/ryu/ofproto/ofproto_v1_3.py
>> @@ -97,7 +97,8 @@ OFPP_FLOOD = 0xfffffffb         # All physical ports
>> except input port and
>>  OFPP_ALL = 0xfffffffc           # All physical ports except input port.
>>  OFPP_CONTROLLER = 0xfffffffd    # Send to controller.
>>  OFPP_LOCAL = 0xfffffffe         # Local openflow "port".
>> -OFPP_ANY = 0xffffffff               # Not associated with a physical port.
>> +OFPP_ANY = 0xffffffff           # Not associated with a physical port.
>> +OFPP_NONE = 0xffffffff          # Not associated with a physical port.
>
>Hi. OF1.1+ doesn't define OFPP_NONE. Only OF1.0 defines OFFP_NONE
>If you need it for compatibility, OFPP_ANY should be used, I suppose.

There are various uses of ofproto.OFPP_NONE in the Ryu codebase, I believe
removing this constant breaks some code paths.  Maybe it should be
retained for backwards compatibility.

For example, in controller.py:

    def send_packet_out(self, buffer_id=0xffffffff, in_port=None,
                        actions=None, data=None):
        if in_port is None:
            in_port = self.ofproto.OFPP_NONE
        packet_out = self.ofproto_parser.OFPPacketOut(
            self, buffer_id, in_port, actions, data)
        self.send_msg(packet_out)


>
>
>>  # All ones is used to indicate all queues in a port (for stats
>>retrieval).
>>  OFPQ_ALL = 0xffffffff
>> diff --git a/ryu/ofproto/ofproto_v1_3_parser.py
>> b/ryu/ofproto/ofproto_v1_3_parser.py
>> index 80a777d..3645d2d 100644
>> --- a/ryu/ofproto/ofproto_v1_3_parser.py
>> +++ b/ryu/ofproto/ofproto_v1_3_parser.py
>> @@ -24,6 +24,7 @@ from . import ofproto_parser
>>  from . import ofproto_v1_3
>>  
>>  import logging
>> +import itertools
>
>itertools isn't used any more.

It is used in the last line of this function (I admit, I didn't look into
what that function does, I only fixed up the missing import):

    def set_ipv6_src_masked(self, src, mask):
        self.wc.ft_set(ofproto_v1_3.OFPXMT_OFB_IPV6_SRC)
        self.wc.ipv6_src_mask = mask
        self.flow.ipv6_src = [x & y for (x, y) in itertools.izip(src,
mask)]


>
>>  LOG = logging.getLogger('ryu.ofproto.ofproto_v1_3_parser')
>>  
>>  _MSG_PARSERS = {}
>> @@ -1832,7 +1833,7 @@ class OFPActionPopMpls(OFPAction):
>>  class OFPActionSetField(OFPAction):
>>      def __init__(self, field):
>>          super(OFPActionSetField, self).__init__()
>> -        set.field = field
>> +        self.field = field
>>  
>>      @classmethod
>>      def parser(cls, buf, offset):
>> @@ -1884,13 +1885,12 @@ class OFPBucket(object):
>>  
>>      @classmethod
>>      def parser(cls, buf, offset):
>> -        (msg.len, msg.weigth, msg.watch_port,
>> -         msg.watch_group) = struct.unpack_from(
>> -             ofproto_v1_3.OFP_BUCKET_PACK_STR, buf, offset)
>> +        (len_, weight, watch_port, watch_group) = struct.unpack_from(
>> +            ofproto_v1_3.OFP_BUCKET_PACK_STR, buf, offset)
>> +        msg = cls(len_, weight, watch_port, watch_group, [])
>>  
>>          length = ofproto_v1_3.OFP_BUCKET_SIZE
>>          offset += ofproto_v1_3.OFP_BUCKET_SIZE
>> -        msg.actions = []
>>          while length < msg.len:
>>              action = OFPAction.parser(buf, offset)
>>              msg.actions.append(action)
>> @@ -2402,7 +2402,7 @@ class
>>OFPGroupFeaturesStatsReply(OFPMultipartReply):
>>  class OFPMeterBandStats(object):
>>      def __init__(self, packet_band_count, byte_band_count):
>>          super(OFPMeterBandStats, self).__init__()
>> -        self.packet_band_count = packet_bound_count
>> +        self.packet_band_count = packet_band_count
>>          self.byte_band_count = byte_band_count
>>  
>>      @classmethod
>> -- 
>> 1.7.9.5
>> 
>> 
>> 
>>-------------------------------------------------------------------------
>>-----
>> Try New Relic Now & We'll Send You this Cool Shirt
>> New Relic is the only SaaS-based application performance monitoring
>>service 
>> that delivers powerful full stack analytics. Optimize and monitor your
>> browser, app, & servers with just a few lines of code. Try New Relic
>> and get this awesome Nerd Life shirt!
>>http://p.sf.net/sfu/newrelic_d2d_may
>> _______________________________________________
>> Ryu-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/ryu-devel
>> 
>
>-- 
>yamahata


------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to