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

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

[Matias Ezequiel Vara Larsen]

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.
+
 If the parameters are valid the message is scheduled for transmission.
 
 If feature VIRTIO_CAN_F_CAN_LATE_TX_ACK has been negotiated the

base-commit: 37c6a406678a5ee891fdf5671298cb4fcfa517f2
-- 
2.42.0

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

[Stefano Garzarella]

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". Also should we cover `sdu` as well?

Thanks,
Stefano

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

[Matias Ezequiel Vara Larsen]

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

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

[Stefano Garzarella]

On Fri, Jun 21, 2024 at 04:27:41PM GMT, Matias Ezequiel Vara Larsen wrote:
>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.

mmm, I'm not sure too, indeed that is only defined for "CAN Message 
Reception". Maybe we need someone more inside CAN or check the CAN 
specifications.

Thanks,
Stefano

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

[Harald Mommer]

On 21.06.24 16:27, Matias Ezequiel Vara Larsen wrote:
> 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.
>>> +
>>
Non-extended CAN frame and can_id contains more than 11 bits (not in 
range 0..7FFH) => broken.

Extended CAN frame and can_id contains more than 29 bits (not in range 
0..1FFFFFFFH) => broken.

See also 5.20.3 "Feature bits", there it's mentioned but not this 
prominently.

Classic CAN frame and sdu length > 8 bytes => broken.

Comes from ISO 11898-1:2015, should have been mentioned more clearly 
here as it was done in 5.20.3. There is not stated that the allowed 
value range is 0..8. Obvious for CAN people, not obvious for anybody else.

CAN FD frame and sdu length > 8 bytes and not in the set { 12, 16, 20, 
24, 32, 48, 64 } bytes => broken.

Comes from ISO 11898-1:2015, is not obvious for people not too familiar 
with CAN FD and is missing in the virtio CAN specification.

This is what was meant. Now some good wording is needed.

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

[Matias Ezequiel Vara Larsen]

On Wed, Jun 26, 2024 at 01:07:17PM +0200, Harald Mommer wrote:
> 
> On 21.06.24 16:27, Matias Ezequiel Vara Larsen wrote:
> > 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.
> > > > +
> > > 
> Non-extended CAN frame and can_id contains more than 11 bits (not in range
> 0..7FFH) => broken.
> 
> Extended CAN frame and can_id contains more than 29 bits (not in range
> 0..1FFFFFFFH) => broken.
> 
> See also 5.20.3 "Feature bits", there it's mentioned but not this
> prominently.
> 
> Classic CAN frame and sdu length > 8 bytes => broken.
> 
> Comes from ISO 11898-1:2015, should have been mentioned more clearly here as
> it was done in 5.20.3. There is not stated that the allowed value range is
> 0..8. Obvious for CAN people, not obvious for anybody else.
> 
> CAN FD frame and sdu length > 8 bytes and not in the set { 12, 16, 20, 24,
> 32, 48, 64 } bytes => broken.
> 
> Comes from ISO 11898-1:2015, is not obvious for people not too familiar with
> CAN FD and is missing in the virtio CAN specification.
> 
> This is what was meant. Now some good wording is needed.
> 
> 
Thanks Harald! I'll add it in v1.

Matias