posix: pipe: Implement POSIX-compliant pipes #79393

brosier01 · 2024-10-03T20:34:39Z

This change introduces a POSIX-compliant pipe feature into Zephyr RTOS. The POSIX_PIPE is required for PSE53 conformance.

The pipe implementation includes non-blocking and blocking behavior for pipe read/write operations, adhering to the POSIX standard. The pipe implementation includes atomic synchronization mechanisms to handle multiple readers and writers, ensuring thread safety.

Tested using ZTEST framework, validating both blocking and non-blocking behaviors with various pipe buffer sizes. Test cases include handling full and empty pipes, ensuring correct behavior when multiple threads attempt simultaneous reads and writes.

github-actions · 2024-10-03T20:35:21Z

Hello @brosier01, and thank you very much for your first pull request to the Zephyr project!
Our Continuous Integration pipeline will execute a series of checks on your Pull Request commit messages and code, and you are expected to address any failures by updating the PR. Please take a look at our commit message guidelines to find out how to format your commit messages, and at our contribution workflow to understand how to update your Pull Request. If you haven't already, please make sure to review the project's Contributor Expectations and update (by amending and force-pushing the commits) your pull request if necessary.
If you are stuck or need help please join us on Discord and ask your question there. Additionally, you can escalate the review when applicable. 😊

cfriedt · 2024-10-03T22:43:41Z

@brosier01 - thanks for the PR 👍

I would make a few suggestions

please keep the PR in draft mode until all tests are passing
please take a look at the most recent implementation attempt
please review the resolved comments in posix: allow for external implementation of option groups #74184 by @keith-packard too.

Thanks!

cfriedt · 2024-10-03T22:18:25Z

lib/posix/options/Kconfig.pipe

+config POSIX_NUM_PIPES
+	int "Number of available pipes"
+	default 2
+	depends on POSIX_API
+	help
+	Specify the number of POSIX pipes available in the system.
+	This value controls how many pipes can be created concurrently.


It would be better if this option was named somewhat more conventionally like other *_MAX values.

See e.g.
https://github.com/zephyrproject-rtos/zephyr/blob/main/include/zephyr/posix/sys/sysconf.h

Additionally, you will need a Kconfig option for POSIX_PIPE_BUF, as well as a feature test macro and sysconf enumeration.

See e.g.
main...cfriedt:zephyr:posix-pipes2

cfriedt · 2024-10-03T23:56:26Z

lib/posix/options/Kconfig.pipe

+	default y
+	depends on POSIX_API


This should depend on I guess FDTABLE and maybe some other options. Kconfig.net might have some hints.

Suggested change

default y

depends on POSIX_API

Can you please ensure POSIX_PIPE is selected with CONFIG_POSIX_AEP_REALTIME_DEDICATED?

Also, below this, please add

if POSIX_PIPE ... endif

cfriedt · 2024-10-03T23:57:25Z

lib/posix/options/Kconfig.pipe

+config POSIX_NUM_PIPES
+	int "Number of available pipes"
+	default 2
+	depends on POSIX_API


please remove this dependency as POSIX_API will likely be deprecated soon.

Suggested change

depends on POSIX_API

cfriedt · 2024-10-03T23:59:11Z

lib/posix/options/pipe.c

+static int pipe_ioctl_vmeth(void *obj, unsigned int request, va_list args);
+
+struct pipe_desc {
+	unsigned char __aligned(4) ring_buffer[_POSIX_PIPE_BUF];


Zephyr has a ring-buffer implementation. It should be used here as well, I would guess.

Hi cfriedt,

My implementation of the posix pipe is based on the ZEPHYR pipe and it needs an aligned(4) buffer to be initialized

cfriedt · 2024-10-04T00:00:02Z

lib/posix/options/pipe.c

+#include <sys/_intsup.h>
+
+#define NUM_PIPES        CONFIG_POSIX_NUM_PIPES
+#define PIPE_BUFFER_SIZE _POSIX_PIPE_BUF


this seems to be overuse of macros. Maybe consider just using CONFIG_POSIX_PIPE_BUF?

ycsin · 2024-10-21T01:49:40Z

lib/posix/options/pipe.c

+	atomic_t readOpened;
+	atomic_t writeOpened;
+	atomic_t isUsed;


convert these to snake_case

ycsin · 2024-10-21T01:51:03Z

lib/posix/options/pipe.c

+static struct fd_op_vtable fs_fd_op_vtable = {.read = pipe_read_vmeth,
+					      .write = pipe_write_vmeth,
+					      .close = pipe_close_vmeth,
+					      .ioctl = pipe_ioctl_vmeth};


If you put a comma , at the end, clang-format will be able to format this nicely for you

Suggested change

.ioctl = pipe_ioctl_vmeth};

.ioctl = pipe_ioctl_vmeth,

};

ycsin · 2024-10-21T02:02:21Z

tests/posix/common/src/pipe.c

@@ -0,0 +1,124 @@
+#include <zephyr/ztest.h>


needs the license identifier

This change introduces a POSIX-compliant pipe feature into Zephyr RTOS. The POSIX_PIPE is required for PSE53 conformance. The pipe implementation includes non-blocking and blocking behavior for pipe read/write operations, adhering to the POSIX standard. The pipe implementation includes atomic synchronization mechanisms to handle multiple readers and writers, ensuring thread safety. Tested using ZTEST framework, validating both blocking and non-blocking behaviors with various pipe buffer sizes. Test cases include handling full and empty pipes, ensuring correct behavior when multiple threads attempt simultaneous reads and writes. Signed-off-by: Bruce Rosier <[email protected]>

github-actions · 2025-01-03T00:33:27Z

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.

zephyrbot added the area: POSIX POSIX API Library label Oct 3, 2024

zephyrbot requested review from cfriedt and ycsin October 3, 2024 20:35

zephyrbot assigned cfriedt Oct 3, 2024

cfriedt reviewed Oct 3, 2024

View reviewed changes

cfriedt reviewed Oct 4, 2024

View reviewed changes

brosier01 marked this pull request as draft October 4, 2024 16:23

brosier01 force-pushed the feature_posix_pipe branch 2 times, most recently from befbc9a to 5f5056f Compare October 5, 2024 19:18

brosier01 marked this pull request as ready for review October 6, 2024 13:53

brosier01 marked this pull request as draft October 6, 2024 13:58

brosier01 force-pushed the feature_posix_pipe branch from 5f5056f to 0d21c1a Compare October 17, 2024 21:41

ycsin reviewed Oct 21, 2024

View reviewed changes

brosier01 force-pushed the feature_posix_pipe branch from 0d21c1a to 574df27 Compare November 2, 2024 14:58

brosier01 requested review from ycsin and cfriedt November 2, 2024 15:59

github-actions bot added the Stale label Jan 3, 2025

github-actions bot closed this Jan 18, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

posix: pipe: Implement POSIX-compliant pipes #79393

posix: pipe: Implement POSIX-compliant pipes #79393

brosier01 commented Oct 3, 2024

github-actions bot commented Oct 3, 2024

cfriedt commented Oct 3, 2024 •

edited

Loading

cfriedt Oct 3, 2024

cfriedt Oct 3, 2024 •

edited

Loading

cfriedt Oct 3, 2024

cfriedt Oct 3, 2024

brosier01 Oct 5, 2024

cfriedt Oct 4, 2024

ycsin Oct 21, 2024

ycsin Oct 21, 2024

ycsin Oct 21, 2024

github-actions bot commented Jan 3, 2025

posix: pipe: Implement POSIX-compliant pipes #79393

posix: pipe: Implement POSIX-compliant pipes #79393

Conversation

brosier01 commented Oct 3, 2024

github-actions bot commented Oct 3, 2024

cfriedt commented Oct 3, 2024 • edited Loading

Choose a reason for hiding this comment

cfriedt Oct 3, 2024 • edited Loading

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

github-actions bot commented Jan 3, 2025

cfriedt commented Oct 3, 2024 •

edited

Loading

cfriedt Oct 3, 2024 •

edited

Loading