Subject: [payload] Comments on draft-ietf-mmusic-media-loopback-16

From: Magnus Westerlund <[email-masked]>
Date: Fri, 21 Oct 2011 11:25:25 +0200

Hi,

I have reviewed An "Extension to the Session Description Protocol (SDP)
for Media Loopback" whos WG last call ended yesterday. I hope this
comments will be addressed as I think there are some serious short
commings and also process steps that must be done.

1. This document contains media types. But it has not been reviewed by
the [email-masked] mailing list. This must be completed prior to
publication request.

2. I do CC the payload WG mailing list as this document really should be
reviewed there. I know the chairs have realized that need.

3. I think the authors and chairs should discuss how to handle the fact
that there is 6 authors on the front page. It is possible to ask for
exception on this, but might be simpler to move one of the authors to a
contributor section.

4. Title: I think the title is indicating that this is only an SDP
solution, but it clearly contains the considerations needed for the
RTP/RTCP part also. I think it should reflect that it is a solution for
RTP transported media loopback including SDP signalling.

5. Abstract: "maintaining voice/real-time Text/video quality,
reliability, and overall performance." The "voice/real-time"
construction appears to indicate that voice isn't real-time. I guess
something like: maintaining real-time voice/Text/video quality,
reliability, and overall performance. would be easier and clearer.

6. Section 3.
No reference to SDP offer answer. And the "Offering entity" is called
agent in SDP O/A terminology.

7. Section 3:
"it MAY do so as defined in section 5.4.1 of
    this specification."

Considering what is written in 5.4.1 I think that section can be moved
to section 3.

8. Section 4:
    An answering entity that is not compliant to this specification and
    which receives an offer with the "loopback" media attributes MAY
    ignore the attribute and treat the incoming offer as a normal
    request.

I would suggest reformulate this. I expect this text to say what is
expected to happen when the answering agent doesn't know what this
attribute is. Which is ignore the attribute, attempt to establish the
session without loopback and send back the answer without the attribute.

I would suggest to clarify that the offering agent should consider
terminating the establishment at this point as loopback was not
available, unless it might actual "ring" if this is an end-point at a user.

9. What about declarative SDP usage?

10. What is really the purpose of the sections 3 and 4. They seem to be
teasers for a very small part of the issue that is covered in detail in
section 5. Why not move the appropriate text in to the relevant subsections?

11. Section 5.3
"    Note: A loopback offer in a given media description MUST NOT
    contain the standard mode attributes sendonly, recvonly, sendrecv,
    or inactive. The loopback-mode attributes (loopback-source and
    loopback-mirror) replace the standard attributes."

This appears to be a strange MUST NOT which I think might encounter
issues. Considering that SIP proxies that looks into this SDP requires
the direction attribute to be present and adds it, what happens now.
And why isn't sendrecv and inactive possible values? sendrecv appears to
be requried for loopback to work, and inactive would put media transport
in an established session on hold. Isn't that reasonable?

12. Section 5.3:
I think one could clarify that the loopback source needs to include the
encapsulation or direct loopback payload formats when suggesting pckt
type loopback.

13. What happens if I  offer both media and pckt loopback at the same
time by including two loopback attributes?

14. Section 5.1:
    loopback-type          = loopback-choice [1*SP loopback-choice]
    loopback-choice        = loopback-type-pkt / loopback-type-media
    loopback-type-pkt      = "rtp-pkt-loopback"
    loopback-type-media    = "rtp-media-loopback"

The ABNF appears broken as it doesn't define the whole attribute. I
would suggest to add a line:

loopback-attr = "a=loopback:" loopback-type

15. Section 5.2 is missing ABNF.
I think it should make clear the attributes definition completely.

16. Section 5
Two new media attributes are defined: one indicates the type of
    loopback and the other indicates the mode of the loopback.

I would claim that this spec defines 3 attributes.

17. Section 5.4:

The loopback-mode attributes (loopbackThe
    port number and the address in the answer (m= line) indicate where
    the answerer would like to receive the media stream.

Strange sentence!

18. Section 5.4:

If an answerer wishes to accept the loopback request it MUST
    include both the loopback mode and loopback type attribute in the
    answer. When a stream is offered with the loopback-source
    attribute, the corresponding stream in the response MUST be
    loopback-mirror and vice versa, provided that answerer is capable
    of supporting the requested loopback-type.

This seems to be in partial conflict with 5.4.1. As my impression of the
above, is that if you don't want to accept, you don't include the
attributes. But 5.4.1 says that it should be included but the port set
to 0.

19. Section 5.4
I am missing clear rules for the cases when I might exclude payload
types from loopback-mode attributes compared to the m= lines. Are this
only the loopback payload types? What happens if I exclude a regular
media payload format?

20. Section 7.1: The packet expansion of the encapsulated format I think
needs better discussion. I think one needs to first recommend PMTU
discovery. Secondly I think it is likely worth discussing when the
loopback source should reduce the packet sizes to avoid fragmentation
versus the need to measure that size in the forward link and thus
causing fragmentation.

21. Section 6.
"An answering entity that is compliant to this specification and
    accepting a media with the loopback type rtp-media-loopback MUST
    transmit all received media back to the sender. "

I think this MUST, MUST have an exception for that the path from the
mirror to the source actually can support the bit-rate required.

22. Section 6.
Media loopback, if a loopback source would send more than one stream how
does the loopback mirror indicate which source is which in its return
stream? SDES CNAME could be copied but would be confusing for anyone
monitoring this stream as SSRCs from both end-points would appear to
have the same CNAME. The ssrc-grouping could possibly be used, but I
think one needs to consider the effect of that the SSRCs to be grouped
are not on the same end-point. I don't even know if my  SRCNAME proposal
could deal with this case well.

23. Section 7.1.1:
    Marker (M) bit: If the received RTP packet is looped back in
    multiple RTP packets, the M bit is set to 1 in the last packet,
    otherwise it is set to 0.

I don't like this definition as it means that complete non fragment
packets will have a m=0 the same as non last fragements. would it not be
better to define it so that all packets have m=1 except non-last fragments.

24.section 7.1.1.:
The RTP timestamp MUST use the same clock rate used by the
    loopback-source.

I think this needs to be defined as the same clock rate used by the
packet being encapsulated. Otherwise this becomes ambigous when the
loopback source have payload types with multiple rates.

25.  Section 7.1.2:

Payload strucutre. I think it is unclear. Which of the following options
do you really want.

Outer RTP header
(optional CSRC list or Header ext)
Receive timestamp
Inner RTP packet {
  - RTP header
  - (optional CSRC list or Header ext)
  - RTP payload
}

or

Outer RTP header
(optional CSRC list or Header ext)
RTP Payload header {
Receive timestamp
Inner RTP packet header fields as specified in 7.1.2
}
Inner RTP payload

Note that the second doesn't echo back the extension header

In general the text is not clear that the inner RTP payload should be
included.

26. Section 7.1.2:
You decide to overwrite the version field in the packet echoed back that
is likely fine, however, why are you overwriting the P and X bits? Why
not include the header extension in what is being echoed back so that
mirror doesn't have to remove them in case they are present.

27. Section 7.1.2: The fragmentation procedures are unclear.
I guess you want the RTP header part to be duplicated with each fragment
but that isn't clearly specified. But, will you include header
extensions also in the repeat if you include them?

28. Section 7.1.2:
    The
    Receive timestamp MUST be based on the same clock used by the
    loopback-source.

This is an impossibility. The two end-points can't have the same clock.
The can have a common clock reference like NTP. But, they are still not
the same clocks, but possibly synchronized well enough for your needs

What I guess you are after is that the clock rate should be the same as
for the packet encapsulated. The Timestamp in the outer header should
also be corrected.

I also think you should clarify that the receive timestamp and the
timestamp in the header needs to be from the same clock in the mirror.
Otherwise the time spent on the mirror for the packet is not useful. I
think it might also be worth actually writing up the measurements that
become possible by these two timestamps plus having the source logg
transmission and arrival times. The fact that one can derive both one
way delay and jitter for each loopback packet is kind of important.

However, to have correct one way delays the mirror clock needs to be
synchronized with the source clock, or at least the current offset.

29. Section 7.1.2:
"Any padding octets in the original packet MUST NOT be included in
    the loopback packet generated by a loopback-mirror."

What is the motivation behind this? If the reason I want to do packet
loopback is to find out if somebody is manipulating the packets between
the source and the mirror you need to include the whole packet as it is,
not throw away parts of it. What if padding is added or removed on path.
That might matter for the one doing the active measurement.

30. Section 7.2.1:
Sequence Number: The RTP sequence number SHOULD be generated by the
    loopback-mirror in the usual manner with a constant random offset.

What is the acceptable cases for when to break it? I assume the
intention with not saying MUST is that one can copy it from the
incomming packet. Thus when source -> mirror packet losses occur causing
wholes in the return path which isn't real loss on the mirror->source
path. It also causes third party monitor to report losses that aren't
real on this stream.

31. Section 7.2.1:
Timestamp: The RTP timestamp denotes the sampling instant for when
    the loopback-mirror is transmitting this packet to the
    loopback-source.  The RTP timestamp MUST be based on the same clock
    used by the loopback-source.  The initial value of the timestamp
    SHOULD be random for security reasons (see Section 5.1 of RFC 3550
    [RFC3550]).

One more case of not correct timestamp definition.

32. Section 7.2.1:
The usage of the CSRC and extension headers for this payload format
should be clarified. Shall the mirror copy these fields or not?

33. Section 8:
Why should a mirror do VoIP Metric Reports Block XR blocks for a video
stream. I think some clarification based on media type is in order here.

34. Section 9.
This section is thoroughly inadequate. The reason is that you have a
forward path that is coupled to the reverse path. If the loopback is to
work correctly the mirror needs to be able to send back what the source
sent to it. Thus it is the loopback source that needs to adapt the media
bit-rate so that both forward and reverse path can handle the flows. If
doesn't the mirror will need to reduce the bit-rate it transmits and
that can only happen in two ways, dropping packets or transcoding them
to a lower rate fulfilling the congestion control requirement. Neither
will fulfill the purpose of the loopback as I see it. This issue needs
discussion, including some considerations in how to actually accomplish
it with the tools we have available.

35. Section 10.1

What is the purpose of this example?

What I see it establish the fact that the end-points can take respecitve
roles of source and mirror. But it doesn't indicate any media which can
be looped back?

36. Section 10.1
Either of the o= lines are in error. Because they can't be the same when
it comes to the ID and version fields.

37. Section 11:
This needs to be very much fleshed out.

First of all the denial of service attack are actually several possible.

One is to use a loopback mirror as a slight amplification source and
primarily a indirection. This is accomplished by attacker spoofing
source address to the targets and sending media to the loopback mirror.

A much more hideous attack is the one where an attacker emits signalling
messages that results in that two mirrors are connected to each other.
As soon as a few packet are injected into that loop it will grow until
the resources are consumed. Any non lost packet arriving at either
mirror will be forwarded and grow 16 bytes per mirror bounce. It will
bounce between the mirrors until fragmentation occurs and then you have
two packets. This will continue until all CPU or bandwidth resources are
consume causing packet loss.

So I think you better explain how one in reality do authenticate the
signalling messages so that only approved initiators of loopback are
allowed. Because doing this without being certain that it is not a
malicous entity setting up the sessions are plain stupid and this should
not be specified if there is no solution to this issue.

I also think a consideration of the media plane security should be done.
Are there any privacy concerns.

38. I also don't see any discussion of the interaction between this an
the high layers. For example if this is implemented in a SIP end-point
is that supposed to ring? Are there any case where media will be allowed
to be played out when doing loopback? I don't think so, but it should be
stated.

39. Section 7.2:

If one has multiple media sources in an RTP session arriving at the
mirror. I expect the mirror to loopback both. However, in which way does
the loopback source determine which mirror stream is related to the one
being sent?

40. Section 7.2: In an RTP session where one uses more than a single
payload type for the source to mirror media, how does the source
determine which original payload type the payload had?

Is there need for solution the Retransmission payload foramt uses RFC 4588?

41. The media type registrations "rate" parameter I think needs to be
redefined. It needs to reference the rate of the payload types it
intends to mirror back.

Also, I don't think 8k Hz clock is typical enough for any of the media
type. Audio has a number of common ones beyond 8k. video uses most
commonly but not exclusively 90k. Text uses yet something else.

Cheers

Magnus Westerlund

----------------------------------------------------------------------
Multimedia Technologies, Ericsson Research EAB/TVM
----------------------------------------------------------------------
Ericsson AB                | Phone  +46 10 7148287
Färögatan 6                | Mobile +46 73 0949079
SE-164 80 Stockholm, Sweden| mailto: [email-masked]
----------------------------------------------------------------------

---

From: Hadriel Kaplan <[email-masked]>
Date: Tue, 25 Oct 2011 18:01:05 +0000

> in an established session on hold. Isn't that reasonable?

How about this:
Since media loopback requires bidirectional RTP, its normal direction mode is "sendrecv"; the "sendrecv" direction attribute MAY be encoded in SDP or not, as per section 5.1 of [RFC3264], since it is implied by default.  If either the loopback source or mirror wish to disable loopback use during a session, the direction attribute "inactive" MUST be used as per [RFC3264].  The direction mode attributes "recvonly" and "sendonly" are incompatible with the loopback mechanism and MUST NOT be indicated when generating an SDP Offer or Answer.  When receiving an SDP Offer or Answer, if "recvonly" or "sendonly" is indicated for loopback, the SDP-receiving agent MAY treat it as a failure of the loopback negotiation or ignore the direction attribute. 

[This last sentence is probably controversial?]

-hadriel

---

From: Magnus Westerlund <[email-masked]>
Date: Thu, 27 Oct 2011 10:55:56 +0200

On 2011-10-25 20:01, Hadriel Kaplan wrote:
>> 11. Section 5.3 "    Note: A loopback offer in a given media
>> description MUST NOT contain the standard mode attributes sendonly,
>> recvonly, sendrecv, or inactive. The loopback-mode attributes
>> (loopback-source and loopback-mirror) replace the standard
>> attributes."
>> 
>> This appears to be a strange MUST NOT which I think might
>> encounter issues. Considering that SIP proxies that looks into this
>> SDP requires the direction attribute to be present and adds it,
>> what happens now. And why isn't sendrecv and inactive possible
>> values? sendrecv appears to be requried for loopback to work, and
>> inactive would put media transport in an established session on
>> hold. Isn't that reasonable?
> 
> How about this: Since media loopback requires bidirectional RTP, its
> normal direction mode is "sendrecv"; the "sendrecv" direction
> attribute MAY be encoded in SDP or not, as per section 5.1 of
> [RFC3264], since it is implied by default.  If either the loopback
> source or mirror wish to disable loopback use during a session, the
> direction attribute "inactive" MUST be used as per [RFC3264].  The
> direction mode attributes "recvonly" and "sendonly" are incompatible
> with the loopback mechanism and MUST NOT be indicated when generating
> an SDP Offer or Answer.  When receiving an SDP Offer or Answer, if
> "recvonly" or "sendonly" is indicated for loopback, the SDP-receiving
> agent MAY treat it as a failure of the loopback negotiation or ignore
> the direction attribute.
> 

That is much better. I think the only "controversial" from my
perspective is the "ignore" part. It is clearly an error.

Cheers

Magnus Westerlund