As the review feedback, this commits address below concern.
1. The TCP keepalive code doesn't use the network parameter
at all. This parameter doesn't seem meaningful for TCP
keepalives. Starting a TCP keepalive with a non-null underpinned
network should throw IllegalArgumentException.
2. The feedback mention that the start version which takes a
@NonNull network should throw NPE if the network is null. But
Starting a NATT keepalive does not always require a underpinned
network. A new IkeSession started from Vpn will also not assign
the underpinned network at the initial stage which means
underpinned will be null until setNetwork() is called. Thus,
the underpinned network should be @Nullable instead.
Fix: 271797087
Test: atest FrameworksNetTests
Change-Id: Ieb57a7b15a06b2ccd94358b65cc00768c4f62e7d
Take a Network parameter to have an one-to-one mapping between
keepalive and its underpinned network on the automatic keepalive.
Existing design could not really tell which network should the
automatic keepalive check for the TCP socket status if there are
multiple automatic keepalives enabled, e.g. Bothe IWLAN and VPN
on WiFi enable the automatic keepalive. The keepalive for IWLAN
should check if there are any TCP sockets on the IWLAN network
instead of VPN network.
Bug: 259000745
Test: atest FrameworksNetTests
Test: Cts in the follow up commit
Change-Id: I7353f4ef43e8fdad02c4d4a0bb5f6efa7d94c1b4
This is much simpler and less error-prone, as well as less
subject to race conditions.
It also allows for cleaning up some TODOs.
Test: FrameworksNetTests
CtsNetTestCases
Bug: 267116236
Change-Id: I470c709446946ef35a0324427defe2f58b434339
Add SocketKeepalive.start with parameter to enable dynamic
keepalive mode based on the existence of TCP connections.
This supports IPSec mode to notify KeepaliveTracker to disable
keepalive when keepalive is unnecessary to improve battery life.
Keepalive is controlled by periodically TCP socket status check
for both enable and disable. This is a transition commit and
is expected to be updated based on the socket creation or
destroy.
Bug: 259000745
Test: m ; atest FrameworksNetTests
Change-Id: Ie4d598d69a73c4931c7d0b6dfde0e459e5dca6b4
(This rolls forward part of a previous change, now that jarjar was fixed
to not get very slow when the number of rules increases).
Autogenerate connectivity jarjar rules at build time, to avoid issues
with forgotten jarjar rules or hard-to-diagnose errors introduced by
incorrect rules.
This change causes all classes in framework-connectivity(-t) and
service-connectivity to be jarjared into android.net.connectivity, but
still avoids jarjaring classes in com.android.server as before, to keep
it small.
For many classes this differs from the original jarjar rule.
Notes on implementation:
- connectivity-jarjar-rules now has a subset
framework-connectivity-jarjar-rules containing only the rules
necessary for framework-connectivity. This is necessary because
framework-connectivity cannot depend on rules generated based on
service-connectivity, as there would be a dependency cycle
(service-connectivity depends on framework-connectivity); Soong even
crashes with a stack overflow.
- framework-wifi.stubs.module_lib is added to
framework-connectivity-pre-jarjar as it is necessary to build it (it
is already in impl_only_libs in the defaults).
It is unclear why framework-connectivity-pre-jarjar could build
before that (possibly because it was only used as "lib" ?)
- Fix package-private visibility; for example NattSocketKeepalive,
TcpSocketKeepalive are not API so should be jarjared, but are used
by ConnectivityManager which is not jarjared, so they are not in the
same package after the change. Package-private members in the
former 2 need to be public to be accessible. Changes in this commit
are all that is needed, as demonstrated by followup commits that move
the classes to a different package without further changes, and that
enforce that no class in an API package gets jarjared.
- framework-connectivity-internal-test-defaults is separated from
framework-connectivity-test-defaults, for unit tests that need to
access internal jarjared classes. Such tests need to use the jarjar
rules themselves too, so this is only appropriate for connectivity
internal unit tests.
Test: atest ConnectivityCoverageTests CtsNetTestCases
Bug: 217129444
Change-Id: Ib1bd939b71c0171d945fc01b96195d2f620ff13b
This reverts commit 53eb35cd82.
Reason for revert: b/233081774, caused long build times in FrameworkNetTests
Change-Id: Ide01c994f694b55fc0e87bb0e81584ac44196e68
Jarjar rules are hard to keep in sync with code, and hard to maintain
manually as the distinction between what should and should not be
jarjared is not always clear. This results in unsafe binaries that are
manually maintained, and developer frustration when something fails due
to incorrect jarjar rules.
Autogenerate jarjar rules at build time instead. This is achieved by
introducing a jarjar-rules-generator python-based library, which scans
pre-jarjar intermediate artifacts, and outputs jarjar rules for every
class to put it in a package specific to the module. The only exceptions
are:
- Classes that are API (module-lib API is the largest API surface of
the module)
- Classes that have unsupportedappusage symbols
- Classes that are excluded manually (for example, because they have
hardcoded external references, like for
ConnectivityServiceInitializer in SystemServer).
This change causes all classes in framework-connectivity(-t) and
service-connectivity to be jarjared into android.net.connectivity, but
still avoids jarjaring classes in com.android.server as before, to keep
it small.
For many classes this differs from the original jarjar rule.
Notes on implementation:
- connectivity-jarjar-rules now has a subset
framework-connectivity-jarjar-rules containing only the rules
necessary for framework-connectivity. This is necessary because
framework-connectivity cannot depend on rules generated based on
service-connectivity, as there would be a dependency cycle
(service-connectivity depends on framework-connectivity); Soong even
crashes with a stack overflow.
- framework-wifi.stubs.module_lib is added to
framework-connectivity-pre-jarjar as it is necessary to build it (it
is already in impl_only_libs in the defaults).
It is unclear why framework-connectivity-pre-jarjar could build
before that (possibly because it was only used as "lib" ?)
- Fix package-private visibility; for example NattSocketKeepalive,
TcpSocketKeepalive are not API so should be jarjared, but are used
by ConnectivityManager which is not jarjared, so they are not in the
same package after the change. Package-private members in the
former 2 need to be public to be accessible. Changes in this commit
are all that is needed, as demonstrated by followup commits that move
the classes to a different package without further changes, and that
enforce that no class in an API package gets jarjared.
- framework-connectivity-internal-test-defaults is separated from
framework-connectivity-test-defaults, for unit tests that need to
access internal jarjared classes. Such tests need to use the jarjar
rules themselves too, so this is only appropriate for connectivity
internal unit tests.
Test: atest ConnectivityCoverageTests CtsNetTestCases
Bug: 217129444
Change-Id: Ied17c3955ea2fda130089265d02908937ad8af1e
Files that are planned to be part of the connectivity module are grouped
in packages/Connectivity, so they can be built separately and moved in
one operation with their history into packages/modules/Connectivity.
This places the files in the existing framework-connectivity-sources
filegroup instead of the current framework-core-sources filegroup. Both
are used the same way in framework-non-updatable-sources.
Bug: 171540887
Test: m
Change-Id: I62d9d91574ace6f5c4624035d190260c3126b91e
