Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gnrc_ipv6_nib: don't auto-configure IPv6 w/o SLAAC on non-6LN interface #12513

Conversation

miri64
Copy link
Member

@miri64 miri64 commented Oct 20, 2019

Contribution description

When the NIB is compiled for 6LN mode (but not a 6LBR), the Stateless Address Autoconfiguration (SLAAC) functionality is disabled, as it is typically not required; see

#ifdef MODULE_GNRC_IPV6_NIB_6LR
#ifndef GNRC_IPV6_NIB_CONF_6LR
#define GNRC_IPV6_NIB_CONF_6LR (1)
#endif
#ifndef GNRC_IPV6_NIB_CONF_SLAAC
#define GNRC_IPV6_NIB_CONF_SLAAC (0)
#endif
#endif

#ifdef MODULE_GNRC_IPV6_NIB_6LN
#ifndef GNRC_IPV6_NIB_CONF_6LN
#define GNRC_IPV6_NIB_CONF_6LN (1)
#endif
#ifndef GNRC_IPV6_NIB_CONF_SLAAC
#define GNRC_IPV6_NIB_CONF_SLAAC (0)
#endif
#ifndef GNRC_IPV6_NIB_CONF_QUEUE_PKT
#define GNRC_IPV6_NIB_CONF_QUEUE_PKT (0)
#endif

However, if a non-6LN interface is also compiled in (still without making the node a border router) an auto-configured address will be assigned in accordance with RFC 6775 to the interface, just assuming the interface is a 6LN interface. As it then only performs duplicate address detection RFC-6775-style then, the address then never becomes valid, as the duplicate address detection according to RFC 4862 (part of the SLAAC functionality) is never performed.

As auto-configuring an address without SLAAC doesn't make sense, this fix makes the interface skip it completely, but provides a warning to the user, so they know what to do.

Testing procedure

Compile examples/gnrc_networking native with both a netdev_tap and socket_zep:

GNRC_NETIF_NUMOF=2 USEMODULE=socket_zep TERMFLAGS="-z [::]:17755 tap0" \
   make -C examples/gnrc_networking all term

With this fix, the link-local address of the Ethernet interface won't receive an auto-configured link-local address and a warning is printed

RIOT native interrupts/signals initialized.
LED_RED_OFF
LED_GREEN_ON
RIOT native board initialized.
RIOT native hardware initialization complete.

SLAAC not activated; won't auto-configure IPv6 for interface 7
    Use GNRC_IPV6_NIB_CONF_SLAAC=1 to activate
main(): This is RIOT! (Version: 2020.01-devel-197-gc93b3-gnrc_netif/enh/split-6lo-6ln)
RIOT network stack example application
All up, running the shell now
> ifconfig
ifconfig
Iface  7  HWaddr: 26:82:DC:3B:CA:E6 
          L2-PDU:1500 MTU:1500  HL:64  RTR  
          Source address length: 6
          Link type: wired
          inet6 group: ff02::2
          inet6 group: ff02::1
          
          Statistics for Layer 2
            RX packets 4  bytes 440
            TX packets 3 (Multicast: 3)  bytes 186
            TX succeeded 3 errors 0
          Statistics for IPv6
            RX packets 4  bytes 384
            TX packets 3 (Multicast: 3)  bytes 144
            TX succeeded 3 errors 0

Iface  8  HWaddr: 45:5B  Channel: 26  NID: 0x23
          Long HWaddr: 00:5A:45:50:0A:00:45:5B 
          L2-PDU:102 MTU:1280  HL:64  RTR  
          6LO  IPHC  
          Source address length: 8
          Link type: wireless
          inet6 addr: fe80::25a:4550:a00:455b  scope: local  VAL
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:ff00:455b
          
          Statistics for Layer 2
            RX packets 0  bytes 0
            TX packets 3 (Multicast: 3)  bytes 86
            TX succeeded 6 errors 0
          Statistics for IPv6
            RX packets 0  bytes 0
            TX packets 3 (Multicast: 3)  bytes 192
            TX succeeded 3 errors 0

Compiling with CFLAGS+=-DGNRC_IPV6_NIB_CONF_SLAAC=1 still auto-configures the address and validates it shortly after the node started (and the warning is not shown ;-)):

> ifconfig
ifconfig
Iface  7  HWaddr: 26:82:DC:3B:CA:E6 
          L2-PDU:1500 MTU:1500  HL:64  RTR  
          Source address length: 6
          Link type: wired
          inet6 addr: fe80::2482:dcff:fe3b:cae6  scope: local  VAL
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:ff3b:cae6
          
          Statistics for Layer 2
            RX packets 5  bytes 526
            TX packets 2 (Multicast: 2)  bytes 140
            TX succeeded 2 errors 0
          Statistics for IPv6
            RX packets 5  bytes 456
            TX packets 2 (Multicast: 2)  bytes 112
            TX succeeded 2 errors 0

Iface  8  HWaddr: 45:5B  Channel: 26  NID: 0x23
          Long HWaddr: 00:5A:45:50:0A:00:45:5B 
          L2-PDU:102 MTU:1280  HL:64  RTR  
          6LO  IPHC  
          Source address length: 8
          Link type: wireless
          inet6 addr: fe80::25a:4550:a00:455b  scope: local  VAL
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:ff00:455b
          
          Statistics for Layer 2
            RX packets 0  bytes 0
            TX packets 2 (Multicast: 2)  bytes 43
            TX succeeded 3 errors 0
          Statistics for IPv6
            RX packets 0  bytes 0
            TX packets 2 (Multicast: 2)  bytes 128
            TX succeeded 2 errors 0

Without the fix and CFLAGS=-DGNRC_IPV6_NIB_CONF_SLAAC=1 not set the Ethernet interface gets a link-local address which will stay tentative forever:

> ifconfig
ifconfig
Iface  7  HWaddr: 26:82:DC:3B:CA:E6 
          L2-PDU:1500 MTU:1500  HL:64  RTR  
          Source address length: 6
          Link type: wired
          inet6 addr: fe80::2482:dcff:fe3b:cae6  scope: local  TNT[1]
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:ff3b:cae6
          
          Statistics for Layer 2
            RX packets 5  bytes 526
            TX packets 1 (Multicast: 1)  bytes 62
            TX succeeded 1 errors 0
          Statistics for IPv6
            RX packets 5  bytes 456
            TX packets 1 (Multicast: 1)  bytes 48
            TX succeeded 1 errors 0

Iface  8  HWaddr: 45:5B  Channel: 26  NID: 0x23
          Long HWaddr: 00:5A:45:50:0A:00:45:5B 
          L2-PDU:102 MTU:1280  HL:64  RTR  
          6LO  IPHC  
          Source address length: 8
          Link type: wireless
          inet6 addr: fe80::25a:4550:a00:455b  scope: local  VAL
          inet6 group: ff02::2
          inet6 group: ff02::1
          inet6 group: ff02::1:ff00:455b
          
          Statistics for Layer 2
            RX packets 0  bytes 0
            TX packets 2 (Multicast: 2)  bytes 43
            TX succeeded 3 errors 0
          Statistics for IPv6
            RX packets 0  bytes 0
            TX packets 2 (Multicast: 2)  bytes 128
            TX succeeded 2 errors 0

Issues/PRs references

Several, but triggered by testing procedures in #12512 and #10499.

@miri64 miri64 added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch labels Oct 20, 2019
@miri64 miri64 added this to the Release 2019.10 milestone Oct 20, 2019
@miri64 miri64 requested review from benpicco and kb2ma October 20, 2019 14:26
@miri64
Copy link
Member Author

miri64 commented Oct 20, 2019

@kb2ma this bug isn't really critical, as it only appears in very special configuration, so if it isn't in by RC2 when you create it, we shouldn't worry about that.

if (!gnrc_netif_is_6ln(netif)) {
LOG_WARNING("SLAAC not activated; won't auto-configure IPv6 for "
"interface %u\n"
" Use " NAME(GNRC_IPV6_NIB_CONF_SLAAC) "=1 to "
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't a simple

"    Use GNRC_IPV6_NIB_CONF_SLAAC=1 to "

suffice? The stringification of a macro to print the macro name seems unnecessarily complex in this situation. Or are there other intentions (for future use)?

Copy link
Member Author

@miri64 miri64 Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main motivation is future-proofness, e.g. if the name of the macro changes. The example will cause an immediate compile-time error while just having a string literal won't.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The example will cause an immediate compile-time error while just having a string literal won't.

Did you test that? I also thought so and did a quick test using a puts(). I was able to put anything in NAME() without the compiler throwing an error.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I didn't I assumed the preprocessor checks what it is provided with. Ok then I do it the simple way ;-).

@cgundogan cgundogan added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines labels Oct 20, 2019
@miri64
Copy link
Member Author

miri64 commented Oct 20, 2019

Also fixed the wording to a less colloquial style.

@cgundogan cgundogan added the Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines label Oct 20, 2019
@cgundogan
Copy link
Member

Alright, the change seems sensible. Still needs a test, though. I will roll up my sleeves in a couple of hours, if nobody tested & confirmed the desired behaviour by then.

@miri64
Copy link
Member Author

miri64 commented Oct 20, 2019

Alright, the change seems sensible. Still needs a test, though. I will roll up my sleeves in a couple of hours, if nobody tested & confirmed the desired behaviour by then.

Don't you trust my results posted above? ^^

@cgundogan
Copy link
Member

Don't you trust my results posted above? ^^

Sure. But you know: the devil is in the detail (: It doesn't hurt to verify your results .. and I just did 👍 looks good.

Copy link
Member

@cgundogan cgundogan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please squash.

@cgundogan cgundogan added Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Oct 20, 2019
When the NIB is compiled for 6LN mode (but not a 6LBR), the Stateless
Address Autoconfiguration (SLAAC) functionality is disabled, as it is
typically not required; see `sys/include/net/gnrc/ipv6/nib/conf.h`, ll.
46 and 55. However, if a non-6LN interface is also compiled in (still
without making the node a border router) an auto-configured address will
be assigned in accordance with [RFC 6775] to the interface, just
assuming the interface is a 6LN interface. As it then only performs
duplicate address detection RFC-6775-style then, the address then never
becomes valid, as the duplicate address detection according to [RFC
4862] (part of the SLAAC functionality) is never performed.

As auto-configuring an address without SLAAC doesn't make sense, this
fix makes the interface skip it completely, but provides a warning to
the user, so they know what to do.

[RFC 6775]: https://tools.ietf.org/html/rfc6775#section-5.2
[RFC 4862]: https://tools.ietf.org/html/rfc4862#section-5.4
@miri64 miri64 force-pushed the gnrc_ipv6_fix/fix/auto-config-with-no-slaac-but-6ln branch from 35ebfbd to 7cba2fb Compare October 20, 2019 22:17
@cgundogan
Copy link
Member

Okay, Murdock shows finally green!

@cgundogan cgundogan merged commit e66283f into RIOT-OS:master Oct 20, 2019
@miri64 miri64 deleted the gnrc_ipv6_fix/fix/auto-config-with-no-slaac-but-6ln branch October 21, 2019 07:25
@miri64
Copy link
Member Author

miri64 commented Oct 21, 2019

Thanks for the review :-)

@miri64
Copy link
Member Author

miri64 commented Oct 21, 2019

Backport provided in #12522

miri64 added a commit to miri64/RIOT that referenced this pull request Oct 22, 2019
Similar as with RIOT-OS#12513, when the NIB is compiled in 6LN mode (but not
6LR mode), the address-resolution state-machine (ARSM) functionality is
disabled in favor of the more simpler address resolution proposed in RFC
6775.

However, if a non-6LN interface is also compiled in (without making it
a router or border router) it will never join the solicited-nodes
multicast address of addresses added to it, resulting in address
resolution to that interface to fail.

If the interface is not a 6LN (which in case 6LN mode is disabled is
always false), a warning is now printed, encouraging the user to
activate the ARSM functionality if needed.
miri64 added a commit to miri64/RIOT that referenced this pull request Oct 22, 2019
Similar as with RIOT-OS#12513, when the NIB is compiled in 6LN mode (but not
6LR mode), the address-resolution state-machine (ARSM) functionality is
disabled in favor of the more simpler address resolution proposed in RFC
6775.

However, if a non-6LN interface is also compiled in (without making it
a router or border router) it will never join the solicited-nodes
multicast address of addresses added to it, resulting in address
resolution to that interface to fail.

If the interface is not a 6LN (which in case 6LN mode is disabled is
always false), a warning is now printed, encouraging the user to
activate the ARSM functionality if needed.
@miri64 miri64 restored the gnrc_ipv6_fix/fix/auto-config-with-no-slaac-but-6ln branch October 31, 2019 18:50
@miri64 miri64 deleted the gnrc_ipv6_fix/fix/auto-config-with-no-slaac-but-6ln branch October 31, 2019 18:51
gdoffe pushed a commit to gdoffe/RIOT that referenced this pull request Dec 17, 2019
Similar as with RIOT-OS#12513, when the NIB is compiled in 6LN mode (but not
6LR mode), the address-resolution state-machine (ARSM) functionality is
disabled in favor of the more simpler address resolution proposed in RFC
6775.

However, if a non-6LN interface is also compiled in (without making it
a router or border router) it will never join the solicited-nodes
multicast address of addresses added to it, resulting in address
resolution to that interface to fail.

If the interface is not a 6LN (which in case 6LN mode is disabled is
always false), a warning is now printed, encouraging the user to
activate the ARSM functionality if needed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: network Area: Networking CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants