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

[PAL/Linux-SGX] Add ECONNRESET to errors allowed from ocall_recv #709

Merged
merged 1 commit into from
Jun 30, 2022

Conversation

boryspoplawski
Copy link
Contributor

@boryspoplawski boryspoplawski commented Jun 30, 2022

Description of the changes

This cause ECONNRESET to be translated to EPERM and confused the app. For example lighttpd printed error messaged, but continued to work.

How to test this PR?

Try running lighttpd example as we do in CI.


This change is Reviewable

Base automatically changed from borys/rename_shim_to_libos_files to master June 30, 2022 12:29
dimakuv
dimakuv previously approved these changes Jun 30, 2022
Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required)

a discussion (no related file):
Interesting that we already had this ECONNRESET in ocall_send() but not in ocall_recv(). Surely because of the man pages: it is mentioned in send, but not mentioned in recv.


mkow
mkow previously approved these changes Jun 30, 2022
Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

Copy link
Member

@mkow mkow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, all discussions resolved, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL)

Copy link
Contributor

@vijaydhanraj vijaydhanraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion, not enough approvals from maintainers (1 more required), not enough approvals from different teams (1 more required, approved so far: ITL) (waiting on @boryspoplawski)

a discussion (no related file):
Thanks @boryspoplawski tested this PR with #693 and I no longer see below messages when I run lighttpd example.

> 02:17:17: connections.c.919) connection closed - read failed: Permission denied`

Copy link
Contributor

@dimakuv dimakuv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @boryspoplawski)

Copy link
Contributor Author

@boryspoplawski boryspoplawski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @vijaydhanraj)

a discussion (no related file):

Previously, vijaydhanraj (Vijay Dhanraj) wrote…

Thanks @boryspoplawski tested this PR with #693 and I no longer see below messages when I run lighttpd example.

> 02:17:17: connections.c.919) connection closed - read failed: Permission denied`

@vijaydhanraj Why is this a blocking comment?


Copy link
Contributor

@vijaydhanraj vijaydhanraj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

a discussion (no related file):

Previously, boryspoplawski (Borys Popławski) wrote…

@vijaydhanraj Why is this a blocking comment?

No, I didn't intend it to be blocking. I have resolved it.


@boryspoplawski boryspoplawski merged commit f0419a8 into master Jun 30, 2022
@boryspoplawski boryspoplawski deleted the borys/translate_econreset branch June 30, 2022 22:52
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