Re: [RFC] net: New API for applications to send and receive data


Jukka Rissanen
 

Hi Johan,

On Wed, 2016-04-27 at 21:56 +0300, Johan Hedberg wrote:
Hi Jukka,

On Wed, Apr 27, 2016, Jukka Rissanen wrote:
 +/**

+ * @brief Callback that is called when there is something received
from
+ * the network.
+ *
+ * @details It is callback responsibility to release the net_buf
when the
+ * data is no longer needed. The callback is called in fiber
context.
+ *
+ * @param context Network context
+ * @param status 0 if ok, <0 if there is an error
+ * @param iov List of net_buf that contain data
+ * @param user_data User supplied data
+ */
+typedef void (*net_readv_cb_t)(struct net_context *context,
+        int status,
+        struct net_buf *iov,
+        void *user_data);
A couple of things: I think "recv" is more intuitive than "read" when
it
comes to network operations (as opposed to e.g. file access). Same
goes
for "send" vs "write".
We have a net_send() API already that is why I used net_read() and
net_write() here. I will drop the v from the function name in next
version.


I don't think the fact that the net_buf parameter might have extra
fragments after it is enough justification for having it named "iov"
instead of simply "buf". Once fragmentation support is there it
should
be the default assumption that any buffer might have a list of
trailing
fragments. So at most I'd just mention this in the function
documentation.
Sure, the parameter name can be changed.



+/**
+ * @brief Register a receiver to get data from network.
+ *
+ * @details Application uses this to get data from network
connection.
+ * Caller needs to specify a callback function that is called when
data
+ * is received by the socket. When the IP stack has received data,
it then
+ * calls user specified callback and it is callback responsibility
to
+ * release the net_buf when the data is no longer needed.
+ * This function will return immediately as it only sets a
callback to be
+ * called when new data is received. Application can remove the
registered
+ * callback by calling the function with same context and setting
cb to NULL.
+ *
+ * @param context Network context
+ * @param cb User supplied callback function
+ * @param user_data User supplied data
+ *
+ * @return 0 if cb registration was ok, <0 otherwise
+ */
+int net_readv(struct net_context *context,
+       net_readv_cb_t cb,
+       void *user_data);
To make the purpose of the function clearer I'd rename this to
net_set_recv_cb() or similar.
Sure, np.



+/**
+ * @brief Callback that is called when user can send
+ * data to the network.
+ *
+ * @details The first call to this function will set *status to 0
and
+ * bytes_sent to 0. Application can then prepare the data to be
sent
+ * in net_buf fragment list. The data to be sent is returned to
the
+ * caller of the function. If application does not wish to send
anything
+ * it can just return NULL. In this case the caller will
deallocate the
+ * iov list. Application should set the *status to <0 to indicate
more
+ * detailed reason for the error if NULL is returned.
+ *
+ * For successfully sent UDP data, the IP stack will call this
+ * callback again with status set to 0 and bytes_sent telling how
many bytes
+ * were sent. If the UDP data was not sent for some reason, then
*status
+ * will have value < 0 and bytes_sent is set to 0.
+ *
+ * For TCP, the callback is called when the network connection has
been
+ * established. In this case the *status is 0 and bytes_sent is 0.
+ * If there is a connection timeout, the status code will be
-ETIMEDOUT
+ * and bytes_sent is set to 0. Other error codes are also possible
in
+ * which case the *status < 0 and bytes_sent will tell how many
bytes were
+ * successfully sent. If the *status is set to 0 then bytes_sent
will tell
+ * how many bytes were sent to the peer.
+ *
+ * The iov fragment list does not contain data that has been
successfully
+ * sent. Also the iov->frags->data of the first data fragment will
point to
+ * first byte that has not yet been sent.
If you want to maintain the same buffer list even after sending all
data
in some of them (something I don't think is a good idea since you
want
to unref and get the bufs back to the pool asap) I'd just set buf-
Hmm, the idea was that already sent buffers would be returned to the
pool immediately and removed from the fragment list. The fragment list
would only contain buffers that need to re-sent. This is needed in TCP,
for UDP we can always send the packets.

len
of all completed buffers to 0, i.e. the start of remaining data would
be
the first buffer in the chain with buf->len > 0. If the idea is to
keep
rotating the return parameter back to the next calls input parameter
I'd
just go forward in the chain and give a pointer to the first buffer
with
data left while having unrefed all the previous ones.


    If all the data was sent
+ * successfully, then the first item of iov list will have its
frags pointer
+ * set to NULL. Application can send more data if it wishes by
returning
+ * a new list of data fragments.
+ *
+ * The callback is called in fiber context.
+ *
+ * @param context Network context
+ * @param status 0 if ok, <0 if there is an error in stack side
+ * Application can return error code if needed via this pointer.
+ * @param bytes_sent How many bytes were sent.
+ * @param iov List of net_buf that contain data. If this is NULL,
then
+ * allocate the buf and fill it with data. If non-NULL, then the
protocol
+ * headers are already there and you can append the data.
+ * @param user_data User supplied data
+ *
+ * @return A valid net_buf that needs to be sent,
+ * NULL if user is not able to send anything.
+ */
+typedef struct net_buf *(*net_writev_cb_t)(struct net_context
*context,
+    int *status,
+    int bytes_sent,
+    struct net_buf *iov,
+    void *user_data);
I'm not really following what exactly the relationship is of the
input
net_buf parameter and the return value of this function. Sounds
unnecessarily complicated to me (the little bit that I did
understand).
Why does this function need an input net_buf parameter to start with?
If the data could not be sent, the stack calls this API again to allow
the app to do something about the situation. This is used in TCP so
that the application can know what data has been transferred
successfully to the peer.


Also, what's the reason this needs to be asynchronous this way? If
the
app is responsible for allocating the net_buf can't it just give it
straight to the stack with net_send() or similar, with an optional
callback to notify completion (or failure) of sending the data over
the
network?
The data to be sent could be given as a parameter when calling
net_write() but there would still be need to call the callback. I did
it like this in first version of API but after some experimenting it
felt more natural that all the data sending would be done from one
place (from cb), and the initial net_write() call would only do
connection establishment when needed.



+ * @brief Reply data to sender.
+ *
+ * @details This is a helper that will help user to reply data to
the
+ * sender. Application can use this function to reply data it
received
+ * via net_readv().
+ *
+ * @param context Network context
+ * @param timeout How long to wait until user can send data. This
is only
+ * used in TCP.
+ * @param iov Data to be sent
+ * @param cb User supplied callback function
+ * @param user_data User supplied data
+ *
+ * @return 0 if cb registration was ok, <0 otherwise
+ */
+int net_replyv(struct net_context *context,
+        int32_t timeout,
+        struct net_buf *iov,
+        net_writev_cb_t cb,
+        void *user_data);
This is basically something like the net_send() I mentioned earlier.
Yes, it is almost the same. As I mentioned to Ivan in another mail,
that in order to avoid to allocate extra context for reply data, the
net_reply() was created. It just swaps the IP addresses of the network
packet. It is not really mandatory but acts as a helper.


Cheers,
Jukka

Join devel@lists.zephyrproject.org to automatically receive all group messages.