Hi Kawai-San,

Thank you for testing my patch!

On 2016年07月04日 11:47, Hiroaki KAWAI wrote:
> Hi Iwase-san, Fujita-san,
>
> I've tested with the patch and looks OK for
> my environment.
>
> The patch affects the entire test suites, and the all
> of tests must be run again.
> https://osrg.github.io/ryu/certification.html
>
> And just FYI, in the openflow 1.3.5 spec,
> 7.3.7 says "The packet format is an Ethernet frame,
> including an Ethernet header and Ethernet payload
> (but no Ethernet FCS)".
> 7.4.1 says "The field total_len counts the number
> of bytes in the Ethernet frame, including an Ethernet
> header and Ethernet payload (but not the Ethernet FCS)".
>
> So the minimum length in openflow data payload is
> 60 bytes length.
>
> # I was afraid of that the openflow spec might be
> # ambiguous about the ethernet frame length.
> ## So my patch was testing 64bytes length.
>
> But now the spec seems clear on this point,
> Iwase-san's patch is nice.

Yes, with my patch, lib/packet/ethernet.py will construct
an Ethernet frame in 60 bytes length (without FCS).

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

If this patch looks okay, I will update my patch,
because this patch does not fix unit tests, so causes some errors...

Thanks,
Iwase

>
>
> On 2016/07/04 9:41, Iwase Yusuke wrote:
>> Hi Kawai-San and Fujita-San,
>>
>>
>> On 2016年07月02日 12:50, FUJITA Tomonori wrote:
>>> CC'ed Iwase-san,
>>>
>>> On Fri, 1 Jul 2016 20:15:10 +0900
>>> Hiroaki KAWAI <[email protected]> wrote:
>>>
>>>> On 2016/07/01 11:36, FUJITA Tomonori wrote:
>>>>> On Wed, 29 Jun 2016 00:26:42 -0700
>>>>> "Kawai, Hiroaki" <[email protected]> wrote:
>>>>>
>>>>>> Some system will use padded paackets for ethernet frame that is at
>>>>>> least 64 bytes long.
>>>>>
>>>>> This patch means that the tester sends an ethernet frame that is less
>>>>> than 64B?
>>>>
>>>> I tested again and confirmed that the tester was
>>>> sending 60B frame with padding.
>>>>
>>>> The problem was that len(model_pkt) == 54 in
>>>> ryu/tests/switch/of13/match/19_ICMPV4_TYPE.json
>>>> for example, and the test is failing.
>>>> # Without padding, the two packets were identical.
>>>
>>> Thanks for the confirmation. I have no strong opinion on this but
>>> better to fix ryu/tests/switch/of13/match/19_ICMPV4_TYPE.json?
>>
>> I'm agree with Fujita-San, to avoid this problem in tester.py may not be 
>> suitable.
>>
>> 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).
>> So, the cause of this problem is that the Ethernet frame data,
>> which constructed by Ryu's Ethernet packet library, is less than 64 bytes 
>> long.
>>
>> The followings can fix this problem?
>>
>> $ git diff
>> diff --git a/ryu/lib/packet/ethernet.py b/ryu/lib/packet/ethernet.py
>> index 33ef9c6..efc777e 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):
>> @@ -45,6 +46,8 @@ class ethernet(packet_base.PacketBase):
>>            ]
>>        }
>>
>> +    _MIN_PAYLOAD_SIZE = 46
>> +
>>        def __init__(self, dst='ff:ff:ff:ff:ff:ff', src='00:00:00:00:00:00',
>>                     ethertype=ether.ETH_TYPE_IP):
>>            super(ethernet, self).__init__()
>> @@ -61,6 +64,12 @@ class ethernet(packet_base.PacketBase):
>>                    buf[ethernet._MIN_LEN:])
>>
>>        def serialize(self, payload, prev):
>> +        # Append padding if the payload is shorter than 46 bytes.
>> +        payload_len = len(payload)
>> +        pad_len = self._MIN_PAYLOAD_SIZE - payload_len
>> +        if pad_len > 0:
>> +            msg_pack_into('!%dx' % pad_len, payload, payload_len)
>> +
>>            return struct.pack(ethernet._PACK_STR,
>>                               addrconv.mac.text_to_bin(self.dst),
>>                               addrconv.mac.text_to_bin(self.src)
>>
>>
>> Thanks,
>> Iwase
>>
>> ------------------------------------------------------------------------------
>> Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
>> Francisco, CA to explore cutting-edge tech and listen to tech luminaries
>> present their vision of the future. This family event has something for
>> everyone, including kids. Get more information and register today.
>> http://sdm.link/attshape
>> _______________________________________________
>> Ryu-devel mailing list
>> [email protected]
>> https://lists.sourceforge.net/lists/listinfo/ryu-devel
>>
>
>
> ------------------------------------------------------------------------------
> Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
> Francisco, CA to explore cutting-edge tech and listen to tech luminaries
> present their vision of the future. This family event has something for
> everyone, including kids. Get more information and register today.
> http://sdm.link/attshape
> _______________________________________________
> Ryu-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/ryu-devel
>

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to