Sync#21
Open
jincysam87 wants to merge 8 commits into
Open
Conversation
Reason for change: Support to scan multiple SSIDs Test Procedure: Test wifi scan API with multiple SSIDs Risks: Low Signed-off-by: [jincysaramma_sam@comcast.com](mailto:jincysaramma_sam@comcast.com)
…ist in WiFi mode (#313) * RDKEMW-18247 Panel is listing as a PLATCO device in Router client list in WiFi mode Reason for change: Set the NetworkManager dhcp-hostname in the connection profile to use the default hostname instead of the device name.
Release of 3.0.0
…308) * RDK-61440: Implementation of handling PowerMode Change in NM plugin Reason for change: Ctrl ntwrk state based on PowerMode transitions Test procedure: Change the PowerMode state and verify the behavior Risks: low Priority: P1 Signed-off-by: Anand N <Anand_N@comcast.com> Co-authored-by: Karunakaran A <karunakaran_amirthalingam@cable.comcast.com>
* AI agent's initial code modifications as per approved plan
* fix(gnome-events): address three IP cache gaps from initial implementation
- Subscribe to notify::options on NMDhcpConfig in all three wiring sites
(device-added, startup walk, ip4/ip6ConfigChangedCb) so dhcpserver stays
current across mid-lease renewals; remove stale TODO comment
- Replace IpFamilyCache::globalAddresses (std::set) + prefix (uint32_t)
with std::map<string,uint32_t> so toIPAddress() always projects ipaddress
and prefix from the same entry
- Remove dead GnomeNetworkManagerEvents::onAddressChangeCb() definition
and declaration, superseded by the cache-diff path in refreshIpFamilyCache
* fix: replace narrow fe80: string check with correct IPv6 fe80::/10 detection
Extract isIPv6LinkLocal() helper into NetworkManagerImplementation.h and use
it at all four call sites. Also remove now-unused #include <set>.
* fix: fix build errors after IpFamilyCache::globalAddresses type change
Update cache insert calls in gnome and rdk proxy fallback paths to use
the map<string, uint32_t> API (insert({addr, prefix})) and remove the
now-absent top-level c->prefix field assignments.
* fix(ip-cache): read IP config from device instead of active connection
refreshIpFamilyCache() was reading addresses from
nm_active_connection_get_ip4/6_config(), which returns NULL on platforms
like xione-uk where NetworkManager does not manage the IP configuration
directly. The signal handlers (ip4ChangedCb, ip6ChangedCb) are connected
to the device-level NMIPConfig objects, so the cache must also read from
nm_device_get_ip4/6_config() to see the same addresses that triggered
the notification.
This mismatch caused the cache to remain empty and no IP_ACQUIRED events
were ever emitted despite signals firing correctly.
* fix(ip-cache): guard DNS array access to prevent out-of-bounds read
nm_ip_config_get_nameservers() returns a NULL-terminated strv. In newer
libnm versions this is guaranteed non-NULL even when empty (returns a
pointer to {NULL}). The previous code accessed dnsArr[1] unconditionally
after checking dnsArr non-NULL, which reads past the single-element
allocation when there are zero DNS servers configured.
On the xione-uk platform, the device-level IP config has addresses from
the kernel but no DNS servers via NetworkManager, so the returned strv
is empty. Reading dnsArr[1] past the allocation boundary causes SIGSEGV
on this embedded platform.
Fix: only access dnsArr[1] when dnsArr[0] is confirmed non-NULL. Also
use the correct const type to avoid the C-style cast.
* fix(ip-cache): emit IP_LOST events on interface disconnect
Three changes fix the missing IP_LOST events when disconnecting:
1. Call refreshIpFamilyCache in deviceStateChangeCb before reporting
INTERFACE_LINK_DOWN or INTERFACE_REMOVED. When NM disconnects a
device, it batches State and Ip4Config/Ip6Config property changes
into a single D-Bus PropertiesChanged signal. libnm updates all
properties atomically then emits notify signals in arbitrary order.
If notify::state fires before notify::ip4-config, the cache was
being cleared (by ReportInterfaceStateChange) before
refreshIpFamilyCache could diff against it. By explicitly calling
refreshIpFamilyCache from the state callback, the diff runs while
the cache still has the old addresses, producing IP_LOST events
through the canonical path with proper logging.
2. ReportInterfaceStateChange now emits IP_LOST for every cached address
before clearing the cache. This is a safety net: if
refreshIpFamilyCache already handled the transition (because
notify::ip4-config fired first), the cache will be empty and this
emits nothing. If it didn't, this catches any remaining addresses.
3. Move the nm_device_get_ip4/6_config() read outside the if(conn) gate
in refreshIpFamilyCache. The device-level IP config does not require
an active connection, so the cache can detect address presence or
absence during teardown when the active connection is already gone.
* fix(ip-cache): emit IP_LOST events reliably on interface disconnect
- refreshIpFamilyCache now checks device state: when
devState <= NM_DEVICE_STATE_DISCONNECTED, it skips the libnm read
so newCache is empty and the diff emits IP_LOST for all cached
addresses. This eliminates the signal-ordering dead zone where
addresses were still present in libnm but about to be cleared.
- Remove IP_LOST emission from ReportInterfaceStateChange (was
duplicating the cache-clear + emit logic). That function now only
manages interface state (connected flags, default interface,
connectivity monitor).
- Move the authoritative 'IP acquired:'/'IP lost:' log line into
ReportIPAddressChange, which is the single guaranteed call site
for every IP event regardless of origin.
- Remove the now-redundant 'IP acquired:'/'IP lost:' prints from
refreshIpFamilyCache's diff section.
Verified: disconnect produces exactly one IP_LOST per cached address,
no spurious IP_ACQUIRED, and IP_LOST events precede LINK_DOWN.
* refactor: replace named IpFamilyCache fields with map-based storage
Replace four hardcoded cache fields (m_ethIPv4Cache, m_wlanIPv4Cache,
m_ethIPv6Cache, m_wlanIPv6Cache) with a single std::map keyed by
(interface, ipFamily) pair. Add getIpCache() convenience accessor.
Change toIPAddress(bool isIPv6) to toIPAddress() with auto-detection
via inet_pton, since the cache is already partitioned by IP family.
This removes ~60 lines of repetitive if/else-if interface selection
boilerplate across all backends (GnomeProxy, GdbusClient, RDKProxy)
and eliminates hardcoded interface-name assumptions from the storage
layer.
* fix(ip-cache): populate IPAddress.ula with actual ULA, not link-local
The IPAddress.ula field was incorrectly being populated with IPv6
link-local addresses (fe80::/10). ULA (Unique Local Address, fc00::/7)
is a different address class — analogous to RFC 1918 private IPv4.
Changes:
- Add isIPv6ULA() helper using the same bitmask as NetworkManager's
nm_ip6_addr_is_ula(): (s6_addr32[0] & 0xfe000000) == 0xfc000000
- Add ulaAddress field to IpFamilyCache, separate from linkLocalAddress
- Map IpFamilyCache::ulaAddress to IPAddress::ula in toIPAddress()
- Add three-way IPv6 classification (link-local / ULA / global) in
GnomeEvents, GnomeProxy, GdbusClient, GdbusEvent, and RDK proxy
- ULA addresses are NOT reported as global: they do not go into
globalAddresses and do not trigger IP_ACQUIRED/IP_LOST events
- Fix pre-existing gdbus bug: globalAddresses.insert() now uses the
correct {address, prefix} pair instead of bare string
- Fix NetworkManager.json example to use valid ULA (fd00::/8 prefix)
- Fix docs example: IPv4 result should have empty ula field
- Replace IN_IS_ADDR_LINKLOCAL macro with isIPv4LinkLocal() inline
helper for consistency with the IPv6 helpers; remove now-unnecessary
arpa/inet.h includes from GnomeProxy and GnomeEvents
* feat(ip-cache): filter MAC-based EUI-64 global IPv6 from GetIPSettings
Store interface HW address in IpFamilyCache.macAddress (populated from
nm_device_get_hw_address). toIPAddress() iterates globalAddresses
preferring non-MAC-based globals; falls back to MAC-based if all are
EUI-64 derived.
Direct-read (fallback) path in GnomeProxy also filters at selection time.
Adds isIPv6MacBased() helper to detect EUI-64 addresses derived from
the interface MAC.
* refactor(GetIPSettings): remove fallback path, serve exclusively from event-driven cache
The event thread seeds the IP cache synchronously during startup
(via refreshIpFamilyCache) before entering g_main_loop_run(), so the
cache is always populated before any RPC can arrive. The fallback
path that read directly from the RPC thread's m_nmClient was
effectively dead code.
Remove the ~250-line fallback (m_nmClient reads, connection lookups,
IPv4/IPv6 address iteration, and fallback cache writes). On cache
miss, return Core::ERROR_GENERAL with a warning log.
Also remove the now-unused isAutoConnectEnabled() helper and its
10 local variable declarations that only served the fallback.
* refactor(ip-cache): split address containers and move helpers out of header
- Split IpFamilyCache into three containers: globalAddresses (map,
event-diffable), linkLocalAddresses (set), uniqueLocalAddresses (set)
- Move isIPv4LinkLocal, isIPv6LinkLocal, isIPv6ULA, toIPAddress from
inline header definitions to NetworkManagerImplementation.cpp
- Add explicit constructors to GlobalAddressInfo for C++11 compatibility
- Simplify event diffing in refreshIpFamilyCache to operate on
globalAddresses keys directly without type filtering
* fix: address PR review feedback on IP cache helpers and proxy
- isIPv6ULA: replace non-portable s6_addr32 with s6_addr[0] byte check
- isIPv6MacBased: rewrite with binary EUI-64 comparison via inet_pton
and memcmp, fixing broken string matching from inet_ntop zero suppression
- Add parseMac() helper accepting both colon-separated and bare hex MACs
- Add #include <cstring> and <cstdio> for memcmp/sscanf
- cleanupSignalHandlers: explicitly disconnect DHCP option callbacks
- Remove stale 'GetIPSettings fallback path' comment
- GetIPSettings: return ERROR_NONE with empty result when cache has no
entry for the requested interface+family, since absence of an address
is not an error
- GetIPSettings: reset result to IPAddress{} with correct ipversion
before cache lookup, and preserve requested family after toIPAddress()
- Remove onAddressChangeCb L2 test: the old callback was replaced by
event-driven cache diffs in refreshIpFamilyCache(); the test had no
assertions and exercised only dead compatibility code
- deviceAddedCB: seed IP cache after attaching signal handlers so
GetIPSettings returns data immediately for hotplugged devices
- Encapsulate m_ipCacheMap/m_ipCacheMutex as private; expose
lookupIpCache() and swapIpCache() locked accessors to prevent
unsynchronised access from outside the class
- lookupIpCache returns IPAddress directly (via toIPAddress() under the
lock) instead of copying the entire IpFamilyCache by value
- refreshIpFamilyCache: make this an internal helper function
- deviceRemovedCB: disconnect all IP-config, DHCP, and device-level
signal handlers symmetrically with deviceAddedCB, and clear the IP
cache (emitting IP_LOST events) to prevent stale entries and handler
accumulation on device removal/re-addition
* fix(gnome): use device-level DHCP config API to fix empty dhcpserver in GetIPSettings
The ActiveConnection's Dhcp4Config property is not populated until the
connection reaches ACTIVATED state, but ip4ChangedCb fires earlier (when
addresses appear on the IP config object). At that point,
nm_active_connection_get_dhcp4_config() returns NULL, causing
refreshIpFamilyCache() to store an empty dhcpserver in the cache.
Switch to nm_device_get_dhcp4_config() / nm_device_get_dhcp6_config()
which read from the Device object's Dhcp4Config property. This property
is set when the device enters IP_CONFIG state and its options are
populated before the IP address appears — ensuring dhcpserver is
available when the cache is built.
Apply the same fix to DHCP signal handler connections in
ip4ConfigChangedCb, ip6ConfigChangedCb, deviceAddedCB,
networkMangerEventMonitor, deviceRemovedCB, and cleanupSignalHandlers.
* build(backends): gate legacy IP members and export backend macros
* test(libnm): update L1 tests for cache-based GetIPSettings and refreshIpFamilyCache
- Remove 5 GetIPSettings error-path tests that exercised libnm API
call sequences no longer present in the cache-based implementation
- Replace 5 GetIPSettings data tests with cache-based equivalents
that populate IpFamilyCache via swapIpCache() and verify the
JSON-RPC response
- Fix 5 event tests (disconnected/unmanaged/unknown for wlan0 and
eth0) by changing nm_device_get_iface and nm_device_get_state
from WillOnce to WillRepeatedly to accommodate additional calls
from refreshIpFamilyCache()
- Fix platformInit test: GetIPSettings now returns success:true
with empty data for valid interfaces when the cache is empty
* tests: fix segfault in cache tests by using _instance global
The Instantiate mock on COMLink was never invoked because
RootConfig parses ConfigLine looking for 'root' at the top
level, but the test's JSON nests it inside 'configuration'.
This caused Root() to take the in-process path via
ServiceAdministrator, bypassing COMLink entirely.
As a result, the NetworkManagerImpl ProxyType member was
never populated (null _realObject). The new cache tests
that call NetworkManagerImpl->swapIpCache() dereferenced
null, crashing at mutex offset 0x3d0.
Fix by using the Plugin::_instance global pointer which is
set during platform_init() in the in-process instantiation
path.
* tests: add 9 new GetIPSettings test scenarios for IP cache behavior
Add test coverage for IP cache and address selection logic:
- IPv6 MAC-based fallback when all globals are MAC-derived
- IPv6 prefer non-MAC-based global address selection
- IPv6 only-cached request for IPv4 returns empty
- Cache invalidation returns success:true with empty fields
- IP version case insensitivity (ipv4/IPV6/etc)
- IPv6 ULA-only cache (no global -> address fields omitted)
- swapIpCache returns old global address keys
- Separate IPv4/IPv6 caches per interface
- Cache clearing via invalid cache swap
* tests: add utility function tests for IP address classification
Add 14 tests for pure utility functions introduced with IP cache:
- isIPv4LinkLocal: link-local (169.254/16), non-link-local, invalid input
- isIPv6LinkLocal: link-local (fe80::/10), non-link-local, invalid input
- isIPv6ULA: ULA (fc00::/7 including fc and fd), non-ULA, invalid input
- isIPv6MacBased: EUI-64 match with colon and plain hex MAC formats,
non-matching, privacy extension, and invalid inputs (also exercises
parseMac indirectly)
* tests: add disconnect cache-clearing event tests
Add 3 behavioral tests verifying that device disconnect events
clear the IP cache:
- disconnect_clears_ipv4_cache_eth0
- disconnect_clears_ipv6_cache_wlan0
- disconnect_clears_both_ip_family_caches
Tests use public APIs (swapIpCache, deviceStateChangeCb, GetIPSettings)
and assert observable behavior rather than internal NM API call sequences.
* fix: restore IP address cache clearing on disconnect for RDK/GDBUS backends
The m_eth*Address and m_wlan*Address fields were no longer being
cleared in ReportInterfaceStateChange on link-down/remove, causing
stale IP settings to be returned by GetIPSettings in the RDK and
GDBUS backends. Restore the clearing, guarded by preprocessor
checks for NM_BACKEND_GDBUS and NM_BACKEND_RDK since these fields
are not defined for the libnm backend.
Also fix lookupIpCache to set out.ipversion from the requested
ipFamily rather than relying on toIPAddress() inference, which
could default to "IPv4" when the cache is valid but contains no
addresses. This makes lookupIpCache self-consistent for any future
callers.
Reason for change: Updated the documentation Test Procedure: Verify docs/NetworkManagerPlugin.md reflects content in definition/NetworkManager.json Priority: P1 Risk: Low
Reason for Change: The persistent m_nmClient created during platform_init() subscribed to all D-Bus PropertyChanged signals from the NetworkManager daemon. Its isolated m_nmContext was never continuously drained, causing every signal (device state changes, DHCP renewals, WiFi scan updates, DNS changes) to queue indefinitely as GSource objects. This produced memory leak that grew gradually over time. The fix adopts the same create-per-call pattern already used by wifiManager: each proxy API call creates a temporary NMClient, uses it for the synchronous libnm query, then destroys it via the contextBusyWatcher drain loop. Between calls no client exists, so no D-Bus subscription is active and no signals accumulate. No GMainLoop or background thread is required since all proxy operations are synchronous. Test Procedure: Longrun scenario and observe for memory leaks Priority: P1 Risks: Medium
Release of 3.3.0
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
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.
No description provided.