net: bt: fix 6lowpan over BLE regression

[fabiobaltieri]

6lowpan over BLE stopped working since multicast filtering was implemented. Seems like Address Registration Option implementation is required for this to work without solicit node multicast, but as that is not currently implemented, nodes are unresponsive.

[jukkar]

Looks good. Could you add a test case for this change so that we will not hit this issue again in the future if the code is changed. So a simple test in tests/net/ipv6/src/main.c, that would check that we do not drop solicited node mcast packet, would be enough.

[fabiobaltieri]

Actually after further investigation I realized that the current behavior has been set explicitly in

66244a0 net: if: No need to always join solicit node mcast group

the multicast stack handles solicited node address correctly when initialized to do so with join_mcast_solicit_node().

Dropping NET_L2_MULTICAST_SKIP_JOIN_SOLICIT_NODE from net_bt_flags() seems to make the ipsp example work again, so that would be an option, but that should not be needed according to https://tools.ietf.org/html/rfc7668#section-3.2.2 (mentioned in the patch above). What I don't understand now is why the Linux host is using solicited node for neighbor discovery and if there is a way of setting up the link to make this work correctly.

Marked the PR as draft while investigating further.

[fabiobaltieri]

I think this is what's happening: according to rfc7668, 6lowpan over BLE does not need solicited node frames, and neighbor discovery happens with NS messages with Address Registration Option (described in rfc6775), which supposedly keeps the neighbor cache hot on the border router:

Registration:
The process during which a LoWPAN node sends a Neighbor
Solicitation message with an Address Registration Option to a
router creating a Neighbor Cache Entry (NCE) for the LoWPAN node
with a specific timeout. Thus, for 6LoWPAN routers, the Neighbor
Cache doesn't behave like a cache. Instead, it behaves as a
registry of all the host addresses that are attached to the
router.

Now the problem is that there's no implementation for this in the zephyr ipv6 stack as far as I can tell, and it seems like ARO is not supported in Linux either (that would have to be defined here as option 33).

So since the node does not send NS ARO messages, Linux tries to do normal host initiated neighbor discovery, which fails because the node stack is discarding those frames.

rfc7668 also states that link local addresses do not have to be registered

A Bluetooth LE 6LN MUST NOT register its link-local address.

so I don't quite get the full story of how link-local address are supposed to be resolved.

Either way, this does not work with the current code base without address registration plus solicited node filtering, and I think that even if we had address registration, it would not work without explicit handling in Linux.

@jukkar: would it make sense to remove NET_L2_MULTICAST_SKIP_JOIN_SOLICIT_NODE from the 6lowpan bluetooth stack as a workaround until rfc6775 is fully implemented? I'll update the PR with that in the meantime, just in case someone else wants to test it.

[jukkar]

would it make sense to remove NET_L2_MULTICAST_SKIP_JOIN_SOLICIT_NODE from the 6lowpan bluetooth stack as a workaround until rfc6775 is fully implemented? I'll update the PR with that in the meantime, just in case someone else wants to test it.

Sure, that seems to be the only usable workaround for this issue atm.

[jukkar]

In your sample commit message, the example list of multicast addresses are

IPv6 multicast addresses (max 8):
        ff02::1
        ff02::1:ff01:c41e
        ff02::1:ff00:1
        ff02::1

Here ff02::1 is mentioned twice. Is this a typo or was it really like this? The address should be there once and it is a bug if mentioned multiple times.

[fabiobaltieri]

Not a typo, it does show up twice, seems like init_app() of samples/bluetooth/ipsp/src/main.c is calling net_if_ipv6_maddr_add() on that address directly, which bypasses the duplicate check (that's in net_ipv6_mld_join).

Removing it or changing it to net_ipv6_mld_join makes the duplicate go away. Don't see why that address should be registered by the application anyway, I'll add a third patch to drop if entirely from the sample you agree with that.

As for the API, should the duplicate detection be moved from net_ipv6_mld_join to net_if_ipv6_maddr_add?

[jukkar]

Removing it or changing it to net_ipv6_mld_join makes the duplicate go away. Don't see why that address should be registered by the application anyway, I'll add a third patch to drop if entirely from the sample you agree with that.

Yes, please add a 3rd patch for that.

As for the API, should the duplicate detection be moved from net_ipv6_mld_join to net_if_ipv6_maddr_add?

Sounds like an error if the same address is there multiple times. So please move the check to when the address is added.

[fabiobaltieri]

Alright, I've updated the PR to add a patch to check for duplicate addresses to net_if_ipv6_maddr_add(), removed the call from the examples and while I was on it I updated the sample documentation to mention the debug config.

Tested the whole thing again, seems to be working fine, the address list looks like this now:

IPv6 multicast addresses (max 4):
ff02::1
ff02::1:ff01:c41e
ff02::1:ff00:1

[jukkar]

Thanks Fabio! Minor nit, looks good otherwise.

[fabiobaltieri]

Updated with a couple of checkpatch fixes.

[jukkar]

LGTM