search

kvetak / RINA

RINA Simulator
https://rinasim.omnetpp.org/
Other
29 stars 11 forks source link

Possibly mistaken condition in DTP (rx side) #5

Closed vmaffione closed 9 years ago

vmaffione commented 9 years ago

Hi, branch master, function DTP::handleDataTransferPDUFromRMT(), line 664.

In my opinion the first comparison operator should be "<" and not "<=". Suppose you receive a first PDU (DRF set) with sequence 38. You set the RcvLWE to 39. When a second PDU comes (sequence 39), you enter the condition at line 664, but you shouldn't, because you should hit the following condition (in order PDU), at line 707. In other words I'm saying that with the current logic you can never enter the condition at 707.

Am I wrong?

Cheers, Vincenzo

screw commented 9 years ago

Hi, you are not completely wrong. See line 632 and pay attention to the "+1" :). I am waiting for Spec update approval, because setting RcvLWE in case of DRF is not mentioned.

In conclusion, I believe that the implementation is correct.

vmaffione commented 9 years ago

Hi, I saw line 632, that's why in my example I wrote "You set the RcvLWE to 39", where 39=38+1, since 38 is the seqnum of the first PDU received (DRF set).

Referring to my example, when the second PDU comes in order (whose sequence number is 39), assuming the DRF is not set, the second PDU will hit the condition at line 664, (a gap or a duplicate gap) while it should hit the condition at line 707 (in order PDU). It hits the condition at line 664 because at that point RcvLWE is 39 (set to that value the previous round), and the PDU seqnum is 39 too.

Please pay attention to my specific example and point me at what you think is mistaken :) Again, I think you need '<' there.

Thanks, Vincenzo

screw commented 9 years ago

Hi, sorry, I was looking at different condition. YES, you are right. I will change it to just "<".

To address your specific example: No, it would work just fine. The other part of the condition: && pdu->getSeqNum() <= state.getMaxSeqNumRcvd() ... && 39 <= 38 would fail, resulting in 'correct' behavior.

Thank you, for pointing it out, m&M.

vmaffione commented 9 years ago

Right, the second condition prevents the '<=' problem to show up in my specific example. Well, we were both right and wrong to some extent :)

Thanks, Vincenzo