packet: Fix minimum ethernet frame length

In the Ethernet frame Spec (Both DIX EthernetII and IEEE 802.3),
the frame size must be at least 64 bytes long (not including the preamble).

  64 bytes = 14 bytes (Header) + 46 bytes (Payload) + 4 bytes (FCS)

This patch appends padding if the payload is less than 46 bytes long.

Reported-by: Kawai, Hiroaki <kawai@iij.ad.jp>
Signed-off-by: IWASE Yusuke <iwase.yusuke0@gmail.com>
Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>

4 files changed, 39 insertions, 6 deletions

diff --git a/ryu/lib/packet/ethernet.py b/ryu/lib/packet/ethernet.py
index 33ef9c6c..dd3a65c1 100644
--- a/ryu/lib/packet/ethernet.py
+++ b/ryu/lib/packet/ethernet.py
@@ -19,6 +19,7 @@ from . import vlan
 from . import mpls
 from . import ether_types as ether
 from ryu.lib import addrconv
+from ryu.lib.pack_utils import msg_pack_into
 
 
 class ethernet(packet_base.PacketBase):
@@ -39,6 +40,7 @@ class ethernet(packet_base.PacketBase):
 
     _PACK_STR = '!6s6sH'
     _MIN_LEN = struct.calcsize(_PACK_STR)
+    _MIN_PAYLOAD_LEN = 46
     _TYPE = {
         'ascii': [
             'src', 'dst'
@@ -61,6 +63,11 @@ class ethernet(packet_base.PacketBase):
                 buf[ethernet._MIN_LEN:])
 
     def serialize(self, payload, prev):
+        # Append padding if the payload is less than 46 bytes long
+        pad_len = self._MIN_PAYLOAD_LEN - len(payload)
+        if pad_len > 0:
+            payload.extend(b'\x00' * pad_len)
+
         return struct.pack(ethernet._PACK_STR,
                            addrconv.mac.text_to_bin(self.dst),
                            addrconv.mac.text_to_bin(self.src),
diff --git a/ryu/lib/packet/packet.py b/ryu/lib/packet/packet.py
index 6f420cb6..85e826fc 100644
--- a/ryu/lib/packet/packet.py
+++ b/ryu/lib/packet/packet.py
@@ -54,7 +54,8 @@ class Packet(object):
                 break
             if proto:
                 self.protocols.append(proto)
-        if rest_data:
+        # If rest_data is all padding, we ignore rest_data
+        if rest_data and six.binary_type(rest_data).strip(b'\x00'):
             self.protocols.append(rest_data)
 
     def serialize(self):
@@ -74,7 +75,7 @@ class Packet(object):
                 data = p.serialize(self.data, prev)
             else:
                 data = six.binary_type(p)
-            self.data = data + self.data
+            self.data = bytearray(data + self.data)
 
     def add_protocol(self, proto):
         """Register a protocol *proto* for this packet.
diff --git a/ryu/tests/unit/packet/test_lldp.py b/ryu/tests/unit/packet/test_lldp.py
index 481ac09e..d8d261c2 100644
--- a/ryu/tests/unit/packet/test_lldp.py
+++ b/ryu/tests/unit/packet/test_lldp.py
@@ -121,7 +121,16 @@ class TestLLDPMandatoryTLV(unittest.TestCase):
         eq_(len(pkt.protocols), 2)
 
         pkt.serialize()
-        eq_(pkt.data, self.data)
+
+        # Note: If ethernet frame is less than 60 bytes length,
+        # ethernet.ethernet() appends padding to the payload.
+        # So, we splits the serialized data to compare.
+        data_len = len(self.data)
+        pkt_data_lldp = pkt.data[:data_len]
+        pkt_data_pad = pkt.data[data_len:]
+        eq_(b'\x00' * (60 - data_len), pkt_data_pad)
+
+        eq_(self.data, pkt_data_lldp)
 
     def test_to_string(self):
         chassis_id = lldp.ChassisID(subtype=lldp.ChassisID.SUB_MAC_ADDRESS,
diff --git a/ryu/tests/unit/packet/test_packet.py b/ryu/tests/unit/packet/test_packet.py
index c48e3727..b822976e 100644
--- a/ryu/tests/unit/packet/test_packet.py
+++ b/ryu/tests/unit/packet/test_packet.py
@@ -93,6 +93,11 @@ class TestPacket(unittest.TestCase):
             + self.dst_ip_bin
 
         buf = e_buf + a_buf
+
+        # Append padding if ethernet frame is less than 60 bytes length
+        pad_len = 60 - len(buf)
+        if pad_len > 0:
+            buf += b'\x00' * pad_len
         eq_(buf, p.data)
 
         # parse
@@ -188,6 +193,11 @@ class TestPacket(unittest.TestCase):
             + self.dst_ip_bin
 
         buf = e_buf + v_buf + a_buf
+
+        # Append padding if ethernet frame is less than 60 bytes length
+        pad_len = 60 - len(buf)
+        if pad_len > 0:
+            buf += b'\x00' * pad_len
         eq_(buf, p.data)
 
         # parse
@@ -1407,9 +1417,9 @@ class TestPacket(unittest.TestCase):
                  b'\x00'
                  b'\x00'
                  b'\x00'
-                 b'\x80\x64\xaa\xaa\xaa\xaa\xaa\xaa'
-                 b'\x00\x00\x00\x04'
-                 b'\x80\x64\xbb\xbb\xbb\xbb\xbb\xbb'
+                 b'\x80\x00\xbb\xbb\xbb\xbb\xbb\xbb'
+                 b'\x00\x00\x00\x00'
+                 b'\x80\x00\xaa\xaa\xaa\xaa\xaa\xaa'
                  b'\x80\x04'
                  b'\x01\x00'
                  b'\x14\x00'
@@ -1418,6 +1428,12 @@ class TestPacket(unittest.TestCase):
 
         buf = e_buf + l_buf + b_buf
 
+        # Append padding if ethernet frame is less than 60 bytes length
+        pad_len = 60 - len(buf)
+        if pad_len > 0:
+            buf += b'\x00' * pad_len
+        eq_(buf, p.data)
+
         # parse
         pkt = packet.Packet(array.array('B', p.data))
         protocols = self.get_protocols(pkt)