You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2023/01/13 09:38:37 UTC
[GitHub] [nuttx] pkarashchenko commented on a diff in pull request #8062: net/tcp: add Selective-ACK support (RFC2018)
pkarashchenko commented on code in PR #8062:
URL: https://github.com/apache/nuttx/pull/8062#discussion_r1069143021
##########
net/tcp/tcp.h:
##########
@@ -144,6 +149,23 @@ struct tcp_poll_s
FAR struct devif_callback_s *cb; /* Needed to teardown the poll */
};
+/* Out-of-order segments */
+
+struct tcp_ofoseg_s
+{
+ uint32_t left; /* Left edge of segment */
+ uint32_t right; /* Right edge of segment */
+ struct iob_s *data; /* Out-of-order buffering */
Review Comment:
```suggestion
FAR struct iob_s *data; /* Out-of-order buffering */
```
##########
net/tcp/tcp_callback.c:
##########
@@ -94,10 +94,156 @@ tcp_data_event(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn,
return flags;
}
+/****************************************************************************
+ * Name: tcp_ofoseg_data_event
+ *
+ * Description:
+ * Handle out-of-order segment to readahead poll.
+ *
+ * Assumptions:
+ * - This function must be called with the network locked.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_NET_TCP_OUT_OF_ORDER
+static uint16_t tcp_ofoseg_data_event(FAR struct net_driver_s *dev,
+ FAR struct tcp_conn_s *conn,
+ uint16_t flags)
+{
+ struct tcp_ofoseg_s *seg;
+ uint32_t rcvseq;
+ int i;
+
+ /* Assume that we will ACK the data. The data will be ACKed if it is
+ * placed in the read-ahead buffer -OR- if it zero length
+ */
+
+ flags |= TCP_SNDACK;
+
+ /* Get the receive sequence number */
+
+ rcvseq = tcp_getsequence(conn->rcvseq);
+
+ ninfo("TCP OFOSEG rcvseq [%" PRIu32 "]\n", rcvseq);
+
+ /* Foreach out-of-order segments */
+
+ i = 0;
Review Comment:
```suggestion
```
##########
net/tcp/tcp_callback.c:
##########
@@ -94,10 +94,156 @@ tcp_data_event(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn,
return flags;
}
+/****************************************************************************
+ * Name: tcp_ofoseg_data_event
+ *
+ * Description:
+ * Handle out-of-order segment to readahead poll.
+ *
+ * Assumptions:
+ * - This function must be called with the network locked.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_NET_TCP_OUT_OF_ORDER
+static uint16_t tcp_ofoseg_data_event(FAR struct net_driver_s *dev,
+ FAR struct tcp_conn_s *conn,
+ uint16_t flags)
+{
+ struct tcp_ofoseg_s *seg;
Review Comment:
```suggestion
FAR struct tcp_ofoseg_s *seg;
```
##########
net/tcp/tcp_input.c:
##########
@@ -257,6 +257,370 @@ static void tcp_snd_wnd_update(FAR struct tcp_conn_s *conn,
}
}
+#ifdef CONFIG_NET_TCP_OUT_OF_ORDER
+
+/****************************************************************************
+ * Name: tcp_rebuild_ofosegs
+ *
+ * Description:
+ * Re-build out-of-order pool from incoming segment
+ *
+ * Input Parameters:
+ * conn - The TCP connection of interest
+ * ofoseg - Pointer to incoming out-of-order segment
+ * start - Index of start postion of segment pool
+ *
+ * Returned Value:
+ * True if incoming data has been consumed
+ *
+ * Assumptions:
+ * The network is locked.
+ *
+ ****************************************************************************/
+
+static bool tcp_rebuild_ofosegs(FAR struct tcp_conn_s *conn,
+ FAR struct tcp_ofoseg_s *ofoseg,
+ int start)
+{
+ struct tcp_ofoseg_s *seg;
+ int i;
+
+ for (i = start; i < conn->nofosegs && ofoseg->data != NULL; i++)
+ {
+ seg = &conn->ofosegs[i];
+
+ /* ofoseg |~~~
+ * segpool |---|
+ */
+
+ if (TCP_SEQ_GTE(ofoseg->left, seg->left))
+ {
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ if (TCP_SEQ_GT(ofoseg->left, seg->right))
+ {
+ continue;
+ }
+
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ else if (ofoseg->left == seg->right)
+ {
+ tcp_dataconcat(&seg->data, &ofoseg->data);
+ seg->right = ofoseg->right;
+ }
+
+ /* ofoseg |--|
+ * segpool |---|
+ */
+
+ else if (TCP_SEQ_LTE(ofoseg->right, seg->right))
+ {
+ iob_free_chain(ofoseg->data);
+ ofoseg->data = NULL;
+ }
+
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ else if (TCP_SEQ_GT(ofoseg->right, seg->right))
+ {
+ ofoseg->data =
+ iob_trimhead(ofoseg->data,
+ TCP_SEQ_SUB(seg->right, ofoseg->left));
+ tcp_dataconcat(&seg->data, &ofoseg->data);
+ seg->right = ofoseg->right;
+ }
+ }
+
+ /* ofoseg |~~~
+ * segpool |---|
+ */
+
+ else
+ {
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ if (ofoseg->right == seg->left)
+ {
+ tcp_dataconcat(&ofoseg->data, &seg->data);
+ seg->data = ofoseg->data;
+ seg->left = ofoseg->left;
+ ofoseg->data = NULL;
+ }
+
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ else if (TCP_SEQ_LT(ofoseg->right, seg->left))
+ {
+ continue;
+ }
+
+ /* ofoseg |---|~|
+ * segpool |--|
+ */
+
+ else if (TCP_SEQ_GTE(ofoseg->right, seg->right))
+ {
+ iob_free_chain(seg->data);
+ *seg = *ofoseg;
+ ofoseg->data = NULL;
+ }
+
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ else if (TCP_SEQ_GT(ofoseg->right, seg->left))
+ {
+ ofoseg->data =
+ iob_trimtail(ofoseg->data,
+ ofoseg->right - seg->left);
+ tcp_dataconcat(&ofoseg->data, &seg->data);
+ seg->data = ofoseg->data;
+ seg->left = ofoseg->left;
+ ofoseg->data = NULL;
+ }
+ }
+ }
+
+ return (ofoseg->data == NULL);
+}
+
+/****************************************************************************
+ * Name: tcp_input_ofosegs
+ *
+ * Description:
+ * Handle incoming TCP data to out-of-order pool
+ *
+ * Input Parameters:
+ * dev - The device driver structure containing the received TCP packet.
+ * conn - The TCP connection of interest
+ * iplen - Length of the IP header (IPv4_HDRLEN or IPv6_HDRLEN).
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions:
+ * The network is locked.
+ *
+ ****************************************************************************/
+
+static void tcp_input_ofosegs(FAR struct net_driver_s *dev,
+ FAR struct tcp_conn_s *conn,
+ unsigned int iplen)
+{
+ struct tcp_ofoseg_s ofoseg;
+ bool rebuild;
+ int len;
+ int i;
Review Comment:
```suggestion
int i = 0;
```
##########
net/tcp/tcp_input.c:
##########
@@ -257,6 +257,370 @@ static void tcp_snd_wnd_update(FAR struct tcp_conn_s *conn,
}
}
+#ifdef CONFIG_NET_TCP_OUT_OF_ORDER
+
+/****************************************************************************
+ * Name: tcp_rebuild_ofosegs
+ *
+ * Description:
+ * Re-build out-of-order pool from incoming segment
+ *
+ * Input Parameters:
+ * conn - The TCP connection of interest
+ * ofoseg - Pointer to incoming out-of-order segment
+ * start - Index of start postion of segment pool
+ *
+ * Returned Value:
+ * True if incoming data has been consumed
+ *
+ * Assumptions:
+ * The network is locked.
+ *
+ ****************************************************************************/
+
+static bool tcp_rebuild_ofosegs(FAR struct tcp_conn_s *conn,
+ FAR struct tcp_ofoseg_s *ofoseg,
+ int start)
+{
+ struct tcp_ofoseg_s *seg;
+ int i;
+
+ for (i = start; i < conn->nofosegs && ofoseg->data != NULL; i++)
+ {
+ seg = &conn->ofosegs[i];
+
+ /* ofoseg |~~~
+ * segpool |---|
+ */
+
+ if (TCP_SEQ_GTE(ofoseg->left, seg->left))
+ {
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ if (TCP_SEQ_GT(ofoseg->left, seg->right))
+ {
+ continue;
+ }
+
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ else if (ofoseg->left == seg->right)
+ {
+ tcp_dataconcat(&seg->data, &ofoseg->data);
+ seg->right = ofoseg->right;
+ }
+
+ /* ofoseg |--|
+ * segpool |---|
+ */
+
+ else if (TCP_SEQ_LTE(ofoseg->right, seg->right))
+ {
+ iob_free_chain(ofoseg->data);
+ ofoseg->data = NULL;
+ }
+
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ else if (TCP_SEQ_GT(ofoseg->right, seg->right))
+ {
+ ofoseg->data =
+ iob_trimhead(ofoseg->data,
+ TCP_SEQ_SUB(seg->right, ofoseg->left));
+ tcp_dataconcat(&seg->data, &ofoseg->data);
+ seg->right = ofoseg->right;
+ }
+ }
+
+ /* ofoseg |~~~
+ * segpool |---|
+ */
+
+ else
+ {
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ if (ofoseg->right == seg->left)
+ {
+ tcp_dataconcat(&ofoseg->data, &seg->data);
+ seg->data = ofoseg->data;
+ seg->left = ofoseg->left;
+ ofoseg->data = NULL;
+ }
+
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ else if (TCP_SEQ_LT(ofoseg->right, seg->left))
+ {
+ continue;
+ }
+
+ /* ofoseg |---|~|
+ * segpool |--|
+ */
+
+ else if (TCP_SEQ_GTE(ofoseg->right, seg->right))
+ {
+ iob_free_chain(seg->data);
+ *seg = *ofoseg;
+ ofoseg->data = NULL;
+ }
+
+ /* ofoseg |---|
+ * segpool |---|
+ */
+
+ else if (TCP_SEQ_GT(ofoseg->right, seg->left))
+ {
+ ofoseg->data =
+ iob_trimtail(ofoseg->data,
+ ofoseg->right - seg->left);
+ tcp_dataconcat(&ofoseg->data, &seg->data);
+ seg->data = ofoseg->data;
+ seg->left = ofoseg->left;
+ ofoseg->data = NULL;
+ }
+ }
+ }
+
+ return (ofoseg->data == NULL);
+}
+
+/****************************************************************************
+ * Name: tcp_input_ofosegs
+ *
+ * Description:
+ * Handle incoming TCP data to out-of-order pool
+ *
+ * Input Parameters:
+ * dev - The device driver structure containing the received TCP packet.
+ * conn - The TCP connection of interest
+ * iplen - Length of the IP header (IPv4_HDRLEN or IPv6_HDRLEN).
+ *
+ * Returned Value:
+ * None
+ *
+ * Assumptions:
+ * The network is locked.
+ *
+ ****************************************************************************/
+
+static void tcp_input_ofosegs(FAR struct net_driver_s *dev,
+ FAR struct tcp_conn_s *conn,
+ unsigned int iplen)
+{
+ struct tcp_ofoseg_s ofoseg;
+ bool rebuild;
+ int len;
+ int i;
+
+ ofoseg.left =
+ tcp_getsequence(((FAR struct tcp_hdr_s *)IPBUF(iplen))->seqno);
+
+ /* Calculate the pending size of out-of-order cache, if the input edge can
+ * not fill the adjacent segments, drop it
+ */
+
+ if (tcp_ofoseg_bufsize(conn) > CONFIG_NET_TCP_OUT_OF_ORDER_BUFSIZE &&
+ ofoseg.left >= conn->ofosegs[0].left)
+ {
+ return;
+ }
+
+ /* Get left/right edge from incoming data */
+
+ len = (dev->d_appdata - dev->d_iob->io_data) - dev->d_iob->io_offset;
+ ofoseg.right = TCP_SEQ_ADD(ofoseg.left, dev->d_iob->io_pktlen - len);
+
+ ninfo("TCP OFOSEG out-of-order "
+ "[%" PRIu32 " : %" PRIu32 " : %" PRIu32 "]\n",
+ ofoseg.left, ofoseg.right, TCP_SEQ_SUB(ofoseg.right, ofoseg.left));
+
+ /* Trim l3/l4 header to reserve appdata */
+
+ dev->d_iob = iob_trimhead(dev->d_iob, len);
+ if (dev->d_iob == NULL)
+ {
+ /* No available data, clear device buffer */
+
+ goto clear;
+ }
+
+ ofoseg.data = dev->d_iob;
+
+ /* Build out-of-order pool */
+
+ rebuild = tcp_rebuild_ofosegs(conn, &ofoseg, 0);
+
+ /* Incoming segment out of order from existing pool, add to new segment */
+
+ if (!rebuild && conn->nofosegs != TCP_SACK_RANGES_MAX)
+ {
+ conn->ofosegs[conn->nofosegs] = ofoseg;
+ conn->nofosegs++;
+ rebuild = true;
+ }
+
+ /* Try Re-order ofosegs */
+
+ if (rebuild &&
+ tcp_reorder_ofosegs(conn->nofosegs, (FAR void *)conn->ofosegs))
+ {
+ /* Re-build out-of-order pool after re-order */
+
+ i = 0;
+ while (i < conn->nofosegs - 1)
Review Comment:
```suggestion
while (i < conn->nofosegs - 1)
```
or change to `for`
##########
net/tcp/tcp_callback.c:
##########
@@ -94,10 +94,156 @@ tcp_data_event(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn,
return flags;
}
+/****************************************************************************
+ * Name: tcp_ofoseg_data_event
+ *
+ * Description:
+ * Handle out-of-order segment to readahead poll.
+ *
+ * Assumptions:
+ * - This function must be called with the network locked.
+ *
+ ****************************************************************************/
+
+#ifdef CONFIG_NET_TCP_OUT_OF_ORDER
+static uint16_t tcp_ofoseg_data_event(FAR struct net_driver_s *dev,
+ FAR struct tcp_conn_s *conn,
+ uint16_t flags)
+{
+ struct tcp_ofoseg_s *seg;
+ uint32_t rcvseq;
+ int i;
Review Comment:
```suggestion
int i = 0;
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org