Hi, thanks for the report!
On Fri, 28 Aug 2015 16:52:41 -0400
A Sydney <[email protected]> wrote:
> I believe I have found a bug in the parser and have provided a possible fix
> below. In addition, I believe there may be a second bug. I would
> appreciate your feedback on both.
>
> The text at the bottom of the email highlights the steps I took which
> generate the first bug and the possible fix.
>
> Questions:
>
> a) Are the steps in 3 below correct? If not, can you please provide
> feedback?
>
> b) Is the output of 4 below what one should expect?
>
> c) Second bug: In the following snippet (from ryu/lib/packet/dhcp.py), it
> appears that "parse_opt" is only set if len(buf) is greater than min_len
> (since there are no other instances of parse_opt in this file). In the
> event that len(buf) =< min_len, would this not result in an error at the
> "return" statement?
>
> ...
> if len(buf) > min_len:
> parse_opt = options.parser(buf[min_len:])
> length += parse_opt.options_len
>
> return (cls(op, addrconv.mac.bin_to_text(chaddr), parse_opt,
> htype, hlen, hops, xid, secs, flags,
> addrconv.ipv4.bin_to_text(ciaddr),
> addrconv.ipv4.bin_to_text(yiaddr),
> addrconv.ipv4.bin_to_text(siaddr),
> addrconv.ipv4.bin_to_text(giaddr),
> sname.decode('ascii'), boot_file),
> None, buf[length:])
> ...
Looks like the code works as you said. That's a bug? At that point,
the parser consumes 236 bytes. If there is still the buffer to parse,
the parse does, right?
> Thank you in advance for your assistance,
> Ali
>
> ==== FIRST BUG REPORT AND PROPOSED FIX ===
>
> 1. Action:
>
> Since the switch truncates pkts to the controller, I use the following to
> have the switch send the entire packet (I've assumed a max pkt size of 1500
> bytes).
> ...
> req = parser.OFPSetConfig(datapath, ofproto_v1_3.OFPC_FRAG_NORMAL, 1500)
> datapath.send_msg(req)
> ...
>
> I also use the following to grab the dhcp header:
>
> ...
> dhcp_pkt = packet.Packet(pkt.protocols[3], parse_cls=dhcp.dhcp)
> ...
>
> 2. Result:
>
> The following error results:
>
> ...
> dhcp_pkt = packet.Packet(pkt.protocols[3], parse_cls=dhcp.dhcp)
> File "/usr/local/lib/python2.7/site-packages/ryu/lib/packet/packet.py",
> line 46, in __init__
> self._parser(parse_cls)
> File "/usr/local/lib/python2.7/site-packages/ryu/lib/packet/packet.py",
> line 55, in _parser
> if proto:
> File
> "/usr/local/lib/python2.7/site-packages/ryu/lib/packet/packet_base.py",
> line 46, in __len__
> return self._MIN_LEN
> AttributeError: 'dhcp' object has no attribute '_MIN_LEN'
> ...
>
> 3. Action:
>
> I added the following two lines to the start of the dhcp class in the
> dhcp.py file around line 135 (PS. These lines also exist in the option
> class):
>
> _UNPACK_STR = '!B'
> _MIN_LEN = struct.calcsize(_UNPACK_STR)
> # existing code is below
> _HLEN_UNPACK_STR = '!BBB'
> _HLEN_UNPACK_LEN = struct.calcsize(_HLEN_UNPACK_STR)
I think that the dhcp parser expects at least 236 bytes so _MIN_LEN
should be 236. If _MIN_LEN is 1, then the parse could get 1 byte
buffer (and then the parser crashes).
btw, I think that the udp parse should properly handle dhcp. With the
following patch, you can just do:
pkt = packet.Packet(buf)
pkt should includes ethernet, ip, udp, and dhcp headers.
diff --git a/ryu/lib/packet/udp.py b/ryu/lib/packet/udp.py
index a80e101..bae6d73 100644
--- a/ryu/lib/packet/udp.py
+++ b/ryu/lib/packet/udp.py
@@ -17,6 +17,7 @@ import struct
from . import packet_base
from . import packet_utils
+from . import dhcp
class udp(packet_base.PacketBase):
@@ -49,11 +50,17 @@ class udp(packet_base.PacketBase):
self.csum = csum
@classmethod
+ def get_packet_type(cls, src_port, dst_port):
+ if (src_port == 68 and dst_port == 67) or (src_port == 67 and dst_port
== 68):
+ return dhcp.dhcp
+ return None
+
+ @classmethod
def parser(cls, buf):
(src_port, dst_port, total_length, csum) = struct.unpack_from(
cls._PACK_STR, buf)
msg = cls(src_port, dst_port, total_length, csum)
- return msg, None, buf[msg._MIN_LEN:total_length]
+ return msg, cls.get_packet_type(src_port, dst_port),
buf[msg._MIN_LEN:total_length]
def serialize(self, payload, prev):
if self.total_length == 0:
------------------------------------------------------------------------------
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel
