[riot-notifications] [RIOT-OS/RIOT] gnrc/tcp : Expose configurations to Kconfig (#14126)

Simon Brummer notifications at github.com
Thu May 28 13:47:35 CEST 2020


@brummer-simon requested changes on this pull request.

The changes did not break any existing tests (tested on "native" and "nucleo-f401re") and the custom configurations are propagated into the binary. From a technical point of view it works good, however there are some descriptions value descriptions that are a bit misleading.

If you fix those the PR is fine by me.

> +/**
+ * @defgroup net_gnrc_tcp_conf GNRC TCP compile configurations
+ * @ingroup net_gnrc_conf
+ *
+ * ## Calculating RTO
+ * To calculate retransmission timeout (RTO), Round Trip Time (RTT) needs to be
+ * taken into account. SRTT (smoothed round-trip time) and RTTVAR (round-trip
+ * time variation) are hence calculated as follows:
+ *
+ *     RTTVAR <- (1 - beta) * RTTVAR + beta * |SRTT - R'|
+ *     SRTT <- (1 - alpha) * SRTT + alpha * R'
+ *
+ * where alpha ( @ref CONFIG_GNRC_TCP_RTO_A_DIV ) and beta
+ * ( @ref CONFIG_GNRC_TCP_RTO_B_DIV) are constants, and R' is the instantaneous
+ * RTT value.
+ *



The comment above is a little bit misleading. CONFIG_GNRC_TCP_RTO_A_DIV is not the alpha value, it is the denominator for alpha.

The default value for Alpha is 1/8, therefore CONFIG_GNRC_TCP_RTO_A_DIV is 8. The same goes for beta and CONFIG_GNRC_TCP_RTO_B_DIV.


>  #endif
 
 /**
  * @brief Default receive window size
  */
 #ifndef GNRC_TCP_DEFAULT_WINDOW
-#define GNRC_TCP_DEFAULT_WINDOW (GNRC_TCP_MSS * GNRC_TCP_MSS_MULTIPLICATOR)
+#define GNRC_TCP_DEFAULT_WINDOW (GNRC_TCP_MSS * CONFIG_GNRC_TCP_MSS_MULTIPLICATOR)
 #endif
 

Why is the default receive window size not configurable?

> @@ -54,22 +79,22 @@ extern "C" {
 /**

Why is the maximum segment size not configurable? The MSS is usually aligned with the underlying network layer for performance reasons, however is could be customized.

> +config GNRC_TCP_MSL
+    int "Maximum segment lifetime (MSL) in microseconds"
+    default 30000000
+    help
+        Maximum segment lifetime (MSL) in microseconds. Default value is 30
+        seconds.
+
+config GNRC_TCP_MSS_MULTIPLICATOR
+    int "Number of MSS sized packets stored in receive buffer"
+    default 1
+    help
+        Configure MSS Multiplicator i.e. number of MSS sized packets stored in
+        receive buffer.
+
+config GNRC_TCP_RCV_BUFFERS
+    int "Number of preallocated receive buffers"

The description could be better. This value determines how many parallel TCP connection can be active at the same time.

> +        transmitted segment. Refer to RFC 6298 for more information.
+
+config GNRC_TCP_RTO_UPPER_BOUND
+    int "Upper bound for RTO in microseconds"
+    default 60000000
+    help
+        Upper bound value for retransmission timeout (RTO) in microseconds.
+        Default value is 60000000 microseconds (60 seconds). Refer to RFC 6298
+        for more information.
+
+config GNRC_TCP_RTO_GRANULARITY
+    int "Clock granularity for RTO in microseconds"
+    default 10000
+    help
+        Clock granularity for retransmission timeout (RTO) for TCP in
+        microseconds. Default value is 10000000 microseconds (10 seconds). 

This is not true. The default granularity is 10 miliseconds.

> +    default 60000000
+    help
+        Upper bound value for retransmission timeout (RTO) in microseconds.
+        Default value is 60000000 microseconds (60 seconds). Refer to RFC 6298
+        for more information.
+
+config GNRC_TCP_RTO_GRANULARITY
+    int "Clock granularity for RTO in microseconds"
+    default 10000
+    help
+        Clock granularity for retransmission timeout (RTO) for TCP in
+        microseconds. Default value is 10000000 microseconds (10 seconds). 
+        Refer to RFC 6298 for more information.
+
+config GNRC_TCP_RTO_A_DIV
+    int "Alpha value for RTO calculation"

As mentioned above it is not the alpha value. Alpha is 1 / GNRC_TCP_RTO_A_DIV

> +        Default value is 60000000 microseconds (60 seconds). Refer to RFC 6298
+        for more information.
+
+config GNRC_TCP_RTO_GRANULARITY
+    int "Clock granularity for RTO in microseconds"
+    default 10000
+    help
+        Clock granularity for retransmission timeout (RTO) for TCP in
+        microseconds. Default value is 10000000 microseconds (10 seconds). 
+        Refer to RFC 6298 for more information.
+
+config GNRC_TCP_RTO_A_DIV
+    int "Alpha value for RTO calculation"
+    default 8
+
+config GNRC_TCP_RTO_B_DIV

As mentioned above it is not the beta value. Beta is 1 / GNRC_TCP_RTO_B_DIV

-- 
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/14126#pullrequestreview-420011438
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20200528/c7433c80/attachment.htm>


More information about the notifications mailing list