Xilinx-CNS / tcpdirect

AMD TCPDirect ultra low latency kernel bypass TCP and UDP implementation for AMD Solarflare network adapters, to be used with corresponding versions of Onload®️ at https://github.com/Xilinx-CNS/onload. The stable branch is currently `v8_1`.
22 stars 16 forks source link

ON-160xx: Proposed release notes addition mentioning onload-dev #47

Closed dchadwic-xilinx closed 1 month ago

dchadwic-xilinx commented 1 month ago

Any thoughts?

pcolledg-amd commented 1 month ago

Headers are required, package is not. Could this be reworded to be unbiased towards the deb/rpm deployment method?

Also, if calling out onload-devel (RPM) we should mention onload-dev (DEB) too.

dchadwic-xilinx commented 1 month ago

@abower-amd

as building from source is completely new, the user will have to follow the installation instructions below which clearly already call out the need for the devel packages

My expectation is that customers who are used to the old way will now be wondering where exactly they should be getting this onload-dev package from. They have not yet learned that this is provided using their current rpmbuild / debuild method, and as a result they may not notice the package sitting among their Onload package binaries (which is the mistake I made!)

dchadwic-xilinx commented 1 month ago

@abower-amd

there's too much overlap with the first section on "Public Onload/ef_vi control plane API".

  The Onload control plane in this Onload-9.0.0 is presented via a new
  public API that can be used by ef_vi applications. As an ef_vi application,
  TCPDirect now uses this API to query the control plane server.

From my perspective as someone less knowledgeable about the implication of this statement: There is no overlap with the statements from the proposed "New onload-devel package required" section.

@matthewr-xilinx any thoughts on whether or not a customer is likely to make the connection between this info and the need to install a new onload-dev package?

abower-amd commented 1 month ago

@abower-amd

there's too much overlap with the first section on "Public Onload/ef_vi control plane API".

  The Onload control plane in this Onload-9.0.0 is presented via a new
  public API that can be used by ef_vi applications. As an ef_vi application,
  TCPDirect now uses this API to query the control plane server.

From my perspective as someone less knowledgeable about the implication of this statement: There is no overlap with the statements from the proposed "New onload-devel package required" section.

The overlap is in the implied dependence on exported headers, but really the underlying commonality is in the fact that TCPDirect is now distributed at source, which was one of the motivations for the independent control plane API, hence why a holistic treatment is more appropriate.

@matthewr-xilinx any thoughts on whether or not a customer is likely to make the connection between this info and the need to install a new onload-dev package?

I didn't say the presence of this paragraph obviated the need to describe the requirement to install the devel package, although I did of course go on to suggest that need was covered in the installation steps!

matthewr-xilinx commented 1 month ago

@matthewr-xilinx any thoughts on whether or not a customer is likely to make the connection between this info and the need to install a new onload-dev package?

Need to be explicit that onload-dev/devel must be installed to build TCPDirect. Key thing is for this to be covered in the install section. But might be useful to add to the "Public Onload/ef_vi control plane API" section too, so there is more chance people will spot it. As you say, not all customers will have the insight to realise that this is required.

abower-amd commented 1 month ago

Thanks for kicking off this revision, now incorporated in #48!