[NETCONF-674] Eliminate use of AsyncSshHandlerReader Created: 25/Apr/20  Updated: 05/Jun/20  Resolved: 05/Jun/20

Status: Resolved
Project: netconf
Component/s: netconf
Affects Version/s: None
Fix Version/s: Aluminium, Sodium SR3, Magnesium SR2

Type: Improvement Priority: Medium
Reporter: Robert Varga Assignee: Robert Varga
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File 10K_sshd220_asyncread.png     PNG File 10K_sshd220_asyncread_bytearray.png     PNG File Screenshot_20200513_225354.png     JPEG File _NETCONF-674__Eliminate_AsyncSshHandlerReader_-_OpenDaylight_JIRA.jpg    
Issue Links:
Blocks
blocks NETCONF-571 Rework SSHD integration Confirmed
Issue split
split to NETCONF-699 Eliminate use of AsyncSshHandlerReade... Resolved
Relates
relates to NETCONF-686 CSIT regression in netconf-csit-1node... Verified

 Description   

AsyncSshHandlerReader attaches to public client-side interface, issuing asynchronous reads. Each such read requires a buffer into which it is going to be fed.

This unfortunate design is a legacy from previous SSH library we used and is not as efficient as we can go.

Specifically, we need to use a relatively small buffer, because we can have a large number of devices – hence 2 are going with 2KiB. This means there are really two levels of queueing going on:

  • sshd-internal buffering from socket to internal client queue
  • internal queue to our async read

Furthermore we have NETCONF message reassembly sitting on top this queueing, which defaults to at most 16 ByteBufs before coalescence. The interplay of these is very unfortunate:

  • sshd reads in 32KiB chunks (based on NETCONF-571)
  • this is then fragmented to 16x2KiB to fit into async reads
  • this is then reassembled to a 32KiB bytebuf

This results in aweful lot of copying for NETCONF message larger than 32KiB, with the hurt being exponential with the size of messages.



 Comments   
Comment by Robert Varga [ 25/Apr/20 ]

The solution is a tighter integration with SSHD, where we subclass SshClient to allow us to tap into internal dispatch of it. Specifically we expose a specialized NETCONF subsystem getter, which feeds directly to a target Netty channel. This integration can tag directly to receive path of SSHD, hence it completely eliminates internal queuing as well as fragmentation, resulting in:

  • sshd reads 32KiB chunks
  • this is then reassembled to a composite bytebuf

This means that the number of chunks in reassembly drops significantly, as we no longer fragment on receive (or at lest not as much, but still NETCONF-571 applies) thus the reassembly buffer can grow to as much as 16x32=512KiB before it is coalesced.

Comment by Jamo Luhrsen [ 13/May/20 ]

rovarga I see one patch associated for this jia is there for
master and cherry picked to sodium. I don't see it for magnesium. Should this be there as well?

I'm also wondering why this ticket does not have that patch in it's gerrit reviews section. the commit message
directly calls out this NETCONF-674 and links to this ticket. some bug in jira maybe... werid.

Comment by Robert Varga [ 13/May/20 ]

You need to select 'Show All Reviews' in the right-hand '...' menu of Gerrit Reviews.

The patches will land in stable/magnesium after Mg SR1, as they are not critical and have landed after the freeze was in effect.

Comment by Jamo Luhrsen [ 13/May/20 ]

yeah, I have it selected as "Show All Reviews"

Comment by Robert Varga [ 13/May/20 ]

Comment by Jamo Luhrsen [ 13/May/20 ]

thanks. Gotta be a problem on my end. oh well.

Generated at Wed Feb 07 20:15:36 UTC 2024 using Jira 8.20.10#820010-sha1:ace47f9899e9ee25d7157d59aa17ab06aee30d3d.