[U-Boot] Possible fix for dns command

[U-Boot] Possible fix for dns command

[Mathias Adam]

Hello,

I'm currently using u-boot on a TI DM816x evaluation board (u-boot as
included in TI's EZSDK 5.02). I manually enabled CONFIG_CMD_DNS. However,
running dhcp followed by dns <some_host_name> only gave timeouts.

I found (using #define DEBUG) that DNS UDP queries are being sent to the
MAC of the DHCP server -- which is not equal to the DNS server, hence the
timeouts. Forcing ARP queries before DNS fixed this (see patch below). I
copyied that line from net/tftp.c (which works out of the box).

However, as I'm quite new to u-boot I'm not sure whether that behaviour is
intended or if my patch breaks other things?

Patch (against plain u-boot-2011.09):

--- net/dns.c	2011-09-29 21:11:15.000000000 +0200
+++
../ti-ezsdk_dm816x-evm_5_02_02_60/board-support/u-boot-2010.06-psp04.00.00.12/net/dns.c	2011-11-16
10:35:38.335903630 +0100
@@ -206,5 +206,8 @@
 	NetSetTimeout(DNS_TIMEOUT, DnsTimeout);
 	NetSetHandler(DnsHandler);
+	/* zero out server ether in case the server ip has changed */
+	memset(NetServerEther, 0, 6);
+
 	DnsSend();
 }



Regards,
Mathias Adam

[U-Boot] Possible fix for dns command

[Mathias Adam]

repost, in case my mail from last week was lost somewhere...

Am Mi, 16.11.2011, 13:37 schrieb Mathias Adam:
> Hello,
>
> I'm currently using u-boot on a TI DM816x evaluation board (u-boot as
> included in TI's EZSDK 5.02). I manually enabled CONFIG_CMD_DNS. However,
> running dhcp followed by dns <some_host_name> only gave timeouts.
>
> I found (using #define DEBUG) that DNS UDP queries are being sent to the
> MAC of the DHCP server -- which is not equal to the DNS server, hence the
> timeouts. Forcing ARP queries before DNS fixed this (see patch below). I
> copyied that line from net/tftp.c (which works out of the box).
>
> However, as I'm quite new to u-boot I'm not sure whether that behaviour is
> intended or if my patch breaks other things?
>
> Patch (against plain u-boot-2011.09):
>
> --- net/dns.c	2011-09-29 21:11:15.000000000 +0200
> +++
../ti-ezsdk_dm816x-evm_5_02_02_60/board-support/u-boot-2010.06-psp04.00.00.12/net/dns.c	2011-11-16
10:35:38.335903630 +0100
> @@ -206,5 +206,8 @@
>  	NetSetTimeout(DNS_TIMEOUT, DnsTimeout);
>  	NetSetHandler(DnsHandler);
>
> +	/* zero out server ether in case the server ip has changed */
> +	memset(NetServerEther, 0, 6);
> +
>  	DnsSend();
>  }
>
>
>
> Regards,
> Mathias Adam
>

[U-Boot] Possible fix for dns command

[Wolfgang Denk]

Dear "Mathias Adam",

In message <c231e6b8530fa0acb9b1389d049cf359.squirrel@www.inft.de> you wrote:
> repost, in case my mail from last week was lost somewhere...

It would help if you submitted this as proper patch, with a good
Subject:, a usable commit message and most of all with your
Signed-off-by: line.  For details please see
http://www.denx.de/wiki/U-Boot/Patches

Thanks.

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Anyone who doesn't believe in miracles is not a realist.
                                                   - David Ben Gurion

[U-Boot] Possible fix for dns command

[Stefano Babic]

On 16/11/2011 13:37, Mathias Adam wrote:
> Hello,
> 
Hi,

> I'm currently using u-boot on a TI DM816x evaluation board (u-boot as
> included in TI's EZSDK 5.02). I manually enabled CONFIG_CMD_DNS. However,
> running dhcp followed by dns <some_host_name> only gave timeouts.
> 
> I found (using #define DEBUG) that DNS UDP queries are being sent to the
> MAC of the DHCP server -- which is not equal to the DNS server, hence the
> timeouts.

I have checked this issue, but I am coming to a different conclusion. I
have sniffed the network between the DHCP server and the target, and the
target is able to recognize the option 6 (DNS - name server) of the DHCP
OFFER.

However, the code setting the variable using by the DNS command
("dnsip") is not set if a file is not loaded by the TFTP server.

In cmd_net.c, you see:

        if ((size = NetLoop(proto)) < 0) {
                show_boot_progress (-81);
                return 1;
        }

After that, netboot_update_env is called setting correctly the
variables. So if the transfer stops or is not successful, the dnsip
variable is not updated.

Can you check (it is not the right fix !) if commenting the above return
statement fixes the issue you report ?

> Forcing ARP queries before DNS fixed this (see patch below).

I do not think this is correct.

Best regards,
Stefano Babic

-- 
=====================================================================
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] Possible fix for dns command

[Mathias Adam]

Hi Stefano,

Am Mi, 23.11.2011, 10:57 schrieb Stefano Babic:
> On 16/11/2011 13:37, Mathias Adam wrote:
>> I'm currently using u-boot on a TI DM816x evaluation board (u-boot as
>> included in TI's EZSDK 5.02). I manually enabled CONFIG_CMD_DNS.
>> However,
>> running dhcp followed by dns <some_host_name> only gave timeouts.
>>
>> I found (using #define DEBUG) that DNS UDP queries are being sent to the
>> MAC of the DHCP server -- which is not equal to the DNS server, hence
>> the
>> timeouts.
>
> I have checked this issue, but I am coming to a different conclusion. I
> have sniffed the network between the DHCP server and the target, and the
> target is able to recognize the option 6 (DNS - name server) of the DHCP
> OFFER.
>
> However, the code setting the variable using by the DNS command
> ("dnsip") is not set if a file is not loaded by the TFTP server.
>
> In cmd_net.c, you see:
>
>         if ((size = NetLoop(proto)) < 0) {
>                 show_boot_progress (-81);
>                 return 1;
>         }
>
> After that, netboot_update_env is called setting correctly the
> variables. So if the transfer stops or is not successful, the dnsip
> variable is not updated.
>
> Can you check (it is not the right fix !) if commenting the above return
> statement fixes the issue you report ?


Just tried that: unfortunately it still doesn't work.

However, the problem I encountered is not that $dnsip wouldn't get set,
this part worked correctly out of the box.  But then, the dns command
doesn't perform an ARP query to get the MAC address of the machine with IP
$dnsip but instead uses whatever MAC address is stored in NetServerEther.

As mentioned before, I found that tftpboot does make an ARP query and
stores the result in NetServerEther. In fact, when I start a "dummy" tftp
access dns works, at least one time:

TI8168_EVM#tftp 0x81000000 :dummy
Using DaVinci EMAC device
TFTP from server 0.0.0.0; our IP address is 17x.xxx.yyy.126; sending
through gateway 17x.xxx.yyy.50
Filename 'dummy'.
Load address: 0x81000000
Loading: T
TFTP error: 'File not found' (1)
Not retrying...
TI8168_EVM#dns google.com
209.85.148.105
TI8168_EVM#dns google.com
Timeout
dns lookup of google.com failed, check setup
TI8168_EVM#

(the DNS server is located behind the default gateway so has the same MAC
address).



>> Forcing ARP queries before DNS fixed this (see patch below).
>
> I do not think this is correct.

hmm, not sure about u-boot, but generally I think it is a good idea to do
an ARP query before sending packets to some IP address  ;-)


Regards,
Mathias Adam

[U-Boot] Possible fix for dns command

[Stefano Babic]

On 23/11/2011 11:58, Mathias Adam wrote:
> Hi Stefano,
> 

Hi Matthias,

> 
> Just tried that: unfortunately it still doesn't work.
> 
> However, the problem I encountered is not that $dnsip wouldn't get set,
> this part worked correctly out of the box.

Ok, thanks to clarify this point.

>  But then, the dns command
> doesn't perform an ARP query to get the MAC address of the machine with IP
> $dnsip but instead uses whatever MAC address is stored in NetServerEther.

Understood, I see now. The main problem is that the MAC address is
already stored, and because there is not an ARP cache we cannot identify
which is the correct IP address.

> As mentioned before, I found that tftpboot does make an ARP query and
> stores the result in NetServerEther. In fact, when I start a "dummy" tftp
> access dns works, at least one time:

Ok, ARP does not return a valid MAC and the stored MAXC address is
removed. I see now. Then we should clear the MAC address before a DNS
query to force an ARP request.

>>> Forcing ARP queries before DNS fixed this (see patch below).
>>
>> I do not think this is correct.
> 
> hmm, not sure about u-boot, but generally I think it is a good idea to do
> an ARP query before sending packets to some IP address  ;-)

Sorry, it was not so clear to me after the first e-mail which was the
issue, and I was supposed u-boot does not take the DHCP option for DNS,
not setting the dnsip variable.

Now that the issue is clear and we agree on the solution, can you supply
a well-formed patch as requested by Wolfgang to be merged into mainline ?

Best regards,
Stefano Babic

-- 
=====================================================================
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] Possible fix for dns command

[Mathias Adam]

Hi Stefano,

Am Mi, 23.11.2011, 13:11 schrieb Stefano Babic:
> On 23/11/2011 11:58, Mathias Adam wrote:
>> As mentioned before, I found that tftpboot does make an ARP query and
>> stores the result in NetServerEther. In fact, when I start a "dummy"
>> tftp
>> access dns works, at least one time:
>
> Ok, ARP does not return a valid MAC and the stored MAXC address is
> removed. I see now. Then we should clear the MAC address before a DNS
> query to force an ARP request.
>
>>>> Forcing ARP queries before DNS fixed this (see patch below).
>>>
>>> I do not think this is correct.
>>
>> hmm, not sure about u-boot, but generally I think it is a good idea to
>> do
>> an ARP query before sending packets to some IP address  ;-)
>
> Sorry, it was not so clear to me after the first e-mail which was the
> issue, and I was supposed u-boot does not take the DHCP option for DNS,
> not setting the dnsip variable.
>
> Now that the issue is clear and we agree on the solution, can you supply
> a well-formed patch as requested by Wolfgang to be merged into mainline ?

No problem, I'm going to prepare a patch and post it.

Regards,
Mathias Adam

[U-Boot] [PATCH] Fix dns command

[Mathias Adam]

The dns command didn't perform an ARP query to get the MAC address of
the DNS server if a MAC address was already stored in NetServerEther.
In that case the stored address was used no matter what IP address it
belongs to.

With this patch, NetServerEther is being cleared before a DNS query to
force an ARP request.

Signed-off-by: Mathias Adam <m.adam--uboot@adamis.de>
---
 net/dns.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/net/dns.c b/net/dns.c
index 7a3f1f9..9d79d7d 100644
--- a/net/dns.c
+++ b/net/dns.c
@@ -202,5 +202,8 @@ DnsStart(void)
 	NetSetTimeout(DNS_TIMEOUT, DnsTimeout);
 	NetSetHandler(DnsHandler);

+	/* zero out server ether in case the server ip has changed */
+	memset(NetServerEther, 0, 6);
+
 	DnsSend();
 }

[U-Boot] [PATCH] Fix dns command

[Graeme Russ]

Hi Mathias,

On Thu, Nov 24, 2011 at 3:27 AM, Mathias Adam <m.adam--uboot@adamis.de> wrote:
> The dns command didn't perform an ARP query to get the MAC address of
> the DNS server if a MAC address was already stored in NetServerEther.
> In that case the stored address was used no matter what IP address it
> belongs to.
>
> With this patch, NetServerEther is being cleared before a DNS query to
> force an ARP request.
>
> Signed-off-by: Mathias Adam <m.adam--uboot@adamis.de>
> ---
> ?net/dns.c | ? ?3 +++
> ?1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/net/dns.c b/net/dns.c
> index 7a3f1f9..9d79d7d 100644
> --- a/net/dns.c
> +++ b/net/dns.c
> @@ -202,5 +202,8 @@ DnsStart(void)
> ? ? ? ?NetSetTimeout(DNS_TIMEOUT, DnsTimeout);
> ? ? ? ?NetSetHandler(DnsHandler);
>
> + ? ? ? /* zero out server ether in case the server ip has changed */
> + ? ? ? memset(NetServerEther, 0, 6);
> +
> ? ? ? ?DnsSend();
> ?}

Why not do this at the start of net-loop - It may introduce more ARP
requests, but at least any new protocols will autmatically avoid the
bug you are experiencing

You could cache the IP address which matches NetServerEther to avoid
the ARP when doing the same command repeatedly

Regards,

Graeme