Skip to content

Conversation

@pabeni
Copy link
Contributor

@pabeni pabeni commented Nov 17, 2021

this change-set implement the functionality discussed in recent public meetings.

Depending on a newly introduced configuration flag, new address are used/notified only if a default route is available for such address/interface.

There are a few quirks due to ipv4 route resolution and libell async notification, see the 3rd patch for the details.

This should close issue #170

Paolo Abeni added 2 commits November 17, 2021 16:48
instead of using a plain sockaddr, use the mptcpd_addr_info
structure, so that we can more easily include additional
metadata.

Additionally change the mptcpd_addr_info layout so that this
don't break existing interface->addrs users.

This make address management more efficent and will simplify
the next patch.
This only adds the configuration-related bits, the
actual check implementation is in the next patch.
@pabeni
Copy link
Contributor Author

pabeni commented Nov 17, 2021

the failure is cause by too old libell version. the gitlab workflows install ubuntu-latest defeault, which is v 0.27, while this changeset uses some functions introduces in libell 0.29. I have no idea how to tell the gitlab workflow to use/install a more recent version.

@pabeni pabeni force-pushed the route_lookup branch 6 times, most recently from 3a6fad7 to d6429af Compare November 17, 2021 16:53
@coveralls
Copy link

coveralls commented Nov 17, 2021

Pull Request Test Coverage Report for Build 1515662526

  • 21 of 127 (16.54%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-2.06%) to 47.338%

Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/network_monitor.c 21 127 16.54%
Totals Coverage Status
Change from base Build 1418473626: -2.06%
Covered Lines: 996
Relevant Lines: 2104

💛 - Coveralls

Copy link
Member

@ossama-othman ossama-othman left a comment

Choose a reason for hiding this comment

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

@pabeni I still have to go through the changes in detail, but I did make a quick pass and left some comments about minor issues.

@ossama-othman ossama-othman added this to the Q4-2021 milestone Nov 18, 2021
@ossama-othman ossama-othman added bug Something isn't working enhancement New feature or request labels Nov 18, 2021
if the relevant flag is enabled, do a route lookup for a
default route at address creation time and notify the
new address only if such route is found.

In case of failure re-schedule the check with a small timeout
and exponential backoff up to 3 times. That is intended to
cope with e.g. DHCP clients configuring the address and the
relevant route in order.

Note that route lookup is async, an racing address removal can
try to delete the relevant address object before the route lookup
completes. Implement a simple refcount mechanism on the mptcp
address object to avoid such issue.

In a similar manner, route lookup can race with interface deletion.
Instead of adding another ref-count, just lookup again the interface
via the known ifindex in the async operation.

Finally default route lookup for ipv4 does not really work on the Linux
kernel: 'ip route get 0.0.0.0 ...' always returns a loopback route. To
fetch the default route, lookup for a test-only address: as no host/network
should use it, it should be reachable only via the default route.

The gitlab workflows is update accordingly, explicitly building
and installing the minimal required libell version
@ossama-othman ossama-othman merged commit fda8f3e into multipath-tcp:master Nov 29, 2021
ossama-othman pushed a commit that referenced this pull request Dec 2, 2021
The new default route availability based address filter introduced in
PR #179 requires features in ELL >= 0.30.  Adjust the ELL version
check and documentation accordingly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants