Comment # 26
on bug 12687
from Michael Mann
(In reply to Oliver Hartkopp from comment #23)
> You patch has at least two issues:
>
> 1. The frame_type_vals still contain RTR. Please check my patch to fix this
> up.
My first patch today (unintentionally) left the RTR logic as is. Subsequent
patches tried to "improve" it, but I don't understand the justification for
effectively removing it entirely. Isn't it just something that isn't supported
in CANFD, but is still supported in "Classic" CAN?
(If you register an account with Gerrit, you can be added to the patch review
so you get notified when updates are made. You can also make comments about
the patch directly within Gerrit. Not necessary, but I've been trying to keep
the discussion in this bug about the "design" of CANFD in Wireshark, the gory
details of code should try to be done in Gerrit.)
> 2. Don't know what canfd_actual_length() is for. Wireshark uses libpcap.
> libpcap uses PF_CAN raw sockets. PF_CAN raw sockets and CAN network
> interfaces provide a plain length value and no data length _code_ (DLC) - so
> there's no need to do any DLC-to-length conversion anywhere in userspace
> (see my provided PDF slides).
Since "Classic CAN" length fits in 4 bits (max value of 8), I wasn't sure if
the "file format" for CAN was the DLC or the actual length. That's why I used
the table in slide 4 of your PDF slides to create canfd_actual_length(). Easy
enough to remove.
>
> > Only potential outstanding issue is how to handle SLL frames. Since
> > CANFD has it's own "type" within SLL, will it still set the "CANFD" flag
> > (0x4), or will its dissection need to be tweaked?
>
> I'm not sure about this either.
>
> Picking up the stuff above: Wireshark uses libpcap. libpcap uses PF_CAN raw
> sockets. As you can see from the libpcap patch the CAN RAW socket just can
> handle both types of CAN frames (CAN / CAN FD). To distinguish CAN and CAN
> FD I'm setting the CANFD_USE bit in the provided content which is used by
> wireshark.
>
> Alternatively we might split this traffic into two separate paths?!?
>
> Having different SSL definitions was intended for PF_PACKET users to make
> clear which kind of frame is provided. But when libpcap is mandatory for
> Wireshark - where do you see the SSL types then?
Wireshark is a network analyzer tool first and a capturing tool second.
Wireshark isn't the only tool that can generate pcap and pcapng files. I'm not
sure of the details of how one could generate a "Linux cooked capture" frame
format (as opposed to the CAN frame format you're currently using), but that's
why I have the hooks into the SLL dissector table (with values I picked up from
your documentation/source code) in my CANFD patch.
Yes it is an "alternate code path" and what I'm trying to figure out is if that
"alternate code path" will still end up with the same format for a CAN packet
where I can use the "CAN FD flag field" to determine if the packet is CANFD or
because the SLL type is explicitly CANFD, the flag won't be set (and the code
needs to be more refactored)
You are receiving this mail because:
- You are watching all bug changes.
