[core] Refax-only (with temporary differences) for 2677 (#2798).

Socketoptions: exported the check for fitting payload size setting to a separate function. CSndBuffer and CRateEstimator get an extra parameter for IP family to be used later. In core.h, removed unused function and added payloadSize() that should be used to return payload size after connection.
Haivision · Sep 19, 2023 · 218c7fd · 218c7fd
1 parent 39a800c
commit 218c7fd
Show file tree

Hide file tree

Showing 9 changed files with 78 additions and 42 deletions.
diff --git a/srtcore/buffer_snd.cpp b/srtcore/buffer_snd.cpp
@@ -64,7 +64,7 @@ using namespace std;
 using namespace srt_logging;
 using namespace sync;
 
-CSndBuffer::CSndBuffer(int size, int maxpld, int authtag)
+CSndBuffer::CSndBuffer(int ip_family, int size, int maxpld, int authtag)
     : m_BufLock()
     , m_pBlock(NULL)
     , m_pFirstBlock(NULL)
@@ -77,6 +77,7 @@ CSndBuffer::CSndBuffer(int size, int maxpld, int authtag)
     , m_iAuthTagSize(authtag)
     , m_iCount(0)
     , m_iBytesCount(0)
+    , m_rateEstimator(ip_family)
 {
     // initial physical buffer of "size"
     m_pBuffer           = new Buffer;

diff --git a/srtcore/buffer_snd.h b/srtcore/buffer_snd.h
@@ -87,7 +87,7 @@ class CSndBuffer
     /// @param size initial number of blocks (each block to store one packet payload).
     /// @param maxpld maximum packet payload (including auth tag).
     /// @param authtag auth tag length in bytes (16 for GCM, 0 otherwise).
-    CSndBuffer(int size = 32, int maxpld = 1500, int authtag = 0);
+    CSndBuffer(int ip_family, int size, int maxpld, int authtag);
     ~CSndBuffer();
 
 public:

diff --git a/srtcore/buffer_tools.cpp b/srtcore/buffer_tools.cpp
@@ -102,11 +102,12 @@ void AvgBufSize::update(const steady_clock::time_point& now, int pkts, int bytes
     m_dTimespanMAvg   = avg_iir_w<1000, double>(m_dTimespanMAvg, timespan_ms, elapsed_ms);
 }
 
-CRateEstimator::CRateEstimator()
+CRateEstimator::CRateEstimator(int /*family*/)
     : m_iInRatePktsCount(0)
     , m_iInRateBytesCount(0)
     , m_InRatePeriod(INPUTRATE_FAST_START_US) // 0.5 sec (fast start)
     , m_iInRateBps(INPUTRATE_INITIAL_BYTESPS)
+    , m_iFullHeaderSize(CPacket::UDP_HDR_SIZE + CPacket::HDR_SIZE)
 {}
 
 void CRateEstimator::setInputRateSmpPeriod(int period)
@@ -142,7 +143,7 @@ void CRateEstimator::updateInputRate(const time_point& time, int pkts, int bytes
         return;
 
     // Required Byte/sec rate (payload + headers)
-    m_iInRateBytesCount += (m_iInRatePktsCount * CPacket::SRT_DATA_HDR_SIZE);
+    m_iInRateBytesCount += (m_iInRatePktsCount * m_iFullHeaderSize);
     m_iInRateBps = (int)(((int64_t)m_iInRateBytesCount * 1000000) / period_us);
     HLOGC(bslog.Debug,
         log << "updateInputRate: pkts:" << m_iInRateBytesCount << " bytes:" << m_iInRatePktsCount

diff --git a/srtcore/buffer_tools.h b/srtcore/buffer_tools.h
@@ -94,7 +94,7 @@ class CRateEstimator
     typedef sync::steady_clock::time_point time_point;
     typedef sync::steady_clock::duration   duration;
 public:
-    CRateEstimator();
+    CRateEstimator(int family);
 
 public:
     uint64_t getInRatePeriod() const { return m_InRatePeriod; }
@@ -124,6 +124,7 @@ class CRateEstimator
     time_point m_tsInRateStartTime;
     uint64_t   m_InRatePeriod; // usec
     int        m_iInRateBps;   // Input Rate in Bytes/sec
+    int        m_iFullHeaderSize;
 };
 
 

diff --git a/srtcore/core.cpp b/srtcore/core.cpp
@@ -5621,7 +5621,7 @@ bool srt::CUDT::prepareBuffers(CUDTException* eout)
     {
         // CryptoControl has to be initialized and in case of RESPONDER the KM REQ must be processed (interpretSrtHandshake(..)) for the crypto mode to be deduced.
         const int authtag = (m_pCryptoControl && m_pCryptoControl->getCryptoMode() == CSrtConfig::CIPHER_MODE_AES_GCM) ? HAICRYPT_AUTHTAG_MAX : 0;
-        m_pSndBuffer = new CSndBuffer(32, m_iMaxSRTPayloadSize, authtag);
+        m_pSndBuffer = new CSndBuffer(AF_INET, 32, m_iMaxSRTPayloadSize, authtag);
         SRT_ASSERT(m_iPeerISN != -1);
         m_pRcvBuffer = new srt::CRcvBuffer(m_iPeerISN, m_config.iRcvBufSize, m_pRcvQueue->m_pUnitQueue, m_config.bMessageAPI);
         // After introducing lite ACK, the sndlosslist may not be cleared in time, so it requires twice a space.

diff --git a/srtcore/core.h b/srtcore/core.h
@@ -328,6 +328,20 @@ class CUDT
     int             peerIdleTimeout_ms()    const { return m_config.iPeerIdleTimeout_ms; }
     size_t          maxPayloadSize()        const { return m_iMaxSRTPayloadSize; }
     size_t          OPT_PayloadSize()       const { return m_config.zExpPayloadSize; }
+    size_t          payloadSize()           const
+    {
+        // If payloadsize is set, it should already be checked that
+        // it is less than the possible maximum payload size. So return it
+        // if it is set to nonzero value. In case when the connection isn't
+        // yet established, return also 0, if the value wasn't set.
+        if (m_config.zExpPayloadSize || !m_bConnected)
+            return m_config.zExpPayloadSize;
+
+        // If SRTO_PAYLOADSIZE was remaining with 0 (default for FILE mode)
+        // then return the maximum payload size per packet.
+        return m_iMaxSRTPayloadSize;
+    }
+
     int             sndLossLength()               { return m_pSndLossList->getLossLength(); }
     int32_t         ISN()                   const { return m_iISN; }
     int32_t         peerISN()               const { return m_iPeerISN; }
@@ -740,13 +754,6 @@ class CUDT
     static loss_seqs_t defaultPacketArrival(void* vself, CPacket& pkt);
     static loss_seqs_t groupPacketArrival(void* vself, CPacket& pkt);
 
-    CRateEstimator getRateEstimator() const
-    {
-        if (!m_pSndBuffer)
-            return CRateEstimator();
-        return m_pSndBuffer->getRateEstimator();
-    }
-
     void setRateEstimator(const CRateEstimator& rate)
     {
         if (!m_pSndBuffer)

diff --git a/srtcore/group_backup.h b/srtcore/group_backup.h
@@ -79,6 +79,14 @@ namespace groups
             : m_stateCounter() // default init with zeros
             , m_activeMaxWeight()
             , m_standbyMaxWeight()
+            // XXX Setting AF_INET6 is a temporary solution for using rate estimator
+            // that counts a rate based on the current link's IP version. The results
+            // for links using IPv4 could be slightly falsified due to that (16 bytes
+            // more per a packet), but this makes the estimation results the same for
+            // the same data sent over the group, regardless of the IP version used
+            // for the currently active link (which in reality results in different
+            // load for the same stream, if links use different IP version).
+            , m_rateEstimate(AF_INET6)
         {
         }
 

diff --git a/srtcore/socketconfig.cpp b/srtcore/socketconfig.cpp
@@ -641,36 +641,10 @@ struct CSrtConfigSetter<SRTO_PAYLOADSIZE>
             throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
         }
 
-        if (!co.sPacketFilterConfig.empty())
+        std::string errorlog;
+        if (!co.payloadSizeFits(size_t(val), AF_INET, (errorlog)))
         {
-            // This means that the filter might have been installed before,
-            // and the fix to the maximum payload size was already applied.
-            // This needs to be checked now.
-            SrtFilterConfig fc;
-            if (!ParseFilterConfig(co.sPacketFilterConfig.str(), fc))
-            {
-                // Break silently. This should not happen
-                LOGC(aclog.Error, log << "SRTO_PAYLOADSIZE: IPE: failing filter configuration installed");
-                throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
-            }
-
-            const size_t efc_max_payload_size = SRT_LIVE_MAX_PLSIZE - fc.extra_size;
-            if (size_t(val) > efc_max_payload_size)
-            {
-                LOGC(aclog.Error,
-                     log << "SRTO_PAYLOADSIZE: value exceeds " << SRT_LIVE_MAX_PLSIZE << " bytes decreased by " << fc.extra_size
-                         << " required for packet filter header");
-                throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
-            }
-        }
-
-        // Not checking AUTO to allow defaul 1456 bytes.
-        if ((co.iCryptoMode == CSrtConfig::CIPHER_MODE_AES_GCM)
-            && (val > (SRT_LIVE_MAX_PLSIZE - HAICRYPT_AUTHTAG_MAX)))
-        {
-            LOGC(aclog.Error,
-                log << "SRTO_PAYLOADSIZE: value exceeds " << SRT_LIVE_MAX_PLSIZE << " bytes decreased by " << HAICRYPT_AUTHTAG_MAX
-                << " required for AES-GCM.");
+            LOGP(aclog.Error, errorlog);
             throw CUDTException(MJ_NOTSUP, MN_INVAL, 0);
         }
 
@@ -1040,6 +1014,47 @@ int CSrtConfig::set(SRT_SOCKOPT optName, const void* optval, int optlen)
     return dispatchSet(optName, *this, optval, optlen);
 }
 
+bool CSrtConfig::payloadSizeFits(size_t val, int /*ip_family*/, std::string& w_errmsg) ATR_NOTHROW
+{
+    if (!this->sPacketFilterConfig.empty())
+    {
+        // This means that the filter might have been installed before,
+        // and the fix to the maximum payload size was already applied.
+        // This needs to be checked now.
+        SrtFilterConfig fc;
+        if (!ParseFilterConfig(this->sPacketFilterConfig.str(), fc))
+        {
+            // Break silently. This should not happen
+            w_errmsg = "SRTO_PAYLOADSIZE: IPE: failing filter configuration installed";
+            return false;
+        }
+
+        const size_t efc_max_payload_size = SRT_LIVE_MAX_PLSIZE - fc.extra_size;
+        if (size_t(val) > efc_max_payload_size)
+        {
+            std::ostringstream log;
+            log << "SRTO_PAYLOADSIZE: value exceeds " << SRT_LIVE_MAX_PLSIZE << " bytes decreased by " << fc.extra_size
+                << " required for packet filter header";
+            w_errmsg = log.str();
+            return false;
+        }
+    }
+
+    // Not checking AUTO to allow defaul 1456 bytes.
+    if ((this->iCryptoMode == CSrtConfig::CIPHER_MODE_AES_GCM)
+            && (val > (SRT_LIVE_MAX_PLSIZE - HAICRYPT_AUTHTAG_MAX)))
+    {
+        std::ostringstream log;
+        log << "SRTO_PAYLOADSIZE: value exceeds " << SRT_LIVE_MAX_PLSIZE
+            << " bytes decreased by " << HAICRYPT_AUTHTAG_MAX
+            << " required for AES-GCM.";
+        w_errmsg = log.str();
+        return false;
+    }
+
+    return true;
+}
+
 #if ENABLE_BONDING
 bool SRT_SocketOptionObject::add(SRT_SOCKOPT optname, const void* optval, size_t optlen)
 {

diff --git a/srtcore/socketconfig.h b/srtcore/socketconfig.h
@@ -342,6 +342,9 @@ struct CSrtConfig: CSrtMuxerConfig
     }
 
     int set(SRT_SOCKOPT optName, const void* val, int size);
+
+    bool payloadSizeFits(size_t val, int ip_family, std::string& w_errmsg) ATR_NOTHROW;
+
 };
 
 template <typename T>