You may be right. I'll check it soon. Thanks for your comments. Olivier
Le mar. 22 nov. 2016 à 06:50, Iwase Yusuke <[email protected]> a écrit : > Hi Olivier, > > Thank you for submitting your patch! > > First, the patch seems to be wrapped unexpectedly when line exceeds > 80(?) characters or so. > Please confirm you mailer settings. > https://git-scm.com/docs/git-format-patch#_mua_specific_hints > > And, the following is some modest comments I found. > > > On 2016年11月19日 05:07, Olivier Desnoë wrote: > > This patch adds the ability to dissect and generate DHCPv6 packets. I > > needed it to develop a simple LDRA app (as defined in RFC 6221). > > > > Signed-off-by: Olivier DESNOE <[email protected]> > > --- > > diff --git a/ryu/lib/packet/dhcp6.py b/ryu/lib/packet/dhcp6.py > > new file mode 100644 > > index 0000000..1102305 > > --- /dev/null > > +++ b/ryu/lib/packet/dhcp6.py > > @@ -0,0 +1,301 @@ > > +# Copyright (C) 2016 Bouygues Telecom. > > +# > > +# 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. > > + > > +""" > > +DHCPv6 packet parser/serializer > > + > > +RFC 3315 > > +DHCP packet format > > + > > + The following diagram illustrates the format of DHCP messages sent > > + between clients and servers: > > + > > + 0 1 2 3 > > + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + | msg_type | transaction_id | > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + | | > > + . options . > > + . (variable) . > > + | | > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + > > + There are two relay agent messages, which share the following format: > > + > > + 0 1 2 3 > > + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + | msg_type | hop_count | | > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | > > + | | > > + | link_address | > > + | | > > + | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-| > > + | | | > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | > > + | | > > + | peer_address | > > + | | > > + | +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-| > > + | | | > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ | > > + . . > > + . options (variable number and length) .... . > > + | | > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + > > +""" > > +import binascii > > binascii is imported, but does not seem to be used. > > > > +import random > > +import struct > > + > > +from . import packet_base > > +from ryu.lib import addrconv > > +from ryu.lib import stringify > > + > > +# DHCPv6 message types > > +DHCPV6_SOLICIT = 1 > > +DHCPV6_ADVERTISE = 2 > > +DHCPV6_REQUEST = 3 > > +DHCPV6_CONFIRM = 4 > > +DHCPV6_RENEW = 5 > > +DHCPV6_REBIND = 6 > > +DHCPV6_REPLY = 7 > > +DHCPV6_RELEASE = 8 > > +DHCPV6_DECLINE = 9 > > +DHCPV6_RECONFIGURE = 10 > > +DHCPV6_INFORMATION_REQUEST = 11 > > +DHCPV6_RELAY_FORW = 12 > > +DHCPV6_RELAY_REPL = 13 > > + > > +# DHCPv6 option-codes > > +DHCPV6_OPTION_CLIENTID = 1 > > +DHCPV6_OPTION_SERVERID = 2 > > +DHCPV6_OPTION_IA_NA = 3 > > +DHCPV6_OPTION_IA_TA = 4 > > +DHCPV6_OPTION_IAADDR = 5 > > +DHCPV6_OPTION_ORO = 6 > > +DHCPV6_OPTION_PREFERENCE = 7 > > +DHCPV6_OPTION_ELAPSED_TIME = 8 > > +DHCPV6_OPTION_RELAY_MSG = 9 > > +DHCPV6_OPTION_AUTH = 11 > > +DHCPV6_OPTION_UNICAST = 12 > > +DHCPV6_OPTION_STATUS_CODE = 13 > > +DHCPV6_OPTION_RAPID_COMMIT = 14 > > +DHCPV6_OPTION_USER_CLASS = 15 > > +DHCPV6_OPTION_VENDOR_CLASS = 16 > > +DHCPV6_OPTION_VENDOR_OPTS = 17 > > +DHCPV6_OPTION_INTERFACE_ID = 18 > > +DHCPV6_OPTION_RECONF_MSG = 19 > > +DHCPV6_OPTION_RECONF_ACCEPT = 20 > > + > > + > > +class dhcp6(packet_base.PacketBase): > > + """DHCPv6 (RFC 3315) header encoder/decoder class. > > + > > + The serialized packet would looks like the ones described > > + in the following sections. > > + > > + * RFC 3315 DHCP packet format > > + > > + An instance has the following attributes at least. > > + Most of them are same to the on-wire counterparts but in host byte > > order. > > + __init__ takes the corresponding args in this order. > > + > > + > > + ============== ==================== > > + Attribute Description > > + ============== ==================== > > + msg_type Identifies the DHCP message type > > + > > + * For unrelayed messages only: > > + transaction_id The transaction ID for this message exchange. > > + > > + * For relayed messages only > > + hop_count Number of relay agents that have relayed this > > + message. > > + link_address A global or site-local address that will be used by > > + the server to identify the link on which the client > > + is located. > > + peer_address The address of the client or relay agent from which > > + the message to be relayed was received. > > + > > + * For unrelayed and relayed messages: > > + options Options carried in this message > > + ============== ==================== > > + """ > > + _MIN_LEN = 8 > > + _DHCPV6_UNPACK_STR = '!I' > > + _DHCPV6_RELAY_UNPACK_STR = '!H16s16s' > > + _DHCPV6_UNPACK_STR_LEN = struct.calcsize(_DHCPV6_UNPACK_STR) > > + _DHCPV6_RELAY_UNPACK_STR_LEN = > > struct.calcsize(_DHCPV6_RELAY_UNPACK_STR) > > + _DHCPV6_PACK_STR = '!I' > > + _DHCPV6_RELAY_PACK_STR = '!H16s16s' > > + > > + def __init__(self, msg_type, options, transaction_id=None, > hop_count=0, > > + link_address='::', peer_address='::'): > > + super(dhcp6, self).__init__() > > + self.msg_type = msg_type > > + self.options = options > > + if transaction_id is None: > > + self.transaction_id = random.randint(0, 0xffffff) > > + else: > > + self.transaction_id = transaction_id > > + self.hop_count = hop_count > > + self.link_address = link_address > > + self.peer_address = peer_address > > + > > + @classmethod > > + def _parser(cls, buf): > > + (msg_type, ) = struct.unpack_from('!B', buf) > > + > > + buf = '\x00' + buf[1:] # unpack xid as a 4-byte integer > > + if msg_type == DHCPV6_RELAY_FORW or msg_type == > DHCPV6_RELAY_REPL: > > + (hop_count, link_address, peer_address) \ > > + = struct.unpack_from(cls._DHCPV6_RELAY_UNPACK_STR, buf) > > + length = struct.calcsize(cls._DHCPV6_RELAY_UNPACK_STR) > > + else: > > + (transaction_id, ) \ > > + = struct.unpack_from(cls._DHCPV6_UNPACK_STR, buf) > > + length = struct.calcsize(cls._DHCPV6_UNPACK_STR) > > + > > + if len(buf) > length: > > + parse_opt = options.parser(buf[length:]) > > + length += parse_opt.options_len > > + > > + if msg_type == DHCPV6_RELAY_FORW or msg_type == > DHCPV6_RELAY_REPL: > > + return (cls(msg_type, parse_opt, 0, hop_count, > > + addrconv.ipv6.bin_to_text(link_address), > > + addrconv.ipv6.bin_to_text(peer_address)), > > + None, buf[length:]) > > + else: > > + return (cls(msg_type, parse_opt, transaction_id), > > + None, buf[length:]) > > There seems to be the case that the local variables 'parse_opt', > 'hop_count', > 'link_address', 'peer_address' and 'transaction_id' might be referenced > before assignment. > e.g.) When NOT len(buf) > length, 'parse_opt' will not be initialized. > > Is it the rare case? > > > > + > > + @classmethod > > + def parser(cls, buf): > > + try: > > + return cls._parser(buf) > > + except: > > + return None, None, buf > > This except statement is too broad for me. > How about 'except struct.error'? > > > > + > > + def serialize(self, payload=None, prev=None): > > + seri_opt = self.options.serialize() > > + if (self.msg_type == DHCPV6_RELAY_FORW or > > + self.msg_type == DHCPV6_RELAY_REPL): > > + pack_str = '%s%ds' % (self._DHCPV6_RELAY_PACK_STR, > > + self.options.options_len) > > + str = struct.pack(pack_str, self.hop_count, > > The value name 'str' seems to shadow built-in type. > How about using 'buf' for it? > > > > + > addrconv.ipv6.text_to_bin(self.link_address), > > + > addrconv.ipv6.text_to_bin(self.peer_address), > > + seri_opt) > > + else: > > + pack_str = '%s%ds' % (self._DHCPV6_PACK_STR, > > + self.options.options_len) > > + str = struct.pack(pack_str, self.transaction_id, seri_opt) > > + str = struct.pack('!B', self.msg_type) + str[1:] > > + return str > > + > > + > > +class options(stringify.StringifyMixin): > > + """DHCP (RFC 3315) options encoder/decoder class. > > + > > + This is used with ryu.lib.packet.dhcp6.dhcp6. > > + > > + """ > > + > > + def __init__(self, option_list=None, options_len=0): > > + super(options, self).__init__() > > + if option_list is None: > > + self.option_list = [] > > + else: > > + self.option_list = option_list > > + self.options_len = options_len > > + > > + @classmethod > > + def parser(cls, buf): > > + opt_parse_list = [] > > + offset = 0 > > + while len(buf) > offset: > > + opt_buf = buf[offset:] > > + opt = option.parser(opt_buf) > > + opt_parse_list.append(opt) > > + offset += opt.length + 4 > > + return cls(opt_parse_list, len(buf)) > > + > > + def serialize(self): > > + seri_opt = "" > > + for opt in self.option_list: > > + seri_opt += opt.serialize() > > + if self.options_len == 0: > > + self.options_len = len(seri_opt) > > + return seri_opt > > + > > + > > +class option(stringify.StringifyMixin): > > + """DHCP (RFC 3315) options encoder/decoder class. > > + > > + This is used with ryu.lib.packet.dhcp6.dhcp6.options. > > + > > + An instance has the following attributes at least. > > + Most of them are same to the on-wire counterparts but in host byte > > order. > > + __init__ takes the corresponding args in this order. > > + > > + The format of DHCP options is: > > + > > + 0 1 2 3 > > + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + | option-code | option-len | > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + | option-data | > > + | (option-len octets) | > > + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > + > > + option-code An unsigned integer identifying the specific option > > + type carried in this option. > > + > > + option-len An unsigned integer giving the length of the > > + option-data field in this option in octets. > > + > > + option-data The data for the option; the format of this data > > + depends on the definition of the option. > > + """ > > + _UNPACK_STR = '!H' > > + _UNPACK_STR_LEN = struct.calcsize(_UNPACK_STR) > > + _PACK_STR = '!HH%ds' > > + > > + def __init__(self, code, data, length=0): > > + super(option, self).__init__() > > + self.code = code > > + self.data = data > > + self.length = length > > + > > + @classmethod > > + def parser(cls, buf): > > + code = struct.unpack_from(cls._UNPACK_STR, buf)[0] > > + buf = buf[cls._UNPACK_STR_LEN:] > > + length = struct.unpack_from(cls._UNPACK_STR, buf)[0] > > + buf = buf[cls._UNPACK_STR_LEN:] > > + value_unpack_str = '%ds' % length > > + data = struct.unpack_from(value_unpack_str, buf)[0] > > + return cls(code, data, length) > > + > > + def serialize(self): > > + if self.length == 0: > > + self.length = len(self.data) > > + options_pack_str = self._PACK_STR % self.length > > + return struct.pack(options_pack_str, self.code, self.length, > > self.data) > > diff --git a/ryu/lib/packet/udp.py b/ryu/lib/packet/udp.py > > index f39fb90..064263c 100644 > > --- a/ryu/lib/packet/udp.py > > +++ b/ryu/lib/packet/udp.py > > @@ -18,6 +18,7 @@ import struct > > from . import packet_base > > from . import packet_utils > > from . import dhcp > > +from . import dhcp6 > > from . import vxlan > > > > > > @@ -53,10 +54,12 @@ class udp(packet_base.PacketBase): > > @staticmethod > > def get_packet_type(src_port, dst_port): > > if ((src_port == 68 and dst_port == 67) or > > - (src_port == 67 and dst_port == 68) or > > - (src_port == 67 and > > - dst_port == 67)): > > + (src_port == 67 and dst_port == 68) or > > + (src_port == 67 and dst_port == 67)): > > return dhcp.dhcp > > + if (((src_port == 546 or src_port == 547) and dst_port == 547) > or > > + (src_port == 547 and (dst_port == 546 or dst_port == > > 547))): > > + return dhcp6.dhcp6 > > if (dst_port == vxlan.UDP_DST_PORT or > > dst_port == vxlan.UDP_DST_PORT_OLD): > > return vxlan.vxlan > > > > > > > > > ------------------------------------------------------------------------------ > > > > > > > > _______________________________________________ > > Ryu-devel mailing list > > [email protected] > > https://lists.sourceforge.net/lists/listinfo/ryu-devel > > >
------------------------------------------------------------------------------
_______________________________________________ Ryu-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/ryu-devel
