Skip to contentSkip to conversation

Fix regression against commit 4259

PATCH In review Jason Wessel 2 participants 3 messages Apr 26, 2008 – Apr 28, 2008

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Fix regression against commit 4259

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.

---
 slirp/udp.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)
slirp/udp.c +4 -3
@@ -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;
#1 lore raw

Hi,

Quoted 8 lines · 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.

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

#2 lore raw
Quoted 12 lines · andrzej zaborowski

Hi,

Quoted 8 lines · 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.

Quoted 6 lines

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.

Quoted 2 lines

and perhaps this shouldn't be for udp only?

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

Quoted 4 lines

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 ####

---
 slirp/udp.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
slirp/udp.c +1 -1
@@ -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;