Coder Social home page Coder Social logo

Comments (16)

negativekelvin avatar negativekelvin commented on July 20, 2024

https://github.com/negativekelvin/mbedtls/tree/mbedtls-2.12.0-idf-dynbuf

from lws-esp32-factory.

lws-team avatar lws-team commented on July 20, 2024

@negativekelvin dude.... thanks for stepping up again.

Personally I wrote the original stuff to get it into mbedtls... it wasn't my concept to be in the wilderness doing it over and over OOT because the project refuses to accept it. I genuinely thought if someone provided it, with the advantages, since ESP32 is not special in terms of being low-end enough to be needing it, the mbedtls guys would see why it is needed on stuff like cortex-M (+ RISC-V...) and then it would simply be a contribution to them and we can all be happy.

Unfortunately that isn't the culture in the mbedtls project. While clearly at 10,000ft there is management, productivity, and forward movement on that project overall, in terms of being able to get their head around non-trivial external contribution for the very low end they are a complete failure: to their mind external contribution is just some burden they have to scrape off their shoes. Not just this series, the whole mbedtls PR queue with their calculated "go-nowhere" comments. In private emails they understand the situation. They are in an era where they just won't work with anyone for real work without an @arm.com email address 'cos NIH.

The fact a third party is willing to do serious work uplevelling the series repeatedly over time is further evidence mbedtls are failing to understand what they should be doing to support low-end devices, including arm devices for tls... since that's what their project is supposed to be about...

from lws-esp32-factory.

FredrikFornstad avatar FredrikFornstad commented on July 20, 2024

Andy
I fully understand you and I agree that having this included upstream would be really nice. But if the mbedtls team does not want it, I guess the second best option is to ask the espressif-team to have it as part of the esp-idf distribution? Has this been done already?

Recently, there seem to be some esp-idf patches introduced into the mbedtls-lib that ships as default with esp-idf, for instance:
mbedtls: configurable options for controlling dynamic memory allocations

Related to the above patch I also found a thread on the forum about lack of memory due to tls and some discussion around it:
Any way to override MBEDTLS_PLATFORM_MEMORY define?

I guess that if your patch could be introduced in esp-idf, then you will not need to include a mbedtls component in lws-esp32-factory (and test-server-demos) anymore?

from lws-esp32-factory.

negativekelvin avatar negativekelvin commented on July 20, 2024

There were only minor tweaks from the previous patch and I was just playing around with it. I think it would be more palatable upstream if it had fewer line changes and was a compile time option which would not be too hard to massage it into. But who knows.

from lws-esp32-factory.

negativekelvin avatar negativekelvin commented on July 20, 2024

am getting stack overflows on test-server-demos though

from lws-esp32-factory.

lws-team avatar lws-team commented on July 20, 2024

@FredrikFornstad those patches seem related to managing existing allocations from different pools. The dynamic stuff is a whole other thing where the connections track their own sizes at runtime, not by a compile-time constant.

The patches you mentioned in the first post are hacks no better than the original build-time hack mbedtls had for setting both directions size limit at once. Different individual tls connections require different rx and tx buffer sizes (cf -factory firmware upload mandating 16K RX) but all you can do is pick one number globally at build-time. Too small and the connections fail due to payload corruption / truncation, since these hacks are illegal for tls protocol.

It's not actually they don't want the feature, the have written privately they may implement it themselves eventually. If espressif find it useful to refer to my patches they're welcome to do so. But I think their current work on top of mbedtls in idf is in the way of platform adaptation, and that 'pays its way' for them to maintain and uplevel it OOT. Even though the hack-on-a-hack patches taken in mbedtls recently came from espressif, they did not have them in idf mbedtls in the two years mbedtls ignored the PR from espressif AFAIK. Therefore I guess espressif don't want the burden of dragging the much larger dynamic alloc patches around OOT either. The solution is mbedtls take them or recreate them, which I expect we'll see in a couple of years, since the utility of it is clear.

think it would be more palatable upstream if it had fewer line changes and was a compile time option which would not be too hard to massage it into. But who knows.

The patch is a major, invasive change to how the library works. It's no more invasive than it has to be to do what it does. And no larger nor more invasive than patches mbedtls applies on itself all the time. Why is it easier to try to explain this as my fault when you can just look at their PRs and see hundreds all ground to a halt the same way?

am getting stack overflows on test-server-demos though

This is solved by giving it more task stack or so?

from lws-esp32-factory.

negativekelvin avatar negativekelvin commented on July 20, 2024

No I just mean employ some psychological tricks by making the patch look small. Small patches are cute and friendly.

Trying 12k stack.

from lws-esp32-factory.

lws-team avatar lws-team commented on July 20, 2024

... the size of the patch is not relevent to getting it into that project. The projectgus hack-on-a-hack patch was a few lines, got comment and sat there for two years until from reading the comments, evidently something happened via other channels making being seen doing something urgent for mbedtls.

They're not dumb, they don't need 'psychological tricks'. They simply have a policy (or had, I studied their commit log about 9 months ago which was unbroken arm.com authors going back 18 months then) which is to only pretend to operate as a FOSS project wrt contribution from outside arm.

from lws-esp32-factory.

negativekelvin avatar negativekelvin commented on July 20, 2024

You're right but all we can do is play games outside the ropes.

high water mark at 508 with 12k stack so far

This band-aid also required negativekelvin/libwebsockets@5c69b86

from lws-esp32-factory.

negativekelvin avatar negativekelvin commented on July 20, 2024

Just to humor myself:
Showing 5 changed files with 916 additions and 727 deletions.
to
Showing 5 changed files with 445 additions and 50 deletions
https://github.com/negativekelvin/mbedtls/tree/mbedtls-2.12.0-idf-dynbuf-cute

from lws-esp32-factory.

lws-team avatar lws-team commented on July 20, 2024

The original series I offered to mbedtls you can actually compare like this:

$ git clone https://github.com/lws-team/mbedtls
$ git checkout origin/dynamic-ssl-buffers
$ git diff d629411212ab85772a8e682695edb2b01fc63d52.. --stat
 include/mbedtls/ssl.h                |   68 ++++----
 include/mbedtls/ssl_internal.h       |   18 ++-
 library/ssl_cli.c                    |  156 +++++++++---------
 library/ssl_srv.c                    |  170 ++++++++++++--------
 library/ssl_tls.c                    | 1129 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------------------------
 tests/suites/test_suite_ssl.function |   12 +-
 6 files changed, 887 insertions(+), 666 deletions(-)

... in other words, I add a net of +221 lines, including the needed test adaptations. You're adding +395 lines. Cute is in the eye of the beholder, but that's more bloated, isn't it?

I think it's great you want to meddle with my patches and keep them alive, either way. But when you maintain a large project, it is actually a good thing someone turns up wanting to refactor internal stuff that needs refactoring as part of other work, not a ding.

Still, I'll have no (additional) complaints if mbedtls turn out to want your "don't do the necessary refactor" and add more lines plumper version... it'll be a win either way. But don't hold your breath unless you're planning on working for arm and getting the magic ticket email address.

from lws-esp32-factory.

negativekelvin avatar negativekelvin commented on July 20, 2024

Well net is one metric but I am concerned with how long is the diff and how easy is it to grok and will that make it easier to apply it to future versions.

from lws-esp32-factory.

lws-team avatar lws-team commented on July 20, 2024

I am concerned

Well, I was "concerned" I couldn't actually use esp32 + mbedtls usefully. I took action about it and wrote from scratch a series aligned with mbedtls that solved it, and tried to get mbedtls to accept the patches.

Actually if mbedtls, the upstream, don't take the series (or as I found, are so up themselves with NIH they will wait their own sweet time to reinvent it with their own name on it) then there's no good solution. It's very expensive to keep stuff going OOT, which is why I am happy you seem motivated to do it. Right now I have no reason to spend any more hours of my life on esp32 or mbedtls.

If you're "concerned", the only useful way to show that is to press mbedtls (honestly... not espressif) to provide a path to accepting this functionality... I don't mean "take the patches" I mean actually have the discussion about how to get from having the working patches that pass the selftests that I provided, which is certainly a very strong POC, to actually accepting some other version of them.

As I have repeatedly said, I have private email with the maintainer and related arm guys they offer no path to co-work on getting this functionality in. They simply won't work with external guys. If you're "concerned" you can be more effective by stopping making up imaginary blockers like not enough "cuteness" and pressure mbedtls about their actual outrageous behaviour.

from lws-esp32-factory.

squonk11 avatar squonk11 commented on July 20, 2024

I was trying to get lws-esp32-factory with the PR #48 work. Unfortunately without success. Is there any plan to restart your activities in order to make it work again?

from lws-esp32-factory.

roysG avatar roysG commented on July 20, 2024

Any update with this for the latest version?

from lws-esp32-factory.

lws-team avatar lws-team commented on July 20, 2024

Latest version of what... lws works fine with mbedtls 3.

Lws work fine with (yesterday's master) esp-idf, two kinds of esp32 and now also c3 (riscv) are built, flashed and run in CI.

If you mean -factory, no, there were no real contributions coming back and I don't have any reason to keep pouring time into it for free. It was also too esp-idf specific, in lws now there are generic lightweight apis for the peripherals and networking and the living code for esp32 uses those, but it doesn't have the UI polish of -factory... eg for wrover

https://libwebsockets.org/git/libwebsockets/tree/minimal-examples/embedded/esp32/esp-wrover-kit

... if you are asking because you want to carry the work forward as opposed to just leech it, those apis are the way forward.

If you mean my mbedtls dynamic allocation patch series, I don't have any reason to play catchup forever with mbedtls codebase while getting the cold shoulder for free either. It was very painful dealing with mbedtls and the peanut gallery. Mbedtls are supposed to have new OS patch governance and a mailing list since my attempt to cooperate with them, but they currently have 224 PRs open going back years, and 700 issues, nothing has changed. They are very good at what they do, their "source is open" but what they do is not about taking in serious external contributions from third parties. ... if you're asking because you want to carry that work forward, my advice is either go work for arm, or take a long walk and go spend your remaining time on the planet doing something more worthwhile.

from lws-esp32-factory.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.