Skip to content

Commit

Permalink
Commit reliability on client after receiving ACK
Browse files Browse the repository at this point in the history
From RFC 8832
https://www.rfc-editor.org/rfc/rfc8832.html#name-procedures, messages
should be ordered till dialing/opening side receives ACK.

```
After the DATA_CHANNEL_OPEN message has been sent,
the sender of that message MAY start sending messages
containing user data without waiting for the reception
of the corresponding DATA_CHANNEL_ACK message. However,
before the DATA_CHANNEL_ACK message or any other message
has been received on a data channel, all other messages
containing user data and belonging to this data channel
MUST be sent ordered, no matter whether the data channel
is ordered or not. After the DATA_CHANNEL_ACK or any
other message has been received on the data channel,
messages containing user data MUST be sent ordered on
ordered data channels and MUST be sent unordered on
unordered data channels. Therefore, receiving a message
containing user data on an unused stream indicates an error.
In that case, the corresponding data channel MUST be closed,
as described in [RFC8831].
```

Without waiting, the data channel open failed on Accept side as the
first message did not have PPI of DCEP. It got a user message
and that caused the Accept to fail.
  • Loading branch information
boks1971 authored and Sean-Der committed Aug 16, 2024
1 parent 0ac5cf6 commit d31efd0
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 6 deletions.
10 changes: 4 additions & 6 deletions datachannel.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,7 @@ func Client(stream *sctp.Stream, config *Config) (*DataChannel, error) {
return nil, fmt.Errorf("failed to send ChannelOpen %w", err)
}
}
dc := newDataChannel(stream, config)

if err := dc.commitReliabilityParams(); err != nil {
return nil, err
}
return dc, nil
return newDataChannel(stream, config), nil
}

// Accept is used to accept incoming data channels over SCTP
Expand Down Expand Up @@ -285,6 +280,9 @@ func (c *DataChannel) handleDCEP(data []byte) error {

switch msg := msg.(type) {
case *channelAck:
if err := c.commitReliabilityParams(); err != nil {
return err
}
c.onOpenComplete()
default:
return fmt.Errorf("%w, wanted ACK got %v", ErrUnexpectedDataChannelType, msg)
Expand Down
26 changes: 26 additions & 0 deletions datachannel_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,8 +384,34 @@ func TestDataChannel(t *testing.T) {
assert.True(t, reflect.DeepEqual(dc0.Config, *cfg), "local config should match")
assert.True(t, reflect.DeepEqual(dc1.Config, *cfg), "remote config should match")

// reliability parameters are committed after data channel open ACK is received on client side,
// wait for open to be completed
openCompleted := make(chan bool)
dc0.OnOpen(func() {
close(openCompleted)
})

var n int

// write a message as ReadChannel loops till user data is read
bridgeProcessAtLeastOne(br)
binary.BigEndian.PutUint32(sbuf, 10)
n, err = dc1.WriteDataChannel(sbuf, true)
assert.Nil(t, err, "Write() should succeed")
assert.Equal(t, len(sbuf), n, "data length should match")

// read data channel open ACK and the user message sent above
bridgeProcessAtLeastOne(br)
_, _, err = dc0.ReadDataChannel(rbuf)
assert.NoError(t, err)

select {
case <-time.After(time.Second * 10):
assert.FailNow(t, "OnOpen() failed to fire 10s")
case <-openCompleted:
}

// test unordered messages
binary.BigEndian.PutUint32(sbuf, 1)
n, err = dc0.WriteDataChannel(sbuf, true)
assert.Nil(t, err, "Write() should succeed")
Expand Down

0 comments on commit d31efd0

Please sign in to comment.