Comment # 29
on bug 12687
from Oliver Hartkopp
(In reply to Michael Mann from comment #26)
> (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?
Only Classic CAN supports RTR.
But you can have STD (== 11 bit Identifier) or XTD (==29 bit Identifier) CAN
frames that may contain data bytes (length according to DLC) OR the CAN frame
is a RTR frame which has a DLC but no data content.
Therefore RTR can not be a 'frame type' like STD or XTD.
As RTR has turned out to be 'bad' they left out RTR frames when defining CAN
FD.
> (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.)
Will do tomorrow ...
> > 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.
Hm - I assumed the entire CAN frame will be written into the file as it was
passed from libpcap. As long as there's not bit signal serialization in the
pcapng file format it should work.
I was reading back the former pcapng file with the 'Classic CAN only' content
which is working fine :-)
> >
> > > 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)
Hm - it turn's out that I'm currently the person that defines the layout of the
'dual use' CAN frame structure for libpcap and its users :-)
The two SLL types are not really relevant for CAN application programmers. It's
a Linux internal thing which only becomes visible when programming PF_PACKET
sockets.
For that reason my suggested changes to libpcap make sense to me as they are
using PF_CAN sockets to get the CAN frames - and this would be a reasonable
extension then.
You are receiving this mail because:
- You are watching all bug changes.
