Skip to content

NET_SOCKETS_OFFLOAD conflicts with POSIX_API #26033

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

Closed
aport opened this issue Jun 7, 2020 · 6 comments · Fixed by #26129
Closed

NET_SOCKETS_OFFLOAD conflicts with POSIX_API #26033

aport opened this issue Jun 7, 2020 · 6 comments · Fixed by #26129
Assignees
Labels
area: Networking area: POSIX POSIX API Library bug The issue is a bug, or the PR is fixing a bug

Comments

@aport
Copy link
Contributor

aport commented Jun 7, 2020

Describe the bug
When using NET_SOCKETS_OFFLOAD, the symbol NET_SOCKETS_POSIX_NAMES is selected. This symbol conflicts with POSIX_API.

To Reproduce
Steps to reproduce the behavior:

  1. Select CONFIG_NET_SOCKETS_OFFLOAD=y
  2. Select CONFIG_POSIX_API=y
  3. west build
  4. See error in configuration conflict

Expected behavior
I should be able to use NET_SOCKETS_OFFLOAD and POSIX_API at the same time. NET_SOCKETS_OFFLOAD should not depend on NET_SOCKETS_POSIX_NAMES.

Impact
I cannot use POSIX_API when using NET_SOCKETS_OFFLOAD.

Additional context
A similar issue was reported in #17353. It sounds like POSIX_API is the "preferred" way of using POSIX BSD sockets in my application. Perhaps NET_SOCKETS_POSIX_NAMES should be deprecated and removed, with breakages fixed where necessary.

@aport aport added the bug The issue is a bug, or the PR is fixing a bug label Jun 7, 2020
@rlubos
Copy link
Collaborator

rlubos commented Jun 8, 2020

Since socket offloading is now done underneath zsock_* API, select NET_SOCKETS_POSIX_NAMES in NET_SOCKETS_OFFLOAD option is just a historical residue, it should be safe to remove it from there. Of course, some of the samples, that relied on NET_SOCKETS_POSIX_NAMES being enabled automatically will need some minor tweaks in prj.conf (not sure how many of them, if any).

As for deprecating NET_SOCKETS_POSIX_NAMES, it sounds like a good idea to do some cleanup, I'm not sure yet though how big task that would be (i. e. if there are any other consequences other than switching to a different header).

@aport
Copy link
Contributor Author

aport commented Jun 8, 2020

It looks like the ublox-sara-r4 driver breaks when NET_SOCKETS_POSIX_NAMES is not selected, because it uses POSIX types and functions.

Should drivers like these include <posix/poll.h> and other associated headers? Or should they instead use the native Zephyr types zsock_pollfd and zsock_poll() and not muck about with the POSIX abstractions?

@carlescufi
Copy link
Member

@aport is this now closed by #26061 or do we need additional clarifications?

@aport
Copy link
Contributor Author

aport commented Jun 11, 2020

@carlescufi as far as I can tell two other drivers will fail to build if the dependency between NET_SOCKETS_OFFLOAD and NET_SOCKETS_POSIX_NAMES is cut.

the files drivers/wifi/simplelink/simplelink_sockets.c and drivers/wifi/eswifi/eswifi_socket_offload.c are built when NET_SOCKETS_OFFLOAD is selected, but use raw POSIX types so they should be migrated as well.

I do not have that hardware but I would be happy to submit a PR for review.

@carlescufi
Copy link
Member

@carlescufi as far as I can tell two other drivers will fail to build if the dependency between NET_SOCKETS_OFFLOAD and NET_SOCKETS_POSIX_NAMES is cut.

the files drivers/wifi/simplelink/simplelink_sockets.c and drivers/wifi/eswifi/eswifi_socket_offload.c are built when NET_SOCKETS_OFFLOAD is selected, but use raw POSIX types so they should be migrated as well.

I do not have that hardware but I would be happy to submit a PR for review.

Thank you very much for the PR. I will wait until the TI maintainer has approved and then I'll merge it.

@aport
Copy link
Contributor Author

aport commented Jun 12, 2020

Just to clarify, this specific issue should be closed when NET_SOCKETS_OFFLOAD no longer selects NET_SOCKETS_POSIX_NAMES. The two PRs I've submitted address the build errors that this removal would have introduced.

So if #26129 is okay and merged, there will be one more PR that modifies the Kconfig to remove this dependency.

I was going to add the change to the PR in question but I felt the changes should be isolated. I could add the Kconfig change as a separate commit to PR #26129 if the team feels that is a better option. Please let me know.

carlescufi pushed a commit that referenced this issue Jun 15, 2020
This change removes references to raw POSIX types and functions,
allowing the drivers to build without NET_SOCKETS_POSIX_NAMES.

After this, the dependency between NET_SOCKETS_OFFLOAD and
NET_SOCKETS_POSIX_NAMES can be removed.

See issue #26033 for additional context

Signed-off-by: Adam Porter <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Networking area: POSIX POSIX API Library bug The issue is a bug, or the PR is fixing a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants