This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH 1/1] Add new TCP and IP functions
- From: Josh Stone <jistone at redhat dot com>
- To: Breno Leitao <leitao at linux dot vnet dot ibm dot com>
- Cc: systemtap at sources dot redhat dot com, Andrà Detsch <adetsch at br dot ibm dot com>
- Date: Mon, 13 Apr 2009 11:49:13 -0700
- Subject: Re: [PATCH 1/1] Add new TCP and IP functions
- References: <49DD1306.9060003@linux.vnet.ibm.com>
Breno Leitao wrote:
> This patch adds some basic functions to the IP and TCP tapsets.
Good thing to cover the basics... :)
> +/* Get the IP header for recent (> 2.6.21) kernels */
> +function __get_skb_iphdr_new:long(skb:long) %{
> + struct sk_buff *skb;
> + skb = (struct sk_buff *)(long)THIS->skb;
> + /* as done by skb_network_header() */
> + #ifdef NET_SKBUFF_DATA_USES_OFFSET
> + THIS->__retvalue = (long)(skb->head + skb->network_header);
> + #else
> + THIS->__retvalue = (long)skb->network_header;
> + #endif
> +%}
> [...]
> +/* returns the TCP header for recent (<2.6.21) kernel */
> +function __get_skb_tcphdr_new:long(skb:long) %{
> + struct sk_buff *skb;
> + skb = (struct sk_buff *)(long)THIS->skb;
> + /* as done by skb_transport_header() */
> + #ifdef NET_SKBUFF_DATA_USES_OFFSET
> + THIS->__retvalue = (long)(skb->head + skb->transport_header);
> + #else
> + THIS->__retvalue = (long)skb->transport_header;
> + #endif
> +%}
Those dereferences on skb need to use kread.
> +/* returns TCP URG flag for a given sk_buff structure */
> +function __tcp_skb_urg:long (skb:long){
> + urg = ntohs(@cast(__get_skb_tcphdr(skb), "tcphdr")->urg) & 32
> + return urg >> 5
> +}
Such bit-manipulation shouldn't be needed, but it looks like our
generated dereference code is not masking out bitfields correctly. I'll
file a bugzilla on this.
> +probe tcp.receive = kernel.function("tcp_v4_rcv"){
> + saddr = ip_ntop(__ip_skb_saddr($skb))
> + daddr = ip_ntop(__ip_skb_daddr($skb))
> + protocol = __ip_skb_proto($skb)
> + dport = __tcp_skb_dport($skb)
> + sport = __tcp_skb_sport($skb)
> + urg = __tcp_skb_urg($skb)
> + ack = __tcp_skb_ack($skb)
> + psh = __tcp_skb_psh($skb)
> + rst = __tcp_skb_rst($skb)
> + syn = __tcp_skb_syn($skb)
> + fin = __tcp_skb_fin($skb)
> +}
Since every one of those __ip and __tcp functions is computing their
respective headers, it might be better to compute the headers once and
pass them around instead of the skb.
> +function display_header(){
> + printf("---------------------------------------------------------\n");
> + printf(" Source IP Dest IP SPort DPort U A P R S F\n");
> + printf("---------------------------------------------------------\n");
> +}
This header and the data printf's should use padding to maintain
alignment. The IP addresses in particular throw everything off for me.
> +probe begin{
> + display_header()
> +}
> [...]
> +probe timer.s(1){
> + display_header()
> +}
These can be consolidated into a single probe body:
probe begin, timer.s(1) { display_header() }
You might not even keep the separate display_header() in that case.
Thanks,
Josh