Skip to content

io fixes on non-blocking FDs#175

Merged
michalvasko merged 1 commit intoCESNET:masterfrom
6WIND:io-eagain
Oct 16, 2019
Merged

io fixes on non-blocking FDs#175
michalvasko merged 1 commit intoCESNET:masterfrom
6WIND:io-eagain

Conversation

@rjarry
Copy link
Copy Markdown
Contributor

@rjarry rjarry commented Oct 15, 2019

These are fixes for problems I encountered with the UNIX transport. Basically, when sending large configurations to netopeer2-server, since the socket fd is non-blocking, we can get EAGAIN errors upon write() if the socket buffer is full and the server does not read fast enough.

These errors are already handled in libnetconf2 but errno is not reset and libyang checks it (since commit CESNET/libyang@609d708). This causes the operation to fail with the following error:

Print error (Resource temporarily unavailable).

Work around the problem by resetting errno to 0 in case of EAGAIN or EINTR.

Also, if the error is EINTR do not wait before trying to read/write again.

I am not sure this is the correct approach, I proposed another (maybe complementary?) solution in CESNET/libyang#903

@codecov-io
Copy link
Copy Markdown

Codecov Report

Merging #175 into master will decrease coverage by 0.05%.
The diff coverage is 17.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #175      +/-   ##
==========================================
- Coverage   38.14%   38.09%   -0.06%     
==========================================
  Files          23       23              
  Lines        9962     9976      +14     
  Branches     1949     1952       +3     
==========================================
  Hits         3800     3800              
- Misses       5217     5226       +9     
- Partials      945      950       +5
Impacted Files Coverage Δ
src/io.c 44.9% <17.64%> (-0.58%) ⬇️
src/session_server_ssh.c 24.31% <0%> (-0.13%) ⬇️
src/session_server_tls.c 28.24% <0%> (-0.11%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 287043a...42026ae. Read the comment docs.

@brchiu
Copy link
Copy Markdown
Contributor

brchiu commented Oct 15, 2019

@rjarry
Thanks for addressing this issue. I also encountered a similar one for SSH case with Print error (No such file or directory) error message. Can your approach apply to it ?

Comment thread src/io.c Outdated
@michalvasko
Copy link
Copy Markdown
Member

Hi,
depending on how we resolve CESNET/libyang#903, resetting errno may not be needed.

Regards,
Michal

@rjarry
Copy link
Copy Markdown
Contributor Author

rjarry commented Oct 15, 2019

@rjarry
Thanks for addressing this issue. I also encountered a similar one for SSH case with Print error (No such file or directory) error message. Can your approach apply to it ?

@brchiu I don't think so, I only handle EAGAIN and EINTR, No such file or directory is ENOENT and is a real error.

When interrupted during a read/write syscall, do not wait before trying
again. Only wait if the error was EAGAIN which means that the other side
of the socket is not ready.

Signed-off-by: Robin Jarry <robin.jarry@6wind.com>
@rjarry
Copy link
Copy Markdown
Contributor Author

rjarry commented Oct 15, 2019

depending on how we resolve CESNET/libyang#903, resetting errno may not be needed.

Indeed, I removed the first commit.

@michalvasko
Copy link
Copy Markdown
Member

Hi,
it is fine now, thanks.

Regards,
Michal

@michalvasko michalvasko merged commit 373a40a into CESNET:master Oct 16, 2019
@rjarry
Copy link
Copy Markdown
Contributor Author

rjarry commented Oct 16, 2019

👍

@rjarry rjarry deleted the io-eagain branch October 16, 2019 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants