ossl_quic_tx_packetiser_generate(): Always report if packets were sent

Even in case of later failure we need to flush
the previous packets.

Reviewed-by: Hugo Landau <hlandau@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #21700)

include/internal/quic_txp.h

@@ -81,20 +81,17 @@ void ossl_quic_tx_packetiser_record_received_closing_bytes(
  * generate any frames, and generating a datagram which coalesces packets for
  * any ELs which do.
  *
- * Returns TX_PACKETISER_RES_FAILURE on failure (e.g. allocation error),
- * TX_PACKETISER_RES_NO_PKT if no packets were sent (e.g. because nothing wants
- * to send anything), and TX_PACKETISER_RES_SENT_PKT if packets were sent.
+ * Returns 0 on failure (e.g. allocation error or other errors), 1 otherwise.
  *
- * *status is filled with status information about the generated packet, if any.
+ * *status is filled with status information about the generated packet.
+ * It is always filled even in case of failure. In particular, packets can be
+ * sent even if failure is later returned.
  * See QUIC_TXP_STATUS for details.
  */
-#define TX_PACKETISER_RES_FAILURE   0
-#define TX_PACKETISER_RES_NO_PKT    1
-#define TX_PACKETISER_RES_SENT_PKT  2
-
 typedef struct quic_txp_status_st {
     int sent_ack_eliciting; /* Was an ACK-eliciting packet sent? */
     int sent_handshake; /* Was a Handshake packet sent? */
+    size_t sent_pkt; /* Number of packets sent (0 if nothing was sent) */
 } QUIC_TXP_STATUS;
 
 int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,

ssl/quic/quic_channel.c

@@ -2372,6 +2372,7 @@ static void ch_default_packet_handler(QUIC_URXE *e, void *arg)
 static int ch_tx(QUIC_CHANNEL *ch)
 {
     QUIC_TXP_STATUS status;
+    int res;
 
     /*
      * RFC 9000 s. 10.2.2: Draining Connection State:
@@ -2412,8 +2413,8 @@ static int ch_tx(QUIC_CHANNEL *ch)
      * Best effort. In particular if TXP fails for some reason we should still
      * flush any queued packets which we already generated.
      */
-    switch (ossl_quic_tx_packetiser_generate(ch->txp, &status)) {
-    case TX_PACKETISER_RES_SENT_PKT:
+    res = ossl_quic_tx_packetiser_generate(ch->txp, &status);
+    if (status.sent_pkt > 0) {
         ch->have_sent_any_pkt = 1; /* Packet was sent */
 
         /*
@@ -2437,13 +2438,12 @@ static int ch_tx(QUIC_CHANNEL *ch)
             ch->rxku_pending_confirm = 0;
 
         ch_update_ping_deadline(ch);
-        break;
-
-    case TX_PACKETISER_RES_NO_PKT:
-        break; /* No packet was sent */
+    }
 
-    default:
+    if (!res) {
         /*
+         * Internal failure (e.g.  allocation, assertion).
+         *
          * One case where TXP can fail is if we reach a TX PN of 2**62 - 1. As
          * per RFC 9000 s. 12.3, if this happens we MUST close the connection
          * without sending a CONNECTION_CLOSE frame. This is actually handled as
@@ -2456,7 +2456,6 @@ static int ch_tx(QUIC_CHANNEL *ch)
          */
         ossl_quic_channel_raise_protocol_error(ch, QUIC_ERR_INTERNAL_ERROR, 0,
                                                "internal error (txp generate)");
-        break; /* Internal failure (e.g.  allocation, assertion) */
     }
 
     /* Flush packets to network. */

ssl/quic/quic_txp.c

@@ -702,7 +702,7 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
      *      sent to the FIFM.
      *
      */
-    int res = TX_PACKETISER_RES_FAILURE, rc;
+    int res = 0, rc;
     uint32_t archetype, enc_level;
     uint32_t conn_close_enc_level = QUIC_ENC_LEVEL_NUM;
     struct txp_pkt pkt[QUIC_ENC_LEVEL_NUM];
@@ -715,6 +715,8 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
          ++enc_level)
         pkt[enc_level].h_valid = 0;
 
+    memset(status, 0, sizeof(*status));
+
     /*
      * Should not be needed, but a sanity check in case anyone else has been
      * using the QTX.
@@ -798,8 +800,6 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
     }
 
     /* 4. Commit */
-    memset(status, 0, sizeof(*status));
-
     for (enc_level = QUIC_ENC_LEVEL_INITIAL;
          enc_level < QUIC_ENC_LEVEL_NUM;
          ++enc_level) {
@@ -833,7 +833,7 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
            && pkt[QUIC_ENC_LEVEL_HANDSHAKE].h.bytes_appended > 0);
 
     /* Flush & Cleanup */
-    res = pkts_done > 0 ? TX_PACKETISER_RES_SENT_PKT : TX_PACKETISER_RES_NO_PKT;
+    res = 1;
 out:
     ossl_qtx_finish_dgram(txp->args.qtx);
 
@@ -842,6 +842,8 @@ int ossl_quic_tx_packetiser_generate(OSSL_QUIC_TX_PACKETISER *txp,
          ++enc_level)
         txp_pkt_cleanup(&pkt[enc_level], txp);
 
+    status->sent_pkt = pkts_done;
+
     return res;
 }
 

test/quic_txp_test.c

@@ -1244,16 +1244,16 @@ static int run_script(int script_idx, const struct script_op *script)
     for (op = script, opn = 0; op->opcode != OPK_END; ++op, ++opn) {
         switch (op->opcode) {
         case OPK_TXP_GENERATE:
-            if (!TEST_int_eq(ossl_quic_tx_packetiser_generate(h.txp, &status),
-                             TX_PACKETISER_RES_SENT_PKT))
+            if (!TEST_true(ossl_quic_tx_packetiser_generate(h.txp, &status))
+                && !TEST_size_t_gt(status.sent_pkt, 0))
                 goto err;
 
             ossl_qtx_finish_dgram(h.args.qtx);
             ossl_qtx_flush_net(h.args.qtx);
             break;
         case OPK_TXP_GENERATE_NONE:
-            if (!TEST_int_eq(ossl_quic_tx_packetiser_generate(h.txp, &status),
-                             TX_PACKETISER_RES_NO_PKT))
+            if (!TEST_true(ossl_quic_tx_packetiser_generate(h.txp, &status))
+                && !TEST_size_t_eq(status.sent_pkt, 0))
                 goto err;
 
             break;