|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT/LWIP PATCH 4/5] Adapt getnameinfo() function to Unikraft
Hi Felipe,
Please see inline.
On 8/15/19 5:05 PM, Felipe Huici wrote:
> Hi Costin, Bogdan,
>
> The build breaks if lwip/ipv6 support isn't selected. I'm not sure what the
> best solution is, but at least a #error with a more user-friendly message
> than that provided by the compiler would be already better?
>
You're right, I missed that one. I will use '#if CONFIG_LWIP_IPV6' to
protect those lines in v2.
> Also, is there any reason for the rather verbose commit message? It seems
> like this should be part of Unikraft's documentation, rather than a
> description of this particular patch.
>
You're right it should be part of documentation. But until we do that, I
think it doesn't hurt leaving it here as an example, because it also
contains details specific to this adaptation.
> There's also a minor comment inline.
>
> Thanks,
>
> -- Felipe
>
> On 02.08.19, 14:57, "Costin Lupu" <costin.lupu@xxxxxxxxx> wrote:
>
> This is how you adapt a function to Unikraft:
> 1. add license if it is missing; in our case, getnameinfo() was initially
> taken
> from musl to OsV and enhanced a bit, so we had to add the BSD license of
> OsV and
> the MIT license of musl; we decided to take the OsV implementation
> because it
> also checks the local /etc/hosts file before issuing DNS requests
> 2. use Unikraft headers
> 3. fix checkpatch issues
> 4. disable some unsupported functionality; in our case, we don't support
> name
> requests for getnameinfo(); we could have if the DNS implementation of
> lwip
> supported DNS resource records
> 5. define constants that are not already provided by the origin code
> 6. add its source file to Makefile.uk
> 7. export the function
>
> Signed-off-by: Costin Lupu <costin.lupu@xxxxxxxxx>
> ---
> Makefile.uk | 1 +
> exportsyms.uk | 1 +
> getnameinfo.c | 135 +++++++++++++++++++++++++++++++++++-------------
> include/netdb.h | 23 +++++++++
> inet.c | 2 +
> 5 files changed, 127 insertions(+), 35 deletions(-)
>
> diff --git a/Makefile.uk b/Makefile.uk
> index cfca555..675d52e 100644
> --- a/Makefile.uk
> +++ b/Makefile.uk
> @@ -81,6 +81,7 @@ LIBLWIP_SRCS-y += $(LIBLWIP_BASE)/init.c|unikraft
> LIBLWIP_SRCS-y += $(LIBLWIP_BASE)/time.c|unikraft
> LIBLWIP_SRCS-y += $(LIBLWIP_BASE)/inet.c|unikraft
> LIBLWIP_SRCS-$(CONFIG_LWIP_SOCKET) += $(LIBLWIP_BASE)/sockets.c|unikraft
> +LIBLWIP_SRCS-$(CONFIG_LWIP_SOCKET) +=
> $(LIBLWIP_BASE)/getnameinfo.c|unikraft
> LIBLWIP_SRCS-y += $(LIBLWIP_EXTRACTED)/core/init.c
> LIBLWIP_SRCS-y += $(LIBLWIP_EXTRACTED)/core/def.c
> LIBLWIP_SRCS-y += $(LIBLWIP_EXTRACTED)/core/inet_chksum.c
> diff --git a/exportsyms.uk b/exportsyms.uk
> index 7362abb..9df9d0d 100644
> --- a/exportsyms.uk
> +++ b/exportsyms.uk
> @@ -38,3 +38,4 @@ inet_pton
> lwip_getaddrinfo
> lwip_freeaddrinfo
> gai_strerror
> +getnameinfo
> diff --git a/getnameinfo.c b/getnameinfo.c
> index fc5acf6..ee8bfe0 100644
> --- a/getnameinfo.c
> +++ b/getnameinfo.c
> @@ -1,14 +1,61 @@
> -#include <osv/debug.h>
> -#include <netdb.h>
> -#include <limits.h>
> -#include <stdlib.h>
> -#include <string.h>
> +/* SPDX-License-Identifier: BSD-3-Clause AND MIT */
> +/*
> + * Copyright (C) 2014, Cloudius Systems, Ltd.
> + * Copyright (c) 2019, University Politehnica of Bucharest.
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the
> distribution.
> + * 3. Neither the name of the author nor the names of any co-contributors
> + * may be used to endorse or promote products derived from this
> software
> + * without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> PURPOSE
> + * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE
> LIABLE
> + * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> CONSEQUENTIAL
> + * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE
> GOODS
> + * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
> + * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
> STRICT
> + * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
> WAY
> + * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +/* For the parts taken from musl (marked as such below), the MIT licence
> + * applies instead:
> + * ----------------------------------------------------------------------
> + * Copyright (c) 2005-2014 Rich Felker, et al.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining
> + * a copy of this software and associated documentation files (the
> + * "Software"), to deal in the Software without restriction, including
> + * without limitation the rights to use, copy, modify, merge, publish,
> + * distribute, sublicense, and/or sell copies of the Software, and to
> + * permit persons to whom the Software is furnished to do so, subject to
> + * the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
> + * IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY
> + * CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT,
> + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE
> + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
> + * ----------------------------------------------------------------------
> + */
> #include <stdio.h>
> -#include <ctype.h>
> -#include <sys/socket.h>
> -#include <netinet/in.h>
> +#include <netdb.h>
> #include <arpa/inet.h>
> -#include "__dns.hh"
>
> int getnameinfo(const struct sockaddr *restrict sa, socklen_t sl,
> char *restrict node, socklen_t nodelen,
> @@ -16,7 +63,7 @@ int getnameinfo(const struct sockaddr *restrict sa,
> socklen_t sl,
> int flags)
> {
> char buf[256];
> - unsigned char reply[512];
> + /*unsigned char reply[512];*/
>
> Any reason for leaving this in the code?
>
This is related to the disabled __dns_get_rr() call below. I left it
commented in the code in order to give more insights about how the
function is called. As soon as we'll have DNS resource records support,
we will remove this as well. I will add a TODO in v2 showing this relation.
> int af = sa->sa_family;
> char line[512];
> FILE *f;
> @@ -24,12 +71,14 @@ int getnameinfo(const struct sockaddr *restrict sa,
> socklen_t sl,
>
> switch (af) {
> case AF_INET:
> - a = (void *)&((struct sockaddr_in *)sa)->sin_addr;
> - if (sl != sizeof(struct sockaddr_in)) return EAI_FAMILY;
> + a = (void *) &((struct sockaddr_in *) sa)->sin_addr;
> + if (sl != sizeof(struct sockaddr_in))
> + return EAI_FAMILY;
> break;
> case AF_INET6:
> - a = (void *)&((struct sockaddr_in6 *)sa)->sin6_addr;
> - if (sl != sizeof(struct sockaddr_in6)) return EAI_FAMILY;
> + a = (void *) &((struct sockaddr_in6 *) sa)->sin6_addr;
> + if (sl != sizeof(struct sockaddr_in6))
> + return EAI_FAMILY;
> break;
> default:
> return EAI_FAMILY;
> @@ -37,41 +86,57 @@ int getnameinfo(const struct sockaddr *restrict sa,
> socklen_t sl,
>
> /* Try to find ip within /etc/hosts */
> if ((node && nodelen) && (af == AF_INET)) {
> - const char *ipstr = inet_ntoa(((struct sockaddr_in
> *)sa)->sin_addr);
> - size_t l = strlen(ipstr);
> + const char *ipstr;
> + size_t l;
> +
> + ipstr = inet_ntoa(((struct sockaddr_in *)sa)->sin_addr);
> + l = strlen(ipstr);
> f = fopen("/etc/hosts", "r");
> - if (f) while (fgets(line, sizeof line, f)) {
> - if (strncmp(line, ipstr, l) != 0)
> - continue;
> + if (f)
> + while (fgets(line, sizeof(line), f)) {
> + char *domain;
>
> - char *domain = strtok(line, " ");
> - if (!domain) continue;
> - domain = strtok(NULL, " ");
> - if (!domain) continue;
> + if (strncmp(line, ipstr, l) != 0)
> + continue;
>
> - if (strlen(domain) >= nodelen) return EAI_OVERFLOW;
> - strcpy(node, domain);
> + domain = strtok(line, " ");
> + if (!domain)
> + continue;
> + domain = strtok(NULL, " ");
> + if (!domain)
> + continue;
> +
> + if (strlen(domain) >= nodelen)
> + return EAI_OVERFLOW;
> + strcpy(node, domain);
> + fclose(f);
> + return 0;
> + }
> + if (f)
> fclose(f);
> - return 0;
> - }
> - if (f) fclose(f);
> }
>
> if (node && nodelen) {
> if ((flags & NI_NUMERICHOST)
> +#if 0
> + /* TODO we currently don't support name requests */
> || __dns_query(reply, a, af, 1) <= 0
> - || __dns_get_rr(buf, 0, 256, 1, reply, RR_PTR, 1) <= 0)
> - {
> - if (flags & NI_NAMEREQD) return EAI_NONAME;
> - inet_ntop(af, a, buf, sizeof buf);
> + || __dns_get_rr(buf, 0, 256, 1, reply, RR_PTR, 1) <= 0)
> {
> +#else
> + || 1) {
> +#endif
> + if (flags & NI_NAMEREQD)
> + return EAI_NONAME;
> + inet_ntop(af, a, buf, sizeof(buf));
> }
> - if (strlen(buf) >= nodelen) return EAI_OVERFLOW;
> + if (strlen(buf) >= nodelen)
> + return EAI_OVERFLOW;
> strcpy(node, buf);
> }
>
> if (serv && servlen) {
> - if (snprintf(buf, sizeof buf, "%d",
> - ntohs(((struct sockaddr_in *)sa)->sin_port))>=servlen)
> + if (snprintf(buf, sizeof(buf), "%d",
> + ntohs(((struct sockaddr_in *) sa)->sin_port)) >= (int)
> servlen)
> return EAI_OVERFLOW;
> strcpy(serv, buf);
> }
> diff --git a/include/netdb.h b/include/netdb.h
> index 0cbcb5e..d31624c 100644
> --- a/include/netdb.h
> +++ b/include/netdb.h
> @@ -26,3 +26,26 @@ struct protoent {
> };
>
> const char *gai_strerror(int errcode);
> +
> +/*
> + * Constants for getnameinfo()
> + */
> +#define NI_MAXHOST 1025
> +#define NI_MAXSERV 32
> +
> +/*
> + * Flag values for getnameinfo()
> + */
> +#define NI_NUMERICHOST 0x01
> +#define NI_NUMERICSERV 0x02
> +#define NI_NOFQDN 0x04
> +#define NI_NAMEREQD 0x08
> +#define NI_DGRAM 0x10
> +#define NI_NUMERICSCOPE 0x20
> +
> +/* Error values for getaddrinfo() not defined by lwip/netdb.h */
> +#define EAI_OVERFLOW 205 /* Argument buffer overflow. */
> +
> +int getnameinfo(const struct sockaddr *addr, socklen_t addrlen,
> + char *host, socklen_t hostlen,
> + char *serv, socklen_t servlen, int flags);
> diff --git a/inet.c b/inet.c
> index f718e38..bbb81a3 100644
> --- a/inet.c
> +++ b/inet.c
> @@ -61,6 +61,8 @@ const char *gai_strerror(int errcode)
> return "Out of memory.";
> case EAI_FAMILY:
> return "The requested address family is not supported.";
> + case EAI_OVERFLOW:
> + return "The buffer pointed to by host or serv was too small.";
> #endif /* LWIP_DNS_API_DEFINE_ERRORS */
> default:
> return "Error on getaddrinfo.";
> --
> 2.20.1
>
>
>
> _______________________________________________
> Minios-devel mailing list
> Minios-devel@xxxxxxxxxxxxxxxxxxxx
> https://lists.xenproject.org/mailman/listinfo/minios-devel
>
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |