Hi,

On 2015年02月20日 06:37, Sam Russell wrote:
> That looks correct to me - in any case the test you've changed will
> pick it up. I can confirm that setting hlen=6 is working as part of my
> dhcp server for linux clients
> (https://github.com/samrussell/trekin/blob/master/trekin/app/trekin.py),
> and would expect that mac.text_to_bin("00:11:22:33:44:55") would
> return 6.

Thank you for checking.
I will post the patch this code based on.

thanks

> 
> On 19 February 2015 at 16:51, Minoru TAKAHASHI
> <[email protected]> wrote:
>> Hi,
>>
>> On 2015年02月17日 19:36, Sam Russell wrote:
>>> I've been working with ryu.lib.packet.dhcp and I believe hlen is being
>>> calculated incorrectly.
>>
>> I think so too.
>>
>>>
>>> If I create a dhcp() object without passing hlen, it attempts to set
>>> this to len(chaddr) - but this gets the length of the string (17 chars
>>> for a MAC address) instead of measuring the 6 octets.
>>>
>>> The code should be changed from this (ryu/lib/packet/dhcp.py line 156-157)
>>>
>>> if hlen == 0:
>>>   self.hlen = len(chaddr)
>>>
>>> to this
>>>
>>> if hlen == 0:
>>>   self.hlen = len(addrconv.mac.bin_to_text(chaddr))
>>
>> According to RFC2131, to set the hlen is "Hardware address length".
>> (e.g.  '6' = 10mb ethernet)
>>
>> So, I will post the following patch.
>> Could you please check?
>> (modification of unittest are also included.)
>>
>> ------------------------------------------------
>>  ryu/lib/packet/dhcp.py             | 2 +-
>>  ryu/tests/unit/packet/test_dhcp.py | 2 +-
>>  2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/ryu/lib/packet/dhcp.py b/ryu/lib/packet/dhcp.py
>> index 5320079..a172e79 100644
>> --- a/ryu/lib/packet/dhcp.py
>> +++ b/ryu/lib/packet/dhcp.py
>> @@ -154,7 +154,7 @@ class dhcp(packet_base.PacketBase):
>>          self.op = op
>>          self.htype = htype
>>          if hlen == 0:
>> -            self.hlen = len(chaddr)
>> +            self.hlen = len(addrconv.mac.text_to_bin(chaddr))
>>          else:
>>              self.hlen = hlen
>>          self.hops = hops
>> diff --git a/ryu/tests/unit/packet/test_dhcp.py 
>> b/ryu/tests/unit/packet/test_dhcp.py
>> index 44a10c5..af37ffe 100644
>> --- a/ryu/tests/unit/packet/test_dhcp.py
>> +++ b/ryu/tests/unit/packet/test_dhcp.py
>> @@ -56,7 +56,7 @@ class Test_dhcp_offer(unittest.TestCase):
>>      options = dhcp.options(option_list=option_list, options_len=50,
>>                             magic_cookie=magic_cookie)
>>
>> -    dh = dhcp.dhcp(op, chaddr, options, htype=htype, hlen=hlen,
>> +    dh = dhcp.dhcp(op, chaddr, options, htype=htype, hlen=0,
>>                     hops=hops, xid=xid, secs=secs, flags=flags,
>>                     ciaddr=ciaddr, yiaddr=yiaddr, siaddr=siaddr,
>>                     giaddr=giaddr, sname=sname, boot_file=boot_file)
>> --
>> 1.9.1
>> ------------------------------------------------
>>
>> thanks
>>
>>>
>>> This does involve doubling up on the bin_to_text operation, which
>>> makes me wonder whether it's best to leave hlen as 0 until the end of
>>> the dhcp._parser() function where chaddr is converted to hex.
>>>
>>> ------------------------------------------------------------------------------
>>> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
>>> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
>>> with Interactivity, Sharing, Native Excel Exports, App Integration & more
>>> Get technology previously reserved for billion-dollar corporations, FREE
>>> http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
>>> _______________________________________________
>>> Ryu-devel mailing list
>>> [email protected]
>>> https://lists.sourceforge.net/lists/listinfo/ryu-devel
>>>

------------------------------------------------------------------------------
Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
from Actuate! Instantly Supercharge Your Business Reports and Dashboards
with Interactivity, Sharing, Native Excel Exports, App Integration & more
Get technology previously reserved for billion-dollar corporations, FREE
http://pubads.g.doubleclick.net/gampad/clk?id=190641631&iu=/4140/ostg.clktrk
_______________________________________________
Ryu-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/ryu-devel

Reply via email to