[U-Boot] [RFC PATCH v2 06/11] net: IPv6 skeleton and environment variables

Chris Packham judge.packham at gmail.com
Tue Nov 24 10:47:58 CET 2015


On Tue, Nov 24, 2015 at 2:06 PM, Joe Hershberger
<joe.hershberger at gmail.com> wrote:
> Hi Chris,
>
> On Mon, Nov 9, 2015 at 1:38 AM, Chris Packham <judge.packham at gmail.com> wrote:
>> Create net6.c and add CONFIG_NET6 to Kconfig/Makefile. Also add
>> support for the following environment variables:
>>  - ip6addr
>>  - gateway6
>>  - serverip6
>>
>> Signed-off-by: Chris Packham <judge.packham at gmail.com>
>> ---
>>
>> Changes in v2:
>> - Split environment variables from main implementation
>> - remove "prefixlength6" environment variable. The prefix length is now
>>   set when specifying the address i.e. setenv ip6addr 2001:db8::1/64.
>>
>>  include/env_callback.h |  8 +++++
>>  include/env_flags.h    |  9 +++++
>>  net/Kconfig            |  5 +++
>>  net/Makefile           |  1 +
>>  net/net6.c             | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 112 insertions(+)
>>  create mode 100644 net/net6.c
>>
>> diff --git a/include/env_callback.h b/include/env_callback.h
>> index 90b95b5..97e245b 100644
>> --- a/include/env_callback.h
>> +++ b/include/env_callback.h
>> @@ -60,6 +60,13 @@
>>  #define NET_CALLBACKS
>>  #endif
>>
>> +#ifdef CONFIG_NET6
>> +#define NET6_CALLBACKS \
>> +       "ip6addr:ip6addr," \
>> +       "serverip6:serverip6,"
>
> Any reason why gateway6 is not in this list? Seems like an oversight
> since you define a handler for it below.
>

Yep my bad. Just an omission on my part.

>> +#else
>> +#define NET6_CALLBACKS
>> +#endif
>>  /*
>>   * This list of callback bindings is static, but may be overridden by defining
>>   * a new association in the ".callbacks" environment variable.
>> @@ -68,6 +75,7 @@
>>         ENV_DOT_ESCAPE ENV_FLAGS_VAR ":flags," \
>>         "baudrate:baudrate," \
>>         NET_CALLBACKS \
>> +       NET6_CALLBACKS \
>>         "loadaddr:loadaddr," \
>>         SILENT_CALLBACK \
>>         SPLASHIMAGE_CALLBACK \
>> diff --git a/include/env_flags.h b/include/env_flags.h
>> index 8823fb9..bf93f15 100644
>> --- a/include/env_flags.h
>> +++ b/include/env_flags.h
>> @@ -65,6 +65,14 @@ enum env_flags_varaccess {
>>  #define NET_FLAGS
>>  #endif
>>
>> +#ifdef CONFIG_NET6
>> +#define NET6_FLAGS \
>> +       "ip6addr:s," \
>> +       "serverip6:s,"
>> +#else
>> +#define NET6_FLAGS
>> +#endif
>> +
>>  #ifndef CONFIG_ENV_OVERWRITE
>>  #define SERIAL_FLAGS "serial#:so,"
>>  #else
>> @@ -74,6 +82,7 @@ enum env_flags_varaccess {
>>  #define ENV_FLAGS_LIST_STATIC \
>>         ETHADDR_FLAGS \
>>         NET_FLAGS \
>> +       NET6_FLAGS \
>>         SERIAL_FLAGS \
>>         CONFIG_ENV_FLAGS_LIST_STATIC
>>
>> diff --git a/net/Kconfig b/net/Kconfig
>> index a44a783..bacd914 100644
>> --- a/net/Kconfig
>> +++ b/net/Kconfig
>> @@ -32,4 +32,9 @@ config NET_TFTP_VARS
>>           If unset, timeout and maximum are hard-defined as 1 second
>>           and 10 timouts per TFTP transfer.
>>
>> +config NET6
>> +       bool "IPv6 support"
>> +       help
>> +         Support for IPv6
>> +
>>  endif   # if NET
>> diff --git a/net/Makefile b/net/Makefile
>> index e9cc8ad..6da8019 100644
>> --- a/net/Makefile
>> +++ b/net/Makefile
>> @@ -20,3 +20,4 @@ obj-$(CONFIG_CMD_PING) += ping.o
>>  obj-$(CONFIG_CMD_RARP) += rarp.o
>>  obj-$(CONFIG_CMD_SNTP) += sntp.o
>>  obj-$(CONFIG_CMD_NET)  += tftp.o
>> +obj-$(CONFIG_NET6)     += net6.o
>> diff --git a/net/net6.c b/net/net6.c
>> new file mode 100644
>> index 0000000..f778cef
>> --- /dev/null
>> +++ b/net/net6.c
>> @@ -0,0 +1,89 @@
>> +/*
>> + * Simple IPv6 network layer implementation.
>> + *
>> + * Based and/or adapted from the IPv4 network layer in net.[hc]
>> + *
>> + * (C) Copyright 2013 Allied Telesis Labs NZ
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <environment.h>
>> +#include <malloc.h>
>> +#include <net.h>
>> +#include <net6.h>
>> +#include "ndisc.h"
>> +
>> +/* Our gateway's IPv6 address */
>> +struct in6_addr net_gateway6 = ZERO_IPV6_ADDR;
>> +/* Our IPv6 addr (0 = unknown) */
>> +struct in6_addr net_ip6 = ZERO_IPV6_ADDR;
>> +/* set server IPv6 addr (0 = unknown) */
>> +struct in6_addr net_server_ip6 = ZERO_IPV6_ADDR;
>> +/* The prefix length of our network */
>> +u_int32_t net_prefix_length;
>> +
>> +static int on_ip6addr(const char *name, const char *value, enum env_op op,
>> +                     int flags)
>> +{
>> +       char *v, *s, *strcopy;
>> +       int i;
>> +
>> +       if (flags & H_PROGRAMMATIC)
>> +               return 0;
>> +
>> +       if (op == env_op_delete) {
>> +               net_prefix_length = 0;
>> +               net_copy_ip6(&net_ip6, &net_null_addr_ip6);
>> +               return 0;
>> +       }
>> +
>> +       strcopy = strdup(value);
>> +       if (strcopy == NULL)
>> +               return -1;
>
> +               return -ENOMEM;
>
>> +
>> +       net_prefix_length = 128;
>
> Does this imply that you are allowed to omit this part of the address
> now? I assume you are not allowed to omit it, hence the proposed code
> below.
>

That was my indention but in practice I think you'd always want to
specify a prefix length. Not specifying one is _probably_ a user
error.

>> +       i = 0;
>> +       s = strcopy;
>
> --------------------
>> +       while (s) {
>> +               v = strsep(&s, "/");
>> +               if (!v)
>> +                       break;
>> +
>> +               switch (i++) {
>> +               case 0:
>> +                       string_to_ip6(v, &net_ip6);
>> +                       break;
>> +               case 1:
>> +                       net_prefix_length = simple_strtoul(v, NULL, 10);
>> +                       break;
>> +               default:
>> +                       break;
>> +               }
>> +       }
>> +       free(strcopy);
>> +
>> +       return 0;
>> +}
> --------------------
> This is a bit convoluted.

Agreed.

>
> How about this:
> +       int retval = 0;
> +
> +       if (s) {
> +               v = strsep(&s, "/");
> +               if (!v) {
> +                       retval = -EINVAL;
> +                       goto err_out;
> +               }
> +
> +               string_to_ip6(v, &net_ip6);
> +
> +               v = strsep(&s, "/");
> +               if (!v) {
> +                       retval = -EINVAL;
> +                       goto err_out;
> +               }
> +
> +               net_prefix_length = simple_strtoul(v, NULL, 10);
> +       }
> +       err_out:
> +       free(strcopy);
> +
> +       return retval;
> +}
>
> It is now linear and will return an error code when parsing fails,
> which will prevent the variable from being changed.
>

Looks good to me. I'll work it into the next re-roll of this series.

>> +U_BOOT_ENV_CALLBACK(ip6addr, on_ip6addr);
>> +
>> +static int on_gatewayip6(const char *name, const char *value, enum env_op op,
>> +                        int flags)
>> +{
>> +       if (flags & H_PROGRAMMATIC)
>> +               return 0;
>> +
>> +       return string_to_ip6(value, &net_gateway6);
>> +}
>> +U_BOOT_ENV_CALLBACK(gatewayip6, on_gatewayip6);
>> +
>> +static int on_serverip6(const char *name, const char *value, enum env_op op,
>> +                       int flags)
>> +{
>> +       if (flags & H_PROGRAMMATIC)
>> +               return 0;
>> +
>> +       return string_to_ip6(value, &net_server_ip6);
>> +}
>> +U_BOOT_ENV_CALLBACK(serverip6, on_serverip6);
>> --
>> 2.5.3
>>
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot


More information about the U-Boot mailing list