On Tue, 22 Feb 2022 18:03:30 +0200
ss ws <sswsdev@xxxxxxxxx> wrote:
> Hi Denis,
Hi,
> I found myself writing several "multi payloads" dissectors like the
> one you describe and I can say the best thing you can do is write a
> compact description of each packet into the info column and separate
> the payloads:
> * Visually using commas
> * And logicaly using something called a 'fence'-- look for other
> dissectors calling 'col_set_fence'
Thanks a lot, I've now implemented that and it works fine. I'll
probably write some script to reformat the tshark output to be more
diff/meld friendly.
> About the second issue:
> I'll let someone else answer but I remember vaugly that filters aren't
> exclusive so you might be able to just register both with the same
> filter string. An alternative I do remember is the trick used for
> "ip.addr" vs "ip.src" and "ip.dst" fields.
> Both "Source Address" and "Destination Address" have their own fields
> with unique filters but by also the dissector is adding a hidden
> field for each of them (the one with the ip.addr filter) so you can
> filter for both of those fields using the unified filter.
> I'd recommend reading that part in the IP dissector for inspiration.
The issue with "ip.addr" is that data is "generated" from custom code
but the interpretation of that data isn't.
In my case the field is fixed but its interpretation changes depending
on if it's an incoming or outgoing usb packet (P2P_DIR_RECV
or P2P_DIR_SENT).
After some trial and errors I've managed to get something that works
but there is probably a better way to do it.
Basically I register it like that without interpreting the field:
> void proto_register_samsung_ipc(void)
> {
> static hf_register_info hf_fmt[] = {
> [...]
> { &hf_samsung_ipc_fmt_type,
> { "samsung-ipc FMT type",
> "samsung-ipc.fmt.type",
> FT_UINT8,
> BASE_HEX,
> NULL,
> 0x0,
> NULL,
> HFILL },
> },
> [...]
> };
> [...]
> proto_register_field_array(proto_samsung_ipc,
> hf_fmt, array_length(hf_fmt));
And before I just didn't decode the values:
> item = proto_tree_add_item(fmt_tree,
> hf_samsung_ipc_fmt_type, tvb, 9,
> 1, ENC_NA);
And now I decode it by setting the field string:
> fmt_type = tvb_get_guint8(tvb, 9);
> if (usb_conv_info->direction == P2P_DIR_RECV)
> fmt_type_str = try_val_to_str(
> fmt_type,
> hf_samsung_ipc_fmt_requests_types);
> else if (usb_conv_info->direction == P2P_DIR_SENT)
> fmt_type_str = try_val_to_str(
> fmt_type,
> hf_samsung_ipc_fmt_responses_types);
> item = proto_tree_add_item(
> fmt_tree,
> hf_samsung_ipc_fmt_type, tvb, 9, 1,
> ENC_NA);
> if (fmt_type_str)
> proto_item_set_text(item,
> "samsung-ipc FMT type: %s (0x%x)",
> fmt_type_str, fmt_type);
So now I'll cleanup the code. I'll also try to get some packet traces
of the Nokia ISI protocol to see how things are displayed to do
something similar with the samsung-ipc dissector.
I've some more questions on what is acceptable or not for upstreaming
(a samsung-ipc) dissector(s):
- The samsung-ipc protocol is a protocol to communicate with some modems
that run firmwares made by/for Samsung. So like AT commands or other
modem protocols we can send commands to it (like call this number) and
receive information (like we have a call from this number).
Is it OK if only the basics are implemented (like the commands,
sequence numbers, etc), but not necessarily more advanced information?
In the example above it would report the command CALL_OUTGOING
but not parse the command data (that includes the phone number and
other parameters).
- The free software implementation(s) of that protocol consist in two
parts:
- kernel drivers that don't do much encoding/decoding beside
adding/removing HDLC frame delimiters and separating channels and
direction
- an userspace library which implements the protocol. It was written
by (originally not by me) by looking at protocol traces.
I've used the same terminology names than the struct fields in
libsamsung-ipc, to stay consistent, even if the field names could be
improved. Is that OK?
- After the HDLC frame delimiters there is 3 bytes before the
samsung-ipc packet length, and I've not yet found where in the
drivers or in libsamsung-ipc there is that offset or where it could
come from.
- The channels are encoded in the USB endpoint, and they are hardcoded
in the code. Even if they are also hardcoded in the vendor kernel of
the Galaxy SIII (GT-I9300), I've not checked if they could change. For
that I'd probably need to check the source code of every other
smartphones or tablets using that protocol and whose modem is
connected through USB or HSIC (a subset of the USB protocol that
works without PHY). Could this stay as-is and settings for that could
probably be implemented later (not necessarily by me) if me or other
people find cases where that hardcoding doesn't work.
Denis.
Attachment:
pgpcF6dQAirNx.pgp
Description: OpenPGP digital signature
