API - Support Stream+Sink for ProxySocket?

[ibigbug]

trafficstars

The tcp stream does take opaque AsyncRead and AsyncWrite as underlying io https://docs.rs/shadowsocks/latest/shadowsocks/relay/tcprelay/proxy_stream/client/struct.ProxyClientStream.html#impl-AsyncRead-for-ProxyClientStream%3CS%3E

thoughts about having the udp socket to take Stream + Sink too ?

https://docs.rs/shadowsocks/latest/shadowsocks/relay/udprelay/proxy_socket/struct.ProxySocket.html

so it can be applied to proxy more general udp transports

[zonyitoo]

There is no Datagram based sockets in Rust's common libraries supports Read / Write or AsyncRead / AsyncWrite , here we are just following the common design in the Rust world.

so it can be applied to proxy more general udp transports

So what exactly problem are you struggling on?

[ibigbug]

there's Sink and Stream https://docs.rs/futures/latest/futures/sink/trait.Sink.html

i'd like to have ss wrap another UDP transport which impls Sink and Stream

[zonyitoo]

You can make a wrapper that wraps ProxyClientStream and make your own Sink

[ibigbug]

how to pass it to https://docs.rs/shadowsocks/latest/shadowsocks/relay/udprelay/proxy_socket/struct.ProxySocket.html#method.from_socket

[ibigbug]

essentially i want to pass something that is Sink here https://github.com/Watfaq/clash-rs/blob/master/clash_lib/src/proxy/shadowsocks/mod.rs#L234

[zonyitoo]

So you want the underlying socket implements Sink and let ProxySocket::from_socket works with types that implemented Sink?

That would require quite a lot of changes in the current implementation.

1. Let ShadowUdpSocket implements Sink
2. Make all the APIs of ProxySocket works with interfaces provided by Stream and Sink.

I don't see the necessity here because the underlying connection is not a "stream" based connection.

[ibigbug]

There's nothing todo with a stream. and I think the Sink and Stream trait is the AsyncRead and AsyncWrite for Udp or any segmented io.

I know it's a fundamental change that requires amount of work.

I've had a look into the current code, it doesn't look like any udpsocket API is accessed other than the io related, which are fully replaceable by the Sink and Stream traits.

I'm happy to make the changes if it we agreed it's way to go

[zonyitoo]

Please make a PR.

[ibigbug]

I'm working on this and realized a lot of the io methods take tokio ToSocketAddrs https://github.com/Watfaq/shadowsocks-rust/blob/7c154d2340849cdabb5971b3b6982f187a6b7efe/crates/shadowsocks-service/src/net/mon_socket.rs#L50

which is handing the DNS to sys resolver, should we change it to take plain SocketAddr and resolve the dns_resolver in the code base?

[zonyitoo]

ToSocketAddrs uses tokio's builtin DNS resolver, which is the libc's builtin getaddrinfo running in a thread-pool. In this project we use hickory-dns by default, which is built upon async/await framework.

[ibigbug]

Yeah I get it. Should we not use sys dns and use hickory dns for all?