* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: Nicolas de Pesloüan @ 2011-06-01 19:34 UTC (permalink / raw)
To: Flavio Leitner; +Cc: David Miller, fubar, jpirko, netdev, andy
In-Reply-To: <4DE68ECB.70802@redhat.com>
Le 01/06/2011 21:11, Flavio Leitner a écrit :
> On 06/01/2011 04:03 PM, David Miller wrote:
>> From: Jay Vosburgh<fubar@us.ibm.com>
>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>>
>>> The "this dingus was added in version X.Y.Z" is there because
>>> users sometimes read the most recent version of the documentation (that
>>> they get from the internet) and then would become confused when their
>>> older distro driver lacked some option described in the documentation.
>>
>> I disagree with this whole concept, because distros backport features
>> like this into their kernel and therefore the feature is showing up in
>> version X.Y.$(Z-20).
>
> It doesn't matter the version if the user can find the feature, so
> distros backporting features works and that info is not useful at all.
> However, when the user doesn't find the feature and search the internet,
> then that info is helpful.
There are *many* new features that get included into the kernel without documenting the exact first
version that provide them. Why should we need this for bonding? Also, because we lack a table that
gives the kernel version matching a bonding version, the user is not really helped by "you need
version X.Y.Z of bonding to have this feature".
So, I'm not sure it helps...
Nicolas.
^ permalink raw reply
* [RFC] Moving files around in drivers/net
From: Joe Perches @ 2011-06-01 19:30 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Paul Gortmaker, Jan Engelhardt, Jeffrey Kirsher
Does anyone still think moving files around in drivers/net
would be sensible and a suitable candidate for inclusion
in 3.1?
Here's what Jeffrey proposed:
http://vger.kernel.org/netconf2010_slides/netconf-jtk.pdf
Here's what I proposed before that.
http://www.spinics.net/lists/netdev/msg149717.html
I agree with Jan that the current 10/100, 1000, 10000 speed
selections mechanisms are less than ideal.
Perhaps it'd be worthwhile to remove that speed distinction.
^ permalink raw reply
* Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters
From: Nicolas de Pesloüan @ 2011-06-01 19:23 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Jay Vosburgh, Flavio Leitner, netdev, davem, andy
In-Reply-To: <20110601163153.GB2784@psychotron.redhat.com>
Le 01/06/2011 18:31, Jiri Pirko a écrit :
> This patch allows to reset failure counters for all enslaved devices.
Hi Jiri,
Why do we need a way to reset those counters? What is the problem with having those counters
monotonically increase until the system is rebooted? Do we have a way to reset other network
statistics (/sys/class/net/eth0/statistics/* for example)?
Except from this "do we need this feature" question, the code sounds good to me.
Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
Nicolas.
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: David Miller @ 2011-06-01 19:22 UTC (permalink / raw)
To: fbl; +Cc: fubar, jpirko, netdev, andy
In-Reply-To: <4DE68ECB.70802@redhat.com>
From: Flavio Leitner <fbl@redhat.com>
Date: Wed, 01 Jun 2011 16:11:07 -0300
> On 06/01/2011 04:03 PM, David Miller wrote:
>> From: Jay Vosburgh <fubar@us.ibm.com>
>> Date: Wed, 01 Jun 2011 09:13:39 -0700
>>
>>> The "this dingus was added in version X.Y.Z" is there because
>>> users sometimes read the most recent version of the documentation (that
>>> they get from the internet) and then would become confused when their
>>> older distro driver lacked some option described in the documentation.
>>
>> I disagree with this whole concept, because distros backport features
>> like this into their kernel and therefore the feature is showing up in
>> version X.Y.$(Z-20).
>
> It doesn't matter the version if the user can find the feature, so
> distros backporting features works and that info is not useful at all.
> However, when the user doesn't find the feature and search the internet,
> then that info is helpful.
So how is the user going to find that FC14 has the feature even
though his FC13 kernel does not?
I'll say it again, this version stuff is completely pointless.
If the user is dabbling with upstream kernels he's a minority,
and clueful enough to figure out this stuff himself.
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: Flavio Leitner @ 2011-06-01 19:11 UTC (permalink / raw)
To: David Miller; +Cc: fubar, jpirko, netdev, andy
In-Reply-To: <20110601.120300.130602301077156524.davem@davemloft.net>
On 06/01/2011 04:03 PM, David Miller wrote:
> From: Jay Vosburgh <fubar@us.ibm.com>
> Date: Wed, 01 Jun 2011 09:13:39 -0700
>
>> The "this dingus was added in version X.Y.Z" is there because
>> users sometimes read the most recent version of the documentation (that
>> they get from the internet) and then would become confused when their
>> older distro driver lacked some option described in the documentation.
>
> I disagree with this whole concept, because distros backport features
> like this into their kernel and therefore the feature is showing up in
> version X.Y.$(Z-20).
It doesn't matter the version if the user can find the feature, so
distros backporting features works and that info is not useful at all.
However, when the user doesn't find the feature and search the internet,
then that info is helpful.
fbl
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: David Miller @ 2011-06-01 19:03 UTC (permalink / raw)
To: fubar; +Cc: jpirko, fbl, netdev, andy
In-Reply-To: <16056.1306944819@death>
From: Jay Vosburgh <fubar@us.ibm.com>
Date: Wed, 01 Jun 2011 09:13:39 -0700
> The "this dingus was added in version X.Y.Z" is there because
> users sometimes read the most recent version of the documentation (that
> they get from the internet) and then would become confused when their
> older distro driver lacked some option described in the documentation.
I disagree with this whole concept, because distros backport features
like this into their kernel and therefore the feature is showing up in
version X.Y.$(Z-20).
And distro kernels are what %99.9999 of users are exposed to.
Therefore, I think this version specification is not only pointless
but even a possible cause of confusion.
^ permalink raw reply
* Re: [BUG] net: cpu offline cause napi stall
From: Heiko Carstens @ 2011-06-01 18:12 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Frank Blaschka, davem, netdev, linux-s390
In-Reply-To: <1306947321.2890.5.camel@edumazet-laptop>
On Wed, Jun 01, 2011 at 06:55:21PM +0200, Eric Dumazet wrote:
> > + /* Append NAPI poll list from offline CPU. */
> > + list_splice_init(&oldsd->poll_list, &sd->poll_list);
> >
> > raise_softirq_irqoff(NET_TX_SOFTIRQ);
> > local_irq_enable();
>
> Please make sure we raise NET_RX_SOFTIRQ on new cpu if necessary.
Well, see two lines below the list_splice_init() call ;)
^ permalink raw reply
* Re: [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: walter harms @ 2011-06-01 17:54 UTC (permalink / raw)
To: Julia Lawall
Cc: Wolfgang Grandegger, kernel-janitors, socketcan-core, netdev,
linux-kernel
In-Reply-To: <1306948213-20767-4-git-send-email-julia@diku.dk>
Am 01.06.2011 19:10, schrieb Julia Lawall:
> From: Julia Lawall <julia@diku.dk>
>
> The failed_get label is used after the call to clk_get has succeeded, so it
> should be moved up above the call to clk_put.
>
> A simplified version of the semantic match that finds this problem is as
> follows: (http://coccinelle.lip6.fr/)
>
> // <smpl>
> @r exists@
> expression e1,e2;
> statement S;
> @@
>
> e1 = clk_get@p1(...);
> ... when != e1 = e2
> when != clk_put(e1)
> when any
> if (...) { ... when != clk_put(e1)
> when != if (...) { ... clk_put(e1) ... }
> * return@p3 ...;
> } else S
> // </smpl>
>
> Signed-off-by: Julia Lawall <julia@diku.dk>
>
> ---
> drivers/net/can/flexcan.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
> index d499056..121739c 100644
> --- a/drivers/net/can/flexcan.c
> +++ b/drivers/net/can/flexcan.c
> @@ -978,8 +978,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> failed_map:
> release_mem_region(mem->start, mem_size);
> failed_req:
> - clk_put(clk);
> failed_get:
> + clk_put(clk);
> failed_clock:
> return err;
> }
>
So failed_req == failed_get, is that intended ?
re,
wh
^ permalink raw reply
* patch for ixgbe-3.3.9 (out-of-tree) driver.
From: Ben Greear @ 2011-06-01 17:50 UTC (permalink / raw)
To: netdev
[-- Attachment #1: Type: text/plain, Size: 672 bytes --]
Using 'ethtool -t ethX' with 82598 chipset causes
kernel splats because spin-lock wasn't initialized properly.
This can probably cause other problems, but I'm not certain
of that.
I can't tell that anyone actually watches the sourceforge list,
so posting it here...
The reason I'm using the out-of-tree driver is that a customer
reports link bouncing when using the 82598 NIC. They are stuck
on a .34 kernel at the moment, so maybe it's fixed upstream already.
At any rate, I can't reproduce the problem locally, but the 3.3.9
driver appears to work for them.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
[-- Attachment #2: ixgbe-3.3.9.patch --]
[-- Type: text/plain, Size: 948 bytes --]
--- ixgbe-3.3.9/src/ixgbe_main.c 2011-04-19 16:06:31.000000000 -0700
+++ ixgbe-3.3.9.ben/src/ixgbe_main.c 2011-06-01 10:31:23.163839562 -0700
@@ -5843,16 +5843,18 @@
adapter->flags |= IXGBE_FLAG_SRIOV_CAPABLE;
if (hw->device_id == IXGBE_DEV_ID_82599_T3_LOM)
adapter->flags2 |= IXGBE_FLAG2_TEMP_SENSOR_CAPABLE;
-#ifdef NETIF_F_NTUPLE
- /* n-tuple support exists, always init our spinlock */
- spin_lock_init(&adapter->fdir_perfect_lock);
-#endif /* NETIF_F_NTUPLE */
adapter->max_msix_q_vectors = IXGBE_MAX_MSIX_Q_VECTORS_82599;
break;
default:
break;
}
- /* Default DCB settings, if applicable */
+
+#ifdef NETIF_F_NTUPLE
+ /* n-tuple support exists, always init our spinlock */
+ spin_lock_init(&adapter->fdir_perfect_lock);
+#endif /* NETIF_F_NTUPLE */
+
+ /* Default DCB settings, if applicable */
adapter->ring_feature[RING_F_DCB].indices = 8;
if (adapter->flags & IXGBE_FLAG_DCB_CAPABLE) {
^ permalink raw reply
* Re: Skipping past TCP lost packet in userspace
From: Rick Jones @ 2011-06-01 17:35 UTC (permalink / raw)
To: Josh Lehan; +Cc: Yuchung Cheng, netdev, jiyengar
In-Reply-To: <4DE5F3E3.2080609@krellan.com>
On Wed, 2011-06-01 at 01:10 -0700, Josh Lehan wrote:
> On 05/31/2011 10:23 AM, Yuchung Cheng wrote:
> > This paper may have a solution to your problem
> > "Minion—an All-Terrain Packet Packhorse to Jump-Start Stalled Internet
> > Transports"
> > http://csweb1.fandm.edu/jiyengar/lair/papers/minion-pfldnet2010.pdf
>
> Nice, thanks for pointing me to this. I appreciate the helpful answer,
> instead of just saying "use UDP" or "use SCTP". That's not the point.
>
> For better or for worse, TCP is realistically the only viable protocol
> for streaming to the largest possible audience these days, hence my
> question about adding this feature to the Linux TCP implementation.
Isn't that treating the symptoms of problems at layers 8 and 9 (*) with
kludges (perhaps hacks if one is feeling charitable) at the user
interface to layer 4? Just how many more little bits can we add to the
great pile before the aroma is overpowering? Or to abuse another
metaphor, is there really any camel's back left here?
And while Linux has had some slightly non-trivial, non-portable
enhancements to its interface to a TCP endpoint (TCP_CORK is something
that comes to mind) I don't think any of them have been anywhere nearly
as large a change to a fundamental semantic of a TCP connection as what
you propose.
rick jones
*
http://www.isc.org/store/logoware-clothing/isc-9-layer-osi-model-cotton-t-shirt
^ permalink raw reply
* Re: [patch net-next-2.6] de2104x: use speed defines instead of number
From: Ben Hutchings @ 2011-06-01 17:26 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem
In-Reply-To: <20110601161932.GA2784@psychotron.redhat.com>
On Wed, 2011-06-01 at 18:19 +0200, Jiri Pirko wrote:
> Wed, Jun 01, 2011 at 05:46:46PM CEST, bhutchings@solarflare.com wrote:
> >The speed/speed_hi fields are defined to hold speed in Mbit/s, not only
> >specific values. I don't see any reason to use the names any more.
>
> Do you mean to remove SPEED_X defines in whole code?
[...]
I have higher priorities - but I would happy to see someone do that.
The definitions have to stay in <linux/ethtool.h> for user space,
though.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare
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
* Re: [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: Joe Perches @ 2011-06-01 17:26 UTC (permalink / raw)
To: Julia Lawall
Cc: Wolfgang Grandegger, kernel-janitors, socketcan-core, netdev,
linux-kernel
In-Reply-To: <1306948213-20767-4-git-send-email-julia@diku.dk>
On Wed, 2011-06-01 at 19:10 +0200, Julia Lawall wrote:
> The failed_get label is used after the call to clk_get has succeeded, so it
> should be moved up above the call to clk_put.
[]
> diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
[]
> @@ -978,8 +978,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
> failed_map:
> release_mem_region(mem->start, mem_size);
> failed_req:
> - clk_put(clk);
> failed_get:
> + clk_put(clk);
> failed_clock:
> return err;
If this is correct, it might be better to rename all the
uses of failed_req to failed_get and delete label failed_req.
^ permalink raw reply
* Re: [patch net-next-2.6] bonding: allow resetting slave failure counters
From: Jay Vosburgh @ 2011-06-01 16:13 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Flavio Leitner, netdev, davem, andy
In-Reply-To: <20110601135321.GA2909@psychotron.brq.redhat.com>
Jiri Pirko <jpirko@redhat.com> wrote:
>Wed, Jun 01, 2011 at 03:28:37PM CEST, fbl@redhat.com wrote:
>>On 06/01/2011 06:40 AM, Jiri Pirko wrote:
>>> This patch allows to reset failure counters for all enslaved devices.
>>>
>>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>>> ---
>>> Documentation/networking/bonding.txt | 7 +++++++
>>> drivers/net/bonding/bond_sysfs.c | 27 +++++++++++++++++++++++++++
>>> 2 files changed, 34 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
>>> index 675612f..2f51d73 100644
>>> --- a/Documentation/networking/bonding.txt
>>> +++ b/Documentation/networking/bonding.txt
>>> @@ -782,6 +782,13 @@ resend_igmp
>>>
>>> This option was added for bonding version 3.7.0.
>>>
>>> +reset_failure_counters
>>> +
>>> + This write-only control file will zero failure counters for
>>> + all slaves. Note there is no appropriate module parameter for this
>>> + since it would not make much sense.
>>> + Write any value to perform reset.
>>
>>nit: many options mention when they were added.
>>i.e. This option was added for bonding version 3.7.1
>
>hmm, is that necessary Jay, Andy?
>
>/me thinks this versioning should be removed at all in the first place...
The "this dingus was added in version X.Y.Z" is there because
users sometimes read the most recent version of the documentation (that
they get from the internet) and then would become confused when their
older distro driver lacked some option described in the documentation.
I don't know if this is "good" or "bad" in an absolute sense,
but I stopped getting questions of that sort after I put these notes
into the documentation. I don't really see a down side, so for this
patch I'd like to see the version go up and one of these notes in the
documentation.
>>
>>fbl
>>
>>> 3. Configuring Bonding Devices
>>> ==============================
>>>
>>> diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
>>> index 88fcb25..9b45164 100644
>>> --- a/drivers/net/bonding/bond_sysfs.c
>>> +++ b/drivers/net/bonding/bond_sysfs.c
>>> @@ -1572,6 +1572,32 @@ out:
>>> static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR,
>>> bonding_show_resend_igmp, bonding_store_resend_igmp);
>>>
>>> +static ssize_t
>>> +bonding_store_reset_failure_counters(struct device *d,
>>> + struct device_attribute *attr,
>>> + const char *buf, size_t count)
>>> +{
>>> + struct slave *slave;
>>> + int i;
>>> + struct bonding *bond = to_bond(d);
>>> +
>>> + if (!rtnl_trylock())
>>> + return restart_syscall();
>>> +
>>> + read_lock(&bond->lock);
>>> + pr_info("%s: Resetting counters.\n", bond->dev->name);
I'd stick "failure" in here somewhere so it's clear what's been
reset. The printk can be done outside the lock as well.
-J
>>> + bond_for_each_slave(bond, slave, i)
>>> + slave->link_failure_count = 0;
>>> + read_unlock(&bond->lock);
>>> +
>>> + rtnl_unlock();
>>> +
>>> + return count;
>>> +}
>>> +
>>> +static DEVICE_ATTR(reset_failure_counters, S_IWUSR, NULL,
>>> + bonding_store_reset_failure_counters);
>>> +
>>> static struct attribute *per_bond_attrs[] = {
>>> &dev_attr_slaves.attr,
>>> &dev_attr_mode.attr,
>>> @@ -1600,6 +1626,7 @@ static struct attribute *per_bond_attrs[] = {
>>> &dev_attr_queue_id.attr,
>>> &dev_attr_all_slaves_active.attr,
>>> &dev_attr_resend_igmp.attr,
>>> + &dev_attr_reset_failure_counters.attr,
>>> NULL,
>>> };
>>>
>>
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* [patch net-next-2.6 v2] bonding: allow resetting slave failure counters
From: Jiri Pirko @ 2011-06-01 16:31 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: Flavio Leitner, netdev, davem, andy
In-Reply-To: <16056.1306944819@death>
This patch allows to reset failure counters for all enslaved devices.
Signed-off-by: Jiri Pirko <jpirko@redhat.com>
v1->v2: added version bump
added version note to doc
moved and adjusted printk
---
Documentation/networking/bonding.txt | 9 +++++++++
drivers/net/bonding/bond_sysfs.c | 28 ++++++++++++++++++++++++++++
drivers/net/bonding/bonding.h | 4 ++--
3 files changed, 39 insertions(+), 2 deletions(-)
diff --git a/Documentation/networking/bonding.txt b/Documentation/networking/bonding.txt
index 675612f..7a8da01 100644
--- a/Documentation/networking/bonding.txt
+++ b/Documentation/networking/bonding.txt
@@ -782,6 +782,15 @@ resend_igmp
This option was added for bonding version 3.7.0.
+reset_failure_counters
+
+ This write-only control file will zero failure counters for
+ all slaves. Note there is no appropriate module parameter for this
+ since it would not make much sense.
+ Write any value to perform reset.
+
+ This option was added for bonding version 3.7.2.
+
3. Configuring Bonding Devices
==============================
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 88fcb25..342b4ed 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -1572,6 +1572,33 @@ out:
static DEVICE_ATTR(resend_igmp, S_IRUGO | S_IWUSR,
bonding_show_resend_igmp, bonding_store_resend_igmp);
+static ssize_t
+bonding_store_reset_failure_counters(struct device *d,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct slave *slave;
+ int i;
+ struct bonding *bond = to_bond(d);
+
+ if (!rtnl_trylock())
+ return restart_syscall();
+
+ pr_info("%s: Resetting failure counters.\n", bond->dev->name);
+
+ read_lock(&bond->lock);
+ bond_for_each_slave(bond, slave, i)
+ slave->link_failure_count = 0;
+ read_unlock(&bond->lock);
+
+ rtnl_unlock();
+
+ return count;
+}
+
+static DEVICE_ATTR(reset_failure_counters, S_IWUSR, NULL,
+ bonding_store_reset_failure_counters);
+
static struct attribute *per_bond_attrs[] = {
&dev_attr_slaves.attr,
&dev_attr_mode.attr,
@@ -1600,6 +1627,7 @@ static struct attribute *per_bond_attrs[] = {
&dev_attr_queue_id.attr,
&dev_attr_all_slaves_active.attr,
&dev_attr_resend_igmp.attr,
+ &dev_attr_reset_failure_counters.attr,
NULL,
};
diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h
index ea1d005..62528f8 100644
--- a/drivers/net/bonding/bonding.h
+++ b/drivers/net/bonding/bonding.h
@@ -24,8 +24,8 @@
#include "bond_3ad.h"
#include "bond_alb.h"
-#define DRV_VERSION "3.7.1"
-#define DRV_RELDATE "April 27, 2011"
+#define DRV_VERSION "3.7.2"
+#define DRV_RELDATE "Jun 1, 2011"
#define DRV_NAME "bonding"
#define DRV_DESCRIPTION "Ethernet Channel Bonding Driver"
--
1.7.4.4
^ permalink raw reply related
* [PATCH 2/2 v3] af-packet: Use existing netdev reference for bound sockets.
From: greearb @ 2011-06-01 17:18 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
In-Reply-To: <1306948733-22441-1-git-send-email-greearb@candelatech.com>
From: Ben Greear <greearb@candelatech.com>
This saves a network device lookup on each packet transmitted,
for sockets that are bound to a network device.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
v3: No changes since last time, but previous patch to hold
prot_hook.dev should fix locking problems with this patch.
:100644 100644 614d659... c1c5f33... M net/packet/af_packet.c
net/packet/af_packet.c | 27 +++++++++++++++------------
1 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 614d659..c1c5f33 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -994,7 +994,8 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
struct sk_buff *skb;
struct net_device *dev;
__be16 proto;
- int ifindex, err, reserve = 0;
+ bool need_rls_dev = false;
+ int err, reserve = 0;
void *ph;
struct sockaddr_ll *saddr = (struct sockaddr_ll *)msg->msg_name;
int tp_len, size_max;
@@ -1006,7 +1007,7 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
err = -EBUSY;
if (saddr == NULL) {
- ifindex = po->ifindex;
+ dev = po->prot_hook.dev;
proto = po->num;
addr = NULL;
} else {
@@ -1017,12 +1018,12 @@ static int tpacket_snd(struct packet_sock *po, struct msghdr *msg)
+ offsetof(struct sockaddr_ll,
sll_addr)))
goto out;
- ifindex = saddr->sll_ifindex;
proto = saddr->sll_protocol;
addr = saddr->sll_addr;
+ dev = dev_get_by_index(sock_net(&po->sk), saddr->sll_ifindex);
+ need_rls_dev = true;
}
- dev = dev_get_by_index(sock_net(&po->sk), ifindex);
err = -ENXIO;
if (unlikely(dev == NULL))
goto out;
@@ -1108,7 +1109,8 @@ out_status:
__packet_set_status(po, ph, status);
kfree_skb(skb);
out_put:
- dev_put(dev);
+ if (need_rls_dev)
+ dev_put(dev);
out:
mutex_unlock(&po->pg_vec_lock);
return err;
@@ -1146,8 +1148,9 @@ static int packet_snd(struct socket *sock,
struct sk_buff *skb;
struct net_device *dev;
__be16 proto;
+ bool need_rls_dev = false;
unsigned char *addr;
- int ifindex, err, reserve = 0;
+ int err, reserve = 0;
struct virtio_net_hdr vnet_hdr = { 0 };
int offset = 0;
int vnet_hdr_len;
@@ -1165,7 +1168,7 @@ static int packet_snd(struct socket *sock,
*/
if (saddr == NULL) {
- ifindex = po->ifindex;
+ dev = po->prot_hook.dev;
proto = po->num;
addr = NULL;
} else {
@@ -1174,13 +1177,12 @@ static int packet_snd(struct socket *sock,
goto out;
if (msg->msg_namelen < (saddr->sll_halen + offsetof(struct sockaddr_ll, sll_addr)))
goto out;
- ifindex = saddr->sll_ifindex;
proto = saddr->sll_protocol;
addr = saddr->sll_addr;
+ dev = dev_get_by_index(sock_net(sk), saddr->sll_ifindex);
+ need_rls_dev = true;
}
-
- dev = dev_get_by_index(sock_net(sk), ifindex);
err = -ENXIO;
if (dev == NULL)
goto out_unlock;
@@ -1320,14 +1322,15 @@ static int packet_snd(struct socket *sock,
if (err > 0 && (err = net_xmit_errno(err)) != 0)
goto out_unlock;
- dev_put(dev);
+ if (need_rls_dev)
+ dev_put(dev);
return len;
out_free:
kfree_skb(skb);
out_unlock:
- if (dev)
+ if (dev && need_rls_dev)
dev_put(dev);
out:
return err;
--
1.7.3.4
^ permalink raw reply related
* [PATCH 1/2] af-packet: Hold reference to bound network devices.
From: greearb @ 2011-06-01 17:18 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
Old code was probably safe, but with this change we
can actually use the netdev object, not just compare
the pointer values.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
:100644 100644 f7250d5... 614d659... M net/packet/af_packet.c
net/packet/af_packet.c | 14 +++++++++-----
1 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index f7250d5..614d659 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -1377,6 +1377,10 @@ static int packet_release(struct socket *sock)
__dev_remove_pack(&po->prot_hook);
__sock_put(sk);
}
+ if (po->prot_hook.dev) {
+ dev_put(po->prot_hook.dev);
+ po->prot_hook.dev = NULL;
+ }
spin_unlock(&po->bind_lock);
packet_flush_mclist(sk);
@@ -1430,6 +1434,8 @@ static int packet_do_bind(struct sock *sk, struct net_device *dev, __be16 protoc
po->num = protocol;
po->prot_hook.type = protocol;
+ if (po->prot_hook.dev)
+ dev_put(po->prot_hook.dev);
po->prot_hook.dev = dev;
po->ifindex = dev ? dev->ifindex : 0;
@@ -1474,10 +1480,8 @@ static int packet_bind_spkt(struct socket *sock, struct sockaddr *uaddr,
strlcpy(name, uaddr->sa_data, sizeof(name));
dev = dev_get_by_name(sock_net(sk), name);
- if (dev) {
+ if (dev)
err = packet_do_bind(sk, dev, pkt_sk(sk)->num);
- dev_put(dev);
- }
return err;
}
@@ -1505,8 +1509,6 @@ static int packet_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len
goto out;
}
err = packet_do_bind(sk, dev, sll->sll_protocol ? : pkt_sk(sk)->num);
- if (dev)
- dev_put(dev);
out:
return err;
@@ -2275,6 +2277,8 @@ static int packet_notifier(struct notifier_block *this, unsigned long msg, void
}
if (msg == NETDEV_UNREGISTER) {
po->ifindex = -1;
+ if (po->prot_hook.dev)
+ dev_put(po->prot_hook.dev);
po->prot_hook.dev = NULL;
}
spin_unlock(&po->bind_lock);
--
1.7.3.4
^ permalink raw reply related
* [PATCH 7/10] drivers/net/davinci_emac.c: add missing clk_put
From: Julia Lawall @ 2011-06-01 17:10 UTC (permalink / raw)
To: David S. Miller
Cc: kernel-janitors, Cyril Chemparathy, Sriramakrishnan A G,
Kevin Hilman, Stefan Weil, netdev, linux-kernel
From: Julia Lawall <julia@diku.dk>
Go to existing error handling code at the end of the function that calls
clk_put.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
expression e1,e2;
statement S;
@@
e1 = clk_get@p1(...);
... when != e1 = e2
when != clk_put(e1)
when any
if (...) { ... when != clk_put(e1)
when != if (...) { ... clk_put(e1) ... }
* return@p3 ...;
} else S
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/net/davinci_emac.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/davinci_emac.c b/drivers/net/davinci_emac.c
index 29a4f06..dcc4a17 100644
--- a/drivers/net/davinci_emac.c
+++ b/drivers/net/davinci_emac.c
@@ -1781,8 +1781,8 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
ndev = alloc_etherdev(sizeof(struct emac_priv));
if (!ndev) {
dev_err(&pdev->dev, "error allocating net_device\n");
- clk_put(emac_clk);
- return -ENOMEM;
+ rc = -ENOMEM;
+ goto free_clk;
}
platform_set_drvdata(pdev, ndev);
@@ -1796,7 +1796,8 @@ static int __devinit davinci_emac_probe(struct platform_device *pdev)
pdata = pdev->dev.platform_data;
if (!pdata) {
dev_err(&pdev->dev, "no platform data\n");
- return -ENODEV;
+ rc = -ENODEV;
+ goto probe_quit;
}
/* MAC addr and PHY mask , RMII enable info from platform_data */
@@ -1929,8 +1930,9 @@ no_dma:
iounmap(priv->remap_addr);
probe_quit:
- clk_put(emac_clk);
free_netdev(ndev);
+free_clk:
+ clk_put(emac_clk);
return rc;
}
^ permalink raw reply related
* [PATCH 4/10] drivers/net/can/flexcan.c: add missing clk_put
From: Julia Lawall @ 2011-06-01 17:10 UTC (permalink / raw)
To: Wolfgang Grandegger; +Cc: kernel-janitors, socketcan-core, netdev, linux-kernel
From: Julia Lawall <julia@diku.dk>
The failed_get label is used after the call to clk_get has succeeded, so it
should be moved up above the call to clk_put.
A simplified version of the semantic match that finds this problem is as
follows: (http://coccinelle.lip6.fr/)
// <smpl>
@r exists@
expression e1,e2;
statement S;
@@
e1 = clk_get@p1(...);
... when != e1 = e2
when != clk_put(e1)
when any
if (...) { ... when != clk_put(e1)
when != if (...) { ... clk_put(e1) ... }
* return@p3 ...;
} else S
// </smpl>
Signed-off-by: Julia Lawall <julia@diku.dk>
---
drivers/net/can/flexcan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/flexcan.c b/drivers/net/can/flexcan.c
index d499056..121739c 100644
--- a/drivers/net/can/flexcan.c
+++ b/drivers/net/can/flexcan.c
@@ -978,8 +978,8 @@ static int __devinit flexcan_probe(struct platform_device *pdev)
failed_map:
release_mem_region(mem->start, mem_size);
failed_req:
- clk_put(clk);
failed_get:
+ clk_put(clk);
failed_clock:
return err;
}
^ permalink raw reply related
* Re: [PATCH v3] af-packet: Add flag to distinguish VID 0 from no-vlan.
From: Eric Dumazet @ 2011-06-01 17:08 UTC (permalink / raw)
To: greearb; +Cc: netdev
In-Reply-To: <1306946950-29360-1-git-send-email-greearb@candelatech.com>
Le mercredi 01 juin 2011 à 09:49 -0700, greearb@candelatech.com a
écrit :
> From: Ben Greear <greearb@candelatech.com>
>
> Currently, user-space cannot determine if a 0 tcp_vlan_tci
> means there is no VLAN tag or the VLAN ID was zero.
>
> Add flag to make this explicit. User-space can check for
> TP_STATUS_VLAN_VALID || tp_vlan_tci > 0, which will be backwards
> compatible. Older could would have just checked for tp_vlan_tci,
> so it will work no worse than before.
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
Acked-by: Eric Dumazet <eric.dumazet@gmail.com>
Thanks
^ permalink raw reply
* Re: [patch net-next-2.6 v2] bonding: allow resetting slave failure counters
From: Flavio Leitner @ 2011-06-01 16:41 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Jay Vosburgh, netdev, davem, andy
In-Reply-To: <20110601163153.GB2784@psychotron.redhat.com>
On 06/01/2011 01:31 PM, Jiri Pirko wrote:
> This patch allows to reset failure counters for all enslaved devices.
>
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>
> v1->v2: added version bump
> added version note to doc
> moved and adjusted printk
> ---
> Documentation/networking/bonding.txt | 9 +++++++++
> drivers/net/bonding/bond_sysfs.c | 28 ++++++++++++++++++++++++++++
> drivers/net/bonding/bonding.h | 4 ++--
> 3 files changed, 39 insertions(+), 2 deletions(-)
Reviewed-by: Flavio Leitner <fbl@redhat.com>
Thanks!
fbl
^ permalink raw reply
* Re: Skipping past TCP lost packet in userspace
From: Bill Sommerfeld @ 2011-06-01 16:57 UTC (permalink / raw)
To: Josh Lehan; +Cc: Yuchung Cheng, netdev, jiyengar
In-Reply-To: <4DE5F3E3.2080609@krellan.com>
On Wed, Jun 1, 2011 at 01:10, Josh Lehan <linux@krellan.com> wrote:
> 2) If the out-of-order data is enough to be useful to the application,
> another ioctl() could be done, to ask the kernel to jump over the gap
> and deliver the data to the application.
this sounds like a different way to spell lseek(fd, gapsize, SEEK_CUR)
of course, allowing a subset of lseek for sockets could very well mess
with any code which (ab)uses lseek to guess the type of a random
descriptor.
^ permalink raw reply
* Re: [BUG] net: cpu offline cause napi stall
From: Eric Dumazet @ 2011-06-01 16:55 UTC (permalink / raw)
To: Heiko Carstens; +Cc: Frank Blaschka, davem, netdev, linux-s390
In-Reply-To: <20110601163628.GA2418@osiris.boeblingen.de.ibm.com>
Le mercredi 01 juin 2011 à 18:36 +0200, Heiko Carstens a écrit :
> On Wed, Jun 01, 2011 at 02:13:19PM +0200, Eric Dumazet wrote:
> > Le mercredi 01 juin 2011 à 12:33 +0200, Frank Blaschka a écrit :
> > > Hi Dave, Eric,
> > >
> > > during heavy network load we turn off/on cpus.
> > > Sometimes this causes a stall on the network device.
> > > Digging into the dump I found out following:
> > >
> > > napi is scheduled but does not run. From the I/O buffers
> > > and the napi state I see napi/rx_softirq processing has stopped
> > > because the budget was reached. napi stays in the
> > > softnet_data poll_list and the rx_softirq was raised again.
> > >
> > > I assume at this time the cpu offline comes in.
> > > the rx softirq is raised/moved to another cpu but napi stays in the poll_list
> > > of the softnet_data of the now offline cpu.
> > >
> > > reviewing dev_cpu_callback (net/core/dev.c) I did not find the poll_list
> > > is transfered to the new cpu. Do you think this could cause the stall or
> > > did I miss something?
> > >
> > > Thx for your help.
> >
> > Hi Frank
> >
> > I believe you are right, I cant see where the poll_list transfert from
> > dead cpu to online cpu is done.
> >
> > Do you want to prepare a patch ?
>
> Frank will be offline until next week. I assume the patch below would fix
> the problem, however its untested. I doubt we can verify that it really
> fixes the problem also until next week (public holiday tomorrow).
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 6561021..6d6a7cf 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -5981,6 +5981,8 @@ static int dev_cpu_callback(struct notifier_block *nfb,
> oldsd->output_queue = NULL;
> oldsd->output_queue_tailp = &oldsd->output_queue;
> }
> + /* Append NAPI poll list from offline CPU. */
> + list_splice_init(&oldsd->poll_list, &sd->poll_list);
>
> raise_softirq_irqoff(NET_TX_SOFTIRQ);
> local_irq_enable();
Same here, I'll be offline for the next 4 days ;)
Please make sure we raise NET_RX_SOFTIRQ on new cpu if necessary.
^ permalink raw reply
* [PATCH v3] af-packet: Add flag to distinguish VID 0 from no-vlan.
From: greearb @ 2011-06-01 16:49 UTC (permalink / raw)
To: netdev; +Cc: Ben Greear
From: Ben Greear <greearb@candelatech.com>
Currently, user-space cannot determine if a 0 tcp_vlan_tci
means there is no VLAN tag or the VLAN ID was zero.
Add flag to make this explicit. User-space can check for
TP_STATUS_VLAN_VALID || tp_vlan_tci > 0, which will be backwards
compatible. Older could would have just checked for tp_vlan_tci,
so it will work no worse than before.
Signed-off-by: Ben Greear <greearb@candelatech.com>
---
v3: Update brace formatting, no functional change.
:100644 100644 72bfa5a... 6d66ce1... M include/linux/if_packet.h
:100644 100644 4005b24... f7250d5... M net/packet/af_packet.c
include/linux/if_packet.h | 1 +
net/packet/af_packet.c | 15 ++++++++++++---
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/linux/if_packet.h b/include/linux/if_packet.h
index 72bfa5a..6d66ce1 100644
--- a/include/linux/if_packet.h
+++ b/include/linux/if_packet.h
@@ -70,6 +70,7 @@ struct tpacket_auxdata {
#define TP_STATUS_COPY 0x2
#define TP_STATUS_LOSING 0x4
#define TP_STATUS_CSUMNOTREADY 0x8
+#define TP_STATUS_VLAN_VALID 0x10 /* auxdata has valid tp_vlan_tci */
/* Tx ring - header status */
#define TP_STATUS_AVAILABLE 0x0
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 4005b24..f7250d5 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -818,7 +818,12 @@ static int tpacket_rcv(struct sk_buff *skb, struct net_device *dev,
getnstimeofday(&ts);
h.h2->tp_sec = ts.tv_sec;
h.h2->tp_nsec = ts.tv_nsec;
- h.h2->tp_vlan_tci = vlan_tx_tag_get(skb);
+ if (vlan_tx_tag_present(skb)) {
+ h.h2->tp_vlan_tci = vlan_tx_tag_get(skb);
+ status |= TP_STATUS_VLAN_VALID;
+ } else {
+ h.h2->tp_vlan_tci = 0;
+ }
hdrlen = sizeof(*h.h2);
break;
default:
@@ -1760,8 +1765,12 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
aux.tp_snaplen = skb->len;
aux.tp_mac = 0;
aux.tp_net = skb_network_offset(skb);
- aux.tp_vlan_tci = vlan_tx_tag_get(skb);
-
+ if (vlan_tx_tag_present(skb)) {
+ aux.tp_vlan_tci = vlan_tx_tag_get(skb);
+ aux.tp_status |= TP_STATUS_VLAN_VALID;
+ } else {
+ aux.tp_vlan_tci = 0;
+ }
put_cmsg(msg, SOL_PACKET, PACKET_AUXDATA, sizeof(aux), &aux);
}
--
1.7.3.4
^ permalink raw reply related
* Re: [BUG] net: cpu offline cause napi stall
From: Heiko Carstens @ 2011-06-01 16:36 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Frank Blaschka, davem, netdev, linux-s390
In-Reply-To: <1306930399.3476.1.camel@edumazet-laptop>
On Wed, Jun 01, 2011 at 02:13:19PM +0200, Eric Dumazet wrote:
> Le mercredi 01 juin 2011 à 12:33 +0200, Frank Blaschka a écrit :
> > Hi Dave, Eric,
> >
> > during heavy network load we turn off/on cpus.
> > Sometimes this causes a stall on the network device.
> > Digging into the dump I found out following:
> >
> > napi is scheduled but does not run. From the I/O buffers
> > and the napi state I see napi/rx_softirq processing has stopped
> > because the budget was reached. napi stays in the
> > softnet_data poll_list and the rx_softirq was raised again.
> >
> > I assume at this time the cpu offline comes in.
> > the rx softirq is raised/moved to another cpu but napi stays in the poll_list
> > of the softnet_data of the now offline cpu.
> >
> > reviewing dev_cpu_callback (net/core/dev.c) I did not find the poll_list
> > is transfered to the new cpu. Do you think this could cause the stall or
> > did I miss something?
> >
> > Thx for your help.
>
> Hi Frank
>
> I believe you are right, I cant see where the poll_list transfert from
> dead cpu to online cpu is done.
>
> Do you want to prepare a patch ?
Frank will be offline until next week. I assume the patch below would fix
the problem, however its untested. I doubt we can verify that it really
fixes the problem also until next week (public holiday tomorrow).
diff --git a/net/core/dev.c b/net/core/dev.c
index 6561021..6d6a7cf 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5981,6 +5981,8 @@ static int dev_cpu_callback(struct notifier_block *nfb,
oldsd->output_queue = NULL;
oldsd->output_queue_tailp = &oldsd->output_queue;
}
+ /* Append NAPI poll list from offline CPU. */
+ list_splice_init(&oldsd->poll_list, &sd->poll_list);
raise_softirq_irqoff(NET_TX_SOFTIRQ);
local_irq_enable();
^ permalink raw reply related
* Re: [patch net-next-2.6] de2104x: use speed defines instead of number
From: Jiri Pirko @ 2011-06-01 16:19 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, davem
In-Reply-To: <1306943206.22348.10.camel@localhost>
Wed, Jun 01, 2011 at 05:46:46PM CEST, bhutchings@solarflare.com wrote:
>The speed/speed_hi fields are defined to hold speed in Mbit/s, not only
>specific values. I don't see any reason to use the names any more.
Do you mean to remove SPEED_X defines in whole code?
>
>Ben.
>
>On Wed, 2011-06-01 at 16:14 +0200, Jiri Pirko wrote:
>> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
>> ---
>> drivers/net/tulip/de2104x.c | 4 ++--
>> 1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/tulip/de2104x.c b/drivers/net/tulip/de2104x.c
>> index e2f6923..1cc426d 100644
>> --- a/drivers/net/tulip/de2104x.c
>> +++ b/drivers/net/tulip/de2104x.c
>> @@ -1507,7 +1507,7 @@ static int __de_get_settings(struct de_private *de, struct ethtool_cmd *ecmd)
>> break;
>> }
>>
>> - ethtool_cmd_speed_set(ecmd, 10);
>> + ethtool_cmd_speed_set(ecmd, SPEED_10);
>>
>> if (dr32(MacMode) & FullDuplex)
>> ecmd->duplex = DUPLEX_FULL;
>> @@ -1529,7 +1529,7 @@ static int __de_set_settings(struct de_private *de, struct ethtool_cmd *ecmd)
>> u32 new_media;
>> unsigned int media_lock;
>>
>> - if (ethtool_cmd_speed(ecmd) != 10)
>> + if (ethtool_cmd_speed(ecmd) != SPEED_10)
>> return -EINVAL;
>> if (ecmd->duplex != DUPLEX_HALF && ecmd->duplex != DUPLEX_FULL)
>> return -EINVAL;
>
>--
>Ben Hutchings, Senior Software Engineer, Solarflare
>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
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox