Huge thanks to our Platinum Members Endace and LiveAction,
and our Silver Member Veeam, for supporting the Wireshark Foundation and project.

Wireshark-bugs: [Wireshark-bugs] [Bug 7806] Enhance to support P2MP-RSVP related TLVs of MPLS OA

Date: Sun, 7 Oct 2012 08:03:44 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7806

--- Comment #5 from Evan Huus <eapache@xxxxxxxxx> 2012-10-07 08:03:43 PDT ---
(In reply to comment #3)
> Hi Evan,
> 
> Thank you for your so-much-quick reply!
> 
> (In reply to comment #2)
> > Hi Tomofumi, thanks for the patch. Two quick questions:
> > 
> > - Why do you register hf_mpls_echo_tlv_fec_rsvp_p2mp_ipv4_ext_tunnel_id as a
> > uint32 and then add it to the tree with proto_tree_add_text as an IPv4 address?
> > Wouldn't it be easier to just register it as an IPv4 field?
> From RFC, _ext_tunnel_id is described just as "unique identifier", not ip
> address,
> hence I keep uint32_t. 
> http://tools.ietf.org/html/rfc4875#section-19.1.1
> 
> But _ext_tunnel_id depends on address type, i.e. IPv4-rspv has 32bit for ext
> tunnel and ipv6 has 128bit as well.
> 
> So I will change to ipv4 addr soon and update the patch.

If I understand correctly then, it is defined in the standard as a unique
identifier that happens to have the correct length to store an IP address (v4
or v6 depending), with the result that everybody just stores an IP address in
it, even though the standard doesn't technically specify that?

If that's the case then the existing code that adds it to the tree with text
and adds a hidden item for filtering is probably fine - it really depends if
people are more likely to want to specify it as a uint or an IP address in the
filter bar.

> > - There are a few places where you use proto_tree_add_text to display an error
> > condition - is there a particular reason you're not using
> > expert_add_info_format?
> This is why packet-mpls-echo.c uses only proto_tree_add_text() for the error.
> I wrote this code looking packet-mpls-echo.c only, so I don't recognize
> expert_add_info().
> 
> I can rewrite my diff to using expert_add_info(). Should I rewrite it?

expert_add_info_format is the new way to indicate possibly malformed packets
and other things of that nature. It probably didn't exist when the dissector
was originally written, and nobody's bothered to update it since, but new code
should be using it. packet-moldudp.c is a nice short dissector that uses it a
few times if you're looking for an example.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.