zhhyu7 commented on code in PR #20627:
URL: https://github.com/apache/nuttx/pull/20627#discussion_r4259536577


##########
net/tcp/tcp_input.c:
##########
@@ -1570,38 +1573,104 @@ static void tcp_input(FAR struct net_driver_s *dev, 
uint8_t domain,
 
         if ((tcp->flags & TCP_URG) != 0)
           {
-            dev->d_urglen = (tcp->urgp[0] << 8) | tcp->urgp[1];
-            if (dev->d_urglen > dev->d_len)
+            uint16_t urgp = (tcp->urgp[0] << 8) | tcp->urgp[1];
+
+            /* BSD/Linux default semantics (tcp_stdurg=0):
+             * The urgent pointer points ONE BYTE PAST the urgent byte.
+             * The single urgent byte is at payload[urgp - 1].
+             *
+             * If urgp is within the current segment (1 <= urgp <= d_len):
+             * - Extract the single urgent byte into conn->urgreadahead.
+             * - Compact the remaining payload in place.
+             * - Advance rcvseq by 1 for the urgent byte.
+             *
+             * If urgp > d_len:
+             * - The urgent byte is located in a future segment not yet
+             *   received; keep current payload intact in normal stream.
+             *
+             * If urgp == 0:
+             * - Invalid urgent offset; keep payload intact in normal stream.
+             */
+
+#ifdef CONFIG_NET_SOCKOPTS
+            if (urgp >= 1 && urgp <= dev->d_len &&
+                !_SO_GETOPT(conn->sconn.s_options, _SO_OOBINLINE))
+#else
+            if (urgp >= 1 && urgp <= dev->d_len)
+#endif
               {
-                /* There is more urgent data in the next segment to come. */
+                FAR struct iob_s *urg_iob;
+                uint8_t urg_byte;
+
+                urg_byte = dev->d_appdata[urgp - 1];
+
+                /* BSD/Linux policy: Keep at most one urgent byte.
+                 * If an unread urgent byte is already buffered,
+                 * overwrite it.
+                 */
+
+                if (conn->urgreadahead != NULL)
+                  {
+                    FAR struct iob_s *uiob = conn->urgreadahead;
+
+                    uiob->io_data[uiob->io_offset] = urg_byte;
+                    uiob->io_len                   = 1;
+                    uiob->io_pktlen                = 1;
+                  }
+                else
+                  {
+                    /* Allocate IOB for the urgent byte */
+
+                    urg_iob = iob_tryalloc(true);
+                    if (urg_iob == NULL)
+                      {
+                        /* Memory allocation failed: do not advance
+                         * sequence number and do not acknowledge this
+                         * data. Drop segment so peer will retransmit.
+                         */
+
+                        nerr("ERROR: Failed to allocate urgent IOB\n");
+                        goto drop;
+                      }
+
+                    if (iob_trycopyin(urg_iob, &urg_byte, 1, 0, true) < 0)
+                      {
+                        iob_free_chain(urg_iob);
+                        nerr("ERROR: Failed to copy urgent byte\n");
+                        goto drop;
+                      }
+
+                    conn->urgreadahead = urg_iob;
+                  }
+
+                /* Signal urgent data availability to poll/select and recv */
+
+                flags |= TCP_URGDATA;
+
+                /* Compact the rest of the payload in place:
+                 * Move bytes past the urgent byte down by 1 byte.
+                 */
+
+                if (urgp < dev->d_len)
+                  {
+                    memmove(&dev->d_appdata[urgp - 1],
+                            &dev->d_appdata[urgp],
+                            dev->d_len - urgp);
+                  }
+
+                dev->d_len -= 1;
+
+                /* Advance sequence number for the consumed urgent byte */
 
-                dev->d_urglen = dev->d_len;
+                net_incr32(conn->rcvseq, 1);

Review Comment:
   ```rcvseq``` is advanced by 1 for the urgent byte before the callback 
decides whether the rest of the segment is accepted. If the data is not 
accepted (for example with ```TCP_STOPPED```, where ```TCP_NEWDATA``` is not 
set), rcvseq no longer matches the peer. On retransmission, 
```tcp_trim_head()``` then drops the first byte of the segment instead of the 
urgent byte, so one byte of normal data is lost. The urgent byte should only be 
consumed together with the data it belongs to.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to