PPID should be per message and not per socket

[ibc]

Currently the SCTP PPID is set per socket (into the options of new net.Socket([options])). However, PPID is supposed to be a property of a SCTP message, this is, I should be able to send SCTP messages with different PPID over the same socket.

Somehow, socket.write() should have a second arguments options which contains the PPID (probably it should be mandatory).

Does it make sense?

[latysheff]

SCTP socket is an instance of node's Duplex stream, I don't know how to pass options when using standard API. You can set socket.ppid every time before writing (though this needs some fix and testing). Or probably worth reuse 'encoding' argument for ppid, if possible. What do you think?

Can you also indicate use case to change ppid? Just for reference.

[ibc]

Can you also indicate use case to change ppid? Just for reference.

You want to send a WebRTC String (ppid 51), then a WebRTC Binary (ppid 52) or you want event to implement and send/receive WebRTC DCEP DataChannel Control Protocol to manage SCTP streams in band (ppid 50):

https://www.iana.org/assignments/sctp-parameters/sctp-parameters.xhtml#sctp-parameters-25

BTW: A critical question: how to read the PPID in the received packets/messages in node-sctp? You can see how we do it in mediasoup.

SCTP socket is an instance of node's Duplex stream, I don't know how to pass options when using standard API. You can set socket.ppid every time before writing (though this needs some fix and testing).

Will that properly work if the lib has still pending DATA chunks to send? this is:

socket.ppid = 51; // WebRTC String
socket.write(SUPER_LONG_STRING_HERE); // So node-sctp lib will create 10f DATA chunks and, AFAIU, it won't send all together but by honoring the SCTP window size, etc.

// Let's assume that just 7 DATA chunks have been sent (there are 10 in total).
// And before node-sctp sends the remaining ones I do:

socket.ppid = 52; // WebRTC Binary

// I hope the new ppid does not affect pending DATA chunks (they must have ppid 51).

socket.write(BINARY_DATA);

// Some concern.

Or probably worth reuse 'encoding' argument for ppid, if possible. What do you think?

I'm fine with any mechanism. Initially, overriding the write() method comes to my mind, but I understand it's not so comfortable from the development point of view. If dynamically changing socket.ppid does work fine, I'll go that way.

[latysheff]

You are right, buffering is an issue. Setting individual packets parameters was a pain for me :)

But did you have a look at socket.createStream(id)? This is SCTP sub-stream mechanism. And a look into code shows that id is also used for ppid. It could be changed, maybe, to be different.

The same mechanism allows to get sub-stream ID (and id=ppid also for now). socket.on('stream', (stream, id) => { stream.on('data', data => { // Incoming data }) })

[ibc]

You are right, buffering is an issue. Setting individual packets parameters was a pain for me :)

The lib may have a this._currentPpid and this._nextPpid.

• this._nextPpid is set via socket.ppid.
• this._currentPpid is set via socket.ppid is there are no queued chunks to send.

This way, each SCPT message (so all its DATA chunks) will contain the ppid that was set before socket.write(message) was called.

But did you have a look at socket.createStream(id)? This is SCTP sub-stream mechanism. And a look into code shows that id is also used for ppid. It could be changed, maybe, to be different.

I don't understand why SCTP stream id should match DATA chunk ppid at all. To be perfectly clear: In WebRTC, browsers can just send and receive SCTP DATA chunks with ppid 50, 51, 52, 53, 54, 56 or 57. Otherwise they will ignore those chunks.

This is, ppid must be up to the application and must not be decided by the SCTP stack at all. The application must instruct the SCTP stack to send "messages" of a given ppid. There is no default ppid AFAIK. Some for reception. The ppid of the received DATA chunk must be notified to the app somehow.

BTW: If node-sctp receives a very long SCTP message splitted into 4 DATA chunks, will the stream object fire data 4 times with partial message fragments? or just once with the full message?

[latysheff]

I'm not saying that stack would decide PPID. For example, if you want to send WebRTC String (ppid 51), you just firstly do socket.createStream(51), or ok, const stream1 = socket.createStream(1, 51), and all that goes to stream1 has ppid 51. SCTP was invented to support streams, and I guess for some similar reasons (different media-content or sub-prorocols). Not sure if peer side (browser) uses streams for different PPIDs. If not, this approach will fail. But can you check? (real-world or specifications)

[latysheff]

BTW: If node-sctp receives a very long SCTP message splitted into 4 DATA chunks, will the stream object fire data 4 times with partial message fragments? or just once with the full message?

I believe this depends on watermark/drain stream mechanisms and timers for SACK, etc. So, in general this is not guaranteed at all. The same as in TCP streams.

[ibc]

No, browsers o not use different SCTP streams for different PPIDs. Browsers implement DataChannel spec. A DataChannel (dc) is just a SCTP bidirectional stream pair with same id in both directions over a single SCTP association. Multiple DataChannels can be created over the same SCTP association.

Those DataChannels are not intended to send different kind of data but just to have different identifiers and provide the app with the ability to manage different "channels" within the same WebRTC connection.

So I was wrong above. socket.ppid is not the way to go. ppid is per SCTP data chunk. We would accept making it "per SCTP stream", but forcing it to be "per socket (SCTP association)" is not good at all.

From my point of view, node-sctp should provide an API focused on SCTP streams rather than socket. socket.write() does not make any sense in the context of SCTP as you send SCTP messages on a specific SCTP stream and with a specific PPID (some for reception).

So, instead of this:

const stream1 = socket.createStream(1, 51); // streamId 1, ppid 51

I'd rather:

const stream1 = socket.createStream(1)

stream1.ppid = 51

stream1.write("hello");

socket.on('stream', (stream, id) => {
  stream.on('data', (data, ppid) => {
    // Incoming data
  })
})

[ibc]

BTW: If node-sctp receives a very long SCTP message splitted into 4 DATA chunks, will the stream object fire data 4 times with partial message fragments? or just once with the full message?

I believe this depends on watermark/drain stream mechanisms and timers for SACK, etc. So, in general this is not guaranteed at all. The same as in TCP streams.

I'm fine if the app is responsible to buffer and reassembly data sequential DATA chunks. In usrsctp lib the app must do it anyway.

[ibc]

BTW I'm testing master branch and I do not see that socket.ppid = N has any effect. socket.write() still generates SCTP data chunks with ppid = 0. Those are the logs in my server (interactive terminal to play):

terminal> bot._sctpSocket.ppid
666

terminal> bot._sctpSocket.write("123")
true

(data) RTC::SctpAssociation::ProcessSctpData()
000000 13 88 13 88 D9 96 E9 DD
000008 28 16 86 87 00 03 00 13
000010 64 49 3D BA 00 00 00 02
000018 00 00 00 00 31 32 33 00

mediasoup:worker[pid:77694] RTC::SctpAssociation::onRecvSctpData() | data chunk received [length:3, streamId:0, SSN:2, TSN:1682521530, PPID:0, context:0, flags:8]

With sctp debug logs:

terminal> bot._sctpSocket.ppid = 22

terminal>  bot._sctpStream.write("123")  sctp:sockets:# 5000/undefined:5000 > write stream 666, 3 bytes +57s
  sctp:assoc:# [5000/undefined:5000] SEND 3 bytes, { stream_id: 666 } +37s
  sctp:assoc: [5000/undefined:5000] send 3 bytes, { stream_id: 666 } +37s
  sctp:assoc: [5000/undefined:5000] check retransmits +0ms
  sctp:chunk new chunk Chunk {
  sctp:chunk   chunkType: 'data',
  sctp:chunk   chunkId: 0,
  sctp:chunk   flags: { E: 1, B: 1, U: undefined, I: 0 },
  sctp:chunk   tsn: null,
  sctp:chunk   stream_id: 666,
  sctp:chunk   ssn: 1,
  sctp:chunk   ppid: undefined,
  sctp:chunk   user_data: <Buffer 31 32 33>
  sctp:chunk } +37s

The very same happens if I use socket.write() instead of stream.write(). PPID is always undefined.

[ibc]

I've been thinking about this. As said above, SCTP is based on streams (when it comes to send/receive data) so socket.write() has no sense as public API IMHO. I would just remove it and force the app to create streams via socket.createStream().

And then, when it comes to PPID stuff, stream.write() could also be dropped and replaced with stream.send(msg) where msg is not a string or binary content, but an Object as follows:

{
  data : String or Binary (mandatory)
  ppid : Number (mandatory)
}

or it could be:

stream.send(data, ppid);

This way the lib could handle per message PPID.

And when it comes to receive data:

stream.on('data', (data, ppid) => { ... });

[ibc]

oh, so node-sctp is indeed using the options.streamId as ppid!!:

https://github.com/latysheff/node-sctp/blob/master/lib/association.js#L1785

Well, that's not correct at all. Also, even if wrong, it's not honored (PPID is always 0).

[latysheff]

Socket.write is ok, if you don't want to use streams. Default stream is 0, by RFC, and in some cases this is what was intended. It should be clearly stated in readme, though.

If reusing Duplex stream 'encoding' argument is safe, API will be socket.send(data, ppid) and stream.send(data, ppid). But what about receiving? I don't know how to handle it with standard API, yet.

[ibc]

Socket.write is ok, if you don't want to use streams. Default stream is 0, by RFC, and in some cases this is what was intended. It should be clearly stated in readme, though.

Fine :)

If reusing Duplex stream 'encoding' argument is safe, API will be socket.send(data, ppid) and stream.send(data, ppid).

Note that you are writing send instead of write. Is intended? I'd be super happy with it, but I think you meant write().

But what about receiving? I don't know how to handle it with standard API, yet.

What about writing data.ppid = DATA_CHUNK_PPID before emitting the "data" event? Or couldn't the lib emit "data" event with an Object { data, ppid } instead?

[latysheff]

Lol, really, why just not use buffer (which is an object) property?! Let's do it.

[ibc]

Lol, really, why just not use buffer (which is an object) property?! Let's do it.

Great! Do you have any expectation of when this could be done? I'm super ready to test it. In fact I've my code done here.

[latysheff]

Do you have any expectation of when this could be done?

In a couple of days, probably.

[ibc]

Super amazing :)

[latysheff]

Please check. UDP examples are modified to show changes.

[ibc]

Testing! let me some time to confirm.

BTW: do you know why this fails (Node 10.15.3)?

let text = "foo";
text.ppid = 1234;
// => TypeError: Cannot create property 'ppid' on string 'foo'

It does not fail when doing it in the Node REPL console. May be V8 compiles strings into some primitive types and do not allow adding properties into it?

[latysheff]

first convert text to buffer! then set buffer property buffer.ppid=1234

[ibc]

yes yes, I know, I was just trying to use a string with ppid property just to test it (but it's not valid it seems).

BTW: I think this is easier than what you do:

const buffer = Buffer.from("foo and bar");
buffer.ppid = sctp.PPID.WEBRTC_STRING;

:)

[ibc]

Ok... IT WORKS!!!

Yes! Given PPID is properly sent in SCTP DATA chunks and it's also present in received DATA chunks. Excellent! Thanks!

[latysheff]

I tested 100Mb buffer and stumbled upon reassembly overflow. Then fixed it by splitting chunk internally in socket _write()

[ibc]

Amazing.

What about people wishing to pass a string to write ()? Using PPID 0 is not a good choice (it won't work in WebRTC browsers, well, somehow Chrome accepts it but Firefox doesn't). May be a new sctp.defaults.ppid for those cases?

[latysheff]

Default should be 0. Don't forget about Sigtran over SCTP.

sctp.connect({}) accepts ppid as option (but need to check) socket.createStream(id) may also have second argument socket.createStream(id, ppid)

[ibc]

Didn't know that. Will that be documented? :)

[ibc]

Umm, honestly, if the "data" event comes with a Buffer as argument, I see no much reason to allow strings in write (). Since SCTP message units are both, a data and a PPID (and PPID is per message) I'd just get rid of any per socket or per stream "default PPID".

[latysheff]

I do not agree. Imagine, I create M3UA connection. So, I set SCTP socket ppid once, and then just write to it.

So, it's up to user:

1. use default 0 (because it is often optional)
2. set per socket or stream (convenience)
3. set every time when buffer is sent

[latysheff]

Will that be documented? :)

need to implement first :)

[ibc]

Yes ok ok, I just said it to simplify the API, but it's perfect as you say.

[latysheff]

Done. Closing issue.

[ibc]

Hi, although I love the current API in master (in which ppid is set as a property into the buffer given to socket.write(buffer) or stream.write(buffer), let me say that others ways to set the "default PPID" do not work yet.

I'm completely fine with dropping them from node-sctp (so they should be removed from the documentation). Just to make it clear, those options do not work:

• socket.ppid = 51
• stream.ppid = 51
• sctpSocket = sctp.connect({ ..., ppid: 51 })
• sctpSocket = sctp.connect({ ..., PPID: 51 })
• sctpStream = sctpSocket.createStream(streamId, 51)
• sctpSocket.write(data, 51)
• sctpSotream.write(data, 51)

None of those work (ppid is always 0 in the generated DATA chunks).

[ibc]

Sorry, I did not update! let me test.

[latysheff]

sctpSocket.write(data, 51) won't work ever, Node's API doesn't allow this

[ibc]

• socket.ppid = 51 => IT WORKS
• stream.ppid = 51 => IT WORKS
• sctpSocket = sctp.connect({ ..., ppid: 51 }) => IT WORKS
• sctpSocket = sctp.connect({ ..., PPID: 51 }) => never supposed to work :)
• sctpStream = sctpSocket.createStream(streamId, 51) => IT WORKS
• sctpSocket.write(data, 51) => never supposed to work :)
• sctpSotream.write(data, 51)=> never supposed to work :)

So everything is ok, sorry. Good work :)

[ibc]

Just a missing stuff in README documentation:

socket.on('stream', (stream, id) => {
  stream.on('data', data => {
    // Incoming data
  })
})

It should mention the ppid:

socket.on('stream', (stream, id) => {
  stream.on('data', data => {
    // Incoming data (data.ppid indicates the SCTP message PPID value)
  })
})

[ibc]

It seems that these lines will give preference to the ppid of the Socket or Stream than the ppid given in the write(data) call:

if (Number.isInteger(chunk.ppid)) options.ppid = chunk.ppid
if (!Number.isInteger(options.ppid)) options.ppid = this.ppid

Shouldn't it be like this instead?

if (Number.isInteger(chunk.ppid)) options.ppid = chunk.ppid
else if (!Number.isInteger(options.ppid)) options.ppid = this.ppid

[ibc]

Just forget my last comment. I don't know exactly why but the issue I mentioned does not happen :)