rust-libp2p icon indicating copy to clipboard operation
rust-libp2p copied to clipboard

feat(transports/unix-stream): add

Open nabijaczleweli opened this issue 5 months ago • 0 comments

Description

This allows dialling with and listening on AF_UNIX/SOCK_STREAM sockets.

Ripped 100% off transports/tcp, as in I cp -r tcp unix-streamed it.

Uses proposed encoding from https://github.com/multiformats/multiaddr/pull/174#issuecomment-2964331099

Notes & open questions

I called the empty tag enum UnixStrm due to a name collision. I don't know what the purpose of it is anyway, but.

As for the encoding, I hope to move it entirely into multiaddr; transports/tcp has

Ok(SocketAddr::new(ipv4.into(), port)),

and

fn ip_to_multiaddr(ip: IpAddr, port: u16) -> Multiaddr {
    Multiaddr::empty().with(ip.into()).with(Protocol::Tcp(port))
}

so the same thing should be available once that lands in multiaddr.

Change checklist

  • [x] I have performed a self-review of my own code
  • [x] I have made corresponding changes to the documentation
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [ ] A changelog entry has been made in the appropriate crates ‒ I don't see a root entry in transports/tcp so I've left it empty

nabijaczleweli avatar Jun 11 '25 23:06 nabijaczleweli

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: mwarning
:x: adamierymenko
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Apr 15 '25 11:04 CLAassistant

thank you mwarning! Do you happen to know what errors are caused by not having this patch is? Is there an issue somewhere on openwrt? Just curious.

laduke avatar Apr 15 '25 14:04 laduke

This is what the original problem: https://github.com/openwrt/openwrt/issues/18019 I am not actually sure this is the correct fix as compilation should have failed if the signature does not match. But there were reports of the segfault beeing fixed now.

mwarning avatar Apr 15 '25 15:04 mwarning

I'm curious how you're building it. miniupnp is part of our normal build process and statically linked into the binary. So unless you're doing something in the OpenWRT build process to remove that completely from the build and include paths, I can definitely see something funky happening. The actual results aren't what I would expect though 🤔

glimberg avatar Apr 16 '25 17:04 glimberg

@glimberg yes,on OpenWrt we dynamicly link in the system miniupnpc library. It was originally done to save space to support 4mb flash devices. But after ZT uses more advanced C++ features and linking to uclibcxx was not possible anymore, this became less relevant, as libstdc++ needs to be linked now and it is rather huge compared to uclibcxx.

mwarning avatar Apr 16 '25 18:04 mwarning

@mwarning: Can you check the CLA? @zerotier team (@laduke, @glimberg, ...): Have you progressed on this ticket/PR?

It will be nice to fix it.

Thanks in advance.

Linked to:

  • https://github.com/zerotier/ZeroTierOne/issues/2325
  • https://github.com/zerotier/ZeroTierOne/issues/2332
  • https://github.com/zerotier/ZeroTierOne/issues/2390
  • https://github.com/openwrt/openwrt/issues/18019
  • https://github.com/openwrt/packages/pull/26088

Neustradamus avatar May 24 '25 13:05 Neustradamus

@mwarning @glimberg

I am not actually sure this is the correct fix as compilation should have failed if the signature does not match. But there were reports of the segfault beeing fixed now.

I know how this happened.

  1. openwrt use include from non-system paths when cross-compiling. https://github.com/openwrt/packages/blob/019a8f1a98e14b8ba337d1dfae143d93ce121886/net/zerotier/patches/0001-fix-miniupnpc-natpmp-include-path.patch changes the MINIUPNPC_IS_NEW_ENOUGH detaction, use a miniupnpc/miniupnpc.h not in system include path. MINIUPNPC_IS_NEW_ENOUGH is 1 .
  2. zerotier make-linux.mk:12 contains INCLUDES?=-Irustybits/target -isystem ext ..., notice cflags -isystem ext
  3. zerotier osdep/PortMapper.cpp:47 contains #include <miniupnpc/miniupnpc.h>, cflags -isystem ext makes it actually include ext/miniupnpc/miniupnpc.h, because there is not miniupnpc/miniupnpc.h in default system include path (/usr/include, etc.).

This is the generated g++ command:

ccache aarch64-openwrt-linux-musl-g++ -Os -pipe -mcpu=generic -fno-caller-saves -fno-plt -fhonour-copts -ffile-prefix-map=/mnt/data/build/istoreos-build/24.10/armsr/openwrt/build_dir/target-aarch64_generic_musl/ZeroTierOne-1.14.1=ZeroTierOne-1.14.1 -ffunction-sections -fdata-sections -Wformat -Werror=format-security -fstack-protector -D_FORTIFY_SOURCE=1 -Wl,-z,now -Wl,-z,relro -Wl,-z,noexecstack -ffunction-sections -fdata-sections  \
  -I/mnt/data/build/istoreos-build/24.10/armsr/openwrt/staging_dir/toolchain-aarch64_generic_gcc-13.3.0_musl/usr/include \
  -I/mnt/data/build/istoreos-build/24.10/armsr/openwrt/staging_dir/toolchain-aarch64_generic_gcc-13.3.0_musl/include -I/mnt/data/build/istoreos-build/24.10/armsr/openwrt/staging_dir/toolchain-aarch64_generic_gcc-13.3.0_musl/include/fortify  -Wall -Wno-deprecated -std=c++17 -pthread \
  -Irustybits/target \
  -isystem ext \
  -Iext/prometheus-cpp-lite-1.0/core/include -Iext-prometheus-cpp-lite-1.0/3rdparty/http-client-lite/include -Iext/prometheus-cpp-lite-1.0/simpleapi/include -DNDEBUG  -DZT_USE_MINIUPNPC \
  -DZT_USE_SYSTEM_MINIUPNPC \
  -DZT_USE_SYSTEM_NATPMP -DZT_NO_TYPE_PUNNING -DZT_ARCH_ARM_HAS_NEON -march=armv8-a+crypto -mtune=generic -mstrict-align -DZT_BUILD_PLATFORM=1 -DZT_BUILD_ARCHITECTURE=4 -DZT_SOFTWARE_UPDATE_DEFAULT="\"disable\"" -D_MT_ALLOCATOR_H -D_POOL_ALLOCATOR_H -D_EXTPTR_ALLOCATOR_H -D_DEBUG_ALLOCATOR_H   -c -o osdep/PortMapper.o osdep/PortMapper.cpp

-I/mnt/data/build/istoreos-build/24.10/armsr/openwrt/staging_dir/toolchain-aarch64_generic_gcc-13.3.0_musl/usr/include is normal include, not a system include. so #include <miniupnpc/miniupnpc.h> is found in -isystem ext.

zerotier use own ext/miniupnpc/miniupnpc.h when compiling, but link to system libminiupnpc when running. that's why there are no errors when compiling, but it crashes when running.

Archlinux workaround this by simply rm -rf ext/miniupnpc/ https://gitlab.archlinux.org/archlinux/packaging/packages/zerotier-one/-/blob/main/PKGBUILD?ref_type=heads#L31

jjm2473 avatar Jun 10 '25 04:06 jjm2473

Dear @zerotier members, @glimberg, @laduke, @joseph-henry: Any progress on this PR?

Neustradamus avatar Jun 10 '25 20:06 Neustradamus

@Neustradamus We don't manage the build for OpenWRT. If someone comes up with a PR that doesn't break our build distributions and fixes issues over there, we'll happily merge it.

glimberg avatar Jun 10 '25 20:06 glimberg

@glimberg What about syncing and using an up-to-date miniupnc version instead of shipping an outdated bundled library? 🤔 It could be done as a submodule, and it also would be great if you could let users decide if they want to use their library as ArchLinux does or use your bundled library.

BTW: It is not about OpenWrt; it happens also on other GNU/Linux distros.

BKPepe avatar Jun 11 '25 09:06 BKPepe