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

drivers: net: native_sim_sockets: new driver #65116

Merged

Conversation

mniestroj
Copy link
Member

@mniestroj mniestroj commented Nov 13, 2023

Add driver for 'native_sim' target that implements offloaded socket
networking by the use of host networking stack and wrapped BSD sockets API.

This driver has following advantages over existing networking drivers for
emulated platforms that are already in tree:

  • no TUN/TAP use means that no additional setup is required on the host
    side:
    • possible to use it within unpriviledged Docker containers, either for
      development or in CI
  • possibility to use and test offloaded sockets
    (CONFIG_NET_SOCKETS_OFFLOAD=y) with emulated target, which allows
    to increase tests coverage of this feature, without requirement of using
    hardware

Native Simulator host libc has different error codes than embedded libc
used by Zephyr. Convert between those.

Depends on (and includes):


Doc preview:
https://builds.zephyrproject.io/zephyr/pr/65116/docs/boards/posix/native_sim/doc/index.html#nsim-per-offloaded-sockets
https://builds.zephyrproject.io/zephyr/pr/65116/docs/connectivity/networking/native_sim_setup.html#using-offloaded-sockets

Comment on lines 47 to 64
struct nsim_sockaddr {
uint16_t sa_family; /* Address family */
char sa_data[]; /* Socket address */
};

struct nsim_sockaddr_in {
uint16_t sin_family; /* AF_INET */
uint16_t sin_port; /* Port number */
uint32_t sin_addr; /* IPv4 address */
};

struct nsim_sockaddr_in6 {
uint16_t sin6_family; /* AF_INET6 */
uint16_t sin6_port; /* Port number */
uint32_t sin6_flowinfo; /* IPv6 flow info */
uint8_t sin6_addr[16];
uint32_t sin6_scope_id; /* Set of interfaces for a scope */
};
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to have separate nsim sockaddr structs here? I mean sockaddr is a sockaddr always, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem that the introduction of struct nsim_* is trying to solve is to have a common layer between the two libc worlds:

  • Native Simulator, which is compiled with host libc
  • Zephyr, which is compiled with embedded libc

In order to pass sockaddr values between those two worlds, we need a structure that is defined the same way. This is where structs like struct nsim_sockaddr_in6 are used, since they are locally defined and libc-independent. This is just to provide bridge between two conflicting libc implementations.


#include <errno.h>

#define NSIM_EPERM 1 /**< Not owner */
Copy link
Member

Choose a reason for hiding this comment

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

Can't we say here

#define NSIM_EPERM EPERM
....

Copy link
Member Author

Choose a reason for hiding this comment

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

Similar as described in #65116 (comment), NSIM_* values are defined to bridge between Native Simulator and Zephyr libc implementations. If we would define #define NSIM_EPERM EPERM, NSIM_EPERM value would be potentially different in Zephyr, when there is embedded libc used, and different in Native Simulator when there is host libc used. So by defining #define NSIM_EPERM 1 we make sure that the value is same in those two contexts.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, don't we need to just convert the errno values between zephyr <-> host, so in either zephyr or in host, meaning we could use the errno value defined in one of them?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, don't we need to just convert the errno values between zephyr <-> host, so in either zephyr or in host, meaning we could use the errno value defined in one of them?

When building for host, we don't have access to Zephyr's errno.h. When building for Zephyr, we don't have access to host's errno.h. Even if we did, there would be symbol conflicts. So the NSOS_MID_ (previously NSIM_) in nsos_errno.h are supposed to be available to both host and Zephyr, making sure that their values are exactly the same, regardless of used libc implementation.

Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

A few comments so far

General requests:

Note: I focus on the "native_simulator" side of things. For the networking side other people are better.

drivers/net/Kconfig Outdated Show resolved Hide resolved
drivers/net/Kconfig Show resolved Hide resolved
drivers/net/native_sim.h Outdated Show resolved Hide resolved
drivers/net/native_sim.h Outdated Show resolved Hide resolved
drivers/net/native_sim_sockets.c Outdated Show resolved Hide resolved
drivers/net/native_sim_adapt.c Outdated Show resolved Hide resolved
drivers/net/native_sim_errno.c Outdated Show resolved Hide resolved
drivers/net/native_sim_adapt.c Outdated Show resolved Hide resolved
drivers/net/native_sim_adapt.c Outdated Show resolved Hide resolved
drivers/net/native_sim_adapt.c Outdated Show resolved Hide resolved
Copy link
Member

@aescolar aescolar left a comment

Choose a reason for hiding this comment

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

Some extra comments

drivers/net/native_sim_sockets.c Outdated Show resolved Hide resolved
drivers/net/native_sim_adapt.c Outdated Show resolved Hide resolved
@aescolar aescolar added the Maintainer At least 2 days before merge, maintainer review required label Nov 13, 2023
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jan 13, 2024
@beriberikix
Copy link

Please remove the stale label. Thanks!

@aescolar aescolar removed the Stale label Jan 15, 2024
@zephyrbot zephyrbot added the area: Base OS Base OS Library (lib/os) label Feb 23, 2024
@mniestroj mniestroj force-pushed the native-sim-offloaded-sockets branch 4 times, most recently from 7f434fb to bd95400 Compare February 23, 2024 15:55
@mniestroj mniestroj force-pushed the native-sim-offloaded-sockets branch from 822c363 to 41f55a5 Compare March 1, 2024 15:38
aescolar
aescolar previously approved these changes Mar 2, 2024
@aescolar
Copy link
Member

aescolar commented Mar 2, 2024

The compliance check is a false positive which will need waiving.


@jukkar @rlubos To me it looks good, your review and blessing as networking maintainers would be the missing step.

Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Needs converting to hwmv2

@nordicjm nordicjm removed the hwmv2-likely-conflict DNM until collab-hwmv2 has been merged label Mar 4, 2024
@aescolar
Copy link
Member

aescolar commented Mar 4, 2024

Please rebase this PR.
Now that the hwmv2 branch has been merged this PR needs rebasing on top of the latest main.
Note that the boards & socs folder has moved, and many boards names as used in tests yaml
and overlays have changed.

@mniestroj mniestroj force-pushed the native-sim-offloaded-sockets branch from 41f55a5 to 7b99a03 Compare March 19, 2024 16:44
@zephyrbot zephyrbot requested a review from pdgendt March 19, 2024 16:45
Add driver for 'native_sim' target that implements offloaded socket
networking by the use of host networking stack and wrapped BSD sockets API.

This driver has following advantages over existing networking drivers for
emulated platforms that are already in tree:
 * no TUN/TAP use means that no additional setup is required on the host
   side:
   * possible to use it within unpriviledged Docker containers, either for
     development or in CI
 * possibility to use and test offloaded sockets
   (CONFIG_NET_SOCKETS_OFFLOAD=y) with emulated target, which allows
   to increase tests coverage of this feature, without requirement of using
   hardware

Native Simulator host libc has different error codes than embedded libc
used by Zephyr. Convert between those.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Extend driver to support DNS by offloading getaddrinfo() and freeaddrinfo()
APIs.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Add support for IPv6 socket offloading, next to existing IPv4 support.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
This allows to use Zephyr TLS subsystem with Native Sim offloaded sockets.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Use NSI_HW_EVENT() in order to periodically check for events in host
sockets. Whenever there is a socket event ready to be processed by Zephyr,
raise native_sim (newly introduced) CPU interrupt, so that Zephyr driver
can signal readiness with k_poll().

Maintain a list of Zephyr poll() executions in Zephyr context. Iterate
through them whenever there is some event to be processed.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
So far only non-blocking accept() and recvfrom() were suported. This patch
implements blocking behavior, with the use of poll(fd, POLLIN) as helper
mechanism.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
mniestroj and others added 5 commits March 19, 2024 17:54
'http_get' is one of the few samples that just require connection to
internet (google.com over HTTP or HTTPS) and check if HTTP GET was
successful. Use it for minimal coverage of Native Simulator offloaded
sockets:

 * DNS resolving
 * basic TCP transfer
 * compatibility with Zephyr TLS subsystem (which required some fcntl()
   operations)

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
'native_sim' builds without any issues as part of twister and it also works
when manually setting up Ethernet virtual device (using
tools/net-tools/net-setup.sh).

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Add configuration for Native Simulator offloaded sockets.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Document how offloaded sockets can be used with 'native_sim' for networking
capability.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Document offloaded sockets driver, which is an alternative to Ethernet
driver for networking applications.

Signed-off-by: Marcin Niestroj <m.niestroj@emb.dev>
Co-authored-by: Alberto Escolar Piedras <alberto.escolar.piedras@nordicsemi.no>
@aescolar
Copy link
Member

(The checkpatch warning is a false positive which should be waived)

@nordicjm the PR has been rebased, please re-review or remove the rejection
@rlubos @jukkar your review as maintainers of the networking subsystem would be very welcome

Copy link
Collaborator

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

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

Build system parts OK

@carlescufi carlescufi merged commit 0f5b5b6 into zephyrproject-rtos:main Mar 22, 2024
25 of 26 checks passed
@mniestroj mniestroj deleted the native-sim-offloaded-sockets branch March 22, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Base OS Base OS Library (lib/os) area: native port Host native arch port (native_posix) area: Networking area: Samples Samples area: Sockets Networking sockets Maintainer At least 2 days before merge, maintainer review required
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants