Support cellular IPv4v6 dual stack in LWIP #4911

mikaleppanen · 2017-08-15T10:18:46Z

Description

Cellular IPv4v6 dual stack support to lwip.

Merged ipv6 address auto negotiation corrections from lwip master branch, which prevented ipv6 cellular connection from working (lwip did not work correctly, if RA prefix had auto negotiation flag set, but on-link flag was not set).
Made correction to lwip to disable ipv6 neighbor solidification probes, if link layer address is not available.
Disabled duplicated address detection for cellular ipv6.
If cellular dual stack support is enabled, tries first cellular context activation for both ipv4v6, and if context activation is not successful, falls back to ipv4 context.
If context is activated for ipv4v6, makes PPP negotiation with both IPCP and IPV6CP protocols, and configures both ipv4 and ipv6 addresses.
If context is only for ipv4, makes PPP negotiation with IPCP protocol, and configures ipv4 address.
If both ipv4 and ipv6 addresses are configured, when DNS address resolution is made, tries to resolve first the preferred stacks address. If address resolution fails, tries to resolve non-preferred stack address.
Added trace dump function that can be used to trace PPP, ipv4 and ipv6 packets in format that can be imported to Wireshark.
Moved lwip memory pools to second memory bank for UBLOX_C027.
Corrected lwip random and tcp isn not to include configuration headers.

Status

READY

Migrations

YES

nsapi_ppp.h function nsapi_ppp_connect(): Added "const nsapi_ip_stack_t stack" parameter
-> can be used in dual stack case to set either ipv4v6 or ipv4 stack, for non-dual
stack case use value DEFAULT_STACK.

ppp_lwip.h function ppp_lwip_if_init(): Added "const nsapi_ip_stack_t stack" parameter
-> can be used in dual stack case to set either ipv4v6 or ipv4 stack, for non-dual
stack case use value DEFAULT_STACK.

lwip_stack.h function mbed_lwip_bringup_2(): Added "const nsapi_ip_stack_t stack" parameter
-> can be used in dual stack case to set either ipv4v6 or ipv4 stack, for non-dual
stack case use value DEFAULT_STACK.

Related PRs

None

Todos

Tests
Documentation

Deploy notes

None

Steps to test or reproduce

None

mikaleppanen · 2017-08-15T10:20:22Z

@SeppoTakalo @kjbracey-arm @hasnainvirk please review.

hasnainvirk · 2017-08-15T10:54:41Z

features/FEATURE_LWIP/lwip-interface/lwip_stack.c

-    ip6_addr_set_allnodes_linklocal(&ip6_allnodes_ll);
-    lwip_netif.mld_mac_filter(&lwip_netif, &ip6_allnodes_ll, NETIF_ADD_MAC_FILTER);
-  }
+        /*


just moved the indentation when if was added, no problem there.

hasnainvirk · 2017-08-15T11:01:03Z

features/netsocket/cellular/generic_modem_driver/PPPCellularInterface.cpp

@@ -262,6 +262,7 @@ PPPCellularInterface::PPPCellularInterface(FileHandle *fh, bool debug)
    _pwd = NULL;
    _fh = fh;
    _debug_trace_on = debug;
+    _stack = IPV4_STACK;


Shouldn't this be IPV4V6_STACK ?

I set the value to default stack. In that case flag configuration defines the stack.

hasnainvirk · 2017-08-15T11:03:37Z

features/netsocket/nsapi_ppp.h

 *
 *  @return             0 on success, negative error code on failure
 */
-nsapi_error_t nsapi_ppp_connect(FileHandle *stream, Callback<void(nsapi_error_t)> status_cb=0, const char *uname=0, const char *pwd=0);
+nsapi_error_t nsapi_ppp_connect(FileHandle *stream, Callback<void(nsapi_error_t)> status_cb=0, const char *uname=0, const char *pwd=0, const nsapi_ip_stack_t stack=IPV4_STACK);


Why default to IPv4 ? Isn't the whole point to have the stack dual configurable and choose at runtime which one is available ?

Set also this to default stack.

hasnainvirk · 2017-08-15T11:32:00Z

features/FEATURE_LWIP/lwip-interface/lwipopts.h

-#define PRINTPKT_SUPPORT            0
+#define PAP_SUPPORT                    0
+#define VJ_SUPPORT                     0
+#define PRINTPKT_SUPPORT               0


moved the indentation when longer define was added, no problem there.

hasnainvirk · 2017-08-15T11:32:13Z

features/FEATURE_LWIP/lwip-interface/lwipopts.h

-#define IP_SOF_BROADCAST            0
-#define IP_SOF_BROADCAST_RECV       0
+#define IP_SOF_BROADCAST               0
+#define IP_SOF_BROADCAST_RECV          0


hasnainvirk · 2017-08-15T11:32:22Z

features/FEATURE_LWIP/lwip-interface/lwipopts.h

-#define MAXNAMELEN                  64     /* max length of hostname or name for auth */
-#define MAXSECRETLEN                64
+#define MAXNAMELEN                     64     /* max length of hostname or name for auth */
+#define MAXSECRETLEN                   64


hasnainvirk · 2017-08-15T11:33:56Z

features/FEATURE_LWIP/lwip-interface/mbed_lib.json

@@ -16,7 +16,11 @@
        "addr-timeout": {


Is this left out for backward compatibility ? Why and how ? Isn't the lwipopts.h also updated ?

"addr-timeout" is the amount of time we wait for preferred system address in case we have only non-preferred system address available.
"both-addr-timeout" is the time to wait that both systems addresses are available.

So the flags have different meaning and both are valid.

hasnainvirk · 2017-08-15T11:34:07Z

features/FEATURE_LWIP/lwip-interface/lwipopts.h


-#define MAXNAMELEN                  64     /* max length of hostname or name for auth */
-#define MAXSECRETLEN                64
+#define MAXNAMELEN                     64     /* max length of hostname or name for auth */


hasnainvirk · 2017-08-15T11:34:16Z

features/FEATURE_LWIP/lwip-interface/lwipopts.h

-#define PAP_SUPPORT                 0
-#define VJ_SUPPORT                  0
-#define PRINTPKT_SUPPORT            0
+#define PAP_SUPPORT                    0


hasnainvirk

There will be issues while using dual stack config, as 'add_dns_addr()' function is still not changed in relation to this PR. This particular function calls dns_setserver() internally with same index , so essentially you will overwrite your previous config.

mikaleppanen · 2017-08-16T06:42:04Z

"Hasnain: there will be issues while using dual stack config, as 'add_dns_addr()".

Yes, this could be improved maybe so that in dual stack system the DNS resolution could fallback to non-preferred system if it does not for some reason work in more preferred. I'll test this a little to see
how it works.

There is no absolute need to modify the function since the IP address get function (mbed_lwip_get_ip_addr()) fetches either the preferred systems IP address or if not available the non-preferred. So a either ipv4 or ipv6 DNS server (google) will be always added to list if network has not provided DNS server.

mikaleppanen · 2017-08-16T06:43:09Z

Jenkins is failing in interface up/down/up/down test since variable holding the address status is not reset on bring down. I'll correct that.

mikaleppanen · 2017-08-18T06:22:06Z

Review defects corrected.

I improved the DNS address addition functionality in case network does not provide DNS server addresses. Now in dual mode both ipv4 and ipv6 DNS addresses are added in order of preference defined by flag configuration.

Corrected IAR memory pools relocation for UBLOX_C027.

Corrected interface up/down/up/down error detected by Jenkins.

hasnainvirk · 2017-08-18T09:37:48Z

@mikaleppanen I like the changes you did to place some lwip stuff on .ethusbram section for IAR compiler. Great work.
Can you please check why CI is failing ? Is this related to RAAS unavailability or a genuine case ?

mikaleppanen · 2017-08-18T09:54:03Z

Yes, ble raas is down. Tools team is aware about it.

Merged lwip 2.0.2 stable path 1 from https://github.com/ARMmbed/lwip Patch allows lwip to do autonomous address configuration for prefixes that are not on-link. ad7cf16 Add David's IPv6 improvements to CHANGELOG af9d783 Fix (bogus) MSVC 2010 warning about uninitialized variable usage in ip6.c It's wrong because the variables are initialized during first loop iteration due to best_addr == NULL 68358d7 nd6: cull destination cache on router removal 7323fa1 nd6: some work on basic RFC 4861 compliance 10eb2ca ip6: improve source address selection 6c06ecd ip6/nd6: route using on-link prefixes, not addresses 9f1714d nd6: improve router selection 2486b41 netif: more ip6 state changes invoke status callback 519d809 nd6: fix Duplicate Address Detection 9f3c6dd nd6: check link status before sending packets 8c761a2 nd6: improve address autoconfiguration support

mikaleppanen · 2017-08-22T07:18:53Z

Updated lwip not to add neighbor cache entry based on router advertisement if link layer address is not present (PPP case)

kjbracey · 2017-08-21T13:54:19Z

features/FEATURE_LWIP/lwip-interface/mbed_lib.json

@@ -16,7 +16,11 @@
        "addr-timeout": {
            "help": "On dual-stack system how long to wait preferred stack's address in seconds",
            "value": 5
-        },
+        },        
+        "both-addr-timeout": {


Not clear from help how this relates to the preferred wait - is it "parallel" or "series"? Could these defaults wait a total of 10, or 5? Having it be an additional wait (if it isn't) might be easier to conceptualise.

Maybe comment to say that it's not terribly meaningful to set both options. How about a boolean that changes what addr-timeout means - extra wait for preferred, or extra wait for all addresses. Make the help clear that is the extra wait on top of the initial wait for any address.

kjbracey · 2017-08-22T08:38:17Z

features/FEATURE_LWIP/lwip-interface/lwip/src/netif/ppp/lwip_auth.c

@@ -751,6 +751,10 @@ void link_established(ppp_pcb *pcb) {
    if (!doing_multilink) {
 	for (i = 0; (protp = protocols[i]) != NULL; ++i)
 	    if (protp->protocol != PPP_LCP
+#if PPP_IPV4_SUPPORT && PPP_IPV6_SUPPORT


This feels like it's in the wrong layer. Shouldn't a suppression check be in ip[v6]cp_lowerup?

kjbracey · 2017-08-22T08:57:51Z

features/FEATURE_LWIP/lwip-interface/lwip/src/core/ipv6/lwip_nd6.c

-    neighbor_cache[neighbor_index].state = ND6_INCOMPLETE;
-    neighbor_cache[neighbor_index].counter.probes_sent = 1;
-    nd6_send_neighbor_cache_probe(&neighbor_cache[neighbor_index], ND6_SEND_FLAG_MULTICAST_DEST);
+    if (netif->hwaddr_len) {


Suggested simple alternative - set state to STALE if interface has no HW address. I believe that should stay in STALE forever, without triggering probes. (Because the entries are not used, so don't transition to DELAY)

Added new configuration option "both-addr-timeout" that defines how long to wait that addresses from both systems become available.

…vailable

If both IPv4 and IPv6 are available, and DNS query fails for first system, second system is tried.

…text

mikaleppanen · 2017-08-22T10:26:28Z

@kjbracey-arm corrected defects.

mikaleppanen · 2017-08-23T06:29:35Z

I think all has approved this. @hasnainvirk can you update status review status. @theotherjimmy @0xc0170 can this progress?

0xc0170 · 2017-08-23T07:51:03Z

/morph test-nightly

0xc0170 · 2017-08-23T09:52:38Z

@mikaleppanen How this can be tested ? Is it referenced PR above to cliapp? Any new example (will cellular example get an update) ?

cc @MarceloSalazar @adbridge

mikaleppanen · 2017-08-23T09:57:48Z

I have tested this manually using cliapp tests (above pr) and mbed-os-example-client with MTS_DRAGONFLY_F411RE. Cellular example will be updated but currently we do not have echo server for ipv6 connectivity (ublox server works only for ipv4).

mbed-bot · 2017-08-23T11:16:30Z

Result: FAILURE

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1094

Build failed!

If DHCP or PPP does not provide DNS addresses, for dual stack, adds both ipv4 and ipv6 DNS addresses to DNS list.

mikaleppanen · 2017-08-23T13:54:44Z

Updated pull request. Changed memory pool areas that caused nightly tests to fail. Tested to compile with failing targets and tested Ethernet connectivity with LPC1768 and UBLOX_C027.

SeppoTakalo · 2017-08-24T13:02:36Z

@0xc0170 Please restart the test.

mikaleppanen · 2017-08-25T13:10:36Z

Could you retest this.

SeppoTakalo · 2017-08-28T07:32:06Z

@0xc0170 @adbridge Please, somebody restart the tests.

studavekar · 2017-08-28T20:36:22Z

/morph test-nightly

mbed-bot · 2017-08-29T21:17:55Z

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test-nightly

Output

mbed Build Number: 1134

All builds and test passed!

SeppoTakalo approved these changes Aug 15, 2017

View reviewed changes

hasnainvirk reviewed Aug 15, 2017

View reviewed changes

0xc0170 added the needs: work label Aug 17, 2017

theotherjimmy changed the title ~~lwip cellular IPv4v6 dual stack support~~ Support cellular IPv4v6 dual stack in LWIP Aug 21, 2017

theotherjimmy added needs: review and removed needs: work labels Aug 21, 2017

Mika Leppänen added 3 commits August 22, 2017 10:17

Removed random and tcp_isn modules header dependencies to lwip conf

23f8ac9

Added hex dump tracing to ipv6, ipv4 and ppp modules

41c647b

kjbracey reviewed Aug 22, 2017

View reviewed changes

Mika Leppänen added 5 commits August 22, 2017 13:14

Added dual Ipv4/Ipv6 stack support to lwip address configuration

2adbf72

Added new configuration option "both-addr-timeout" that defines how long to wait that addresses from both systems become available.

Corrected lwip not to send NS probes when link layer address is not a…

b1d41f9

…vailable

Added Ipv4/Ipv6 stack stack support to lwip DNS queries

457f940

If both IPv4 and IPv6 are available, and DNS query fails for first system, second system is tried.

Either ipv4 or ipv4v6 PPP and IP stacks are enabled based on 3GPP con…

dd346ec

…text

Corrected lwip to enable disabling of ipcp and ipv6cp protocols

942aec5

kjbracey approved these changes Aug 22, 2017

View reviewed changes

Mika Leppänen added 2 commits August 23, 2017 16:47

Relocated lwip memory pool on memory restricted platforms

0d5ac6e

Improved manual DNS address addition for dual stack case

32131b6

If DHCP or PPP does not provide DNS addresses, for dual stack, adds both ipv4 and ipv6 DNS addresses to DNS list.

adbridge added needs: CI and removed needs: review labels Aug 24, 2017

adbridge added ready for merge and removed needs: CI labels Aug 30, 2017

0xc0170 added the release-version: 5.6.0 label Sep 4, 2017

0xc0170 merged commit c0fe3b5 into ARMmbed:master Sep 4, 2017

0xc0170 removed the ready for merge label Sep 4, 2017

Support cellular IPv4v6 dual stack in LWIP #4911

Support cellular IPv4v6 dual stack in LWIP #4911

Uh oh!

Conversation

mikaleppanen commented Aug 15, 2017

Description

Status

Migrations

Related PRs

Todos

Deploy notes

Steps to test or reproduce

Uh oh!

mikaleppanen commented Aug 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hasnainvirk left a comment

Choose a reason for hiding this comment

Uh oh!

mikaleppanen commented Aug 16, 2017

Uh oh!

mikaleppanen commented Aug 16, 2017

Uh oh!

mikaleppanen commented Aug 18, 2017

Uh oh!

hasnainvirk commented Aug 18, 2017

Uh oh!

mikaleppanen commented Aug 18, 2017

Uh oh!

mikaleppanen commented Aug 22, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mikaleppanen commented Aug 22, 2017

Uh oh!

mikaleppanen commented Aug 23, 2017

Uh oh!

0xc0170 commented Aug 23, 2017

Uh oh!

0xc0170 commented Aug 23, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

0xc0170 commented Aug 23, 2017 •

edited

Loading

mikaleppanen commented Aug 23, 2017 •

edited

Loading