yamt commented on a change in pull request #4070:
URL: https://github.com/apache/incubator-nuttx/pull/4070#discussion_r663806067
##########
File path: net/tcp/tcp_send.c
##########
@@ -365,6 +365,13 @@ static void tcp_sendcommon(FAR struct net_driver_s *dev,
uint32_t rcvseq = tcp_getsequence(conn->rcvseq);
uint16_t recvwndo = tcp_get_recvwindow(dev, conn);
+#ifdef CONFIG_NET_TCP_WINDOW_SCALE
+ if (recvwndo > 0 && conn->rcv_scale > 0)
+ {
+ recvwndo >>= conn->rcv_scale;
+ }
+#endif
Review comment:
rcv_adv update below should use the unscaled value
##########
File path: net/tcp/tcp_send.c
##########
@@ -669,11 +677,24 @@ void tcp_synack(FAR struct net_driver_s *dev, FAR struct
tcp_conn_s *conn,
/* We send out the TCP Maximum Segment Size option with our ACK. */
- tcp->optdata[0] = TCP_OPT_MSS;
- tcp->optdata[1] = TCP_OPT_MSS_LEN;
- tcp->optdata[2] = tcp_mss >> 8;
- tcp->optdata[3] = tcp_mss & 0xff;
- tcp->tcpoffset = ((TCP_HDRLEN + TCP_OPT_MSS_LEN) / 4) << 4;
+ tcp->optdata[optlen++] = TCP_OPT_MSS;
+ tcp->optdata[optlen++] = TCP_OPT_MSS_LEN;
+ tcp->optdata[optlen++] = tcp_mss >> 8;
+ tcp->optdata[optlen++] = tcp_mss & 0xff;
+
+#ifdef CONFIG_NET_TCP_WINDOW_SCALE
+ if (tcp->flags == TCP_SYN ||
+ ((tcp->flags == (TCP_ACK | TCP_SYN)) && conn->snd_scale))
Review comment:
0 is a valid value for the window scaling option.
i guess you need a separate bit to record "window scaling option received".
##########
File path: net/tcp/Kconfig
##########
@@ -106,6 +106,27 @@ config NET_TCP_FAST_RETRANSMIT_WATERMARK
missing segment, without waiting for a retransmission
timer to
expire.
+config NET_TCP_WINDOW_SCALE
+ bool "Enable TCP/IP Window Scale Option"
+ default n
+ ---help---
+ RFC1323:
+ 2. TCP WINDOW SCALE OPTION
+ The window scale extension expands the definition of
the TCP
+ window to 32 bits and then uses a scale factor to carry
this 32-
+ bit value in the 16-bit Window field of the TCP header
(SEG.WND in
+ RFC-793).
+
+if NET_TCP_WINDOW_SCALE
+
+config NET_TCP_WINDOW_SCALE_FACTOR
+ int "TCP/IP Window Scale Factor"
+ default 7
Review comment:
why 7?
##########
File path: net/tcp/Kconfig
##########
@@ -106,6 +106,27 @@ config NET_TCP_FAST_RETRANSMIT_WATERMARK
missing segment, without waiting for a retransmission
timer to
expire.
+config NET_TCP_WINDOW_SCALE
+ bool "Enable TCP/IP Window Scale Option"
+ default n
+ ---help---
+ RFC1323:
+ 2. TCP WINDOW SCALE OPTION
+ The window scale extension expands the definition of
the TCP
+ window to 32 bits and then uses a scale factor to carry
this 32-
+ bit value in the 16-bit Window field of the TCP header
(SEG.WND in
+ RFC-793).
+
+if NET_TCP_WINDOW_SCALE
+
+config NET_TCP_WINDOW_SCALE_FACTOR
+ int "TCP/IP Window Scale Factor"
+ default 7
Review comment:
i guess 0 is a reasonable default because i suspect 16-bit window is
just enough for the majority of setup.
maybe a more reasonable thing is to calculate the minimum value which can
represent our max window. (from IOB and RECVBUF configs)
##########
File path: net/tcp/Kconfig
##########
@@ -106,6 +106,27 @@ config NET_TCP_FAST_RETRANSMIT_WATERMARK
missing segment, without waiting for a retransmission
timer to
expire.
+config NET_TCP_WINDOW_SCALE
+ bool "Enable TCP/IP Window Scale Option"
+ default n
+ ---help---
+ RFC1323:
+ 2. TCP WINDOW SCALE OPTION
+ The window scale extension expands the definition of
the TCP
+ window to 32 bits and then uses a scale factor to carry
this 32-
+ bit value in the 16-bit Window field of the TCP header
(SEG.WND in
+ RFC-793).
+
+if NET_TCP_WINDOW_SCALE
+
+config NET_TCP_WINDOW_SCALE_FACTOR
+ int "TCP/IP Window Scale Factor"
+ default 7
Review comment:
> maybe a more reasonable thing is to calculate the minimum value which
can represent our max window. (from IOB and RECVBUF configs)
as recv buffer size can change during the lifetime of the connection, maybe
just from IOB configurations.
##########
File path: net/tcp/Kconfig
##########
@@ -106,6 +106,27 @@ config NET_TCP_FAST_RETRANSMIT_WATERMARK
missing segment, without waiting for a retransmission
timer to
expire.
+config NET_TCP_WINDOW_SCALE
+ bool "Enable TCP/IP Window Scale Option"
+ default n
+ ---help---
+ RFC1323:
+ 2. TCP WINDOW SCALE OPTION
+ The window scale extension expands the definition of
the TCP
+ window to 32 bits and then uses a scale factor to carry
this 32-
+ bit value in the 16-bit Window field of the TCP header
(SEG.WND in
+ RFC-793).
+
+if NET_TCP_WINDOW_SCALE
+
+config NET_TCP_WINDOW_SCALE_FACTOR
+ int "TCP/IP Window Scale Factor"
+ default 7
Review comment:
@anchao do you want to do this "find a better scaling value" things in
this PR?
##########
File path: net/tcp/Kconfig
##########
@@ -106,6 +106,27 @@ config NET_TCP_FAST_RETRANSMIT_WATERMARK
missing segment, without waiting for a retransmission
timer to
expire.
+config NET_TCP_WINDOW_SCALE
+ bool "Enable TCP/IP Window Scale Option"
+ default n
+ ---help---
+ RFC1323:
+ 2. TCP WINDOW SCALE OPTION
+ The window scale extension expands the definition of
the TCP
+ window to 32 bits and then uses a scale factor to carry
this 32-
+ bit value in the 16-bit Window field of the TCP header
(SEG.WND in
+ RFC-793).
+
+if NET_TCP_WINDOW_SCALE
+
+config NET_TCP_WINDOW_SCALE_FACTOR
+ int "TCP/IP Window Scale Factor"
+ default 7
Review comment:
ok.
then i will merge this later today unless anyone beats me or raises an
objection.
(i don't like to merge a complex patch within 24 hours)
##########
File path: net/tcp/tcp_send.c
##########
@@ -372,6 +384,13 @@ static void tcp_sendcommon(FAR struct net_driver_s *dev,
/* Update the Receiver Window */
+#ifdef CONFIG_NET_TCP_WINDOW_SCALE
+ if (recvwndo > 0 && conn->rcv_scale > 0)
+ {
+ recvwndo <<= conn->rcv_scale;
+ }
+#endif
+
Review comment:
adjusting the window for UINT16_MAX and scaling here can confuse
tcp_should_send_recvwindow.
i feel it's simpler to do it in tcp_get_recvwindow.
how do you think?
##########
File path: net/sixlowpan/sixlowpan_tcpsend.c
##########
@@ -258,7 +258,12 @@ static int sixlowpan_tcp_header(FAR struct tcp_conn_s
*conn,
/* Update the TCP received window based on I/O buffer availability */
uint32_t rcvseq = tcp_getsequence(conn->rcvseq);
- uint16_t recvwndo = tcp_get_recvwindow(dev, conn);
+ uint32_t recvwndo = tcp_get_recvwindow(dev, conn);
+
+ if (recvwndo > UINT16_MAX)
+ {
+ recvwndo = UINT16_MAX;
+ }
Review comment:
doesn't this copy of tcp need scaling?
--
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-unsubscr...@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
