[Qemu-devel] Re: weird slirp problems (dns lookups stopped working, and maybe more) [PATCH]

[Igor Kovalenko]

On 1/12/07, Jason Wessel <jason.wessel at windriver.com> wrote:
>
> I posted a similar patch last year, but it was not accepted for some
> reason.  Making this change allows UDP nfs to work to a remote file
> server as well.
>
> I would recommend, making one further change as included in this patch.
> It allows KGDBOE to operate on the local host or from a remote host
> because it allow the special 10.0.2.2 address to be translated.
>
> I have included the patch in the e-mail once again.
>
> signed-off-by: jason.wessel at windriver.com
>
> Jason.
>
>
>
>
> Juergen Lock wrote:
> > On Wed, Jan 10, 2007 at 12:04:03AM +0100, Juergen Lock wrote:
> >
> >> ...
> >> Ok, garrison on irc just helped solve this mystery:  This (the same)
> >> one actually goes out first, before the 10.0.2.3 one (I didn't notice
> >> at first), and since slirp reuses the socket because the source ip and
> >> port hasnt changed (slirp/udp.c lines 172 and up, it doesn't check the
> >> dest ip), the second packet with the 10.0.2.3 dest ip (which gets
> >> replaced with the hosts's dns) goes out wrong.  And the reason this
> >> worked previously with the same guest is multicast started working
> >> only recently...
> >>
> >
> > And here's the fix I just added to the FreeBSD qemu port: (thanx
> garrison!)
> >
> > Index: qemu/slirp/udp.c
> > @@ -205,8 +208,6 @@
> >         /* udp_last_so = so; */
> >         so->so_laddr = ip->ip_src;
> >         so->so_lport = uh->uh_sport;
> > -       so->so_faddr = ip->ip_dst; /* XXX */
> > -       so->so_fport = uh->uh_dport; /* XXX */
> >
> >         if ((so->so_iptos = udp_tos(so)) == 0)
> >           so->so_iptos = ip->ip_tos;
> > @@ -216,6 +217,15 @@
> >          * and if it is, do the fork_exec() etc.
> >          */
> >       }
> > +
> > +     /*
> > +      * Assign destination unconditionally
> > +      *
> > +      * This fixes the case where packets are sent from the same
> > +      * source ip/port to different destination ips/ports
> > +      */
> > +     so->so_faddr = ip->ip_dst; /* XXX */
> > +     so->so_fport = uh->uh_dport; /* XXX */
> >
> >       iphlen += sizeof(struct udphdr);
> >       m->m_len -= iphlen;
> >
> >
> > _______________________________________________
> > Qemu-devel mailing list
> > Qemu-devel at nongnu.org
> > http://lists.nongnu.org/mailman/listinfo/qemu-devel
> >
>
>
>
> Index: qemu/slirp/udp.c
> ===================================================================
> --- qemu.orig/slirp/udp.c
> +++ qemu/slirp/udp.c
> @@ -205,8 +205,6 @@ udp_input(m, iphlen)
>           /* udp_last_so = so; */
>           so->so_laddr = ip->ip_src;
>           so->so_lport = uh->uh_sport;
> -         so->so_faddr = ip->ip_dst; /* XXX */
> -         so->so_fport = uh->uh_dport; /* XXX */
>
>           if ((so->so_iptos = udp_tos(so)) == 0)
>             so->so_iptos = ip->ip_tos;
> @@ -216,6 +214,13 @@ udp_input(m, iphlen)
>            * and if it is, do the fork_exec() etc.
>            */
>         }
> +       /* Always reset the from address as it can change,
> +        * as with NFS for example where it will talk to
> +        * the same destination port with multiple source
> +        * addresses. Or different gdb session to kgdboe.
> +        */
> +       so->so_faddr = ip->ip_dst; /* XXX */
> +       so->so_fport = uh->uh_dport; /* XXX */
>
>         iphlen += sizeof(struct udphdr);
>         m->m_len -= iphlen;
> @@ -312,7 +317,8 @@ 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)
> {
> +    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(0x000000ff)) == htonl(0xff))
>              saddr.sin_addr.s_addr = alias_addr.s_addr;
>
>
Indeed here it is
http://lists.gnu.org/archive/html/qemu-devel/2006-08/msg00392.html
seems like that comment there is wrong though - the patch helps setting
correct destination addr/port, not the source ones.

IMHO slirp UDP proxy bugfix is unconditionally worth committing :) but it
should be separated from host loopback address translation enhancement. It
is not clear if that would work at all in case guest is talking via UDP to
different destinations from the same source address.

-- 
Kind regards,
Igor V. Kovalenko