Re: LUA chained dissector drops data parameter

[Peter Wu <peter () lekensteyn nl>]

On Fri, Nov 29, 2019 at 03:25:51PM +0000, Kanstrup, Mikael wrote:
> Hi Peter,
>
> Thanks a lot for an extensive answer with links to related discussions!
>
> Have proto data use been considered for passing data around instead? I
> suppose there's a performance penalty to using that over directly
> passing the pointer. For my scenario the data parameter can quite
> easily be replaced with p_get_proto_data / p_add_proto_data. This
> works for cases where the LUA script itself does not need access the
> data passed around.


That appears to be a suitable API according to doc/README.dissector:

    The two most common use cases for p_add_proto_data/p_get_proto_data
    are for persistent data about the packet for the lifetime of the
    capture (file scope) and to exchange data between dissectors across
    a single packet (packet scope).

> I uploaded a change for this discussion here. I have not done any
> extensive testing but it shows the idea:
> https://code.wireshark.org/review/35260
>
> Performance wise using callgrind with tshark -V on my packet capture
> file with 16000 packets I get around 0.5% higher numbers with proto
> data compared to directly passing the pointer.

Thanks for the quick checks. It could potentially get worse if there are
more protocols using this API, but it should only be a small constant.

How do you plan to make the data available to Lua? Is it read-only for
Lua, or writable by Lua as well? The 'private_table' API is not very
well specified in terms of lifetime and scope, and only accepts strings
at the moment. I am not sure if we want to make it a fixed part of the
public API.

Kind regards,
Peter

> /Mikael
>
> ________________________________
> Från: Wireshark-dev för Peter Wu
> Skickat: den 26 november 2019 01:40
> Till: Developer support list for Wireshark
> Ämne: Re: [Wireshark-dev] LUA chained dissector drops data parameter
>
> Hi Mikael,
>
> On Mon, Nov 18, 2019 at 05:20:54PM +0000, Kanstrup, Mikael wrote:
> > Hi,
> >
> > I'm working on dissecting a proprietary protocol that extends Bluetooth HCI_ACL with a LUA dissector. As there's no 
> > heuristics dissector list registered for this particular protocol I thought something similar could be achieved 
> > with a chained dissector. I retrieve the original HCI_ACL dissector handle and replace it with my own LUA 
> > dissector. In LUA dissector apply some heuristics and if it's not my own protocol then call the original HCI_ACL 
> > dissector via the handle.
> >
> > Code looks like this:
> >
> > local proto_test = Proto("test", "Use chaining as heuristic dissector")
> > local proto_default_acl
> >
> > function is_test_proto(tvb, pinfo)
> >     -- Apply heuristics to determine if own protocol
> >     return false
> > end
> >
> > function proto_test.dissector(tvb, pinfo, tree)
> >     if not is_test_proto(tvb, pinfo) then
> >         return proto_default_acl:call(tvb, pinfo, tree)
> >     end
> >
> >     pinfo.cols.protocol = "test"
> >     tree = tree:add(proto_test, tvb)
> >     return tvb:len()
> > end
> >
> > function proto_test.init()
> >     local hci_type = DissectorTable.get("hci_h4.type")
> >     local pattern = 0x02 -- ACL
> >     proto_default_acl = hci_type:get_dissector(pattern)
> >     hci_type:add(pattern, proto_test)
> > end
> >
> > This unfortunately did not work and I was not able to find out why until I started looking at the HCI_ACL dissector 
> > code itself.
> >
> > static gint
> > dissect_bthci_acl(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data)
> > {
> > <...>
> >     /* Reject the packet if data is NULL */
> >     if (data == NULL)
> >         return 0;
> >
> > The above NULL check is hit for all calls coming from the LUA dissector. The LUA dissector function prototype does 
> > not have the data parameter and it appears it's simply lost when chaining calls through LUA.
>
> Correct, Lua always passes a NULL parameter. Due to type-safety,
> Lua dissectors cannot pass arbitrary data.
>
> > Any suggestions on how to approach this? Would it be possible to
> > extend the LUA dissector interface with another function prototype
> > that supports the data parameter? Just support relaying the parameter
> > in chained dissectors, not modifying or doing any fancy stuff with it.
>
> There is unfortunately no runtime registration of types for the 'data'
> parameter. If you can somehow keep track that a C dissector was from a
> particular dissector table, and then recognize that the Lua dissector
> was registered with the same table, then hopefully it should be possible
> to propagate the data parameter as special case.
>
> Long-term, we might have to revisit how to pass data between (multiple
> layers of) dissectors. Not just Lua, but also C. For Lua, see also:
>
> Bug 15931 - Add Lua support for arbitrary data parameter in dissector calls.
> https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=15931
>
> (+cc Huang)
> Discussion about another mechanism to pass data between dissectors:
> https://code.wireshark.org/review/35159
>
> Discussion about another (abandoned) new mechanism to pass data between
> dissectors:
> https://code.wireshark.org/review/34049
___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    https://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://www.wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe