Re: [U-Boot] [RFC PATCH 5/8] net: ipv6 support
Hi Chris, On Tue, Nov 3, 2015 at 2:54 PM, Chris Packham wrote: > On Wed, Nov 4, 2015 at 8:41 AM, Joe Hershberger > wrote: > > >>> I'm actually testing with x86 on QEMU so I think LE is all good. I'll >>> remove the comment. >> >> Great. Have you done any testing in sandbox? I'd really like to see >> unit tests go in as part of this series. >> > > I'll take a look. For the parsing etc that should be doable. I think I > may run into problems with the linux network stack getting in the way > for some of the link-local handling. Are there any existing tests for > the networking code? Yes, there is test/dm/eth.c that does some pinging to test out the Ethernet stack. Also, sandbox has drivers/net/sandbox-raw.c that allows sandbox to interact with the host network adapter. See board/sandbox/README.sandbox Cheers, -Joe ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 5/8] net: ipv6 support
On Wed, Nov 4, 2015 at 8:41 AM, Joe Hershberger wrote: >> I'm actually testing with x86 on QEMU so I think LE is all good. I'll >> remove the comment. > > Great. Have you done any testing in sandbox? I'd really like to see > unit tests go in as part of this series. > I'll take a look. For the parsing etc that should be doable. I think I may run into problems with the linux network stack getting in the way for some of the link-local handling. Are there any existing tests for the networking code? ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [RFC PATCH 5/8] net: ipv6 support
Hi Chris, On Tue, Nov 3, 2015 at 4:11 AM, Chris Packham wrote: > Hi Joe, > > I've answered a few questions below. I'll address your comments more > completely before sending another round next week (or I can sit on it > for longer if you want me to give you some breathing room). Feel free to send it when you're ready. > On Tue, Nov 3, 2015 at 9:43 AM, Joe Hershberger > wrote: >> Hi Chris, >> >> On Mon, Oct 12, 2015 at 2:43 AM, Chris Packham >> wrote: >>> Adds basic support for IPv6. Neighbor discovery and ping6 are the only >>> things supported at the moment. >>> >>> Helped-by: Hanna Hawa [endian & alignment fixes] >>> Signed-off-by: Chris Packham >>> --- >>> Now we have something functional. With this you can do something like >>> 'setenv ipaddr6 3ffe::2' and 'ping6 3ffe::1' should work. >>> >>> I seem to have a problem that when you send a ping6 for a non-existent >>> address that ends up stuck and the next non-ipv6 net operation tries to >>> resolve it. I suspect this is because the pending neighbor discovery >>> information isn't cleaned up properly, I need to look into that. >>> >>> The environment variable prefixlength6 is a bit fiddly. No-one uses a >>> netmask6 (it'd mean a lot of ::...) it's almost always done by >>> including the prefix length in the address (e.g. ip addr add >>> 2001:db8::1/64) I'm contemplating adopting that syntax and dropping >>> prefixlength6. >>> >>> This patch is bigger than I'd like it to be but I'm not sure how to >>> split it up and keep the parts build able. >> >> It is pretty huge. It's taken me a while to get through it. >> > > Thanks for making the effort to review. I realise it's pretty huge and > I'll put some effort into splitting it futher. One obvious thing that > could be split out is the new environment variables. Sounds good. >>> common/Kconfig | 15 ++ >>> common/cmd_net.c | 28 >>> include/env_callback.h | 9 ++ >>> include/env_flags.h| 10 ++ >>> include/net.h | 5 +- >>> include/net6.h | 212 >>> net/Kconfig| 5 + >>> net/Makefile | 3 + >>> net/ndisc.c| 269 +++ >>> net/ndisc.h| 27 >>> net/net.c | 36 - >>> net/net6.c | 375 >>> + >>> net/ping6.c| 111 +++ >>> 13 files changed, 1102 insertions(+), 3 deletions(-) >>> create mode 100644 net/ndisc.c >>> create mode 100644 net/ndisc.h >>> create mode 100644 net/net6.c >>> create mode 100644 net/ping6.c >>> >>> diff --git a/common/Kconfig b/common/Kconfig >>> index 2c42b8e..c72563d 100644 >>> --- a/common/Kconfig >>> +++ b/common/Kconfig >>> @@ -389,6 +389,15 @@ config CMD_NET >>> bootp - boot image via network using BOOTP/TFTP protocol >>> tftpboot - boot image via network using TFTP protocol >>> >>> +config CMD_NET6 >>> + bool "ipv6 commands" >>> + select NET >>> + select NET6 >>> + default n >>> + help >>> + IPv6 network commands >>> + tftpboot6 - boot image via network using TFTP protocol >> >> This is added in the next patch, so should probably move there. >> > > Will do. > >>> + >>> config CMD_TFTPPUT >>> bool "tftp put" >>> help >>> @@ -420,6 +429,12 @@ config CMD_PING >>> help >>> Send ICMP ECHO_REQUEST to network host >>> >>> +config CMD_PING6 >>> + bool "ping6" >>> + depends on CMD_NET6 >>> + help >>> + Send ICMPv6 ECHO_REQUEST to network host >> >> What makes ping inseparable from the core support? >> > > Mainly testing, I can't test the core code without ping. But I can see > that from a patch submission point of few splitting it out will make > review easier. One simple way to prove out the core without ping would maybe be to rebase and switch tftp with ping and make sure that tftp works without ping. Then you at least know you have enough, but you won't know if you left in more than you need. >>> + >>> config CMD_CDP >>> bool "cdp" >>> help --8< snip 8<-- >>> diff --git a/include/env_callback.h b/include/env_callback.h >>> index 90b95b5..9027f3f 100644 >>> --- a/include/env_callback.h >>> +++ b/include/env_callback.h >>> @@ -60,6 +60,14 @@ >>> #define NET_CALLBACKS >>> #endif >>> >>> +#ifdef CONFIG_NET6 >>> +#define NET6_CALLBACKS \ >>> + "ip6addr:ip6addr," \ >>> + "serverip6:serverip6," \ >>> + "prefixlength6:prefixlength6," >> >> I like the other nomenclature better as well (included in the address). >> > > I've actually already implemented the code to include the prefixlength > in the address and it makes things a lot more usable (the parsing code > is a bit more complicated). Sounds good. >>> +#else >>> +#define NET6_CALLBACKS >>> +#endif >>> /* >>> * This list of callback bindings is static, but may be overridden by >>> defi
Re: [U-Boot] [RFC PATCH 5/8] net: ipv6 support
Hi Joe,
I've answered a few questions below. I'll address your comments more
completely before sending another round next week (or I can sit on it
for longer if you want me to give you some breathing room).
On Tue, Nov 3, 2015 at 9:43 AM, Joe Hershberger
wrote:
> Hi Chris,
>
> On Mon, Oct 12, 2015 at 2:43 AM, Chris Packham
> wrote:
>> Adds basic support for IPv6. Neighbor discovery and ping6 are the only
>> things supported at the moment.
>>
>> Helped-by: Hanna Hawa [endian & alignment fixes]
>> Signed-off-by: Chris Packham
>> ---
>> Now we have something functional. With this you can do something like
>> 'setenv ipaddr6 3ffe::2' and 'ping6 3ffe::1' should work.
>>
>> I seem to have a problem that when you send a ping6 for a non-existent
>> address that ends up stuck and the next non-ipv6 net operation tries to
>> resolve it. I suspect this is because the pending neighbor discovery
>> information isn't cleaned up properly, I need to look into that.
>>
>> The environment variable prefixlength6 is a bit fiddly. No-one uses a
>> netmask6 (it'd mean a lot of ::...) it's almost always done by
>> including the prefix length in the address (e.g. ip addr add
>> 2001:db8::1/64) I'm contemplating adopting that syntax and dropping
>> prefixlength6.
>>
>> This patch is bigger than I'd like it to be but I'm not sure how to
>> split it up and keep the parts build able.
>
> It is pretty huge. It's taken me a while to get through it.
>
Thanks for making the effort to review. I realise it's pretty huge and
I'll put some effort into splitting it futher. One obvious thing that
could be split out is the new environment variables.
>> common/Kconfig | 15 ++
>> common/cmd_net.c | 28
>> include/env_callback.h | 9 ++
>> include/env_flags.h| 10 ++
>> include/net.h | 5 +-
>> include/net6.h | 212
>> net/Kconfig| 5 +
>> net/Makefile | 3 +
>> net/ndisc.c| 269 +++
>> net/ndisc.h| 27
>> net/net.c | 36 -
>> net/net6.c | 375
>> +
>> net/ping6.c| 111 +++
>> 13 files changed, 1102 insertions(+), 3 deletions(-)
>> create mode 100644 net/ndisc.c
>> create mode 100644 net/ndisc.h
>> create mode 100644 net/net6.c
>> create mode 100644 net/ping6.c
>>
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 2c42b8e..c72563d 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -389,6 +389,15 @@ config CMD_NET
>> bootp - boot image via network using BOOTP/TFTP protocol
>> tftpboot - boot image via network using TFTP protocol
>>
>> +config CMD_NET6
>> + bool "ipv6 commands"
>> + select NET
>> + select NET6
>> + default n
>> + help
>> + IPv6 network commands
>> + tftpboot6 - boot image via network using TFTP protocol
>
> This is added in the next patch, so should probably move there.
>
Will do.
>> +
>> config CMD_TFTPPUT
>> bool "tftp put"
>> help
>> @@ -420,6 +429,12 @@ config CMD_PING
>> help
>> Send ICMP ECHO_REQUEST to network host
>>
>> +config CMD_PING6
>> + bool "ping6"
>> + depends on CMD_NET6
>> + help
>> + Send ICMPv6 ECHO_REQUEST to network host
>
> What makes ping inseparable from the core support?
>
Mainly testing, I can't test the core code without ping. But I can see
that from a patch submission point of few splitting it out will make
review easier.
>> +
>> config CMD_CDP
>> bool "cdp"
>> help
>> diff --git a/common/cmd_net.c b/common/cmd_net.c
>> index b2f3c7b..271f91d 100644
>> --- a/common/cmd_net.c
>> +++ b/common/cmd_net.c
>> @@ -11,6 +11,7 @@
>> #include
>> #include
>> #include
>> +#include
>>
>> static int netboot_common(enum proto_t, cmd_tbl_t *, int, char * const []);
>>
>> @@ -284,6 +285,33 @@ U_BOOT_CMD(
>> );
>> #endif
>>
>> +#ifdef CONFIG_CMD_PING6
>> +int do_ping6(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> +{
>> + if (argc < 2)
>> + return -1;
>> +
>> + if (string_to_ip6(argv[1], &net_ping_ip6) != 0)
>> + return CMD_RET_USAGE;
>> +
>> + if (net_loop(PING6) < 0) {
>> + printf("ping6 failed; host %pI6c is not alive\n",
>> + &net_ping_ip6);
>> + return 1;
>> + }
>> +
>> + printf("host %pI6c is alive\n", &net_ping_ip6);
>> +
>> + return 0;
>> +}
>> +
>> +U_BOOT_CMD(
>> + ping6, 2, 1, do_ping6,
>> + "send ICMPv6 ECHO_REQUEST to network host",
>> + "pingAddress"
>> +);
>> +#endif /* CONFIG_CMD_PING6 */
>> +
>> #if defined(CONFIG_CMD_CDP)
>>
>> static void cdp_update_env(void)
>> diff --git a/include/env_callback.h b/include/env_callback.h
>> index 90b95b5..9027f3f 100644
>> --- a/include/env_callback.h
>> +++
Re: [U-Boot] [RFC PATCH 5/8] net: ipv6 support
Hi Chris,
On Mon, Oct 12, 2015 at 2:43 AM, Chris Packham wrote:
> Adds basic support for IPv6. Neighbor discovery and ping6 are the only
> things supported at the moment.
>
> Helped-by: Hanna Hawa [endian & alignment fixes]
> Signed-off-by: Chris Packham
> ---
> Now we have something functional. With this you can do something like
> 'setenv ipaddr6 3ffe::2' and 'ping6 3ffe::1' should work.
>
> I seem to have a problem that when you send a ping6 for a non-existent
> address that ends up stuck and the next non-ipv6 net operation tries to
> resolve it. I suspect this is because the pending neighbor discovery
> information isn't cleaned up properly, I need to look into that.
>
> The environment variable prefixlength6 is a bit fiddly. No-one uses a
> netmask6 (it'd mean a lot of ::...) it's almost always done by
> including the prefix length in the address (e.g. ip addr add
> 2001:db8::1/64) I'm contemplating adopting that syntax and dropping
> prefixlength6.
>
> This patch is bigger than I'd like it to be but I'm not sure how to
> split it up and keep the parts build able.
It is pretty huge. It's taken me a while to get through it.
> common/Kconfig | 15 ++
> common/cmd_net.c | 28
> include/env_callback.h | 9 ++
> include/env_flags.h| 10 ++
> include/net.h | 5 +-
> include/net6.h | 212
> net/Kconfig| 5 +
> net/Makefile | 3 +
> net/ndisc.c| 269 +++
> net/ndisc.h| 27
> net/net.c | 36 -
> net/net6.c | 375
> +
> net/ping6.c| 111 +++
> 13 files changed, 1102 insertions(+), 3 deletions(-)
> create mode 100644 net/ndisc.c
> create mode 100644 net/ndisc.h
> create mode 100644 net/net6.c
> create mode 100644 net/ping6.c
>
> diff --git a/common/Kconfig b/common/Kconfig
> index 2c42b8e..c72563d 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -389,6 +389,15 @@ config CMD_NET
> bootp - boot image via network using BOOTP/TFTP protocol
> tftpboot - boot image via network using TFTP protocol
>
> +config CMD_NET6
> + bool "ipv6 commands"
> + select NET
> + select NET6
> + default n
> + help
> + IPv6 network commands
> + tftpboot6 - boot image via network using TFTP protocol
This is added in the next patch, so should probably move there.
> +
> config CMD_TFTPPUT
> bool "tftp put"
> help
> @@ -420,6 +429,12 @@ config CMD_PING
> help
> Send ICMP ECHO_REQUEST to network host
>
> +config CMD_PING6
> + bool "ping6"
> + depends on CMD_NET6
> + help
> + Send ICMPv6 ECHO_REQUEST to network host
What makes ping inseparable from the core support?
> +
> config CMD_CDP
> bool "cdp"
> help
> diff --git a/common/cmd_net.c b/common/cmd_net.c
> index b2f3c7b..271f91d 100644
> --- a/common/cmd_net.c
> +++ b/common/cmd_net.c
> @@ -11,6 +11,7 @@
> #include
> #include
> #include
> +#include
>
> static int netboot_common(enum proto_t, cmd_tbl_t *, int, char * const []);
>
> @@ -284,6 +285,33 @@ U_BOOT_CMD(
> );
> #endif
>
> +#ifdef CONFIG_CMD_PING6
> +int do_ping6(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> +{
> + if (argc < 2)
> + return -1;
> +
> + if (string_to_ip6(argv[1], &net_ping_ip6) != 0)
> + return CMD_RET_USAGE;
> +
> + if (net_loop(PING6) < 0) {
> + printf("ping6 failed; host %pI6c is not alive\n",
> + &net_ping_ip6);
> + return 1;
> + }
> +
> + printf("host %pI6c is alive\n", &net_ping_ip6);
> +
> + return 0;
> +}
> +
> +U_BOOT_CMD(
> + ping6, 2, 1, do_ping6,
> + "send ICMPv6 ECHO_REQUEST to network host",
> + "pingAddress"
> +);
> +#endif /* CONFIG_CMD_PING6 */
> +
> #if defined(CONFIG_CMD_CDP)
>
> static void cdp_update_env(void)
> diff --git a/include/env_callback.h b/include/env_callback.h
> index 90b95b5..9027f3f 100644
> --- a/include/env_callback.h
> +++ b/include/env_callback.h
> @@ -60,6 +60,14 @@
> #define NET_CALLBACKS
> #endif
>
> +#ifdef CONFIG_NET6
> +#define NET6_CALLBACKS \
> + "ip6addr:ip6addr," \
> + "serverip6:serverip6," \
> + "prefixlength6:prefixlength6,"
I like the other nomenclature better as well (included in the address).
> +#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 +76,7 @@
> ENV_DOT_ESCAPE ENV_FLAGS_VAR ":flags," \
> "baudrate:baudrate," \
> NET_CALLBACKS \
> + NET6_CALLBACKS \
> "loadaddr:loadaddr," \
> SILENT_CALLBACK \
> SPLASHIMAGE_CALLBACK \
> diff
[U-Boot] [RFC PATCH 5/8] net: ipv6 support
Adds basic support for IPv6. Neighbor discovery and ping6 are the only
things supported at the moment.
Helped-by: Hanna Hawa [endian & alignment fixes]
Signed-off-by: Chris Packham
---
Now we have something functional. With this you can do something like
'setenv ipaddr6 3ffe::2' and 'ping6 3ffe::1' should work.
I seem to have a problem that when you send a ping6 for a non-existent
address that ends up stuck and the next non-ipv6 net operation tries to
resolve it. I suspect this is because the pending neighbor discovery
information isn't cleaned up properly, I need to look into that.
The environment variable prefixlength6 is a bit fiddly. No-one uses a
netmask6 (it'd mean a lot of ::...) it's almost always done by
including the prefix length in the address (e.g. ip addr add
2001:db8::1/64) I'm contemplating adopting that syntax and dropping
prefixlength6.
This patch is bigger than I'd like it to be but I'm not sure how to
split it up and keep the parts build able.
common/Kconfig | 15 ++
common/cmd_net.c | 28
include/env_callback.h | 9 ++
include/env_flags.h| 10 ++
include/net.h | 5 +-
include/net6.h | 212
net/Kconfig| 5 +
net/Makefile | 3 +
net/ndisc.c| 269 +++
net/ndisc.h| 27
net/net.c | 36 -
net/net6.c | 375 +
net/ping6.c| 111 +++
13 files changed, 1102 insertions(+), 3 deletions(-)
create mode 100644 net/ndisc.c
create mode 100644 net/ndisc.h
create mode 100644 net/net6.c
create mode 100644 net/ping6.c
diff --git a/common/Kconfig b/common/Kconfig
index 2c42b8e..c72563d 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -389,6 +389,15 @@ config CMD_NET
bootp - boot image via network using BOOTP/TFTP protocol
tftpboot - boot image via network using TFTP protocol
+config CMD_NET6
+ bool "ipv6 commands"
+ select NET
+ select NET6
+ default n
+ help
+ IPv6 network commands
+ tftpboot6 - boot image via network using TFTP protocol
+
config CMD_TFTPPUT
bool "tftp put"
help
@@ -420,6 +429,12 @@ config CMD_PING
help
Send ICMP ECHO_REQUEST to network host
+config CMD_PING6
+ bool "ping6"
+ depends on CMD_NET6
+ help
+ Send ICMPv6 ECHO_REQUEST to network host
+
config CMD_CDP
bool "cdp"
help
diff --git a/common/cmd_net.c b/common/cmd_net.c
index b2f3c7b..271f91d 100644
--- a/common/cmd_net.c
+++ b/common/cmd_net.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
static int netboot_common(enum proto_t, cmd_tbl_t *, int, char * const []);
@@ -284,6 +285,33 @@ U_BOOT_CMD(
);
#endif
+#ifdef CONFIG_CMD_PING6
+int do_ping6(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
+{
+ if (argc < 2)
+ return -1;
+
+ if (string_to_ip6(argv[1], &net_ping_ip6) != 0)
+ return CMD_RET_USAGE;
+
+ if (net_loop(PING6) < 0) {
+ printf("ping6 failed; host %pI6c is not alive\n",
+ &net_ping_ip6);
+ return 1;
+ }
+
+ printf("host %pI6c is alive\n", &net_ping_ip6);
+
+ return 0;
+}
+
+U_BOOT_CMD(
+ ping6, 2, 1, do_ping6,
+ "send ICMPv6 ECHO_REQUEST to network host",
+ "pingAddress"
+);
+#endif /* CONFIG_CMD_PING6 */
+
#if defined(CONFIG_CMD_CDP)
static void cdp_update_env(void)
diff --git a/include/env_callback.h b/include/env_callback.h
index 90b95b5..9027f3f 100644
--- a/include/env_callback.h
+++ b/include/env_callback.h
@@ -60,6 +60,14 @@
#define NET_CALLBACKS
#endif
+#ifdef CONFIG_NET6
+#define NET6_CALLBACKS \
+ "ip6addr:ip6addr," \
+ "serverip6:serverip6," \
+ "prefixlength6:prefixlength6,"
+#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 +76,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..6e1891d 100644
--- a/include/env_flags.h
+++ b/include/env_flags.h
@@ -65,6 +65,15 @@ enum env_flags_varaccess {
#define NET_FLAGS
#endif
+#ifdef CONFIG_CMD_NET6
+#define NET6_FLAGS \
+ "ip6addr:s," \
+ "serverip6:s," \
+ "prefixlength6:d,"
+#else
+#define NET6_FLAGS
+#endif
+
#ifndef CONFIG_ENV_OVERWRITE
#define SERIAL_FLAGS "serial#:so,"
#else
@@ -74,6 +83,7 @@ enum env_flags_varaccess {
#define ENV_FLAGS_LIST_STATIC \
ETHADDR_FLAGS \
NET_FLAGS \
+ NET6_FLAGS \
SERIAL_FLAGS \
