Re: [PATCH] virtio-can: define out of rage can-id

[Matias Ezequiel Vara Larsen <mvaralar@redhat.com>]

On Wed, Jun 05, 2024 at 10:42:19AM +0200, Stefano Garzarella wrote:
> On Tue, May 21, 2024 at 04:11:42PM GMT, Matias Ezequiel Vara Larsen wrote:
> > Explain when a message is out of range.
> > 
> > Signed-off-by: Matias Ezequiel Vara Larsen <mvaralar@redhat.com>
> > ---
> > * This patch applies on top of virtio-1.4, which has not been released
> >  yet.
> > ---
> > device-types/can/description.tex | 3 +++
> > 1 file changed, 3 insertions(+)
> > 
> > diff --git a/device-types/can/description.tex b/device-types/can/description.tex
> > index 2511d9c..98b163b 100644
> > --- a/device-types/can/description.tex
> > +++ b/device-types/can/description.tex
> > @@ -191,6 +191,9 @@ \subsubsection{Controller Mode}\label{sec:Device Types / CAN Device / Device Ope
> > invalid state with VIRTIO_CAN_RESULT_NOT_OK in \field{result} and MUST
> > NOT schedule the message for transmission.
> > 
> > +Note that a message is out of range when a standard frame uses more than 11
> > +bits of \field{can_id} or when an extended frame uses more than 29 bits.
> > +
> 
> It's not clear to me where we refer to an out-of-range message before. I
> only found "\field{can_id} or \field{sdu} length are out of range" where
> IIUC we talk about the length of certain fields is out of range. Are we
> talking about the same thing?
> 
> If so, I think we should talk explicitly about the "length of the fields".

Yes, I will rephrase it by using the "length of the fields" wording.

> Also should we cover `sdu` as well?
> 

I think we should cover it but I am not sure when the `sdu` length is
out of range. In the reception section (5.20.5.3), the spec states:

`If the feature VIRTIO_CAN_F_CAN_FD has been negotiated the maximal
possible sdu length is 64, if the feature has not been negotiated the
maximal possible sdu length is 8.  The actual length of the sdu is
determined by the length.`

For transmission, I guess `sdu` lenght is out of range if the length
does not fulfill the requirement above but I am not sure.

Matias