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

feat: sctp connection using usrsctp #11

Open
wants to merge 73 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 64 commits
Commits
Show all changes
73 commits
Select commit Hold shift + click to select a range
3d746b2
feat: sctp connection using usrsctp
lchenut Mar 8, 2024
654a69a
feat: added sctp ping&pong examples
lchenut Mar 8, 2024
4b9c506
Unregister address while closing
lchenut Mar 21, 2024
9b51d57
change ci building examples
lchenut Apr 15, 2024
c8a9fd3
Merge remote-tracking branch 'origin/master' into sctp-protocol
lchenut Jul 24, 2024
9b5c58d
chore: split files
lchenut Jul 26, 2024
3aa14e9
chore: expose what should be exposed & removed artifact object fields
lchenut Jul 26, 2024
d09aa30
fix: examples
lchenut Jul 26, 2024
ccf671c
chore: shuffle part of the code to its correct file
lchenut Jul 26, 2024
3f11895
chore: rename handleUpcall into recvCallback
lchenut Jul 26, 2024
a53f87f
chore: remove import & change variable name
lchenut Jul 26, 2024
d6f73ce
feat: check if socket is nil after accepting
lchenut Jul 26, 2024
8910d1e
chore: renaming address into raddr
lchenut Jul 26, 2024
73e4731
chore: rewrite connect / accept to manage error while calling usrsctp…
lchenut Jul 26, 2024
bd5841c
chore: update comment
lchenut Jul 26, 2024
a9df70a
Merge remote-tracking branch 'origin/master' into sctp-protocol
lchenut Aug 13, 2024
b77410b
fix: ci
lchenut Aug 20, 2024
ac65c6f
chore: remove useless field
lchenut Aug 20, 2024
df5eddd
feat: add testsctp & make it work
lchenut Aug 22, 2024
62cc2c7
chore: remove timer handler
lchenut Aug 23, 2024
2d2f012
Merge remote-tracking branch 'origin/master' into sctp-protocol
lchenut Aug 23, 2024
201a163
feat: add async exception tracking
lchenut Aug 23, 2024
4a59265
style: pass nph on sctp files
lchenut Aug 23, 2024
96a363e
feat: add sctp tracker counter
lchenut Aug 23, 2024
197c0c3
fix: readloop is back
lchenut Aug 23, 2024
722db0f
fix: windows ci
lchenut Aug 26, 2024
54612e9
chore: remove raddr from SctpConn
lchenut Aug 26, 2024
ecc4fbb
fix: defined EINPROGRESS when using windows
lchenut Aug 26, 2024
d6b00e5
fix: windows & macos ci
lchenut Aug 26, 2024
6e96abe
feat: add testsctp.nim to runalltest.nim
lchenut Aug 26, 2024
785d2bb
chore: turn on sctp debug
lchenut Aug 26, 2024
c8ebe7e
fix: windows EINPROGRESS error
lchenut Aug 26, 2024
2b1228f
fix: debug all
lchenut Aug 26, 2024
108841d
fix: remove useless code
lchenut Aug 26, 2024
5fcc26d
fix: change EINPROGRESS again
lchenut Aug 27, 2024
57cd0f0
fix: change EINPROGRESS again
lchenut Aug 27, 2024
e914f69
fix: change EINPROGRESS again
lchenut Aug 27, 2024
183d8d5
feat: add a test
lchenut Aug 27, 2024
f317070
chore: move socket/error constant to sctp_utils.nim
lchenut Aug 27, 2024
2a80d57
fix: change EINPROGRESS again
lchenut Aug 27, 2024
3305615
fix: multiple initialization causing issues
lchenut Aug 28, 2024
41a091f
Merge remote-tracking branch 'origin/master' into sctp-protocol
lchenut Aug 30, 2024
b97ffaf
fix: closing sctp connection should close underlying dtls connection
lchenut Aug 30, 2024
f0ff34c
fix: remove printf for windows temporarily
lchenut Aug 30, 2024
a59bb76
fix: add iphlpapi for windows
lchenut Sep 5, 2024
4a80ffd
chore: change add errno to log
lchenut Sep 5, 2024
5f50e55
feat: replace sentFuture by sendQueue
lchenut Sep 6, 2024
374c83d
feat: better use of SctpState
lchenut Sep 6, 2024
0d7f15e
fix: read now raises WebRtcError
lchenut Sep 6, 2024
ab707d4
fix: conninput and connect are now awaited
lchenut Sep 6, 2024
13e4560
fix: readLoop is now stop when SctpConn is closed
lchenut Sep 6, 2024
1d66aac
chore: remove useless code and rewrite trace/warn
lchenut Sep 10, 2024
f231a9d
fix: remove doAssert
lchenut Sep 10, 2024
3647e11
fix: renaming close into stop and change logs
lchenut Sep 10, 2024
baf91b9
chore: move `toFlags` to a more relevant place
lchenut Sep 10, 2024
413f323
feat: close every connection when stopping transport
lchenut Sep 10, 2024
857e5da
chore: remove exposed attributes
lchenut Sep 12, 2024
21e0cfe
chore: add description and remove trace
lchenut Sep 12, 2024
185642b
chore: remove comments & exposition
lchenut Sep 12, 2024
9c6b197
chore: update logs
lchenut Sep 12, 2024
38d40f8
chore: remove outdated comments
lchenut Sep 12, 2024
8d26ed6
chore: update comment
lchenut Sep 12, 2024
a37f9df
chore: remove unsafeAddr
lchenut Sep 20, 2024
f41f255
fix: macos error
lchenut Sep 20, 2024
ae7d377
fix: use port 0 for tests
lchenut Oct 4, 2024
4f8f901
fix: split test and build examples in ci
lchenut Oct 4, 2024
7d612aa
chore: move logutils from sctp_utils into its own file
lchenut Oct 4, 2024
5adb9ae
docs: `SctpConn` and `SctpMessageParameters` field
lchenut Oct 4, 2024
fa54c20
docs: add why ws2_32 and iphlpapi are necessary on windows
lchenut Oct 4, 2024
48e7d44
chore: remove empty line
lchenut Oct 4, 2024
c1fc6dc
feat: add the option to gracefully close outgoing channels
lchenut Oct 11, 2024
b63044b
chore: add a trace of which notification we receive
lchenut Oct 11, 2024
2e6055a
fix: change Socklen into SockLen
lchenut Oct 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions examples/ping.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import chronos, stew/byteutils
import ../webrtc/udp_transport
import ../webrtc/stun/stun_transport
import ../webrtc/dtls/dtls_transport
import ../webrtc/sctp/[sctp_transport, sctp_connection]

proc main() {.async.} =
let laddr = initTAddress("127.0.0.1:4244")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we fix a local address?

Copy link
Collaborator Author

@lchenut lchenut Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's an example and ping.nim works with pong.nim. I could improve those with some arguments or with an interactive prompt, but, again, it's not meant to be run in the CI, just to be shown as an example of how you can create an SCTP ping.

let udp = UdpTransport.new(laddr)
let stun = Stun.new(udp)
let dtls = Dtls.new(stun)
let sctp = Sctp.new(dtls)
Comment on lines +9 to +12
Copy link
Contributor

@diegomrsantos diegomrsantos Sep 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the final instance that matter is sctp, maybe we could just have something like Sctp.new(laddr)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There will be something similar in the datachannel PR: Webrtc.new(...) will be used.
If we use something like Sctp.new(laddr), it means we have to stop all transports if we choose to stop Sctp, which I don't like. I'd rather have a way to manage the whole stack by itself (with Webrtc) or to manage each part of the stack (like this example).


let conn = await sctp.connect(initTAddress("127.0.0.1:4242"), sctpPort = 13)
while true:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do this only a few times, maybe 10?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an example, not a test, having an infinite loop is not really bad practice imo.

await conn.write("ping".toBytes)
let msg = await conn.read()
echo "Received: ", string.fromBytes(msg.data)
await sleepAsync(1.seconds)

waitFor(main())
27 changes: 27 additions & 0 deletions examples/pong.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import chronos, stew/byteutils
import ../webrtc/udp_transport
import ../webrtc/stun/stun_transport
import ../webrtc/dtls/dtls_transport
import ../webrtc/sctp/[sctp_transport, sctp_connection]

proc sendPong(conn: SctpConn) {.async.} =
var i = 0
while true:
let msg = await conn.read()
echo "Received: ", string.fromBytes(msg.data)
await conn.write(("pong " & $i).toBytes)
i.inc()

proc main() {.async.} =
let laddr = initTAddress("127.0.0.1:4242")
let udp = UdpTransport.new(laddr)
let stun = Stun.new(udp)
let dtls = Dtls.new(stun)
let sctp = Sctp.new(dtls)

sctp.listen(13)
while true:
let conn = await sctp.accept()
asyncSpawn conn.sendPong()

waitFor(main())
3 changes: 3 additions & 0 deletions tests/runalltests.nim
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,8 @@

{.used.}

{.passc: "-DSCTP_DEBUG".}

import teststun
import testdtls
import testsctp
102 changes: 102 additions & 0 deletions tests/testsctp.nim
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
# Nim-WebRTC
# Copyright (c) 2024 Status Research & Development GmbH
# Licensed under either of
# * Apache License, version 2.0, ([LICENSE-APACHE](LICENSE-APACHE))
# * MIT license ([LICENSE-MIT](LICENSE-MIT))
# at your option.
# This file may not be copied, modified, or distributed except according to
# those terms.

{.used.}

import chronos
import ../webrtc/udp_transport
import ../webrtc/stun/stun_transport
import ../webrtc/dtls/dtls_transport
import ../webrtc/sctp/sctp_transport
import ../webrtc/sctp/sctp_connection
import ./asyncunit

suite "SCTP":
teardown:
checkLeaks()

type
SctpStackForTest = object
localAddress: TransportAddress
udp: UdpTransport
stun: Stun
dtls: Dtls
sctp: Sctp

proc initSctpStack(localAddress: TransportAddress): SctpStackForTest =
result.localAddress = localAddress
result.udp = UdpTransport.new(result.localAddress)
result.stun = Stun.new(result.udp)
result.dtls = Dtls.new(result.stun)
result.sctp = Sctp.new(result.dtls)
result.sctp.listen()

proc closeSctpStack(self: SctpStackForTest) {.async: (raises: [CancelledError]).} =
await self.sctp.stop()
await self.dtls.stop()
await self.stun.stop()
await self.udp.close()

asyncTest "Two SCTP nodes connecting to each other, then sending/receiving data":
var
sctpServer = initSctpStack(initTAddress("127.0.0.1:4444"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to use 0 port to avoid errors with port reuse.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed those. It makes me change some things in udp_transport.nim and Stun transport/connection. If you think it's not appropriate to make those changes in this PR, I can revert these changes and create another one.

sctpClient = initSctpStack(initTAddress("127.0.0.1:5555"))
let
serverConnFut = sctpServer.sctp.accept()
clientConn = await sctpClient.sctp.connect(sctpServer.localAddress)
serverConn = await serverConnFut

await clientConn.write(@[1'u8, 2, 3, 4])
check (await serverConn.read()).data == @[1'u8, 2, 3, 4]

await serverConn.write(@[5'u8, 6, 7, 8])
check (await clientConn.read()).data == @[5'u8, 6, 7, 8]

await clientConn.write(@[10'u8, 11, 12, 13])
await serverConn.write(@[14'u8, 15, 16, 17])
check (await clientConn.read()).data == @[14'u8, 15, 16, 17]
check (await serverConn.read()).data == @[10'u8, 11, 12, 13]

await allFutures(clientConn.close(), serverConn.close())
await allFutures(sctpClient.closeSctpStack(), sctpServer.closeSctpStack())

asyncTest "Two DTLS nodes connecting to the same DTLS server, sending/receiving data":
var
sctpServer = initSctpStack(initTAddress("127.0.0.1:4444"))
sctpClient1 = initSctpStack(initTAddress("127.0.0.1:5555"))
sctpClient2 = initSctpStack(initTAddress("127.0.0.1:6666"))
let
serverConn1Fut = sctpServer.sctp.accept()
serverConn2Fut = sctpServer.sctp.accept()
clientConn1 = await sctpClient1.sctp.connect(sctpServer.localAddress)
clientConn2 = await sctpClient2.sctp.connect(sctpServer.localAddress)
serverConn1 = await serverConn1Fut
serverConn2 = await serverConn2Fut

await serverConn1.write(@[1'u8, 2, 3, 4])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any particular meaning to these arrays? We can avoid the repetition by defining a const for the same values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No particular meaning, no. I just wanted different message sent every time I read/write.

await serverConn2.write(@[5'u8, 6, 7, 8])
await clientConn1.write(@[9'u8, 10, 11, 12])
await clientConn2.write(@[13'u8, 14, 15, 16])
check:
(await clientConn1.read()).data == @[1'u8, 2, 3, 4]
(await clientConn2.read()).data == @[5'u8, 6, 7, 8]
(await serverConn1.read()).data == @[9'u8, 10, 11, 12]
(await serverConn2.read()).data == @[13'u8, 14, 15, 16]
await allFutures(clientConn1.close(), serverConn1.close())

await serverConn2.write(@[5'u8, 6, 7, 8])
await clientConn2.write(@[13'u8, 14, 15, 16])
check:
(await clientConn2.read()).data == @[5'u8, 6, 7, 8]
(await serverConn2.read()).data == @[13'u8, 14, 15, 16]
await allFutures(clientConn2.close(), serverConn2.close())

await allFutures(sctpClient1.closeSctpStack(),
sctpClient2.closeSctpStack(),
sctpServer.closeSctpStack())
17 changes: 15 additions & 2 deletions webrtc.nimble
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,18 @@ var cfg =
" --threads:on --opt:speed"

when defined(windows):
cfg = cfg & " --clib:ws2_32"
cfg = cfg & " --clib:ws2_32 --clib:iphlpapi"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to add a comment explaining why this is necessary.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


import hashes

proc buildExample(filename: string, run = false, extraFlags = "") =
var excstr = nimc & " " & lang & " " & flags & " -p:. " & extraFlags
excstr.add(" examples/" & filename)
exec excstr
if run:
exec "./examples/" & filename.toExe
rmFile "examples/" & filename.toExe

proc runTest(filename: string) =
var excstr = nimc & " " & lang & " -d:debug " & cfg & " " & flags
excstr.add(" -d:nimOldCaseObjects") # TODO: fix this in binary-serialization
Expand All @@ -36,5 +44,10 @@ proc runTest(filename: string) =
exec excstr & " -r " & " tests/" & filename
rmFile "tests/" & filename.toExe

task test, "Run test":
task test, "Run the test suite":
runTest("runalltests")
exec "nimble build_example"
Copy link
Contributor

@diegomrsantos diegomrsantos Sep 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure this should be part of the test task

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, I created a "Build examples" task.


task build_example, "Build the examples":
buildExample("ping")
buildExample("pong")
Loading
Loading