On 11/7/2010 10:46 AM, Hadriel Kaplan wrote:
Howdy, The current packet-netflow.c dissector has a big "switch
(pen_type) {...}" block in dissect_v9_v10_pdu_data(), which looks up
specific known netflow/ipfix fields as it walks netflow v9/10 PDUs.
Unfortunately, it's a bit of a hack as pen_type is a guint64 and a
switch statement will silently cast it to an int. I say
"unfortunately", because I discovered to my chagrin that it's a
*signed* int, so any case statement can't use a constant greater than
0x7fffffff, which given how the current code works, means one can't
have a Private Enterprise Number greater than 0x7fff and use it to
define a known field in this code. As it turns out, my Enterprise
number is higher than that. (Cace Technology's is just under it,
which is why the current code works for Cace's netflow fields)
Looking at the code a bit I see that currently "pen"
seems to be effectively limited to 16 bits even though 32 bits are
fetched from the tvbuff:
dissect_v9_v10_template_fields(...) {
...
guint16 pen;
...
if ((ver == 10) && (type & 0x8000)) { /* IPFIX only */
pen = tvb_get_ntohl(tvb,offset+4);
...
}
Is this a bug ? Are Enterprise Numbers greater that 16 bits allowed ?
(It would seem so).
In practical terms: Are there currently values > 16 bits ?
If not, one hack might be to define pen_type in
dissect_v9_v10_pdu_data() as guint32 which gets around the "silent cast"
issue for the moment.
Maybe a slightly better fix might be to have a separate function to
handle the pen_type for each different Enterprise Number (even if
there's some duplication).
So the question is: should I submit a bug with another hack to patch
it for my specific cases, or is there work going on by someone
already to re-do packet-netflow.c? (it could do with a re-write,
starting with pulling v9/v10 out of it and into a separate file)
I'm not aware of any work to redo packet-netflow.c; I suggest submitting
a patch;
If someone else is working on packet_netflow.c they'll presumably let us
know.
Bill
