Skip to content

Deprecate NET_SOCKETS_POSIX_NAMES option #69950

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 21 commits into from
Mar 27, 2024
Merged

Deprecate NET_SOCKETS_POSIX_NAMES option #69950

merged 21 commits into from
Mar 27, 2024

Conversation

jukkar
Copy link
Member

@jukkar jukkar commented Mar 8, 2024 •
edited
Loading

The CONFIG_NET_SOCKETS_POSIX_NAMES option is marked as deprecated in favor of using normal POSIX socket API includes found under the include/zephyr/posix directory. If you want to use BSD socket API calls, you need to select POSIX_API and use the socket headers found in the POSIX subsystem. If you do not want to or cannot enable POSIX_API, then you must use zsock_ prefix when working with BSD socket calls.

This is initial work in order to simplify the network and Posix related config options. The use of CONFIG_NET_SOCKETS_POSIX_NAMES is currently confusing as it conflicts with CONFIG_POSIX_API.

@jukkar jukkar force-pushed the devel/deprecate-net-socket-posix-names branch from 0ef2c42 to 7f26700 Compare March 8, 2024 08:59
rlubos
rlubos reviewed Mar 8, 2024
View reviewed changes
Copy link
Collaborator

@rlubos rlubos left a comment

Choose a reason for hiding this comment

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

I really really like the idea to finally clean this up.

include/zephyr/net/socket.h

return ret;
}

/** POSIX wrapper for @ref zsock_sendto */
static inline ssize_t sendto(int sock, const void *buf, size_t len, int flags,
Copy link
Collaborator

@rlubos rlubos Mar 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I wonder, could we remove those duplicate wrappers already, and just use the POSIX headers below in case CONFIG_NET_SOCKETS_POSIX_NAMES is enabled?

Copy link
Member Author

Choose a reason for hiding this comment

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

If one sets CONFIG_NET_SOCKETS_POSIX_NAMES=y and CONFIG_POSIX_API=n, then there is a compilation error if these symbols are not in socket.h. It looks like we are stuck with these until the NET_SOCKETS_POSIX_NAMES is removed from the Kconfig file, and then we can remove the settings.

@jukkar jukkar force-pushed the devel/deprecate-net-socket-posix-names branch 3 times, most recently from 95f1acf to 8a8a3bb Compare March 8, 2024 15:12
jukkar added 2 commits March 27, 2024 11:00
@jukkar
tests: net: socket: tls_ext: Use POSIX_API
f6cc99b 
The test expects to use various POSIX APIs so enable the config.
Enabling POSIX_API requires also increase of max file descriptors.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
tests: net: sockets: Remove use of NET_SOCKETS_POSIX_NAMES
0eb10bf 
The setting is deprecated so change the code to either use the
native zsock_* API or enable POSIX_API to use the BSD socket API.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar jukkar force-pushed the devel/deprecate-net-socket-posix-names branch from 42e9249 to 2847384 Compare March 27, 2024 09:13
@jukkar
Copy link
Member Author

jukkar commented Mar 27, 2024

Changes:

  • update to latest main
  • disabled netusb in networking samples for native_sim board so that CI passes (can be enabled later if needed)

rlubos
rlubos previously approved these changes Mar 27, 2024
View reviewed changes
jukkar added 12 commits March 27, 2024 12:31
@jukkar
samples: net: Change to use CONFIG_POSIX_API
484f1d3 
Change the sample applications that use network socket API to
use the POSIX_API config because the NET_SOCKETS_POSIX_NAMES is
deprecated. Convert also the zsock_ API calls to plain BSD
socket API calls when applicable.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
mgmt: updatehub: Use zsock_ API functions
3d64794 
The library should be using internal socket API functions
so that we do not need to depend on POSIX_API.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
mgmt: mcumgr: transport: udp: Select POSIX_API
a652b19 
The NET_SOCKET_POSIX_NAMES is deprecated so enable POSIX_API
to use the socket API.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
doc: net: Remove the mention of NET_SOCKETS_POSIX_NAMES
23d3563 
The option is deprecated so remove it from documentation.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
doc: migration-guide-3.7: Add NET_SOCKETS_POSIX_NAMES info
dcb7869 
Add CONFIG_NET_SOCKETS_POSIX_NAMES deprecation information.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
shell: mqtt: Avoid using POSIX function names
e7c0be0 
This commit fixes this error seen in CI so that things
work even if CONFIG_POSIX_API is enabled.

subsys/shell/backends/shell_mqtt.c:727:12: error:
  conflicting types for 'write'; have
 'int(const struct shell_transport *, const void *, size_t,  size_t *)'
  727 | static int write(const struct shell_transport *transport,
                         const void *data, size_t length)

include/zephyr/posix/unistd.h:230:9: note: previous declaration
 of 'write' with type
 'ssize_t(int,  const void *, size_t)'
  230 | ssize_t write(int file, const void *buffer, size_t count);

subsys/shell/backends/shell_mqtt.c:787:12: error:
 conflicting types for 'read'; have
 'int(const struct shell_transport *, void *, size_t,  size_t *)'
  787 | static int read(const struct shell_transport *transport,
                        void *data, size_t length, size_t *cnt)

include/zephyr/posix/unistd.h:231:9: note: previous declaration
 of 'read' with type
 'ssize_t(int,  void *, size_t)'
  231 | ssize_t read(int file, void *buffer, size_t count);

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
samples: net: echo-server: Adjust CONFIG_MAX_THREAD_BYTES
cd82021 
This fixes this error printed by CI:

Too many thread objects (21)
Increase CONFIG_MAX_THREAD_BYTES to 3

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
tests: net: lwm2m: Fix zsock_fcntl impl name
79703c2 
The syscall name has _impl postfix so adjusting the stub
name.

zephyr-sdk-0.16.5-1/x86_64-zephyr-elf/bin/../lib/gcc/
  x86_64-zephyr-elf/12.2.0/../../../../x86_64-zephyr-elf/bin/ld.bfd:
  app/libapp.a(lwm2m_engine.c.obj): in function `zsock_fcntl_impl':
.../syscalls/socket.h:363: undefined reference to `z_impl_zsock_fcntl_impl'

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
samples: net: sockets: can: Fix the test termination criteria
3a39d5f 
If enabling CONFIG_POSIX_API, we get three file descriptors open
before any socket descriptors. The sample exit criteria checks
socket 0 and 1 which is no longer a valid one, so change the
check to be either 0 or 3, and 1 or 4 so that the test can be
run in both when POSIX API is enabled or disabled.

Also disable color printing for the test run so that it is
easier to read the output log.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
net: sockets: Use only Zephyr POSIX headers
cc9eda6 
Use only Zephyr specific POSIX header files so that the whole
system is getting values from the same files. There was an issue with
native_sim run of tests/net/socket/af_packet which had O_BLOCKING set
to 0x4000 from include/zephyr/posix/fcntl.h, but then the file
subsys/net/lib/socket/sockets.c was having O_BLOCKING set to 0x0800
because different header files were used.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
net: Use always zephyr/posix/fcntl.h
7fdf630 
Changing remaining users of fcntl.h to use the include from our own
POSIX file so that the values in there are consistent in all parts
of the sources.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar
samples: net: Disable native_sim net USB driver in samples
259935c 
The native_sim USB driver gives this error currently if one
enables CONFIG_POSIX_API.

drivers/usb/device/usb_dc_native_posix_adapt.c:22:10: \
 fatal error: sys/un.h: No such file or directory
   22 | #include <sys/un.h>
      |          ^~~~~~~~~~
compilation terminated.

Disable the netusb support from networking shells for native_sim
board for now so that CI can pass.

Signed-off-by: Jukka Rissanen <[email protected]>
@jukkar jukkar force-pushed the devel/deprecate-net-socket-posix-names branch from 2847384 to 259935c Compare March 27, 2024 10:31
@jukkar
Copy link
Member Author

jukkar commented Mar 27, 2024

Changes:

  • fixed the http_get sample compilation

@jukkar jukkar requested a review from cfriedt March 27, 2024 16:03
@cfriedt
Copy link
Member

cfriedt commented Mar 27, 2024

This is rad 🤙Thanks again @jukkar

@dleach02 dleach02 merged commit a613e24 into zephyrproject-rtos:main Mar 27, 2024
25 checks passed
@jukkar jukkar deleted the devel/deprecate-net-socket-posix-names branch March 28, 2024 06:33
gminn added a commit to memfault/memfault-zephyr-http-example that referenced this pull request Apr 4, 2024
@gminn
fix(build): fix build by enabling the POSIX API
aac54c3 
 ### Summary

 Zephyr [recently deprecated](zephyrproject-rtos/zephyr#69950)
 the `CONFIG_NET_SOCKETS_POSIX_NAMES` option, which was was previously
 `y` by default when `CONFIG_POSIX_API=n` but is now n by default.
 This option matters because it adds the POSIX API to the `net/socket.h`
 header, which we reference in `memfault_platform_http.c`. This fix
 instead makes our http client code include the `posix/*` headers to get
 the POSIX API.

 ### Test Plan

 Builds successfully.
@gminn gminn mentioned this pull request Apr 4, 2024
Merged
gminn added a commit to memfault/memfault-zephyr-http-example that referenced this pull request Apr 4, 2024
@gminn
fix(build): fix build by enabling the POSIX API (#1)
ebda3d7 
### Summary

Zephyr [recently
deprecated](zephyrproject-rtos/zephyr#69950)
the `CONFIG_NET_SOCKETS_POSIX_NAMES` option, which was was previously
 `y` by default when `CONFIG_POSIX_API=n` but is now `n` by default.
 This option matters because it adds the POSIX API to the `net/socket.h`
 header, which we reference in `memfault_platform_http.c`. This fix
 instead makes our http client code include the `posix/*` headers to get
 the POSIX API.

 ### Test Plan

 Builds successfully.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kartben kartben kartben left review comments

@henrikbrixandersen henrikbrixandersen henrikbrixandersen left review comments

@cfriedt cfriedt cfriedt approved these changes

@pdgendt pdgendt pdgendt approved these changes

@rlubos rlubos rlubos approved these changes

@alexanderwachter alexanderwachter Awaiting requested review from alexanderwachter

@bjarki-andreasen bjarki-andreasen Awaiting requested review from bjarki-andreasen

@jfischer-no jfischer-no Awaiting requested review from jfischer-no

@martinjaeger martinjaeger Awaiting requested review from martinjaeger

@nandojve nandojve Awaiting requested review from nandojve

@nashif nashif Awaiting requested review from nashif

@ssharks ssharks Awaiting requested review from ssharks

@str4t0m str4t0m Awaiting requested review from str4t0m

@tbursztyka tbursztyka Awaiting requested review from tbursztyka

@ycsin ycsin Awaiting requested review from ycsin

Assignees

@rlubos rlubos

Labels
area: CAN area: mcumgr area: modbus area: Modem area: Networking area: POSIX POSIX API Library area: Samples Samples area: Sockets Networking sockets area: UpdateHub UpdateHub
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@jukkar @cfriedt @kartben @henrikbrixandersen @pdgendt @rlubos @zephyrbot @dleach02