-
Notifications
You must be signed in to change notification settings - Fork 41
RDKB 63035 - HotSPot - IPv6 connectivity from LAN clients is not working #174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | ||
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prefix, sizeof(prefix)); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: lines 2457-2458, 2462-2463 use tabs while the surrounding code uses spaces. This creates mixed indentation which can cause issues with different editor settings and violates coding standards. Replace tabs with spaces to match the indentation style of the rest of the file.
| DEG_PRINT1("63035 - Debug current_wan_ifname:%s hotspot_wan_ifname:%s \n", wan_interface, hotspot_wan_ifname); | ||
| if (strcmp(wan_interface, hotspot_wan_ifname) == 0) | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | ||
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prefix, sizeof(prefix)); | ||
| } | ||
| else | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Debug statements contain the bug ticket number '63035' which makes them temporary/issue-specific rather than general-purpose logging. These debug statements should either be removed before merging or rewritten to be generic debug messages without the ticket number, as they will become obsolete after the bug is resolved.
| DEG_PRINT1("63035 - Debug current_wan_ifname:%s hotspot_wan_ifname:%s \n", wan_interface, hotspot_wan_ifname); | |
| if (strcmp(wan_interface, hotspot_wan_ifname) == 0) | |
| { | |
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prefix, sizeof(prefix)); | |
| } | |
| else | |
| { | |
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | |
| DEG_PRINT1("Debug current_wan_ifname:%s hotspot_wan_ifname:%s \n", wan_interface, hotspot_wan_ifname); | |
| if (strcmp(wan_interface, hotspot_wan_ifname) == 0) | |
| { | |
| DEG_PRINT1("Line:%d\n", __LINE__); | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prefix, sizeof(prefix)); | |
| } | |
| else | |
| { | |
| DEG_PRINT1("Line:%d\n", __LINE__); |
| DEG_PRINT1("63035 - Debug log prefix:%s pref_rx:%s out:%s\n",prefix,pref_rx,out); | ||
| if ( strlen(pref_rx) != 0 ) | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Multiple debug statements throughout the function (lines 2457, 2462, 2478, 2483, 2489, 2512, 2521, 2535, 2547, 2549) only print line numbers without meaningful context. These provide limited debugging value and should either be removed or enhanced with descriptive messages about what operation is being performed.
|
|
||
| if (prefix[0] != '\0' && strlen(prefix) != 0 ) | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d prefix:%s\n", __LINE__, prefix); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: line 2541 uses tabs while surrounding code uses spaces. This should use consistent spacing to match the file's indentation style.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | ||
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prefix, sizeof(prefix)); | ||
| } | ||
| else | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | ||
| sysevent_get(sr->sefd, sr->setok, "ipv6_prefix_ula", prefix, sizeof(prefix)); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: The code inside the if-else blocks uses tabs that appear misaligned. The DEG_PRINT1 statements and sysevent_get calls should be consistently indented with spaces or tabs to match the rest of the codebase. The current mix creates readability issues.
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prefix, sizeof(prefix)); | |
| } | |
| else | |
| { | |
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | |
| sysevent_get(sr->sefd, sr->setok, "ipv6_prefix_ula", prefix, sizeof(prefix)); | |
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prefix, sizeof(prefix)); | |
| } | |
| else | |
| { | |
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | |
| sysevent_get(sr->sefd, sr->setok, "ipv6_prefix_ula", prefix, sizeof(prefix)); |
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| else | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging has been added with ticket-specific prefixes (63035). These debug statements should either be removed before merging or converted to use proper logging levels. Debug prints with line numbers provide minimal value and clutter the code. Consider keeping only the essential log statements with meaningful context.
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | ||
| if ((IsHotspotActive) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging has been added with ticket-specific prefixes (63035). These debug statements should either be removed before merging or converted to use proper logging levels. Debug prints with line numbers provide minimal value and clutter the code. Consider keeping only the essential log statements with meaningful context.
| } | ||
| else | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging has been added with ticket-specific prefixes (63035). These debug statements should either be removed before merging or converted to use proper logging levels. Debug prints with line numbers provide minimal value and clutter the code. Consider keeping only the essential log statements with meaningful context.
|
|
||
| sysevent_set(sr->sefd, sr->setok, "lan_ipaddr_v6",token, 0); | ||
| */ | ||
| DEG_PRINT1("63035 - Line:%d, lan_if:%s, token:%s, pref_len:%d\n", __LINE__, lan_if,token,pref_len); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging has been added with ticket-specific prefixes (63035). These debug statements should either be removed before merging or converted to use proper logging levels. Debug prints with line numbers provide minimal value and clutter the code. Consider keeping only the essential log statements with meaningful context.
|
|
||
| if(!strncmp(out,"true",strlen(out))) | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging has been added with ticket-specific prefixes (63035). These debug statements should either be removed before merging or converted to use proper logging levels. Debug prints with line numbers provide minimal value and clutter the code. Consider keeping only the essential log statements with meaningful context.
| pt = out; | ||
| while((token = strtok_r(pt, ",", &pt))) | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging has been added with ticket-specific prefixes (63035). These debug statements should either be removed before merging or converted to use proper logging levels. Debug prints with line numbers provide minimal value and clutter the code. Consider keeping only the essential log statements with meaningful context.
| } | ||
| memset(prefix,0,sizeof(prefix)); | ||
|
|
||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging has been added with ticket-specific prefixes (63035). These debug statements should either be removed before merging or converted to use proper logging levels. Debug prints with line numbers provide minimal value and clutter the code. Consider keeping only the essential log statements with meaningful context.
| DEG_PRINT1("63035 - Line:%d prefix:%s\n", __LINE__, prefix); | ||
| SetV6Route(interface_name,prefix); | ||
| token_pref = strtok(prefix,"/"); | ||
| DEG_PRINT1("63035 - Line:%d, interface_name:%s, token_pref:%s, pref_len:%d\n", __LINE__, interface_name, token_pref,pref_len); | ||
| AssignIpv6Addr(interface_name,token_pref,pref_len); | ||
| } | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | ||
| } | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging has been added with ticket-specific prefixes (63035). These debug statements should either be removed before merging or converted to use proper logging levels. Debug prints with line numbers provide minimal value and clutter the code. Consider keeping only the essential log statements with meaningful context.
| sysevent_get(sr->sefd, sr->setok, "current_wan_ifname", wan_interface, sizeof(wan_interface)); | ||
|
|
||
| DEG_PRINT1("63035 - Debug current_wan_ifname:%s mesh_wan_ifname:%s \n", wan_interface, mesh_wan_ifname); | ||
| if ((IsHotspotActive) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The strcmp will crash if mesh_wan_ifname is empty (which happens when PSM_VALUE_GET_STRING fails at line 2447-2452). Either check that mesh_wan_ifname is non-empty before calling strcmp, or ensure the entire condition short-circuits properly when wan_interface is empty.
| if ((IsHotspotActive) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| if (IsHotspotActive || | |
| (mesh_wan_ifname && mesh_wan_ifname[0] != '\0' && | |
| wan_interface[0] != '\0' && | |
| strcmp(wan_interface, mesh_wan_ifname) == 0)) |
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| DEG_PRINT1("63035 - Debug current_wan_ifname:%s mesh_wan_ifname:%s \n", wan_interface, mesh_wan_ifname); | ||
| if (IsHotspotActive() || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excessive debug logging referencing ticket number RDKB-63035 should be removed or reduced before merging to production. Debug statements that only print line numbers provide minimal value and clutter the code. Consider removing most of these debug statements and keeping only the informative ones (lines 2456, 2474, 2514, 2549, 2552) that show actual variable values.
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); |
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | ||
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", hotspot_lan_prefix, sizeof(hotspot_lan_prefix)); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: Line 2460 uses a mix of tabs and spaces that doesn't align with line 2459. Both lines should use consistent indentation (4 spaces based on surrounding code).
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", hotspot_lan_prefix, sizeof(hotspot_lan_prefix)); | |
| DEG_PRINT1("63035 - Line:%d\n", __LINE__); | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", hotspot_lan_prefix, sizeof(hotspot_lan_prefix)); |
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", hotspot_lan_prefix, sizeof(hotspot_lan_prefix)); |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation with tabs instead of spaces. Line 2465 uses tabs for indentation while the surrounding code uses spaces. This should be consistent with the rest of the file's indentation style.
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", hotspot_lan_prefix, sizeof(hotspot_lan_prefix)); | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", hotspot_lan_prefix, sizeof(hotspot_lan_prefix)); |
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| SetV6Route(lan_if,hotspot_lan_prefix); | ||
| } | ||
| else | ||
| { | ||
| SetV6Route(lan_if,prefix); | ||
| } | ||
| char *token; |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Logic error: code at lines 2490-2497 should be wrapped within WAN_FAILOVER_SUPPORTED ifdef block. When WAN_FAILOVER_SUPPORTED is not defined, the variables mesh_wan_ifname, hotspot_wan_ifname, and wan_interface are initialized to empty strings. This causes the strcmp at line 2490 to evaluate to true (empty string equals empty string), leading to SetV6Route being called with an empty hotspot_lan_prefix. This is likely not the intended behavior. Either wrap this conditional block in WAN_FAILOVER_SUPPORTED or add a check to ensure these variables are not empty before comparing.
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| { | |
| SetV6Route(lan_if,hotspot_lan_prefix); | |
| } | |
| else | |
| { | |
| SetV6Route(lan_if,prefix); | |
| } | |
| char *token; | |
| #ifdef WAN_FAILOVER_SUPPORTED | |
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| { | |
| SetV6Route(lan_if,hotspot_lan_prefix); | |
| } | |
| else | |
| { | |
| SetV6Route(lan_if,prefix); | |
| } | |
| #else | |
| SetV6Route(lan_if,prefix); | |
| #endif | |
| char *token; |
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| SetV6Route(lan_if,hotspot_lan_prefix); | ||
| } | ||
| else | ||
| { | ||
| SetV6Route(lan_if,prefix); | ||
| } | ||
| char *token; |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation with tabs instead of spaces. Lines 2490-2497 use tabs for indentation while the surrounding code uses spaces. This should be consistent with the rest of the file's indentation style.
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| { | |
| SetV6Route(lan_if,hotspot_lan_prefix); | |
| } | |
| else | |
| { | |
| SetV6Route(lan_if,prefix); | |
| } | |
| char *token; | |
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| { | |
| SetV6Route(lan_if,hotspot_lan_prefix); | |
| } | |
| else | |
| { | |
| SetV6Route(lan_if,prefix); | |
| } | |
| char *token; |
| { | ||
| SetV6Route(lan_if,prefix); | ||
| } | ||
| char *token; |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation with tabs instead of spaces. Line 2498 uses tabs for indentation while the surrounding code uses spaces. This should be consistent with the rest of the file's indentation style.
| char *token; | |
| char *token; |
| { | ||
| SetV6Route(lan_if,prefix); | ||
| char *token; | ||
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to add both the routes.
|
|
||
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", hotspot_lan_prefix, sizeof(hotspot_lan_prefix)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename this variable , this is not the prefix received from hotspot wan. keep something like prev_lan_global_prefix
Reason for change: Added Firewall rule for LAN traffic Test Procedure: - TBD Risks: None Priority: P3 Signed-off-by: rirfha948 <rasina_irfhan@comcast.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| SetV6Route(lan_if,prev_lan_global_prefix); | ||
| } | ||
| SetV6Route(lan_if,prefix); | ||
| char *token; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same strcmp condition from lines 2463 and 2490 is duplicated. If the condition is true at line 2463, it will still be true at line 2490. Consider refactoring to use a boolean variable set once after the first check (line 2463) to avoid repeating the comparison logic and improve code maintainability.
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| { | |
| SetV6Route(lan_if,prev_lan_global_prefix); | |
| } | |
| SetV6Route(lan_if,prefix); | |
| char *token; | |
| bool is_special_wan_if = ((strcmp(wan_interface, hotspot_wan_ifname) == 0) || | |
| (strcmp(wan_interface, mesh_wan_ifname) == 0)); | |
| if (is_special_wan_if) | |
| { | |
| SetV6Route(lan_if,prev_lan_global_prefix); | |
| } | |
| SetV6Route(lan_if,prefix); | |
| char *token; |
| if (strcmp(current_wan_ifname,default_wan_ifname ) == 0) | ||
| { | ||
| fprintf(fp, "-A FORWARD ! -s %s -i %s -j LOG_FORWARD_DROP\n", prefix, lan_ifname); | ||
| fprintf(fp, "-A FORWARD -s %s -i %s -j LOG_FORWARD_DROP\n", prefix, wan6_ifname); | ||
| } |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional check compares current_wan_ifname with default_wan_ifname using strcmp. Both variables are extern and expected to be initialized in firewall.c. However, if either variable is empty (e.g., if sysevent values are not set), strcmp will compare empty strings and return 0 (equal), causing the DROP rules to be applied even when they shouldn't be. Consider adding validation to check that both variables contain non-empty values before the strcmp comparison.
| if (strcmp(current_wan_ifname,default_wan_ifname ) == 0) | |
| { | |
| fprintf(fp, "-A FORWARD ! -s %s -i %s -j LOG_FORWARD_DROP\n", prefix, lan_ifname); | |
| fprintf(fp, "-A FORWARD -s %s -i %s -j LOG_FORWARD_DROP\n", prefix, wan6_ifname); | |
| } | |
| if (current_wan_ifname[0] != '\0' && | |
| default_wan_ifname[0] != '\0' && | |
| strcmp(current_wan_ifname, default_wan_ifname) == 0) | |
| { | |
| fprintf(fp, "-A FORWARD ! -s %s -i %s -j LOG_FORWARD_DROP\n", prefix, lan_ifname); | |
| fprintf(fp, "-A FORWARD -s %s -i %s -j LOG_FORWARD_DROP\n", prefix, wan6_ifname); | |
| } |
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| SetV6Route(lan_if,prev_lan_global_prefix); | ||
| } | ||
| SetV6Route(lan_if,prefix); | ||
| char *token; |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inconsistent indentation: These lines use tabs instead of spaces, which is inconsistent with the surrounding code. The indentation should match the style used elsewhere in the function.
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| { | |
| SetV6Route(lan_if,prev_lan_global_prefix); | |
| } | |
| SetV6Route(lan_if,prefix); | |
| char *token; | |
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| { | |
| SetV6Route(lan_if,prev_lan_global_prefix); | |
| } | |
| SetV6Route(lan_if,prefix); | |
| char *token; |
| } | ||
|
|
||
| // Function sets the route and assign the ULA address to lan interfaces | ||
| #define PSM_HOTSPOT_WAN_IFNAME "dmsb.wanmanager.if.3.Name" |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The macro PSM_HOTSPOT_WAN_IFNAME is defined right before the function routeset_ula. This placement is unusual and makes it easy to miss. Consider moving this macro definition to the top of the file alongside PSM_MESH_WAN_IFNAME (line 174) within the WAN_FAILOVER_SUPPORTED ifdef block, since both macros serve similar purposes and are used together in WAN failover scenarios.
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | ||
| { | ||
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prev_lan_global_prefix, sizeof(prev_lan_global_prefix)); | ||
| } |
Copilot
AI
Jan 20, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional check on line 2463 and 2490 uses strcmp with wan_interface, hotspot_wan_ifname, and mesh_wan_ifname without verifying these strings are non-empty. If PSM_VALUE_GET_STRING fails or returns empty strings, strcmp could be comparing empty strings. Consider adding checks to ensure these variables contain valid interface names before using them in strcmp operations, or checking for empty strings within the condition.
| if ( (strcmp(wan_interface, hotspot_wan_ifname) == 0) || (strcmp(wan_interface, mesh_wan_ifname) == 0)) | |
| { | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prev_lan_global_prefix, sizeof(prev_lan_global_prefix)); | |
| } | |
| if (wan_interface[0] != '\0' && | |
| ((hotspot_wan_ifname[0] != '\0' && strcmp(wan_interface, hotspot_wan_ifname) == 0) || | |
| (mesh_wan_ifname[0] != '\0' && strcmp(wan_interface, mesh_wan_ifname) == 0))) | |
| { | |
| sysevent_get(sr->sefd, sr->setok, "lan_prefix", prev_lan_global_prefix, sizeof(prev_lan_global_prefix)); | |
| } |
Reason for change:
DROP rules has been added only if CURRENT WAN IFNAME is same as DEFAULT WAN IFNAME
Priority: P1
1.Is this a Bug or a User Story (US)?
This is a Bug - RDKB-63035
If it is a User Story:
Have all dependent PRs from other components been listed (if any)?
Does the commit message include both the User Story ticket and the Subtask ticket?
Will be all changes related to the User Story squashed and merged in a single commit?
Has the PR been raised only after completing all changes for the User Story (no partial changes)?
Has code development for the User Story been completed?
If yes, has the Gerrit topic or list of all dependent PRs across components (including meta-layer changes) been shared?
NOT APPLICABLE
Is there a validation log available in the Jira ticket for verifying builds with the updated generic-srcrev.inc across all platforms?
Yes
If yes, have the links to validation comments been shared?
Validation logs have been attached
image
Jenkins link:
https://gerrit.teamccp.com/#/c/943667/
Verification Build

