This is the mail archive of the systemtap@sourceware.org mailing list for the systemtap project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/1] Add new TCP and IP functions


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]