RE: [PATCH for-next 09/10] IB/mlx4: Add timestamp_mask and hca_core_clock to query_device

Hefty, Sean
2015-05-29T16:52:44Z

> What is the issue here? The timestamp is created when the processing is
> finished by the nic and the completion entry becomes available.

The timestamp is generated when a work completion entry is written. If there's a clear use case for this, it hasn't been described. The use case you mentioned only works if there is a 1:1 relationship between a packet and a work completion. That is not what is being defined here.

> It is exactly defined like any other cycle counters in hardware and it is
> exposed using an API that allows multiple vendors to use these cycle
> counters without regard to a particular vendors implementation.

I disagree. This is associated with a specific implementation. It assumes that the counter is part of a CQ entry, and that the counter is written when the completion is written. There's nothing that requires other vendors to follow that model, nor is it clear that this is a generic or useful enough operation that other vendors would want to follow this model. Why not have the time stamp record the start of the transaction? The end? Have two stamps, once for the first packet, and one for the last? Limit this to single packet operations only?

> > The use case given by Christoph only speaks of packet level time stamps.
> O=
> > ne could argue that such a use case would place the stamp near the
> packet (=
> > similar to the GRH), rather than embedded into a work completion. This
> wou=
> > ld allow time stamps even in the absence of a work completion.
> >
> > IMO, vendors already have ways to expose vendor specific features to
> user s=
> > pace. I would mark this as vendor specific and keep it out of the core.
>
> How exactly would that work? How can we attach vendor specific extension
> to a completion structure?

I'm just stating that there is at other ways of exposing this sort of feature. A time stamp could just as easily be written with the data, similar to the grh. One of the points of defining the verbs extensions was exactly so that a vendor could export their own functionality.

- Sean