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

Fix to sending IPV6 UPD packet fails IPv6 enabled in SoftAP intf #12312

Merged
merged 1 commit into from Feb 3, 2020
Merged

Fix to sending IPV6 UPD packet fails IPv6 enabled in SoftAP intf #12312

merged 1 commit into from Feb 3, 2020

Conversation

cy-arsm
Copy link

@cy-arsm cy-arsm commented Jan 27, 2020

Modified LwIP mbed-os wrapper LWIPInterface.cpp to add an api remove interface which will be called as part of WhdSoftAPInterface::stop() to support sending UDP IPv6 packet is sent over link-local interface
Issue: udp_sendto() Fails for multicast IPV6 packet when interface is switched from SoftAP to STA mode.
When SoftAP start is called, add_ethernet_interface() will be called internally to add interface details into netif_list.
When switching from SoftAP to STA mode, add_ethernet_interface() will be called again to append the interace details into netif_list.
When udp_sendto() is called, ip6_route() will return interface as NULL since it consider device as single interface.

Summary of changes

LWIPInterface.cpp
LWIP::remove_ethernet_interface() api is added to remove the interface from the netif_list.

WhdSoftAPInterface.cpp
WhdSoftAPInterface::stop(), calling remove_ethernet_interface() from the network list which was added as part of WhdSoftAPInterface::start()

WhdSTAInterface.cpp
WhdSTAInterface::stop(), calling remove_ethernet_interface() from the network list which was added as part of WhdSTAInterface::start()

Impact of changes

Migration actions required

Documentation


Pull request type

[x] Patch update (Bug fix / Target update / Docs update / Test update / Refactor)
[] Feature update (New feature / Functionality change / New API)
[] Major update (Breaking change E.g. Return code change / API behaviour change)

Test results

[x] No Tests required for this change (E.g docs only update)
[x] Covered by existing mbed-os tests (Greentea or Unittest)
[] Tests / results supplied as part of this PR

Reviewers


@ciarmcom ciarmcom requested review from a team January 27, 2020 08:00
@ciarmcom
Copy link
Member

@cy-arsm, thank you for your changes.
@ARMmbed/mbed-os-ipcore @ARMmbed/mbed-os-maintainers please review.

@sathishm6
Copy link

@AnttiKauppila @0xc0170 - For review. We need this fix is needed for our release, hence we need this fix in mbed-os 5.15.1 release. Thanks.

@@ -355,6 +355,15 @@ nsapi_error_t WhdSTAInterface::disconnect()
}
whd_emac_wifi_link_state_changed(_whd_emac.ifp, WHD_FALSE);

// remove the interface added in connect
if (_interface) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems like this would prevent ever connecting again.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@morser499 , local private member _interface is updated with the interface details in add_ehternet_interface()API call.
Hence _interface is updated to NULL inside remove_ethernet_interface()API call.
As part of connect interface is updated and as part of disconnect interface is removed. Hence reconnecting is not blocked.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have verified multiple time's connect -> disconnect -> connect to ensure re-connection is successful.

Issue: udp_sendto() Fails for multicast IPV6 packet when interface is switched from SoftAP to STA mode.
When SoftAP start is called,  add_ethernet_interface() will be called internally to add interface details into  netif_list.
When switching from SoftAP to STA mode,  add_ethernet_interface() will be called again to append the interace details into netif_list.
When udp_sendto() is called, ip6_route() will return interface as NULL since it consider device as single interface.
Fix: SoftAP mode Stop, call remove_ethernet_interface() to remove the interface from the netif_list.
@sathishm6
Copy link

@maclobdell - This change is required in mbed-os 5.15.1. Can you get this change reviewed and merged for 5.15.1. Thanks

@AnttiKauppila
Copy link

Isn't this a Feature update as you are adding new function to APIs?

@mergify mergify bot added needs: CI and removed needs: review labels Feb 3, 2020
@0xc0170
Copy link
Contributor

0xc0170 commented Feb 3, 2020

CI started

@mbed-ci
Copy link

mbed-ci commented Feb 3, 2020

Test run: SUCCESS

Summary: 11 of 11 test jobs passed
Build number : 1
Build artifacts

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 3, 2020

Isn't this a Feature update as you are adding new function to APIs?

Looks like it is! It is ready for merging, the version will be checked.

cc @bulislaw

@0xc0170 0xc0170 merged commit 8b829e5 into ARMmbed:master Feb 3, 2020
@0xc0170 0xc0170 requested a review from bulislaw February 3, 2020 16:55
@mergify mergify bot removed the ready for merge label Feb 3, 2020
@bulislaw
Copy link
Member

bulislaw commented Feb 3, 2020

That is a new feature, so it shouldn't go to a patch release.

@ifyall
Copy link

ifyall commented Feb 3, 2020

@bulislaw, I am not certain that is correct. The SoftAP feature from WhdSoftAPInterface.cpp has existed for a number of releases and IPV6 multicast pre-exists this as well. In this case, there is a defect where when you are using both together, you can have a failure. This PR fixes that defect. It doesn't introduce a new feature per-se.

Thoughts?

Ian

@adbridge adbridge added release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0 and removed release-version: 5.15.1 labels Feb 6, 2020
@ifyall
Copy link

ifyall commented Feb 18, 2020

@adbridge , please exclude this PR from all releases. We are working on an additional PR that fixes issues introduced in this one.

Thank you,

Ian

@adbridge
Copy link
Contributor

@ifyall unfortunately this has already made it to alpha-2 (currently being released) but these are only pre-releases for 6.0 and not for general usage anyway. Also as our releases come off master now and this one was merged 16 days ago it could be troublesome to back out.

@ifyall
Copy link

ifyall commented Feb 19, 2020

@adbridge OK, thank you.

@adbridge
Copy link
Contributor

@ifyall would you like to raise a PR against master to back this out in readiness for the next pre-release or do you propose to just land additional PRs with fixes on top of this one ?

adbridge added a commit that referenced this pull request Mar 10, 2020
…evert

Reverting #12312 as it breaking current WiFI connect()->Disconnect() sequence
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devices: cypress release-version: 6.0.0-alpha-2 Second pre-release version of 6.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet