samples: net: sockets: Allow to build and test with POSIX subsys #18736

pfalcon · 2019-08-28T10:57:26Z

With CONFIG_POSIX_API enabled, these samples now build under Zephyr
with exactly the same source as e.g. Linux (or in general, other POSIX
systems). However, building without CONFIG_POSIX_API (i.e. with
CONFIG_NET_SOCKETS_POSIX_NAMES aux option) is retained for now.

Add testcase definitions to build these samples with CONFIG_POSIX_API
in CI.

Fixes: #17353

Signed-off-by: Paul Sokolovsky paul.sokolovsky@linaro.org

pfalcon · 2019-08-28T11:11:51Z

This patch is kinda culmination of efforts in #16621, #16626, #18548. Those patches enable to build existing 3rd-party POSIX software for Zephyr with much less changes than before (with the aim to have zero changes).

This patch demoes that magic in-tree, using existing socket samples (which were always written to be POSIX-compatible). So, now exact same source file (down to the includes used) which works on e.g. Linux, can be built on Zephyr without any lowy patching.

(Here, the effect is somewhat diluted, as the sample continue to contain adhoc Zephyr includes to build for a case when CONFIG_POSIX_API is not defined. I'm a bit shy to remove it right away, because story of POSIX subsys and socket offloading is not clear.)

And to remind, the idea of #17706 is to continue (incremental) work in that direction, requiring asymptotically less patching to build real-world POSIX projects for Zephyr.

jukkar

Looks good, minor tweaks needed.

jukkar · 2019-08-28T11:10:50Z

samples/net/sockets/big_http_download/sample.yaml

@@ -8,3 +8,12 @@ tests:
    min_ram: 32
    min_flash: 128
    tags: net socket
+  sample.net.sockets.big_http_download.posix:
+    filter: TOOLCHAIN_HAS_NEWLIB == 1


Please move the common values between sample.net.sockets.big_http_download and sample.net.sockets.big_http_download.posix to common block.
So something like this:

common: harness: net tags: net socket min_ram: 32 min_flash: 128

Please move the common values between

Fixed.

jukkar · 2019-08-28T11:11:06Z

samples/net/sockets/dumb_http_server/sample.yaml

@@ -8,3 +8,12 @@ tests:
    min_ram: 32
    min_flash: 96
    tags: net socket
+  sample.net.sockets.dumb_http_server.posix:
+    filter: TOOLCHAIN_HAS_NEWLIB == 1
+    harness: net


jukkar · 2019-08-28T11:11:32Z

samples/net/sockets/echo_async/sample.yaml

@@ -7,3 +7,10 @@ tests:
    harness: net
    platform_whitelist: qemu_x86
    tags: net socket
+  sample.net.sockets.echo_async.posix:
+    harness: net


jukkar · 2019-08-28T11:11:43Z

samples/net/sockets/http_get/sample.yaml

@@ -8,3 +8,12 @@ tests:
    min_ram: 32
    min_flash: 80
    tags: net socket
+  sample.net.sockets.http_get.posix:
+    filter: TOOLCHAIN_HAS_NEWLIB == 1


jukkar · 2019-08-28T12:41:44Z

samples/net/sockets/http_get/sample.yaml

 tests:
  sample.net.sockets.http_get:
+    harness: net
+  sample.net.sockets.http_get.posix:
    filter: TOOLCHAIN_HAS_NEWLIB == 1
    harness: net


Most of the settings in sample.net.sockets.http_get.posix could be removed as they are now in common: block

Ack, overlooked it, fixed.

jukkar · 2019-08-28T12:43:11Z

samples/net/sockets/big_http_download/sample.yaml

 tests:
  sample.net.sockets.big_http_download:
-    filter: TOOLCHAIN_HAS_NEWLIB == 1
    harness: net


The individual test section cannot be left empty so I was just wondering if we could set here for example some config option so that the harness setting could be removed.

The individual test section cannot be left empty

Yes. I looked into that, pykwalify (following limitation of upstream kwalify scheme doesn't allow that, e.g. Grokzen/pykwalify#149).

The normal YAML syntax then would be: sample.net.sockets.big_http_download: {}, but I considered that too magic for not YAML lawyers among us, and used

sample.net.sockets.big_http_download: harness: net

as a placeholder.

What about adding

extra_configs: - CONFIG_POSIX_API=n

which is a "dummy" setting in this case, instead of the harness:net?

which is a "dummy" setting in this case, instead of the harness:net

I can do that, but are you sure it will be improvement? (I'd use CONFIG_NET_SOCKETS_POSIX_NAMES=y then however, to show what we test with, not what without.)

ioannisg · 2019-08-29T18:21:44Z

@pfalcon could you please fix the CI failures here?

pfalcon · 2019-08-29T18:49:29Z

could you please fix the CI failures here?

I'm trying, I'm trying.

@vanti, there's a problem with http_get sample and cc3220sf_launchxl. One of cc3220sf_launchxl components forcibly defines CONFIG_POSIX_API, which should never happen, as it's app-level option. If you need just one of POSIX subsystems, like Pthreads, then please vote up #12965 and help implement it. Here, trying to define CONFIG_POSIX_API behind the app's back leads to array of issues: it's incompatible with CONFIG_NET_SOCKETS_POSIX_NAMES=y which the app already selects, any socket offloading implementation is incompatible with POSIX subsystem, because of technical debt in socket offloading subsystem (it should utilize fdtable sub-subsys to be baseline-compatible with POSIX subsys). So, let's discuss way forward with this (please include @rlubos and @mike-scott who are other stakeholder in offloading).

In the meantime, I see no other choice as to blacklist cc3220sf_launchxl from this sample.

With CONFIG_POSIX_API enabled, these samples now build under Zephyr with exactly the same source as e.g. Linux (or in general, other POSIX systems). However, building without CONFIG_POSIX_API (i.e. with CONFIG_NET_SOCKETS_POSIX_NAMES aux option) is retained for now. Add testcase definitions to build these samples with CONFIG_POSIX_API in CI. Fixes: zephyrproject-rtos#17353 Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>

ioannisg

@pfalcon this looks ok to me.
I noticed, in some sample .yaml files you have applied a filter expression starting with "and not" - I think you could skip the initial "and", right?

pfalcon · 2019-08-29T22:03:26Z

@pfalcon this looks ok to me.

Thanks.

I noticed, in some sample .yaml files you have applied a filter expression starting with "and not" - I think you could skip the initial "and", right?

Please see #18770. In short, as of now it's required, but hopefully we'll fix that.

PR zephyrproject-rtos#18780 introduces a way to decouple pthread support from the general CONFIG_POSIX_API global switch. This commit modifies the build of SimpleLink components to take advantage of it, since SimpleLink libraries only require pthread (and sem) support, not entire POSIX. This fixes the build errors in the http_get sample introduced by the merge of zephyrproject-rtos#18736. Signed-off-by: Vincent Wan <vincent.wan@linaro.org>

PR zephyrproject-rtos#18780 introduces a way to decouple pthread support from the general CONFIG_POSIX_API global switch. This commit modifies the build of SimpleLink components to take advantage of it, since SimpleLink libraries only require pthread, sem, clock, and sleep support, not entire POSIX API. This fixes the build errors in the http_get sample introduced by the merge of zephyrproject-rtos#18736. As such, this patch also removes cc3220sf_launchxl exclude from sample.yaml of that sample. Signed-off-by: Vincent Wan <vincent.wan@linaro.org> Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>

Build errors were introduced by the merge of zephyrproject-rtos#18736. Until PR zephyrproject-rtos#18780 is approved to allow the SimpleLink libraries to build without CONFIG_POSIX_API, this patch excludes cc3235sf_launchxl from the test build. Signed-off-by: Vincent Wan <vincent.wan@linaro.org>

Build errors were introduced by the merge of #18736. Until PR #18780 is approved to allow the SimpleLink libraries to build without CONFIG_POSIX_API, this patch excludes cc3235sf_launchxl from the test build. Signed-off-by: Vincent Wan <vincent.wan@linaro.org>

PR zephyrproject-rtos#18780 introduces a way to decouple pthread support from the general CONFIG_POSIX_API global switch. This commit modifies the build of SimpleLink components to take advantage of it, since SimpleLink libraries only require pthread, sem, clock, and sleep support, not entire POSIX API. This fixes the build errors in the http_get sample introduced by the merge of zephyrproject-rtos#18736. As such, this patch also removes cc3220sf_launchxl exclude from sample.yaml of that sample. Signed-off-by: Vincent Wan <vincent.wan@linaro.org> Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>

PR #18780 introduces a way to decouple pthread support from the general CONFIG_POSIX_API global switch. This commit modifies the build of SimpleLink components to take advantage of it, since SimpleLink libraries only require pthread, sem, clock, and sleep support, not entire POSIX API. This fixes the build errors in the http_get sample introduced by the merge of #18736. As such, this patch also removes cc3220sf_launchxl exclude from sample.yaml of that sample. Signed-off-by: Vincent Wan <vincent.wan@linaro.org> Signed-off-by: Paul Sokolovsky <paul.sokolovsky@linaro.org>

pfalcon added area: Networking area: POSIX POSIX API Library labels Aug 28, 2019

pfalcon added this to the v2.0.0 milestone Aug 28, 2019

pfalcon requested review from jukkar, PiotrZierhoffer, tbursztyka, ioannisg, tgorochowik and galak August 28, 2019 10:57

zephyrbot added the area: Samples Samples label Aug 28, 2019

jukkar requested changes Aug 28, 2019

View reviewed changes

pfalcon force-pushed the samples-sockets-posix branch from c6e2f19 to a74d533 Compare August 28, 2019 12:16

jukkar reviewed Aug 28, 2019

View reviewed changes

pfalcon force-pushed the samples-sockets-posix branch from a74d533 to 28d9268 Compare August 28, 2019 12:57

pfalcon mentioned this pull request Aug 29, 2019

sanitycheck: Merge common vs per-test "filter" field values semantically #18770

Merged

pfalcon force-pushed the samples-sockets-posix branch 3 times, most recently from 68d2240 to 66ed8b3 Compare August 29, 2019 12:27

jukkar approved these changes Aug 29, 2019

View reviewed changes

jukkar closed this Aug 29, 2019

jukkar reopened this Aug 29, 2019

pfalcon force-pushed the samples-sockets-posix branch from 66ed8b3 to 7b52e12 Compare August 29, 2019 18:50

pfalcon mentioned this pull request Aug 29, 2019

posix: Kconfig: Allow to enable individual components, POSIX_API just enables all #18780

Merged

ioannisg approved these changes Aug 29, 2019

View reviewed changes

ioannisg reviewed Aug 29, 2019

View reviewed changes

ioannisg merged commit 5eb974d into zephyrproject-rtos:master Aug 30, 2019

pfalcon mentioned this pull request Aug 30, 2019

Configuring with POSIX_API disables NET_SOCKETS_POSIX_NAMES #17353

Closed

vanti mentioned this pull request Aug 30, 2019

modules: Kconfig.simplelink: Remove POSIX_API from being selected #18810

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

samples: net: sockets: Allow to build and test with POSIX subsys #18736

samples: net: sockets: Allow to build and test with POSIX subsys #18736

pfalcon commented Aug 28, 2019

pfalcon commented Aug 28, 2019

jukkar left a comment

jukkar Aug 28, 2019

pfalcon Aug 28, 2019

jukkar Aug 28, 2019

jukkar Aug 28, 2019

jukkar Aug 28, 2019

jukkar Aug 28, 2019

pfalcon Aug 28, 2019

jukkar Aug 28, 2019

pfalcon Aug 28, 2019

jukkar Aug 28, 2019

pfalcon Aug 28, 2019

ioannisg commented Aug 29, 2019

pfalcon commented Aug 29, 2019

ioannisg left a comment

pfalcon commented Aug 29, 2019

samples: net: sockets: Allow to build and test with POSIX subsys #18736

samples: net: sockets: Allow to build and test with POSIX subsys #18736

Conversation

pfalcon commented Aug 28, 2019

pfalcon commented Aug 28, 2019

jukkar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ioannisg commented Aug 29, 2019

pfalcon commented Aug 29, 2019

ioannisg left a comment

Choose a reason for hiding this comment

pfalcon commented Aug 29, 2019