Net app API #540

jukkar · 2017-06-19T11:35:49Z

There has been discussion and complaints that the current net_context API that applications can use is too low level and requires lot of effort from application developer to create a bug free application. Many networking operations that various applications do, are very similar and can be provided by a library. In order to address these issues I created a higher level API that applications can use.

The API consists of two parts:

Application initialization support.

The net_app_init() makes sure that we have IP address, routes etc. configured and the IP stack is ready to serve before continuing. User can supply a timeout to the function call, and also give a hint what kind of support it needs from the IP stack. It is possible to use only the net_app_init() without the connectivity support if needed.

Connectivity support

Developer can create a simple TCP/UDP server or client application with only few function calls.

Example for the TCP server:

net_app_init_server(&tcp, SOCK_STREAM, IPPROTO_TCP, NULL,
		    MY_PORT, NULL, NULL, NULL);
net_app_set_cb(&tcp, NULL, tcp_received, NULL, NULL);
net_app_wait_connection(&tcp);

Example for UDP server:

net_app_init_server(&udp, SOCK_DGRAM, IPPROTO_UDP, NULL,
		    MY_PORT, NULL, NULL, NULL);
net_app_set_cb(&udp, NULL, udp_received, pkt_sent, NULL);
net_app_wait_connection(&udp);

Example TCP client:

net_app_init_client(ctx, SOCK_STREAM, IPPROTO_TCP, NULL,
		    NULL, peer, PEER_PORT, WAIT_TIME,
		    NULL, NULL, user_data);
net_app_set_cb(ctx, tcp_connected, tcp_received, NULL, NULL);
net_app_connect(ctx, CONNECT_TIME);

Example UDP client:

net_app_init_client(ctx, SOCK_DGRAM, IPPROTO_UDP, NULL,
		    NULL, peer, PEER_PORT, WAIT_TIME,
		    NULL, NULL, user_data);
net_app_set_cb(ctx, udp_connected, udp_received, NULL, NULL);
net_app_connect(ctx, CONNECT_TIME);

So the developer needs to setup the proper callbacks that will be called in various stages of the connection flow.

I created a TLS support for TCP server in this initial pull request. The TLS support is transparent to the application, all the encryption and decryption happens inside the net_app API. Only thing the application needs to do is to setup the TLS, and prepare the certificates for mbedtls library. With this transparent TLS support, it is possible to create for example MQTT over TLS support quite easily.

In order to try the TLS support, the net-tools project have this
zephyrproject-rtos/net-tools#8
PR that provides stunnel configuration so that echo-client can be run in Linux side and it can communicate with zephyr echo-server sample over TLS.

This net_app API will replace the internal net_sample_app API that was used by some of the network sample applications found under samples/net directory.

The current patchset contains these patches:

net_app core functions
conversion of echo-client and echo-server samples to use this new API
base TLS support
setting up the echo-server to use the TLS if configured so

Future plans:

create TLS TCP client support
create DTLS UDP client and server support
convert existing network samples to use this API

lpereira · 2017-06-19T21:35:41Z

If we're doing this to simplify things:

Any reason to ask the user to specify both DGRAM/UDP or STREAM/TCP? Can't DGRAM/STREAM be inferred from the protocol?
I get really uneasy with functions that accepts multiple parameters of the same type; it's too easy to mix them up and difficult to know what they are from the call site. I'd prefer more functions to set each parameter rather than a single function with a lot of parameters.

tbursztyka

I think there are rooms for improving net_context API first, this current attempt of an extra layer on top of net_context is not as simple as it could

For instance:

net_context's callbacks. The pb with those is the stack they are ran from: net's one. We could maybe move towards using kpoll objects (actually net_context was designed long before this objects appeared)..
It would also simplify a lot of things for the BSD socket layer I think. (it would not save or bloat memory though I think)
tx_slab/data_pool: I think we will need to work on smarter buffer handling, this really needs to go away at some point.

About @lpereira's comment on type/protocol, I disagree: the base function should ask for both. But starting from that idea: nothing prevents the api to expose some dedicated and simple inline function such as net_app_init_tcp_server() or net_app_init_udp_client() etc...

tbursztyka · 2017-06-20T06:39:25Z

include/net/net_app.h

+			struct sockaddr *server_addr,
+			u16_t port,
+			net_pkt_get_slab_func_t tx_slab,
+			net_pkt_get_pool_func_t data_pool,


Not sure we want to expose this tx_slab/data_pool thingy. It's very low level stuff, and in the future (hopefully) we will be able to get rid of it completely. So imo it should stay on net_context side for those who really need it, but not exposed on net_app.h

tbursztyka · 2017-06-20T06:41:43Z

include/net/net_app.h

+			u16_t peer_port,
+			s32_t timeout,
+			net_pkt_get_slab_func_t tx_slab,
+			net_pkt_get_pool_func_t data_pool,


tbursztyka · 2017-06-20T06:46:40Z

include/net/net_app.h

+};
+
+/** Network application context. */
+struct net_app_ctx {


A context on top of another context. Maybe simplifying net_context first would be wiser.

GAnthony · 2017-06-20T18:25:24Z

Adding this net_app_init() is a good idea to simplify the examples.

I wonder if we could go even one step further and have this initialization happen during boot-up automatically, say, during pre-application initialization?

Something like:

SYS_INIT(net_app_init(), _SYS_INIT_LEVEL_APPLICATION, ...);

perhaps its parameters can be added as Kconfig options?

That way, it will hide this common initialization from all the examples, and the function call could be completely removed from all the net (and new socket) samples.

Looking at the new BSD socket_echo.c example (#498), the only non-standard bit of code (other than the headers) keeping it from being a portable BSD socket app is that bit of Zephyr-specific initialization code (init_net()).

Hiding the initialization could be another small step to help future porting of third party BSD socket apps and protocols to Zephyr.

pfalcon · 2017-06-20T19:31:27Z

@GAnthony : Right, it's about the same idea I shared on mailing list in response to the RFC behind this patch: https://lists.zephyrproject.org/pipermail/zephyr-devel/2017-June/007799.html

GAnthony · 2017-06-20T22:04:05Z

@pfalcon: Ah, yes, thanks for pointing that out. I must have overlooked that part of the email thread. In any case, I agree it would help simplify the apps to somehow hide that initialization.

jukkar · 2017-06-26T12:42:27Z

New version uploaded.
Changes:

added helpers for creating TCP / UDP listeners as suggested by @lpereira and @tbursztyka
calling net_app_init() automatically as suggested by @GAnthony and @pfalcon I made the auto init optional (it is on by default) so that application developer can call net_app_init() itself if really necessary
refactored the code
supports now both client and server roles for TLS

pfalcon

calling net_app_init() automatically as suggested by @GAnthony and @pfalcon I made the auto init optional (it is on by default) so that application developer can call net_app_init() itself if really necessary

Thanks!

Couple of possible improvements in the inline comments, but otherwise looks good.

pfalcon · 2017-06-27T13:45:27Z

include/net/net_app.h

+ *
+ * @return 0 if ok, <0 if error.
+ */
+int net_app_wait_connection(struct net_app_ctx *ctx);


Maybe a nitpick, but from the name or a description of this function, it's not clear whether this really waits for connection (blocks until it happens), or whether it starts to listen for connections (non-blocking). Name suggests that it really waits, though description leans towards starting to listen. If so, why not call this func e.g. net_app_listen() ?

pfalcon · 2017-06-27T14:35:27Z

subsys/net/lib/app/init.c

+	ret = net_app_init("Initializing network", flags,
+			   K_SECONDS(CONFIG_NET_APP_INIT_TIMEOUT));
+	if (ret < 0) {
+		NET_ERR("Network initialization failed (%d)", ret);


I'd suggest to make this printk() and k_panic(), to make sure this cannot be missed, regardless of logging options.

askawu · 2017-06-28T06:33:04Z

subsys/net/lib/app/net_app.c

+		client_or_server == MBEDTLS_SSL_IS_CLIENT ? "client" :
+							    "server");
+
+exit:


We probably need to call mbedtls_ssl_free(), mbedtls_ssl_config_free(), mbedtls_ctr_drbg_free(), and mbedtls_entropy_free() if something goes wrong.

jukkar · 2017-06-28T14:08:35Z

Sent a new version. Changes:

net_app_wait_connection() renamed to net_app_listen()
changed the net_app_init() failure to print using printk() (using k_panic() seemed like overkill atm)
added mbedtls resource cleaning if TLS init fails

The network application API is a higher level API for creating client and server type applications. Instead of applications dealing with low level details, the network application API provides services that most of the applications can use directly. This commit removes the internal net_sample_*() API and converts the existing users of it to use the new net_app API. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

This commit will convert echo-server to use the net app API when creating the listening service. Most of the network setup code will be removed from echo-server by this commit. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

This commit will convert echo-client to use the net app API when creating the connection to peer. Most of the network setup code will be removed from echo-client by this commit. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

Removing CONFIG_NET_APP_SETTINGS from prj.conf file as the sample does not use or need any IP addresses. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

As the net app API is automatically initialized, there is no need to call net_app_init() by the http client and server sample applications. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

Next two commits will increase the mbedtls ram usage a bit and https client and server sample test will fail. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

Add Kconfig option that can be used to enable various debug options in mbedtls config file. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

This changes increases content buffer length MBEDTLS_SSL_MAX_CONTENT_LEN to 1500 bytes so that we can use this config for echo-client and echo-server network sample applications which need to send bigger data than 1024 bytes. Removing MBEDTLS_PEM_PARSE_C as we do not have any cert in PEM format. Place various MBEDTLS debug options behind CONFIG_MBEDTLS_DEBUG Kconfig option which was introduced in previous commit. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

jukkar · 2017-06-28T17:05:56Z

Getting past shippable seems to be tricky for this PR. I sent a new version:

increased RAM size for https_client running in qemu
changed back to NET_ERR() for net_app_init() as using printk() caused build errors, we can fix this later if really needed

) - Documentation said that UART could use tty1, which was not true and would cause an error to be thrown. The error will still be thrown, but the docs now only mention tty0. Signed-off-by: James Prestwood <james.prestwood@intel.com>

jukkar added the net label Jun 19, 2017

jukkar self-assigned this Jun 19, 2017

jukkar requested review from nashif, pfalcon and tbursztyka June 19, 2017 11:35

tbursztyka reviewed Jun 20, 2017

View reviewed changes

jukkar force-pushed the net-app-api branch from bd64961 to 7abe5af Compare June 26, 2017 12:38

jukkar changed the title ~~[RFC] Net app API~~ Net app API Jun 26, 2017

jukkar force-pushed the net-app-api branch 3 times, most recently from ef9454e to 3612bf5 Compare June 27, 2017 07:25

pfalcon previously approved these changes Jun 27, 2017

View reviewed changes

askawu reviewed Jun 28, 2017

View reviewed changes

jukkar dismissed pfalcon’s stale review via b7fd9ea June 28, 2017 14:06

jukkar force-pushed the net-app-api branch from 3612bf5 to b7fd9ea Compare June 28, 2017 14:06

jukkar added 8 commits June 28, 2017 20:02

samples: net: wpan: No need to define IP addresses

79c3ddd

Removing CONFIG_NET_APP_SETTINGS from prj.conf file as the sample does not use or need any IP addresses. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

samples: net: http: Remove net_app_init() calls

849c600

As the net app API is automatically initialized, there is no need to call net_app_init() by the http client and server sample applications. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

samples: net: https: Increase the RAM for client and server

26c25f8

Next two commits will increase the mbedtls ram usage a bit and https client and server sample test will fail. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

ext: lib: mbedtls: Add Kconfig option to enable mbedtls debugging

4d7158f

Add Kconfig option that can be used to enable various debug options in mbedtls config file. Signed-off-by: Jukka Rissanen <jukka.rissanen@linux.intel.com>

jukkar force-pushed the net-app-api branch from b7fd9ea to f997215 Compare June 28, 2017 17:03

pfalcon approved these changes Jun 29, 2017

View reviewed changes

jukkar requested a review from Vudentz June 30, 2017 09:03

nashif merged commit a5898da into zephyrproject-rtos:master Jun 30, 2017

zephyrbot mentioned this pull request Sep 16, 2019

arc: hsdk: bug fixes and new feature supports #19173

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Net app API #540

Net app API #540

jukkar commented Jun 19, 2017

lpereira commented Jun 19, 2017

tbursztyka left a comment

tbursztyka Jun 20, 2017

tbursztyka Jun 20, 2017

tbursztyka Jun 20, 2017

GAnthony commented Jun 20, 2017

pfalcon commented Jun 20, 2017

GAnthony commented Jun 20, 2017

jukkar commented Jun 26, 2017

pfalcon left a comment

pfalcon Jun 27, 2017

pfalcon Jun 27, 2017

askawu Jun 28, 2017

jukkar commented Jun 28, 2017

jukkar commented Jun 28, 2017

Net app API #540

Net app API #540

Conversation

jukkar commented Jun 19, 2017

lpereira commented Jun 19, 2017

tbursztyka left a comment

Choose a reason for hiding this comment

tbursztyka Jun 20, 2017

Choose a reason for hiding this comment

tbursztyka Jun 20, 2017

Choose a reason for hiding this comment

tbursztyka Jun 20, 2017

Choose a reason for hiding this comment

GAnthony commented Jun 20, 2017

pfalcon commented Jun 20, 2017

GAnthony commented Jun 20, 2017

jukkar commented Jun 26, 2017

pfalcon left a comment

Choose a reason for hiding this comment

pfalcon Jun 27, 2017

Choose a reason for hiding this comment

pfalcon Jun 27, 2017

Choose a reason for hiding this comment

askawu Jun 28, 2017

Choose a reason for hiding this comment

jukkar commented Jun 28, 2017

jukkar commented Jun 28, 2017