Re: [lwip-devel] Last changes by Marc

[Jonathan Larmour]

Frédéric BERNON wrote:
> Some notes about last changes by Marc Boucher. We have talk yesterday 
> with him about some of these changes.

I didn't see anything about these changes anywhere? Nor anything in the 
Changelog. I'm also surprised by all these changes this close to 1.3 when 
other less problematic changes are not being done. That being said, most of 
the changes look good.

> - "Add a different memp pool for MEMP_NUM_TCPIP_MSG_API & 
> MEMP_NUM_TCPIP_MSG_INPKT". I'm in flavor of this change, since it avoid 
> problems when we receive lot of packets (or when we got a DoS attack). 
> Perhaps "INPKT" is not so clear. "INPACKET"? (it's a detail of course).

INPKT seems ok to me.

> - "pbuf": split the flags fields in flags & type don't cause me any 
> problems  (except I would rename "flgs" in "flags").

I agree.

> - "PBUF_FLAG_PUSH": don't cause me any problem, but it's only used by 
> socket layer. If we add LWIP_SOCKET option, we can use it to disable the 
> code in tcp_in.c if anyone want.

Yes please.

> - "tcpip.c / ETHARP_TCPIP_INPUT / ETHARP_TCPIP_ETHINPUT": No problem, 
> but, to be honest, I'm in flavor to remove ETHARP_TCPIP_INPUT & 
> ETHARP_TCPIP_ETHINPUT option, and to only support "ETHINPUT" case. It 
> will simplify the code, and is the only way to have no concurrent access 
> in ARP module. No objects to change that?

That seems fine to me. Worth doing for 1.3 I think since we're breaking old 
ports as it is, so no reason to hold back here (and cause more breakage for 
1.4).

> - "tcpip.c / TCPIP_MSG_TIMEOUT": this part increase for evryone the 
> footprint (even if you don't use pppoe, your linker should remove 
> tcpip_timeout, but not the part in the main loop's switch). Add option?

Yes please.

> - "sockets.c / check netconn_peer": I don't see no case where 
> netconn_peer should return an error. It's increase the footprint, so, is 
> it really useful (do you already have the case with cvs head code)?

netconn_peer can return ERR_CONN.

> - "sockets.c / lwip_recvfrom": I don't have test it, and for me it 
> should be ok, but, it change the behavior of the function: before, the 
> function return when it add processed a netbuf. Now, it wait until got 
> "len" bytes. I note this is in this case that PBUF_FLAG_PUSH is used.

It does match standard sockets API behaviour a bit better (as used on other 
systems), but I don't think it's a big issue and it changes code size very 
little.

Jifl
--
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["The best things in life aren't things."]------      Opinions==mine