diff mbox series

[v3,net-next,1/5] ipv4: Update fib_table_lookup tracepoint to take common nexthop

Message ID 20190402030234.26802-2-dsahern@kernel.org
State Changes Requested
Delegated to: David Miller
Headers show
Series net: More movement to fib_nh_common | expand

Commit Message

David Ahern April 2, 2019, 3:02 a.m. UTC 
From: David Ahern <dsahern@gmail.com>

Update fib_table_lookup tracepoint to take a fib_nh_common struct and
dump the v6 gateway address if the nexthop uses it.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/trace/events/fib.h | 45 ++++++++++++++++++++++++++-------------------
 net/ipv4/fib_trie.c        |  2 +-
 2 files changed, 27 insertions(+), 20 deletions(-)

Comments

Martin KaFai Lau April 2, 2019, 4:01 p.m. UTC | #1 
On Mon, Apr 01, 2019 at 08:02:30PM -0700, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
> 
> Update fib_table_lookup tracepoint to take a fib_nh_common struct and
> dump the v6 gateway address if the nexthop uses it.
> 
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
>  include/trace/events/fib.h | 45 ++++++++++++++++++++++++++-------------------
>  net/ipv4/fib_trie.c        |  2 +-
>  2 files changed, 27 insertions(+), 20 deletions(-)
> 
> diff --git a/include/trace/events/fib.h b/include/trace/events/fib.h
> index 61ea7a24c8e5..7f83b6eafc5c 100644
> --- a/include/trace/events/fib.h
> +++ b/include/trace/events/fib.h
> @@ -13,9 +13,9 @@
>  TRACE_EVENT(fib_table_lookup,
>  
>  	TP_PROTO(u32 tb_id, const struct flowi4 *flp,
> -		 const struct fib_nh *nh, int err),
> +		 const struct fib_nh_common *nhc, int err),
>  
> -	TP_ARGS(tb_id, flp, nh, err),
> +	TP_ARGS(tb_id, flp, nhc, err),
>  
>  	TP_STRUCT__entry(
>  		__field(	u32,	tb_id		)
> @@ -28,14 +28,17 @@ TRACE_EVENT(fib_table_lookup,
>  		__field(	__u8,	flags		)
>  		__array(	__u8,	src,	4	)
>  		__array(	__u8,	dst,	4	)
> -		__array(	__u8,	gw,	4	)
> -		__array(	__u8,	saddr,	4	)
saddr is no longer useful?

> +		__array(	__u8,	gw4,	4	)
> +		__array(	__u8,	gw6,	16	)
>  		__field(	u16,	sport		)
>  		__field(	u16,	dport		)
>  		__dynamic_array(char,  name,   IFNAMSIZ )
>  	),
>  
>  	TP_fast_assign(
> +		struct in6_addr in6_zero = {};
> +		struct net_device *dev;
> +		struct in6_addr *in6;
>  		__be32 *p32;
>  
>  		__entry->tb_id = tb_id;
> @@ -62,33 +65,37 @@ TRACE_EVENT(fib_table_lookup,
>  			__entry->dport = 0;
>  		}
>  
> -		if (nh) {
> -			struct net_device *dev;
> +		dev = nhc ? nhc->nhc_dev : NULL;
> +		__assign_str(name, dev ? dev->name : "-");
>  
> -			p32 = (__be32 *) __entry->saddr;
> -			*p32 = nh->nh_saddr;
> +		if (nhc) {
> +			if (nhc->nhc_family == AF_INET) {
> +				p32 = (__be32 *) __entry->gw4;
> +				*p32 = nhc->nhc_gw.ipv4;
>  
> -			p32 = (__be32 *) __entry->gw;
> -			*p32 = nh->fib_nh_gw4;
> +				in6 = (struct in6_addr *)__entry->gw6;
> +				*in6 = in6_zero;
> +			} else if (nhc->nhc_family == AF_INET6) {
> +				p32 = (__be32 *) __entry->gw4;
> +				*p32 = 0;
>  
> -			dev = nh->fib_nh_dev;
> -			__assign_str(name, dev ? dev->name : "-");
> +				in6 = (struct in6_addr *)__entry->gw6;
> +				*in6 = nhc->nhc_gw.ipv6;
> +			}
>  		} else {
> -			p32 = (__be32 *) __entry->saddr;
> +			p32 = (__be32 *) __entry->gw4;
>  			*p32 = 0;
>  
> -			p32 = (__be32 *) __entry->gw;
> -			*p32 = 0;
> -
> -			__assign_str(name, "-");
> +			in6 = (struct in6_addr *)__entry->gw6;
> +			*in6 = in6_zero;
>  		}
>  	),
>  
> -	TP_printk("table %u oif %d iif %d proto %u %pI4/%u -> %pI4/%u tos %d scope %d flags %x ==> dev %s gw %pI4 src %pI4 err %d",
> +	TP_printk("table %u oif %d iif %d proto %u %pI4/%u -> %pI4/%u tos %d scope %d flags %x ==> dev %s gw %pI4/%pI6c err %d",
>  		  __entry->tb_id, __entry->oif, __entry->iif, __entry->proto,
>  		  __entry->src, __entry->sport, __entry->dst, __entry->dport,
>  		  __entry->tos, __entry->scope, __entry->flags,
> -		  __get_str(name), __entry->gw, __entry->saddr, __entry->err)
> +		  __get_str(name), __entry->gw4, __entry->gw6, __entry->err)
>  );
>  #endif /* _TRACE_FIB_H */
>  
> diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
> index 1e3b492690f9..13b3327206f9 100644
> --- a/net/ipv4/fib_trie.c
> +++ b/net/ipv4/fib_trie.c
> @@ -1498,7 +1498,7 @@ int fib_table_lookup(struct fib_table *tb, const struct flowi4 *flp,
>  #ifdef CONFIG_IP_FIB_TRIE_STATS
>  			this_cpu_inc(stats->semantic_match_passed);
>  #endif
> -			trace_fib_table_lookup(tb->tb_id, flp, nh, err);
> +			trace_fib_table_lookup(tb->tb_id, flp, &nh->nh_common, err);
>  
>  			return err;
>  		}
> -- 
> 2.11.0
>
David Ahern April 2, 2019, 4:03 p.m. UTC | #2 
On 4/2/19 10:01 AM, Martin Lau wrote:
> On Mon, Apr 01, 2019 at 08:02:30PM -0700, David Ahern wrote:
>> From: David Ahern <dsahern@gmail.com>
>>
>> Update fib_table_lookup tracepoint to take a fib_nh_common struct and
>> dump the v6 gateway address if the nexthop uses it.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> ---
>>  include/trace/events/fib.h | 45 ++++++++++++++++++++++++++-------------------
>>  net/ipv4/fib_trie.c        |  2 +-
>>  2 files changed, 27 insertions(+), 20 deletions(-)
>>
>> diff --git a/include/trace/events/fib.h b/include/trace/events/fib.h
>> index 61ea7a24c8e5..7f83b6eafc5c 100644
>> --- a/include/trace/events/fib.h
>> +++ b/include/trace/events/fib.h
>> @@ -13,9 +13,9 @@
>>  TRACE_EVENT(fib_table_lookup,
>>  
>>  	TP_PROTO(u32 tb_id, const struct flowi4 *flp,
>> -		 const struct fib_nh *nh, int err),
>> +		 const struct fib_nh_common *nhc, int err),
>>  
>> -	TP_ARGS(tb_id, flp, nh, err),
>> +	TP_ARGS(tb_id, flp, nhc, err),
>>  
>>  	TP_STRUCT__entry(
>>  		__field(	u32,	tb_id		)
>> @@ -28,14 +28,17 @@ TRACE_EVENT(fib_table_lookup,
>>  		__field(	__u8,	flags		)
>>  		__array(	__u8,	src,	4	)
>>  		__array(	__u8,	dst,	4	)
>> -		__array(	__u8,	gw,	4	)
>> -		__array(	__u8,	saddr,	4	)
> saddr is no longer useful?
> 

I have not found it useful, and I use this tracepoint a lot. The line
length is really long as is and adding the ipv6 address just makes it worse.
Martin KaFai Lau April 2, 2019, 5:06 p.m. UTC | #3 
On Tue, Apr 02, 2019 at 10:03:34AM -0600, David Ahern wrote:
> On 4/2/19 10:01 AM, Martin Lau wrote:
> > On Mon, Apr 01, 2019 at 08:02:30PM -0700, David Ahern wrote:
> >> From: David Ahern <dsahern@gmail.com>
> >>
> >> Update fib_table_lookup tracepoint to take a fib_nh_common struct and
> >> dump the v6 gateway address if the nexthop uses it.
> >>
> >> Signed-off-by: David Ahern <dsahern@gmail.com>
> >> ---
> >>  include/trace/events/fib.h | 45 ++++++++++++++++++++++++++-------------------
> >>  net/ipv4/fib_trie.c        |  2 +-
> >>  2 files changed, 27 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/include/trace/events/fib.h b/include/trace/events/fib.h
> >> index 61ea7a24c8e5..7f83b6eafc5c 100644
> >> --- a/include/trace/events/fib.h
> >> +++ b/include/trace/events/fib.h
> >> @@ -13,9 +13,9 @@
> >>  TRACE_EVENT(fib_table_lookup,
> >>  
> >>  	TP_PROTO(u32 tb_id, const struct flowi4 *flp,
> >> -		 const struct fib_nh *nh, int err),
> >> +		 const struct fib_nh_common *nhc, int err),
> >>  
> >> -	TP_ARGS(tb_id, flp, nh, err),
> >> +	TP_ARGS(tb_id, flp, nhc, err),
> >>  
> >>  	TP_STRUCT__entry(
> >>  		__field(	u32,	tb_id		)
> >> @@ -28,14 +28,17 @@ TRACE_EVENT(fib_table_lookup,
> >>  		__field(	__u8,	flags		)
> >>  		__array(	__u8,	src,	4	)
> >>  		__array(	__u8,	dst,	4	)
> >> -		__array(	__u8,	gw,	4	)
> >> -		__array(	__u8,	saddr,	4	)
> > saddr is no longer useful?
> > 
> 
> I have not found it useful, and I use this tracepoint a lot. The line
> length is really long as is and adding the ipv6 address just makes it worse.
Good to know that this removal is intentional.  Otherwise, it looks like
it is removed by accident or because nh_saddr is not available in nhc.

Please update the commit message accordingly to avoid future confusion in
case others may want to re-add it.
David Miller April 2, 2019, 5:41 p.m. UTC | #4 
From: Martin Lau <kafai@fb.com>
Date: Tue, 2 Apr 2019 17:06:08 +0000

> Please update the commit message accordingly to avoid future confusion in
> case others may want to re-add it.

+1
diff mbox series

Patch

diff --git a/include/trace/events/fib.h b/include/trace/events/fib.h
index 61ea7a24c8e5..7f83b6eafc5c 100644
--- a/include/trace/events/fib.h
+++ b/include/trace/events/fib.h
@@ -13,9 +13,9 @@ 
 TRACE_EVENT(fib_table_lookup,
 
 	TP_PROTO(u32 tb_id, const struct flowi4 *flp,
-		 const struct fib_nh *nh, int err),
+		 const struct fib_nh_common *nhc, int err),
 
-	TP_ARGS(tb_id, flp, nh, err),
+	TP_ARGS(tb_id, flp, nhc, err),
 
 	TP_STRUCT__entry(
 		__field(	u32,	tb_id		)
@@ -28,14 +28,17 @@  TRACE_EVENT(fib_table_lookup,
 		__field(	__u8,	flags		)
 		__array(	__u8,	src,	4	)
 		__array(	__u8,	dst,	4	)
-		__array(	__u8,	gw,	4	)
-		__array(	__u8,	saddr,	4	)
+		__array(	__u8,	gw4,	4	)
+		__array(	__u8,	gw6,	16	)
 		__field(	u16,	sport		)
 		__field(	u16,	dport		)
 		__dynamic_array(char,  name,   IFNAMSIZ )
 	),
 
 	TP_fast_assign(
+		struct in6_addr in6_zero = {};
+		struct net_device *dev;
+		struct in6_addr *in6;
 		__be32 *p32;
 
 		__entry->tb_id = tb_id;
@@ -62,33 +65,37 @@  TRACE_EVENT(fib_table_lookup,
 			__entry->dport = 0;
 		}
 
-		if (nh) {
-			struct net_device *dev;
+		dev = nhc ? nhc->nhc_dev : NULL;
+		__assign_str(name, dev ? dev->name : "-");
 
-			p32 = (__be32 *) __entry->saddr;
-			*p32 = nh->nh_saddr;
+		if (nhc) {
+			if (nhc->nhc_family == AF_INET) {
+				p32 = (__be32 *) __entry->gw4;
+				*p32 = nhc->nhc_gw.ipv4;
 
-			p32 = (__be32 *) __entry->gw;
-			*p32 = nh->fib_nh_gw4;
+				in6 = (struct in6_addr *)__entry->gw6;
+				*in6 = in6_zero;
+			} else if (nhc->nhc_family == AF_INET6) {
+				p32 = (__be32 *) __entry->gw4;
+				*p32 = 0;
 
-			dev = nh->fib_nh_dev;
-			__assign_str(name, dev ? dev->name : "-");
+				in6 = (struct in6_addr *)__entry->gw6;
+				*in6 = nhc->nhc_gw.ipv6;
+			}
 		} else {
-			p32 = (__be32 *) __entry->saddr;
+			p32 = (__be32 *) __entry->gw4;
 			*p32 = 0;
 
-			p32 = (__be32 *) __entry->gw;
-			*p32 = 0;
-
-			__assign_str(name, "-");
+			in6 = (struct in6_addr *)__entry->gw6;
+			*in6 = in6_zero;
 		}
 	),
 
-	TP_printk("table %u oif %d iif %d proto %u %pI4/%u -> %pI4/%u tos %d scope %d flags %x ==> dev %s gw %pI4 src %pI4 err %d",
+	TP_printk("table %u oif %d iif %d proto %u %pI4/%u -> %pI4/%u tos %d scope %d flags %x ==> dev %s gw %pI4/%pI6c err %d",
 		  __entry->tb_id, __entry->oif, __entry->iif, __entry->proto,
 		  __entry->src, __entry->sport, __entry->dst, __entry->dport,
 		  __entry->tos, __entry->scope, __entry->flags,
-		  __get_str(name), __entry->gw, __entry->saddr, __entry->err)
+		  __get_str(name), __entry->gw4, __entry->gw6, __entry->err)
 );
 #endif /* _TRACE_FIB_H */
 
diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c
index 1e3b492690f9..13b3327206f9 100644
--- a/net/ipv4/fib_trie.c
+++ b/net/ipv4/fib_trie.c
@@ -1498,7 +1498,7 @@  int fib_table_lookup(struct fib_table *tb, const struct flowi4 *flp,
 #ifdef CONFIG_IP_FIB_TRIE_STATS
 			this_cpu_inc(stats->semantic_match_passed);
 #endif
-			trace_fib_table_lookup(tb->tb_id, flp, nh, err);
+			trace_fib_table_lookup(tb->tb_id, flp, &nh->nh_common, err);
 
 			return err;
 		}