net: custom NET_LINK_ADDR_MAX_LENGTH with CONFIG_MODEM_CELLULAR #92172
Conversation
Max 16 bytes NET_LINK_ADDR_MAX_LENGTH with CONFIG_MODEM_CELLULAR to reflect IMEI length. Signed-off-by: Jani Hirsimäki <[email protected]>
github-actions
bot
requested review from
jukkar,
pdgendt,
rerickson1,
rlubos and
ssharks
|
| @@ -30,7 +30,9 @@ extern "C" { | |||
| */ | |||
|
|
|||
| /** Maximum length of the link address */ | |||
| #if defined(CONFIG_NET_L2_PHY_IEEE802154) || defined(CONFIG_NET_L2_PPP) | |||
| #if defined(CONFIG_MODEM_CELLULAR) | |||
| #define NET_LINK_ADDR_MAX_LENGTH 16 | |||
There was a problem hiding this comment.
How well is this tested with IPv6? The 16 byte link address is not common and because this causes all network interfaces to have max length larger than usual.
I am a bit worried about the NS & RA handling in
zephyr/subsys/net/ip/ipv6_nbr.c
Line 1078 in dc140c0
The padding variable might overflow easily
zephyr/subsys/net/ip/ipv6_nbr.c
Line 1088 in dc140c0
Is this change fixing a real issue or was it just done because we can?
There was a problem hiding this comment.
Is it possible to add a test for this so that we can verify that at least the tests work?
There was a problem hiding this comment.
It is real issue with ipv6 and when using NCS SLM application from ext-mcu and in this case from cellular modem.
In this kind of usage, recent changes to net_link_addr struct to not use pointers caused that these changes are needed and the assertion in:
ASSERTION FAIL [linkaddr->len >= 6] @ WEST_TOPDIR/zephyr/subsys/net/l2/ppp/ipv6cp.c:40
I did't find any cellular modem related tests.
There was a problem hiding this comment.
I did't find any cellular modem related tests.
Could we enable the option for relevant IPv6 tests so that this gets tested?
For example adding a new test setup with the cellular option to tests/net/ipv6/testcase.yaml, there might be other tests too where this could be done, this directory does at least some of the IPv6 neighbor testing.
By just adding new max value and not having any automated testing for it is a no-go.
There was a problem hiding this comment.
It is real issue with ipv6 and when using NCS SLM application from ext-mcu and in this case from cellular modem.
I am sure things work ok if only using cellular modem. I am mostly concerned what happens with multiple interface scenario like having wifi enabled at the same time.
| err = net_if_set_link_addr(modem_ppp_get_iface(data->ppp), data->imei, | ||
| ARRAY_SIZE(data->imei), | ||
| NET_LINK_UNKNOWN); | ||
| if (err) { | ||
| LOG_WRN("Failed to set link address on PPP interface (%d)", err); | ||
| } | ||
|
|
There was a problem hiding this comment.
Instead of adding the new address length, I think this should be changed to use a portion of the IMEI to set the 'MAC address' of the interface. For example, use the least significant digits of the IMEI to derive a MAC address.
|
Just my 2c, |
Max 16 bytes NET_LINK_ADDR_MAX_LENGTH with CONFIG_MODEM_CELLULAR to reflect IMEI length.