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

Request to add more return values for 'esp_netif_receive'. (IDFGH-9398) #10770

Closed
Zmmfly opened this issue Feb 15, 2023 · 2 comments
Closed

Request to add more return values for 'esp_netif_receive'. (IDFGH-9398) #10770

Zmmfly opened this issue Feb 15, 2023 · 2 comments
Assignees
Labels
Resolution: Done Issue is done internally Status: Done Issue is done internally Type: Feature Request Feature request for IDF

Comments

@Zmmfly
Copy link

Zmmfly commented Feb 15, 2023

Is your feature request related to a problem?

In PPPoS application, if baudrate >= 921600, the data processing speed is less than the data arrival speed; sometimes, 'esp_netif_receive' will return ESP_OK, but actually the reception is not successful, which will cause the PPP link to be lost.

Describe the solution you'd like.

'esp_netif_receive' returns ESP_FAIL or other values for error handling when receiving fails, which will improve stability when hardware flow control pins are not connected and no software flow control.

Describe alternatives you've considered.

  1. I tried to increase the size of lwip pbuf, but because of the serial port fifo size and no idle interrupt, the amount of data notified each time is very small, and the pbuf can always be exhausted, so it is unstable.
  2. I tried to add a pre-filter before the data enters 'esp_netif_receive', and call 'esp_netif_receive' after receiving the complete PPP packet. Although there is an improvement, it is also unstable because I am not familiar with the structure of the PPP packet.
  3. Connect the flow control pin, this is a solution mentioned in an issue, and it works:
    pppos_example: Modem Disconnect from PPP Server (IDFGH-8640) #10085 (comment)

Additional context.

  1. esp-netif_lwip-ppp: pppos_input_tcpip failed with -1 (IDFGH-5466) #7205
  2. pppos_example: Modem Disconnect from PPP Server (IDFGH-8640) #10085
@Zmmfly Zmmfly added the Type: Feature Request Feature request for IDF label Feb 15, 2023
@github-actions github-actions bot changed the title Request to add more return values for 'esp_netif_receive'. Request to add more return values for 'esp_netif_receive'. (IDFGH-9398) Feb 15, 2023
@espressif-bot espressif-bot added the Status: Opened Issue is new label Feb 15, 2023
@david-cermak
Copy link
Collaborator

Hi @Zmmfly

Thank you for this feature request, it makes perfect sense (we might even consider it a bugfix, since we should report problems to upper layers, especially when it's not restricted by lwIP types/API, but fully under IDF control).
Unfortunately this might be a breaking change, so will have to think about deploying this change (in v5.1? for users who have their custom network drivers working on v5.0 -- it would still work, but compile with warnings).

Unrelated to the request, but an idea to explore maybe, since you linked some PPPoS issues with memory. You can also check if this change helps a bit? #10719 (comment)

One more note, it might be possible to work around the esp_netif layers and create a custom input function, that would pass the packets directly to lwip:

err_t my_esp_netif_receive(esp_netif_t *esp_netif, void *buffer, size_t len, void *eb)
{
    auto *netif_internal = (struct esp_netif_obj*)esp_netif;
    auto *related = (struct lwip_peer2peer_ctx*)netif_internal->related_data;
    return pppos_input_tcpip(related->ppp, (u8_t *)buffer, len);
}

(not recommended, as directly interfacing lwip and hijacking internal structs of esp-netif)

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Mar 27, 2023
@Zmmfly
Copy link
Author

Zmmfly commented Apr 3, 2023

Hi @david-cermak
Thank you for your reply; I am very sorry to reply so late; regarding this breaking change, I am also suggesting that it be implemented in v5.1 and be compatible with older versions;

Regarding the Issue you mentioned (#10719 (comment)), in the current project, which uses PPPoS and can ensure sufficient memory, the same problem still occurs;

Yes, I did a similar job, modifying a version of my own: adding return values, multiple retries in the upper level calls after receiving failures, and still failing after reaching the specified number of retries before finally dropping the packet.

Finally, in PPPoS high-rate applications, puts uart interrupts into IRAM can only be a workaround in the absence of hardware flow control, the most secure or use hardware flow control.

@espressif-bot espressif-bot added Status: Reviewing Issue is being reviewed and removed Status: In Progress Work is in progress labels Apr 11, 2023
@espressif-bot espressif-bot added Resolution: NA Issue resolution is unavailable Status: Done Issue is done internally Resolution: Done Issue is done internally and removed Status: Reviewing Issue is being reviewed Resolution: NA Issue resolution is unavailable labels Apr 20, 2023
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: Feature Request Feature request for IDF
Projects
None yet
Development

No branches or pull requests

3 participants