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 2021/06/01 10:09:14 UTC

[GitHub] [incubator-nuttx] yamt opened a new pull request #3818: tcp: recv window update improvement

yamt opened a new pull request #3818:
URL: https://github.com/apache/incubator-nuttx/pull/3818


   ## Summary
   
   ## Impact
   
   ## Testing
   
   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #3818: tcp: recv window update improvement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #3818:
URL: https://github.com/apache/incubator-nuttx/pull/3818


   


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on a change in pull request #3818: tcp: recv window update improvement

Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #3818:
URL: https://github.com/apache/incubator-nuttx/pull/3818#discussion_r650387042



##########
File path: net/tcp/tcp_recvwindow.c
##########
@@ -163,3 +163,80 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
 
   return recvwndo;
 }
+
+bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn)
+{
+  FAR struct net_driver_s *dev = conn->dev;
+  uint16_t win;
+  size_t maxiob;
+  uint16_t maxwin;
+  uint16_t oldwin;
+  uint32_t rcvseq;
+  uint16_t adv;
+  uint16_t mss;
+
+  /* If the window shrunk, don't send. */
+
+  rcvseq = tcp_getsequence(conn->rcvseq);
+  if (TCP_SEQ_GTE(conn->rcv_adv, rcvseq))

Review comment:
       ok

##########
File path: net/tcp/tcp_recvwindow.c
##########
@@ -163,3 +163,80 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
 
   return recvwndo;
 }
+
+bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn)
+{
+  FAR struct net_driver_s *dev = conn->dev;
+  uint16_t win;
+  size_t maxiob;
+  uint16_t maxwin;
+  uint16_t oldwin;
+  uint32_t rcvseq;
+  uint16_t adv;
+  uint16_t mss;
+
+  /* If the window shrunk, don't send. */
+
+  rcvseq = tcp_getsequence(conn->rcvseq);
+  if (TCP_SEQ_GTE(conn->rcv_adv, rcvseq))
+    {
+      oldwin = TCP_SEQ_SUB(conn->rcv_adv, rcvseq);
+    }
+  else
+    {
+      /* This can happen for example when:
+       *
+       * - we shrunk the window
+       * - zero window probes advanced rcvseq
+       */
+
+      oldwin = 0;
+    }
+
+  win = tcp_get_recvwindow(dev, conn);
+  if (win <= oldwin)
+    {
+      return false;
+    }
+
+  adv = win - oldwin;
+
+  /* The following conditions are inspired from NetBSD TCP stack.
+   *
+   * - If we can extend the window by the half of the max possible size,
+   *   send it.
+   *
+   * - If we can extend the window by 2 * mss, send it.
+   */
+
+  /* Calculate the max possible window size for the connection.
+   * This needs to be in sync with tcp_get_recvwindow().
+   */
+
+  maxiob = (CONFIG_IOB_NBUFFERS - CONFIG_IOB_THROTTLE) * CONFIG_IOB_BUFSIZE;
+  if (maxiob >= UINT16_MAX)
+    {
+      maxwin = UINT16_MAX;
+    }
+  else
+    {
+      maxwin = maxiob;
+    }

Review comment:
       do you mean the maxwin calculation? i will.

##########
File path: net/tcp/tcp_recvwindow.c
##########
@@ -163,3 +163,80 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
 
   return recvwndo;
 }
+
+bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn)
+{
+  FAR struct net_driver_s *dev = conn->dev;
+  uint16_t win;
+  size_t maxiob;
+  uint16_t maxwin;
+  uint16_t oldwin;
+  uint32_t rcvseq;
+  uint16_t adv;
+  uint16_t mss;
+
+  /* If the window shrunk, don't send. */
+
+  rcvseq = tcp_getsequence(conn->rcvseq);
+  if (TCP_SEQ_GTE(conn->rcv_adv, rcvseq))

Review comment:
       done

##########
File path: net/tcp/tcp_recvwindow.c
##########
@@ -163,3 +163,80 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
 
   return recvwndo;
 }
+
+bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn)
+{
+  FAR struct net_driver_s *dev = conn->dev;
+  uint16_t win;
+  size_t maxiob;
+  uint16_t maxwin;
+  uint16_t oldwin;
+  uint32_t rcvseq;
+  uint16_t adv;
+  uint16_t mss;
+
+  /* If the window shrunk, don't send. */
+
+  rcvseq = tcp_getsequence(conn->rcvseq);
+  if (TCP_SEQ_GTE(conn->rcv_adv, rcvseq))
+    {
+      oldwin = TCP_SEQ_SUB(conn->rcv_adv, rcvseq);
+    }
+  else
+    {
+      /* This can happen for example when:
+       *
+       * - we shrunk the window
+       * - zero window probes advanced rcvseq
+       */
+
+      oldwin = 0;
+    }
+
+  win = tcp_get_recvwindow(dev, conn);
+  if (win <= oldwin)
+    {
+      return false;
+    }
+
+  adv = win - oldwin;
+
+  /* The following conditions are inspired from NetBSD TCP stack.
+   *
+   * - If we can extend the window by the half of the max possible size,
+   *   send it.
+   *
+   * - If we can extend the window by 2 * mss, send it.
+   */
+
+  /* Calculate the max possible window size for the connection.
+   * This needs to be in sync with tcp_get_recvwindow().
+   */
+
+  maxiob = (CONFIG_IOB_NBUFFERS - CONFIG_IOB_THROTTLE) * CONFIG_IOB_BUFSIZE;
+  if (maxiob >= UINT16_MAX)
+    {
+      maxwin = UINT16_MAX;
+    }
+  else
+    {
+      maxwin = maxiob;
+    }

Review comment:
       done




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on pull request #3818: tcp: recv window update improvement

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3818:
URL: https://github.com/apache/incubator-nuttx/pull/3818#issuecomment-859247820


   last push was an update after https://github.com/apache/incubator-nuttx/pull/3865


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on pull request #3818: tcp: recv window update improvement

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3818:
URL: https://github.com/apache/incubator-nuttx/pull/3818#issuecomment-857410086


   > marked as a draft as i found a bug
   
   fixed in the last push


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #3818: tcp: recv window update improvement

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #3818:
URL: https://github.com/apache/incubator-nuttx/pull/3818#discussion_r650336475



##########
File path: net/tcp/tcp_recvwindow.c
##########
@@ -163,3 +163,80 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
 
   return recvwndo;
 }
+
+bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn)
+{
+  FAR struct net_driver_s *dev = conn->dev;
+  uint16_t win;
+  size_t maxiob;
+  uint16_t maxwin;
+  uint16_t oldwin;
+  uint32_t rcvseq;
+  uint16_t adv;
+  uint16_t mss;
+
+  /* If the window shrunk, don't send. */
+
+  rcvseq = tcp_getsequence(conn->rcvseq);
+  if (TCP_SEQ_GTE(conn->rcv_adv, rcvseq))
+    {
+      oldwin = TCP_SEQ_SUB(conn->rcv_adv, rcvseq);
+    }
+  else
+    {
+      /* This can happen for example when:
+       *
+       * - we shrunk the window
+       * - zero window probes advanced rcvseq
+       */
+
+      oldwin = 0;
+    }
+
+  win = tcp_get_recvwindow(dev, conn);
+  if (win <= oldwin)
+    {
+      return false;
+    }
+
+  adv = win - oldwin;
+
+  /* The following conditions are inspired from NetBSD TCP stack.
+   *
+   * - If we can extend the window by the half of the max possible size,
+   *   send it.
+   *
+   * - If we can extend the window by 2 * mss, send it.
+   */
+
+  /* Calculate the max possible window size for the connection.
+   * This needs to be in sync with tcp_get_recvwindow().
+   */
+
+  maxiob = (CONFIG_IOB_NBUFFERS - CONFIG_IOB_THROTTLE) * CONFIG_IOB_BUFSIZE;
+  if (maxiob >= UINT16_MAX)
+    {
+      maxwin = UINT16_MAX;
+    }
+  else
+    {
+      maxwin = maxiob;
+    }

Review comment:
       should we add a new function for line 212-224 and let here and tcp_get_recvwindow call it?

##########
File path: net/tcp/tcp_recvwindow.c
##########
@@ -163,3 +163,80 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
 
   return recvwndo;
 }
+
+bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn)
+{
+  FAR struct net_driver_s *dev = conn->dev;
+  uint16_t win;
+  size_t maxiob;
+  uint16_t maxwin;
+  uint16_t oldwin;
+  uint32_t rcvseq;
+  uint16_t adv;
+  uint16_t mss;
+
+  /* If the window shrunk, don't send. */
+
+  rcvseq = tcp_getsequence(conn->rcvseq);
+  if (TCP_SEQ_GTE(conn->rcv_adv, rcvseq))

Review comment:
       TCP_SEQ_GT

##########
File path: net/tcp/tcp_recvwindow.c
##########
@@ -163,3 +163,80 @@ uint16_t tcp_get_recvwindow(FAR struct net_driver_s *dev,
 
   return recvwndo;
 }
+
+bool tcp_should_send_recvwindow(FAR struct tcp_conn_s *conn)
+{
+  FAR struct net_driver_s *dev = conn->dev;
+  uint16_t win;
+  size_t maxiob;
+  uint16_t maxwin;
+  uint16_t oldwin;
+  uint32_t rcvseq;
+  uint16_t adv;
+  uint16_t mss;
+
+  /* If the window shrunk, don't send. */
+
+  rcvseq = tcp_getsequence(conn->rcvseq);
+  if (TCP_SEQ_GTE(conn->rcv_adv, rcvseq))
+    {
+      oldwin = TCP_SEQ_SUB(conn->rcv_adv, rcvseq);
+    }
+  else
+    {
+      /* This can happen for example when:
+       *
+       * - we shrunk the window
+       * - zero window probes advanced rcvseq
+       */
+
+      oldwin = 0;
+    }
+
+  win = tcp_get_recvwindow(dev, conn);
+  if (win <= oldwin)
+    {
+      return false;
+    }
+
+  adv = win - oldwin;
+
+  /* The following conditions are inspired from NetBSD TCP stack.
+   *
+   * - If we can extend the window by the half of the max possible size,
+   *   send it.
+   *
+   * - If we can extend the window by 2 * mss, send it.
+   */
+
+  /* Calculate the max possible window size for the connection.
+   * This needs to be in sync with tcp_get_recvwindow().
+   */
+
+  maxiob = (CONFIG_IOB_NBUFFERS - CONFIG_IOB_THROTTLE) * CONFIG_IOB_BUFSIZE;
+  if (maxiob >= UINT16_MAX)
+    {
+      maxwin = UINT16_MAX;
+    }
+  else
+    {
+      maxwin = maxiob;
+    }

Review comment:
       Yes.




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] yamt commented on pull request #3818: tcp: recv window update improvement

Posted by GitBox <gi...@apache.org>.
yamt commented on pull request #3818:
URL: https://github.com/apache/incubator-nuttx/pull/3818#issuecomment-857361432


   marked as a draft as i found a bug


-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org