DTrace IP Provider [PSARC/2008/302 FastTrack timeout 05/07/2008]

[Brendan Gregg - Sun Microsystems]

G'Day Darren,

On Mon, May 12, 2008 at 09:50:08PM +0800, Darren Reed wrote:
> Erik Nordmark wrote:
> >Adam,
> >
> >Overall this looks good. One nit though.
> >
> >>/*
> >> * ipinfo contains common IP info for both IPv4 and IPv6.
> >> */
> >>typedef struct ipinfo {
> >>        uint8_t ip_ver;                 /* IP version (4, 6) */
> >>        uint16_t ip_plength;            /* payload length */
> >>        string ip_saddr;                /* source address */
> >>        string ip_daddr;                /* destination address */
> >>} ipinfo_t;
> >
> >I don't think "payload length" has a natural definition when trying to 
> >look at what IP actually carries because of the IPv6 extension headers.
> >For instance, for a 100 byte IPv6 packet with a 8 byte hop-by-hop 
> >options header, a fragment header, and a TCP header, what is the 
> >payload length?
> >The IPv6 header field for payload length is just 40 less than the size 
> >of the IPv6 packet. But the payload that IP delivers to TCP is less 
> >all the extension headers. And some might think that "payload" refers 
> >to the TCP payload.
> >
> >Thus I think it would be more clear to have the IP provider expose the 
> >IP packet length instead of payload length.
> >
> >Sorry for not catching this earlier.
> 
> Hmmm, what position should be taken with respect to IPv6 jumbograms?
> 
> We don't support them now, but should implementing them mean this
> interface (the dtrace provider) needs to be changed?
> 
> Otherwise, I can't see why ip_plength shouldn't be 32bits.

Good point, the uint16_t ip_plength came from IPv4, and as a translator
(/usr/lib/dtrace/ip.d) derived member it can be whatever we like.  Sounds
like 32 bits is a safer choice - I'll make the tiny change.

cheers,

Brendan

-- 
Brendan
[CA, USA]