[U-Boot-Users] [PATCH] net: make ARP timeout configurable

[U-Boot-Users] [PATCH] net: make ARP timeout configurable

[Guennadi Liakhovetski]

Currently the timeout waiting for an ARP reply is hard set to 5 seconds. 
On i.MX31ADS due to a hardware "strangeness" up to four first IP packets 
to the boards get lost, which typically are ARP replies. By configuring 
the timeout to a lower value we significantly improve the first network 
transfer time on this board. The timeout is specified in deciseconds, 
because it has to be converted to hardware ticks, and CFG_HZ ranges from 
900 to 27000000 on different boards.

Signed-off-by: Guennadi Liakhovetski <lg@denx.de>

---

diff --git a/README b/README
index a9663a3..9249064 100644
--- a/README
+++ b/README
@@ -1547,6 +1547,10 @@ The following options need to be configured:
 		before giving up the operation. If not defined, a
 		default value of 5 is used.
 
+		CONFIG_ARP_TIMEOUT
+
+		Timeout waiting for an ARP reply in deciseconds.
+
 - Command Interpreter:
 		CONFIG_AUTO_COMPLETE
 
diff --git a/net/net.c b/net/net.c
index 44feee2..2128bd4 100644
--- a/net/net.c
+++ b/net/net.c
@@ -94,11 +94,16 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define ARP_TIMEOUT		5UL		/* Seconds before trying ARP again */
+#ifndef	CONFIG_ARP_TIMEOUT
+# define ARP_TIMEOUT		50UL	/* Deciseconds before trying ARP again */
+#else
+# define ARP_TIMEOUT		CONFIG_ARP_TIMEOUT
+#endif
+
 #ifndef	CONFIG_NET_RETRY_COUNT
-# define ARP_TIMEOUT_COUNT	5		/* # of timeouts before giving up  */
+# define ARP_TIMEOUT_COUNT	5	/* # of timeouts before giving up  */
 #else
-# define ARP_TIMEOUT_COUNT  (CONFIG_NET_RETRY_COUNT)
+# define ARP_TIMEOUT_COUNT	CONFIG_NET_RETRY_COUNT
 #endif
 
 #if 0
@@ -129,7 +134,7 @@ uchar		NetOurEther[6];		/* Our ethernet address			*/
 uchar		NetServerEther[6] =	/* Boot server enet address		*/
 			{ 0, 0, 0, 0, 0, 0 };
 IPaddr_t	NetOurIP;		/* Our IP addr (0 = unknown)		*/
-IPaddr_t	NetServerIP;		/* Our IP addr (0 = unknown)		*/
+IPaddr_t	NetServerIP;		/* Server IP addr (0 = unknown)		*/
 volatile uchar *NetRxPkt;		/* Current receive packet		*/
 int		NetRxPktLen;		/* Current rx packet length		*/
 unsigned	NetIPID;		/* IP packet ID				*/
@@ -253,7 +258,7 @@ void ArpTimeoutCheck(void)
 	t = get_timer(0);
 
 	/* check for arp timeout */
-	if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ) {
+	if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ / 10) {
 		NetArpWaitTry++;
 
 		if (NetArpWaitTry >= ARP_TIMEOUT_COUNT) {
@@ -494,7 +499,7 @@ restart:
 		 *	Check the ethernet for a new packet.  The ethernet
 		 *	receive routine will process it.
 		 */
-			eth_rx();
+		eth_rx();
 
 		/*
 		 *	Abort if ctrl-c was pressed.

[U-Boot-Users] [PATCH] net: make ARP timeout configurable

[Ben Warren]

Hi Guennadi,

Guennadi Liakhovetski wrote:
> Currently the timeout waiting for an ARP reply is hard set to 5 seconds. 
> On i.MX31ADS due to a hardware "strangeness" up to four first IP packets 
> to the boards get lost, which typically are ARP replies. By configuring 
> the timeout to a lower value we significantly improve the first network 
> transfer time on this board. The timeout is specified in deciseconds, 
> because it has to be converted to hardware ticks, and CFG_HZ ranges from 
> 900 to 27000000 on different boards.
>
>   
I like this, but let's stick to SI units please, probably ms.  This 
isn't an important enough calculation to worry about precision, only 
overflow, so just make sure you handle order of operations properly and 
it should be fine.  If I'm not thinking this through properly, which is 
entirely possible, make the code do a /100 to get into ds rather than 
asking the user to provide it.  I'm concerned that people will say 
***WTF is a decisecond!***
> Signed-off-by: Guennadi Liakhovetski <lg@denx.de>
>
> ---
>
> diff --git a/README b/README
> index a9663a3..9249064 100644
> --- a/README
> +++ b/README
> @@ -1547,6 +1547,10 @@ The following options need to be configured:
>  		before giving up the operation. If not defined, a
>  		default value of 5 is used.
>  
> +		CONFIG_ARP_TIMEOUT
> +
> +		Timeout waiting for an ARP reply in deciseconds.
> +
>  - Command Interpreter:
>  		CONFIG_AUTO_COMPLETE
>  
> diff --git a/net/net.c b/net/net.c
> index 44feee2..2128bd4 100644
> --- a/net/net.c
> +++ b/net/net.c
> @@ -94,11 +94,16 @@
>  
>  DECLARE_GLOBAL_DATA_PTR;
>  
> -#define ARP_TIMEOUT		5UL		/* Seconds before trying ARP again */
> +#ifndef	CONFIG_ARP_TIMEOUT
> +# define ARP_TIMEOUT		50UL	/* Deciseconds before trying ARP again */
> +#else
> +# define ARP_TIMEOUT		CONFIG_ARP_TIMEOUT
> +#endif
> +
>  #ifndef	CONFIG_NET_RETRY_COUNT
> -# define ARP_TIMEOUT_COUNT	5		/* # of timeouts before giving up  */
> +# define ARP_TIMEOUT_COUNT	5	/* # of timeouts before giving up  */
>  #else
> -# define ARP_TIMEOUT_COUNT  (CONFIG_NET_RETRY_COUNT)
> +# define ARP_TIMEOUT_COUNT	CONFIG_NET_RETRY_COUNT
>  #endif
>  
>  #if 0
> @@ -129,7 +134,7 @@ uchar		NetOurEther[6];		/* Our ethernet address			*/
>  uchar		NetServerEther[6] =	/* Boot server enet address		*/
>  			{ 0, 0, 0, 0, 0, 0 };
>  IPaddr_t	NetOurIP;		/* Our IP addr (0 = unknown)		*/
> -IPaddr_t	NetServerIP;		/* Our IP addr (0 = unknown)		*/
> +IPaddr_t	NetServerIP;		/* Server IP addr (0 = unknown)		*/
>  volatile uchar *NetRxPkt;		/* Current receive packet		*/
>  int		NetRxPktLen;		/* Current rx packet length		*/
>  unsigned	NetIPID;		/* IP packet ID				*/
> @@ -253,7 +258,7 @@ void ArpTimeoutCheck(void)
>  	t = get_timer(0);
>  
>  	/* check for arp timeout */
> -	if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ) {
> +	if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ / 10) {
>  		NetArpWaitTry++;
>  
>  		if (NetArpWaitTry >= ARP_TIMEOUT_COUNT) {
> @@ -494,7 +499,7 @@ restart:
>  		 *	Check the ethernet for a new packet.  The ethernet
>  		 *	receive routine will process it.
>  		 */
> -			eth_rx();
> +		eth_rx();
>   
Not sure why you did this.  Is it indented from the if() once applied?

regards,
Ben

[U-Boot-Users] [PATCH] net: make ARP timeout configurable

[Guennadi Liakhovetski]

On Thu, 3 Apr 2008, Ben Warren wrote:

> Hi Guennadi,
> 
> Guennadi Liakhovetski wrote:
> > Currently the timeout waiting for an ARP reply is hard set to 5 seconds. On
> > i.MX31ADS due to a hardware "strangeness" up to four first IP packets to the
> > boards get lost, which typically are ARP replies. By configuring the timeout
> > to a lower value we significantly improve the first network transfer time on
> > this board. The timeout is specified in deciseconds, because it has to be
> > converted to hardware ticks, and CFG_HZ ranges from 900 to 27000000 on
> > different boards.
> > 
> I like this, but let's stick to SI units please, probably ms.  This isn't an
> important enough calculation to worry about precision, only overflow, so just
> make sure you handle order of operations properly and it should be fine.  If
> I'm not thinking this through properly, which is entirely possible, make the
> code do a /100 to get into ds rather than asking the user to provide it.  I'm
> concerned that people will say ***WTF is a decisecond!***

Ok, expect a new version shortly.

> > @@ -253,7 +258,7 @@ void ArpTimeoutCheck(void)
> >  	t = get_timer(0);
> >   	/* check for arp timeout */
> > -	if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ) {
> > +	if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ / 10) {
> >  		NetArpWaitTry++;
> >   		if (NetArpWaitTry >= ARP_TIMEOUT_COUNT) {
> > @@ -494,7 +499,7 @@ restart:
> >  		 *	Check the ethernet for a new packet.  The ethernet
> >  		 *	receive routine will process it.
> >  		 */
> > -			eth_rx();
> > +		eth_rx();
> >   
> Not sure why you did this.  Is it indented from the if() once applied?

It is not under an if - it is another hunk.

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office at denx.de

[U-Boot-Users] [PATCH] net: make ARP timeout configurable

[Ben Warren]

Guennadi Liakhovetski wrote:
> On Thu, 3 Apr 2008, Ben Warren wrote:
>
>   
>> Hi Guennadi,
>>
>> Guennadi Liakhovetski wrote:
>>     
>>> Currently the timeout waiting for an ARP reply is hard set to 5 seconds. On
>>> i.MX31ADS due to a hardware "strangeness" up to four first IP packets to the
>>> boards get lost, which typically are ARP replies. By configuring the timeout
>>> to a lower value we significantly improve the first network transfer time on
>>> this board. The timeout is specified in deciseconds, because it has to be
>>> converted to hardware ticks, and CFG_HZ ranges from 900 to 27000000 on
>>> different boards.
>>>
>>>       
>> I like this, but let's stick to SI units please, probably ms.  This isn't an
>> important enough calculation to worry about precision, only overflow, so just
>> make sure you handle order of operations properly and it should be fine.  If
>> I'm not thinking this through properly, which is entirely possible, make the
>> code do a /100 to get into ds rather than asking the user to provide it.  I'm
>> concerned that people will say ***WTF is a decisecond!***
>>     
>
> Ok, expect a new version shortly.
>
>   
>>> @@ -253,7 +258,7 @@ void ArpTimeoutCheck(void)
>>>  	t = get_timer(0);
>>>   	/* check for arp timeout */
>>> -	if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ) {
>>> +	if ((t - NetArpWaitTimerStart) > ARP_TIMEOUT * CFG_HZ / 10) {
>>>  		NetArpWaitTry++;
>>>   		if (NetArpWaitTry >= ARP_TIMEOUT_COUNT) {
>>> @@ -494,7 +499,7 @@ restart:
>>>  		 *	Check the ethernet for a new packet.  The ethernet
>>>  		 *	receive routine will process it.
>>>  		 */
>>> -			eth_rx();
>>> +		eth_rx();
>>>   
>>>       
>> Not sure why you did this.  Is it indented from the if() once applied?
>>     
>
> It is not under an if - it is another hunk.
>   
Oh yeah.  Sorry.

regards,
Ben