[riot-notifications] [RIOT-OS/RIOT] sys/posix/socket: implement MSG_PEEK for recvfrom (#16850)

Martine Lenders notifications at github.com
Thu Sep 16 14:33:42 CEST 2021


@miri64 commented on this pull request.



> +    uint8_t recv_buf[RECV_BUF_SIZE];
+    ringbuffer_t peek_rb;

Given that this adds _**1.5 KB** per allocatable socket_ I would rather like to see this feature to be optional. For instance, by introducing a pseudo-module `posix_sockets_msg_peek`.

> @@ -479,6 +498,16 @@ int socket(int domain, int type, int protocol)
             errno = EAFNOSUPPORT;
             res = -1;
     }
+
+    char *buf_ptr = malloc(PEEK_BUF_SIZE * sizeof(char));

Not sure why `malloc` is used here, rather than `recv_buf`. The socket pools here should make clear, that this module tries to avoid using `malloc`, which should always only be a fall-back if nothing else is possible in embedded programming.

> + * Todo: How to get used MTU size?
+ */
+#define RECV_BUF_SIZE              (1500)

We could make this configurable during compile time. Beyond that: we have the basis for Path-MTU discovery in place in GNRC (and I believe so does lwIP). The only thing missing is an implementation of the Destination Cache in the NIB that would be the entry point for storage (based on what ICMPv6 errors report) and look-up.

>  /* enough to create sockets both with socket() and accept() */
 #define _ACTUAL_SOCKET_POOL_SIZE   (SOCKET_POOL_SIZE + \
                                     (SOCKET_POOL_SIZE * SOCKET_TCP_QUEUE_SIZE))
 #define SOCKET_BLKSIZE             (512)
 
+/**
+ * @brief   Receive buffer, large enough to fit a whole packet with max MTU
+ *
+ * Todo: How to get used MTU size?
+ */
+#define RECV_BUF_SIZE              (1500)
+
+/**
+ * @brief   Peek buffer, large enough to fit a whole packet with max MTU
+ */
+#define PEEK_BUF_SIZE              RECV_BUF_SIZE

Why is a separate macro needed for this, if it is not configurable?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/16850#pullrequestreview-756232088
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20210916/1f5d2d39/attachment.htm>


More information about the notifications mailing list