qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Samuel Thibault <samuel.thibault@ens-lyon.org>
Cc: qemu-devel@nongnu.org, thuth@redhat.com, jan.kiszka@siemens.com
Subject: Re: [Qemu-devel] [PATCH 2/2] slirp: Send RDNSS in RA only if host has an IPv6 DNS server
Date: Mon, 27 Mar 2017 11:56:00 -0300	[thread overview]
Message-ID: <a81b0761-eda4-144d-edfe-f283ecfa6bb6@amsat.org> (raw)
In-Reply-To: <20170326184634.25042-3-samuel.thibault@ens-lyon.org>

Hi Samuel,

On 03/26/2017 03:46 PM, Samuel Thibault wrote:
> Previously we would always send an RDNSS option in the RA, making the guest
> try to resolve DNS through IPv6, even if the host does not actually have
> and IPv6 DNS server available.
>
> This makes the RDNSS option enabled only when an IPv6 DNS server is
> available.
>
> Signed-off-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> ---
>  slirp/ip6_icmp.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/slirp/ip6_icmp.c b/slirp/ip6_icmp.c
> index d0f5cc1456..00183e5945 100644
> --- a/slirp/ip6_icmp.c
> +++ b/slirp/ip6_icmp.c
> @@ -189,18 +189,22 @@ void ndp_send_ra(Slirp *slirp)
>      t->m_data += NDPOPT_PREFIXINFO_LEN;
>      pl_size += NDPOPT_PREFIXINFO_LEN;
>
> -#ifndef _WIN32
>      /* Prefix information (NDP option) */
> -    /* disabled for windows for now, until get_dns6_addr is implemented */
> -    struct ndpopt *opt3 = mtod(t, struct ndpopt *);
> -    opt3->ndpopt_type = NDPOPT_RDNSS;
> -    opt3->ndpopt_len = NDPOPT_RDNSS_LEN / 8;
> -    opt3->ndpopt_rdnss.reserved = 0;
> -    opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval);
> -    opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6;
> -    t->m_data += NDPOPT_RDNSS_LEN;
> -    pl_size += NDPOPT_RDNSS_LEN;
> -#endif
> +    {

Why don't declare at function begining and remove this { } ?
Else reading the file one might ask himself "what if () condition was 
missed here?".
Except this indentation issue the patch is Ok for me.

> +        struct in6_addr addr;
> +        uint32_t scope_id;
> +        if (get_dns6_addr(&addr, &scope_id) >= 0) {
> +            /* Host system does have an IPv6 DNS server, announce our proxy.  */
> +            struct ndpopt *opt3 = mtod(t, struct ndpopt *);
> +            opt3->ndpopt_type = NDPOPT_RDNSS;
> +            opt3->ndpopt_len = NDPOPT_RDNSS_LEN / 8;
> +            opt3->ndpopt_rdnss.reserved = 0;
> +            opt3->ndpopt_rdnss.lifetime = htonl(2 * NDP_MaxRtrAdvInterval);
> +            opt3->ndpopt_rdnss.addr = slirp->vnameserver_addr6;
> +            t->m_data += NDPOPT_RDNSS_LEN;
> +            pl_size += NDPOPT_RDNSS_LEN;
> +        }
> +    }
>
>      rip->ip_pl = htons(pl_size);
>      t->m_data -= sizeof(struct ip6) + pl_size;
>

Phil.

  reply	other threads:[~2017-03-27 14:56 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-26 18:46 [Qemu-devel] [PATCH 0/2] Fix spurious RDNSS announce Samuel Thibault
2017-03-26 18:46 ` [Qemu-devel] [PATCH 1/2] slirp: Make RA build more flexible Samuel Thibault
2017-03-27 14:50   ` Philippe Mathieu-Daudé
2017-03-26 18:46 ` [Qemu-devel] [PATCH 2/2] slirp: Send RDNSS in RA only if host has an IPv6 DNS server Samuel Thibault
2017-03-27 14:56   ` Philippe Mathieu-Daudé [this message]
2017-03-27 15:04     ` Samuel Thibault

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=a81b0761-eda4-144d-edfe-f283ecfa6bb6@amsat.org \
    --to=f4bug@amsat.org \
    --cc=jan.kiszka@siemens.com \
    --cc=qemu-devel@nongnu.org \
    --cc=samuel.thibault@ens-lyon.org \
    --cc=thuth@redhat.com \
    /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).