On Tue, 21 Aug 2012 13:00:36 +0900 Isaku Yamahata <[email protected]> wrote:
> Basically looks good. Some nitpicks below. > > pack_str is class constant and private, so it should be capital, _PACK_STR. > some comments inlined. It's python style rule? > On Tue, Aug 21, 2012 at 10:00:40AM +0900, FUJITA Tomonori wrote: >> diff --git a/ryu/lib/packet/ipv4.py b/ryu/lib/packet/ipv4.py >> new file mode 100644 >> index 0000000..1bf726c >> --- /dev/null >> +++ b/ryu/lib/packet/ipv4.py >> @@ -0,0 +1,80 @@ >> +# Copyright (C) 2012 Nippon Telegraph and Telephone Corporation. >> +# >> +# Licensed under the Apache License, Version 2.0 (the "License"); >> +# you may not use this file except in compliance with the License. >> +# You may obtain a copy of the License at >> +# >> +# http://www.apache.org/licenses/LICENSE-2.0 >> +# >> +# Unless required by applicable law or agreed to in writing, software >> +# distributed under the License is distributed on an "AS IS" BASIS, >> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> +# implied. >> +# See the License for the specific language governing permissions and >> +# limitations under the License. >> + >> +import struct >> +from . import packet_base >> +from . import udp >> +from ryu.ofproto.ofproto_parser import msg_pack_into >> +from ryu.ofproto import inet >> + >> +class ipv4(packet_base.PacketBase): >> + pack_str = '!BBHHHBBHII' >> + >> + def __init__(self, version, header_length, tos, total_length, >> + identification, flags, offset, ttl, proto, csum, >> + src, dst): >> + super(ipv4, self).__init__() >> + self.version = version >> + self.header_length = header_length >> + self.tos = tos >> + self.total_length = total_length >> + self.identification = identification >> + self.flags = flags >> + self.offset = offset >> + self.ttl = ttl >> + self.proto = proto >> + self.csum = csum >> + self.src = src >> + self.dst = dst >> + self.length = header_length * 4 >> + >> + @classmethod >> + def parser(cls, buf): >> + (version, tos, total_length, identification, flags, ttl, proto, >> csum, >> + src, dst) = struct.unpack_from(cls.pack_str, buf) >> + header_length = version & 0xf >> + version = version >> 4 >> + offset = flags & ((1 << 15) - 1) >> + flags = flags >> 15 >> + msg = cls(version, header_length, tos, total_length, identification, >> + flags, offset, ttl, proto, csum, src, dst) >> + >> + if msg.length > struct.calcsize(ipv4.pack_str): >> + self.extra = buf[struct.calcsize(ipv4.pack_str):msg.length] >> + >> + return msg, ipv4.get_packet_type(proto) >> + >> + @staticmethod >> + def carry_around_add(a, b): >> + c = a + b >> + return (c & 0xffff) + (c >> 16) >> + >> + def checksum(self, data): >> + s = 0 >> + for i in range(0, len(data), 2): >> + w = data[i] + (data[i+1] << 8) >> + s = self.carry_around_add(s, w) >> + return ~s & 0xffff > > 1-complement check sum should be split out? Maybe we'll add ipv6 support. Yeah, it can be used for others. But let's do it when we actually add such. >> + >> + def serialize(self, buf, offset): >> + version = self.version << 4 | self.header_length >> + flags = self.flags << 15 | self.offset >> + msg_pack_into(ipv4.pack_str, buf, offset, version, self.tos, >> + self.total_length, self.identification, flags, >> + self.ttl, self.proto, 0, self.src, self.dst) >> + self.csum = self.checksum(buf[offset:offset+self.length]) >> + msg_pack_into('H', buf, offset + 10, self.csum) >> + >> +ipv4.register_packet_type(udp.udp, inet.IPPROTO_UDP) >> diff --git a/ryu/lib/packet/packet.py b/ryu/lib/packet/packet.py >> new file mode 100644 >> index 0000000..baa9be3 >> --- /dev/null >> +++ b/ryu/lib/packet/packet.py >> @@ -0,0 +1,49 @@ >> +# Copyright (C) 2012 Nippon Telegraph and Telephone Corporation. >> +# >> +# Licensed under the Apache License, Version 2.0 (the "License"); >> +# you may not use this file except in compliance with the License. >> +# You may obtain a copy of the License at >> +# >> +# http://www.apache.org/licenses/LICENSE-2.0 >> +# >> +# Unless required by applicable law or agreed to in writing, software >> +# distributed under the License is distributed on an "AS IS" BASIS, >> +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or >> +# implied. >> +# See the License for the specific language governing permissions and >> +# limitations under the License. >> + >> +from . import ethernet >> + >> + >> +class Packet(object): > > class Packet is unused yet? (I'm just confirming it.) It's used. You need it to parse a packet: pkt = packet.Packet(array.array('B', msg.data)) v = pkt.find_protocol('vlan') You also need it to build a packet: e = ethernet.ethernet(dst, src, ethertype) p = packet.Packet() p.add_protocol(e) >> + def __init__(self, data=None): >> + super(Packet, self).__init__() >> + self.data = data >> + self.protocols = [] >> + self.parsed_bytes = 0 >> + if self.data: >> + # Do we need to handle non ethernet? >> + self.parser(ethernet.ethernet) >> + >> + def parser(self, cls): >> + while cls: >> + proto, cls = cls.parser(self.data[self.parsed_bytes:]) >> + if proto: >> + self.parsed_bytes += proto.length >> + self.protocols.append(proto) >> + >> + def serialize(self): >> + offset = 0 >> + self.data = bytearray().zfill(0) > > I suppose that this makes no effect. So this can be eliminated. I see. ------------------------------------------------------------------------------ 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
