RDKB-65961 : MACVLAN#39
Open
PalakshaOS wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new TR-181 VLAN parameter (MACVLAN) and begins integrating it into untagged interface creation/deletion logic to support MACVLAN-based “truly untagged” traffic handling.
Changes:
- Added
MACVLANto the VLAN data model (XML + DML get/set) andDML_VLANstruct storage. - Loaded
macvlanconfiguration from PSM during VLAN termination initialization. - Added MACVLAN-based creation/deletion branches in
ethernet_apis.cfor untagged interface handling.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| source/TR-181/middle_layer_src/vlan_internal.c | Reads MACVLAN value from PSM into the VLAN termination config structure. |
| source/TR-181/middle_layer_src/vlan_dml.c | Exposes MACVLAN via TR-181 get/set handlers. |
| source/TR-181/middle_layer_src/ethernet_apis.c | Adds MACVLAN/bridge branching for untagged interface create/delete (currently has compile/API issues). |
| source/TR-181/include/vlan_apis.h | Extends DML_VLAN with a macvlan field. |
| config/RdkVlanManager.xml | Adds MACVLAN parameter definition to the data model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
875
to
876
| if (pEntry->MACVLAN) { | ||
| // Create untagged interface using MACVLAN for truly untagged traffic |
Comment on lines
+919
to
+920
| else if (pEntry->VLANId == -1) | ||
| { |
| return ANSC_STATUS_FAILURE; | ||
| } | ||
|
|
||
| if(Entry->MACVLAN) { |
Comment on lines
+967
to
+968
| CcspTraceInfo(("%s-%d: Deleting untagged without MACVLAN %s/n", | ||
| __FUNCTION__, __LINE__, pEntry->Name)); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
source/TR-181/middle_layer_src/ethernet_apis.c:943
EthLink_DeleteUnTaggedInterface()references undefined/invalid fields (Entry->MACVLANandpEntry->VLANId).PDML_ETHERNEThas noMACVLANorVLANIdmembers, so this won’t compile, and it also diverges from the current create path (which always creates an untagged MACVLAN interface when both HALs are disabled).
if(Entry->MACVLAN) {
CcspTraceInfo(("%s-%d: Deleting untagged VLAN interface %s\n",
__FUNCTION__, __LINE__, pEntry->Name));
v_secure_system("ip link set %s down", pEntry->Name);
Comment on lines
362
to
366
| pthread_mutex_lock(&vlan_access_mutex); | ||
| //Set EthLink to False. it will take care UnTagged Created Vlan Interface | ||
| if (Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| if (pEntry->macvlan == 0 && Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| { | ||
| CcspTraceError(("%s-%d: Failed to Enable EthLink\n", __FUNCTION__, __LINE__)); |
Comment on lines
+405
to
+409
| else if (pEntry->VLANId == -1) | ||
| { | ||
| /* If the VLANID = -1, the VLAN is a bridge, delete the bridge and delete the interface from the bridge */ | ||
| if(strcmp(pEntry->BaseInterface, pEntry->Name)) | ||
| { |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
source/TR-181/include/vlan_apis.h:95
DML_VLAN_INITdoes not initialize the newly addedmacvlanmember. Some call sites (e.g., rollback/new entry paths) callDML_VLAN_INITwithout necessarily zeroing the whole struct first, which can makemacvlanindeterminate and affect enable/disable branching. Initialize it explicitly to a known default (0).
UINT macvlan;
CHAR Path[1024];
}
DML_VLAN, *PDML_VLAN;
Comment on lines
362
to
366
| pthread_mutex_lock(&vlan_access_mutex); | ||
| //Set EthLink to False. it will take care UnTagged Created Vlan Interface | ||
| if (Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| if (pEntry->macvlan == 0 && Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| { | ||
| CcspTraceError(("%s-%d: Failed to Enable EthLink\n", __FUNCTION__, __LINE__)); |
Comment on lines
+405
to
+409
| else if (pEntry->VLANId == -1) | ||
| { | ||
| /* If the VLANID = -1, the VLAN is a bridge, delete the bridge and delete the interface from the bridge */ | ||
| if(strcmp(pEntry->BaseInterface, pEntry->Name)) | ||
| { |
Comment on lines
603
to
607
| void * Vlan_Enable(void *Arg) | ||
| { | ||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID -1 CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); | ||
| ANSC_STATUS returnStatus = ANSC_STATUS_SUCCESS; | ||
| vlan_link_status_e status; |
Comment on lines
363
to
367
| //Set EthLink to False. it will take care UnTagged Created Vlan Interface | ||
| if (Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| if (pEntry->macvlan == 0 && Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| { | ||
| CcspTraceError(("%s-%d: Failed to Enable EthLink\n", __FUNCTION__, __LINE__)); | ||
| } |
Comment on lines
+405
to
+409
| else if (pEntry->VLANId == -1) | ||
| { | ||
| /* If the VLANID = -1, the VLAN is a bridge, delete the bridge and delete the interface from the bridge */ | ||
| if(strcmp(pEntry->BaseInterface, pEntry->Name)) | ||
| { |
Comment on lines
86
to
92
| UINT LastChange; | ||
| CHAR LowerLayers[1024]; | ||
| CHAR BaseInterface[64]; | ||
| INT VLANId; | ||
| UINT TPId; | ||
| UINT macvlan; | ||
| CHAR Path[1024]; |
Comment on lines
363
to
367
| //Set EthLink to False. it will take care UnTagged Created Vlan Interface | ||
| if (Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| if (pEntry->macvlan == 0 && Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| { | ||
| CcspTraceError(("%s-%d: Failed to Enable EthLink\n", __FUNCTION__, __LINE__)); | ||
| } |
| pthread_exit(NULL); | ||
| } | ||
|
|
||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); |
| pthread_mutex_lock(&vlan_access_mutex); | ||
| //Create Vlan Tagged Interface | ||
| if(pEntry->VLANId > 0) { | ||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID GREATER CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); |
Comment on lines
+363
to
+367
| if (strcmp(ParamName, "MACVLAN") == 0) | ||
| { | ||
| *puLong = p_Vlan->macvlan; | ||
| return TRUE; | ||
| } |
Comment on lines
+635
to
+639
| if (strcmp(ParamName, "MACVLAN") == 0) | ||
| { | ||
| p_Vlan->macvlan = uValue; | ||
| return TRUE; | ||
| } |
Comment on lines
88
to
92
| CHAR BaseInterface[64]; | ||
| INT VLANId; | ||
| UINT TPId; | ||
| UINT macvlan; | ||
| CHAR Path[1024]; |
Comment on lines
363
to
367
| //Set EthLink to False. it will take care UnTagged Created Vlan Interface | ||
| if (Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| if (pEntry->macvlan == 0 && Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| { | ||
| CcspTraceError(("%s-%d: Failed to Enable EthLink\n", __FUNCTION__, __LINE__)); | ||
| } |
Comment on lines
+405
to
+417
| else if (pEntry->VLANId == -1) | ||
| { | ||
| /* If the VLANID = -1, the VLAN is a bridge, delete the bridge and delete the interface from the bridge */ | ||
| if(strcmp(pEntry->BaseInterface, pEntry->Name)) | ||
| { | ||
| //v_secure_system("ifconfig %s down", pEntry->Name); | ||
| //v_secure_system("brctl delif %s %s", pEntry->Name, pEntry->BaseInterface); | ||
| //v_secure_system("brctl delbr %s", pEntry->Name); | ||
| v_secure_system("ip link set %s down", pEntry->Name); | ||
| v_secure_system("ip link set %s nomaster", pEntry->BaseInterface); | ||
| v_secure_system("ip link delete %s type bridge", pEntry->Name); | ||
| } | ||
| } |
Comment on lines
90
to
93
| INT VLANId; | ||
| UINT TPId; | ||
| UINT macvlan; | ||
| CHAR Path[1024]; |
Comment on lines
363
to
367
| //Set EthLink to False. it will take care UnTagged Created Vlan Interface | ||
| if (Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| if (pEntry->macvlan == 1 && Vlan_SetEthLink(pEntry, FALSE, FALSE) == ANSC_STATUS_FAILURE) | ||
| { | ||
| CcspTraceError(("%s-%d: Failed to Enable EthLink\n", __FUNCTION__, __LINE__)); | ||
| } |
Comment on lines
+405
to
+418
| else if (pEntry->VLANId == -1) | ||
| { | ||
| /* If the VLANID = -1, the VLAN is a bridge, delete the bridge and delete the interface from the bridge */ | ||
| if(strcmp(pEntry->BaseInterface, pEntry->Name)) | ||
| { | ||
| /* Remove interface from bridge */ | ||
| v_secure_system("ovs-vsctl --if-exists del-port %s", | ||
| pEntry->BaseInterface); | ||
|
|
||
| /* Delete bridge */ | ||
| v_secure_system("ovs-vsctl --if-exists del-br %s", | ||
| pEntry->Name); | ||
| } | ||
| } |
Comment on lines
90
to
93
| INT VLANId; | ||
| UINT TPId; | ||
| UINT macvlan; | ||
| CHAR Path[1024]; |
| Vlan_DeleteInterface(pEntry); | ||
| #endif | ||
| } | ||
| else if (pEntry->VLANId == -1) |
| { | ||
| /* If the VLANID = -1 and if MACVLAN config set to zero, the VLAN is a bridge, create the bridge and add the interface to the bridge */ | ||
| CcspTraceError(("%s-%d: PALAKSHA INSIDE VLAN ID -1 CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); | ||
| #define MAX_BR_RETRIES 10 |
Comment on lines
+692
to
+696
| v_secure_system("ovs-vsctl --may-exist add-br %s >>/tmp/ovs.log 2>>/tmp/ovs.log", pEntry->Name); | ||
|
|
||
| /* Check whether the bridge exists */ | ||
| if (v_secure_system("ovs-vsctl br-exists %s >>/tmp/ovs.log 2>>/tmp/ovs.log", pEntry->Name) == 0) | ||
| { |
| else | ||
| { | ||
| v_secure_system("ovs-vsctl --may-exist add-port %s %s",pEntry->Name,pEntry->BaseInterface); | ||
| v_secure_system("ifconfig %s up", pEntry->Name); |
Comment on lines
+678
to
+680
| else if (pEntry->VLANId == -1 && pEntry->macvlan == 0) | ||
| { | ||
| /* If the VLANID = -1 and if MACVLAN config set to zero, the VLAN is a bridge, create the bridge and add the interface to the bridge */ |
| pthread_exit(NULL); | ||
| } | ||
|
|
||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); |
| pthread_mutex_lock(&vlan_access_mutex); | ||
| //Create Vlan Tagged Interface | ||
| if(pEntry->VLANId > 0) { | ||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID GREATER CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); |
| Vlan_DeleteInterface(pEntry); | ||
| #endif | ||
| } | ||
| else if (pEntry->VLANId == -1) |
| { | ||
| /* If the VLANID = -1 and if MACVLAN config set to zero, the VLAN is a bridge, create the bridge and add the interface to the bridge */ | ||
| CcspTraceError(("%s-%d: PALAKSHA INSIDE VLAN ID -1 CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); | ||
| #define MAX_BR_RETRIES 10 |
Comment on lines
+693
to
+695
| snprintf(buffer, 1028,"ovs-vsctl --may-exist add-br %s >>/tmp/ovs.log 2>>/tmp/ovs.log", pEntry->Name); | ||
| system(buffer); | ||
| //v_secure_system("ovs-vsctl --may-exist add-br %s >>/tmp/ovs.log 2>>/tmp/ovs.log", pEntry->Name); |
Comment on lines
+698
to
+699
| if (v_secure_system("ovs-vsctl br-exists %s >>/tmp/ovs.log 2>>/tmp/ovs.log", pEntry->Name) == 0) | ||
| { |
| else | ||
| { | ||
| v_secure_system("ovs-vsctl --may-exist add-port %s %s",pEntry->Name,pEntry->BaseInterface); | ||
| v_secure_system("ifconfig %s up", pEntry->Name); |
| CHAR BaseInterface[64]; | ||
| INT VLANId; | ||
| UINT TPId; | ||
| UINT macvlan; |
Comment on lines
+618
to
+621
| v_secure_system("ip link add link %s name %s type vlan id -1", pEntry->Alias , pEntry->Name); | ||
|
|
||
| v_secure_system("ip link set %s up", pEntry->Name); | ||
|
|
Comment on lines
+411
to
+412
| v_secure_system("ovs-vsctl --if-exists del-port %s", | ||
| pEntry->BaseInterface); |
Comment on lines
728
to
729
| if (Vlan_SetEthLink(pEntry, TRUE, FALSE) == ANSC_STATUS_FAILURE) | ||
| { |
Comment on lines
+390
to
+395
| /* get cfg macvlan from psm */ | ||
| snprintf( acPSMQuery, sizeof( acPSMQuery ), PSM_VLANMANAGER_MACVLAN, nIndex + 1 ); | ||
| if ( CCSP_SUCCESS == DmlVlanGetPSMRecordValue( acPSMQuery, acPSMValue ) ) | ||
| { | ||
| pVlan[nIndex].macvlan = atoi(acPSMValue) ; | ||
| } |
| return ANSC_STATUS_FAILURE; | ||
| } | ||
|
|
||
| v_secure_system("ip link add link %s name %s type vlan id -1", pEntry->Alias , pEntry->Name); |
Comment on lines
+405
to
+410
| #if !defined(VLAN_MANAGER_HAL_ENABLED) | ||
| else if(pEntry->VLANId == -1) | ||
| { | ||
| Vlan_DeleteInterface(pEntry); | ||
| } | ||
| #endif |
| pthread_exit(NULL); | ||
| } | ||
|
|
||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); |
| #else | ||
| if(pEntry->VLANId > 0 || (pEntry->VLANId == -1 && pEntry->macvlan == 0)) { | ||
| #endif | ||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID GREATER CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); |
| } | ||
| else | ||
| { | ||
| CcspTraceError(("%s-%d: PALAKSHA INSIDE FULL ELSE CASE calling Vlan_SetEthLink\n", __FUNCTION__, __LINE__)); |
| if(pEntry->VLANId > 0 || (pEntry->VLANId == -1 && pEntry->macvlan == 0)) { | ||
| #endif | ||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID GREATER CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); | ||
| if (Vlan_SetEthLink(pEntry, TRUE, TRUE) == ANSC_STATUS_FAILURE) |
| return ANSC_STATUS_FAILURE; | ||
| } | ||
|
|
||
| v_secure_system("ip link add link %s name %s type ipvlan mode l2", pEntry->Alias , pEntry->Name); |
| pVlan[nIndex].TPId = atoi(acPSMValue) ; | ||
| } | ||
|
|
||
| /* get cfg macvlan from psm */ |
| { | ||
| CcspTraceError(("%s-%d: PALAKSHA INSIDE FULL ELSE CASE calling Vlan_SetEthLink\n", __FUNCTION__, __LINE__)); | ||
| //Enable EthLink and it will take care Creation of UnTagged Vlan Interface. | ||
| if (Vlan_SetEthLink(pEntry, TRUE, FALSE) == ANSC_STATUS_FAILURE) |
| pthread_exit(NULL); | ||
| } | ||
|
|
||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); |
| #else | ||
| if(pEntry->VLANId > 0 || (pEntry->VLANId == -1 && pEntry->macvlan == 0)) { | ||
| #endif | ||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID GREATER CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); |
| } | ||
| else | ||
| { | ||
| CcspTraceError(("%s-%d: PALAKSHA INSIDE FULL ELSE CASE calling Vlan_SetEthLink\n", __FUNCTION__, __LINE__)); |
| if(pEntry->VLANId > 0 || (pEntry->VLANId == -1 && pEntry->macvlan == 0)) { | ||
| #endif | ||
| CcspTraceError(("%s-%d: PALAKSHA MAIN VLAN_ENABLE VLAN ID GREATER CASE %d %d %s %s\n", __FUNCTION__, __LINE__, pEntry->VLANId,pEntry->macvlan, pEntry->BaseInterface, pEntry->Name)); | ||
| if (Vlan_SetEthLink(pEntry, TRUE, TRUE) == ANSC_STATUS_FAILURE) |
| return ANSC_STATUS_FAILURE; | ||
| } | ||
|
|
||
| v_secure_system("ip link add link %s name %s type ipvlan mode l2", pEntry->Alias , pEntry->Name); |
| pVlan[nIndex].TPId = atoi(acPSMValue) ; | ||
| } | ||
|
|
||
| /* get cfg macvlan from psm */ |
Comment on lines
+609
to
+612
|
|
||
| v_secure_system("ip link add link %s name %s type type vlan id 0", pEntry->Alias , pEntry->Name); | ||
|
|
||
| v_secure_system("ip link set %s up", pEntry->Name); |
| } | ||
| else | ||
| { | ||
| CcspTraceError(("%s-%d: PALAKSHA INSIDE FULL ELSE CASE calling Vlan_SetEthLink\n", __FUNCTION__, __LINE__)); |
Signed-off-by: Palaksha Gowda <palaksha.gowda@sky.uk>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
source/TR-181/middle_layer_src/vlan_apis.c:729
- When interface creation fails, this function sets pEntry->Status = VLAN_IF_ERROR above, but later it unconditionally sets pEntry->Status = VLAN_IF_UP (a few lines below this hunk), which overwrites the error state. Gate the final UP assignment on successful operations (or on pEntry->Status not already being VLAN_IF_ERROR).
CcspTraceError(("%s-%d: PALAKSHA INSIDE FULL ELSE CASE calling Vlan_SetEthLink\n", __FUNCTION__, __LINE__));
//Enable EthLink and it will take care Creation of UnTagged Vlan Interface.
if (Vlan_SetEthLink(pEntry, TRUE, FALSE) == ANSC_STATUS_FAILURE)
{
CcspTraceError(("%s-%d: Failed to Enable EthLink\n", __FUNCTION__, __LINE__));
Comment on lines
89
to
93
| CHAR BaseInterface[64]; | ||
| INT VLANId; | ||
| UINT TPId; | ||
| UINT macvlan; | ||
| CHAR Path[1024]; |
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
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.