Tue 04 Dec 2007 09:16:14 PM UTC, comment #16:
Tested with 2 pktif-interfaces running under win32 (each with a different MAC address but connected to the same physical network). They both get an IP so everything seems to be fine.
|
Mon 03 Dec 2007 08:35:24 PM UTC, comment #15:
Checked in. The only thing missing now is a real-world-test. Hope I'll be able to do this with a virtual machine...
|
Mon 03 Dec 2007 04:51:21 PM UTC, comment #14:
Sorry, of course it does. Seems like I mixed up 'inp' and 'netif_list'...
But it was kind of early when I wrote that! ;-)
I'll take your post as approval and check it (meaning the mix of your code and mine) in.
|
Mon 03 Dec 2007 03:09:30 PM UTC, comment #13:
Oh, I didn't think it was the same - I thought the version I gave would skip inp in netif_list.
|
Mon 03 Dec 2007 07:26:51 AM UTC, comment #12:
Your code does the same as my code, but your coed it's a little better to read!
|
Sun 02 Dec 2007 10:49:14 PM UTC, comment #11:
I'm not clear on why the first conditional is anything other than:
if (first) {
?
If you want to of course you could add the extra 'if':
if (first) {
netif = netif_list;
first = 0;
} else {
netif = netif->next;
}
if
(netif == inp)
netif = netif->next;
} while(netif != NULL);
|
Sun 02 Dec 2007 02:45:16 PM UTC, comment #10:
> In that case, I think the best thing to do might be to check
> whether the 'inp' netif does match (in terms of IP address, mask,
> broadcast addr, etc.) and only resort to scanning the netif list
> if it doesn't. This will make things faster for the common case too!
Here's a patch for this. In ip_input, the code in the loop mainly stays the same but the simple for-loop is changed to a do..while and is started with inp, if that doesn't match, netif_list is searched.
The downside of this is that if inp doesn't match, it is checked twice (one time first and one time since it is on netif_list, too). But since this is a rather uncommon case, I think that's ok.
(file #14535)
|
Fri 30 Nov 2007 03:30:58 PM UTC, comment #9:
I'm not sure, but what it might be is to allow multiple netifs on the same underlying hardware interface, i.e. IP aliases. The hardware interface doesn't know about matching IPs, so it would have to just pick one netif to pass it up with. But ip_input would need to sort it out and get a proper match on the interface.
In that case, I think the best thing to do might be to check whether the 'inp' netif does match (in terms of IP address, mask, broadcast addr, etc.) and only resort to scanning the netif list if it doesn't. This will make things faster for the common case too!
|
Fri 30 Nov 2007 08:52:06 AM UTC, comment #8:
The UDP patch works kind of nice, I'll check it in.
As to the patch to ip.c (checking for DHCP first, then processing the netif_list): I'm not sure it works without this: for the udp.c patch to work, it is necessary that the netif is correct. So When having multiple netifs on the same network, the code in ip_input might select the wrong netif to pass to udp_input...
But this is a generic problem in ip_input (selecting a netif from the list instead of passing the 'inp' netif after it is checked). I'm sure there is a reason for this code, but I just don't see it: it seems kind of unsafe to accept a packet from one netif and tell UDP it is from another netif?
|
Thu 29 Nov 2007 07:49:07 AM UTC, comment #7:
> Uh, don't you have this the wrong way round? It will make DHCP
> faster, but everything else slower because it will be doing the
> partial packet decoding and checks every time for every packet.
You're right of course. See how it can make you blind working on one thing all the time ;-)
>> Next, it would be good to prevent user-applications to bind to
>> DHCP_CLIENT_PORT when LWIP_DHCP is enabled
> I don't think we should worry about this.
Maybe you're right there, too. I wouldn't know a decent solution to check this, anyway.
I'll apply the first patch once I found a way to test this at home (Vmware shold somehow allow me to test on multilple networks).
|
Thu 29 Nov 2007 01:33:16 AM UTC, comment #6:
The first patch looks good, thanks (although I can't test it :-| ).
> BTW: ip_input could be sped up if checking for DHCP_CLIENT_PORT
> before walking the netif list (second file).
Uh, don't you have this the wrong way round? It will make DHCP faster, but everything else slower because it will be doing the partial packet decoding and checks every time for every packet.
DHCP is a very infrequent event, and everything else should be optimised.
> Next, it would be good to prevent user-applications to bind to
> DHCP_CLIENT_PORT when LWIP_DHCP is enabled, but there's no way
> of knowing whether the pcb belongs to dhcp.c or a user app in
> udp_bind :-(
I don't think we should worry about this. It would have to be a very deliberate act, so may even be intentional. I wouldn't think it would be good to add code to prevent something that's most likely to be irrelevant, and if anyone did do it, it's probably no a coincidence, and so they would have to remove the check!
|
Wed 28 Nov 2007 09:48:30 PM UTC, comment #5:
Here's patch for udp_input (first file).
Couldn't test it much since I don't have two networks to connect to... But it works with ethernet-netif and loopif.
BTW: ip_input could be sped up if checking for DHCP_CLIENT_PORT before walking the netif list (second file).
Next, it would be good to prevent user-applications to bind to DHCP_CLIENT_PORT when LWIP_DHCP is enabled, but there's no way of knowing whether the pcb belongs to dhcp.c or a user app in udp_bind :-(
(file #14504, file #14505)
|
Wed 28 Nov 2007 12:07:43 PM UTC, comment #4:
> And keep in mind that udp_input also includes a hack to receive DHCP messages
Damn, I was wrong: the hack to receive DHCP messages on a netif that is down is in ip_input, not udp_input :-)
|
Wed 28 Nov 2007 08:54:26 AM UTC, comment #3:
But we already have the netif in udp_input. Searching it based on the transaction ID seems kind of weird... We only have to find the correct pcb. And keep in mind that udp_input also includes a hack to receive DHCP messages, I only would make that hack a little bit 'better' :-)
|
Wed 28 Nov 2007 08:49:03 AM UTC, comment #2:
Can't we use the dhcp's transaction id (which is 32bits) to initialize it with the netif* value or a netif index ? (but I doesn't read the code)
|
Wed 28 Nov 2007 06:51:53 AM UTC, comment #1:
Just an idea (as a reminder for later):
In udp_input, before looping over udp_pcbs to find the correct pcb, check (when LWIP_DHCP) if src-port == DHCP_SERVER and dst-port == DHCP_CLIENT, then check if inp->dhcp is != NULL and inp->dhcp->pcb is != NULL and then use that pcb. If not, don't loop over udp_pcbs -> no client software can use the DHCP port if DHCP is enabled.
Binding multiple dhcp pcbs to dhcp client port doesn't work yet...
|
Tue 27 Nov 2007 09:21:24 PM UTC, original submission:
This is from reading the code only but I'm pretty sure:
DHCP pcbs can't be bound to a specific netif (since at the time DHCP starts, that netif has no IP address and binding is done using IP addresses). That means when having multiple IP addresses, any of the active DHCP pcbs will get an incoming DHCP packet.
For that to be solved, the DHCP input code (parts of it in udp.c) should be changed to identify the netif a DHCP pcb wants to use. In other words, DHCP UDP pcbs should somehow bind to a specific netif rather then receive on all netifs.
|