buglwIP - A Lightweight TCP/IP stack - Bugs: bug #27704, autoip starts with wrong address -...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #27704: autoip starts with wrong address - ntohl

Submitted by:  Tamas Somogyi <tsomogyi>
Submitted on:  Wed 14 Oct 2009 04:10:04 PM UTC  
 
Category: IPv4Severity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: CVS Head

(Jump to the original submission Jump to the original submission)

Tue 27 Oct 2009 08:31:49 PM UTC, comment #6:

Thanks.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 27 Oct 2009 10:39:34 AM UTC, comment #5:

Right.
Just the doxygen comment for autoip_create_addr() should be updated as you renamed IPAddr -> ipaddr.

Luca

Luca Ceresoli <lucaceresoli>
Thu 15 Oct 2009 02:09:27 PM UTC, comment #4:

Fixed by adding htonl() to the default implementation of LWIP_AUTOIP_CREATE_SEED_ADDR().

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 15 Oct 2009 11:01:51 AM UTC, comment #3:

But as I said in comment #1, in lwIP, addresses are stored in network byte order, so the address loaded by LWIP_AUTOIP_CREATE_SEED_ADDR() will be network byte order, too. Only the default implementation of LWIP_AUTOIP_CREATE_SEED_ADDR() is wrong and returns host byte order. I'll fix that.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 15 Oct 2009 07:39:26 AM UTC, comment #2:

goldsimon# yes, it always returns 169.254.254.169 on little-endian (e.g. our TI DSP or Windows) platforms, because the wrong (mirrored) part is kept by the mask "addr = AUTOIP_NET | (addr & 0xffff);"
This also means that having two or more devices on the network, they all picks up the same address (169.254.254.169) causing IP address conflict! In this case "addr += netif->autoip->tried_llipaddr;" doesn't help either, because the wrong part is masked.
And as AUTOIP_NET is defined as a 32-bit number, the compiler will generate the correct byte representation both on little and big endian platforms, thus ntohl() to be removed.

Tamas Somogyi <tsomogyi>
Wed 14 Oct 2009 06:01:48 PM UTC, comment #1:

Whether this is a bug or not, I guess that depends on the defition of the macro LWIP_AUTOIP_CREATE_SEED_ADDR() - whether it returns the address in host or network byte order.

I'd vote for network byte order, but that doesn't seem to be defined. But then, the default implementation is clearly wrong since it returns host byte order.

Thus, on win32, I get the address 169.254.254.169, which is a correct autoip address, but not what we want.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Wed 14 Oct 2009 04:10:04 PM UTC, original submission:

Current code in autoip.c:
static void
autoip_create_addr(...)
{
...
u32_t addr = ntohl(LWIP_AUTOIP_CREATE_SEED_ADDR(netif));
...

Should be:
u32_t addr = LWIP_AUTOIP_CREATE_SEED_ADDR(netif);

So ntohl() should be removed, otherwise it reverses the byte order on little-endian platforms and makes the whole IP address calculation wrong.

I'm wondering if nobody noticed this e.g. on Windows so far...

Tamas Somogyi <tsomogyi>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by lucaceresoli (Posted a comment)
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by tsomogyi (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Thu 15 Oct 2009 02:09:27 PM UTCgoldsimonStatusIn Progress=>Fixed
      Open/ClosedOpen=>Closed
    Thu 15 Oct 2009 11:01:51 AM UTCgoldsimonAssigned toNone=>goldsimon
      StatusNone=>In Progress

    Back to the top


    Powered by Savane 3.1-cleanup1