RDKBNETWOR-66 : Implement MAP-E in RdkWanManager#134
RDKBNETWOR-66 : Implement MAP-E in RdkWanManager#134Krithiksha11 wants to merge 15 commits intordkcentral:mainfrom
Conversation
Reason for change: Receive the MAP-E options from DhcpManager and configure the tunnel interface. Test Procedure: Verify functionality by passing DHCPv6 Option 94 and the ip6tnl tunnel interface is correctly configured and brought up. Testing Done : Results are captured in RDKBNETWOR-66 Risks: None. Signed-off-by: Krithiksha Prabhakar <krithiksha.prabhakar@telekom-digital.com>
Reason for change: Enclose MAP-E logic within FEATURE_MAPE flag Signed-off-by: Krithiksha Prabhakar <krithiksha.prabhakar@telekom-digital.com>
Reason for change: Write the PSID vlaues to /proc/nat_port so that the kernel reads these value and configures correct ports for MAPE. Test Procedure: Verify functionality by passing DHCPv6 Option 94 and the ip6tnl tunnel interface is correctly configured and brought up. Testing Done : Results are captured in RDKBNETWOR-66 Risks: None. Signed-off-by: Krithiksha Prabhakar <krithiksha.prabhakar@telekom-digital.com>
| char wan_interface[BUFLEN_64]={0}; | ||
|
|
||
| CcspTraceInfo(("%s %d - stop MAP-E\n", __FUNCTION__, __LINE__)); | ||
| if (syscfg_get(NULL, SYSCFG_WAN_INTERFACE, wan_interface, sizeof(wan_interface)) != ANSC_STATUS_SUCCESS) |
There was a problem hiding this comment.
Why do we need a syscfg, we can use the current interface name.
| } | ||
| #endif | ||
|
|
||
| #ifdef FEATURE_MAPE |
There was a problem hiding this comment.
Isn't it similar to the previous if condition for the MAPE? can we have single condition?
There was a problem hiding this comment.
Yes, merged to single if condition
| CcspTraceError(("%s %d Error resetting MAP-T configuration", __FUNCTION__, __LINE__)); | ||
| } | ||
|
|
||
| /* Clear DHCPv4 client */ |
There was a problem hiding this comment.
Starting DHCPv4 should be common for both MAPE and MAPT.
There was a problem hiding this comment.
Moved outside if condition
| CcspTraceInfo(("%s %d: Stopping DHCP v4\n", __FUNCTION__, __LINE__)); | ||
| WanManager_StopDhcpv4Client(p_VirtIf, STOP_DHCP_WITH_RELEASE); | ||
| } | ||
| if (p_VirtIf->IP.Dhcp4cStatus == DHCPC_STARTED) |
There was a problem hiding this comment.
Stopping DHCPv4 should be common for both MAPE and MAPT
There was a problem hiding this comment.
Moved outside MAPT condition
source/WanManager/wanmgr_net_utils.c
Outdated
| inet_ntop(AF_INET6, &in6, tunnel_source, sizeof(tunnel_source)); | ||
|
|
||
| //tunnel_source | ||
| ret = v_secure_system("ip -6 tunnel add %s mode ip4ip6 remote %s local %s encaplimit none dev %s", TUNNEL_NAME, br_ipv6_prefix, tunnel_source, "erouter0"); |
There was a problem hiding this comment.
Please don't hard code erouter0
| #endif //FEATURE_MAPT | ||
|
|
||
| #ifdef FEATURE_MAPE | ||
| else if ( pInterface->Selection.Status == WAN_IFACE_ACTIVE && p_VirtIf->Status == WAN_IFACE_STATUS_UP && p_VirtIf->MAP.dhcp6cMAPparameters.mapType == MAP_TYPE_MAPE && p_VirtIf->MAP.MapeStatus == WAN_IFACE_MAPE_STATE_UP) |
There was a problem hiding this comment.
Please add it with the MAPT if condition itself. Both are same transition
| } | ||
| #endif //FEATURE_MAPT | ||
|
|
||
| #ifdef FEATURE_MAPE |
There was a problem hiding this comment.
Lets keep both transitions together.
source/WanManager/wanmgr_net_utils.c
Outdated
| #define BUFF_SIZE_64 64 | ||
| #define BUFF_SIZE_1024 1024 | ||
| #define IPV4_ADDR_LEN_IN_BITS 32 | ||
| #define BUF_LEN (10 * (sizeof(struct inotify_event) + 255 + 1)) |
There was a problem hiding this comment.
is this used in the code?
source/WanManager/wanmgr_net_utils.c
Outdated
| } | ||
|
|
||
| #ifdef FEATURE_MAPE | ||
| ANSC_STATUS WanManager_MAPEConfiguration(ipc_map_data_t *dhcp6cMAPEMsgBody) |
There was a problem hiding this comment.
Can this function be moved to a new file for MAPE?
There was a problem hiding this comment.
Moved to new file as suggested
1b28a7c to
109927b
Compare
Reason for change: Address the review comments. Test Procedure: Verify functionality by passing DHCPv6 Option 94 and the ip6tnl tunnel interface is correctly configured and brought up. Testing Done : Results are captured in RDKBNETWOR-66 Risks: None. Signed-off-by: Krithiksha Prabhakar <krithiksha.prabhakar@telekom-digital.com>
109927b to
5185d67
Compare
|
Hi @Krithiksha11 : Please append a credit to NOTICE at top level, to match the new DT code: I will clear off the BD failure. |
Reason for change: Appended a credit to NOTICE at top level, to match the new DT code. Risks: None. Signed-off-by: Krithiksha Prabhakar <krithiksha.prabhakar@telekom-digital.com>
Hi @mhughesacn, I have added the same. Please review it. Thank you |
|
Thank you @Krithiksha11 |
config/RdkWanManager_v2.xml
Outdated
| <name>Enable</name> | ||
| <type>boolean</type> | ||
| <syntax>bool</syntax> | ||
| <writable>true</writable> |
There was a problem hiding this comment.
I think it would be better to keep these DMLs read-only, as we're only using RBUS events or IPC methods to send lease details to the WAN Manager.
If these DMLs are writable, it opens up the possibility for someone to modify the configuration directly, which could conflict with the current design.
I understand that the BBF specification defines them as writable, but allowing write access here could introduce issues.
|
Hi @Krithiksha11 : I just noticed this: If you make any more changes to this PR, please will you fix the headers on the new files you created. They say "this component's Licenses.txt file", but newer headers say "this component's LICENSE file". We changed the license name a long time ago, but headers have been slow to catch up. Many thanks! |
Reason for change: Address the review comments. Test Procedure: Verify functionality by passing DHCPv6 Option 94 and the ip6tnl tunnel interface is correctly configured and brought up. Testing Done : Results are captured in RDKBNETWOR-66 Risks: None. Signed-off-by: Krithiksha Prabhakar <krithiksha.prabhakar@telekom-digital.com>
1f6e090 to
17f486c
Compare
Hi @mhughesacn, I have addressed the requested change. Please review them. Thank you! |
|
Looks good - thank you! |
There was a problem hiding this comment.
Pull Request Overview
This PR implements MAP-E support in RdkWanManager by adding the capability to receive MAP-E options from DhcpManager and configure the tunnel interface accordingly. The implementation includes creating a new MAP-E configuration module, updating the state machine to handle MAP-E transitions, and providing TR-181 data model support for MAP objects.
- Unifies MAP-T and MAP-E handling in the WAN interface state machine
- Adds new MAP-E configuration module with tunnel setup functionality
- Introduces comprehensive TR-181 data model support for MAP objects
Reviewed Changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| source/WanManager/wanmgr_sysevents.h | Adds MAP-E feature flag definitions and syscfg variables |
| source/WanManager/wanmgr_sysevents.c | Updates state machine to use unified MAP_ACTIVE state |
| source/WanManager/wanmgr_net_utils.h | Updates function signatures to use unified MAP data structure |
| source/WanManager/wanmgr_net_utils.c | Refactors MAP-T functions to use unified data structure |
| source/WanManager/wanmgr_mape.h | New header defining MAP-E configuration functions |
| source/WanManager/wanmgr_mape.c | New module implementing MAP-E tunnel configuration |
| source/WanManager/wanmgr_interface_sm.h | Adds MAP-E interface status types |
| source/WanManager/wanmgr_interface_sm.c | Implements unified MAP state machine transitions |
| source/WanManager/wanmgr_dhcpv6_apis.c | Updates to use unified MAP data structure |
| source/WanManager/wanmgr_dhcp_event_handler.c | Adds MAP-E event handling support |
| source/WanManager/wanmgr_data.c | Adds MAP data structure initialization |
| source/WanManager/Makefile.am | Includes new MAP-E source file in build |
| source/TR-181/middle_layer_src/* | Complete TR-181 data model implementation for MAP objects |
| config/RdkWanManager_v2.xml | XML configuration for MAP TR-181 objects |
| NOTICE | Updates copyright to include Deutsche Telekom |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
source/WanManager/wanmgr_mape.c
Outdated
| syscfg_unset (NULL, "mape_psid"); | ||
| syscfg_unset (NULL, "mape_psid_len"); | ||
| syscfg_unset (NULL, "mape_ipv4_address"); | ||
| syscfg_unset (NULL, "mape_ipv4_address"); |
There was a problem hiding this comment.
Duplicate syscfg_unset call for 'mape_ipv4_address' on consecutive lines 245 and 246. Remove the duplicate call.
| syscfg_unset (NULL, "mape_ipv4_address"); |
There was a problem hiding this comment.
Removed the duplicate call.
| CcspTraceInfo(("%s %d - stop MAP-E\n", __FUNCTION__, __LINE__)); | ||
|
|
||
| snprintf(cmd, sizeof(cmd), "iproute del default dev %s", p_VirtIf->Name); | ||
| system(cmd); |
There was a problem hiding this comment.
Direct system() calls should be replaced with secure_wrapper functions like v_secure_system() for security. The cmd variable on line 346 is constructed from user input and should also use secure execution.
| system(cmd); | |
| v_secure_system(cmd); |
There was a problem hiding this comment.
Replaced system() with v_secure_system() calls.
| system(cmd); | ||
|
|
||
| system("ip link set dev ip6tnl down"); | ||
| system("ip -6 tunnel del ip6tnl"); |
There was a problem hiding this comment.
Direct system() calls should be replaced with secure_wrapper functions like v_secure_system() for security. The cmd variable on line 346 is constructed from user input and should also use secure execution.
| system(cmd); | |
| system("ip link set dev ip6tnl down"); | |
| system("ip -6 tunnel del ip6tnl"); | |
| v_secure_system(cmd); | |
| v_secure_system("ip link set dev ip6tnl down"); | |
| v_secure_system("ip -6 tunnel del ip6tnl"); |
There was a problem hiding this comment.
Replaced system() with v_secure_system() calls.
fbf6e57 to
5b4c096
Compare
5b4c096 to
ef719db
Compare
Signed-off-by: Parthiban Selvaraj <parthiban.selvaraj@sky.uk>
Reason for change: Receive the MAP-E options from DhcpManager and configure the tunnel interface.
Test Procedure: Verify functionality by passing DHCPv6 Option 94 and the ip6tnl tunnel interface is correctly configured and brought up.
Testing Done : Results are captured in RDKBNETWOR-66
Risks: None.