Support multiple IPv4 netmasks per interface #68419
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rlubos
reviewed
Feb 1, 2024
There was a problem hiding this comment.
Looks good, thanks, howerver we should get rid of those now deprecated functions usage within the codebase.
One of the users is recently added DHCPv4 server:
https://github.com/zephyrproject-rtos/zephyr/blob/main/subsys/net/lib/dhcpv4/dhcpv4_server.c#L1234
but in this particular case it should be straightforward fix, just take the netmask for the selected server_addr .
jukkar
force-pushed
the
devel/support-multiple-ipv4-netmasks-per-interface
branch
from
e2b8dc5
to
2777801
Compare
zephyrbot
added
area: Samples
zephyrbot
requested review from
bperseghetti,
Dat-NguyenDuy,
kartben,
Kludentwo,
krish2718,
manuargue,
nashif and
PetervdPerk-NXP
krish2718
previously approved these changes
Feb 2, 2024
jukkar
force-pushed
the
devel/support-multiple-ipv4-netmasks-per-interface
branch
from
2777801
to
6cc20ca
Compare
rlubos
reviewed
Feb 2, 2024
jukkar
force-pushed
the
devel/support-multiple-ipv4-netmasks-per-interface
branch
2 times, most recently
from
b780dea
to
51052ef
Compare
jukkar
force-pushed
the
devel/support-multiple-ipv4-netmasks-per-interface
branch
3 times, most recently
from
7dc45fa
to
ed8ccb1
Compare
The netmask should be tied to the IPv4 address instead of being global for the network interface. If there is only one IPv4 address specified to the network interface, nothing changes from user point of view. But if there are more than one IPv4 address / network interface, the netmask must be specified to each address separately. This means that net_if_ipv4_get_netmask() and net_if_ipv4_set_netmask() functions should not be used as they only work reliably if there is only one IPv4 address in the network interface. The new net_if_ipv4_get_netmask_by_addr() and net_if_ipv4_set_netmask_by_addr() functions should be used as they make sure that the netmask is tied to correct IPv4 address in the network interface. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Make sure that the tests work properly when each IPv4 address in the network interface gets its own netmask. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
This commit deprecates these legacy netmask get/set routines net_if_ipv4_set_netmask() net_if_ipv4_set_netmask_by_index() net_if_ipv4_get_netmask() as they do not work well if there are multiple IPv4 address assigned to the network interface. User should use these functions instead net_if_ipv4_set_netmask_by_addr() net_if_ipv4_set_netmask_by_addr_by_index() net_if_ipv4_get_netmask_by_addr() as they make sure the netmask it bound to correct IPv4 address. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Make sure that the drivers use the new IPv4 netmask setting API. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Make sure that the samples use the new IPv4 netmask setting API. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Convert various array loops in the net_if.c to use the ARRAY_FOR_EACH() macro. This makes the code more robust as we do not need to keep track of the separate define that tells the array size. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
No need to loop through network interface addresses, just listen DHCPv4 bound event and use that as a condition to continue. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
Convert various networking subsystem files to use ARRAY_FOR_EACH macro to make the looping more robust. Signed-off-by: Jukka Rissanen <jukka.rissanen@nordicsemi.no>
jukkar
force-pushed
the
devel/support-multiple-ipv4-netmasks-per-interface
branch
from
ed8ccb1
to
c2f238c
Compare
rlubos
approved these changes
Feb 6, 2024
krish2718
approved these changes
Feb 6, 2024
|
Setting milestone as 3.7 as we are past the RC1 for 3.6. Please correct the milestone if you disagree |
aescolar
added
hwmv2-likely-conflict
pdgendt
reviewed
Feb 27, 2024
cfriedt
approved these changes
Feb 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The netmask should be tied to the IPv4 address instead of being global for the network interface.
If there is only one IPv4 address specified to the network interface, nothing changes from user point of view. But if there are more than one IPv4 address / network interface, the netmask must be specified to each address separately.
This means that the
net_if_ipv4_get_netmask()andnet_if_ipv4_set_netmask()functions should not be used as they will only work reliably if there is only one IPv4 address in the network interface.Instead new
net_if_ipv4_get_netmask_by_addr()andnet_if_ipv4_set_netmask_by_addr()should be used instead as they make sure that the netmask is tied to correct IPv4 address in the network interface.No need to try to rush and try to get this to 3.6, existing functionality works for most of the use cases.