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

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@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

Hi Chris,

On Mon, Nov 9, 2015 at 1:38 AM, Chris Packham <judge.packham@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@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.

> +#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.

> +       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.

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.

> +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@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

On Tue, Nov 24, 2015 at 2:06 PM, Joe Hershberger
<joe.hershberger@gmail.com> wrote:
> Hi Chris,
>
> On Mon, Nov 9, 2015 at 1:38 AM, Chris Packham <judge.packham@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@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@lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot