Skip to content

udpspeeder-simd: add package#29901

Open
connollydavid wants to merge 1 commit into
openwrt:masterfrom
connollydavid:udpspeeder-simd
Open

udpspeeder-simd: add package#29901
connollydavid wants to merge 1 commit into
openwrt:masterfrom
connollydavid:udpspeeder-simd

Conversation

@connollydavid

Copy link
Copy Markdown

📦 Package Details

Maintainer: @connollydavid

Description: Adds a package for UDPspeeder-simd, a SIMD-accelerated fork of
UDPspeeder that improves a high-latency, lossy link with Reed-Solomon forward
error correction. Coexists with the existing net/udpspeeder package (distinct
binary/config/init paths, no CONFLICTS); dynamically linked (DEPENDS: +libstdcpp
+librt +libatomic). On targets without a SIMD path (e.g. MIPS/ath79) it uses a
portable word-width path plus portable Reed-Solomon speedups, which is the
benefit for low-end devices. Requested in
#28562

🧪 Run Testing Details

  • OpenWrt Version: snapshot (SDK gcc-14.4.0, musl)
  • OpenWrt Target/Subtarget: x86/64
  • OpenWrt Device: x86_64 — built and packaged via the snapshot SDK; runtime
    exercised by the package's test.sh in CI

✅ Formalities

  • I have reviewed the CONTRIBUTING.md file.

(No patch in this PR — it adds a new package.)

Release notes

New package: udpspeeder-simd — a SIMD-accelerated UDPspeeder (FEC UDP tunnel),
installable alongside the existing udpspeeder package.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds a new OpenWrt feed package udpspeeder-simd (a SIMD-accelerated UDPspeeder fork) with its own binary name, init script, and UCI config so it can be installed alongside the existing net/udpspeeder.

Changes:

  • Introduces net/udpspeeder-simd package Makefile sourcing from a pinned git commit and installing binary/config/init assets.
  • Adds procd-based init script and default UCI configuration for managing tunnels via /etc/config/udpspeeder-simd.
  • Adds a CI runtime test.sh to sanity-check that the installed binary runs and prints a usage banner.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
net/udpspeeder-simd/Makefile New package definition, source pinning, build preparation, and install steps.
net/udpspeeder-simd/files/udpspeeder-simd.init New procd init script wiring UCI options into daemon invocation.
net/udpspeeder-simd/files/udpspeeder-simd.config New default UCI config template for a tunnel instance.
net/udpspeeder-simd/test.sh New CI runtime smoke test for the installed binary.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread net/udpspeeder-simd/test.sh Outdated
Comment thread net/udpspeeder-simd/files/udpspeeder-simd.init
@connollydavid connollydavid force-pushed the udpspeeder-simd branch 2 times, most recently from 3f45292 to 7f5bc7c Compare July 3, 2026 15:25
@connollydavid

connollydavid commented Jul 3, 2026

Copy link
Copy Markdown
Author

Thanks! Both fixed and pushed:

  • test.sh: now greps the package version (idiomatic), which also avoids matching the program name that a loader error echoes.
  • init: fix_latency moved into local, duplicate sock_buf dropped.

To grep the version I also fixed the banner, which had truncated it to 10 chars.

@codemarauder, you maintain net/udpspeeder; want to be on udpspeeder-simd too (co-maintainer, or primary if you'd rather)?

@connollydavid

Copy link
Copy Markdown
Author

Pushed a fix for the two 32-bit ARM failures (cortex-a9, cortex-a15).

The fork's xor_spe.S marked the stack non-executable with .section .note.GNU-stack,"",@progbits. On 32-bit ARM @ starts a line comment, so gas dropped @progbits and errored on the trailing comma. Changing it to %progbits, which gas accepts on every target, fixes ARM and leaves x86, aarch64, mips, ppc and riscv unchanged.

Fixed in the source repo and bumped PKG_SOURCE_VERSION + PKG_MIRROR_HASH here to match.

Comment thread net/udpspeeder-simd/test.sh Outdated
Comment thread net/udpspeeder-simd/Makefile Outdated
@connollydavid

Copy link
Copy Markdown
Author

Thanks for this, and apologies. My gitversion one-liner was wrong: $$(shell ...) gets double-expanded by the $(eval $(call ...)) around Build/Prepare, so make ran git rev-parse HEAD itself, the pattern never matched, and the version came out empty. CI caught it.

Fixed now with a substitution that keeps no $ in the pattern, the same idea as your netdata configure.ac:

sed -i 's/gitversion = \\".*\\"/gitversion = \\"$(PKG_VERSION)\\"/' $(PKG_BUILD_DIR)/makefile

And you're right about test.sh: once the version is baked in properly the generic tests cover the check, so I've dropped it.

I built both locally against the SDK for x86_64 and 32-bit ARM (arm_cortex-a15) this time and the binary reports the version grand. Sorry, I should have run it under qemu-arm before submitting. Cheers.

@BKPepe BKPepe added the AI It looks like this is issue or pull request was created by AI or it includes AI comments label Jul 5, 2026
@connollydavid

Copy link
Copy Markdown
Author

@BKPepe please do let me know if you need anything more,

there's data on performance of -simd improvements here: https://slartibardfast.github.io/UDPspeeder/

it's had about 5 months real world usage, I'm interested in adding RISC-v and other future simd targets (ARM's SME2, is on the list!!) as they become available.

thanks for you time on this,
-dc

@BKPepe BKPepe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Look, I added the AI label here just because it seems to me that almost all (maybe even absolutely all) of the comments were written by an AI agent, and it's like talking to a brick wall. :) I don't think we're quite at the point yet where one person controls one agent and the other controls another, letting them talk to each other and then waiting to see what we actually learn from the conversation.

And because of that, my motivation to do a review is kind of dropping. Especially when I write that test.sh isn't needed because it's enabled generically, and then you come back saying you looked at how I handled it and those test.sh scripts were added previously, and then the AI agent sees the change in the master branch and figures out that I removed them. :)

AI can be a good helper, but we just shouldn't abuse it.

And I absolutely don't understand why we should have two packages here that basically do the same thing. That's just maintenance hell. I've got the udspeeder package here, and now there's some other third repository getting in the mix.

Why even keep it separate when it's basically the exact same thing?

PKG_RELEASE:=1

PKG_SOURCE_PROTO:=git
PKG_SOURCE_URL:=https://github.com/connollydavid/UDPspeeder-simd.git

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Your forked repository has two extra commits compared to https://github.com/slartibardfast/UDPspeeder, and there's no pull request there. I mean, I'm looking at three repositories here. One is a completely unnecessary fork when it could have been solved with a patch ideally submitted to upstream, which would make it appear in the second repository, and since the second repository wants to get it into the third one, it would show up in this pull request. https://github.com/wangyu-/UDPspeeder/pull/354

Pretty convoluted, right? It's basically all about the exact same thing. And somehow I still don't know what exactly the advantage is of using your fork.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

hi,

yes, https://github.com/slartibardfast/UDPspeeder was for the initial idea.
but now it's complete, and had been requested on the bug ticket,
i've moved it to my this account & submitted it from here.

wangyu- doesn't appear to be available, or interested so i've now closed https://github.com/wangyu-/UDPspeeder/pull/354.

the only reason for including it is it takes us closer to line rate, at much lower cpu usage.

thanks, i'll take a look at the points you've raised.

p.s. when i seen CoPilot jump-in, i relaxed a little, i'll be much more careful in next commit

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

p.s. when i seen CoPilot jump-in, i relaxed a little, i'll be much more careful in next commit

Don't worry, there won't be a re-run of Copilot from my side, because I no longer have the credits to review pull requests here. 🤦 🙈

wangyu- doesn't appear to be available, or interested

@wangyu- ? :)

@connollydavid connollydavid Jul 5, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

thanks Josef, if there's anything more i can do, please let me know.

fwiw, I intend to maintain udpspeeder-simd fully from now, including for security / bug reports.
I use it daily on otherwise quite crappy hardware with simd :)

(thinking for udpspeeder itself:
connollydavid@e9b1d20)

Comment thread net/udpspeeder-simd/Makefile Outdated
Comment thread net/udpspeeder-simd/Makefile Outdated
Add a package for UDPspeeder-simd, a SIMD-accelerated fork of UDPspeeder. It
improves a high-latency, lossy link with Reed-Solomon forward error correction.
On targets with no SIMD path (for example MIPS) it falls back to a portable
word-width implementation, so low-end devices such as ath79 benefit from the
portable optimisations. It installs alongside the existing udpspeeder package
with distinct paths, so the two do not conflict.

This provides the performance-optimised variant requested in
openwrt#28562.

Signed-off-by: David Connolly <david@connol.ly>
@connollydavid

Copy link
Copy Markdown
Author

fixed PKG_BUILD_FLAGS:=no-mips16, ah I should've thought of that :( when i fixed the cflags. v sorry for the duff CI run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Add package AI It looks like this is issue or pull request was created by AI or it includes AI comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants