Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a Netty module #20

Open
pschichtel opened this issue Aug 5, 2020 · 14 comments
Open

Implement a Netty module #20

pschichtel opened this issue Aug 5, 2020 · 14 comments

Comments

@pschichtel
Copy link
Owner

Since the whole idea of "fluently integrating with Java NIO" kind of failed with the SPI being too closed down and troublesome in conjunction with epoll. Instead it might be a good strategy to implement this libary in the form of a Netty module. This would also probably allow to reuse netty's existing and battle tested epoll integration.

@pschichtel
Copy link
Owner Author

@mscheibler You suggested that you might toy around with this. Has there been any work done or is there any other related info you can share?

@mscheibler
Copy link
Contributor

Sorry to say that i did not take the time to do it.

My first though about this was to create several classes that implement the Netty interfaces for Channel, Codec, aso. as wrappers of the JavaCAN classes. So it would be possible to retain the JavaCAN library as zero-dependency-lib.

I will try to look into this next week to see whether this is possible. As you said it might be necessary to adapt JavaCAN to use the Netty epoll. But this would mean that JavaCAN becomes dependent to Netty.

@pschichtel
Copy link
Owner Author

pschichtel commented Aug 21, 2020

After reading up about epoll a bit recently due to the issues, I think it would not be a bad thing to rely on Netty's well tested epoll code instead of my strictly-thread-unsafe implementation. The state of EPoll is a mess and using it correctly turned out to be fairly tricky (even the JDKs impl is not properly using it in all scenarios). Going for netty would also instantly add support for kqueue on BSD (SocketCAN is available there as well) and maybe even io_uring in the future.

But I think (correct me if you find out this is wrong) the way the current JavaCAN implementation is structured, it should still be possible to provide it with our own selector interface as well as a netty integration.

So maybe the project should be restructured into 3 modules

  • javacan-core: all the general CAN interfacing code and native bindings
  • javacan-epoll: our own epoll-based selector
  • javacan-netty: netty bindings

@pschichtel
Copy link
Owner Author

@mscheibler in case you haven't already worked in any netty related changes: It might be a good idea to check @splatch's implementation for PLC4X out (see #22).

@pschichtel pschichtel changed the title Implement as a Netty module Implement a Netty module Aug 31, 2020
@splatch
Copy link
Contributor

splatch commented Aug 31, 2020

Our way in Apache PLC4X is really basic. I didn't fight for full support for NIO. What we did is just copying data between JavaCAN to netty pipeline via supplied "old IO" bridge from netty. Its not pretty, probably far from optimal, but it works.

@pschichtel
Copy link
Owner Author

I broke the library into 2 parts now: javacan-core and javacan-epoll for javacan 3.x, so we are now free to add a netty module only based on javacan-core.

@mscheibler
Copy link
Contributor

I spent a few hours this week looking into the Netty internals and it seems that i imagined it easier than it will be. ;)
However i got a encouraging comment from the Netty developers, see Using Nettys epoll implementation for Linux SocketCAN communication.
The Netty epoll implementations have a lot of IP stuff already included and i have to work my way down to understand the details. I will try to get some expertise from my colleagues, as this deep down Linux code is not my field of expertise.

If we succeed with the Netty integration, then it is most likely that the raw-channel implementation would be byte-stream based (as it is a SOCK_RAW on the Linux side) and the BCM is datagram (SOCK_DGRAM on the Linux side) based.
The framing would be done by a handler in the Netty channel pipeline.

@pschichtel
Copy link
Owner Author

Yep, just had a look at it myself. Many places around there are really not made for non-IP based networking. Other implementations of CAN in Netty all just implement OIO. From what I can see, we either have to implement a lot of stuff ourselves or submit patches upstream to make certain areas more compatible with non-IP based networking. CAN is pretty different to IP in many places.

@pschichtel
Copy link
Owner Author

I started a branch for the netty impl: https://github.com/pschichtel/JavaCAN/tree/feature/netty

@pschichtel
Copy link
Owner Author

Created an upstream issue: netty/netty#10524

@splatch
Copy link
Contributor

splatch commented Sep 3, 2020

From what I see the jnaCan uses also old io just to bridge payloads. I am looking forward to see epoll stuff coming but I can't commit myself to it. Our focus so far is getting subset of CANopen implemented in Apache PLC4X, hence your transport layer in its current form serves us entirely.
If you're fine with old-io bridge as a start then I am happy to move what I did with PLC4X (quite similar to jnaCan) to your project.

@pschichtel
Copy link
Owner Author

Sure, I'd want to implement OIO either way as a fallback, so if you want just create a pull request against the feature/netty branch.

@pschichtel
Copy link
Owner Author

I originally planned to include a basic netty module in the 3.0.0 release, but I will keep it out for now. The feature/netty branch still exists with the basic prep work I did a while back (mostly around configuration). I think all breaking changes necessary have been done in master, so adding the netty integration afterwards should not be an issue.

I will not personally work on the netty integration as I currently don't have a use for it, but I'm happy to review any PRs that come in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants