Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next-2.6] ipv4: af_inet.c cleanups
From: Eric Dumazet @ 2009-08-27 13:35 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv4/af_inet.c |  112 +++++++++++++++++++++----------------------
 1 files changed, 55 insertions(+), 57 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 197d024..6c30a73 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -124,7 +124,6 @@ static struct list_head inetsw[SOCK_MAX];
 static DEFINE_SPINLOCK(inetsw_lock);
 
 struct ipv4_config ipv4_config;
-
 EXPORT_SYMBOL(ipv4_config);
 
 /* New destruction routine */
@@ -139,12 +138,12 @@ void inet_sock_destruct(struct sock *sk)
 	sk_mem_reclaim(sk);
 
 	if (sk->sk_type == SOCK_STREAM && sk->sk_state != TCP_CLOSE) {
-		printk("Attempt to release TCP socket in state %d %p\n",
+		pr_err("Attempt to release TCP socket in state %d %p\n",
 		       sk->sk_state, sk);
 		return;
 	}
 	if (!sock_flag(sk, SOCK_DEAD)) {
-		printk("Attempt to release alive inet socket %p\n", sk);
+		pr_err("Attempt to release alive inet socket %p\n", sk);
 		return;
 	}
 
@@ -157,6 +156,7 @@ void inet_sock_destruct(struct sock *sk)
 	dst_release(sk->sk_dst_cache);
 	sk_refcnt_debug_dec(sk);
 }
+EXPORT_SYMBOL(inet_sock_destruct);
 
 /*
  *	The routines beyond this point handle the behaviour of an AF_INET
@@ -219,6 +219,7 @@ out:
 	release_sock(sk);
 	return err;
 }
+EXPORT_SYMBOL(inet_listen);
 
 u32 inet_ehash_secret __read_mostly;
 EXPORT_SYMBOL(inet_ehash_secret);
@@ -435,9 +436,11 @@ int inet_release(struct socket *sock)
 	}
 	return 0;
 }
+EXPORT_SYMBOL(inet_release);
 
 /* It is off by default, see below. */
 int sysctl_ip_nonlocal_bind __read_mostly;
+EXPORT_SYMBOL(sysctl_ip_nonlocal_bind);
 
 int inet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 {
@@ -519,6 +522,7 @@ out_release_sock:
 out:
 	return err;
 }
+EXPORT_SYMBOL(inet_bind);
 
 int inet_dgram_connect(struct socket *sock, struct sockaddr * uaddr,
 		       int addr_len, int flags)
@@ -532,6 +536,7 @@ int inet_dgram_connect(struct socket *sock, struct sockaddr * uaddr,
 		return -EAGAIN;
 	return sk->sk_prot->connect(sk, (struct sockaddr *)uaddr, addr_len);
 }
+EXPORT_SYMBOL(inet_dgram_connect);
 
 static long inet_wait_for_connect(struct sock *sk, long timeo)
 {
@@ -641,6 +646,7 @@ sock_error:
 		sock->state = SS_DISCONNECTING;
 	goto out;
 }
+EXPORT_SYMBOL(inet_stream_connect);
 
 /*
  *	Accept a pending connection. The TCP layer now gives BSD semantics.
@@ -668,6 +674,7 @@ int inet_accept(struct socket *sock, struct socket *newsock, int flags)
 do_err:
 	return err;
 }
+EXPORT_SYMBOL(inet_accept);
 
 
 /*
@@ -699,6 +706,7 @@ int inet_getname(struct socket *sock, struct sockaddr *uaddr,
 	*uaddr_len = sizeof(*sin);
 	return 0;
 }
+EXPORT_SYMBOL(inet_getname);
 
 int inet_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
 		 size_t size)
@@ -711,9 +719,11 @@ int inet_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
 
 	return sk->sk_prot->sendmsg(iocb, sk, msg, size);
 }
+EXPORT_SYMBOL(inet_sendmsg);
 
 
-static ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset, size_t size, int flags)
+static ssize_t inet_sendpage(struct socket *sock, struct page *page, int offset,
+			     size_t size, int flags)
 {
 	struct sock *sk = sock->sk;
 
@@ -780,6 +790,7 @@ int inet_shutdown(struct socket *sock, int how)
 	release_sock(sk);
 	return err;
 }
+EXPORT_SYMBOL(inet_shutdown);
 
 /*
  *	ioctl() calls you can issue on an INET socket. Most of these are
@@ -798,44 +809,45 @@ int inet_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
 	struct net *net = sock_net(sk);
 
 	switch (cmd) {
-		case SIOCGSTAMP:
-			err = sock_get_timestamp(sk, (struct timeval __user *)arg);
-			break;
-		case SIOCGSTAMPNS:
-			err = sock_get_timestampns(sk, (struct timespec __user *)arg);
-			break;
-		case SIOCADDRT:
-		case SIOCDELRT:
-		case SIOCRTMSG:
-			err = ip_rt_ioctl(net, cmd, (void __user *)arg);
-			break;
-		case SIOCDARP:
-		case SIOCGARP:
-		case SIOCSARP:
-			err = arp_ioctl(net, cmd, (void __user *)arg);
-			break;
-		case SIOCGIFADDR:
-		case SIOCSIFADDR:
-		case SIOCGIFBRDADDR:
-		case SIOCSIFBRDADDR:
-		case SIOCGIFNETMASK:
-		case SIOCSIFNETMASK:
-		case SIOCGIFDSTADDR:
-		case SIOCSIFDSTADDR:
-		case SIOCSIFPFLAGS:
-		case SIOCGIFPFLAGS:
-		case SIOCSIFFLAGS:
-			err = devinet_ioctl(net, cmd, (void __user *)arg);
-			break;
-		default:
-			if (sk->sk_prot->ioctl)
-				err = sk->sk_prot->ioctl(sk, cmd, arg);
-			else
-				err = -ENOIOCTLCMD;
-			break;
+	case SIOCGSTAMP:
+		err = sock_get_timestamp(sk, (struct timeval __user *)arg);
+		break;
+	case SIOCGSTAMPNS:
+		err = sock_get_timestampns(sk, (struct timespec __user *)arg);
+		break;
+	case SIOCADDRT:
+	case SIOCDELRT:
+	case SIOCRTMSG:
+		err = ip_rt_ioctl(net, cmd, (void __user *)arg);
+		break;
+	case SIOCDARP:
+	case SIOCGARP:
+	case SIOCSARP:
+		err = arp_ioctl(net, cmd, (void __user *)arg);
+		break;
+	case SIOCGIFADDR:
+	case SIOCSIFADDR:
+	case SIOCGIFBRDADDR:
+	case SIOCSIFBRDADDR:
+	case SIOCGIFNETMASK:
+	case SIOCSIFNETMASK:
+	case SIOCGIFDSTADDR:
+	case SIOCSIFDSTADDR:
+	case SIOCSIFPFLAGS:
+	case SIOCGIFPFLAGS:
+	case SIOCSIFFLAGS:
+		err = devinet_ioctl(net, cmd, (void __user *)arg);
+		break;
+	default:
+		if (sk->sk_prot->ioctl)
+			err = sk->sk_prot->ioctl(sk, cmd, arg);
+		else
+			err = -ENOIOCTLCMD;
+		break;
 	}
 	return err;
 }
+EXPORT_SYMBOL(inet_ioctl);
 
 const struct proto_ops inet_stream_ops = {
 	.family		   = PF_INET,
@@ -862,6 +874,7 @@ const struct proto_ops inet_stream_ops = {
 	.compat_getsockopt = compat_sock_common_getsockopt,
 #endif
 };
+EXPORT_SYMBOL(inet_stream_ops);
 
 const struct proto_ops inet_dgram_ops = {
 	.family		   = PF_INET,
@@ -887,6 +900,7 @@ const struct proto_ops inet_dgram_ops = {
 	.compat_getsockopt = compat_sock_common_getsockopt,
 #endif
 };
+EXPORT_SYMBOL(inet_dgram_ops);
 
 /*
  * For SOCK_RAW sockets; should be the same as inet_dgram_ops but without
@@ -1016,6 +1030,7 @@ out_illegal:
 	       p->type);
 	goto out;
 }
+EXPORT_SYMBOL(inet_register_protosw);
 
 void inet_unregister_protosw(struct inet_protosw *p)
 {
@@ -1031,6 +1046,7 @@ void inet_unregister_protosw(struct inet_protosw *p)
 		synchronize_net();
 	}
 }
+EXPORT_SYMBOL(inet_unregister_protosw);
 
 /*
  *      Shall we try to damage output packets if routing dev changes?
@@ -1141,7 +1157,6 @@ int inet_sk_rebuild_header(struct sock *sk)
 
 	return err;
 }
-
 EXPORT_SYMBOL(inet_sk_rebuild_header);
 
 static int inet_gso_send_check(struct sk_buff *skb)
@@ -1369,7 +1384,6 @@ int inet_ctl_sock_create(struct sock **sk, unsigned short family,
 	}
 	return rc;
 }
-
 EXPORT_SYMBOL_GPL(inet_ctl_sock_create);
 
 unsigned long snmp_fold_field(void *mib[], int offt)
@@ -1676,19 +1690,3 @@ static int __init ipv4_proc_init(void)
 
 MODULE_ALIAS_NETPROTO(PF_INET);
 
-EXPORT_SYMBOL(inet_accept);
-EXPORT_SYMBOL(inet_bind);
-EXPORT_SYMBOL(inet_dgram_connect);
-EXPORT_SYMBOL(inet_dgram_ops);
-EXPORT_SYMBOL(inet_getname);
-EXPORT_SYMBOL(inet_ioctl);
-EXPORT_SYMBOL(inet_listen);
-EXPORT_SYMBOL(inet_register_protosw);
-EXPORT_SYMBOL(inet_release);
-EXPORT_SYMBOL(inet_sendmsg);
-EXPORT_SYMBOL(inet_shutdown);
-EXPORT_SYMBOL(inet_sock_destruct);
-EXPORT_SYMBOL(inet_stream_connect);
-EXPORT_SYMBOL(inet_stream_ops);
-EXPORT_SYMBOL(inet_unregister_protosw);
-EXPORT_SYMBOL(sysctl_ip_nonlocal_bind);

^ permalink raw reply related

* Re: [PATCH 3/3] tracing/events: convert NAPI's tracepoint and fix DEFINE_TRACE_FN
From: Neil Horman @ 2009-08-27 12:56 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Ingo Molnar, Steven Rostedt, Frederic Weisbecker, David Miller,
	Wei Yongjun, LKML, Netdev
In-Reply-To: <4A9664C2.3000004@cn.fujitsu.com>

On Thu, Aug 27, 2009 at 06:49:38PM +0800, Xiao Guangrong wrote:
> 
> 
> Xiao Guangrong wrote:
> > References: <4A966288.8020402@cn.fujitsu.com>
> > In-Reply-To: <4A966288.8020402@cn.fujitsu.com>
> > Content-Type: text/plain; charset=UTF-8
> > Content-Transfer-Encoding: 7bit
> > 
> 
> It's look like my mail client has some problems, I'll check it and resend
> this mail. Sorry!
> 
> Thanks,
> Xiao
> 
Please do, thank you!
Neil

> > - Convert NAPI's tracepoint via TRACE_EVENT macro, the output information
> >   like below:
> > 
> >    sshd-2503  [000]    71.920846: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
> >    sshd-2503  [000]    72.020291: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
> >    sshd-2503  [000]    72.020418: napi_poll: ifname=eth0 state=NAPI_STATE_SCHED weigth=16 poll=pcnet32_poll
> > 
> > - Remove the "DECLARE_TRACE" definiens in include/trace/define_trace.h,
> >   because TRACE_EVENT not use it
> > 
> > - Undef DEFINE_TRACE_FN after TRACE_EVENT/TRACE_EVENT_FN expanded,
> >   then we can include more TRACE_EVENT .h file in one .c file
> >   (the .c file has defined CREATE_TRACE_POINTS)
> > 
> > Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> 
> 

^ permalink raw reply

* Re: [PATCH 1/3] drop_monitor: fix trace_napi_poll_hit()
From: Neil Horman @ 2009-08-27 12:46 UTC (permalink / raw)
  To: Xiao Guangrong
  Cc: Ingo Molnar, Steven Rostedt, Frederic Weisbecker, David Miller,
	Wei Yongjun, LKML, Netdev
In-Reply-To: <4A966288.8020402@cn.fujitsu.com>

On Thu, Aug 27, 2009 at 06:40:08PM +0800, Xiao Guangrong wrote:
> The net_dev of backlog napi is NULL, like below:
> 
> __get_cpu_var(softnet_data).backlog.dev == NULL
> 
> So, we should check it in napi tracepoint's probe function
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
> ---
>  net/core/drop_monitor.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
> index 9d66fa9..d311202 100644
> --- a/net/core/drop_monitor.c
> +++ b/net/core/drop_monitor.c
> @@ -182,7 +182,8 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
>  	/*
>  	 * Ratelimit our check time to dm_hw_check_delta jiffies
>  	 */
> -	if (!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
> +	if (!napi->dev ||
> +	    !time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
>  		return;
>  
>  	rcu_read_lock();
> -- 
> 1.6.1.2
> 
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Thanks!
Neil


^ permalink raw reply

* Re: TCP keepalive timer problem
From: Eric Dumazet @ 2009-08-27 12:45 UTC (permalink / raw)
  To: Li_Xin2; +Cc: linux-kernel, netdev
In-Reply-To: <0939B589FC103041945B9F13274963E303B1AD89@CORPUSMX90A.corp.emc.com>

Please dont top post on these lists, find my answers below

Li_Xin2@emc.com a écrit :
>  
> Thanks for your quick reply, let me explain my problem in detail.
> 
> Suppose the client side of communication sets the keep alive socket option, connects to
> server, then > we pulls out the network cable of server box. After the connection is idle for TCP_KEEPIDLE 

seconds, the first keepalive probe packet is sent, and of course no reply is received. 

Just after the first probe packet, the client sends some data. No response is received, and 

as you said, the normal retransmission takes place and no further keepalive probe will be sent. 
> 
> 	The problem is: application that tries the keepalive mechanism expects communication peer 

crash detection within TCP_KEEPIDLE + TCP_KEEPCNT * TCP_KEEPINTVL seconds. Application may set

 relative smaller TCP_KEEPIDLE, TCP_KEEPCNT and TCP_KEEPINTVL value so that peer crash can be

 detected quickly, for example, 60 seconds. But if the keepalive is intervened with 

retransmission, the latter takes higher priority, so that peer crash will be detected after

 13 to 30 minutes, which may not be acceptable for some applications.
> 
> We tried TCP implementation on Windows XP SP3, the keepalive and retransmission don't intervene.
> 


> Regards,
> Xin Li
> EMC Shanghai R&D Centre
> Email: Li_Xin2@emc.com
> Tel: 86 21 6095 1100 x 2257
> 
> -----Original Message-----
> From: Eric Dumazet [mailto:eric.dumazet@gmail.com] 
> Sent: 2009年8月25日 21:13
> To: Li, Xin
> Cc: linux-kernel@vger.kernel.org; Linux Netdev List
> Subject: Re: TCP keepalive timer problem
> 
> Li_Xin2@emc.com a écrit :
>> Greetings,
>>
>> I found one problem in Linux TCP keepalive timer processing, after
>> searching on google, I found Daniel Stempel reported the same problem in
>> 2007 (http://lkml.indiana.edu/hypermail/linux/kernel/0702.2/1136.html),
>> but got no answer. So I have to reraise it.
>>
>> Can anyone help answer this two-years long question?
>>
>>
> 
> You should explain your problem in detail, since Daniel one was probably different.
> 
> He mentioned "(timeout is set to e.g. 30 seconds)" which is kind of nasty, given normal one is 7200
> 
> If some packets are in flight, keepalive is not fired at all, since normal
> retransmits should take place (check tcp_retries2 sysctl).
> 
> TCP Keepalive is only fired when no trafic occurred for a long time, only if 
> SO_KEEPALIVE socket option was enabled by application.
> 
> tcp_retries2 (integer; default: 15)
>     The maximum number of times a TCP packet is retransmitted in established state
> before giving up. The default value is 15, which corresponds to a duration of
> approximately between 13 to 30 minutes, depending on the retransmission timeout.
> The RFC 1122 specified minimum limit of 100 seconds is typically deemed too short. 
> 

RFC1122 , section 4.2.3.6 tells :

Keep-alive packets MUST only be sent when no data or acknowledgement packets have
 been received for the connection within an interval. This interval MUST be 
configurable and MUST default to no less than two hours. 

So :

Normal tcp_retries2 settings should make sure connection is reset if packets in flight are not acknowledged way before TCP_KEEPIDLE (>= 7200 seconds)


Now, 7200 seconds might be inappropriate for special needs, and considering
there is no way to change tcp_retries2 for a given socket (only choice being the global
tcp_retries2 setting), I would vote for a change in our stack, to *relax* RFC,
and get smaller keepalive timers if possible.

So when keepalive_timer fires, we should not care of outgoing packets,
only care on tp->rcv_tstamp, timestamp of last received ACK.


diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index b144a26..719f198 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -484,18 +484,13 @@ static void tcp_keepalive_timer (unsigned long data)
 			}
 		}
 		tcp_send_active_reset(sk, GFP_ATOMIC);
-		goto death;
+		tcp_done(sk);
+		goto out;
 	}
 
 	if (!sock_flag(sk, SOCK_KEEPOPEN) || sk->sk_state == TCP_CLOSE)
 		goto out;
 
-	elapsed = keepalive_time_when(tp);
-
-	/* It is alive without keepalive 8) */
-	if (tp->packets_out || tcp_send_head(sk))
-		goto resched;
-
 	elapsed = tcp_time_stamp - tp->rcv_tstamp;
 
 	if (elapsed >= keepalive_time_when(tp)) {
@@ -522,13 +517,7 @@ static void tcp_keepalive_timer (unsigned long data)
 	TCP_CHECK_TIMER(sk);
 	sk_mem_reclaim(sk);
 
-resched:
 	inet_csk_reset_keepalive_timer (sk, elapsed);
-	goto out;
-
-death:
-	tcp_done(sk);
-
 out:
 	bh_unlock_sock(sk);
 	sock_put(sk);

^ permalink raw reply related

* Re: [PATCH 1/2] alchemy: add au1000-eth platform device
From: Florian Fainelli @ 2009-08-27 12:42 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Ralf Baechle, linux-mips, Manuel Lauss, David Miller, netdev
In-Reply-To: <4A92D5D1.60009@ru.mvista.com>

Hello,

Le Monday 24 August 2009 20:02:57 Sergei Shtylyov, vous avez écrit :
> Hello.
>
> Florian Fainelli wrote:
> >>>This patch adds the board code to register a per-board au1000-eth
> >>>platform device to be used wit the au1000-eth platform driver in a
> >>>subsequent patch. Note that the au1000-eth driver knows about the
> >>>default driver settings such that we do not need to pass any
> >>>platform_data informations in most cases except db1x00.
> >>
> >>    Sigh, NAK...
> >>    Please don't register the SoC device per board, do it in
> >>alchemy/common/platfrom.c and find a way to pass the board specific
> >>platform data from the board file there instead -- something like
> >>arch/arm/mach-davinci/usb.c does.
> >
> > Ok, like I promised, this was the per-board device registration. Do you
> > prefer something like this:
>
>     I certainly do, but still not in this incarnation... :-)
>
> > --
> > From fd75b7c7fa3c05c21122c43e43260d2785475a79 Mon Sep 17 00:00:00 2001
> > From: Florian Fainelli <florian@openwrt.org>
> > Date: Tue, 18 Aug 2009 17:53:21 +0200
> > Subject: [PATCH] alchemy: add au1000-eth platform device (v2)
> >
> > This patch makes the board code register the au1000-eth
> > platform device. The au1000-eth platform data can be
> > overriden with the au1xxx_override_eth0_cfg function
> > like it has to be done for the Bosporus board.
> >
> > Changes from v1:
> > - remove per-board platform.c file
> > - add an override function to pass custom eth0 platform_data PHY settings
> >
> > Signed-off-by: Florian Fainelli <florian@openwrt.org>
> > ---
> > diff --git a/arch/mips/alchemy/common/platform.c
> > b/arch/mips/alchemy/common/platform.c index 117f99f..559294a 100644
> > --- a/arch/mips/alchemy/common/platform.c
> > +++ b/arch/mips/alchemy/common/platform.c
> > @@ -19,6 +19,7 @@
> >  #include <asm/mach-au1x00/au1xxx.h>
> >  #include <asm/mach-au1x00/au1xxx_dbdma.h>
> >  #include <asm/mach-au1x00/au1100_mmc.h>
> > +#include <asm/mach-au1x00/au1xxx_eth.h>
> >
> >  #define PORT(_base, _irq)				\
> >  	{						\
> > @@ -331,6 +332,76 @@ static struct platform_device pbdb_smbus_device = {
> >  };
> >  #endif
> >
> > +/* Macro to help defining the Ethernet MAC resources */
> > +#define MAC_RES(_base, _enable, _irq)			\
> > +	{						\
> > +		.start	= CPHYSADDR(_base),		\
> > +		.end	= CPHYSADDR(_base + 0xffff),	\
> > +		.flags	= IORESOURCE_MEM,		\
> > +	},						\
> > +	{						\
> > +		.start	= CPHYSADDR(_enable),		\
> > +		.end	= CPHYSADDR(_enable + 0x3),	\
> > +		.flags	= IORESOURCE_MEM,		\
> > +	},						\
> > +	{						\
> > +		.start	= _irq,				\
> > +		.end	= _irq,				\
> > +		.flags	= IORESOURCE_IRQ		\
> > +	}
> > +
> > +static struct resource au1xxx_eth0_resources[] = {
> > +#if defined(CONFIG_SOC_AU1000)
> > +	MAC_RES(AU1000_ETH0_BASE, AU1000_MAC0_ENABLE, AU1000_MAC0_DMA_INT),
> > +#elif defined(CONFIG_SOC_AU1100)
> > +	MAC_RES(AU1100_ETH0_BASE, AU1100_MAC0_ENABLE, AU1100_MAC0_DMA_INT),
> > +#elif defined(CONFIG_SOC_AU1550)
> > +	MAC_RES(AU1550_ETH0_BASE, AU1550_MAC0_ENABLE, AU1550_MAC0_DMA_INT),
> > +#elif defined(CONFIG_SOC_AU1500)
> > +	MAC_RES(AU1500_ETH0_BASE, AU1500_MAC0_ENABLE, AU1500_MAC0_DMA_INT),
> > +#endif
> > +};
> > +
> > +static struct resource au1xxx_eth1_resources[] = {
> > +#if defined(CONFIG_SOC_AU1000)
> > +	MAC_RES(AU1000_ETH1_BASE, AU1000_MAC1_ENABLE, AU1000_MAC1_DMA_INT),
> > +#elif defined(CONFIG_SOC_AU1550)
> > +	MAC_RES(AU1550_ETH1_BASE, AU1550_MAC1_ENABLE, AU1550_MAC1_DMA_INT),
> > +#elif defined(CONFIG_SOC_AU1500)
> > +	MAC_RES(AU1500_ETH1_BASE, AU1500_MAC1_ENABLE, AU1500_MAC1_DMA_INT),
> > +#endif
> > +};
> > +
> > +static struct au1000_eth_platform_data au1xxx_eth0_platform_data = {
> > +	.phy1_search_mac0 = 1,
> > +};
>
>     I'm not sure that the default platfrom data is really a great idea...

Can you elaborate a bit more ? We actually need to make the Ethernet MAC driver aware of some PHY settings.

>
> > +#ifndef CONFIG_SOC_AU1100
> > +static struct platform_device au1xxx_eth1_device = {
> > +	.name		= "au1000-eth",
> > +	.id		= 1,
> > +	.num_resources	= ARRAY_SIZE(au1xxx_eth1_resources),
> > +	.resource	= au1xxx_eth1_resources,
>
>     And where's the platfrom data for the second Ethernet?

There is no need to, as the driver originally did not override any specific settings on the second MAC (afair).

>
> > +};
> > +#endif
> > +
> > +void __init au1xxx_override_eth0_cfg(struct au1000_eth_platform_data
> > *eth_data) +{
> > +	if (!eth_data)
> > +		return;
> > +
> > +	memcpy(&au1xxx_eth0_platform_data, eth_data,
> > +		sizeof(struct au1000_eth_platform_data));
>
>     Why not just set the pointer in au1xxx_eth0_device. And really, why not
> make the function more generic, with a prototype like:

For the same reasons as explained below, MAC1 did not need any specific change.

>
> void __init au1xxx_override_eth_cfg(unsigned port, struct
> 				    au1000_eth_platform_data *eth_data);
>
> > +}
> > +
> >  static struct platform_device *au1xxx_platform_devices[] __initdata = {
> >  	&au1xx0_uart_device,
> >  	&au1xxx_usb_ohci_device,
> > @@ -351,17 +422,25 @@ static struct platform_device
> > *au1xxx_platform_devices[] __initdata = { #ifdef SMBUS_PSC_BASE
> >  	&pbdb_smbus_device,
> >  #endif
> > +	&au1xxx_eth0_device,
> >  };
> >
> >  static int __init au1xxx_platform_init(void)
> >  {
> >  	unsigned int uartclk = get_au1x00_uart_baud_base() * 16;
> > -	int i;
> > +	int i, ni;
> >
> >  	/* Fill up uartclk. */
> >  	for (i = 0; au1x00_uart_data[i].flags; i++)
> >  		au1x00_uart_data[i].uartclk = uartclk;
> >
> > +	/* Register second MAC if enabled in pinfunc */
> > +#ifndef CONFIG_SOC_AU1100
> > +	ni = (int)((au_readl(SYS_PINFUNC) & (u32)(SYS_PF_NI2)) >> 4);
> > +	if (!(ni + 1))
>
>     Why so complex, and how can (ni + 1) ever be 0?! :-/

This is left-over debugging stub, I will rework it. About complexity, this line is taken directly from the au1000_eth driver.

>     Doesn't that field when 0 mean the pins configured for MAC1 and when 1
> -- for GPIO? Why not just:
>
> 	if (!(au_readl(SYS_PINFUNC) & SYS_PF_NI2))
>
> > +		platform_device_register(&au1xxx_eth1_device);
> > +#endif
> > +
>
> WBR, Sergei



-- 
Best regards, Florian Fainelli
Email: florian@openwrt.org
Web: http://openwrt.org
IRC: [florian] on irc.freenode.net
-------------------------------

^ permalink raw reply

* Re: [evb] RE: [PATCH][RFC] net/bridge: add basic VEPA support
From: Or Gerlitz @ 2009-08-27 12:35 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Paul Congdon (UC Davis), arnd, anna.fischer, netdev, bridge,
	davem, adobriyan, virtualization, evb@yahoogroups.com
In-Reply-To: <20090810082037.17b352c2@nehalam>

Stephen Hemminger wrote:
> Or Gerlitz <ogerlitz@voltaire.com> wrote:
>> Looking in macvlan_set_multicast_list() it acts in a similar manner to macvlan_set_mac_address() in the sense that it calls dev_mc_sync(). I assume what's left is to add macvlan_hash_xxx multicast logic to map/unmap multicast groups to what macvlan devices want to receive them and this way the flooding can be removed, correct?
> The device can just flood all multicast packets, since the filtering is done on the receive path anyway.
for each multicast packet, macvlan_broadcast is invoked and calls 
skb_clone/ netif_rx for each device, now a smart scheme that takes into 
account (hash) the multicast list of the different macvlan  devices 
would save the skb_clone call, isn't it?

Or.


^ permalink raw reply

* [PATCH net-next-2.6] tcp: keepalive cleanups
From: Eric Dumazet @ 2009-08-27 12:26 UTC (permalink / raw)
  To: David S. Miller; +Cc: Linux Netdev List

Introduce keepalive_probes(tp) helper, and use it, like 
keepalive_time_when(tp) and keepalive_intvl_when(tp)

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/tcp.h    |    5 +++++
 net/ipv4/tcp.c       |    6 +++---
 net/ipv4/tcp_timer.c |    3 +--
 3 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 88af843..cbb2a48 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1007,6 +1007,11 @@ static inline int keepalive_time_when(const struct tcp_sock *tp)
 	return tp->keepalive_time ? : sysctl_tcp_keepalive_time;
 }
 
+static inline int keepalive_probes(const struct tcp_sock *tp)
+{
+	return tp->keepalive_probes ? : sysctl_tcp_keepalive_probes;
+}
+
 static inline int tcp_fin_time(const struct sock *sk)
 {
 	int fin_timeout = tcp_sk(sk)->linger2 ? : sysctl_tcp_fin_timeout;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9114524..59f69a6 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2336,13 +2336,13 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
 		val = !!(tp->nonagle&TCP_NAGLE_CORK);
 		break;
 	case TCP_KEEPIDLE:
-		val = (tp->keepalive_time ? : sysctl_tcp_keepalive_time) / HZ;
+		val = keepalive_time_when(tp) / HZ;
 		break;
 	case TCP_KEEPINTVL:
-		val = (tp->keepalive_intvl ? : sysctl_tcp_keepalive_intvl) / HZ;
+		val = keepalive_intvl_when(tp) / HZ;
 		break;
 	case TCP_KEEPCNT:
-		val = tp->keepalive_probes ? : sysctl_tcp_keepalive_probes;
+		val = keepalive_probes(tp);
 		break;
 	case TCP_SYNCNT:
 		val = icsk->icsk_syn_retries ? : sysctl_tcp_syn_retries;
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index b144a26..c520fb6 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -499,8 +499,7 @@ static void tcp_keepalive_timer (unsigned long data)
 	elapsed = tcp_time_stamp - tp->rcv_tstamp;
 
 	if (elapsed >= keepalive_time_when(tp)) {
-		if ((!tp->keepalive_probes && icsk->icsk_probes_out >= sysctl_tcp_keepalive_probes) ||
-		     (tp->keepalive_probes && icsk->icsk_probes_out >= tp->keepalive_probes)) {
+		if (icsk->icsk_probes_out >= keepalive_probes(tp)) {
 			tcp_send_active_reset(sk, GFP_ATOMIC);
 			tcp_write_err(sk);
 			goto out;

^ permalink raw reply related

* Re: [PATCH 7/7] MAINTAINERS: update information for sfc network driver
From: Ben Hutchings @ 2009-08-27 12:23 UTC (permalink / raw)
  To: David Miller; +Cc: joe, netdev, linux-net-drivers
In-Reply-To: <20090826.180445.102441324.davem@davemloft.net>

On Wed, 2009-08-26 at 18:04 -0700, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Wed, 26 Aug 2009 11:47:47 -0700
> 
> > On Wed, 2009-08-26 at 19:19 +0100, Ben Hutchings wrote:
> >> Update the current maintainers and add our email addresses.  Robert
> >> Stonehouse has moved on to other projects.
> >> 
> >> Move the linux-net-drivers alias to the top since it should always
> >> reach the current maintainers at Solarflare.  Add netdev as the
> >> relevant mailing list.
> > []
> >> +M:	linux-net-drivers@solarflare.com (alias for current maintainers)
> > 
> > Hi Ben
> > 
> > This form: "M:	bare address (extra stuff)" doesn't work
> > with the get_maintainers script.
> > 
> > Can you please use something like this instead?
> > 
> > M:	Solarflare linux maintainers <linux-net-drivers@solarflare.com>
> > 
> > Perhaps this diff below.
> 
> Joe's version seems based upon reasonable requests, so I've
> applied his diff.
> 
> Thanks.

Ack, and thanks.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* [PATCH] tipc: fix test of bearer_priority range in tipc_register_media()
From: Roel Kluin @ 2009-08-27 12:03 UTC (permalink / raw)
  To: Per Liden, tipc-discussion, Andrew Morton, David S. Miller,
	netdev

For the bearer_priority to be less than TIPC_MIN_LINK_PRI and greater than
TIPC_MAX_LINK_PRI is logically impossible.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
index a7a3677..327011f 100644
--- a/net/tipc/bearer.c
+++ b/net/tipc/bearer.c
@@ -119,7 +119,7 @@ int  tipc_register_media(u32 media_type,
 		warn("Media <%s> rejected, no broadcast address\n", name);
 		goto exit;
 	}
-	if ((bearer_priority < TIPC_MIN_LINK_PRI) &&
+	if ((bearer_priority < TIPC_MIN_LINK_PRI) ||
 	    (bearer_priority > TIPC_MAX_LINK_PRI)) {
 		warn("Media <%s> rejected, illegal priority (%u)\n", name,
 		     bearer_priority);

^ permalink raw reply related

* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-27 11:19 UTC (permalink / raw)
  To: Rusty Russell
  Cc: virtualization, netdev, kvm, linux-kernel, mingo, linux-mm, akpm,
	hpa, gregory.haskins
In-Reply-To: <200908272040.02628.rusty@rustcorp.com.au>

On Thu, Aug 27, 2009 at 08:40:02PM +0930, Rusty Russell wrote:
> On Thu, 27 Aug 2009 08:15:18 pm Michael S. Tsirkin wrote:
> > On Tue, Aug 25, 2009 at 09:40:40PM +0930, Rusty Russell wrote:
> > > Also, see other fixes to the lguest launcher since then which might
> > > be relevant to this code:
> > > 	lguest: get more serious about wmb() in example Launcher code
> > 
> > Heh, this just gets one step closer to a real wmb.  I just used the
> > correct code from linux, so I think nothing needs to be done in vhost.
> > Apropos this change in lguest: why is a compiler barrier sufficient? The
> > comment says devices are run in separate threads (presumably from
> > guest?), if so don't you need to tell CPU that there's a barrier as
> > well?
> 
> Yep, but x86 only :)

Okay, writes are ordered them. But mb() orders reads as well,
so it'll have to have a real one IMO, even in userspace?

>  The kernel uses a real insn if XMM/XMM2, but I don't
> know if userspace needs that.  I just use compiler barriers.
> 
> Thanks,
> Rusty.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH] ip6tables: Read outside array bounds
From: Roel Kluin @ 2009-08-27 11:23 UTC (permalink / raw)
  To: David S. Miller, Patrick McHardy, netdev, Andrew Morton

Check bounds before reading from the s6_addr array. It read 1 past
the end at s6_addr[16] and eui64[] was also read 1 past the end.

Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
diff --git a/net/ipv6/netfilter/ip6t_eui64.c b/net/ipv6/netfilter/ip6t_eui64.c
index db610ba..7b40a20 100644
--- a/net/ipv6/netfilter/ip6t_eui64.c
+++ b/net/ipv6/netfilter/ip6t_eui64.c
@@ -43,8 +43,8 @@ eui64_mt6(const struct sk_buff *skb, const struct xt_match_param *par)
 			eui64[0] ^= 0x02;
 
 			i = 0;
-			while (ipv6_hdr(skb)->saddr.s6_addr[8 + i] == eui64[i]
-			       && i < 8)
+			while (i < 8 && ipv6_hdr(skb)->saddr.s6_addr[8 + i] ==
+					eui64[i])
 				i++;
 
 			if (i == 8)

^ permalink raw reply related

* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Rusty Russell @ 2009-08-27 11:10 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: virtualization, netdev, kvm, linux-kernel, mingo, linux-mm, akpm,
	hpa, gregory.haskins
In-Reply-To: <20090827104517.GB8545@redhat.com>

On Thu, 27 Aug 2009 08:15:18 pm Michael S. Tsirkin wrote:
> On Tue, Aug 25, 2009 at 09:40:40PM +0930, Rusty Russell wrote:
> > Also, see other fixes to the lguest launcher since then which might
> > be relevant to this code:
> > 	lguest: get more serious about wmb() in example Launcher code
> 
> Heh, this just gets one step closer to a real wmb.  I just used the
> correct code from linux, so I think nothing needs to be done in vhost.
> Apropos this change in lguest: why is a compiler barrier sufficient? The
> comment says devices are run in separate threads (presumably from
> guest?), if so don't you need to tell CPU that there's a barrier as
> well?

Yep, but x86 only :)  The kernel uses a real insn if XMM/XMM2, but I don't
know if userspace needs that.  I just use compiler barriers.

Thanks,
Rusty.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Rusty Russell @ 2009-08-27 11:03 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Arnd Bergmann, virtualization, kvm, netdev, linux-kernel,
	linux-mm, hpa, mingo, akpm
In-Reply-To: <20090827101026.GA8545@redhat.com>

On Thu, 27 Aug 2009 07:40:26 pm Michael S. Tsirkin wrote:
> On Wed, Aug 26, 2009 at 03:40:59PM +0200, Arnd Bergmann wrote:
> > On Tuesday 25 August 2009, Michael S. Tsirkin wrote:
> > > >  I'd like to avoid that here,
> > > > though it's kind of ugly.  We'd need VHOST_GET_FEATURES (and ACK) to take a
> > > > struct like:
> > > > 
> > > >       u32 feature_size;
> > > >       u32 features[];
> > 
> > Hmm, variable length ioctl arguments, I'd rather not go there.
> > The ioctl infrastructure already has a length argument encoded
> > in the ioctl number. We can use that if we need more, e.g.
> > 
> > /* now */
> > #define VHOST_GET_FEATURES     _IOR(VHOST_VIRTIO, 0x00, __u64)
> > /*
> >  * uncomment if we run out of feature bits:
> > 
> > struct vhost_get_features2 {
> > 	__u64 bits[2];
> > };
> > #define VHOST_GET_FEATURES2     _IOR(VHOST_VIRTIO, 0x00, \
> > 			struct  vhost_get_features2)
> >  */
> 
> 
> I thought so, too. Rusty, agree?

Yep, am convinced.  Make it u64 to stop us having to do this tomorrow, then
we can always extend later.

Thanks,
Rusty.

^ permalink raw reply

* [PATCH 3/3] tracing/events: convert NAPI's tracepoint and fix DEFINE_TRACE_FN
From: Xiao Guangrong @ 2009-08-27 10:56 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Steven Rostedt, Frederic Weisbecker, Neil Horman, David Miller,
	Wei Yongjun, LKML, Netdev
In-Reply-To: <4A966288.8020402@cn.fujitsu.com>

- Convert NAPI's tracepoint via TRACE_EVENT macro, the output information
  like below:

   sshd-2503  [000]    71.920846: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
   sshd-2503  [000]    72.020291: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
   sshd-2503  [000]    72.020418: napi_poll: ifname=eth0 state=NAPI_STATE_SCHED weigth=16 poll=pcnet32_poll

- Remove the "DECLARE_TRACE" definiens in include/trace/define_trace.h,
  because TRACE_EVENT not use it

- Undef DEFINE_TRACE_FN after TRACE_EVENT/TRACE_EVENT_FN expanded,
  then we can include more TRACE_EVENT .h file in one .c file
  (the .c file has defined CREATE_TRACE_POINTS)

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 include/trace/define_trace.h |    5 +---
 include/trace/events/napi.h  |   43 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index a89ed59..6158741 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -31,10 +31,6 @@
 		assign, print, reg, unreg)			\
 	DEFINE_TRACE_FN(name, reg, unreg)
 
-#undef DECLARE_TRACE
-#define DECLARE_TRACE(name, proto, args)	\
-	DEFINE_TRACE(name)
-
 #undef TRACE_INCLUDE
 #undef __TRACE_INCLUDE
 
@@ -62,6 +58,7 @@
 #endif
 
 #undef TRACE_EVENT
+#undef TRACE_EVENT_FN
 #undef TRACE_HEADER_MULTI_READ
 
 /* Only undef what we defined in this file */
diff --git a/include/trace/events/napi.h b/include/trace/events/napi.h
index a8989c4..22802d1 100644
--- a/include/trace/events/napi.h
+++ b/include/trace/events/napi.h
@@ -1,11 +1,44 @@
-#ifndef _TRACE_NAPI_H_
-#define _TRACE_NAPI_H_
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM napi
+
+#if !defined(_TRACE_NAPI_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_NAPI_H
 
 #include <linux/netdevice.h>
 #include <linux/tracepoint.h>
 
-DECLARE_TRACE(napi_poll,
+#define show_napi_state(state) state ? __print_flags(state, "|",	\
+	{ (1UL << NAPI_STATE_SCHED),	"NAPI_STATE_SCHED"	},	\
+	{ (1UL << NAPI_STATE_DISABLE),	"NAPI_STATE_DISABLE"	},	\
+	{ (1UL << NAPI_STATE_NPSVC),	"NAPI_STATE_NPSVC"	}) : "0"
+
+TRACE_EVENT(napi_poll,
+
 	TP_PROTO(struct napi_struct *napi),
-	TP_ARGS(napi));
 
-#endif
+	TP_ARGS(napi),
+
+	TP_STRUCT__entry(
+		__field(	unsigned long,	state		)
+		__field(	int,		weight		)
+		__field(	void *,		poll		)
+		__string(	ifname,		(napi->dev ?
+				napi->dev->name : "backlog")	)
+	),
+
+	TP_fast_assign(
+		__entry->state	= napi->state;
+		__entry->weight	= napi->weight;
+		__entry->poll	= napi->poll;
+		__assign_str(ifname, napi->dev ? napi->dev->name : "backlog")
+	),
+
+	TP_printk("ifname=%s state=%s weigth=%d poll=%pf",
+		  __get_str(ifname), show_napi_state(__entry->state),
+		  __entry->weight, __entry->poll)
+);
+
+#endif /* _TRACE_NAPI_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
1.6.1.2



^ permalink raw reply related

* Re: [PATCH 3/3] tracing/events: convert NAPI's tracepoint and fix DEFINE_TRACE_FN
From: Xiao Guangrong @ 2009-08-27 10:49 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Steven Rostedt, Frederic Weisbecker, Neil Horman, David Miller,
	Wei Yongjun, LKML, Netdev
In-Reply-To: <4A96635C.9070404@cn.fujitsu.com>



Xiao Guangrong wrote:
> References: <4A966288.8020402@cn.fujitsu.com>
> In-Reply-To: <4A966288.8020402@cn.fujitsu.com>
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 7bit
> 

It's look like my mail client has some problems, I'll check it and resend
this mail. Sorry!

Thanks,
Xiao

> - Convert NAPI's tracepoint via TRACE_EVENT macro, the output information
>   like below:
> 
>    sshd-2503  [000]    71.920846: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
>    sshd-2503  [000]    72.020291: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
>    sshd-2503  [000]    72.020418: napi_poll: ifname=eth0 state=NAPI_STATE_SCHED weigth=16 poll=pcnet32_poll
> 
> - Remove the "DECLARE_TRACE" definiens in include/trace/define_trace.h,
>   because TRACE_EVENT not use it
> 
> - Undef DEFINE_TRACE_FN after TRACE_EVENT/TRACE_EVENT_FN expanded,
>   then we can include more TRACE_EVENT .h file in one .c file
>   (the .c file has defined CREATE_TRACE_POINTS)
> 
> Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>


^ permalink raw reply

* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-27 10:45 UTC (permalink / raw)
  To: Rusty Russell
  Cc: virtualization, netdev, kvm, linux-kernel, mingo, linux-mm, akpm,
	hpa, gregory.haskins
In-Reply-To: <200908252140.41295.rusty@rustcorp.com.au>

On Tue, Aug 25, 2009 at 09:40:40PM +0930, Rusty Russell wrote:
> Also, see other fixes to the lguest launcher since then which might
> be relevant to this code:
> 	lguest: get more serious about wmb() in example Launcher code

Heh, this just gets one step closer to a real wmb.  I just used the
correct code from linux, so I think nothing needs to be done in vhost.
Apropos this change in lguest: why is a compiler barrier sufficient? The
comment says devices are run in separate threads (presumably from
guest?), if so don't you need to tell CPU that there's a barrier as
well?

> 	lguest: clean up length-used value in example launcher

OK, fixing that.

Thanks!

-- 
MST

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH 3/3] tracing/events: convert NAPI's tracepoint and fix DEFINE_TRACE_FN
From: Xiao Guangrong @ 2009-08-27 10:43 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Steven Rostedt, Frederic Weisbecker, Neil Horman, David Miller,
	Wei Yongjun, LKML, Netdev

References: <4A966288.8020402@cn.fujitsu.com>
In-Reply-To: <4A966288.8020402@cn.fujitsu.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

- Convert NAPI's tracepoint via TRACE_EVENT macro, the output information
  like below:

   sshd-2503  [000]    71.920846: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
   sshd-2503  [000]    72.020291: napi_poll: ifname=eth0 state=0 weigth=16 poll=pcnet32_poll
   sshd-2503  [000]    72.020418: napi_poll: ifname=eth0 state=NAPI_STATE_SCHED weigth=16 poll=pcnet32_poll

- Remove the "DECLARE_TRACE" definiens in include/trace/define_trace.h,
  because TRACE_EVENT not use it

- Undef DEFINE_TRACE_FN after TRACE_EVENT/TRACE_EVENT_FN expanded,
  then we can include more TRACE_EVENT .h file in one .c file
  (the .c file has defined CREATE_TRACE_POINTS)

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 include/trace/define_trace.h |    5 +---
 include/trace/events/napi.h  |   43 +++++++++++++++++++++++++++++++++++++----
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index a89ed59..6158741 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -31,10 +31,6 @@
 		assign, print, reg, unreg)			\
 	DEFINE_TRACE_FN(name, reg, unreg)
 
-#undef DECLARE_TRACE
-#define DECLARE_TRACE(name, proto, args)	\
-	DEFINE_TRACE(name)
-
 #undef TRACE_INCLUDE
 #undef __TRACE_INCLUDE
 
@@ -62,6 +58,7 @@
 #endif
 
 #undef TRACE_EVENT
+#undef TRACE_EVENT_FN
 #undef TRACE_HEADER_MULTI_READ
 
 /* Only undef what we defined in this file */
diff --git a/include/trace/events/napi.h b/include/trace/events/napi.h
index a8989c4..22802d1 100644
--- a/include/trace/events/napi.h
+++ b/include/trace/events/napi.h
@@ -1,11 +1,44 @@
-#ifndef _TRACE_NAPI_H_
-#define _TRACE_NAPI_H_
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM napi
+
+#if !defined(_TRACE_NAPI_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_NAPI_H
 
 #include <linux/netdevice.h>
 #include <linux/tracepoint.h>
 
-DECLARE_TRACE(napi_poll,
+#define show_napi_state(state) state ? __print_flags(state, "|",	\
+	{ (1UL << NAPI_STATE_SCHED),	"NAPI_STATE_SCHED"	},	\
+	{ (1UL << NAPI_STATE_DISABLE),	"NAPI_STATE_DISABLE"	},	\
+	{ (1UL << NAPI_STATE_NPSVC),	"NAPI_STATE_NPSVC"	}) : "0"
+
+TRACE_EVENT(napi_poll,
+
 	TP_PROTO(struct napi_struct *napi),
-	TP_ARGS(napi));
 
-#endif
+	TP_ARGS(napi),
+
+	TP_STRUCT__entry(
+		__field(	unsigned long,	state		)
+		__field(	int,		weight		)
+		__field(	void *,		poll		)
+		__string(	ifname,		(napi->dev ?
+				napi->dev->name : "backlog")	)
+	),
+
+	TP_fast_assign(
+		__entry->state	= napi->state;
+		__entry->weight	= napi->weight;
+		__entry->poll	= napi->poll;
+		__assign_str(ifname, napi->dev ? napi->dev->name : "backlog")
+	),
+
+	TP_printk("ifname=%s state=%s weigth=%d poll=%pf",
+		  __get_str(ifname), show_napi_state(__entry->state),
+		  __entry->weight, __entry->poll)
+);
+
+#endif /* _TRACE_NAPI_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
-- 
1.6.1.2

^ permalink raw reply related

* [PATCH 1/3] drop_monitor: fix trace_napi_poll_hit()
From: Xiao Guangrong @ 2009-08-27 10:40 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Steven Rostedt, Frederic Weisbecker, Neil Horman, David Miller,
	Wei Yongjun, LKML, Netdev

The net_dev of backlog napi is NULL, like below:

__get_cpu_var(softnet_data).backlog.dev == NULL

So, we should check it in napi tracepoint's probe function

Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com>
---
 net/core/drop_monitor.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/core/drop_monitor.c b/net/core/drop_monitor.c
index 9d66fa9..d311202 100644
--- a/net/core/drop_monitor.c
+++ b/net/core/drop_monitor.c
@@ -182,7 +182,8 @@ static void trace_napi_poll_hit(struct napi_struct *napi)
 	/*
 	 * Ratelimit our check time to dm_hw_check_delta jiffies
 	 */
-	if (!time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
+	if (!napi->dev ||
+	    !time_after(jiffies, napi->dev->last_rx + dm_hw_check_delta))
 		return;
 
 	rcu_read_lock();
-- 
1.6.1.2

^ permalink raw reply related

* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Michael S. Tsirkin @ 2009-08-27 10:10 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: virtualization, Rusty Russell, kvm, netdev, linux-kernel,
	linux-mm, hpa, mingo, akpm
In-Reply-To: <200908261540.59900.arnd@arndb.de>

On Wed, Aug 26, 2009 at 03:40:59PM +0200, Arnd Bergmann wrote:
> On Tuesday 25 August 2009, Michael S. Tsirkin wrote:
> > >  I'd like to avoid that here,
> > > though it's kind of ugly.  We'd need VHOST_GET_FEATURES (and ACK) to take a
> > > struct like:
> > > 
> > >       u32 feature_size;
> > >       u32 features[];
> 
> Hmm, variable length ioctl arguments, I'd rather not go there.
> The ioctl infrastructure already has a length argument encoded
> in the ioctl number. We can use that if we need more, e.g.
> 
> /* now */
> #define VHOST_GET_FEATURES     _IOR(VHOST_VIRTIO, 0x00, __u64)
> /*
>  * uncomment if we run out of feature bits:
> 
> struct vhost_get_features2 {
> 	__u64 bits[2];
> };
> #define VHOST_GET_FEATURES2     _IOR(VHOST_VIRTIO, 0x00, \
> 			struct  vhost_get_features2)
>  */


I thought so, too. Rusty, agree?

-- 
MST

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCHv4 2/2] vhost_net: a kernel-level virtio server
From: Rusty Russell @ 2009-08-27  9:59 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: virtualization, netdev, kvm, linux-kernel, mingo, linux-mm, akpm,
	hpa, gregory.haskins
In-Reply-To: <20090826165655.GA23632@redhat.com>

On Thu, 27 Aug 2009 02:26:55 am Michael S. Tsirkin wrote:
> On Tue, Aug 25, 2009 at 04:16:34PM +0300, Michael S. Tsirkin wrote:
> > > > +	/* If they don't want an interrupt, don't send one, unless empty. */
> > > > +	if ((flags & VRING_AVAIL_F_NO_INTERRUPT) && vq->inflight)
> > > > +		return;
> > > 
> > > And I wouldn't support notify on empty at all, TBH.
> > 
> > If I don't, virtio net in guest uses a timer, which might be expensive.
> > Will need to check what this does.
> > 
> > >  It should
> > > definitely be conditional on the guest accepting the NOTIFY_ON_EMPTY
> > > feature.
> 
> lguest does not do it this way though, do it?

Does when a patch in my current queue is applied though.

Thanks,
Rusty.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [Bug #14016] mm/ipw2200 regression
From: Mel Gorman @ 2009-08-27  9:45 UTC (permalink / raw)
  To: Zhu Yi
  Cc: Andrew Morton, Johannes Weiner, Pekka Enberg, Rafael J. Wysocki,
	Linux Kernel Mailing List, Kernel Testers List,
	Bartlomiej Zolnierkiewicz, Mel Gorman, netdev@vger.kernel.org,
	linux-mm@kvack.org, James Ketrenos, Chatre, Reinette,
	linux-wireless@vger.kernel.org,
	ipw2100-devel@lists.sourceforge.net
In-Reply-To: <1251364289.3704.176.camel@debian>

On Thu, Aug 27, 2009 at 05:11:29PM +0800, Zhu Yi wrote:
> On Wed, 2009-08-26 at 22:44 +0800, Andrew Morton wrote:
> > 
> > It is perhaps pretty simple to make the second (GFP_ATOMIC) allocation
> > go away.  The code is already conveniently structured to do this:
> > 
> >         do {
> >                 chunk = (struct fw_chunk *)(data + offset);
> >                 offset += sizeof(struct fw_chunk);
> >                 /* build DMA packet and queue up for sending */
> >                 /* dma to chunk->address, the chunk->length bytes from
> > data +
> >                  * offeset*/
> >                 /* Dma loading */
> >                 rc = ipw_fw_dma_add_buffer(priv, shared_phys + offset,
> > 
> > le32_to_cpu(chunk->address),
> > 
> > le32_to_cpu(chunk->length));
> >                 if (rc) {
> >                         IPW_DEBUG_INFO("dmaAddBuffer Failed\n");
> >                         goto out;
> >                 }
> > 
> >                 offset += le32_to_cpu(chunk->length);
> >         } while (offset < len);
> > 
> > what is the typical/expected value of chunk->length here?  If it's
> > significantly less than 4096*(2^6), could we convert this function to
> > use a separate DMAable allocation per fw_chunk?
> 
> Unfortunately, the largest chunk size for the latest 3.1 firmware is
> 0x20040, which also requires order 6 page allocation. I'll try to use
> the firmware DMA command block (64 slots) to handle the image (each for
> 4k, totally 256k).
> 

That would be preferable as trying to make alloc-6 atomic allocations isn't
going to pan out. As I noted, doing it as GFP_KERNEL is possible but it'll
manifest as weird stalls periodically when the driver is loaded due to
reclaim and if the system is swapless, it might not work at all if memory
is mostly anonymous.

If the DMA command block doesn't work out, what is the feasibility of holding
onto the order-6 allocation once the module is loaded instead of allocing
for the duration of the firmware loading and then freeing it again?

-- 
Mel Gorman
Part-time Phd Student                          Linux Technology Center
University of Limerick                         IBM Dublin Software Lab

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Ingo Molnar @ 2009-08-27  9:34 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Neil Horman, David Miller, fweisbec, billfink, netdev, brice,
	gallatin
In-Reply-To: <alpine.DEB.2.00.0908262021300.18923@gandalf.stny.rr.com>


* Steven Rostedt <rostedt@goodmis.org> wrote:

> 
> On Wed, 26 Aug 2009, Neil Horman wrote:
> > > 
> > I'm not sure how the addition of an ftrace module constitutes a change to the
> > tracing infrastructure, but whatever, yes, no biggy.  I've bugun modifying the
> > TRACE_EVENT that I added to export the data I need directly.  Should be pretty
> > straightforward.  Dave I'll have a patch up on netdev in a day or two after I
> > test it.  Steven, should this still just go to netdev with a cc to you?  I'd
> > like to avoid repeating the same confusion here a second time around if I can
> 
> Yes, please Cc myself, and Ingo on those changes. I see where the 
> confusion came. It is where the code changes. The code in 
> kernel/trace is considered ftrace internals (there's internal 
> tracing upkeep that is needed for all plugins). [...]

yeah - i pointed that out in the very first mail to David 9 days ago 
when this patch broke the build in linux-next: kernel/trace/ is like 
net/core/. It would be nice and important if the networking tree 
treated it as such in the future.
 
See the:

  [PATCH -next] trace_skb: fix build when CONFIG_NET is not enabled

discussion on lkml:

  http://lkml.org/lkml/2009/8/17/378

Thanks,

	Ingo

^ permalink raw reply

* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Ingo Molnar @ 2009-08-27  9:28 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: David Miller, nhorman, rostedt, fweisbec, billfink, netdev, brice,
	gallatin
In-Reply-To: <20090827003536.GC30341@infradead.org>


* Christoph Hellwig <hch@infradead.org> wrote:

> On Thu, Aug 27, 2009 at 01:58:26AM +0200, Ingo Molnar wrote:
>
> > I'm sorry you got that impression, but you are a maintainer 
> > yourself so you might perhaps understand it why sooner or later, 
> > if a maintainer's review does not get acted upon, one has to 
> > insist on clean patches in stronger terms.
> 
> Cool down a bit :) [...]

Hello Pot, Kettle here ;-)

I guess i'll have to test the limits of your patience by queueing up 
some bad commit into fs/libfs.c via say the iommu tree, without acks 
and with commit log damage, which patch then triggers a build 
failure and a crash in linux-next (like this one did), and refuse to 
revert and not do anything substantial about your (initially polite) 
review feedback for 2 weeks (like it happened here), and see how 
measured your response will be after the 12th mail that gets faced 
with such passive-aggressive inaction ;-)

At that point, will your wall of patience finally start to crumble a 
tiny bit and will you resort to using the taboo term 'crappy patch' 
perhaps, like i did here? ;-)

Anyway, as Steve said it's now finally water under the bridge, time 
to move on.

	Ingo

^ permalink raw reply

* Re: [Bug #14016] mm/ipw2200 regression
From: Zhu Yi @ 2009-08-27  9:11 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Mel Gorman, Johannes Weiner, Pekka Enberg, Rafael J. Wysocki,
	Linux Kernel Mailing List, Kernel Testers List,
	Bartlomiej Zolnierkiewicz, Mel Gorman,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-mm-Bw31MaZKKs3YtjvyW6yDsg@public.gmane.org, James Ketrenos,
	Chatre, Reinette,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	ipw2100-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
In-Reply-To: <20090826074409.606b5124.akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org>

On Wed, 2009-08-26 at 22:44 +0800, Andrew Morton wrote:
> 
> It is perhaps pretty simple to make the second (GFP_ATOMIC) allocation
> go away.  The code is already conveniently structured to do this:
> 
>         do {
>                 chunk = (struct fw_chunk *)(data + offset);
>                 offset += sizeof(struct fw_chunk);
>                 /* build DMA packet and queue up for sending */
>                 /* dma to chunk->address, the chunk->length bytes from
> data +
>                  * offeset*/
>                 /* Dma loading */
>                 rc = ipw_fw_dma_add_buffer(priv, shared_phys + offset,
> 
> le32_to_cpu(chunk->address),
> 
> le32_to_cpu(chunk->length));
>                 if (rc) {
>                         IPW_DEBUG_INFO("dmaAddBuffer Failed\n");
>                         goto out;
>                 }
> 
>                 offset += le32_to_cpu(chunk->length);
>         } while (offset < len);
> 
> what is the typical/expected value of chunk->length here?  If it's
> significantly less than 4096*(2^6), could we convert this function to
> use a separate DMAable allocation per fw_chunk?

Unfortunately, the largest chunk size for the latest 3.1 firmware is
0x20040, which also requires order 6 page allocation. I'll try to use
the firmware DMA command block (64 slots) to handle the image (each for
4k, totally 256k).

Thanks,
-yi

^ permalink raw reply

* Re: Receive side performance issue with multi-10-GigE and NUMA
From: Ingo Molnar @ 2009-08-27  9:06 UTC (permalink / raw)
  To: Neil Horman
  Cc: Steven Rostedt, David Miller, fweisbec, billfink, netdev, brice,
	gallatin
In-Reply-To: <20090827011718.GA5315@localhost.localdomain>


* Neil Horman <nhorman@tuxdriver.com> wrote:

> > TRACE_EVENT can be a little harder to learn, because it is all 
> > MACRO magic, but once you understand them, you'll find that they 
> > are very easy.
> 
> Yeah, macro magic is an understatement.  But I'll have the 
> conversions done in the next few days, no worries.

Cool, thanks Neil!

[ And we tracing folks are rather fond of that macro abuse, so if 
  you can think of ways it could be made even more abusively 
  C-alike, we are all ears ;-) ]

	Ingo

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox