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

increase GNRC_NETIF_IPV6_ADDRS_NUMOF #20295

Closed

Conversation

MrKevinWeiss
Copy link
Contributor

Contribution description

It seems something happened and we have no available slot for adding an address. This makes the release tests fail.

Here is a solution but maybe someone has something better

Testing procedure

BUILD_IN_DOCKER=1 BOARD=samr21-xpro make flash term -C tests/net/gnrc_udp/
ifconfig 5 add beef::1

It should be able to add.

Issues/PRs references

@MrKevinWeiss MrKevinWeiss requested a review from miri64 as a code owner January 24, 2024 12:42
@github-actions github-actions bot added Area: tests Area: tests and testing framework Area: examples Area: Example Applications labels Jan 24, 2024
@chrysn
Copy link
Member

chrysn commented Jan 24, 2024

Should we just increase the default? The default values should be usable, and those who need a memory minimum can still tweak it down.

That way it'd also solve an issue in #16840 which otherwise needs a not-quite-kosher filtering of which prefix is the better one at the 6LBR.

There is no more room left (causing the release specs to fail).
It would be nice to be able to add one...
@MrKevinWeiss
Copy link
Contributor Author

Should we just increase the default? The default values should be usable, and those who need a memory minimum can still tweak it down.

That way it'd also solve an issue in #16840 which otherwise needs a not-quite-kosher filtering of which prefix is the better one at the 6LBR.

I think the philosophy of RIOT is more about keeping it small by default, I would rather get an error saying I need more space (maybe the error message can be improved). It is easier to get to the perfect solution™ that way than silently reserving data that is not being used.

If anyone feels strongly in either direction I am happy to support it. I just want the examples/tests to pass the release specs!

@benpicco
Copy link
Contributor

benpicco commented Jan 24, 2024

Is that 'something happened' that you now have a global address in addition to the link-local one?

Can you share the output of ifconfig?

For me this works just fine on examples/gnrc_networking without having to increase the number of addresses:

2024-01-24 21:17:10,429 # Iface  7  HWaddr: 00:6C  Channel: 26  NID: 0x23  PHY: O-QPSK 
2024-01-24 21:17:10,429 #           Long HWaddr: AE:5B:73:4B:A5:4A:80:6C 
2024-01-24 21:17:10,429 #            State: IDLE 
2024-01-24 21:17:10,430 #           ACK_REQ  L2-PDU:102  MTU:1280  HL:64  RTR  
2024-01-24 21:17:10,430 #           RTR_ADV  6LO  IPHC  
2024-01-24 21:17:10,430 #           Source address length: 8
2024-01-24 21:17:10,430 #           Link type: wireless
2024-01-24 21:17:10,431 #           inet6 addr: fe80::ac5b:734b:a54a:806c  scope: link  VAL
2024-01-24 21:17:10,431 #           inet6 group: ff02::2
2024-01-24 21:17:10,431 #           inet6 group: ff02::1
2024-01-24 21:17:10,431 #           inet6 group: ff02::1:ff4a:806c
2024-01-24 21:17:10,431 #           inet6 group: ff02::1a
2024-01-24 21:17:10,431 #           
2024-01-24 21:17:10,432 #           Statistics for Layer 2
2024-01-24 21:17:10,432 #             RX packets 0  bytes 0
2024-01-24 21:17:10,432 #             TX packets 3 (Multicast: 3)  bytes 0
2024-01-24 21:17:10,432 #             TX succeeded 3 errors 0
2024-01-24 21:17:10,432 #           Statistics for IPv6
2024-01-24 21:17:10,433 #             RX packets 0  bytes 0
2024-01-24 21:17:10,433 #             TX packets 3 (Multicast: 3)  bytes 178
2024-01-24 21:17:10,433 #             TX succeeded 3 errors 0
2024-01-24 21:17:10,433 # 

2024-01-24 21:17:23,875 # > ifconfig 7 add beef::1
2024-01-24 21:17:23,875 # success: added beef::1/64 to interface 7

2024-01-24 21:17:27,124 # > ifconfig
2024-01-24 21:17:27,124 # Iface  7  HWaddr: 00:6C  Channel: 26  NID: 0x23  PHY: O-QPSK 
2024-01-24 21:17:27,124 #           Long HWaddr: AE:5B:73:4B:A5:4A:80:6C 
2024-01-24 21:17:27,124 #            State: IDLE 
2024-01-24 21:17:27,125 #           ACK_REQ  L2-PDU:102  MTU:1280  HL:64  RTR  
2024-01-24 21:17:27,125 #           RTR_ADV  6LO  IPHC  
2024-01-24 21:17:27,125 #           Source address length: 8
2024-01-24 21:17:27,126 #           Link type: wireless
2024-01-24 21:17:27,126 #           inet6 addr: fe80::ac5b:734b:a54a:806c  scope: link  VAL
2024-01-24 21:17:27,126 #           inet6 addr: beef::1  scope: global  VAL
2024-01-24 21:17:27,127 #           inet6 group: ff02::2
2024-01-24 21:17:27,127 #           inet6 group: ff02::1
2024-01-24 21:17:27,127 #           inet6 group: ff02::1:ff4a:806c
2024-01-24 21:17:27,127 #           inet6 group: ff02::1a
2024-01-24 21:17:27,127 #           inet6 group: ff02::1:ff00:1
2024-01-24 21:17:27,127 #           
2024-01-24 21:17:27,128 #           Statistics for Layer 2
2024-01-24 21:17:27,128 #             RX packets 0  bytes 0
2024-01-24 21:17:27,128 #             TX packets 4 (Multicast: 4)  bytes 0
2024-01-24 21:17:27,129 #             TX succeeded 4 errors 0
2024-01-24 21:17:27,129 #           Statistics for IPv6
2024-01-24 21:17:27,129 #             RX packets 0  bytes 0
2024-01-24 21:17:27,130 #             TX packets 4 (Multicast: 4)  bytes 242
2024-01-24 21:17:27,130 #             TX succeeded 4 errors 0

@chrysn
Copy link
Member

chrysn commented Jan 24, 2024

I'd say it's about keeping reasonably small defaults, and provided you can get the message through.

If every tutorial that's supposed to run on a user's network starts with "Because we don't know your network config, let's be safe and set GNRC_NETIF_IPV6_ADDRS_NUMOF to 4" (or "let's set up an isolated network so we're sure things work"), I think that's where practicality and usability overrules compact defaults. More so if also our own tests have to overwrite it.

@benpicco
Copy link
Contributor

Reducing memory footprint is always much harder than increasing it because you have 1000 little cuts to tweak.

I think the default of two addresses is sensible, if you need more you might as well bump that config value.

@MrKevinWeiss
Copy link
Contributor Author

Is that 'something happened' that you now have a global address in addition to the link-local one?

Yup, I just didn't have the time to find out why it was added.

Can you share the output of ifconfig?

The release output has it. I also was able to reproduce it on the samr21-xpro locally (but I may not be able to go into the office today to get the output again).
Here is a failure for the gnrc_networking example

For me this works just fine on examples/gnrc_networking without having to increase the number of addresses.

It seems like you don't have the default global address in your ifconfig compared to the ones in the RC1 action run.

@MrKevinWeiss MrKevinWeiss added the Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch label Jan 25, 2024
@MrKevinWeiss
Copy link
Contributor Author

If every tutorial that's supposed to run on a user's network starts with "Because we don't know your network config, let's be safe and set GNRC_NETIF_IPV6_ADDRS_NUMOF to 4" (or "let's set up an isolated network so we're sure things work"), I think that's where practicality and usability overrules compact defaults. More so if also our own tests have to overwrite it.

Also makes sense (up to a point, of we are getting 100s of messages saying why different config values are there it becomes a bit strange).

I am happy do either, I would like a solution though since the release has a pretty tight deadline. I guess the downside of setting the config values this way is that overwritten them with just the CLI no longer is a possible.

@benpicco
Copy link
Contributor

benpicco commented Jan 25, 2024

It seems like you don't have the default global address in your ifconfig compared to the ones in the RC1 action run.

There is no such thing as a 'default global address' - looks like you have a border router running in your setup that announces that prefix.

You can check nib route or nib abr.

would like a solution though since the release has a pretty tight deadline

That deadline is entirely set by us.
I know we often disable or botch tests to 'fix CI' but that’s not something we should to for the release.
In this case it just looks like you have some concurrent experiment running on the IoT lab that interferes with yours - try setting a different PAN ID.

@MrKevinWeiss
Copy link
Contributor Author

Thanks @benpicco for clarifying, it would seem like there is a border router not only somewhere in IOT labs when I was trying to run the tests (and the flash actually worked) but also in locally.

try setting a different PAN ID.

I think that the best way forward would be something like this, if there is already a global address then change the PAN ID or channel and remote the assigned one and replace it with the known. Since this would be a release specs change I think we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: examples Area: Example Applications Area: tests Area: tests and testing framework Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants