Skip to content
New issue

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

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

Already on GitHub? # to your account

default interface selected when gateway is valid #54

Merged
merged 5 commits into from
Jan 3, 2022
Merged

Conversation

d-a-v
Copy link
Owner

@d-a-v d-a-v commented Dec 20, 2021

When interface STA was becoming "UP", it was selected as default interface.
Now it is selected as default interface (= the way out for unknown destination address) only when
there is no default interface, or when default interface is AP (cannot be the way out),
and the STA interface address is not private.

This PR is included in esp8266/Arduino#8319
which embeds a similar test for ethernet interfaces.

@d-a-v
Copy link
Owner Author

d-a-v commented Dec 21, 2021

After a second thought,

if ((netif_default == NULL || netif_default == netif_ap) && !ip_addr_islinklocal(&netif->ip_addr))

could/should be replaced by checking for non-zero netif->gw_address

@mcspr
Copy link
Collaborator

mcspr commented Dec 21, 2021

why check ip at all though?
isn't up set either from dhcp or static or autoip, so we already assume it should work?

@d-a-v
Copy link
Owner Author

d-a-v commented Dec 21, 2021

If interface IP address is not routable, then it is useless to define this interface as default one to get to internet.
But this test indeed useless because we don't have 'localhost'.
The presence of a gw address should be sufficient to check whether the interface could be the default one.

@d-a-v d-a-v changed the title default interface selected when address is global = not local default interface selected when gateway is valid Dec 21, 2021
@mcspr
Copy link
Collaborator

mcspr commented Dec 22, 2021

Yeah, true. git-grep status_callback shows up in netif_set_{up,down}, but also via netif_set_ipaddr for v4/v6 triggered by something and if it is something valid there's another status_callback call

Should it also de-register itself? i.e. not only act when netif is up?

  • netif_set_up -> flag is UP + gw + default is null or ap -> set sta as default
  • netif_set_down -> no flag UP -> reset to null if sta is default

ethernet netif is still manual though, should it also have callback and take priority from sta?

@d-a-v
Copy link
Owner Author

d-a-v commented Dec 22, 2021

Yeah, true. git-grep status_callback shows up in netif_set_{up,down}, but also via netif_set_ipaddr for v4/v6 triggered by something and if it is something valid there's another status_callback call

I understand you agree with where these changes happens.

Should it also de-register itself? i.e. not only act when netif is up?

Yes, I shall update code with your proposal.

ethernet netif is still manual though, should it also have callback and take priority from sta?

Yes ethernet should have the same netif callback.
About priority I don't know. I think I should remove taking precedence over AP from STA.
And about "race" between STA or Ethernet, I'd say it's on the user code itself.
WiFi starts off by default. If user brings up both STA and ethernet, then we have to decide which one of the first up or the last up gets to be the default route.
I'd say the first one would have it, the following up interfaces would not change the already default interface set, but user code is always able to call netif_set_default().

@d-a-v
Copy link
Owner Author

d-a-v commented Dec 28, 2021

ethernet netif is still manual though, should it also have callback and take priority from sta?

LwipIntfDev<RawDev>::netif_status_callback() is a callback used when the interface comes up or down (up from dhcp or manual).
It has been updated to look like lwip2's netif status callback.

Unless forced, the ethernet interface will not be used as default if sta was selected before.
(and sta will not if ethernet was selected before)

@d-a-v d-a-v merged commit 450bb63 into master Jan 3, 2022
@d-a-v d-a-v deleted the defaultInterface branch January 3, 2022 13:13
someburner added a commit to someburner/esp82xx-nonos-linklayer-opensdk that referenced this pull request Feb 16, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants