Re: [Wireshark-commits] rev 39384: /trunk/epan/dissectors/ /trunk/epan/dissectors/: packet-pdcp-lte.c

[Anders Broman <a.broman () bredband net>]

Martin Mathieson skrev 2011-10-12 20:22:
> On Wed, Oct 12, 2011 at 6:42 PM, Guy Harris <guy () alum mit edu 
> <mailto:guy () alum mit edu>> wrote:
>
>     (Paging LTE experts here....)
>
>     On Oct 12, 2011, at 8:02 AM, wmeier () wireshark org
>     <mailto:wmeier () wireshark org> wrote:
>
>     >
>     http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=39384
>     <http://anonsvn.wireshark.org/viewvc/viewvc.cgi?view=rev&revision=39384>
>     >
>     > User: wmeier
>     > Date: 2011/10/12 08:02 AM
>     >
>     > Log:
>     > Fix a benign bug: Use correct proto_tree_add_item() encoding arg.
>
>     At least as I read RFC 3095:
>
>       UOR-2-TS
>
>         0   1   2   3   4   5   6   7
>       +---+---+---+---+---+---+---+---+
>       | 1   1   0 |        TS         |
>       +===+===+===+===+===+===+===+===+
>       |T=1| M |          SN           |
>       +---+---+---+---+---+---+---+---+
>       | X |            CRC            |
>       +---+---+---+---+---+---+---+---+
>
>     neither the old code nor the new code are correct - the "M" bit is
>     in the octet after the TS field.
>
>     I don't see anything obvious in 3GPP TS 36.323 itself that says
>     the format is different; does something in a later RFC specify
>     something different?
>
>
> Guy, I'm sure to be missing something, but I don't see what is wrong 
> with the current code, pasted here:
>
>     if (T) {
>         /* UOR-2-TS format */
>
>         /* TS */
>         guint8 ts = tvb_get_guint8(tvb, offset) & 0x1f;
>         proto_tree_add_uint(tree, hf_pdcp_lte_rohc_ts, tvb, offset, 1, 
> ts);
>         offset++;
>
>         /* Large CID */
>         if (p_pdcp_info->large_cid_present) {
>             offset = dissect_large_cid(tree, tvb, offset);
>         }
>
>         /* m */
>         proto_tree_add_item(tree, hf_pdcp_lte_rohc_m, tvb, offset, 1, 
> ENC_BIG_ENDIAN);
>
>         /* TODO: */
>     }
>
>
> 'offset' is incremented after TS is dissected.  The strange 
> +====+=====+ notation indicates that if we have a large CID, thats 
> where it appears.
> hf_pdcp_lte_rohc_m has the bitmask 0x40, which is OK.
>
> Note that the ROHC support in this file has been superceded by 
> packet-rohc.c.  Its been on my TODO list for a long time that I rip 
> out  this implementation and call the one in packet-rohc.c instead 
> (after carefully checking that everything here was already there or 
> merge it across).  As far as I know, RFC 3095 is used as-is in PDCP 
> (note that there are corrections/clarifications for this RFC, don't 
> remember where)
From packet-roch.c :-)
 *
 * Ref:
 * http://www.ietf.org/rfc/rfc3095.txt         RObust Header 
Compression (ROHC): Framework and four profiles: RTP, UDP, ESP, and 
uncompressed
 * http://datatracker.ietf.org/doc/rfc4815/    RObust Header 
Compression (ROHC): Corrections and Clarifications to RFC 3095
 * http://datatracker.ietf.org/doc/rfc5225/    RObust Header 
Compression Version 2 (ROHCv2): Profiles for RTP, UDP, IP, ESP and UDP-Lite
> Regards,
> Martin
>
>
>
> ___________________________________________________________________________
> Sent via:    Wireshark-dev mailing list<wireshark-dev () wireshark org>
> Archives:    http://www.wireshark.org/lists/wireshark-dev
> Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
>               mailto:wireshark-dev-request () wireshark org?subject=unsubscribe

___________________________________________________________________________
Sent via:    Wireshark-dev mailing list <wireshark-dev () wireshark org>
Archives:    http://www.wireshark.org/lists/wireshark-dev
Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev
             mailto:wireshark-dev-request () wireshark org?subject=unsubscribe