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.
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
