Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RCV erase undecrypted packet instead of dropping. #2649

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions srtcore/buffer_rcv.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,47 @@ int CRcvBuffer::insert(CUnit* unit)
return 0;
}

int CRcvBuffer::erase(int32_t seqno)
{
const int end_pos = incPos(m_iStartPos, m_iMaxPosOff);
// Drop by packet seqno range.
const int offset = CSeqNo::seqoff(m_iStartSeqNo, seqno);
if (offset < 0)
{
LOGC(rbuflog.Debug, log << "CRcvBuffer.erase(): nothing to erase. Requested @" << seqno
<< ". Buffer start " << m_iStartSeqNo << ".");
return 0;
}

const int pos = incPos(m_iStartPos, offset);
if (!m_entries[pos].pUnit)
return 0;

const bool bMsgOrderFlag = packetAt(pos).getMsgOrderFlag();
releaseUnitInPos(pos);

if (m_bMessageAPI && !bMsgOrderFlag && !m_tsbpd.isEnabled())
{
--m_numOutOfOrderPackets;
if (pos == m_iFirstReadableOutOfOrder) {
m_iFirstReadableOutOfOrder = -1;
updateFirstReadableOutOfOrder();
}
}

HLOGC(rbuflog.Debug, log << "CRcvBuffer.erase(): @" << seqno << ".");

// Check if a unit before m_iFirstNonreadPos was erased.
const bool needUpdateNonreadPos = offset <= getRcvDataSize();
if (needUpdateNonreadPos)
{
m_iFirstNonreadPos = m_iStartPos;
updateNonreadPos();
}

return 1;
}

int CRcvBuffer::dropUpTo(int32_t seqno)
{
IF_RCVBUF_DEBUG(ScopedLog scoped_log);
Expand Down
6 changes: 6 additions & 0 deletions srtcore/buffer_rcv.h
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ class CRcvBuffer
// TODO: Previously '-2' also meant 'already acknowledged'. Check usage of this value.
int insert(CUnit* unit);

/// Erase a packet from the buffer based on the packet sequence number.
/// The entry is marked EntryState_Empty. The CUnit is marked free.
/// @param seqno packet sequence number.
/// @return the number of packets erased.
int erase(int32_t seqno);

/// Drop packets in the receiver buffer from the current position up to the seqno (excluding seqno).
/// @param [in] seqno drop units up to this sequence number
/// @return number of dropped packets.
Expand Down
17 changes: 8 additions & 9 deletions srtcore/core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9850,7 +9850,7 @@ int srt::CUDT::handleSocketPacketReception(const vector<CUnit*>& incoming, bool&
}
}

int buffer_add_result = m_pRcvBuffer->insert(u);
const int buffer_add_result = m_pRcvBuffer->insert(u);
if (buffer_add_result < 0)
{
// The insert() result is -1 if at the position evaluated from this packet's
Expand Down Expand Up @@ -9878,17 +9878,16 @@ int srt::CUDT::handleSocketPacketReception(const vector<CUnit*>& incoming, bool&
adding_successful = false;
IF_HEAVY_LOGGING(exc_type = "UNDECRYPTED");

// Drop a packet from the receiver buffer.
// Dropping depends on the configuration mode. If message mode is enabled, we have to drop the whole message.
// Otherwise just drop the exact packet.
const int iDropCnt = (m_config.bMessageAPI)
? m_pRcvBuffer->dropMessage(SRT_SEQNO_NONE, SRT_SEQNO_NONE, u->m_Packet.getMsgSeq(m_bPeerRexmitFlag))
: m_pRcvBuffer->dropMessage(u->m_Packet.getSeqNo(), u->m_Packet.getSeqNo(), SRT_MSGNO_NONE);
// Erase the packet from the receiver buffer.
// In message mode the whole message could be dropped.
// However, when decryption fails the message number in the packet cannot be trusted.
// The packet was added to the buffer based on the sequence number, therefore sequence number should be used to erase it from the buffer.
// TODO: Erasing the packet means it has to be added back to the loss list.
const int iEraseCnt = m_pRcvBuffer->erase(u->m_Packet.getSeqNo());

LOGC(qrlog.Warn, log << CONID() << "Decryption failed. Seqno %" << u->m_Packet.getSeqNo()
<< ", msgno " << u->m_Packet.getMsgSeq(m_bPeerRexmitFlag) << ". Dropping " << iDropCnt << ".");
<< ", msgno " << u->m_Packet.getMsgSeq(m_bPeerRexmitFlag) << ".");
ScopedLock lg(m_StatsLock);
m_stats.rcvr.dropped.count(stats::BytesPackets(iDropCnt * rpkt.getLength(), iDropCnt));
m_stats.rcvr.undecrypted.count(stats::BytesPackets(rpkt.getLength(), 1));
}
}
Expand Down
2 changes: 1 addition & 1 deletion srtcore/srt.h
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ enum SRT_KM_STATE
SRT_KM_S_NOSECRET = 3, // Stream encrypted and no secret to decrypt Keying Material
SRT_KM_S_BADSECRET = 4 // Stream encrypted and wrong secret is used, cannot decrypt Keying Material
#ifdef ENABLE_AEAD_API_PREVIEW
,SRT_KM_S_BADCRYPTOMODE = 5 // Stream encrypted but wrong ccryptographic mode is used, cannot decrypt. Since v1.6.0.
,SRT_KM_S_BADCRYPTOMODE = 5 // Stream encrypted but wrong cryptographic mode is used, cannot decrypt. Since v1.6.0.
#endif
};

Expand Down