Re: [PATCH RFC] can: add optional DLC element to Classical CAN frame structure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 01.11.20 15:50, Vincent MAILHOL wrote:
On 30.10.20 06:33, Oliver Hartkopp wrote:
    can0  60D   [F]  F2 C6 B6 1D 80 FB BC 7E
    can1  60D   [F]  F2 C6 B6 1D 80 FB BC 7E
    can0  5DD   [9]  7E 55 56 5B C0 23 B0 53
    can1  5DD   [9]  7E 55 56 5B C0 23 B0 53
    can0  573   [E]  20 8E A3 31 1B 54 B2 16
    can1  573   [E]  20 8E A3 31 1B 54 B2 16
    can0  470   [3]  31 9C 06
    can1  470   [3]  31 9C 06
    can0  465   [4]  93 75 A2 08
    can1  465   [4]  93 75 A2 08


Works fine :-)

I'm not really sure about the view of Classical CAN DLCs in this 'long'
candump representation.

There are people that use this format as base for further processing -
instead of the logfile format m(
So it may be a bad decision to put the DLC value in the length position
here.

There was actually zero thought when I wrote this. I just wanted to
test the kernel land and this was the first hack which came to my
mind.

No problem. I would have chosen the same approach ...

candump is not an easy one to modify. When putting the DLC in the
picture, I can see three option:

   1. Do not modify the format. Nothing will break but it would be
      impossible to know the actual DLC value of a Classical CAN frame
      of length 8.

   2. Ask people doing processing on the candump output to modify their
      tool if they want to use the cc-len8-dlc switch (if not using it,
      should not break).

   3. The in-between solution: use a command line switch (e.g. candump
      -8 any) select option 1 or 2 at run time.

None of the solution is perfect.

Right.

I personally do not like option 3. So far, the command "candump any"
would capture all the information. For example, their is no switch to
enable/disable the capture of CAN-FD frames so it would seems odd to
me to have it only for the DLC.

In fact candump tries to switch the socket into CAN FD mode and when it fails -> ¯\_(ツ)_/¯

But there is a difference in the standard output:
CAN FD frames have always two digits for the length, e.g. [32] or [04] while the standard Classical CAN frame has one digit, e.g. [5] or [0]

Option 1 would be inconvenient for people who have needs similar to
mine. Option 2 would be inconvenient for people who want to use
cc-len8-dlc but do not want to touch their tools built on top of
candump.

At the end of the day, I can not think of a perfect solution but I
actually liked your previous idea of using parenthesis. The rule would
be that [x] would designate a DLC of len2dlc(x) and a length of x; and
(x) would designate a DLC of x and length of 8. On a security point of
view, I think that the impact would be minimal. If we change the
format form [x] to (x), parsing tools build on top should directly
crash and the user should notice directly.

Yes. After you shared your thoughts I would indeed tend to provide an option '-8' and then always print the raw DLC in parenthesis e.g. (0) or (F).

IMO this len8_dlc stuff is an expert feature that normal users do not care about. Therefore it should be disabled by default.

I changed the logfile format in a backward compatible way and therefore
the cansend command line format too:

(1604004658.444168) vcan0 2C0##02643
(1604004658.494492) vcan0 615#R8_9
(1604004658.645395) vcan0 6FF#
(1604004658.695682) vcan0 623##318F88F7043C0
(1604004658.746046) vcan0 63A##28D1F37
(1604004658.796214) vcan0 1117DEA5##0BC281D711098
(1604004658.846397) vcan0 1490B893#R8_F
(1604004658.896585) vcan0 1F494EC8##39C47E2
(1604004658.947142) vcan0 1D1EC448#3DC1C81345D7ED7E_D
(1604004658.997689) vcan0 1C8661BB##14DEA7568D459160D
(1604004659.047851) vcan0 2D8#AFE0546E6522130D_E
(1604004659.098216) vcan0 1596E833##3E6789514EF10B466E6789514EF10B466

Not the most sexy solution but I have to admit that it does the job
and fits well with the other tools. Also, I am not able to think of a
better solutions.

At least is is backwards compatible with older versions of canplayer and log2asc.

It doesn't win a design price but I asked my wife what to put as a len8_dlc delimiter. And the '_' won as it was still good to distinguish when looking at the log file format output.

Not sure if anyone also uses this log format as base for further
processing. Somewhere, someone might be impacted as well. But there is
probably nothing that we can do about that.

Yes, I know people using the log file format - and lib.h and lib.c

I will need more time to complete a full test and review.

Overall, I think that you did a good job in the naming convention and
the choice of the command line options.

Thanks!

Best regards,
Oliver



[Index of Archives]     [Automotive Discussions]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]     [CAN Bus]

  Powered by Linux