Support POE on hasivo devices (s1100wp-8gt_se)

Hardware Questions and Recommendations
61

I was now trying to move the code to ubus... It took some time, but I can at least read values. The problem is, that ubusd does not run with root rights and when I try to set pse configs, I ran into missing CAP_NET_ADMIN permission: " Result: kernel returns EPERM (-1) ". I couldn't find any information on how to overcome this. Some others are suggesting another user space daemon that acts like a bridge between ubus and the kernel netlink, but this is again similar to netifd.

Potentially I could keep the GET/LIST piece for PSE operations in ubus and move the SET to netifd. But then the entire process exists in two places which makes it complicated as well.

Don't get me wrong, I am open to change it, but the netifd solution works already...

I thought of creating a PR to get feedback from other folks as well.
Wdyt @bevanweiss ?

cheers

62

I think there needs to be explicit ACLs added for ubus (in a json file) to allow the write (SET) usage.
I think the EPERM error might be returned from ubusd/rpcd here, rather than the OS.

From google AI
The ubus ACL (Access Control List) module in OpenWrt is a security feature managed primarily by rpcd (the OpenWrt RPC daemon) that restricts which users or services can access, call, or modify specific ubus objects and methods. These ACLs are defined in JSON files located in /usr/share/rpcd/acl.d/.

Also an example file looks like:

{
    "myapp": {
        "description": "Role for my app",
        "read": {
            "ubus": {
                "system": ["info"]
            }
        },
        "write": {
            "ubus": {
                "network.interface": ["up", "down"]
            }
        }
    }
}
63

Thanks for quick reply!
From what I researched, you can't set kernel permissions via ACL. It is rather "can user foo call network.interface.status", but if ubusd sends something like ETHTOOL_MSG_PSE_SET, the kernel checks if the sending process (ubusd) has the CAP_NET_ADMIN permission.
To grant ubusd those permission, something like:

setcap cap_net_admin+ep /usr/sbin/ubusd

could work, but a) it requires setcap in the image and b) it looks to me somehow hacky. Besides more permissions are required, then really needed.

I researched some other methods, with help of google, forum and LLMs:

1) build a privileged helper daemon:

  • runs with CAP_NET_ADMIN or as root
  • registers own ubus objekt (pse)
  • handles netlink PSE operations

cons: another binary, and more logic to review
pros: less privileges to ubusd

2) use rpcd

  • add PSE control to a rpcd plugin
  • runs already with more permissions

3) UCI + hotplug script

pros:

  • privileges are existing
  • simple and only scripts, but also not really native
    cons:
  • not real time, requires reload

4) ethtool wrapper in init.d

  • simple shell scrit

pros:

  • easy
    cons:
  • rather hacky and not deeply integrated

and last but not least the existing :
5) netifd variant
pros

  • PSE controls power on phsyical ports, somehow similar to device/interface management
  • it handles also link settings, MTU etc
  • UCI integration simple
  • runs already as root
  • already exposes network.device + interface.* to ubus
  • no extra daemon needed

cons:

  • complex mature codebase ...
  • add libnl dependency to netifd

Happy to get your thoughts... I am open to input :slight_smile: I posted that also in the PR, so maybe some others share feedback as well.

64

pinging also other folks for input: @Noltari @nbd @systemcrash @robimarko @danpawlik @andrewjlamarche @plappermaul @jonasj @svanheule ...
And sorry for bothering you on Sundays :wink:

65

I mean, if it could be added to netifd that would be great since ethtool was expanded to support PSE

1 Like
66

We will continue to follow the netifd path ... :slight_smile:

1 Like
67

Iā€™ll try to get the Hasivo HS104 driver stuff sorted out tonight, so that I can put in the PR for it.

I have received a DS18B20 one-wire thermometer and a SOT223 NFET to have a go at testing the other STC8 features also (thermal sense / fan control). But my board is also missing the resistors to link to these devices, which I think are 0402 size.. and I donā€™t have any of these. Iā€™ve got some on order, will be interesting to see if I can manage to hand solder them.. thankfully they donā€™t have to be pretty.

1 Like
68

@OpenWrtAWESOME I think weā€™re missing a patch from upstream.

When I try to build the PSE_REGULATOR stuff in OpenWRT itā€™s throwing a build error that itā€™s unable to find a definition of regulator_request_power_budget and another.

It looks like my previous offline code for the HS104 targetted a different kernel version also, so Iā€™ve got to do a little more correction of it.

69

That's correct, for full working version that is required...
You see it in my branch here: https://github.com/openwrt/openwrt/commit/0721bf3bb7f0f171e60cec0cb6df1b1e526574f1#diff-1e76c839a78f82c0e55ef16ab0f25314f2d658bf583948d0e3127a29a793d55d
This you can use... The branch was working for POE with all the details, except regulating power. But I think we can do that later - it's not really required for simple use ...

So I didn't focus on that much for now...

netifd, makefile and pse were more important to me for now.

but it's no big deal I guess:-)

70

nice! sounds great. I would still suggest to get the first STC PR merged, so we have some babysteps done. The other pieces can go to the next commit :slight_smile:

Btw, I ordered a s1300wp-8xgt-4s+ , this should potentially have the same chip and fans etc.

71

Iā€™m seeing the build error even when just trying to bring in the base kernel PSE_REGULATOR items.

Iā€™ll put in a fix PR for it tonight, since we shouldnā€™t leave the kernel broken for standard CONFIG options.

I donā€™t believe I have any actions left for the STC8 PR. Just waiting on @hauke (or other) to have some time to re-review and merge I believe. Although that PR does show a complaint about the committer email being a Github noreply.. so maybe I need to force push myself, rather than letting Github to the rebase and force push. If so, thatā€™s a tonight activity..

1 Like
72

I think there's still an architectural issue we'll encounter around the pse-pd stuff, and the LED triggers.

From my testing there's no background polling of the pse-pd power states. Which means that things like an LED trigger for pse-pd 'delivering' will only fire when something actually reads the pse-pd state, and the I2C read is triggered.
This is probably another one of those 'wish this was handled in upstream Linux already' situations.

A hacky interim solution is likely to just have something in user-space doing a periodic polling of the various pse-pd activated ports. I don't think this is something that should exist in the HS104 driver itself though, since that then diverges from the other existing pse-pd drivers.

73

I understand your argumentation, but i don't get why this should actually fail, because in my test branch the mentioned case works. I mean the LED switches on and off when I plugin a PSE device (and this is based on your stc8 PR) . So what should the user-space script replace or cover exactly ?

74

Tomorrow night I'll put together the package stuff for the Hasivo HS104 PSE-PD stuff, and put in the PR for it (assuming it all works in my testing).
I'll also look at putting together packages for the other PSE-PD chips that are in Linux mainline, so that other switches can use them.

I think got enough data sheet stuff for two other PSE-PD chips also.
The IP804AR (from another forum post), and the RTL8238B (and maybe others in that family based on some info from the new switch I have)
image

However I've only got HS104PTI switches currently... so others would need to do testing for me (but that should be easy once the rest of the PoE stuff goes in).

Will need to see once we've got the rest in whether there are any issues with the periodic polling.

2 Likes
75

Okay makes sense, I have also now a working version based on your latest stc PR ...I can push it later, so you can compare.
But I don't want to take that PR away from you, as you did the fundamental work :wink:

  1. The ethtool issue is now also resolved I assume, please check this https://github.com/openwrt/openwrt/pull/22020

  2. @bevanweiss Would you mind pinging some other folks in the netifd PR? There is not really much activity, and this is another blocker: https://github.com/openwrt/netifd/pull/65 :slight_smile:

  3. the periodic pulling I still don't fully get, because for me everything works (power updates, state changes directly when I plugin or plugoff a cable, deactivating the port via ubus, ethtool as well)

76

@bevanweiss Here you find my latest version... The major functionality is fully working: ethtool, netifd, ubus, luci control and power measurement.

I applied the same logic, like you did for the STC. Maybe it reduces your effort to do things twice?

Happy weekend.

77

@OpenWrtAWESOME I think I see why you're seeing different behaviour to me.
target/linux/generic/files/drivers/net/pse-pd/hasivo_hs104.c Lines 547-550

You've got polling work happening within the PSE-PD driver instance. Which doesn't align with how upstream linux does things (at least from when I looked at it last). Upstream doesn't have any polling loop within the pse-pd drivers.
If we try to add that to the Hasivo HS104 driver, I very much doubt it will get into linux upstream.

I see there's a few other discrepancies between your repo and what mine has, so hopefully I can work through them tonight also. Maybe my interpretation of the HS104 datasheets was wrong..

78

Definitely looks like I got the HS104 port allocations wrong for the discrete flag registers...

48= 1 Port 1 is allowed to open and other ports are closed

So (0x08 >> port_num) seems like the correct logic (rather than the BIT(port)).. I'll dig more tonight..

79

If you are not faster ... next weekend I have time again :slight_smile:

Btw some progress:

netifd changes [merged]
luci changes [merged]
backports [multiple merged]
mfd stc8 [progress]

And the first driver is on the way :slight_smile:

80

@bevanweiss I took now some time again to understand the other drivers better. I see were you coming from. My approach is kinda "hacky" in comparison to the others:

Summary:

  • there is zero pulling in other drivers in the upstream
  • status is retrieved via ops callbacks when ethtool asks
  • for async events like over-current, over-temp , pd detection the pattern is IRQ driven, so for example devm_pse_irq_helper() is used
  • pse_core uses a work_struct internally for notification delivery

Some potential solutions to avoid the polling every 500msec:

  1. Add some LED trigger support to pse_core itself, so it's not in every driver. It seems the code already calls pi_get_admin_state() and pi_get_pw_status() on every ethtool query and knows about enable/disable events. It could register triggers like pse:pi0:delivering generically for all PSE controllers -> This would require some patches in the upstream, but I don't know if that is a solid approach...But it looks at least somehow clean.

  2. Some Userspace daemon again, that checks for ethtool --show-pse ...and writes to /sys/class/leds/*/brightness. That would be simple, but I personally do not like it much (additional different code to handle etc)

  3. Separate Kernel Module for this device that registers LED triggers and queries PSE status. But also this is additional complexity...

  4. Additional openwrt patch for the LEDs? This would keep the hs104 clean for a upstream commit and we could patch it after in openwrt to trigger the LEDs?

  5. We could keep it in the stc8 MFD driver: So another hs104 register poll and update the LEDs. Also semi nice, because it creates cross-driver hardware deps.

Additionally do you have any other ideas on potential IRQ triggers for this hardware?
Thoughts?