qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Jason Wessel <jason.wessel@windriver.com>
To: qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH] Fix regression against commit 4259
Date: Mon, 28 Apr 2008 07:50:22 -0500	[thread overview]
Message-ID: <4815C80E.1020104@windriver.com> (raw)
In-Reply-To: <fb249edb0804271542p7bd8d6caxe71b6fe8a5a33d8e@mail.gmail.com>

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;

      reply	other threads:[~2008-04-28 12:50 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-26 21:30 [Qemu-devel] [PATCH] Fix regression against commit 4259 Jason Wessel
2008-04-27 22:42 ` andrzej zaborowski
2008-04-28 12:50   ` Jason Wessel [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4815C80E.1020104@windriver.com \
    --to=jason.wessel@windriver.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).