* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Michael Chan @ 2011-03-08 19:43 UTC (permalink / raw)
To: Neil Horman
Cc: Ben Hutchings, netdev@vger.kernel.org, David S. Miller,
Dmitry Kravkov, Eddie Wai, Eilon Greenstein
In-Reply-To: <20110308193601.GA19669@hmsreliant.think-freely.org>
On Tue, 2011-03-08 at 11:36 -0800, Neil Horman wrote:
> On Tue, Mar 08, 2011 at 11:07:57AM -0800, Michael Chan wrote:
> >
> > On Tue, 2011-03-08 at 10:59 -0800, Ben Hutchings wrote:
> > > On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> > > > bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> > > > bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> > > > bnx2 and bnx2x (the former being a regex subset of the later).
> > >
> > > Since when does strcmp() do a regex match?
> >
> > Yeah, strcmp() does NULL-terminated string compare, right?
> >
> Sorry, poor choice of words. It doesn't do a regex match, I only ment to
> illustrate that bnx2 is a substring of bnx2x.
>
> strcmp does this:
>
> strcmp(const char *str1, const char *str2) {
> ...
> while (*s1 || *s2) {
> ...
> }
> ...
> }
>
> Since bnx2 is a substring of bnx2 both if clauses are a match, since you'll hit
> the null terminator of the shorter string in both, which means we call both
> initalization functions.
No, that should not happen. When we compare NULL-terminated "bnx2" with
NULL-terminated "bnx2x", it won't match because the '\0' won't match the
'x', right?
I think the patch is good. We can avoid the unnecessary strcmp() when
we have a match with "bnx2" already. But we should not get both matches
even without the patch.
>
> I'll leave commentary on initalization in is_cnic_dev to the authors :)
>
> Neil
>
>
> >
> > >
> > > Ben.
> > >
> > > > This causes
> > > > initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
> > > > catastrophic, is definately not expected. Fix this by choosing either the bnx2
> > > > or bnx2x initalization path, not both
> > > >
> > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > CC: David S. Miller <davem@davemloft.net>
> > > > CC: Michael Chan <mchan@broadcom.com>
> > > > CC: Dmitry Kravkov <dmitry@broadcom.com>
> > > > CC: Eddie Wai <waie@broadcom.com>
> > > > CC: Eilon Greenstein <eilong@broadcom.com>
> > > > ---
> > > > drivers/net/cnic.c | 2 +-
> > > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> > > > index 271a1f0..18b59ad 100644
> > > > --- a/drivers/net/cnic.c
> > > > +++ b/drivers/net/cnic.c
> > > > @@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
> > > >
> > > > if (!strcmp(drvinfo.driver, "bnx2"))
> > > > cdev = init_bnx2_cnic(dev);
> > > > - if (!strcmp(drvinfo.driver, "bnx2x"))
> > > > + else if (!strcmp(drvinfo.driver, "bnx2x"))
> > > > cdev = init_bnx2x_cnic(dev);
> > > > if (cdev) {
> > > > write_lock(&cnic_dev_lock);
> > >
> >
> >
> > --
> > 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] can: wrong index used in inner loop
From: roel @ 2011-03-08 19:52 UTC (permalink / raw)
To: Wolfgang Grandegger, socketcan-core, netdev, Andrew Morton, LKML
Index i was already used in the outer loop.
Signed-off-by: Roel Kluin <roel.kluin@gmail.com>
---
drivers/net/can/usb/esd_usb2.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index 05a5275..dc53c83 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -659,7 +659,7 @@ failed:
static void unlink_all_urbs(struct esd_usb2 *dev)
{
struct esd_usb2_net_priv *priv;
- int i;
+ int i, j;
usb_kill_anchored_urbs(&dev->rx_submitted);
for (i = 0; i < dev->net_count; i++) {
@@ -668,8 +668,8 @@ static void unlink_all_urbs(struct esd_usb2 *dev)
usb_kill_anchored_urbs(&priv->tx_submitted);
atomic_set(&priv->active_tx_jobs, 0);
- for (i = 0; i < MAX_TX_URBS; i++)
- priv->tx_contexts[i].echo_index = MAX_TX_URBS;
+ for (j = 0; j < MAX_TX_URBS; j++)
+ priv->tx_contexts[j].echo_index = MAX_TX_URBS;
}
}
}
^ permalink raw reply related
* Re: [stable] [stable-2.6.32.y] ixgbe: fix for 82599 erratum on Header Splitting
From: Greg KH @ 2011-03-08 19:48 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: gregkh, Don Skidmore, stable, netdev
In-Reply-To: <1299366890-10333-1-git-send-email-jeffrey.t.kirsher@intel.com>
On Sat, Mar 05, 2011 at 03:14:50PM -0800, Jeff Kirsher wrote:
> From: Don Skidmore <donald.c.skidmore@intel.com>
>
> We have found a hardware erratum on 82599 hardware that can lead to
> unpredictable behavior when Header Splitting mode is enabled. So
> we are no longer enabling this feature on affected hardware.
>
> This is an updated version of the following commit which applies to the stable
> linux-2.6.32.y tree:
Applied, thanks.
greg k-h
^ permalink raw reply
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
From: Rémi Denis-Courmont @ 2011-03-08 20:59 UTC (permalink / raw)
To: netdev
In-Reply-To: <20110308.112923.39177383.davem@davemloft.net>
Hello,
On March 8th 2011 21:29:23 David Miller, you wrote :
> You don't even read what I tell you, which makes reviewing your work
> insanely frustrating.
Fair enough.
You were the one complaining about the Nokia mail server being insanely
broken (in private email by the way). So I have to read stuff at home, and
implement it the next day at work. Informations do get lost at night. I don't
need to mention I don't like this is frustrating for us Nokians as well.
(...)
> It also irks me that you persisted to provide the most terse possible
> one-line commit message. Have a conversation in your commit message,
> this isn't for you it's for other people trying to understand your
> work.
I'm trying to fix the terrible mess that ST-Ericsson left of the Phonet stack
here. This wouldn't be an issue if the crap hadn't been merged, or if I had
been given enough time to voice my concerns.
Just one WTF? out of many:
static u8 data[4] = {
0x03, 0x04,
};
data[2] = pn->tx_fc;
data[3] = pn->rx_fc;
(Those coffee-deprivated, note the 'static' keyword.) I wonder if the guy ever
wrote C (not C++) before he came to kernel.org and left as quickly.
I am totally fine with strictness on your part. I do genuinely believe it
improves code quality and such, and do try to apply similar patterns when I am
the reviewer. So I present my apologizes for my series of unintended
violations of The Process.
I just wish you'd been as strict back then. Nobody would be having this
pointless non-technical discussion... Well... I'll fix it tomorrow because I
don't want that kind of crap in the Linux kernel anyway.
Thanks every one you for your cooperation.
--
Rémi
^ permalink raw reply
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
From: David Miller @ 2011-03-08 21:09 UTC (permalink / raw)
To: remi; +Cc: netdev
In-Reply-To: <201103082259.41844.remi@remlab.net>
From: "Rémi Denis-Courmont" <remi@remlab.net>
Date: Tue, 8 Mar 2011 22:59:41 +0200
> I just wish you'd been as strict back then.
The only thing worse that putting crappy code into the tree is having
patches rot in patchwork because the maintainer isn't responsive at
reviewing things.
^ permalink raw reply
* Re: [net-next-2.6 00/14][pull request] Intel Wired LAN Driver Updates
From: Jeff Kirsher @ 2011-03-08 21:10 UTC (permalink / raw)
To: David Miller
Cc: netdev@vger.kernel.org, gospo@redhat.com, bphilips@novell.com
In-Reply-To: <20110308.111137.112604487.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 855 bytes --]
On Tue, 2011-03-08 at 11:11 -0800, David Miller wrote:
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Mon, 7 Mar 2011 18:24:54 -0800
>
> > The following series contains fixes and cleanups for e1000e and ixgbe, as
> > well as support for FCoE DDP in target mode for ixgbe. In igb, added
> > additional stats for OS2BMC feature.
> >
> > The following are changes since commit 541ac7c9b30ee2ff84ad87f27e0bc069e143afb5:
> > bonding: COW before overwriting the destination MAC address
> >
> > and are available in the git repository at:
> > master.kernel.org:/pub/scm/linux/kernel/git/jkirsher/net-next-2.6 master
>
> Pulled.
>
> Please address the feedback given on the lack of docbook descriptions for
> all of those fcoe device ops.
Thanks Dave.
I will work with Yi to get FCoE documentation patches together.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: [PATCH 11/17] mlx4_en: Enabling new steering
From: Or Gerlitz @ 2011-03-08 21:10 UTC (permalink / raw)
To: Yevgeny Petrilin; +Cc: davem, netdev, vlad, Roland Dreier, nirmu, Aleksey Senin
In-Reply-To: <4D74B0C9.9020108@mellanox.co.il>
Yevgeny Petrilin <yevgenyp@mellanox.co.il> wrote:
> +++ b/drivers/infiniband/hw/mlx4/main.c
> @@ -623,6 +623,14 @@ static int mlx4_ib_mcg_attach(struct ib_qp *ibqp, union
[...]
> + if (mdev->dev->caps.vep_mc_steering && ibqp->qp_type == IB_QPT_RAW_ETHERTYPE) {
Guys,
Didn't you want to refer here to the "raw ethernet QP" which is typed
to IB_QPT_RAW_PACKET by the series posted on December last year, see
http://marc.info/?l=linux-rdma&m=129162076818154 --
IB_QPT_RAW_ETHERTYPE is a rename of IB's IB_QPT_RAW_ETY done by
commit a2ebf07ae53e65bd073f96877e4818f2e89271ae
Or.
^ permalink raw reply
* Re: [patch net-next-2.6 4/8] bonding: wrap slave state work
From: Nicolas de Pesloüan @ 2011-03-08 21:23 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, shemminger, kaber, fubar, eric.dumazet, andy
In-Reply-To: <20110308071805.GB2826@psychotron.redhat.com>
Le 08/03/2011 08:18, Jiri Pirko a écrit :
> Mon, Mar 07, 2011 at 08:55:12PM CET, nicolas.2p.debian@gmail.com wrote:
>> Le 07/03/2011 10:58, Jiri Pirko a écrit :
>>>>>
>>>>> +static inline void bond_set_active_slave(struct slave *slave)
>>>>> +{
>>>>> + slave->backup = 0;
>>>>
>>>> In the comment above, you said that the possible value for backup
>>>> corresponds with BOND_STATE_ACTIVE and BOND_STATE_BACKUP.
>>>>
>>>> So, should be:
>>>>
>>>> slave->backup = BOND_STATE_ACTIVE;
>>>>
>>>>> +}
>>>>> +
>>>>> +static inline void bond_set_backup_slave(struct slave *slave)
>>>>> +{
>>>>> + slave->backup = 1;
>>>>
>>>> slave->backup = BOND_STATE_BACKUP;
>>>>
>>>
>>> Well, I think it's weird and misleading to assign some define to :1
>>> bitfield. Should be 0 or 1, nothing else.
>>
>> Agreed, but the comment appears missleading... May be you should fix the comment, not the code.
>
> Hmm. I thought that the comment is accurate. BOND_STATE_ACTIVE
> corresponds with 0, BOND_STATE_BACKUP corresponds with 1. Anyway, let me
> know how would you like to formulate this and I can repost (or do a
> little comment-changing followup)
>
> Thanks Nicolas
Hi Jiri,
I'm probably too perfectionist. Keep it the way it is.
Reviewed-by: Nicolas de Pesloüan <nicolas.2p.debian@free.fr>
^ permalink raw reply
* Re: [Patch V2] bonding: fix netpoll in active-backup mode
From: Andy Gospodarek @ 2011-03-08 21:24 UTC (permalink / raw)
To: Amerigo Wang; +Cc: linux-kernel, Neil Horman, Jay Vosburgh, netdev
In-Reply-To: <1299578336-5888-1-git-send-email-amwang@redhat.com>
On Tue, Mar 08, 2011 at 05:58:56PM +0800, Amerigo Wang wrote:
> V2: avoid calling slave_diable_netpoll() with write_lock_bh() held.
>
> netconsole doesn't work in active-backup mode, because we don't do anything
> for nic failover in active-backup mode. We should disable netpoll on the
> failing slave when it is detected down and enable netpoll when it becomes
> the active slave.
>
> Tested by ifdown the current active slave and ifup it again for several times,
> netconsole works well.
>
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
>
It seems like you are going to a lot of trouble to fix a bug where
netpoll will not be setup on any interface that is down when enslaved.
That seems to be the only path that would not have slave->np setup
properly at enslavement.
Did you ever try just this?
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 0592e6d..8d93044 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1352,8 +1352,6 @@ static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
read_lock(&bond->lock);
bond_for_each_slave(bond, slave, i) {
- if (!IS_UP(slave->dev))
- continue;
err = slave_enable_netpoll(slave);
if (err) {
__bond_netpoll_cleanup(bond);
^ permalink raw reply related
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
From: Rémi Denis-Courmont @ 2011-03-08 21:30 UTC (permalink / raw)
To: netdev
In-Reply-To: <20110308.130926.246531377.davem@davemloft.net>
Le mardi 8 mars 2011 23:09:26 David Miller, vous avez écrit :
> From: "Rémi Denis-Courmont" <remi@remlab.net>
> Date: Tue, 8 Mar 2011 22:59:41 +0200
>
> > I just wish you'd been as strict back then.
>
> The only thing worse that putting crappy code into the tree is having
> patches rot in patchwork because the maintainer isn't responsive at
> reviewing things.
Patch submitted:
Date: Mon, 27 Sep 2010 10:37:59 +0530
Patch merged:
Date: Mon, 27 Sep 2010 21:50:59 -0700
So the patches had been "rotting" for 0 days 23:13:00. Come on.
--
Rémi Denis-Courmont
http://www.remlab.info/
http://fi.linkedin.com/in/remidenis
^ permalink raw reply
* Re: [PATCH 11/17] mlx4_en: Enabling new steering
From: Roland Dreier @ 2011-03-08 21:30 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Yevgeny Petrilin, davem, netdev, vlad, nirmu, Aleksey Senin
In-Reply-To: <AANLkTikftARWMbmy8zHbRShBbbnQhXhiequzdR_aPBst@mail.gmail.com>
On Tue, Mar 8, 2011 at 1:10 PM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
>> + if (mdev->dev->caps.vep_mc_steering && ibqp->qp_type == IB_QPT_RAW_ETHERTYPE) {
>
> Guys,
>
> Didn't you want to refer here to the "raw ethernet QP" which is typed
> to IB_QPT_RAW_PACKET by the series posted on December last year, see
> http://marc.info/?l=linux-rdma&m=129162076818154 --
> IB_QPT_RAW_ETHERTYPE is a rename of IB's IB_QPT_RAW_ETY done by
> commit a2ebf07ae53e65bd073f96877e4818f2e89271ae
Good catch. I think the drivers/infiniband part of this patch should
just be dropped -- we don't have any raw QP support of any kind, right?
- R.
^ permalink raw reply
* Re: [PATCH 1/8] Phonet: fix NULL-deref in a8059512b120362b15424f152b2548fe8b11bd0c
From: David Miller @ 2011-03-08 21:34 UTC (permalink / raw)
To: remi; +Cc: netdev
In-Reply-To: <201103082330.31652.remi@remlab.net>
From: "Rémi Denis-Courmont" <remi@remlab.net>
Date: Tue, 8 Mar 2011 23:30:31 +0200
> Le mardi 8 mars 2011 23:09:26 David Miller, vous avez écrit :
>> From: "Rémi Denis-Courmont" <remi@remlab.net>
>> Date: Tue, 8 Mar 2011 22:59:41 +0200
>>
>> > I just wish you'd been as strict back then.
>>
>> The only thing worse that putting crappy code into the tree is having
>> patches rot in patchwork because the maintainer isn't responsive at
>> reviewing things.
>
> Patch submitted:
> Date: Mon, 27 Sep 2010 10:37:59 +0530
>
> Patch merged:
> Date: Mon, 27 Sep 2010 21:50:59 -0700
>
> So the patches had been "rotting" for 0 days 23:13:00. Come on.
I gave you nearly a full day.
If I do my daily email scan twice, and see the same patch on both
occaisions without a review I am very likely to merge the thing
or poke the maintainer.
^ permalink raw reply
* Re: [patch net-next-2.6] net: reinject arps into bonding slave instead of master
From: Nicolas de Pesloüan @ 2011-03-08 21:34 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jiri Pirko, netdev, davem, shemminger, kaber, fubar, eric.dumazet
In-Reply-To: <20110308024356.GV11864@gospo.rdu.redhat.com>
Le 08/03/2011 03:43, Andy Gospodarek a écrit :
> On Tue, Mar 08, 2011 at 12:09:48AM +0100, Nicolas de Pesloüan wrote:
>>
>> After Jiri's last patch series, bond_arp_rcv() is not registered anymore
>> as a protocol handler on bond0, but directly called from inside
>> bond_handle_frame(), through bond->recv_probe.
>>
>> Because bond_handler_frame() is a rx_handler for the slave interfaces,
>> bond_arp_rcv() is now called at the slave level and not a the master
>> level anymore.
>>
>> Hence this patch and the reason I thought it should work.
>>
>> Did you tested this patch with Jiri's previous patches applied before?
>>
>
> No, I have not tested them yet. I looked at this problem first as
> Jiri's patch did not fix a regression in code that is actually in
> net-next right now.
For as far as I understand, the regression was introduced by Jiri's patches that are not yet
accepted in net-next-2.6, so this path definitely need the full patch series to be applied before.
And there is no special urgency to fix this regression, because it doesn't "exist" yet anywhere
except in some sandboxes.
This patch change the reinjection to inject at the slave level instead of the master level, because
in Jiri's patch series, ARP handling moved from a normal protocol handler registered at the master
level to the rx_handler at the slave level.
Jiri, do I miss something ?
> I don't have a problem with the work you and Jiri are doing on this
> latest patch series, but the frequency of patch revisions and now
> regressions concerns me.
I agree with your concerns, and this is the reason why Jiri and I are working hard to try and fight
every possible problems in the whole package of patch. I think that, at the end, Jiri and I will
have to properly resubmit the whole thing, for the convenient of those who will have to decide to
accept it or not.
Nicolas.
^ permalink raw reply
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Neil Horman @ 2011-03-08 21:40 UTC (permalink / raw)
To: Michael Chan
Cc: Ben Hutchings, netdev@vger.kernel.org, David S. Miller,
Dmitry Kravkov, Eddie Wai, Eilon Greenstein
In-Reply-To: <1299613396.12601.21.camel@HP1>
On Tue, Mar 08, 2011 at 11:43:16AM -0800, Michael Chan wrote:
>
> On Tue, 2011-03-08 at 11:36 -0800, Neil Horman wrote:
> > On Tue, Mar 08, 2011 at 11:07:57AM -0800, Michael Chan wrote:
> > >
> > > On Tue, 2011-03-08 at 10:59 -0800, Ben Hutchings wrote:
> > > > On Tue, 2011-03-08 at 13:56 -0500, Neil Horman wrote:
> > > > > bnx2 cards can work with the cnic driver, but when the cnic driver detects a
> > > > > bnx2 card, is_cnic_dev erroneously calls the initalization routines for both
> > > > > bnx2 and bnx2x (the former being a regex subset of the later).
> > > >
> > > > Since when does strcmp() do a regex match?
> > >
> > > Yeah, strcmp() does NULL-terminated string compare, right?
> > >
> > Sorry, poor choice of words. It doesn't do a regex match, I only ment to
> > illustrate that bnx2 is a substring of bnx2x.
> >
> > strcmp does this:
> >
> > strcmp(const char *str1, const char *str2) {
> > ...
> > while (*s1 || *s2) {
> > ...
> > }
> > ...
> > }
> >
> > Since bnx2 is a substring of bnx2 both if clauses are a match, since you'll hit
> > the null terminator of the shorter string in both, which means we call both
> > initalization functions.
>
> No, that should not happen. When we compare NULL-terminated "bnx2" with
> NULL-terminated "bnx2x", it won't match because the '\0' won't match the
> 'x', right?
>
> I think the patch is good. We can avoid the unnecessary strcmp() when
> we have a match with "bnx2" already. But we should not get both matches
> even without the patch.
>
Crap, sorry, I rescind this (although you're right, it probably wouldn't hurt to
roll this change in, just to save a potential second strcmp). Anywho, I'm
getting this error on the ifup of my bnx2 interface:
bnx2i: iSCSI not supported, dev=eth1
That message appears to only get set (from my read) on cards driven by the bnx2x
driver, and when I saw the 2 ifs rather than the if/else, I jumped to a
conclusion that we must be using the bnx2x init path on a bnx2 driver.
Apologies.
/me wanders off to see where completion_status gets set to
ISCSI_KCQE_COMPLETION_STATUS_ISCSI_NOT_SUPPORTED for bnx2.
Neil
> >
> > I'll leave commentary on initalization in is_cnic_dev to the authors :)
> >
> > Neil
> >
> >
> > >
> > > >
> > > > Ben.
> > > >
> > > > > This causes
> > > > > initalization of bnx2 to unilaterally fail in the cnic driver, which, while not
> > > > > catastrophic, is definately not expected. Fix this by choosing either the bnx2
> > > > > or bnx2x initalization path, not both
> > > > >
> > > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > CC: David S. Miller <davem@davemloft.net>
> > > > > CC: Michael Chan <mchan@broadcom.com>
> > > > > CC: Dmitry Kravkov <dmitry@broadcom.com>
> > > > > CC: Eddie Wai <waie@broadcom.com>
> > > > > CC: Eilon Greenstein <eilong@broadcom.com>
> > > > > ---
> > > > > drivers/net/cnic.c | 2 +-
> > > > > 1 files changed, 1 insertions(+), 1 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/cnic.c b/drivers/net/cnic.c
> > > > > index 271a1f0..18b59ad 100644
> > > > > --- a/drivers/net/cnic.c
> > > > > +++ b/drivers/net/cnic.c
> > > > > @@ -5292,7 +5292,7 @@ static struct cnic_dev *is_cnic_dev(struct net_device *dev)
> > > > >
> > > > > if (!strcmp(drvinfo.driver, "bnx2"))
> > > > > cdev = init_bnx2_cnic(dev);
> > > > > - if (!strcmp(drvinfo.driver, "bnx2x"))
> > > > > + else if (!strcmp(drvinfo.driver, "bnx2x"))
> > > > > cdev = init_bnx2x_cnic(dev);
> > > > > if (cdev) {
> > > > > write_lock(&cnic_dev_lock);
> > > >
> > >
> > >
> > > --
> > > 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
* Re: [Patch V2] bonding: fix netpoll in active-backup mode
From: Neil Horman @ 2011-03-08 21:41 UTC (permalink / raw)
To: Andy Gospodarek; +Cc: Amerigo Wang, linux-kernel, Jay Vosburgh, netdev
In-Reply-To: <20110308212405.GX11864@gospo.rdu.redhat.com>
On Tue, Mar 08, 2011 at 04:24:05PM -0500, Andy Gospodarek wrote:
> On Tue, Mar 08, 2011 at 05:58:56PM +0800, Amerigo Wang wrote:
> > V2: avoid calling slave_diable_netpoll() with write_lock_bh() held.
> >
> > netconsole doesn't work in active-backup mode, because we don't do anything
> > for nic failover in active-backup mode. We should disable netpoll on the
> > failing slave when it is detected down and enable netpoll when it becomes
> > the active slave.
> >
> > Tested by ifdown the current active slave and ifup it again for several times,
> > netconsole works well.
> >
> > Signed-off-by: WANG Cong <amwang@redhat.com>
> > Cc: Neil Horman <nhorman@tuxdriver.com>
> >
>
> It seems like you are going to a lot of trouble to fix a bug where
> netpoll will not be setup on any interface that is down when enslaved.
> That seems to be the only path that would not have slave->np setup
> properly at enslavement.
>
> Did you ever try just this?
>
+1, this is what I was suggesting. Just setup netpoll on all the slaves
regardless of status, and let their netpoll state follow that of the bond.
Neil
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 0592e6d..8d93044 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -1352,8 +1352,6 @@ static int bond_netpoll_setup(struct net_device *dev, struct netpoll_info *ni)
>
> read_lock(&bond->lock);
> bond_for_each_slave(bond, slave, i) {
> - if (!IS_UP(slave->dev))
> - continue;
> err = slave_enable_netpoll(slave);
> if (err) {
> __bond_netpoll_cleanup(bond);
> --
> 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 net-next-2.6] net: reinject arps into bonding slave instead of master
From: Nicolas de Pesloüan @ 2011-03-08 21:44 UTC (permalink / raw)
To: Andy Gospodarek
Cc: Jiri Pirko, netdev, davem, shemminger, kaber, fubar, eric.dumazet
In-Reply-To: <20110308134247.GW11864@gospo.rdu.redhat.com>
Le 08/03/2011 14:42, Andy Gospodarek a écrit :
> I'm pretty sure this patch will have the same catastrophic problem your
> last one did. By cloning and setting skb2->dev = orig_dev you just
> inserted a frame identical to the one we received right back into the
> stack. It only took a few minutes for my box to melt as one frame on
> the wire will cause an infinite number of frames to be received by the
> stack.
I agree with Andy. We still keep one reinject (netif_rx), which is probably better that two
(__netif_receive_skb), but not enough.
I really think we need a general framework for late delivery of final packets to packet handler
registered somewhere in the rx_handler path.
Jiri, is this patch the one you announced as "I have some kind nice solution in mind and I'm going
to submit that as a patch later (too many patches are in the wind atm)" ?
^ permalink raw reply
* Re: [PATCH] cnic: fix double initalization of bnx2 based cards
From: Michael Chan @ 2011-03-08 21:41 UTC (permalink / raw)
To: Neil Horman
Cc: Ben Hutchings, netdev@vger.kernel.org, David S. Miller,
Dmitry Kravkov, Eddie Wai, Eilon Greenstein
In-Reply-To: <20110308214028.GD19669@hmsreliant.think-freely.org>
On Tue, 2011-03-08 at 13:40 -0800, Neil Horman wrote:
> > No, that should not happen. When we compare NULL-terminated "bnx2" with
> > NULL-terminated "bnx2x", it won't match because the '\0' won't match the
> > 'x', right?
> >
> > I think the patch is good. We can avoid the unnecessary strcmp() when
> > we have a match with "bnx2" already. But we should not get both matches
> > even without the patch.
> >
> Crap, sorry, I rescind this (although you're right, it probably wouldn't hurt to
> roll this change in, just to save a potential second strcmp). Anywho, I'm
> getting this error on the ifup of my bnx2 interface:
> bnx2i: iSCSI not supported, dev=eth1
>
> That message appears to only get set (from my read) on cards driven by the bnx2x
> driver, and when I saw the 2 ifs rather than the if/else, I jumped to a
> conclusion that we must be using the bnx2x init path on a bnx2 driver.
> Apologies.
>
> /me wanders off to see where completion_status gets set to
> ISCSI_KCQE_COMPLETION_STATUS_ISCSI_NOT_SUPPORTED for bnx2.
On bnx2, this error code is set by the firmware if iSCSI is not
configured in NVRAM.
Thanks.
^ permalink raw reply
* Re: Network performance with small packets
From: Shirley Ma @ 2011-03-08 21:57 UTC (permalink / raw)
To: Rusty Russell
Cc: Michael S. Tsirkin, Krishna Kumar2, David Miller, kvm, netdev,
steved
In-Reply-To: <201102091107.20270.rusty@rustcorp.com.au>
On Wed, 2011-02-09 at 11:07 +1030, Rusty Russell wrote:
> I've finally read this thread... I think we need to get more serious
> with our stats gathering to diagnose these kind of performance issues.
>
> This is a start; it should tell us what is actually happening to the
> virtio ring(s) without significant performance impact...
Should we also add similar stat on vhost vq as well for monitoring
vhost_signal & vhost_notify?
Shirley
^ permalink raw reply
* kbench_mod
From: David Miller @ 2011-03-08 22:10 UTC (permalink / raw)
To: netdev
I just added a special kernel module to the net_test_tools tree,
it allows more accurate measurement of routing lookups.
It does cycle counts across one routing lookup.
The flow key is controlled by module parameters, iif != 0 means
input route lookup, otherwise we do an output lookup.
What's cool is you can jack-up the warmup_count value and then
run insmod under "perf record" and it just works.
^ permalink raw reply
* Re: [PATCH 1/2 net-2.6] r6040: fix multicast operations
From: David Miller @ 2011-03-08 22:22 UTC (permalink / raw)
To: florian; +Cc: netdev
In-Reply-To: <201103071109.40853.florian@openwrt.org>
From: Florian Fainelli <florian@openwrt.org>
Date: Mon, 7 Mar 2011 11:09:40 +0100
> From: Shawn Lin <shawn@dmp.com.tw>
>
> The original code does not work well when the number of mulitcast
> address to handle is greater than MCAST_MAX. It only enable promiscous
> mode instead of multicast hash table mode, so the hash table function
> will not be activated and all multicast frames will be recieved in this
> condition.
>
> This patch fixes the following issues with the r6040 NIC operating in
> multicast:
>
> 1) When the IFF_ALLMULTI flag is set, we should write 0xffff to the NIC
> hash table registers to make it process multicast traffic.
>
> 2) When the number of multicast address to handle is smaller than
> MCAST_MAX, we should use the NIC multicast registers MID1_{L,M,H}.
>
> 3) The hashing of the address was not correct, due to an invalid
> substraction (15 - (crc & 0x0f)) instead of (crc & 0x0f) and an
> incorrect crc algorithm (ether_crc_le) instead of (ether_crc).
>
> 4) If necessary, we should set HASH_EN flag in MCR0 to enable multicast
> hash table function.
>
> Reported-by: Marc Leclerc <marc-leclerc@signaturealpha.com>
> Tested-by: Marc Leclerc <marc-leclerc@signaturealpha.com>
> Signed-off-by: Shawn Lin <shawn@dmp.com.tw>
> Signed-off-by: Albert Chen <albert.chen@rdc.com.tw>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2 net-2.6] r6040: bump to version 0.27 and date 23Feb2011
From: David Miller @ 2011-03-08 22:22 UTC (permalink / raw)
To: florian; +Cc: netdev
In-Reply-To: <201103071109.42377.florian@openwrt.org>
From: Florian Fainelli <florian@openwrt.org>
Date: Mon, 7 Mar 2011 11:09:42 +0100
> From: Florian Fainelli <florian@openwrt.org>
>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> Signed-off-by: David S. Miller <davem@davemloft.net>
Applied.
^ permalink raw reply
* [PATCH net-next-2.6] ixgbe: fix compile failure in ixgbe_init_mbx_params_pf
From: Andy Gospodarek @ 2011-03-08 22:24 UTC (permalink / raw)
To: netdev, davem, Emil Tantilov, Jeff Kirsher
This commit:
commit d7c8a29fc8bd20ba45ec2f52577ed04a988a9500
Author: Emil Tantilov <emil.s.tantilov@intel.com>
Date: Thu Mar 3 09:25:02 2011 +0000
ixgbe: improve logic in ixgbe_init_mbx_params_pf
incorrectly added a line that accessed mbx->udelay. I'm sure the intent
was mbx->usec_delay. This patch fixes the compilation error.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
---
drivers/net/ixgbe/ixgbe_mbx.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ixgbe/ixgbe_mbx.c b/drivers/net/ixgbe/ixgbe_mbx.c
index 3cf8aec..c7ed82e 100644
--- a/drivers/net/ixgbe/ixgbe_mbx.c
+++ b/drivers/net/ixgbe/ixgbe_mbx.c
@@ -453,7 +453,7 @@ void ixgbe_init_mbx_params_pf(struct ixgbe_hw *hw)
return;
mbx->timeout = 0;
- mbx->udelay = 0;
+ mbx->usec_delay = 0;
mbx->stats.msgs_tx = 0;
mbx->stats.msgs_rx = 0;
^ permalink raw reply related
* Re: [PATCH] dsa/mv88e6060: support nonzero mii base address
From: David Miller @ 2011-03-08 22:24 UTC (permalink / raw)
To: buytenh; +Cc: jacmet, netdev
In-Reply-To: <20110307182413.GZ16649@mail.wantstofly.org>
From: Lennert Buytenhek <buytenh@wantstofly.org>
Date: Mon, 7 Mar 2011 19:24:13 +0100
> On Mon, Mar 07, 2011 at 04:49:47PM +0100, Peter Korsgaard wrote:
>
>> The mv88e6060 uses either the lower 16 or upper 16 mii addresses,
>> depending on the value of the EE_CLK/ADDR4 pin. Support both
>> configurations by using the sw_addr setting as base address.
>>
>> Signed-off-by: Peter Korsgaard <jacmet@sunsite.dk>
>
> Looks good!
>
> Acked-by: Lennert Buytenhek <buytenh@wantstofly.org>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next-2.6] ixgbe: fix compile failure in ixgbe_init_mbx_params_pf
From: David Miller @ 2011-03-08 22:26 UTC (permalink / raw)
To: andy; +Cc: netdev, davem, emil.s.tantilov, jeffrey.t.kirsher
In-Reply-To: <1299623047-25656-1-git-send-email-andy@greyhouse.net>
From: Andy Gospodarek <andy@greyhouse.net>
Date: Tue, 8 Mar 2011 17:24:07 -0500
> This commit:
>
> commit d7c8a29fc8bd20ba45ec2f52577ed04a988a9500
> Author: Emil Tantilov <emil.s.tantilov@intel.com>
> Date: Thu Mar 3 09:25:02 2011 +0000
>
> ixgbe: improve logic in ixgbe_init_mbx_params_pf
>
> incorrectly added a line that accessed mbx->udelay. I'm sure the intent
> was mbx->usec_delay. This patch fixes the compilation error.
>
> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
Applied, thanks Andy.
^ permalink raw reply
* Re: [PATCH 11/17] mlx4_en: Enabling new steering
From: David Miller @ 2011-03-08 22:26 UTC (permalink / raw)
To: roland; +Cc: or.gerlitz, yevgenyp, netdev, vlad, nirmu, alekseys
In-Reply-To: <AANLkTim8XDT5YKJc4wroyEDJc1X+xZOFL9y_090F5eLr@mail.gmail.com>
From: Roland Dreier <roland@kernel.org>
Date: Tue, 8 Mar 2011 13:30:25 -0800
> On Tue, Mar 8, 2011 at 1:10 PM, Or Gerlitz <or.gerlitz@gmail.com> wrote:
>>> + if (mdev->dev->caps.vep_mc_steering && ibqp->qp_type == IB_QPT_RAW_ETHERTYPE) {
>>
>> Guys,
>>
>> Didn't you want to refer here to the "raw ethernet QP" which is typed
>> to IB_QPT_RAW_PACKET by the series posted on December last year, see
>> http://marc.info/?l=linux-rdma&m=129162076818154 --
>> IB_QPT_RAW_ETHERTYPE is a rename of IB's IB_QPT_RAW_ETY done by
>> commit a2ebf07ae53e65bd073f96877e4818f2e89271ae
>
> Good catch. I think the drivers/infiniband part of this patch should
> just be dropped -- we don't have any raw QP support of any kind, right?
Once this is all sorted out, please someone respin this entire patch
set.
Thanks.
^ 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