Skip to content

Commit 6967ad3

Browse files
gbn: change implication of processNACK signature
The processNACK function returns two booleans as its signature. The first boolean used to only indicate if the NACK was part of the queue, but is with this commit expanded to indicate if the queue needs to be resent or not as a result of the NACK. The second boolean indicates if the queue sequence base was bumped or not. Previously, the bumped value for processNACK didn't accurately reflect if the queue sequence base was actually bumped or not for the cases when we didn't trigger a resend. This commit addresses that issue. Updating the bumped value to be correct has no real impact on the current implementation of the gbn conn, but it might start having an effect if the implementation is modified in the future.
1 parent c0eea25 commit 6967ad3

File tree

2 files changed

+16
-9
lines changed

2 files changed

+16
-9
lines changed

gbn/gbn_conn.go

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -582,15 +582,12 @@ func (g *GoBackNConn) receivePacketsForever() error { // nolint:gocyclo
582582
// sent was dropped, or maybe we sent a duplicate
583583
// message. The NACK message contains the sequence
584584
// number that the receiver was expecting.
585-
inQueue, bumped := g.sendQueue.processNACK(m.Seq)
586-
587-
// If the NACK sequence number is not in our queue
588-
// then we ignore it. We must have received the ACK
589-
// for the sequence number in the meantime.
590-
if !inQueue {
591-
g.log.Tracef("NACK seq %d is not in the "+
592-
"queue. Ignoring", m.Seq)
585+
shouldResend, bumped := g.sendQueue.processNACK(m.Seq)
593586

587+
// If we don't need to resend the queue after processing
588+
// the NACK, we can continue without sending the resend
589+
// signal.
590+
if !shouldResend {
594591
continue
595592
}
596593

gbn/queue.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,12 @@ func (q *queue) processACK(seq uint8) bool {
188188
}
189189

190190
// processNACK processes an incoming NACK of a given sequence number.
191+
// The function returns two booleans. The first boolean is set to true if we
192+
// should resend the queue after processing the NACK. The second boolean is set
193+
// to true if the NACK sequence number is a packet in the queue which isn't the
194+
// queue base, and we therefore don't need to resend any of the packets before
195+
// the NACK sequence number. This equivalent to receiving the ACKs for the
196+
// packets before the NACK sequence number.
191197
func (q *queue) processNACK(seq uint8) (bool, bool) {
192198
q.baseMtx.Lock()
193199
defer q.baseMtx.Unlock()
@@ -203,11 +209,15 @@ func (q *queue) processNACK(seq uint8) (bool, bool) {
203209
// trigger a resend.
204210
if seq == q.sequenceTop {
205211
q.sequenceBase = q.sequenceTop
206-
return true, false
212+
213+
return false, true
207214
}
208215

209216
// Is the NACKed sequence even in our queue?
210217
if !containsSequence(q.sequenceBase, q.sequenceTop, seq) {
218+
q.cfg.log.Tracef("NACK seq %d is not in the queue. Ignoring.",
219+
seq)
220+
211221
return false, false
212222
}
213223

0 commit comments

Comments
 (0)