protocols/rssi: clamp peer-advertised connection parameters

include/rogue/protocols/rssi/Controller.h

@@ -401,6 +401,40 @@ class Controller : public rogue::EnableSharedFromThis<rogue::protocols::rssi::Co
      */
     uint8_t curMaxCumAck();
 
+    /**
+     * @brief Clamps peer-advertised connection parameters to the locally legal range.
+     *
+     * @details
+     * The RSSI SYN/SYN-ACK handshake adopts the peer's advertised parameters. A peer
+     * advertising out-of-range values (zero outstanding segments, zero timeouts, a
+     * sub-header-size segment, or a window larger than the locally offered maximum)
+     * would otherwise drive the controller into illegal state (for example
+     * `curMaxBuffers == 0` makes the outbound wait loop spin forever). This helper
+     * clamps in place to the legal range instead of rejecting the connection, which
+     * preserves backward/forward compatibility with conformant peers. SLAC firmware
+     * never advertises invalid parameters, so in practice this is defense-in-depth.
+     *
+     * Pure (no member state) so it can be unit tested directly. Mirrors the firmware
+     * `validPeerParams`/`clampWindowSize`/`clampBufferSize` checks in surf
+     * `protocols/rssi/v1/rtl/RssiConnFsm.vhd`.
+     *
+     * @param maxBuffers     Peer max outstanding segments; clamped to [1, locMaxBuffers].
+     * @param maxSegment     Peer max segment size; clamped to [HeaderSize, locMaxSegment].
+     * @param cumAckTout     Peer cumulative-ACK timeout; clamped to >= 1.
+     * @param retranTout     Peer retransmit timeout; clamped to >= 1.
+     * @param nullTout       Peer null-segment timeout; clamped to >= 1.
+     * @param locMaxBuffers  Locally offered max outstanding segments (upper bound).
+     * @param locMaxSegment  Locally offered max segment size (upper bound).
+     * @return True if any parameter was modified, false if all were already legal.
+     */
+    static bool clampPeerParams(uint8_t& maxBuffers,
+                                uint16_t& maxSegment,
+                                uint16_t& cumAckTout,
+                                uint16_t& retranTout,
+                                uint16_t& nullTout,
+                                uint8_t locMaxBuffers,
+                                uint16_t locMaxSegment);
+
     /** @brief Resets runtime counters. */
     void resetCounters();
 

src/rogue/protocols/rssi/Controller.cpp

@@ -696,6 +696,51 @@ int8_t rpr::Controller::retransmit(uint8_t id) {
     return 1;
 }
 
+//! Clamp peer-advertised connection parameters to the locally legal range
+bool rpr::Controller::clampPeerParams(uint8_t& maxBuffers,
+                                      uint16_t& maxSegment,
+                                      uint16_t& cumAckTout,
+                                      uint16_t& retranTout,
+                                      uint16_t& nullTout,
+                                      uint8_t locMaxBuffers,
+                                      uint16_t locMaxSegment) {
+    bool clamped = false;
+
+    // Max outstanding segments: at least 1, never more than locally offered
+    if (maxBuffers == 0) {
+        maxBuffers = 1;
+        clamped    = true;
+    } else if (maxBuffers > locMaxBuffers) {
+        maxBuffers = locMaxBuffers;
+        clamped    = true;
+    }
+
+    // Max segment size: must hold at least a header, never more than locally offered
+    if (maxSegment < static_cast<uint16_t>(rpr::Header::HeaderSize)) {
+        maxSegment = static_cast<uint16_t>(rpr::Header::HeaderSize);
+        clamped    = true;
+    } else if (maxSegment > locMaxSegment) {
+        maxSegment = locMaxSegment;
+        clamped    = true;
+    }
+
+    // Timeouts: a zero value would make the corresponding timer expire immediately
+    if (cumAckTout == 0) {
+        cumAckTout = 1;
+        clamped    = true;
+    }
+    if (retranTout == 0) {
+        retranTout = 1;
+        clamped    = true;
+    }
+    if (nullTout == 0) {
+        nullTout = 1;
+        clamped  = true;
+    }
+
+    return clamped;
+}
+
 //! Convert rssi time to microseconds
 void rpr::Controller::convTime(struct timeval& tme, uint32_t rssiTime) {
     float units = std::pow(10, -TimeoutUnit);
@@ -790,6 +835,19 @@ struct timeval& rpr::Controller::stateClosedWait() {
             curMaxCumAck_  = head->maxCumulativeAck;
             lastAckRx_     = head->acknowledge;
 
+            // Clamp peer-advertised parameters to the locally legal range before
+            // they are converted to timers or used as window/buffer bounds. A
+            // conformant peer (including SLAC firmware) is unaffected.
+            if (clampPeerParams(curMaxBuffers_,
+                                curMaxSegment_,
+                                curCumAckTout_,
+                                curRetranTout_,
+                                curNullTout_,
+                                locMaxBuffers_,
+                                locMaxSegment_)) {
+                log_->warning("Clamped out-of-range peer RSSI parameters. server=%d", server_);
+            }
+
             // Convert times
             convTime(retranToutD1_, curRetranTout_);
             convTime(cumAckToutD1_, curCumAckTout_);

tests/cpp/protocols/CMakeLists.txt

@@ -1,4 +1,5 @@
 add_subdirectory(packetizer)
+add_subdirectory(rssi)
 
 if (NOT NO_PYTHON)
    add_subdirectory(xilinx)

tests/cpp/protocols/rssi/CMakeLists.txt

@@ -0,0 +1,7 @@
+rogue_add_cpp_test(rogue-cpp-protocols-rssi-param-clamp
+   SOURCES
+      test_param_clamp.cpp
+   LABELS
+      cpp-core
+      no-python
+)

tests/cpp/protocols/rssi/test_param_clamp.cpp

@@ -0,0 +1,178 @@
+/**
+ * ----------------------------------------------------------------------------
+ * Company    : SLAC National Accelerator Laboratory
+ * ----------------------------------------------------------------------------
+ * Description:
+ * Regression: rpr::Controller::clampPeerParams() clamps peer-advertised RSSI
+ * connection parameters to the locally legal range. The SYN/SYN-ACK handshake
+ * adopts the peer's advertised parameters verbatim; an out-of-range peer value
+ * (zero outstanding segments, zero timeouts, a sub-header-size segment, or a
+ * window larger than the locally offered maximum) would otherwise drive the
+ * controller into illegal state (for example curMaxBuffers == 0 makes the
+ * outbound wait loop spin forever). Conformant peers, including SLAC firmware,
+ * must be left untouched.
+ * ----------------------------------------------------------------------------
+ * This file is part of the rogue software platform. It is subject to
+ * the license terms in the LICENSE.txt file found in the top-level directory
+ * of this distribution and at:
+ *    https://confluence.slac.stanford.edu/display/ppareg/LICENSE.html.
+ * No part of the rogue software platform, including this file, may be
+ * copied, modified, propagated, or distributed except according to the terms
+ * contained in the LICENSE.txt file.
+ * ----------------------------------------------------------------------------
+ **/
+
+#include <cstdint>
+
+#include "doctest/doctest.h"
+#include "rogue/protocols/rssi/Controller.h"
+#include "rogue/protocols/rssi/Header.h"
+
+namespace rpr = rogue::protocols::rssi;
+
+// Local bounds used by the tests (rogue defaults: 32 buffers, 1400 byte segment)
+static const uint8_t kLocMaxBuffers  = 32;
+static const uint16_t kLocMaxSegment = 1400;
+
+TEST_CASE("clampPeerParams leaves a conformant peer untouched") {
+    uint8_t maxBuffers  = 8;
+    uint16_t maxSegment = 512;
+    uint16_t cumAckTout = 5;
+    uint16_t retranTout = 20;
+    uint16_t nullTout   = 1000;
+
+    bool clamped = rpr::Controller::clampPeerParams(maxBuffers,
+                                                    maxSegment,
+                                                    cumAckTout,
+                                                    retranTout,
+                                                    nullTout,
+                                                    kLocMaxBuffers,
+                                                    kLocMaxSegment);
+
+    CHECK_FALSE(clamped);
+    CHECK_EQ(maxBuffers, static_cast<uint8_t>(8));
+    CHECK_EQ(maxSegment, static_cast<uint16_t>(512));
+    CHECK_EQ(cumAckTout, static_cast<uint16_t>(5));
+    CHECK_EQ(retranTout, static_cast<uint16_t>(20));
+    CHECK_EQ(nullTout, static_cast<uint16_t>(1000));
+}
+
+TEST_CASE("clampPeerParams raises a zero outstanding-segment count to 1") {
+    uint8_t maxBuffers  = 0;
+    uint16_t maxSegment = 512;
+    uint16_t cumAckTout = 5;
+    uint16_t retranTout = 20;
+    uint16_t nullTout   = 1000;
+
+    bool clamped = rpr::Controller::clampPeerParams(maxBuffers,
+                                                    maxSegment,
+                                                    cumAckTout,
+                                                    retranTout,
+                                                    nullTout,
+                                                    kLocMaxBuffers,
+                                                    kLocMaxSegment);
+
+    CHECK(clamped);
+    CHECK_EQ(maxBuffers, static_cast<uint8_t>(1));
+}
+
+TEST_CASE("clampPeerParams caps an oversized window at the local maximum") {
+    uint8_t maxBuffers  = 200;
+    uint16_t maxSegment = 512;
+    uint16_t cumAckTout = 5;
+    uint16_t retranTout = 20;
+    uint16_t nullTout   = 1000;
+
+    bool clamped = rpr::Controller::clampPeerParams(maxBuffers,
+                                                    maxSegment,
+                                                    cumAckTout,
+                                                    retranTout,
+                                                    nullTout,
+                                                    kLocMaxBuffers,
+                                                    kLocMaxSegment);
+
+    CHECK(clamped);
+    CHECK_EQ(maxBuffers, kLocMaxBuffers);
+}
+
+TEST_CASE("clampPeerParams raises a sub-header segment size to the header size") {
+    uint8_t maxBuffers  = 8;
+    uint16_t maxSegment = 4;  // smaller than the 8-byte header
+    uint16_t cumAckTout = 5;
+    uint16_t retranTout = 20;
+    uint16_t nullTout   = 1000;
+
+    bool clamped = rpr::Controller::clampPeerParams(maxBuffers,
+                                                    maxSegment,
+                                                    cumAckTout,
+                                                    retranTout,
+                                                    nullTout,
+                                                    kLocMaxBuffers,
+                                                    kLocMaxSegment);
+
+    CHECK(clamped);
+    CHECK_EQ(maxSegment, static_cast<uint16_t>(rpr::Header::HeaderSize));
+}
+
+TEST_CASE("clampPeerParams caps an oversized segment at the local maximum") {
+    uint8_t maxBuffers  = 8;
+    uint16_t maxSegment = 9000;  // larger than locally offered
+    uint16_t cumAckTout = 5;
+    uint16_t retranTout = 20;
+    uint16_t nullTout   = 1000;
+
+    bool clamped = rpr::Controller::clampPeerParams(maxBuffers,
+                                                    maxSegment,
+                                                    cumAckTout,
+                                                    retranTout,
+                                                    nullTout,
+                                                    kLocMaxBuffers,
+                                                    kLocMaxSegment);
+
+    CHECK(clamped);
+    CHECK_EQ(maxSegment, kLocMaxSegment);
+}
+
+TEST_CASE("clampPeerParams raises zero timeouts to 1") {
+    uint8_t maxBuffers  = 8;
+    uint16_t maxSegment = 512;
+    uint16_t cumAckTout = 0;
+    uint16_t retranTout = 0;
+    uint16_t nullTout   = 0;
+
+    bool clamped = rpr::Controller::clampPeerParams(maxBuffers,
+                                                    maxSegment,
+                                                    cumAckTout,
+                                                    retranTout,
+                                                    nullTout,
+                                                    kLocMaxBuffers,
+                                                    kLocMaxSegment);
+
+    CHECK(clamped);
+    CHECK_EQ(cumAckTout, static_cast<uint16_t>(1));
+    CHECK_EQ(retranTout, static_cast<uint16_t>(1));
+    CHECK_EQ(nullTout, static_cast<uint16_t>(1));
+}
+
+TEST_CASE("clampPeerParams clamps multiple out-of-range fields at once") {
+    uint8_t maxBuffers  = 0;
+    uint16_t maxSegment = 0;
+    uint16_t cumAckTout = 0;
+    uint16_t retranTout = 0;
+    uint16_t nullTout   = 0;
+
+    bool clamped = rpr::Controller::clampPeerParams(maxBuffers,
+                                                    maxSegment,
+                                                    cumAckTout,
+                                                    retranTout,
+                                                    nullTout,
+                                                    kLocMaxBuffers,
+                                                    kLocMaxSegment);
+
+    CHECK(clamped);
+    CHECK_EQ(maxBuffers, static_cast<uint8_t>(1));
+    CHECK_EQ(maxSegment, static_cast<uint16_t>(rpr::Header::HeaderSize));
+    CHECK_EQ(cumAckTout, static_cast<uint16_t>(1));
+    CHECK_EQ(retranTout, static_cast<uint16_t>(1));
+    CHECK_EQ(nullTout, static_cast<uint16_t>(1));
+}