Skip to content
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

CONFIG_LWIP_L2_TO_L3_COPY unnecessary to use NAPT feature in lwip (IDFGH-11763) #12861

Closed
3 tasks done
RobertGnz opened this issue Dec 22, 2023 · 1 comment
Closed
3 tasks done
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Bug bugs in IDF

Comments

@RobertGnz
Copy link

RobertGnz commented Dec 22, 2023

Answers checklist.

  • I have read the documentation ESP-IDF Programming Guide and the issue is not addressed there.
  • I have updated my IDF branch (master or release) to the latest version and checked that the issue is present there.
  • I have searched the issue tracker for a similar issue and not found a similar issue.

IDF version.

5.1.2

Operating System used.

Windows

How did you build your project?

Eclipse IDE

If you are using Windows, please specify command line type.

None

What is the expected behavior?

1- As stated in the documentation I enabled CONFIG_LWIP_IP_FORWARD and CONFIG_LWIP_IPV4_NAPT and CONFIG_LWIP_L2_TO_L3_COPY to get napt functionnality. The result of it was that napt worked as expected.

2- In another test I enabled CONFIG_LWIP_IP_FORWARD and CONFIG_LWIP_IPV4_NAPT but not CONFIG_LWIP_L2_TO_L3_COPY. And the result of it was also that napt worked as expected.

Conclusion: enabling CONFIG_LWIP_L2_TO_L3_COPY seems to be unnecessary (despite what the docuentation says). So it could be interesting to update code and documentation to remove the need of CONFIG_LWIP_L2_TO_L3_COPY.

What is the actual behavior?

lwip file: wlanif.c
function: esp_netif_recv_ret_t wlanif_input(void *, void , size_t, void )

1- with CONFIG_LWIP_L2_TO_L3_COPY:

#ifdef CONFIG_LWIP_L2_TO_L3_COPY
p = pbuf_alloc(PBUF_RAW, len, PBUF_RAM);
if (p == NULL) {
esp_netif_free_rx_buffer(esp_netif, l2_buff);
return ESP_NETIF_OPTIONAL_RETURN_CODE(ESP_ERR_NO_MEM);
}
memcpy(p->payload, buffer, len);
esp_netif_free_rx_buffer(esp_netif, l2_buff);
#else
p = esp_pbuf_allocate(esp_netif, buffer, len, l2_buff);
if (p == NULL) {
esp_netif_free_rx_buffer(esp_netif, l2_buff);
return ESP_NETIF_OPTIONAL_RETURN_CODE(ESP_ERR_NO_MEM);
}
#endif

=> napt works as expected

2- without CONFIG_LWIP_L2_TO_L3_COPY:

#undef CONFIG_LWIP_L2_TO_L3_COPY // to desable CONFIG_LWIP_L2_TO_L3_COPY

#ifdef CONFIG_LWIP_L2_TO_L3_COPY
p = pbuf_alloc(PBUF_RAW, len, PBUF_RAM);
if (p == NULL) {
esp_netif_free_rx_buffer(esp_netif, l2_buff);
return ESP_NETIF_OPTIONAL_RETURN_CODE(ESP_ERR_NO_MEM);
}
memcpy(p->payload, buffer, len);
esp_netif_free_rx_buffer(esp_netif, l2_buff);
#else
p = esp_pbuf_allocate(esp_netif, buffer, len, l2_buff);
if (p == NULL) {
esp_netif_free_rx_buffer(esp_netif, l2_buff);
return ESP_NETIF_OPTIONAL_RETURN_CODE(ESP_ERR_NO_MEM);
}
#endif

=> CONFIG_LWIP_L2_TO_L3_COPY desabled BUT napt works as expected.

Seems that CONFIG_LWIP_L2_TO_L3_COPY is unnecessary.

Steps to reproduce.

  1. Step
  2. Step
  3. Step
    ...

Build or installation Logs.

No response

More Information.

No response

@RobertGnz RobertGnz added the Type: Bug bugs in IDF label Dec 22, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label Dec 22, 2023
@github-actions github-actions bot changed the title CONFIG_LWIP_L2_TO_L3_COPY unnecessary to use NAPT feature in lwip CONFIG_LWIP_L2_TO_L3_COPY unnecessary to use NAPT feature in lwip (IDFGH-11763) Dec 22, 2023
@espressif-bot espressif-bot added Status: In Progress Work is in progress Status: Reviewing Issue is being reviewed and removed Status: Opened Issue is new Status: In Progress Work is in progress labels Jan 16, 2024
@espressif-bot espressif-bot added Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Reviewing Issue is being reviewed labels Jan 25, 2024
@Alvin1Zhang
Copy link
Collaborator

Thanks for reporting, fix is available at dfb1e0e, feel free to reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Bug bugs in IDF
Projects
None yet
Development

No branches or pull requests

4 participants