On Thu, Jun 07, 2012 at 11:02:58AM +0900, FUJITA Tomonori wrote:
> On Thu, 7 Jun 2012 09:55:13 +0900
> Simon Horman <[email protected]> wrote:
> 
> > On Thu, Jun 07, 2012 at 08:03:31AM +0900, FUJITA Tomonori wrote:
> >> On Thu, 7 Jun 2012 07:48:34 +0900
> >> Isaku Yamahata <[email protected]> wrote:
> >> 
> >> > I meant ethernet frame parser like dpkt. Some parsers depends on frame 
> >> > size.
> >> 
> >> I see. Here's an update patch.
> >> 
> >> btw, dpkt can't handle vlan header. So it's not so usuful in most of
> >> production environments. We can add the feature to it but looks like
> >> the development activity of dpkt is pretty low. So I'm not sure if it
> >> is the library that we should use. Anyone knows other python ethernet
> >> libraries?
> >> 
> >> 
> >> -
> >> >From b287192116e4838a4871431a69a68db292afc6ff Mon Sep 17 00:00:00 2001
> >> From: FUJITA Tomonori <[email protected]>
> >> Date: Thu, 7 Jun 2012 07:58:21 +0900
> >> Subject: [PATCH] Add Nicira Extension NXT_PACKET_IN support
> >> 
> >> Signed-off-by: FUJITA Tomonori <[email protected]>
> >> ---
> >>  ryu/ofproto/ofproto_v1_0.py        |    6 ++++++
> >>  ryu/ofproto/ofproto_v1_0_parser.py |   34 
> >> ++++++++++++++++++++++++++++++++++
> >>  2 files changed, 40 insertions(+), 0 deletions(-)
> >> 
> >> diff --git a/ryu/ofproto/ofproto_v1_0.py b/ryu/ofproto/ofproto_v1_0.py
> >> index 5c9c205..624bc76 100644
> >> --- a/ryu/ofproto/ofproto_v1_0.py
> >> +++ b/ryu/ofproto/ofproto_v1_0.py
> >> @@ -574,6 +574,7 @@ NXT_FLOW_MOD = 13
> >>  NXT_FLOW_REMOVED = 14
> >>  NXT_FLOW_MOD_TABLE_ID = 15
> >>  NXT_SET_PACKET_IN_FORMAT = 16
> >> +NXT_PACKET_IN = 17
> >>  NXT_SET_CONTROLLER_ID = 20
> >>  
> >>  # enum nx_role
> >> @@ -624,6 +625,11 @@ NX_SET_PACKET_IN_FORMAT_SIZE = 20
> >>  assert (calcsize(NX_SET_PACKET_IN_FORMAT_PACK_STR) +
> >>          NICIRA_HEADER_SIZE == NX_SET_PACKET_IN_FORMAT_SIZE)
> >>  
> >> +NX_PACKET_IN_PACK_STR = '!IHBBQH6x'
> >> +NX_PACKET_IN_SIZE = 40
> >> +assert (calcsize(NX_PACKET_IN_PACK_STR) +
> >> +        NICIRA_HEADER_SIZE == NX_PACKET_IN_SIZE)
> >> +
> >>  NX_CONTROLLER_ID_PACK_STR = '!6xH'
> >>  NX_CONTROLLER_ID_SIZE = 24
> >>  assert (calcsize(NX_CONTROLLER_ID_PACK_STR) +
> >> diff --git a/ryu/ofproto/ofproto_v1_0_parser.py 
> >> b/ryu/ofproto/ofproto_v1_0_parser.py
> >> index 07acce4..f81ebd0 100644
> >> --- a/ryu/ofproto/ofproto_v1_0_parser.py
> >> +++ b/ryu/ofproto/ofproto_v1_0_parser.py
> >> @@ -1368,6 +1368,40 @@ class NXTSetPacketInFormat(NiciraHeader):
> >>                        self.format)
> >>  
> >>  
> >> [email protected]_nx_subtype(ofproto_v1_0.NXT_PACKET_IN)
> >> +class NXTPacketIn(NiciraHeader):
> >> +    def __init__(self, datapath, buffer_id, total_len, reason, table_id,
> >> +                 cookie, match_len, match, frame):
> >> +        super(NXTPacketIn, self).__init__(
> >> +            datapath, ofproto_v1_0.NXT_PACKET_IN)
> >> +        self.buffer_id = buffer_id
> >> +        self.total_len = total_len
> >> +        self.reason = reason
> >> +        self.table_id = table_id
> >> +        self.cookie = cookie
> >> +        self.match_len = match_len
> >> +        self.match = match
> >> +        self.frame = frame
> >> +
> >> +    @classmethod
> >> +    def parser(cls, datapath, buf, offset):
> >> +        (buffer_id, total_len, reason, table_id,
> >> +                 cookie, match_len) = struct.unpack_from(
> >> +            ofproto_v1_0.NX_PACKET_IN_PACK_STR, buf, offset)
> >> +
> >> +        offset += (ofproto_v1_0.NX_PACKET_IN_SIZE
> >> +                   - ofproto_v1_0.NICIRA_HEADER_SIZE)
> >> +
> >> +        match = nx_match.NXMatch.parser(buf, offset, match_len)
> >> +        pad_len = (match_len + 7) / 8 * 8
> >> +        offset += match_len + pad_len
> > 
> > I think that the calculation of pad_len above includes the match_len.
> > I think that either pad_len should be:
> > 
> >     pad_len = (match_len + 7) / 8 * 8 - match_len
> > 
> > Or pad_len could be removed offset could be incremented directly.
> > 
> >     offset += (match_len + 7) / 8 * 8
> 
> Oops, here's an updated one.

Thanks. To be honest I missed the problem on my first two readings of your
patch.

Reviewed-by: Simon Horman <[email protected]>

> diff --git a/ryu/ofproto/ofproto_v1_0.py b/ryu/ofproto/ofproto_v1_0.py
> index 5c9c205..624bc76 100644
> --- a/ryu/ofproto/ofproto_v1_0.py
> +++ b/ryu/ofproto/ofproto_v1_0.py
> @@ -574,6 +574,7 @@ NXT_FLOW_MOD = 13
>  NXT_FLOW_REMOVED = 14
>  NXT_FLOW_MOD_TABLE_ID = 15
>  NXT_SET_PACKET_IN_FORMAT = 16
> +NXT_PACKET_IN = 17
>  NXT_SET_CONTROLLER_ID = 20
>  
>  # enum nx_role
> @@ -624,6 +625,11 @@ NX_SET_PACKET_IN_FORMAT_SIZE = 20
>  assert (calcsize(NX_SET_PACKET_IN_FORMAT_PACK_STR) +
>          NICIRA_HEADER_SIZE == NX_SET_PACKET_IN_FORMAT_SIZE)
>  
> +NX_PACKET_IN_PACK_STR = '!IHBBQH6x'
> +NX_PACKET_IN_SIZE = 40
> +assert (calcsize(NX_PACKET_IN_PACK_STR) +
> +        NICIRA_HEADER_SIZE == NX_PACKET_IN_SIZE)
> +
>  NX_CONTROLLER_ID_PACK_STR = '!6xH'
>  NX_CONTROLLER_ID_SIZE = 24
>  assert (calcsize(NX_CONTROLLER_ID_PACK_STR) +
> diff --git a/ryu/ofproto/ofproto_v1_0_parser.py 
> b/ryu/ofproto/ofproto_v1_0_parser.py
> index 07acce4..f16fff9 100644
> --- a/ryu/ofproto/ofproto_v1_0_parser.py
> +++ b/ryu/ofproto/ofproto_v1_0_parser.py
> @@ -1368,6 +1368,39 @@ class NXTSetPacketInFormat(NiciraHeader):
>                        self.format)
>  
>  
> [email protected]_nx_subtype(ofproto_v1_0.NXT_PACKET_IN)
> +class NXTPacketIn(NiciraHeader):
> +    def __init__(self, datapath, buffer_id, total_len, reason, table_id,
> +                 cookie, match_len, match, frame):
> +        super(NXTPacketIn, self).__init__(
> +            datapath, ofproto_v1_0.NXT_PACKET_IN)
> +        self.buffer_id = buffer_id
> +        self.total_len = total_len
> +        self.reason = reason
> +        self.table_id = table_id
> +        self.cookie = cookie
> +        self.match_len = match_len
> +        self.match = match
> +        self.frame = frame
> +
> +    @classmethod
> +    def parser(cls, datapath, buf, offset):
> +        (buffer_id, total_len, reason, table_id,
> +                 cookie, match_len) = struct.unpack_from(
> +            ofproto_v1_0.NX_PACKET_IN_PACK_STR, buf, offset)
> +
> +        offset += (ofproto_v1_0.NX_PACKET_IN_SIZE
> +                   - ofproto_v1_0.NICIRA_HEADER_SIZE)
> +
> +        match = nx_match.NXMatch.parser(buf, offset, match_len)
> +        offset += (match_len + 7) / 8 * 8
> +        frame = buf[offset:]
> +        if total_len < len(frame):
> +            frame = frame[:total_len]
> +        return cls(datapath, buffer_id, total_len, reason, table_id,
> +                   cookie, match_len, match, frame)
> +
> +
>  class NXTSetControllerId(NiciraHeader):
>      def __init__(self, datapath, controller_id):
>          super(NXTSetControllerId, self).__init__(
> 

------------------------------------------------------------------------------
Live Security Virtual Conference
Exclusive live event will cover all the ways today's security and 
threat landscape has changed and how IT managers can respond. Discussions 
will include endpoint security, mobile security and the latest in malware 
threats. http://www.accelacomm.com/jaw/sfrnl04242012/114/50122263/
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to