Re: [PATCH 1/2 v2] PCI/portdrv: Enable reporting inter-switch P2P links

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Mon, 14 Oct 2024 15:10:57 +0530
Shivasharan Srikanteshwara <shivasharan.srikanteshwara@broadcom.com> wrote:

> On Fri, Oct 4, 2024 at 4:09 PM Jonathan Cameron <Jonathan.Cameron@huawei.com>
> wrote:
> >
> > On Thu, 3 Oct 2024 14:41:07 -0600
> > Sumanesh Samanta <sumanesh.samanta@broadcom.com> 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