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

[Statistics] Perf stats loss packet calculation in case of packet reordering #899

Open
maxsharabayko opened this issue Oct 8, 2019 · 3 comments · May be fixed by #1219
Open

[Statistics] Perf stats loss packet calculation in case of packet reordering #899

maxsharabayko opened this issue Oct 8, 2019 · 3 comments · May be fixed by #1219
Labels
[core] Area: Changes in SRT library core Priority: High Type: Bug Indicates an unexpected problem or unintended behavior
Milestone

Comments

@maxsharabayko
Copy link
Collaborator

If a packet arrives out of order, then regardless of the currently used reorder tolerance, all the packets in the gap of sequence numbes are considered as lost packets in performance statistics.
Refer to SRT_TRACEBSTATS::pktRcvLoss.

@maxsharabayko maxsharabayko added Priority: Low Type: Bug Indicates an unexpected problem or unintended behavior [core] Area: Changes in SRT library core labels Oct 8, 2019
@maxsharabayko maxsharabayko added this to the v1.4.1 milestone Oct 8, 2019
@maxsharabayko maxsharabayko modified the milestones: v1.4.1, v1.4.2 Nov 11, 2019
@maxsharabayko maxsharabayko self-assigned this Dec 27, 2019
@ethouris ethouris assigned ethouris and unassigned maxsharabayko Mar 3, 2020
@ethouris
Copy link
Collaborator

The problem: Currently loss statistics are shifted to the very beginning and happens before any other processing. So it simply notifies all losses seen by a jump-over.

This could be changed, however the reason as to why packet can be counted as lost got complicated by both FEC and reordering recognition. The packet should be counted as lost when FEC-recovered, even though it's no longer seen as lost after exitting the filter, but then a packet that (also this one) that was notified as early change in case of reorder recognition working, should not be added to loss statistics, and instead those that were submitted to loss report out of the fresh loss container should.

The simplest way for me would be: notify all packets, for which the LOSSREPORT was sent. But then this would have to deal with those packets that were lost, but no lossreport was sent, which are twofold:

  • FEC rebuilt
  • FEC irrecoverable in no loss mode

An additional problem to this is also that a packet that was FEC-recovered because it was the last packet in the group followed by a FEC-control packet will be recovered even if it actually wasn't lost because it was recovered. Hard to say if this should be counted as lost. The problem here is that this packet won't even be notified in the fresh loss, and therefore after a tolerance-number of packets passed, the matter will be already settled. This could be solved by having an extra flag in the fresh loss records that declares whether the counted here loss is recorded for later recovery, or for just statistical notification. Not sure if it's worth a shot. If not, I'd state such a packet should be simply qualified as not lost (as-if reordered).

@mbakholdina
Copy link
Collaborator

If the packet is received out of order, there can not be sequence discontinuity by default. I suggest not to fix this until we sync up on this and decide whether reorder tolerance should affect pktRcvLossTotal and pktRcvLoss statistics.

@mbakholdina mbakholdina changed the title Perf stats loss packet calculation in case of packet reordering [Statistics] Perf stats loss packet calculation in case of packet reordering Mar 25, 2020
@ethouris
Copy link
Collaborator

If the packet is received out of order, there can not be sequence discontinuity by default.

The packet processing unit can only see a jump-over. It doesn't know yet if this is because of the lost packet or reordered packet. This will be known (or suspected) later, when processing another packet among those that will arrive in future - but at the moment when the reordered packet comes in it will be a completely different processing session and that above situation will be already long forgotten.

Unless we have some special container to remember those packets for the sake of statistics. Sequence numbers from this container will be dismissed the following way:

  • After receiving a late packet with R=0, matching sequence from here will be removed (can be added to reordered packet count, if we want this stat)
  • When ACK-ing packets, all remaining sequences up to the ACK number will be notified as lost packets (and this would be the only place where lost packets are counted)

A potentially dangerous situation might be that it's a container that might potentially grow in size with only a conditional control. Possibly extra check needs to be added that limits its size from upside.

Important thing is also that with this change the loss stats will not be calculated at the moment when a jump-over is seen, but later at the moment of ACK-ing packets. This might influence the numbers seen in comparison to previous versions. Although the statistics should be the same exact in the long term.

I suggest not to fix this until we sync up on this and decide whether reorder tolerance should affect pktRcvLossTotal and pktRcvLoss statistics.

Right, of course. Although my recommendation would be that packets counted as "lost" should be all those that have never arrived - no matter what exactly action has been undertaken at the processing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[core] Area: Changes in SRT library core Priority: High Type: Bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants