uspot: update to Git HEAD (2026-06-22)

[f00b4r0]

📦 Package Details

Maintainer: me

Description:
f5be8d0ee836 radius-client: fix printf format warning for uint64_t
815a28c49293 CMakeLists: set minimum required version to 3.10 for cmake 4.x
913980c0249f uspotfilter: switch to an RTNL active poll system

Fixes: f00b4r0/uspot#32

[f00b4r0]

@1715173329 forgot to ping you sorry. I'd like to get this in in time for 25.12.5 and 24.10.8 🙏

[BKPepe]

CI/CD complains about:

uspot: [warn] Binary /lib/bpf/uspot.o contains a hardcoded build path
/lib/bpf/uspot.o: ELF 64-bit LSB relocatable, eBPF, version 1 (SYSV), not stripped
uspot: [warn] Binary /lib/bpf/uspot.o is not stripped

And as well that there is not --version, but it can be overriden.

uspot: [warn] Version check (/usr/bin/uspot)
uspot: First 10 lines of the last output:
  Usage: uspot <CMD>
  CMD in:
  	debugon
  	debugoff
  	clients
  	log
  	restart
uspot: [pass] File /usr/bin/uspot-das is executable
uspot: [warn] Version check (/usr/bin/uspot-das)
uspot: First 10 lines of the last output:
  /usr/bin/uspot-das: unrecognized option: ?
  usage: /usr/bin/uspot-das -u <uspot>
   -u <uspot>	use <uspot> configuration
   -h		show this help message

All of this needs to be fixed before merging.

[f00b4r0]

All of this needs to be fixed before merging.

No.

[BKPepe]

Look, I'm actually fine with you disagreeing. CI/CD testing is in place here, and it's definitely not here to be ignored. I'm not going to fix this for you. As I wrote, the version check can be bypassed by actually testing the uspot, which is doable. An example of this is test.sh in this repository for the other packages, and at the same time, the version check can be skipped, but I already mentioned that in my previous reply.

What is definitely a no-go for me is that it contains a hard-coded build path and that the library isn't stripped. That's what the CI/CD is failing on, and it definitely needs to be addressed. Without that, we are certainly not going to merge it, let alone into stable releases. Especially since the issue you are linking to is a year old, and therefore wasn't a priority.

[f00b4r0]

Look, I'm actually fine with you disagreeing. CI/CD testing is in place here, and it's definitely not here to be ignored.

Is it not? You mean that schizophrenic CI/CD that half passes, half fails checks here? What am I supposed to trust?

I'm not going to fix this for you.

Neither am I.

As I wrote, the version check can be bypassed by actually testing the uspot, which is doable.

I don't care about this.

An example of this is test.sh in this repository for the other packages, and at the same time, the version check can be skipped, but I already mentioned that in my previous reply.

So I need to create a dummy file that essentially 'exit(0)' to pass a test? That's some impressive level of QA.

What is definitely a no-go for me is that it contains a hard-coded build path and that the library isn't stripped.

This hasn't been a problem until now though.

That's what the CI/CD is failing on, and it definitely needs to be addressed. Without that, we are certainly not going to merge it, let alone into stable releases. Especially since the issue you are linking to is a year old, and therefore wasn't a priority.

It was a priority, but tracking down the bug (unsuccessfully) and trying to work it out with @jow- in ucode (where it actually belongs) took that long because you know, $life.

This is a bugfix PR, I'm not introducing any new feature and I'm not in the mood to do so. I don't care about your CI/CD shenanigans especially considering I'm not touching the build recipe which was last updated by... YOU, so if you felt there was something wrong with it, why didn't you take the opportunity to fix it as well?

The build recipe hasn't changed in ages and I was never told there was anything wrong with it. If there is, then please do provide useful pointers, otherwise I don't take requests. This is opensource, software is provided "as-is".

kthxbye

[f00b4r0]

Also, the BPF module uses $(call CompileBPF) for build so if there is a problem with this it's not in uspot and last I checked, these modules are not meant to be stripped. Happy to be told otherwise, but all the packages featuring a BPF module use $(INSTALL_DATA) (or worse, plain cp), something a quick git grep will show you. So maybe the thing that needs fixing is your awesome CI/CD and not this package.

[1715173329]

Yeah that's right bpf objects should not be stripped in general. Unless you make some special compatibility changes otherwise it will be mostly broken after strip.

[f00b4r0]

@1715173329 thanks for thus confirming that all CI failures are false positives.

[f00b4r0]

@1715173329 thanks a lot. Could you cherry-pick into 25.12 please? It's a straight c-p, I just checked. I'll prepare a PR for 24.10 which is out of sync. Thanks!

[1715173329]

Would be better if you can do that. I'm a bit busy these days.

[f00b4r0]

I'm confused: I don't have commit rights. Do you mean you want a PR for the cherry-pick as well?

[1715173329]

Do you mean you want a PR for the cherry-pick as well?

Yes thanks.

[f00b4r0]

Done: #29835 and #29836
Thank you.

[BKPepe]

If there is, then please do provide useful pointers, otherwise I don't take requests. This is opensource, software is provided "as-is".

Exactly. Since this is open-source and software is provided "as-is", the review process and CI/CD are here precisely to maintain code quality and stability for all users.

The CI/CD works as intended and validates the actual state on specific platforms. By default, it automatically tries to detect the version using --version or --help. Of course, some packages (like yours) don't have this, which is expected, and that's why there is an option to bypass it using a test-version.sh file. However, since you haven't done that over the last 3 days, I went ahead and disabled the generic testing for your package in this PR: #29868

If you actually care about having some form of testing in place, you can always create a test.sh file like this:

case "$1" in
uspot)
    uspot 2>&1 | grep -q "Usage: uspot"

    uspot-das -h 2>&1 | grep -q "uspot-das"
    ;;
esac

But you know how it goes. It is what it is, and I'm not going to do all the work for you. :)

[f00b4r0]

What's the point of testing the "version" of a package built from a git hash checkout, though, I don't know.
I'm certainly not going to implement version display in uspot as it would require editing the script at every turn.
As evidenced today, the CI/CD only provided false positives for uspot, yielding a 0-SNR and missing the only actual problem in the code.

[BKPepe]

This conversation is getting way too heated on your end, so I won’t be participating in this discussion anymore. At the end of the day, the CI/CD will be fixed for you, so it's not "schizophrenic" as you claimed. And who knows, maybe next time you should implement it yourself to have some testing. :-)

Have a nice day.

[f00b4r0]

Heated? I'm merely stating facts.
You on the other hand boldly claimed that the package was broken and had "no-go" issues, which it hadn't (at least not the ones you were trying to get me to "fix"). That too, is a fact.

[1715173329]

Well, you can test something beyond the version, just like the examples from @BKPepe.

BTW I was thinking simply testing version is stupid as well (and maybe still nowadays), but it does help in some cases: what if the program raises segmentation fault? It builds, but does not work properly. For scripts, you may also face syntax errors. In this case the version check (or similar dumb checks) will let you know something is going wrong at least.

For "no-go" issues, @BKPepe is also working on it (see openwrt/actions-shared-workflows#123).

We don't have to let things get that bad ;)

[champtar]

@f00b4r0 running the binary with just --version or --help catch some breakage that we had in the past, it's not bullet proof and has some false positive for sure, but it's worth it IMO.

Yes fixing the false positive is some extra work, and it's nicer if the package maintainer takes care of it, so instead of just skipping the check with an exit(0), you can implement something meaningful like loading the ucode files to catch when it's completely broken. It can also catch missing dependencies (maybe you use some packages that are present because they are dependencies of luci)

Heated? I'm merely stating facts.

FWIW the whole exchange reads pretty confrontational to me, starting with a single No. sets the tone, and being dismissive about the CI is unnecessary.

eBPF binaries must not be stripped (at least not with strip), but having hard coded paths is often a security issue, and nobody has confirmed it's a false positive, so even if the fix needs to happen somewhere else, it would be great to not completely ignore the CI just because the problem was already present, or because 'it works (TM)'.

[f00b4r0]

@1715173329 I certainly agree that proper testing is good, although here as you could see the uspot-das daemon and bin/uspot executables worked properly (even though failing the version check): the SNAFU was in a separate service that wasn't tested.

My point is merely that when you bombard contributors with false positives or mixed signals (half passing / half failing), the confidence in the CI/CD is lost, and I think that's counterproductive. As for testing uspot (or other services I suppose) in a generic way, this particular error would have been caught by simply running the init script to start the service (although I can see cases where conditional loading in ucode could significantly delay error discovery). I don't know however if that's feasible in a test scenario.

Lastly, it is indeed my opinion that blocking a merge and peremptorily requesting that changes be made to software or package that is already accepted and where the proposed changeset consists only of bugfixes to satisfy some arbitrary test scenarios (as was done here) is also a no-go in my open-source book, especially when no actionable pointer (to examples or proper documentation - btw is there any?) is provided. It's perfectly legitimate to suggest them, but blocking a such a bugfix merge on that basis, in particular when said changes are in deed not warranted (and in fact plain wrong), is not acceptable IMHO.

I believe the above statements are reasonable and do not constitute a heated argument, at least that is not how they are intended. I should probably mention that the only reason I care about properly updating uspot in the OpenWrt packages repo is because of my open-source ethos: it brings me nothing. So at the end of the day if that becomes too much of a hurdle, I'll just quit. I also believe I'm not alone sharing that state of mind, looking at various PRs and forum posts.

My 2c.

[f00b4r0]

@f00b4r0 running the binary with just --version or --help catch some breakage that we had in the past, it's not bullet proof and has some false positive for sure, but it's worth it IMO.

Sure. Then at least don't fail the test if the parameter is not recognised? Neither are mandatory in any shape or form.

FWIW the whole exchange reads pretty confrontational to me, starting with a single No. sets the tone, and being dismissive about the CI is unnecessary.

I've addressed that in my previous reply I think.

eBPF binaries must not be stripped (at least not with strip), but having hard coded paths is often a security issue, and nobody has confirmed it's a false positive,

That is not how I read openwrt/actions-shared-workflows#123 (comment) though?

so even if the fix needs to happen somewhere else, it would be great to not completely ignore the CI just because the problem was already present, or because 'it works (TM)'.

Again, conflicting (and plain incorrect) CI output is totally confusing. If I had implemented the requested changes, the package would have passed the tests and yet would have become completely unusable. I don't think that's a Good Thing™

Furthermore I'd like to point out again that the package is using default build recipes so when CI complains about the output of said default build recipes, it might be a good idea not to shoot the messenger because then it's really frustrating for said messenger.

I hope this makes sense.

[champtar]

Everyone acknowledge that the CI is not perfect, nor are the repo maintainers, but to put some perspective, this whole discussion started because you are contributing to 1 of 2 packages that package eBPF binary in this repo (looking at CompileEBF), so you hit a super specific false positive. The striping check is pretty much 100% reliable, just not relevant for eBPF, which not a lot of people know about.
With that in mind, asking you to fix the striping without further investigation was a pretty normal ask.

I fully understand you pushing back against having to fix false positive, or bugs in the toolchain, but IMO package maintainers need to have a quick look at the CI failures and triage / comment / ask for help if needed, it doesn't take much time if the load is shared, and we can't progress if everyone starts to just ignore the CI feedback.

BTW @BKPepe is actually replacing all the formalities checks with a super fast webhook which should improve the contributor experience a lot, ie being able to iterate quickly on the trivial stuff, with feedback as comment.

Regarding the version check, what is the difference between an unknown argument and a typo that break argument parsing ?
Making it opt-out is the only way to gain wide adoption IMO, but yes, maybe how it's being rolled out could be improved. I don't think asking each maintainer to opt-out as needed the next time they touch their package is too much, but a link next to the error in CI to some documentation might not hurt.

WIthout looking at the eBPF binary (on my phone only for some days) I'm still not convinced having hard coded build path is correct, even if it might only be a reproducibility issue in the end, it's worth checking at some point.
Again not your problem to solve, but all help is welcome (running readelf with the right flags and looking for build_dir)

To conclude we all want the best for OpenWrt, let's help each other any way we can.