Plans on the horizon

[jacobslusser]

Hello @Rob-Hague (CC @WojciechNagorski).

As mentioned in my recent PR, I have a few focus areas that I thought I might start working on and wanted to give you a preview. In no particular order...

Including ILogger support and more verbose tracing options
This is initially what I came to the project source code looking for. Where I work, we make fairly extensive use of the SFTP features of SSH.NET (thank you) and occasionally connections fail and I would be interested in getting more information about the cause. So I'm looking to update some of the exceptions, add ILogger uses, or both.

Performance improvements: heap allocations
I've only just started doing some profiling but can see there are lots of areas we are allocating byte[] arrays that could benefit from using ReadOnyMemory<byte> or pooling. For example, Message.GetPacket popped up on a few of my early profiles that could benefit from some optimization.

Performance improvements: async all the way down
In the aforementioned profiling, I also spotted SocketAbstractions.Send and have a SendAsync version that I am testing and will eventually submit a PR for. Lots of other places that could benefit from Async version of methods, but as mentioned in our discussion here, some of which will need discussion with the group because they could make breaking changes in the API or require a lot of conditional compilation.

AOT compilation support
I found that Message.WriteBytes is doing a lookup of MessageAttribute on every packet/message write and contributes a non-trivial amount to the overhead of each message. This got me thinking about not only caching/eliminating the lookup for this attribute, but for AOT down the road. Much longer term goal, but I ran the AOT analyzer to identify problem areas and it doesn't look too difficult.

Documentation
I would be happy to update some of the docs (or add them) and/or the wiki with some more information.

So that's what has popped into my head in the first few days of spending time with the code. I'm sure I will come across more areas that will look like fun problems to work on.

Some question that might help me and others:

• Is there a place where roadmap like work is tracked? I'm assuming from Rob's Infrastructure work items issue, that using the Issues is the approach for documenting roadmap like work and discussing in.
• Do we like the wiki for documentation or do we want to create a docs/ folder with markdown files? I tend to prefer the second option since it lives with the repo instead of with GitHub.
• What is our release cadence and methodology? I'm already seeing possible breaking changes to the API which we would only want to entertain in a major version bump, but not sure when or how we decide that.

[Rob-Hague]

Thanks for the write-up!

Including ILogger support and more verbose tracing options

There is some diagnostics using (legacy) TraceSource (see the wiki and #1263). Using Microsoft.Extensions.Logging would be a good upgrade.

Performance improvements: heap allocations

Agreed, sounds good.

Performance improvements: async all the way down

A good thing that I have also been thinking about lately. We should eventually look to replace all the Begin/End ("APM") methods with truly async implementations (the current implementations mostly just dispatch onto the threadpool). For this we need to make Session.MessageListener async, and we need to create Session.SendMessageAsync. Session.SendMessage uses locking, so to make this asynchronous we should probably convert that lock into a SemaphoreSlim which has WaitAsync (the motivation behind #1265 🙂) or a type which wraps SemaphoreSlim and looks like a lock (I have a small prototype for that).

AOT compilation support

See #1216 which fixes the warnings. Replacing the MessageAttribute system could be good for performance. In effect that is just specifying the message number, which could just be e.g. defined on an interface.

Documentation

Great. The README could perhaps also get a refresh with more examples. e.g. by following the template of the Microsoft.Extensions.Logging package linked above.

Is there a place where roadmap like work is tracked

No, but this discussion seems as good as any

Do we like the wiki for documentation

I have no preference here (if someone is motivated to write documentation, then whichever format makes them the most motivated is good for me). I will defer to others on that one.

What is our release cadence and methodology?

I don't know if there is one. For some background, the library has been nobly stewarded by approximately 1 maintainer for most of its lifetime, and has only recently started to get a bit more love. So things are not so well-defined in that sense. But from my short experience, breaking changes are reasonable particularly in the lower-level areas.

---

From my side, my current list of interests (I tend to jump between things - I know it's bad - I'm not reserving any for myself):

• Investigate/stabilise random failing tests in CI Unstable integration test Test_Sftp_Multiple_Async_Upload_And_Download_10Files_5MB_Each #1253 / Deadlock in InternalUploadFile #957
• Add support for OpenSSH certificates (I have a branch for this)
• Send client KEXINIT first (see discussion in Allow the client to initiate the key exchange #972, I'm running a version of this in production) Send the client key exchange init in Connect #1274
• Add support for aes-gcm cipher and etm hmacs Support ETM (Encrypt-then-MAC) variants for HMAC #1316 (I think these will be part of the same change abstracting the packet serialisation logic from Session.SendMessage. There is an existing PR for aes-gcm)
• Async as discussed
• Replace cipher logic with BCL implementations
• Fix ShellStream Fix a few issues with ShellStream #1322 and PipeStream (there are existing PRs for this)
• Null-annotate the library
• Replace BigInteger with BCL version (I have a branch for this, it's waiting on the cipher BCL implementations so that we don't need the custom methods defined on our BigInteger)
• Replace the dictionaries in ConnectionInfo with a type that guarantees order
• Relax the endpoint logic (remove DNS lookups?) in the forwarded ports/SOCKS implementations (something like Possibility not to specify BoundHost when using ForwardedPortRemote #1234 On SOCKS5 proxy: set hostname, not always IP #1072)
• Merge (or at least add) the "Shared" tests project into the solution

[fmargueirat]

It would be great to see aes-gcm ciphers in main branch. I saw there are some PRs for it, but I was not able to get them to work, after downloading the code and compile it locally. I'm a fairly noob when it comes to GIT (I usually download approved/latest releases), so I am probably missing some steps.

[Rob-Hague]

It's possible those PRs simply don't build (I didn't check). The library these days does not require anything special to build it, but that might not have been true when they were last updated (quite a lot has changed regarding the build "infrastructure" since then)

[fmargueirat]

@Rob-Hague, I think the PRs are for older versions and the code on which they were forked eventually changed and that's why they are not building. I still think adding the support in the latest version, would be a good idea.