On Mon, 14 Oct 2024 15:10:57 +0530 Shivasharan Srikanteshwara <shivasharan.srikanteshwara@xxxxxxxxxxxx> wrote: > On Fri, Oct 4, 2024 at 4:09 PM Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx> > wrote: > > > > On Thu, 3 Oct 2024 14:41:07 -0600 > > Sumanesh Samanta <sumanesh.samanta@xxxxxxxxxxxx> wrote: > > > > > Hi Jonathan, > > > > > > >> Need more data that 'there is a link' for this. > > > >>I'd like to see some info on bandwidth and latency. > > > > > > As you too noted in your comments, for now, we are only addressing p2p > > > connection between "virtual switches", i.e. switches that look > > > different to the host, but are actually part of the same physical > > > hardware. > > > Given that, I am not sure what we should display for bandwidth and > > > latency. There is no physical link to traverse between the virtual > > > switches, and usually we take that as "infinite" bandwidth and "zero" > > > latency. > > > > For a case where you have no information, not having attributes is > > sensible. If there is information (CXL CDAT provides this for switches > > for instance) then we should have an interface that provides space for > > that information. > > > > > As such, any number here will make little sense until we > > > start supporting p2p connection between physical switches. > > > > As above, it makes sense in a switch as well - if the information > > is available. > > > > > We could, > > > of course, have some encoding for the time being, like have "INF" for > > > bandwidth and 0 for latency, but again, those will not be very useful > > > till the day this scheme is extended to physical switch and we display > > > real values, like bandwidth and latency for a x16 PCIe link. Thoughts? > > > > Hide the sysfs attributes for latency and bandwidth if we simply don't > > know. Software built on top of this can then assume full bandwidth > > is available or better still run some measurements to establish the > > missing data. > > > > All I really meant by this suggestion is a directory with space for > > other info is probably more extensible than a single file. > > Hi Jonathan, > We will make the changes to add a directory for p2p_link related information > to be exposed to user. We will only populate the information related to the > inter-switch P2P links. Rest of the attributes can be added for devices that > report them at a later stage. > Please check if the directory structure makes sense to you: > /sys/devices/.../B:D:F/p2p_link/links -> Reading this file will return the > same > information that is returned currently by the p2p_link file. Sounds good to me. Jonathan