Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] Add firmware label support to iproute2
From: Stephen Hemminger @ 2010-08-18 21:41 UTC (permalink / raw)
  To: Narendra_K; +Cc: netdev, Matt_Domsch, Charles_Rose, Jordan_Hargrave
In-Reply-To: <EDA0A4495861324DA2618B4C45DCB3EE612E34@blrx3m08.blr.amer.dell.com>

On Fri, 13 Aug 2010 18:06:26 +0530
<Narendra_K@Dell.com> wrote:

> > -----Original Message-----
> > From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> > Sent: Thursday, August 12, 2010 11:43 PM
> > To: K, Narendra
> > Cc: netdev@vger.kernel.org; Domsch, Matt; Rose, Charles; Hargrave,
> > Jordan
> > Subject: Re: [PATCH] Add firmware label support to iproute2
> > 
> > On Thu, 12 Aug 2010 12:35:37 -0500
> > Narendra K <Narendra_K@dell.com> wrote:
> > 
> > > +#ifndef LIBNETDEVNAME_PRESENT
> > >  			filter_dev = *argv;
> > > +#else
> > > +			if (netdev_alias_to_kernelname(*argv,
> kernel_name) <
> > 0)
> > > +				show_firmware_alias_usage(*argv);
> > > +			filter_dev = kernel_name;
> > > +#endif
> > >  		}
> > 
> > like the kernel, I don't like ifdef cases in main code.
> > You should put in stub inline that returns appropriate error.
> > 
> 
> Stephen,
> 
> Thanks for the suggestions. I will rework the patch and send a revised
> version soon.
> 
> > What happens if alias matches existing interface name?
> 
> If the user inputs an existing 'ethN' name, the function
> 'netdev_alias_to_kernelaname' will return it as it is. If the user
> inputs 'Embedded NIC N', then the function would do a sysfs lookup and
> return the corresponding 'ethN' name.

The netdev_alias_to_kernelname should only happen after normal lookup failed.
Also how does ifindex to name mapping work?

-- 

^ permalink raw reply

* Re: GTSM and TCP accept problem
From: Stephen Hemminger @ 2010-08-18 21:39 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Herbert Xu, netdev
In-Reply-To: <1281951825.2524.5.camel@edumazet-laptop>

On Mon, 16 Aug 2010 11:43:45 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> Le samedi 14 août 2010 à 21:46 -0400, Stephen Hemminger a écrit :
> > The implementation of Generalized TTL Security has a problem
> > on the TCP accept side. Since there can be different number of
> > hops for each peer, the listener ends up doing
> > 
> >    socket
> >    bind
> >    set TTL to 255
> >    listen
> >    while (nfd = accept(peer)) {
> >      info = lookup(peer)
> >      set MINTTL to 255 - info->ttl_hops
> > 
> > 
> > The problem is that a rogue peer can still do three way
> > handshake causing the accept to succeed. But the rogue will
> > leave a stuck connection that will then have to timeout.
> > 
> > The only ways I have come up to deal with this are:
> >    * have short timeout on initial data (recycle faster)
> >    * push peer;MINTTL table down into kernel (per socket)
> >    * have BGP do this through iptables which is a non-starter
> >      for a general application that needs to run on BSD, and
> >      would mess up existing firewall rules.
> > 
> > Ideas welcome..
> > --
> 
> Another idea would be to store the TTL of the SYN packet (or third
> packet) and let application read it after accept(), allowing it to
> reject the connection if it doesnt match expected TTL.

could be in tcp_info?
-- 

^ permalink raw reply

* Re: [PATCH 16/16] drivers/net/qlcnic: Use available error codes
From: David Miller @ 2010-08-18 21:28 UTC (permalink / raw)
  To: julia
  Cc: amit.salecha, anirban.chakraborty, linux-driver, netdev,
	linux-kernel, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008161828520.19313@ask.diku.dk>

From: Julia Lawall <julia@diku.dk>
Date: Mon, 16 Aug 2010 18:29:13 +0200 (CEST)

> From: Julia Lawall <julia@diku.dk>
> 
> The error code is stored in the variable err, but it is the variable ret
> that is returned instead.  So store the error code in ret.  Err is then
> useless.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <julia@diku.dk>

Applied.

^ permalink raw reply

* Re: [PATCH 8/16] net/ax25: Use available error codes
From: David Miller @ 2010-08-18 21:28 UTC (permalink / raw)
  To: julia; +Cc: ralf, linux-hams, linux-kernel, netdev, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008161826390.19313@ask.diku.dk>

From: Julia Lawall <julia@diku.dk>
Date: Mon, 16 Aug 2010 18:26:57 +0200 (CEST)

> From: Julia Lawall <julia@diku.dk>
> 
> Error codes are stored in err, but the return value is always 0.  Return
> err instead.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <julia@diku.dk>

Applied.

^ permalink raw reply

* Re: [PATCH 13/16] net/ax25: Use available error codes
From: David Miller @ 2010-08-18 21:28 UTC (permalink / raw)
  To: julia; +Cc: jreuter, ralf, linux-hams, linux-kernel, netdev, kernel-janitors
In-Reply-To: <Pine.LNX.4.64.1008161828030.19313@ask.diku.dk>

From: Julia Lawall <julia@diku.dk>
Date: Mon, 16 Aug 2010 18:28:19 +0200 (CEST)

> From: Julia Lawall <julia@diku.dk>
> 
> Error codes are stored in err, but the return value is always 0.  Return
> err instead.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
 ...
> Signed-off-by: Julia Lawall <julia@diku.dk>

Applied.

^ permalink raw reply

* Re: [patch] net/sched: remove unneeded NULL check
From: David Miller @ 2010-08-18 21:27 UTC (permalink / raw)
  To: error27
  Cc: hadi, kaber, jarkao2, eric.dumazet, shemminger, netdev,
	kernel-janitors
In-Reply-To: <20100814210949.GB645@bicker>

From: Dan Carpenter <error27@gmail.com>
Date: Sat, 14 Aug 2010 23:09:49 +0200

> There is no need to check "s".  nla_data() doesn't return NULL.  Also we
> already dereferenced "s" at this point so it would have oopsed ealier if
> it were NULL.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next 2.6.37?] drivers/net/pcmcia: Use pr_<level> and netdev_<level>
From: David Miller @ 2010-08-18 21:27 UTC (permalink / raw)
  To: joe; +Cc: linux, w.sang, netdev, linux-pcmcia
In-Reply-To: <1281651771.15486.55.camel@Joe-Laptop>

From: Joe Perches <joe@perches.com>
Date: Thu, 12 Aug 2010 15:22:51 -0700

> On Mon, 2010-08-09 at 17:34 +0200, Dominik Brodowski wrote:
>> look good from a PCMCIA point of view, therefore:
>> Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>
> 
> If PCMCIA is still being looked after, then here's
> another for you, maybe for 2.6.37.
> 
> Use the more descriptive logging message styles.
> 
> There are whitespace/indentation errors in the original
> sources that these changes do not modify, so checkpatch
> errors were cheerfully ignored.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Applied.

^ permalink raw reply

* Re: [PATCH 2/2] s2io: Update driver version
From: David Miller @ 2010-08-18 21:27 UTC (permalink / raw)
  To: jon.mason; +Cc: netdev, ramkrishna.vepa, sivakumar.subramani, sreenivasa.honnur
In-Reply-To: <1281625384-26927-2-git-send-email-jon.mason@exar.com>

From: Jon Mason <jon.mason@exar.com>
Date: Thu, 12 Aug 2010 10:03:04 -0500

> Update s2io driver's version to reflect the series of patches.
> 
> Signed-off-by: Jon Mason <jon.mason@exar.com>

Applied.

^ permalink raw reply

* Re: [PATCH 1/2] s2io: remove unused code
From: David Miller @ 2010-08-18 21:27 UTC (permalink / raw)
  To: jon.mason; +Cc: netdev, ramkrishna.vepa, sivakumar.subramani, sreenivasa.honnur
In-Reply-To: <1281625384-26927-1-git-send-email-jon.mason@exar.com>

From: Jon Mason <jon.mason@exar.com>
Date: Thu, 12 Aug 2010 10:03:03 -0500

> usr_addrs is never referenced outside the initial struct population
> during open, thus making it unnecessary.  Remove it, the code that
> references it, and the struct that it is the only user of.
> 
> Signed-off-by: Jon Mason <jon.mason@exar.com>

Applied.

^ permalink raw reply

* Re: Broadcom 5709 take long time to bring up
From: Rick Jones @ 2010-08-18 21:25 UTC (permalink / raw)
  To: Mitchell Erblich
  Cc: Benjamin Li, Joe Jin, Michael Chan, terry.liu@oracle.com,
	netdev@vger.kernel.org
In-Reply-To: <70AE41F9-6FB7-43E3-A0C0-450E964A9B56@earthlink.net>

Mitchell Erblich wrote:
> On Aug 18, 2010, at 9:08 AM, Benjamin Li wrote:
> 
> 
>>Hi Joe,
>>
>>Just a couple of quick questions:
>>
>>1.  It is possible that autonegotiation could take in the orders of
>>seconds depending on the remote partner.   
> 
> 
> No. It happens in ms.
> 
> Mitchell Erblich

Both may be correct, depending on the context.  The IEEE specs likely mandate 
that autonegotiation take no longer than N ms, but there may indeed be kit out 
there that takes rather longer...

rick jones

^ permalink raw reply

* Re: [PATCH v3] PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)
From: David Miller @ 2010-08-18 21:23 UTC (permalink / raw)
  To: xeb; +Cc: eric.dumazet, netdev
In-Reply-To: <E1Olg2s-0007BB-00.xeb-mail-ru@f30.mail.ru>

From: Dmitry Kozlov <xeb@mail.ru>
Date: Wed, 18 Aug 2010 14:38:58 +0400

> Hi Eric!
> Thanks for comments. All your comments was accepted except:
> 
>> > +static struct ppp_channel_ops pptp_chan_ops = {
>> 
>> static const ?
> 
> not, because po->chan.ops = &pptp_chan_ops which is not const

Yes it is:

davem@sunset:~/src/GIT/linux-2.6$ git grep ppp_channel_ops -- include/linux/
include/linux/ppp_channel.h:struct ppp_channel_ops {
include/linux/ppp_channel.h:	const struct ppp_channel_ops *ops; /* operations for this channel */
davem@sunset:~/src/GIT/linux-2.6$ 

^ permalink raw reply

* Re: Broadcom 5709 take long time to bring up
From: Mitchell Erblich @ 2010-08-18 20:44 UTC (permalink / raw)
  To: Benjamin Li
  Cc: Joe Jin, Michael Chan, terry.liu@oracle.com,
	netdev@vger.kernel.org
In-Reply-To: <1282147735.3880.83.camel@localhost>


On Aug 18, 2010, at 9:08 AM, Benjamin Li wrote:

> Hi Joe,
> 
> Just a couple of quick questions:
> 
> 1.  It is possible that autonegotiation could take in the orders of
> seconds depending on the remote partner.   

No. It happens in ms.

Mitchell Erblich

> Could you describe the remote
> partner?
> 
> 2.  Are there iSCSI offload connections?  With iSCSI there is
> additionial overhead to cleanup/setup iSCSI connections on the
> chip/firmware when ifup'ing/ifdown'ing a network interface.
> 
> 3.  Could you provide the kernel logs during this up/down test?  I
> wanted to see if there was anything interesting in the kernel logs.
> 
> Thanks again.
> 
> -Ben
> 
> 
> On Wed, 2010-08-18 at 07:34 -0700, Joe Jin wrote:
>> Hi Michael,
>> 
>> We try boning two broadcom 5709 nics with active-backup mode, we try to
>> down/up one of the nics, I found some ping packets loss as below:
>> # ifdown eth0 
>> # ifup eth0  <-- no pkg loss 
>> # ifdown eth1 
>> # ifup eth1  <-- 2 pkg loss 
>> # ifdown eth0 
>> # ifup eth0  <-- 10 pkg loss 
>> # ifdown eth1 
>> # ifup eth1 <-- no pkg loss 
>> # ifdown eth0 
>> # ifup eth0  <-- 10 pkg loss 
>> 
>> module bond parameter are:
>> "mode=1 miimon=100 primary=eth0 downdelay=100 updelay=100"
>> 
>> we have tried the latest driver(2.0.8c) of broadcom and Redhat el5.5
>> official driver(2.0.2), result are same.
>> 
>> I checked and debugged the driver found it caused by take long time(~3s)
>> to bring up the device. here are link state when call if up:
>> # ifdown eth0 
>> # ifup eth0   
>> #ip -o link | grep eth0 
>> 2: eth0: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
>> # ifdown eth1 
>> # ifup eth1   
>> #ip -o link | grep eth1 
>> 3: eth1: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
>> # ifdown eth0 
>> # ifup eth0   
>> #ip -o link | grep eth0 
>> 2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
>> # ifdown eth1 
>> # ifup eth1   
>> #ip -o link | grep eth1 
>> 3: eth1: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
>> # ifdown eth0 
>> # ifup eth0 
>> #ip -o link | grep eth0 
>> 2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
>> 
>> So, the question is how long alway will take when bring up the interface?
>> 3 seconds as expection or no? if not, is that a firmware issure or driver
>> issue?
>> 
>> Thanks,
>> Joe
>> 
>> 
>> 
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


^ permalink raw reply

* [PATCH net-next-2.6] atm: remove a net_device_stats clear
From: Eric Dumazet @ 2010-08-18 20:44 UTC (permalink / raw)
  To: David Miller
  Cc: shemminger, tklauser, linux, netdev, linux-arm-kernel,
	kernel-janitors, error27
In-Reply-To: <20100818.131043.226780154.davem@davemloft.net>

Le mercredi 18 août 2010 à 13:10 -0700, David Miller a écrit :
> From: Stephen Hemminger <shemminger@vyatta.com>
> Date: Wed, 18 Aug 2010 08:33:04 -0700
> 
> > On Wed, 18 Aug 2010 09:04:24 +0200
> > Tobias Klauser <tklauser@distanz.ch> wrote:
> > 
> >>  
> >> -	memset (&priv(dev)->stats, 0, sizeof (struct net_device_stats));
> >> +	memset(&dev->stats, 0, sizeof(struct net_device_stats));
> > 
> > This is incorrect, just remove the memset.  The stats are initialized
> > when device is created.  The Linux device driver convention is to
> > keep stats when device is set down and brought back up; that is what
> > the majority of other drivers do.
> 
> Yep, both the ether1 and ether3 patch have this problem.  Looks
> like we'll see v4 coming some :-)

To be fair, we accepted some bits like that in the past ;)

[PATCH net-next-2.6] atm: remove a net_device_stats clear

No need to clear device stats in lec_open()

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
net/atm/lec.c |    1 -
1 file changed, 1 deletion(-)

diff --git a/net/atm/lec.c b/net/atm/lec.c
index d98bde1..181d70c 100644
--- a/net/atm/lec.c
+++ b/net/atm/lec.c
@@ -220,7 +220,6 @@ static unsigned char *get_tr_dst(unsigned char *packet, unsigned char *rdesc)
 static int lec_open(struct net_device *dev)
 {
 	netif_start_queue(dev);
-	memset(&dev->stats, 0, sizeof(struct net_device_stats));
 
 	return 0;
 }



^ permalink raw reply related

* Re: [PATCH v2] ether1: Use net_device_stats from struct net_device
From: David Miller @ 2010-08-18 20:10 UTC (permalink / raw)
  To: shemminger
  Cc: tklauser, linux, netdev, linux-arm-kernel, kernel-janitors,
	error27
In-Reply-To: <20100818083304.2ef2eea4@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 18 Aug 2010 08:33:04 -0700

> On Wed, 18 Aug 2010 09:04:24 +0200
> Tobias Klauser <tklauser@distanz.ch> wrote:
> 
>>  
>> -	memset (&priv(dev)->stats, 0, sizeof (struct net_device_stats));
>> +	memset(&dev->stats, 0, sizeof(struct net_device_stats));
> 
> This is incorrect, just remove the memset.  The stats are initialized
> when device is created.  The Linux device driver convention is to
> keep stats when device is set down and brought back up; that is what
> the majority of other drivers do.

Yep, both the ether1 and ether3 patch have this problem.  Looks
like we'll see v4 coming some :-)

^ permalink raw reply

* [PATCH 2/4] platform: Facilitate the creation of pseudo-platform buses
From: Patrick Pannuto @ 2010-08-18 19:15 UTC (permalink / raw)
  To: linux-kernel
  Cc: ppannuto, linux-arm-msm, magnus.damm, grant.likely, gregkh,
	Kevin Hilman, Paul Mundt, Magnus Damm, Rafael J. Wysocki,
	Eric Miao, Dmitry Torokhov, netdev
In-Reply-To: <1282158943-11902-1-git-send-email-ppannuto@codeaurora.org>

(lkml.org seems to have lost August 3rd...)
RFC: http://lkml.indiana.edu/hypermail/linux/kernel/1008.0/01342.html
 v1: http://lkml.indiana.edu/hypermail/linux/kernel/1008.0/01942.html
 v2: http://lkml.indiana.edu/hypermail/linux/kernel/1008.1/00958.html

Based on the idea and code originally proposed by Kevin Hilman:
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg31161.html

Changes from v2:
   * Remove the pseudo_platform_bus_[un]register functions
   * Split affected platform functions:
       - Platform functions are split into a wrapper (with the
         original function name) that retains the 'bus-setting'
         and 'parent-setting' duties, and then exports all the
         heavy-lifting to new __platform* counterparts
       - "Pseudo" buses work the same as the platform bus now,
         writing their own wrappers and calling the __platform
         functions
   * Export a *lot* of platform symbols to allow for intialization
     of "pseudo" platform bus types.
       - I personally like this model a lot, I think it is very
         clean from an implementation perspective, even if it
         does export a lot of new symbols
       - Thanks to Michal Miroslaw <mirqus@gmail.com> for the
         suggestion here
   * Add more use-cases justifying the need of platform-ish bus types

Changes from v1:

   * "Pseudo" buses are no longer init'd, they are [un]registered by:
       - pseudo_platform_bus_register(struct bus_type *)
       - pseudo_platform_bus_unregister(struct bus_type *)
   * These registrations [de]allocate a dev_pm_ops structure for the
     pseudo bus_type
   * Do not overwrite the parent if .bus is already set (that is, allow
     pseudo bus devices to be root devices)

   * Split into 2 patches:
       - 1/2: Already sent separately, but included here for clarity
       - 2/2: The real meat of the patch (this patch)

INTRO

As SOCs become more popular, the desire to quickly define a simple,
but functional, bus type with only a few unique properties becomes
desirable. As they become more complicated, the ability to nest these
simple busses and otherwise orchestrate them to match the actual
topology also becomes desirable.

Also, as power management becomes more challenging, grouping devices
on the same SOC under the same /logical bus/ provides a very natural
interface and location for power-management issues for that SOC.

This is a fairly natural extension of the "platform" bus; ultimately,
this patch series allows for the creation of multiple platform buses (SOC
buses), with the correct name and quirks for each of the "platforms" (SOCs)
they are trying to support.

EXAMPLE USAGE

See follow-on patches implementing the MSM bus and moving the
MSM uart device / driver onto it.

Change-Id: I10d2fa4671302e81be8f9cac01a3855cef4eeebf
Cc: Kevin Hilman <khilman@deeprootsystems.com>
Signed-off-by: Patrick Pannuto <ppannuto@codeaurora.org>
---
 drivers/base/platform.c         |  184 ++++++++++++++++++++------------------
 include/linux/platform_device.h |   87 ++++++++++++++++++
 2 files changed, 184 insertions(+), 87 deletions(-)

diff --git a/drivers/base/platform.c b/drivers/base/platform.c
index b69ccb4..e75640f 100644
--- a/drivers/base/platform.c
+++ b/drivers/base/platform.c
@@ -224,25 +224,10 @@ int platform_device_add_data(struct platform_device *pdev, const void *data,
 }
 EXPORT_SYMBOL_GPL(platform_device_add_data);
 
-/**
- * platform_device_add - add a platform device to device hierarchy
- * @pdev: platform device we're adding
- *
- * This is part 2 of platform_device_register(), though may be called
- * separately _iff_ pdev was allocated by platform_device_alloc().
- */
-int platform_device_add(struct platform_device *pdev)
+int __platform_device_add(struct platform_device *pdev)
 {
 	int i, ret = 0;
 
-	if (!pdev)
-		return -EINVAL;
-
-	if (!pdev->dev.parent)
-		pdev->dev.parent = &platform_bus;
-
-	pdev->dev.bus = &platform_bus_type;
-
 	if (pdev->id != -1)
 		dev_set_name(&pdev->dev, "%s.%d", pdev->name,  pdev->id);
 	else
@@ -289,6 +274,27 @@ int platform_device_add(struct platform_device *pdev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(__platform_device_add);
+
+/**
+ * platform_device_add - add a platform device to device hierarchy
+ * @pdev: platform device we're adding
+ *
+ * This is part 2 of platform_device_register(), though may be called
+ * separately _iff_ pdev was allocated by platform_device_alloc().
+ */
+int platform_device_add(struct platform_device *pdev)
+{
+	if (!pdev)
+		return -EINVAL;
+
+	if (!pdev->dev.parent)
+		pdev->dev.parent = &platform_bus;
+
+	pdev->dev.bus = &platform_bus_type;
+
+	return __platform_device_add(pdev);
+}
 EXPORT_SYMBOL_GPL(platform_device_add);
 
 /**
@@ -476,13 +482,8 @@ static void platform_drv_shutdown(struct device *_dev)
 	drv->shutdown(dev);
 }
 
-/**
- * platform_driver_register - register a driver for platform-level devices
- * @drv: platform driver structure
- */
-int platform_driver_register(struct platform_driver *drv)
+int __platform_driver_register(struct platform_driver *drv)
 {
-	drv->driver.bus = &platform_bus_type;
 	if (drv->probe)
 		drv->driver.probe = platform_drv_probe;
 	if (drv->remove)
@@ -492,6 +493,18 @@ int platform_driver_register(struct platform_driver *drv)
 
 	return driver_register(&drv->driver);
 }
+EXPORT_SYMBOL_GPL(__platform_driver_register);
+
+/**
+ * platform_driver_register - register a driver for platform-level devices
+ * @drv: platform driver structure
+ */
+int platform_driver_register(struct platform_driver *drv)
+{
+	drv->driver.bus = &platform_bus_type;
+
+	return __platform_driver_register(drv);
+}
 EXPORT_SYMBOL_GPL(platform_driver_register);
 
 /**
@@ -504,25 +517,9 @@ void platform_driver_unregister(struct platform_driver *drv)
 }
 EXPORT_SYMBOL_GPL(platform_driver_unregister);
 
-/**
- * platform_driver_probe - register driver for non-hotpluggable device
- * @drv: platform driver structure
- * @probe: the driver probe routine, probably from an __init section
- *
- * Use this instead of platform_driver_register() when you know the device
- * is not hotpluggable and has already been registered, and you want to
- * remove its run-once probe() infrastructure from memory after the driver
- * has bound to the device.
- *
- * One typical use for this would be with drivers for controllers integrated
- * into system-on-chip processors, where the controller devices have been
- * configured as part of board setup.
- *
- * Returns zero if the driver registered and bound to a device, else returns
- * a negative error code and with the driver not registered.
- */
-int __init_or_module platform_driver_probe(struct platform_driver *drv,
-		int (*probe)(struct platform_device *))
+int __init_or_module __platform_driver_probe(struct platform_driver *drv,
+		int (*probe)(struct platform_device *),
+		int (*bustype_driver_register)(struct platform_driver *))
 {
 	int retval, code;
 
@@ -531,7 +528,7 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
 
 	/* temporary section violation during probe() */
 	drv->probe = probe;
-	retval = code = platform_driver_register(drv);
+	retval = code = bustype_driver_register(drv);
 
 	/*
 	 * Fixup that section violation, being paranoid about code scanning
@@ -550,6 +547,30 @@ int __init_or_module platform_driver_probe(struct platform_driver *drv,
 		platform_driver_unregister(drv);
 	return retval;
 }
+EXPORT_SYMBOL_GPL(__platform_driver_probe);
+
+/**
+ * platform_driver_probe - register driver for non-hotpluggable device
+ * @drv: platform driver structure
+ * @probe: the driver probe routine, probably from an __init section
+ *
+ * Use this instead of platform_driver_register() when you know the device
+ * is not hotpluggable and has already been registered, and you want to
+ * remove its run-once probe() infrastructure from memory after the driver
+ * has bound to the device.
+ *
+ * One typical use for this would be with drivers for controllers integrated
+ * into system-on-chip processors, where the controller devices have been
+ * configured as part of board setup.
+ *
+ * Returns zero if the driver registered and bound to a device, else returns
+ * a negative error code and with the driver not registered.
+ */
+int __init_or_module platform_driver_probe(struct platform_driver *drv,
+		int (*probe)(struct platform_device *))
+{
+	return __platform_driver_probe(drv, probe, &platform_driver_register);
+}
 EXPORT_SYMBOL_GPL(platform_driver_probe);
 
 /**
@@ -627,12 +648,13 @@ static ssize_t modalias_show(struct device *dev, struct device_attribute *a,
 	return (len >= PAGE_SIZE) ? (PAGE_SIZE - 1) : len;
 }
 
-static struct device_attribute platform_dev_attrs[] = {
+struct device_attribute platform_dev_attrs[] = {
 	__ATTR_RO(modalias),
 	__ATTR_NULL,
 };
+EXPORT_SYMBOL_GPL(platform_dev_attrs);
 
-static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
+int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 {
 	struct platform_device	*pdev = to_platform_device(dev);
 
@@ -640,6 +662,7 @@ static int platform_uevent(struct device *dev, struct kobj_uevent_env *env)
 		(pdev->id_entry) ? pdev->id_entry->name : pdev->name);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(platform_uevent);
 
 static const struct platform_device_id *platform_match_id(
 			const struct platform_device_id *id,
@@ -668,7 +691,7 @@ static const struct platform_device_id *platform_match_id(
  * and compare it against the name of the driver. Return whether they match
  * or not.
  */
-static int platform_match(struct device *dev, struct device_driver *drv)
+int platform_match(struct device *dev, struct device_driver *drv)
 {
 	struct platform_device *pdev = to_platform_device(dev);
 	struct platform_driver *pdrv = to_platform_driver(drv);
@@ -680,10 +703,11 @@ static int platform_match(struct device *dev, struct device_driver *drv)
 	/* fall-back to driver name match */
 	return (strcmp(pdev->name, drv->name) == 0);
 }
+EXPORT_SYMBOL_GPL(platform_match);
 
 #ifdef CONFIG_PM_SLEEP
 
-static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
+int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
 {
 	struct platform_driver *pdrv = to_platform_driver(dev->driver);
 	struct platform_device *pdev = to_platform_device(dev);
@@ -695,7 +719,7 @@ static int platform_legacy_suspend(struct device *dev, pm_message_t mesg)
 	return ret;
 }
 
-static int platform_legacy_resume(struct device *dev)
+int platform_legacy_resume(struct device *dev)
 {
 	struct platform_driver *pdrv = to_platform_driver(dev->driver);
 	struct platform_device *pdev = to_platform_device(dev);
@@ -707,7 +731,7 @@ static int platform_legacy_resume(struct device *dev)
 	return ret;
 }
 
-static int platform_pm_prepare(struct device *dev)
+int platform_pm_prepare(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -717,19 +741,16 @@ static int platform_pm_prepare(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_prepare);
 
-static void platform_pm_complete(struct device *dev)
+void platform_pm_complete(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 
 	if (drv && drv->pm && drv->pm->complete)
 		drv->pm->complete(dev);
 }
-
-#else /* !CONFIG_PM_SLEEP */
-
-#define platform_pm_prepare		NULL
-#define platform_pm_complete		NULL
+EXPORT_SYMBOL_GPL(platform_pm_complete);
 
 #endif /* !CONFIG_PM_SLEEP */
 
@@ -752,6 +773,7 @@ int __weak platform_pm_suspend(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_suspend);
 
 int __weak platform_pm_suspend_noirq(struct device *dev)
 {
@@ -768,6 +790,7 @@ int __weak platform_pm_suspend_noirq(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_suspend_noirq);
 
 int __weak platform_pm_resume(struct device *dev)
 {
@@ -786,6 +809,7 @@ int __weak platform_pm_resume(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_resume);
 
 int __weak platform_pm_resume_noirq(struct device *dev)
 {
@@ -802,19 +826,13 @@ int __weak platform_pm_resume_noirq(struct device *dev)
 
 	return ret;
 }
-
-#else /* !CONFIG_SUSPEND */
-
-#define platform_pm_suspend		NULL
-#define platform_pm_resume		NULL
-#define platform_pm_suspend_noirq	NULL
-#define platform_pm_resume_noirq	NULL
+EXPORT_SYMBOL_GPL(platform_pm_resume_noirq);
 
 #endif /* !CONFIG_SUSPEND */
 
 #ifdef CONFIG_HIBERNATION
 
-static int platform_pm_freeze(struct device *dev)
+int platform_pm_freeze(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -831,8 +849,9 @@ static int platform_pm_freeze(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_freeze);
 
-static int platform_pm_freeze_noirq(struct device *dev)
+int platform_pm_freeze_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -847,8 +866,9 @@ static int platform_pm_freeze_noirq(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_freeze_noirq);
 
-static int platform_pm_thaw(struct device *dev)
+int platform_pm_thaw(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -865,8 +885,9 @@ static int platform_pm_thaw(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_thaw);
 
-static int platform_pm_thaw_noirq(struct device *dev)
+int platform_pm_thaw_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -881,8 +902,9 @@ static int platform_pm_thaw_noirq(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_thaw_noirq);
 
-static int platform_pm_poweroff(struct device *dev)
+int platform_pm_poweroff(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -899,8 +921,9 @@ static int platform_pm_poweroff(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_poweroff);
 
-static int platform_pm_poweroff_noirq(struct device *dev)
+int platform_pm_poweroff_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -915,8 +938,9 @@ static int platform_pm_poweroff_noirq(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_poweroff_noirq);
 
-static int platform_pm_restore(struct device *dev)
+int platform_pm_restore(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -933,8 +957,9 @@ static int platform_pm_restore(struct device *dev)
 
 	return ret;
 }
+EXPORT_SYMBOL_GPL(platform_pm_restore);
 
-static int platform_pm_restore_noirq(struct device *dev)
+int platform_pm_restore_noirq(struct device *dev)
 {
 	struct device_driver *drv = dev->driver;
 	int ret = 0;
@@ -949,17 +974,7 @@ static int platform_pm_restore_noirq(struct device *dev)
 
 	return ret;
 }
-
-#else /* !CONFIG_HIBERNATION */
-
-#define platform_pm_freeze		NULL
-#define platform_pm_thaw		NULL
-#define platform_pm_poweroff		NULL
-#define platform_pm_restore		NULL
-#define platform_pm_freeze_noirq	NULL
-#define platform_pm_thaw_noirq		NULL
-#define platform_pm_poweroff_noirq	NULL
-#define platform_pm_restore_noirq	NULL
+EXPORT_SYMBOL_GPL(platform_pm_restore_noirq);
 
 #endif /* !CONFIG_HIBERNATION */
 
@@ -980,15 +995,9 @@ int __weak platform_pm_runtime_idle(struct device *dev)
 	return pm_generic_runtime_idle(dev);
 };
 
-#else /* !CONFIG_PM_RUNTIME */
-
-#define platform_pm_runtime_suspend NULL
-#define platform_pm_runtime_resume NULL
-#define platform_pm_runtime_idle NULL
-
 #endif /* !CONFIG_PM_RUNTIME */
 
-static const struct dev_pm_ops platform_dev_pm_ops = {
+const struct dev_pm_ops platform_dev_pm_ops = {
 	.prepare = platform_pm_prepare,
 	.complete = platform_pm_complete,
 	.suspend = platform_pm_suspend,
@@ -1007,6 +1016,7 @@ static const struct dev_pm_ops platform_dev_pm_ops = {
 	.runtime_resume = platform_pm_runtime_resume,
 	.runtime_idle = platform_pm_runtime_idle,
 };
+EXPORT_SYMBOL_GPL(platform_dev_pm_ops);
 
 struct bus_type platform_bus_type = {
 	.name		= "platform",
diff --git a/include/linux/platform_device.h b/include/linux/platform_device.h
index 5417944..6d7d399 100644
--- a/include/linux/platform_device.h
+++ b/include/linux/platform_device.h
@@ -53,6 +53,7 @@ extern int platform_device_add_resources(struct platform_device *pdev,
 					 const struct resource *res,
 					 unsigned int num);
 extern int platform_device_add_data(struct platform_device *pdev, const void *data, size_t size);
+extern int __platform_device_add(struct platform_device *pdev);
 extern int platform_device_add(struct platform_device *pdev);
 extern void platform_device_del(struct platform_device *pdev);
 extern void platform_device_put(struct platform_device *pdev);
@@ -67,12 +68,16 @@ struct platform_driver {
 	const struct platform_device_id *id_table;
 };
 
+extern int __platform_driver_register(struct platform_driver *);
 extern int platform_driver_register(struct platform_driver *);
 extern void platform_driver_unregister(struct platform_driver *);
 
 /* non-hotpluggable platform devices may use this so that probe() and
  * its support may live in __init sections, conserving runtime memory.
  */
+extern int __platform_driver_probe(struct platform_driver *driver,
+		int (*probe)(struct platform_device *),
+		int (*bustype_driver_register)(struct platform_driver *));
 extern int platform_driver_probe(struct platform_driver *driver,
 		int (*probe)(struct platform_device *));
 
@@ -84,6 +89,88 @@ extern struct platform_device *platform_create_bundle(struct platform_driver *dr
 					struct resource *res, unsigned int n_res,
 					const void *data, size_t size);
 
+extern struct device_attribute platform_dev_attrs[];
+extern int platform_uevent(struct device *dev, struct kobj_uevent_env *env);
+extern int platform_match(struct device *dev, struct device_driver *drv);
+
+#ifdef CONFIG_PM_SLEEP
+extern int platform_pm_prepare(struct device *dev);
+extern void platform_pm_complete(struct device *dev);
+#else  /* !CONFIG_PM_SLEEP */
+#define platform_pm_prepare		NULL
+#define platform_pm_complete		NULL
+#endif /* !CONFIG_PM_SLEEP */
+
+#ifdef CONFIG_SUSPEND
+extern int __weak platform_pm_suspend(struct device *dev);
+extern int __weak platform_pm_suspend_noirq(struct device *dev);
+extern int __weak platform_pm_resume(struct device *dev);
+extern int __weak platform_pm_resume_noirq(struct device *dev);
+#else  /* !CONFIG_SUSPEND */
+#define platform_pm_suspend		NULL
+#define platform_pm_resume		NULL
+#define platform_pm_suspend_noirq	NULL
+#define platform_pm_resume_noirq	NULL
+#endif /* !CONFIG_SUSPEND */
+
+#ifdef CONFIG_HIBERNATION
+extern int platform_pm_freeze(struct device *dev);
+extern int platform_pm_freeze_noirq(struct device *dev);
+extern int platform_pm_thaw(struct device *dev);
+extern int platform_pm_thaw_noirq(struct device *dev);
+extern int platform_pm_poweroff(struct device *dev);
+extern int platform_pm_poweroff_noirq(struct device *dev);
+extern int platform_pm_restore(struct device *dev);
+extern int platform_pm_restore_noirq(struct device *dev);
+#else  /* !CONFIG_HIBERNATION */
+#define platform_pm_freeze		NULL
+#define platform_pm_thaw		NULL
+#define platform_pm_poweroff		NULL
+#define platform_pm_restore		NULL
+#define platform_pm_freeze_noirq	NULL
+#define platform_pm_thaw_noirq		NULL
+#define platform_pm_poweroff_noirq	NULL
+#define platform_pm_restore_noirq	NULL
+#endif /* !CONFIG_HIBERNATION */
+
+#ifdef CONFIG_PM_RUNTIME
+extern int __weak platform_pm_runtime_suspend(struct device *dev);
+extern int __weak platform_pm_runtime_resume(struct device *dev);
+extern int __weak platform_pm_runtime_idle(struct device *dev);
+#else  /* !CONFIG_PM_RUNTIME */
+#define platform_pm_runtime_suspend NULL
+#define platform_pm_runtime_resume NULL
+#define platform_pm_runtime_idle NULL
+#endif /* !CONFIG_PM_RUNTIME */
+
+extern const struct dev_pm_ops platform_dev_pm_ops;
+
+#define PLATFORM_BUS_TEMPLATE \
+	.name		= "XXX_OVERRIDEME_XXX",	\
+	.dev_attrs	= platform_dev_attrs,	\
+	.match		= platform_match,	\
+	.uevent		= platform_uevent,	\
+	.pm		= &platform_dev_pm_ops
+
+#define PLATFORM_PM_OPS_TEMPLATE \
+	.prepare = platform_pm_prepare,			\
+	.complete = platform_pm_complete,		\
+	.suspend = platform_pm_suspend,			\
+	.resume = platform_pm_resume,			\
+	.freeze = platform_pm_freeze,			\
+	.thaw = platform_pm_thaw,			\
+	.poweroff = platform_pm_poweroff,		\
+	.restore = platform_pm_restore,			\
+	.suspend_noirq = platform_pm_suspend_noirq,	\
+	.resume_noirq = platform_pm_resume_noirq,	\
+	.freeze_noirq = platform_pm_freeze_noirq,	\
+	.thaw_noirq = platform_pm_thaw_noirq,		\
+	.poweroff_noirq = platform_pm_poweroff_noirq,	\
+	.restore_noirq = platform_pm_restore_noirq,	\
+	.runtime_suspend = platform_pm_runtime_suspend,	\
+	.runtime_resume = platform_pm_runtime_resume,	\
+	.runtime_idle = platform_pm_runtime_idle
+
 /* early platform driver interface */
 struct early_platform_driver {
 	const char *class_str;
-- 
1.7.2.1

^ permalink raw reply related

* Re: [PATCH 1/1] ehea: Allocate stats buffer with GFP_KERNEL
From: Jay Vosburgh @ 2010-08-18 17:49 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, brking, ossthema, osstklei, raisch, netdev
In-Reply-To: <1282140652.2194.90.camel@edumazet-laptop>

Eric Dumazet <eric.dumazet@gmail.com> wrote:

>Le jeudi 01 juillet 2010 à 22:48 -0700, David Miller a écrit :
>> From: Brian King <brking@linux.vnet.ibm.com>
>> Date: Wed, 30 Jun 2010 16:59:12 -0500
>> 
>> > 
>> > Since ehea_get_stats calls ehea_h_query_ehea_port, which
>> > can sleep, we can also sleep when allocating a page in
>> > this function. This fixes some memory allocation failure
>> > warnings seen under low memory conditions.
>> > 
>> > Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
>> 
>> Applied to net-next-2.6
>> --
>
>I believe there is a problem with this patch and/or bonding.
>
>If we say ndo_get_stats() methods are allowed to sleep, then 
>bond_get_stats() should be updated, because it currently calls
>dev_get_stats() from a read_lock_bh(&bond->lock); section.
>
>Are we allowed to sleep inside a read_lock_bh() ?

	Nope.

	And bonding's not the only call site that holds a lock over the
call to ndo_get_stats / dev_get_stats; net/core/net-sysfs.c:netstat_show
does it as well.

	I presume that bonding and netstat_show are holding a lock to
keep a list of interfaces from changing, since there's no other locking
that's guaranteed to be held for a call to dev_get_stats.

	In any event, ehea is doing an hcall to the hypervisor, which
may return "long busy," after which ehea sleeps for however long the
hypervisor told it to wait before trying again.

	So, the real question is whether the ndo_get_stats* functions
are permitted to sleep.  If they are, then bonding and netstat_show both
need to change.  If not, then ehea needs to change.  Ehea is probably
not alone in this; I poked around a bit, and it looks like mlx4 may also
sleep in ndo_get_stats.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: Broadcom 5709 take long time to bring up
From: Benjamin Li @ 2010-08-18 16:08 UTC (permalink / raw)
  To: Joe Jin; +Cc: Michael Chan, terry.liu@oracle.com, netdev@vger.kernel.org
In-Reply-To: <20100818143432.GA29386@joejin-pc.cn.oracle.com>

Hi Joe,

Just a couple of quick questions:

1.  It is possible that autonegotiation could take in the orders of
seconds depending on the remote partner.   Could you describe the remote
partner?

2.  Are there iSCSI offload connections?  With iSCSI there is
additionial overhead to cleanup/setup iSCSI connections on the
chip/firmware when ifup'ing/ifdown'ing a network interface.

3.  Could you provide the kernel logs during this up/down test?  I
wanted to see if there was anything interesting in the kernel logs.

Thanks again.

-Ben


On Wed, 2010-08-18 at 07:34 -0700, Joe Jin wrote:
> Hi Michael,
> 
> We try boning two broadcom 5709 nics with active-backup mode, we try to
> down/up one of the nics, I found some ping packets loss as below:
> # ifdown eth0 
> # ifup eth0  <-- no pkg loss 
> # ifdown eth1 
> # ifup eth1  <-- 2 pkg loss 
> # ifdown eth0 
> # ifup eth0  <-- 10 pkg loss 
> # ifdown eth1 
> # ifup eth1 <-- no pkg loss 
> # ifdown eth0 
> # ifup eth0  <-- 10 pkg loss 
> 
> module bond parameter are:
> "mode=1 miimon=100 primary=eth0 downdelay=100 updelay=100"
> 
> we have tried the latest driver(2.0.8c) of broadcom and Redhat el5.5
> official driver(2.0.2), result are same.
> 
> I checked and debugged the driver found it caused by take long time(~3s)
> to bring up the device. here are link state when call if up:
> # ifdown eth0 
> # ifup eth0   
> #ip -o link | grep eth0 
> 2: eth0: <BROADCAST,MULTICAST,SLAVE,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
> # ifdown eth1 
> # ifup eth1   
> #ip -o link | grep eth1 
> 3: eth1: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
> # ifdown eth0 
> # ifup eth0   
> #ip -o link | grep eth0 
> 2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
> # ifdown eth1 
> # ifup eth1   
> #ip -o link | grep eth1 
> 3: eth1: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
> # ifdown eth0 
> # ifup eth0 
> #ip -o link | grep eth0 
> 2: eth0: <NO-CARRIER,BROADCAST,MULTICAST,SLAVE,UP> mtu 1500 qdisc pfifo_fast master bond0 qlen 1000\    link/ether 00:26:55:4f:1f:18 brd ff:ff:ff:ff:ff:ff 
> 
> So, the question is how long alway will take when bring up the interface?
> 3 seconds as expection or no? if not, is that a firmware issure or driver
> issue?
> 
> Thanks,
> Joe
> 
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



^ permalink raw reply

* [PATCH 3/3] ibmveth: Driver version 1.04
From: Robert Jennings @ 2010-08-18 16:08 UTC (permalink / raw)
  To: David Miller, netdev, Santiago Leon, Brian King
In-Reply-To: <20100818160515.GA29528@linux.vnet.ibm.com>

Bump driver version.

Signed-off-by: Robert Jennings <rcj@linux.vnet.ibm.com>
---
 drivers/net/ibmveth.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: b/drivers/net/ibmveth.c
===================================================================
--- a/drivers/net/ibmveth.c
+++ b/drivers/net/ibmveth.c
@@ -111,7 +111,7 @@ static struct proc_dir_entry *ibmveth_pr
 
 static const char ibmveth_driver_name[] = "ibmveth";
 static const char ibmveth_driver_string[] = "IBM i/pSeries Virtual Ethernet Driver";
-#define ibmveth_driver_version "1.03"
+#define ibmveth_driver_version "1.04"
 
 MODULE_AUTHOR("Santiago Leon <santil@us.ibm.com>");
 MODULE_DESCRIPTION("IBM i/pSeries Virtual Ethernet Driver");

^ permalink raw reply

* [PATCH 2/3] ibmveth: Enable IPv6 checksum offload
From: Robert Jennings @ 2010-08-18 16:07 UTC (permalink / raw)
  To: David Miller, netdev, Santiago Leon, Brian King
In-Reply-To: <20100818160515.GA29528@linux.vnet.ibm.com>

This patch enables TCP checksum offload support for IPv6 on ibmveth.
This completely eliminates the generation and checking of the checksum
for IPv6 packets that are completely virtual and never touch a physical
network.  A basic TCPIPV6_STREAM netperf run showed a ~30% throughput
improvement when an MTU of 64000 was used.

This featured is enabled by default, as is the case for IPv4 checksum
offload.  When checksum offload is enabled the driver will negotiate
IPv4 and IPv6 offload with the firmware separately and enable what
is available.  As long as either IPv4 or IPv6 offload is supported
and enabled the device will report that checksum offload is enabled.
The device stats, available through ethtool, will display which 
checksum offload features are supported/enabled by firmware.

Performance testing against a stock kernel shows no regression for IPv4
or IPv6 in terms of throughput or processor utilization with checksum
disabled or enabled.  

Signed-off-by: Robert Jennings <rcj@linux.vnet.ibm.com>

---
 drivers/net/ibmveth.c |   59 +++++++++++++++++++++++++++++++++++++++++---------
 drivers/net/ibmveth.h |    2 ++
 2 files changed, 50 insertions(+), 11 deletions(-)

Index: b/drivers/net/ibmveth.c
===================================================================
--- a/drivers/net/ibmveth.c
+++ b/drivers/net/ibmveth.c
@@ -50,6 +50,7 @@
 #include <linux/proc_fs.h>
 #include <linux/in.h>
 #include <linux/ip.h>
+#include <linux/ipv6.h>
 #include <linux/slab.h>
 #include <net/net_namespace.h>
 #include <asm/hvcall.h>
@@ -134,6 +135,8 @@ struct ibmveth_stat ibmveth_stats[] = {
 	{ "rx_no_buffer", IBMVETH_STAT_OFF(rx_no_buffer) },
 	{ "tx_map_failed", IBMVETH_STAT_OFF(tx_map_failed) },
 	{ "tx_send_failed", IBMVETH_STAT_OFF(tx_send_failed) },
+	{ "fw_enabled_ipv4_csum", IBMVETH_STAT_OFF(fw_ipv4_csum_support) },
+	{ "fw_enabled_ipv6_csum", IBMVETH_STAT_OFF(fw_ipv6_csum_support) },
 };
 
 /* simple methods of getting data from the current rxq entry */
@@ -739,6 +742,7 @@ static void ibmveth_set_rx_csum_flags(st
 		 */
 		adapter->rx_csum = 0;
 		dev->features &= ~NETIF_F_IP_CSUM;
+		dev->features &= ~NETIF_F_IPV6_CSUM;
 	}
 }
 
@@ -747,10 +751,15 @@ static void ibmveth_set_tx_csum_flags(st
 	struct ibmveth_adapter *adapter = netdev_priv(dev);
 
 	if (data) {
-		dev->features |= NETIF_F_IP_CSUM;
+		if (adapter->fw_ipv4_csum_support)
+			dev->features |= NETIF_F_IP_CSUM;
+		if (adapter->fw_ipv6_csum_support)
+			dev->features |= NETIF_F_IPV6_CSUM;
 		adapter->rx_csum = 1;
-	} else
+	} else {
 		dev->features &= ~NETIF_F_IP_CSUM;
+		dev->features &= ~NETIF_F_IPV6_CSUM;
+	}
 }
 
 static int ibmveth_set_csum_offload(struct net_device *dev, u32 data,
@@ -758,7 +767,8 @@ static int ibmveth_set_csum_offload(stru
 {
 	struct ibmveth_adapter *adapter = netdev_priv(dev);
 	unsigned long set_attr, clr_attr, ret_attr;
-	long ret;
+	unsigned long set_attr6, clr_attr6;
+	long ret, ret6;
 	int rc1 = 0, rc2 = 0;
 	int restart = 0;
 
@@ -772,10 +782,13 @@ static int ibmveth_set_csum_offload(stru
 	set_attr = 0;
 	clr_attr = 0;
 
-	if (data)
+	if (data) {
 		set_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
-	else
+		set_attr6 = IBMVETH_ILLAN_IPV6_TCP_CSUM;
+	} else {
 		clr_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
+		clr_attr6 = IBMVETH_ILLAN_IPV6_TCP_CSUM;
+	}
 
 	ret = h_illan_attributes(adapter->vdev->unit_address, 0, 0, &ret_attr);
 
@@ -786,14 +799,33 @@ static int ibmveth_set_csum_offload(stru
 					 set_attr, &ret_attr);
 
 		if (ret != H_SUCCESS) {
-			rc1 = -EIO;
-			ibmveth_error_printk("unable to change checksum offload settings."
-					     " %d rc=%ld\n", data, ret);
+			ibmveth_error_printk("unable to change IPv4 checksum "
+					     "offload settings. %d rc=%ld\n",
+					     data, ret);
 
 			ret = h_illan_attributes(adapter->vdev->unit_address,
 						 set_attr, clr_attr, &ret_attr);
 		} else
+			adapter->fw_ipv4_csum_support = data;
+
+		ret6 = h_illan_attributes(adapter->vdev->unit_address,
+					 clr_attr6, set_attr6, &ret_attr);
+
+		if (ret6 != H_SUCCESS) {
+			ibmveth_error_printk("unable to change IPv6 checksum "
+					     "offload settings. %d rc=%ld\n",
+					     data, ret);
+
+			ret = h_illan_attributes(adapter->vdev->unit_address,
+						 set_attr6, clr_attr6,
+						 &ret_attr);
+		} else
+			adapter->fw_ipv6_csum_support = data;
+
+		if (ret == H_SUCCESS || ret6 == H_SUCCESS)
 			done(dev, data);
+		else
+			rc1 = -EIO;
 	} else {
 		rc1 = -EIO;
 		ibmveth_error_printk("unable to change checksum offload settings."
@@ -821,9 +853,9 @@ static int ibmveth_set_tx_csum(struct ne
 	struct ibmveth_adapter *adapter = netdev_priv(dev);
 	int rc = 0;
 
-	if (data && (dev->features & NETIF_F_IP_CSUM))
+	if (data && (dev->features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)))
 		return 0;
-	if (!data && !(dev->features & NETIF_F_IP_CSUM))
+	if (!data && !(dev->features & (NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM)))
 		return 0;
 
 	if (data && !adapter->rx_csum)
@@ -910,7 +942,12 @@ static netdev_tx_t ibmveth_start_xmit(st
 	desc.fields.flags_len = IBMVETH_BUF_VALID | skb->len;
 
 	if (skb->ip_summed == CHECKSUM_PARTIAL &&
-	    ip_hdr(skb)->protocol != IPPROTO_TCP && skb_checksum_help(skb)) {
+	    ((skb->protocol == htons(ETH_P_IP) &&
+	      ip_hdr(skb)->protocol != IPPROTO_TCP) ||
+	     (skb->protocol == htons(ETH_P_IPV6) &&
+	      ipv6_hdr(skb)->nexthdr != IPPROTO_TCP)) &&
+	    skb_checksum_help(skb)) {
+
 		ibmveth_error_printk("tx: failed to checksum packet\n");
 		tx_dropped++;
 		goto out;
Index: b/drivers/net/ibmveth.h
===================================================================
--- a/drivers/net/ibmveth.h
+++ b/drivers/net/ibmveth.h
@@ -149,6 +149,8 @@ struct ibmveth_adapter {
     void *bounce_buffer;
     dma_addr_t bounce_buffer_dma;
 
+    u64 fw_ipv6_csum_support;
+    u64 fw_ipv4_csum_support;
     /* adapter specific stats */
     u64 replenish_task_cycles;
     u64 replenish_no_mem;

^ permalink raw reply

* [PATCH 1/3] ibmveth: Remove duplicate checksum offload setup code
From: Robert Jennings @ 2010-08-18 16:05 UTC (permalink / raw)
  To: David Miller, netdev, Santiago Leon, Brian King

Remove code in the device probe function where we set up the checksum
offload feature and replace it with a call to an existing function that
is doing the same.  This is done to clean up the driver in preparation
of adding IPv6 checksum offload support.

Signed-off-by: Robert Jennings <rcj@linux.vnet.ibm.com>

---
 drivers/net/ibmveth.c |   18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

Index: b/drivers/net/ibmveth.c
===================================================================
--- a/drivers/net/ibmveth.c
+++ b/drivers/net/ibmveth.c
@@ -1222,10 +1222,8 @@ static const struct net_device_ops ibmve
 static int __devinit ibmveth_probe(struct vio_dev *dev, const struct vio_device_id *id)
 {
 	int rc, i;
-	long ret;
 	struct net_device *netdev;
 	struct ibmveth_adapter *adapter;
-	unsigned long set_attr, ret_attr;
 
 	unsigned char *mac_addr_p;
 	unsigned int *mcastFilterSize_p;
@@ -1310,21 +1308,7 @@ static int __devinit ibmveth_probe(struc
 
 	ibmveth_debug_printk("registering netdev...\n");
 
-	ret = h_illan_attributes(dev->unit_address, 0, 0, &ret_attr);
-
-	if (ret == H_SUCCESS && !(ret_attr & IBMVETH_ILLAN_ACTIVE_TRUNK) &&
-	    !(ret_attr & IBMVETH_ILLAN_TRUNK_PRI_MASK) &&
-	    (ret_attr & IBMVETH_ILLAN_PADDED_PKT_CSUM)) {
-		set_attr = IBMVETH_ILLAN_IPV4_TCP_CSUM;
-
-		ret = h_illan_attributes(dev->unit_address, 0, set_attr, &ret_attr);
-
-		if (ret == H_SUCCESS) {
-			adapter->rx_csum = 1;
-			netdev->features |= NETIF_F_IP_CSUM;
-		} else
-			ret = h_illan_attributes(dev->unit_address, set_attr, 0, &ret_attr);
-	}
+	ibmveth_set_csum_offload(netdev, 1, ibmveth_set_tx_csum_flags);
 
 	rc = register_netdev(netdev);
 

^ permalink raw reply

* Re: [Patch 1/2] s2io: remove lro parameter
From: Jon Mason @ 2010-08-18 15:36 UTC (permalink / raw)
  To: Amerigo Wang
  Cc: netdev@vger.kernel.org, bhutchings@solarflare.com,
	Ramkrishna Vepa, sgruszka@redhat.com, davem@davemloft.net
In-Reply-To: <20100818075530.5926.67960.sendpatchset@localhost.localdomain>

On Wed, Aug 18, 2010 at 12:51:09AM -0700, Amerigo Wang wrote:
> 
> As suggested by David, this parameter can die, we can use ethtool
> to turn LRO on/off. Compile tests only.

Needs some additional removal

Line 41 and 42 " * lro: Specifies whether to enable Large Receive ..."
As you have removed the modparm

Line 8286 "if (sp->lro)"
As it will always be on in this instance


Also, all references to "sp->lro" can be converted to "(dev->features
& NETIF_F_LRO)", removing an unnecessary variable.

Thanks,
Jon

> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> 
> ---
> diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
> index 18bc5b7..b981fa9 100644
> --- a/drivers/net/s2io.c
> +++ b/drivers/net/s2io.c
> @@ -496,8 +496,6 @@ S2IO_PARM_INT(rxsync_frequency, 3);
>  /* Interrupt type. Values can be 0(INTA), 2(MSI_X) */
>  S2IO_PARM_INT(intr_type, 2);
>  /* Large receive offload feature */
> -static unsigned int lro_enable = 1;
> -module_param_named(lro, lro_enable, uint, 0);
>  
>  /* Max pkts to be aggregated by LRO at one time. If not specified,
>   * aggregation happens until we hit max IP pkt size(64K)
> @@ -6735,13 +6733,10 @@ static int s2io_ethtool_set_flags(struct net_device *dev, u32 data)
>  		return -EINVAL;
>  
>  	if (data & ETH_FLAG_LRO) {
> -		if (lro_enable) {
> -			if (!(dev->features & NETIF_F_LRO)) {
> -				dev->features |= NETIF_F_LRO;
> -				changed = 1;
> -			}
> -		} else
> -			rc = -EINVAL;
> +		if (!(dev->features & NETIF_F_LRO)) {
> +			dev->features |= NETIF_F_LRO;
> +			changed = 1;
> +		}
>  	} else if (dev->features & NETIF_F_LRO) {
>  		dev->features &= ~NETIF_F_LRO;
>  		changed = 1;
> @@ -7911,7 +7906,7 @@ s2io_init_nic(struct pci_dev *pdev, const struct pci_device_id *pre)
>  	else
>  		sp->device_type = XFRAME_I_DEVICE;
>  
> -	sp->lro = lro_enable;
> +	sp->lro = 1;
>  
>  	/* Initialize some PCI/PCI-X fields of the NIC. */
>  	s2io_init_pci(sp);
> @@ -8047,8 +8042,7 @@ s2io_init_nic(struct pci_dev *pdev, const struct pci_device_id *pre)
>  	dev->netdev_ops = &s2io_netdev_ops;
>  	SET_ETHTOOL_OPS(dev, &netdev_ethtool_ops);
>  	dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
> -	if (lro_enable)
> -		dev->features |= NETIF_F_LRO;
> +	dev->features |= NETIF_F_LRO;
>  	dev->features |= NETIF_F_SG | NETIF_F_IP_CSUM;
>  	if (sp->high_dma_flag == true)
>  		dev->features |= NETIF_F_HIGHDMA;
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v2] ether1: Use net_device_stats from struct net_device
From: Stephen Hemminger @ 2010-08-18 15:33 UTC (permalink / raw)
  To: Tobias Klauser
  Cc: David S. Miller, Russell King, netdev, linux-arm-kernel,
	kernel-janitors, Dan Carpenter
In-Reply-To: <1282115064-3701-1-git-send-email-tklauser@distanz.ch>

On Wed, 18 Aug 2010 09:04:24 +0200
Tobias Klauser <tklauser@distanz.ch> wrote:

>  
> -	memset (&priv(dev)->stats, 0, sizeof (struct net_device_stats));
> +	memset(&dev->stats, 0, sizeof(struct net_device_stats));

This is incorrect, just remove the memset.  The stats are initialized
when device is created.  The Linux device driver convention is to
keep stats when device is set down and brought back up; that is what
the majority of other drivers do.

-- 

^ permalink raw reply

* Re: [PATCH net-next-2.6] vxge: Implement 64bit stats
From: Eric Dumazet @ 2010-08-18 15:28 UTC (permalink / raw)
  To: Jon Mason
  Cc: David Miller, netdev, Ramkrishna Vepa, Sivakumar Subramani,
	Sreenivasa Honnur
In-Reply-To: <20100818152531.GB9202@exar.com>

Le mercredi 18 août 2010 à 10:25 -0500, Jon Mason a écrit :
> On Wed, Aug 18, 2010 at 06:42:54AM -0700, Eric Dumazet wrote:
> > vxge_get_stats() is racy, since it clears a block of memory (net_stats)
> > possibly still used by other cpus.
> > 
> > We can update this driver to full 64bit stats, since ndo_get_stats64()
> > provides a private block to store results, and driver maintains 64bit
> > counters already.
> > 
> > We also remove net_stats field from struct vxge_sw_stats
> 
> It looks sane.  Assuming you have not tested it already, I'll give it a
> quick sanity check.

I did not test it, I would be grateful if you can do this ;)

Thanks



^ permalink raw reply

* Re: [PATCH net-next-2.6] vxge: Implement 64bit stats
From: Jon Mason @ 2010-08-18 15:25 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, netdev, Ramkrishna Vepa, Sivakumar Subramani,
	Sreenivasa Honnur
In-Reply-To: <1282138974.2194.83.camel@edumazet-laptop>

On Wed, Aug 18, 2010 at 06:42:54AM -0700, Eric Dumazet wrote:
> vxge_get_stats() is racy, since it clears a block of memory (net_stats)
> possibly still used by other cpus.
> 
> We can update this driver to full 64bit stats, since ndo_get_stats64()
> provides a private block to store results, and driver maintains 64bit
> counters already.
> 
> We also remove net_stats field from struct vxge_sw_stats

It looks sane.  Assuming you have not tested it already, I'll give it a
quick sanity check.

Thanks,
Jon

> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> ---
>  drivers/net/vxge/vxge-main.c |   22 +++++++---------------
>  drivers/net/vxge/vxge-main.h |    1 -
>  2 files changed, 7 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/net/vxge/vxge-main.c b/drivers/net/vxge/vxge-main.c
> index c7c5605..ac90196 100644
> --- a/drivers/net/vxge/vxge-main.c
> +++ b/drivers/net/vxge/vxge-main.c
> @@ -2914,26 +2914,18 @@ static int vxge_change_mtu(struct net_device *dev, int new_mtu)
>  }
>  
>  /**
> - * vxge_get_stats
> + * vxge_get_stats64
>   * @dev: pointer to the device structure
> + * @stats: pointer to struct rtnl_link_stats64
>   *
> - * Updates the device statistics structure. This function updates the device
> - * statistics structure in the net_device structure and returns a pointer
> - * to the same.
>   */
> -static struct net_device_stats *
> -vxge_get_stats(struct net_device *dev)
> +static struct rtnl_link_stats64 *
> +vxge_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *net_stats)
>  {
> -	struct vxgedev *vdev;
> -	struct net_device_stats *net_stats;
> +	struct vxgedev *vdev = netdev_priv(dev);
>  	int k;
>  
> -	vdev = netdev_priv(dev);
> -
> -	net_stats = &vdev->stats.net_stats;
> -
> -	memset(net_stats, 0, sizeof(struct net_device_stats));
> -
> +	/* net_stats already zeroed by caller */
>  	for (k = 0; k < vdev->no_of_vpath; k++) {
>  		net_stats->rx_packets += vdev->vpaths[k].ring.stats.rx_frms;
>  		net_stats->rx_bytes += vdev->vpaths[k].ring.stats.rx_bytes;
> @@ -3102,7 +3094,7 @@ vxge_vlan_rx_kill_vid(struct net_device *dev, unsigned short vid)
>  static const struct net_device_ops vxge_netdev_ops = {
>  	.ndo_open               = vxge_open,
>  	.ndo_stop               = vxge_close,
> -	.ndo_get_stats          = vxge_get_stats,
> +	.ndo_get_stats64        = vxge_get_stats64,
>  	.ndo_start_xmit         = vxge_xmit,
>  	.ndo_validate_addr      = eth_validate_addr,
>  	.ndo_set_multicast_list = vxge_set_multicast,
> diff --git a/drivers/net/vxge/vxge-main.h b/drivers/net/vxge/vxge-main.h
> index 2e3b064..d4be07e 100644
> --- a/drivers/net/vxge/vxge-main.h
> +++ b/drivers/net/vxge/vxge-main.h
> @@ -172,7 +172,6 @@ struct vxge_msix_entry {
>  
>  struct vxge_sw_stats {
>  	/* Network Stats (interface stats) */
> -	struct net_device_stats net_stats;
>  
>  	/* Tx */
>  	u64 tx_frms;
> 
> 

^ permalink raw reply

* Re: [PATCH v5] PPTP: PPP over IPv4 (Point-to-Point Tunneling Protocol)
From: Eric Dumazet @ 2010-08-18 15:22 UTC (permalink / raw)
  To: Dmitry Kozlov; +Cc: netdev
In-Reply-To: <E1OljPK-0006rQ-00.xeb-mail-ru@f238.mail.ru>

Le mercredi 18 août 2010 à 18:14 +0400, Dmitry Kozlov a écrit :
> This patch contains:
> 1. pptp driver
> 2. gre demultiplexer driver for demultiplexing gre packets with different gre version
>    so ip_gre and pptp may coexists
> 3. ip_gre modification
> 4. other stuff
> 
> Changes from patch v4:
> 1. using spinlock instead mutex
> 2. fixed coding style issues
> 

> +static int __init pptp_init_module(void)
> +{
> +	int err = 0;
> +	printk(KERN_INFO "PPTP driver version " PPTP_DRIVER_VERSION "\n");
> +
> +	if (gre_add_protocol(&gre_pptp_protocol, GREPROTO_PPTP) < 0) {
> +		printk(KERN_INFO "PPTP: can't add protocol\n");
> +		goto out;
> +	}
> +
> +	err = proto_register(&pptp_sk_proto, 0);
> +	if (err) {
> +		printk(KERN_INFO "PPTP: can't register sk_proto\n");
> +		goto out_inet_del_protocol;
> +	}
> +
> +	err = register_pppox_proto(PX_PROTO_PPTP, &pppox_pptp_proto);
> +	if (err) {
> +		printk(KERN_INFO "PPTP: can't register pppox_proto\n");
> +		goto out_unregister_sk_proto;
> +	}
> +
> +	callid_sock = (struct pppox_sock **)vmalloc((MAX_CALLID + 1) * sizeof(void *));
> +	memset(callid_sock, 0, (MAX_CALLID + 1) * sizeof(void *));
> +
> +out:
> +	return err;
> +out_unregister_sk_proto:
> +	proto_unregister(&pptp_sk_proto);
> +out_inet_del_protocol:
> +	gre_del_protocol(&gre_pptp_protocol, GREPROTO_PPTP);
> +	return err;
> +}



1) Please test return from vmalloc(), it can be NULL

Also, if you need to clear it, you might call 

__vmalloc(size, GFP_KERNEL | __GFP_ZERO, PAGE_KERNEL);

(and result is already cleared)


2) For new code, it's advised to use :

pr_err(xxx) instead of printk(KERN_ERR xxx)
pr_info(xxx) instead of printk(KERN_INFO xxx)


3) After skb_dst_drop(skb), you dont need to call skb_dst_set(skb,
NULL), as it is already done (for example in pptp_rcv())

4) Since you touch some includes, you also can use __packed
instead of __attribute__ ((__packed__))


5) I feel a bit uncomfortable with lookup_chan_dst()

+static int lookup_chan_dst(__u16 call_id, __be32 d_addr)
+{
+       struct pppox_sock *sock;
+       struct pptp_opt *opt;
+       int i;
+
+       rcu_read_lock();
+       for (i = find_next_bit(callid_bitmap, MAX_CALLID, 1); i < MAX_CALLID; i = find_next_bit(callid_bitmap, MAX_CALLID, i + 1)) {

Split this too long line please, its really awful

+               sock = rcu_dereference(callid_sock[i]);
+               opt = &sock->proto.pptp;
+               if (opt->dst_addr.call_id == call_id && opt->dst_addr.sin_addr.s_addr == d_addr)
+                       break;
+       }
+       rcu_read_unlock();
+
+       return i < MAX_CALLID;
+}

Once you get a bit in bitmask, and rcu_dereference(callid_sock[i]),
there is no guarantee sock is not NULL.

You should add a test.
If sock is not NULL, then you have the guarantee (thanks to RCU) that
pointer is safe until the rcu_read_unlock()


Thanks



^ 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