[Qemu-devel] [PATCH] Fix regression against commit 4259

[Qemu-devel] [PATCH] Fix regression against commit 4259

[Jason Wessel]

Stuart Brady found a regression based on the 4259 commit where
DNS via slirp to the special address does not work properly.

I verified that DNS to the default special address 10.0.2.3 does not
work.   When using the "special address" for DNS it must be treated as
a special case.  In the original 4259, I did not have a test case for
the loopback DNS.  The translation logic should be used in this case
as well as for any loopback directed requests.

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

---
 slirp/udp.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -318,11 +318,12 @@ int udp_output(struct socket *so, struct
     struct sockaddr_in saddr, daddr;
 
     saddr = *addr;
-    if ((so->so_faddr.s_addr & htonl(0xffffff00)) == special_addr.s_addr &&
-        addr->sin_addr.s_addr == htonl(0x7f000001)) {
-        saddr.sin_addr.s_addr = so->so_faddr.s_addr;
+    if ((so->so_faddr.s_addr & htonl(0xffffff00)) == special_addr.s_addr) {
         if ((so->so_faddr.s_addr & htonl(0x000000ff)) == htonl(0xff))
             saddr.sin_addr.s_addr = alias_addr.s_addr;
+        else if (addr->sin_addr.s_addr == htonl(0x7f000001) ||
+                 ((so->so_faddr.s_addr & htonl(CTL_DNS)) == htonl(CTL_DNS)))
+            saddr.sin_addr.s_addr = so->so_faddr.s_addr;
     }
     daddr.sin_addr = so->so_laddr;
     daddr.sin_port = so->so_lport;

Re: [Qemu-devel] [PATCH] Fix regression against commit 4259

[andrzej zaborowski]

Hi,

On 26/04/2008, Jason Wessel <jason.wessel@windriver.com> wrote:
> Stuart Brady found a regression based on the 4259 commit where
>  DNS via slirp to the special address does not work properly.
>
>  I verified that DNS to the default special address 10.0.2.3 does not
>  work.   When using the "special address" for DNS it must be treated as
>  a special case.  In the original 4259, I did not have a test case for
>  the loopback DNS.  The translation logic should be used in this case
>  as well as for any loopback directed requests.

Thanks for the fix, I committed it with a tiny change but I'm not sure
if the special case shouldn't be CTL_ALIAS, i.e. the condition would
be:

         else if (addr->sin_addr.s_addr == loopback_addr.s_addr ||
                  (ntohl(so->so_faddr.s_addr) & 0xff) != CTL_ALIAS)

and perhaps this shouldn't be for udp only?  Also, in
((so->so_faddr.s_addr & htonl(CTL_DNS)) == htonl(CTL_DNS)), masking
with htonl(CTL_DNS) is probably not what we want.

Regards

Re: [Qemu-devel] [PATCH] Fix regression against commit 4259

[Jason Wessel]

andrzej zaborowski wrote:
> Hi,
> 
> On 26/04/2008, Jason Wessel <jason.wessel@windriver.com> wrote:
>> Stuart Brady found a regression based on the 4259 commit where
>>  DNS via slirp to the special address does not work properly.
>>
>>  I verified that DNS to the default special address 10.0.2.3 does not
>>  work.   When using the "special address" for DNS it must be treated as
>>  a special case.  In the original 4259, I did not have a test case for
>>  the loopback DNS.  The translation logic should be used in this case
>>  as well as for any loopback directed requests.
> 
> Thanks for the fix, I committed it with a tiny change but I'm not sure
> if the special case shouldn't be CTL_ALIAS, i.e. the condition would
> be:
> 
>          else if (addr->sin_addr.s_addr == loopback_addr.s_addr ||
>                   (ntohl(so->so_faddr.s_addr) & 0xff) != CTL_ALIAS)

It would seem reasonable to implement it this way.

> 
> and perhaps this shouldn't be for udp only?

It is already udp only because we are in the udp.c :-)


> Also, in
> ((so->so_faddr.s_addr & htonl(CTL_DNS)) == htonl(CTL_DNS)), masking
> with htonl(CTL_DNS) is probably not what we want.
> 

I would certainly agree with you here, while it works, it is less
efficient and not exactly what we want.  I retested all the different
cases with the patch you recommended (see below) and all the test
cases pass.

Thanks,
Jason.


#### CLIP HERE ####

Signed-off-by: Jason Wessel <jason.wessel@windriver.com>

---
 slirp/udp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/slirp/udp.c
+++ b/slirp/udp.c
@@ -322,7 +322,7 @@ int udp_output(struct socket *so, struct
         if ((so->so_faddr.s_addr & htonl(0x000000ff)) == htonl(0xff))
             saddr.sin_addr.s_addr = alias_addr.s_addr;
         else if (addr->sin_addr.s_addr == loopback_addr.s_addr ||
-                 ((so->so_faddr.s_addr & htonl(CTL_DNS)) == htonl(CTL_DNS)))
+                 (ntohl(so->so_faddr.s_addr) & 0xff) != CTL_ALIAS)
             saddr.sin_addr.s_addr = so->so_faddr.s_addr;
     }
     daddr.sin_addr = so->so_laddr;