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

Sending large payloads over PPPInterface socket fails #5774

Closed
janjongboom opened this issue Jan 2, 2018 · 7 comments
Closed

Sending large payloads over PPPInterface socket fails #5774

janjongboom opened this issue Jan 2, 2018 · 7 comments

Comments

@janjongboom
Copy link
Contributor

Description

  • Type: Bug

Bug

Target
Any target implementing PPPInterface

Toolchain:
All

mbed-os sha:
Mbed OS 5.7

When you create a request in mbed-http it creates a buffer, which it then sends over HTTP. There is no streaming or anything involved, and as long as there's enough memory available that's fine. However... A user has found out that when you send a file larger than 1400 bytes over PPPInterface the request fails, as the UART buffer used to communicate with the cellular modem gets full. I believe that splitting the payload up should be done by the driver in this case, as I don't know the optimal buffer size. I think this is reproducable by just doing:

TCPSocket socket; // attach it etc.
char buffer[10000] = { 1 };
socket.send(buffer, 10000);

Bug report here.

@ciarmcom
Copy link
Member

ciarmcom commented Jan 3, 2018

ARM Internal Ref: IOTMORF-1855

@0xc0170
Copy link
Contributor

0xc0170 commented Jan 3, 2018

cc @hasnainvirk @kjbracey-arm

@kjbracey
Copy link
Contributor

kjbracey commented Jan 3, 2018

I'm going to start by hoping that this isn't an actual bug, and go in with an API explanation.

In general a send is allowed to perform a partial write, rather than block until completion.

See https://stackoverflow.com/questions/12657063/does-send-allways-send-whole-buffer

However, the behaviour in mbed OS has actually changed recently.

In earlier versions of mbed OS, a blocking socket would actually only block until there was some buffer space available, then fill as much buffer space as it could, then return. So a request to send 10K would probably only do a small amount on most platforms. Amount taken would be limited by lwIP buffer space.

This PR modified behaviour, but it only went into mbed OS 5.7: #5466

In POSIX a write on a blocking file descriptor can exit early if interrupted by a signal. Any program that could need to continue after any signal would need to check for this and call send/write in a loop. But aside from signal interruption, write on a stream socket should not otherwise return early. Reference: http://pubs.opengroup.org/onlinepubs/009695399/functions/write.html

As mbed OS doesn't have signals, there should be no possibility of early return, so users shouldn't have to worry about that loop, so blocking socket write should run to completion. PR#5466 modified that for mbed OS 5.7, but for mbed OS 5.6 and earlier, use a loop:

char buffer[10000] = { 1 };
ssize_t count = 0;
while (count < sizeof buffer) {
    ssize_t ret = socket.send(buffer + count, sizeof buffer - count);
    if (ret < 0) {
        // error handling
    }
    count += ret;
 }

Does that work?

In mbed OS 5.7 or later, the send should block until 10000 is sent.

@janjongboom
Copy link
Contributor Author

janjongboom commented Jan 3, 2018

@kjbracey-arm Thanks, have asked the user. I don't have any boards on me that implement PPPInterface.

I'll see if I can change the behavior for Mbed OS 5.6 in mbed-http.

@kjbracey
Copy link
Contributor

kjbracey commented Jan 3, 2018

I would expect the same basic behaviour to be seen on any network interface. I wouldn't expect 10K to go in 1 go on K64F Ethernet, for example. (Or if it does, then try 40K...)

Assuming that is the only problem, and there isn't some other PPP-specific thing.

@janjongboom
Copy link
Contributor Author

janjongboom commented Jan 3, 2018

I can indeed reproduce the same issue on K64F LWIP on Mbed OS 5.6.6, does not happen on Mbed OS 5.7.1.

Will post a fix in mbed-http too.

@janjongboom
Copy link
Contributor Author

Fixed in mbed-http here: https://os.mbed.com/teams/sandbox/code/mbed-http/rev/71fc1b1894f8/

Issue does not reproduce on Mbed OS 5.7 anymore.

ATM-HSW pushed a commit to ATM-HSW/libHttp that referenced this issue Jun 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants