Skip to content

net: BSD Sockets like API initial implementation #498

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

Merged
merged 13 commits into from
Jun 28, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions include/kernel.h
Original file line number Diff line number Diff line change
Expand Up @@ -1458,6 +1458,34 @@ static inline int k_queue_is_empty(struct k_queue *queue)
return (int)sys_slist_is_empty(&queue->data_q);
}

/**
* @brief Peek element at the head of queue.
*
* Return element from the head of queue without removing it.
*
* @param queue Address of the queue.
*
* @return Head element, or NULL if queue is empty.
*/
static inline void *k_queue_peek_head(struct k_queue *queue)
{
return sys_slist_peek_head(&queue->data_q);
}

/**
* @brief Peek element at the tail of queue.
*
* Return element from the tail of queue without removing it.
*
* @param queue Address of the queue.
*
* @return Tail element, or NULL if queue is empty.
*/
static inline void *k_queue_peek_tail(struct k_queue *queue)
{
return sys_slist_peek_tail(&queue->data_q);
}

/**
* @brief Statically define and initialize a queue.
*
Expand Down Expand Up @@ -1615,6 +1643,36 @@ struct k_fifo {
#define k_fifo_is_empty(fifo) \
k_queue_is_empty((struct k_queue *) fifo)

/**
* @brief Peek element at the head of fifo.
*
* Return element from the head of fifo without removing it. A usecase
* for this is if elements of the fifo are themselves containers. Then
* on each iteration of processing, a head container will be peeked,
* and some data processed out of it, and only if the container is empty,
* it will be completely remove from the fifo.
*
* @param fifo Address of the fifo.
*
* @return Head element, or NULL if the fifo is empty.
*/
#define k_fifo_peek_head(fifo) \
k_queue_peek_head((struct k_queue *) fifo)

/**
* @brief Peek element at the tail of fifo.
*
* Return element from the tail of fifo (without removing it). A usecase
* for this is if elements of the fifo are themselves containers. Then
* it may be useful to add more data to the last container in fifo.
*
* @param fifo Address of the fifo.
*
* @return Tail element, or NULL if fifo is empty.
*/
#define k_fifo_peek_tail(fifo) \
k_queue_peek_tail((struct k_queue *) fifo)

/**
* @brief Statically define and initialize a fifo.
*
Expand Down
19 changes: 15 additions & 4 deletions include/net/net_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,13 @@ struct net_conn_handle;
* anyway. This saves 12 bytes / context in IPv6.
*/
struct net_context {
/** User data.
*
* First member of the structure to let users either have user data
* associated with a context, or put contexts into a FIFO.
*/
void *user_data;

/** Reference count
*/
atomic_t refcount;
Expand Down Expand Up @@ -206,10 +213,6 @@ struct net_context {
*/
net_context_connect_cb_t connect_cb;

/** User data.
*/
void *user_data;

#if defined(CONFIG_NET_CONTEXT_NET_PKT_POOL)
/** Get TX net_buf pool for this context.
*/
Expand Down Expand Up @@ -237,6 +240,14 @@ struct net_context {
/** TCP connection information */
struct net_tcp *tcp;
#endif /* CONFIG_NET_TCP */

#if defined(CONFIG_NET_SOCKETS)
/** Per-socket packet or connection queues */
union {
struct k_fifo recv_q;
struct k_fifo accept_q;
};
#endif /* CONFIG_NET_SOCKETS */
};

static inline bool net_context_is_used(struct net_context *context)
Expand Down
20 changes: 17 additions & 3 deletions include/net/net_pkt.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,9 @@ struct net_pkt {
sys_snode_t sent_list;
#endif

u8_t sent : 1; /* Is this sent or not
u8_t sent_or_eof: 1; /* For outgoing packet: is this sent or not
* For incoming packet of a socket: last
* packet before EOF
* Used only if defined(CONFIG_NET_TCP)
*/
u8_t forwarding : 1; /* Are we forwarding this pkt
Expand Down Expand Up @@ -188,13 +190,25 @@ static inline void net_pkt_set_next_hdr(struct net_pkt *pkt, u8_t *hdr)
#if defined(CONFIG_NET_TCP)
static inline u8_t net_pkt_sent(struct net_pkt *pkt)
{
return pkt->sent;
return pkt->sent_or_eof;
}

static inline void net_pkt_set_sent(struct net_pkt *pkt, bool sent)
{
pkt->sent = sent;
pkt->sent_or_eof = sent;
}

#if defined(CONFIG_NET_SOCKETS)
static inline u8_t net_pkt_eof(struct net_pkt *pkt)
{
return pkt->sent_or_eof;
}

static inline void net_pkt_set_eof(struct net_pkt *pkt, bool eof)
{
pkt->sent_or_eof = eof;
}
#endif
#endif

#if defined(CONFIG_NET_ROUTE)
Expand Down
45 changes: 45 additions & 0 deletions include/net/socket.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright (c) 2017 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/

#ifndef __NET_SOCKET_H
#define __NET_SOCKET_H

#include <sys/types.h>
#include <zephyr/types.h>
#include <net/net_ip.h>

#ifdef __cplusplus
extern "C" {
#endif

int zsock_socket(int family, int type, int proto);
int zsock_close(int sock);
int zsock_bind(int sock, const struct sockaddr *addr, socklen_t addrlen);
int zsock_connect(int sock, const struct sockaddr *addr, socklen_t addrlen);
int zsock_listen(int sock, int backlog);
int zsock_accept(int sock, struct sockaddr *addr, socklen_t *addrlen);
ssize_t zsock_send(int sock, const void *buf, size_t len, int flags);
ssize_t zsock_recv(int sock, void *buf, size_t max_len, int flags);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be logical to place these definitions in a zsocket.h header, to be included by Zephyr applications wanting to use the native Zephyr socket symbols, and leave socket.h to define the POSIX only symbols, as below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know, this turns one header into two, and reading literally, puts a networking-related header outside net/. Why would this be better? As it stands now, everything socket-related is in one header, but apps can configure subset of the functionality needed, like with the rest of subsystems in Zephyr.

Copy link
Collaborator

Choose a reason for hiding this comment

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

A few reasons:

  1. It's often a standard module convention to have symbols declared in their own header. Makes it easy to find things. (I know, Zephyr doesn't always follow this convention).
  2. These are Zephyr APIs, which may have divergence from true POSIX APIs (declared in socket.h).
  3. It makes it easier to implement a new socket provider for the socket APIs for offloading. zsocket could just be one (the default) socket provider. Socket.h (or socket.c) would be the switch layer.

I assumed zsocket.h would also go into net/.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's often a standard module convention to have symbols declared in their own header. Makes it easy to find things.

Well, that's actually mu biggest concern - if I'd see socket.h, I would never think there's also zsocket.h, or vice-versa. Zephyr doesn't split IPv4 and IPv6 headers. lwIP defines both native prefixed functions (lwip_recv(), etc.) and POSIXy macros in the same header, sockets.h. And in this case, it's also the single module, it just defines 2 API sets, one being alias of another (but it's the same API).

These are Zephyr APIs, which may have divergence from true POSIX APIs (declared in socket.h).

Well, there're no "true POSIX APIs", only "BSD Sockets like API" ("like" is important). True POSIX APIs live in <sys/socket.h>, Zephyr's BSD Sockets like API lives in <net/socket.h>.

It makes it easier to implement a new socket provider for the socket APIs for offloading. zsocket could just be one (the default) socket provider. Socket.h (or socket.c) would be the switch layer.

Ok, this finally explains why you suggest this change! But why do you want to do it this way? I imagined it would be done just the same way as offloading already implemented in net_context - that each individual zsock_*() will be the place of switching, and that offloading will be supported per netif.

You instead seem to propose that socket offloading will be global, not per netif. Rereading https://jira.zephyrproject.org/browse/ZEP-2271?focusedCommentId=19002&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-19002 , that seems to be what's written in the user story, but if you don't mind, I'd like to get an ack form @jukkar and @tbursztyka that it's ok to proceed with such design right away (vs further discuss it), as it leaves many questions open, e.g. why we have 2 inconsistently implemented offloading mechanisms. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Independent of where socket offloading occurs, it just seems it would be logical and future-proof to have zsocket.h defining native zsock functions, and socket.h declaring the socket functions, mapping to zsock. Anyways, just an opinion.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Independent of where socket offloading occurs, it just seems it would be logical and future-proof to have zsocket.h defining native zsock functions, and socket.h declaring the socket functions

Gil, I appreciate the idea, but after additional checking, I don't see such pattern of splitting the same API across 2+ headers to be used in Zephyr. And as I mentioned above, these are indeed intended to be 2 facets of the same API, not 2 different APIs. I'll be happy to make such a change however is there will be 2nd vote for it. Otherwise, well, if it proves to be useful/need, we may need to do such a refactor in the future (e.g. when we'll have better defined POSIX API layer). Thanks for understanding.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Understood.


#if defined(CONFIG_NET_SOCKETS_POSIX_NAMES)
Copy link
Collaborator

Choose a reason for hiding this comment

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

then could do here:
#include <zsocket.h>

#define socket zsock_socket
#define close zsock_close
#define bind zsock_bind
#define connect zsock_connect
#define listen zsock_listen
#define accept zsock_accept
#define send zsock_send
#define recv zsock_recv

#define inet_ntop net_addr_ntop
#define inet_pton net_addr_pton
#endif

#ifdef __cplusplus
}
#endif

#endif /* __NET_SOCKET_H */
18 changes: 18 additions & 0 deletions samples/net/socket_echo/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Makefile - simple socket-based echo server

#
# Copyright (c) 2017 Linaro Limited
#
# SPDX-License-Identifier: Apache-2.0
#

BOARD ?= qemu_x86
CONF_FILE ?= prj_$(BOARD).conf

include $(ZEPHYR_BASE)/Makefile.inc

ifeq ($(CONFIG_NET_L2_BLUETOOTH), y)
QEMU_EXTRA_FLAGS = -serial unix:/tmp/bt-server-bredr
else
include $(ZEPHYR_BASE)/samples/net/common/Makefile.ipstack
endif
4 changes: 4 additions & 0 deletions samples/net/socket_echo/Makefile.posix
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# This makefile builds socket_echo sample for POSIX system, like Linux

socket_echo: src/socket_echo.c
$(CC) $^ -o $@
26 changes: 26 additions & 0 deletions samples/net/socket_echo/prj_qemu_x86.conf
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# General config
CONFIG_NEWLIB_LIBC=y

# Networking config
CONFIG_NETWORKING=y
CONFIG_NET_IPV4=y
CONFIG_NET_IPV6=n
CONFIG_NET_TCP=y
CONFIG_NET_SOCKETS=y
CONFIG_NET_SOCKETS_POSIX_NAMES=y

# Network driver config
CONFIG_NET_SLIP_TAP=y
CONFIG_TEST_RANDOM_GENERATOR=y

# Without CONFIG_NET_BUF_LOG printf() doesn't work
Copy link
Member

Choose a reason for hiding this comment

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

I do not really understand how net_buf logging could affect printf(), could you elaborate the issue more.

BTW, typically I have this setting in my prj.conf files

CONFIG_NET_LOG=y
CONFIG_SYS_LOG_NET_LEVEL=2

and then setting relevant net debug options to "y" and increasing CONFIG_SYS_LOG_NET_LEVEL to 4, will then start to print debug output.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do not really understand how net_buf logging could affect printf(), could you elaborate the issue more.

I don't understand either, and yet it's like that - w/o CONFIG_NET_BUF_LOG=y, printf() output is not shown (network echo works). I did 10-15 mins of "genetic algorithm" debugging, semi-randomly trying other logging and output options and only CONFIG_NET_BUF_LOG looked like a culprit. Didn't debug further, as you may imagine, that can be quite a timesink ;-)

CONFIG_NET_BUF_LOG=y
Copy link
Member

Choose a reason for hiding this comment

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

Normally one does not need to enable this unless one is debugging the actual net_buf implementation.


# Network address config
CONFIG_NET_APP_SETTINGS=y
CONFIG_NET_APP_MY_IPV4_ADDR="192.0.2.1"
CONFIG_NET_APP_PEER_IPV4_ADDR="192.0.2.2"

# Network debug config
#CONFIG_NET_DEBUG_SOCKETS=y
CONFIG_SYS_LOG_NET_LEVEL=2
3 changes: 3 additions & 0 deletions samples/net/socket_echo/src/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
include $(ZEPHYR_BASE)/samples/net/common/Makefile.common

obj-y += socket_echo.o
76 changes: 76 additions & 0 deletions samples/net/socket_echo/src/socket_echo.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
/*
* Copyright (c) 2017 Linaro Limited
*
* SPDX-License-Identifier: Apache-2.0
*/

#include <stdio.h>

#ifndef __ZEPHYR__

#include <netinet/in.h>
#include <sys/socket.h>
#include <arpa/inet.h>
#include <unistd.h>

#define init_net()

#else

#include <net/socket.h>
#include <kernel.h>
#include <net_sample_app.h>

void init_net(void)
{
int ret = net_sample_app_init("socket_echo", NET_SAMPLE_NEED_IPV4,
K_SECONDS(3));

if (ret < 0) {
printf("Application init failed\n");
k_panic();
}
}

#endif

int main(void)
{
int serv;
struct sockaddr_in bind_addr;

init_net();

serv = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);

bind_addr.sin_family = AF_INET;
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if this app would support also IPv6. This can be enabled later, if no time to do that now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So (also partially answering @lpereira's comment below), the primary purpose of this sample was to show that the same code (modulo includes of course) can be used both on POSIX and Zephyr. Turns out, I missed to include "Makefile.posix" to make that obvious, I'll add it. Then, Linux for example supports "dual stack" sockets which can be accessed via IPv4 or IPv6, but I don't think we have that feature in Zephyr. Otherwise, to support IPv4 and IPv6 at the same time, there would be needed 2 sockets, and poll()/select(), which are scheduled so "stage 3" of Sockets implementation (this patch being stage 1).

Perhaps you mean either IPv4 or IPv6? That can be done, though again, the idea was to show how sockets can be portable between POSIX and Zephyr, without extra #ifdef noise.

Copy link
Member

Choose a reason for hiding this comment

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

I meant having dual IPv4 and IPv6 at the same time. But it requires two sockets at the moment and support for poll/select as you said, so we can postpone this for later stage.

bind_addr.sin_addr.s_addr = htonl(INADDR_ANY);
bind_addr.sin_port = htons(4242);
bind(serv, (struct sockaddr *)&bind_addr, sizeof(bind_addr));

listen(serv, 5);

while (1) {
struct sockaddr_in client_addr;
socklen_t client_addr_len = sizeof(client_addr);
char addr_str[32];
int client = accept(serv, (struct sockaddr *)&client_addr,
&client_addr_len);
inet_ntop(client_addr.sin_family, &client_addr.sin_addr,
addr_str, sizeof(addr_str));
printf("Connection from %s\n", addr_str);

while (1) {
char buf[128];
int len = recv(client, buf, sizeof(buf), 0);

if (len == 0) {
break;
}
send(client, buf, len, 0);
}

close(client);
printf("Connection from %s closed\n", addr_str);
}
}
1 change: 1 addition & 0 deletions subsys/net/lib/Kbuild
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
obj-$(CONFIG_NET_SOCKETS) += sockets/
Copy link
Collaborator

Choose a reason for hiding this comment

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

Then, maybe the subdirectory should be zsocket/ or zsock/ instead of sockets/?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure, why? There're dns, http on the same level, not zdns, zhttp. (There's zoap, yeah, but it's an exception.)

obj-$(CONFIG_ZOAP) += zoap/
obj-$(CONFIG_DNS_RESOLVER) += dns/
obj-$(CONFIG_MQTT_LIB) += mqtt/
Expand Down
2 changes: 2 additions & 0 deletions subsys/net/lib/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

menu "Network Protocols"

source "subsys/net/lib/sockets/Kconfig"

source "subsys/net/lib/zoap/Kconfig"

source "subsys/net/lib/dns/Kconfig"
Expand Down
4 changes: 4 additions & 0 deletions subsys/net/lib/Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
ifdef CONFIG_NET_SOCKETS
include $(srctree)/subsys/net/lib/sockets/Makefile
endif

ifdef CONFIG_ZOAP
include $(srctree)/subsys/net/lib/zoap/Makefile
endif
Expand Down
35 changes: 35 additions & 0 deletions subsys/net/lib/sockets/Kconfig
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Kconfig - BSD Sockets like API

#
# Copyright (c) 2017 Linaro Limited.
#
# SPDX-License-Identifier: Apache-2.0
#

menuconfig NET_SOCKETS
bool "BSD Sockets like API"
default n
help
Provide BSD Sockets like API on top of native Zephyr networking API.

if NET_SOCKETS

config NET_SOCKETS_POSIX_NAMES
bool "Standard POSIX names for Sockets API"
default n
help
By default, Sockets API function are prefixed with `zsock_` to avoid
namespacing issues. If this option is enabled, they will be provided
with standard POSIX names like socket(), recv(), and close(), to help
with porting existing code. Note that close() may require a special
attention, as in POSIX it closes any file descriptor, while with this
option enaled, it will still apply only to sockets.

config NET_DEBUG_SOCKETS
bool "Debug BSD Sockets like API calls"
default n
help
Enables logging for sockets code. (Logging level is defined by
SYS_LOG_NET_LEVEL setting).

endif # NET_SOCKETS
3 changes: 3 additions & 0 deletions subsys/net/lib/sockets/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
ccflags-y += -I$(srctree)/subsys/net/lib/sockets

obj-$(CONFIG_NET_SOCKETS) += sockets.o
Loading