* Re: [PATCH] Add eeprom_bad_csum_allow module option to e1000.
From: David Miller @ 2007-10-23 21:51 UTC (permalink / raw)
To: auke-jan.h.kok; +Cc: jeff, ajax, linux-kernel, netdev
In-Reply-To: <471E6121.9010008@intel.com>
From: "Kok, Auke" <auke-jan.h.kok@intel.com>
Date: Tue, 23 Oct 2007 14:01:21 -0700
> We help everyone out, and if you merge this patch you will prevent
> users from getting to us for support in the first place.
If using the bad eeprom has to be explicitly enabled by the user, your
argument holds no water. We just need to make sure the patch does
that.
^ permalink raw reply
* Re: [PATCH] Add eeprom_bad_csum_allow module option to e1000.
From: David Miller @ 2007-10-23 21:53 UTC (permalink / raw)
To: davej; +Cc: jeff, auke-jan.h.kok, ajax, linux-kernel, netdev
In-Reply-To: <20071023212026.GF7793@redhat.com>
From: Dave Jones <davej@redhat.com>
Date: Tue, 23 Oct 2007 17:20:26 -0400
> Indeed. This is a common enough problem that not including it causes
> more pain than its worth. I have two affected boxes myself that I
> actually thought the hardware was dead before I tried ajax's patch.
>
> People aren't going to report this as a bug. They aren't going to
> try out patches, they're going to do what I did and stick another
> network card in the box and go on with life.
>
> Our users deserve better than this.
Seconded. The resistence to this patch is just flat-out rediculious,
just like it was in the e100 case.
And I think all of this "e1000 is different!" talk is merely a
scarecrow for the fact that Intel simply doesn't want this patch
merged for some other reason.
^ permalink raw reply
* Re: [VLAN]: MAINTAINERS update
From: Ben Greear @ 2007-10-23 22:17 UTC (permalink / raw)
To: Patrick McHardy; +Cc: David Miller, Linux Netdev List
In-Reply-To: <471E2362.7040104@trash.net>
Patrick McHardy wrote:
>
> ------------------------------------------------------------------------
>
> [VLAN]: MAINTAINERS update
>
> Ben stepped down from VLAN maintainership due to a lack of time,
> add myself as new maintainer.
>
ACK. Thanks Patrick!
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [RFD] iptables: mangle table obsoletes filter table
From: Bill Davidsen @ 2007-10-23 22:27 UTC (permalink / raw)
To: Al Boldi
Cc: Valdis.Kletnieks, Patrick McHardy, netfilter-devel, netdev,
linux-net, linux-kernel
In-Reply-To: <200710210731.58959.a1426z@gawab.com>
Al Boldi wrote:
> Valdis.Kletnieks@vt.edu wrote:
>> On Sat, 20 Oct 2007 06:40:02 +0300, Al Boldi said:
>>> Sure, the idea was to mark the filter table obsolete as to make people
>>> start using the mangle table to do their filtering for new setups. The
>>> filter table would then still be available for legacy/special setups.
>>> But this would only be possible if we at least ported the REJECT target
>>> to mangle.
>> That's *half* the battle. The other half is explaining why I should move
>> from a perfectly functional setup that uses the filter table. What gains
>> do I get from doing so? What isn't working that I don't know about? etc?
>>
>> In other words - why do I want to move from filter to mangle?
>
> This has already been explained in this thread; here it is again:
>
> Al Boldi wrote:
>>>> The problem is that people think they are safe with the filter table,
>>>> when in fact they need the prerouting chain to seal things. Right now
>>>> this is only possible in the mangle table.
>>> Why do they need PREROUTING?
>> Well, for example to stop any transient packets being forwarded. You could
>> probably hack around this using mark's, but you can't stop the implied
>> route lookup, unless you stop it in prerouting.
>
> Basically, you have one big unintended gaping whole in your firewall, that
> could easily be exploited for DoS attacks at the least, unless you put in
> specific rules to limit this.
>
Well... true enough on a small firewall machine with a really fast link,
maybe. I like your point about efficiency better, it's more logical,
like putting an ACCEPT of established connections before a lot of low
probability rules. The only time I have seen rules actually bog a
machine was when a major ISP sent out a customer "upgrade" with a bug
which caused certain connections to be SYN-SYN/ACK-RST leaving half open
sockets. They sent out 160k of them and the blocking list became very
long as blocking rules were added.
> Plus, it's outrageously incorrect to accept invalid packets, just because
> your filtering infrastructure can only reject packets after they have been
> prerouted.
>
As long as the filter table doesn't go away, sometimes things change
after PREROUTING, like NAT, and additional rules must be used.
--
Bill Davidsen <davidsen@tmr.com>
"We have more to fear from the bungling of the incompetent than from
the machinations of the wicked." - from Slashdot
^ permalink raw reply
* [PATCH 7/12] eexpress: fix !SMP unused-var warning
From: Jeff Garzik @ 2007-10-23 22:36 UTC (permalink / raw)
To: LKML; +Cc: akpm, netdev
In-Reply-To: <tueoct232007613pm.likjasfdlkjdas987@havoc.gtf.org>
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
diff --git a/drivers/net/eexpress.c b/drivers/net/eexpress.c
index 9c85e50..70509ed 100644
--- a/drivers/net/eexpress.c
+++ b/drivers/net/eexpress.c
@@ -651,9 +651,9 @@ static void eexp_timeout(struct net_device *dev)
*/
static int eexp_xmit(struct sk_buff *buf, struct net_device *dev)
{
- struct net_local *lp = netdev_priv(dev);
short length = buf->len;
#ifdef CONFIG_SMP
+ struct net_local *lp = netdev_priv(dev);
unsigned long flags;
#endif
^ permalink raw reply related
* [PATCH 8/12] ni5010: kill unused variable
From: Jeff Garzik @ 2007-10-23 22:36 UTC (permalink / raw)
To: LKML; +Cc: akpm, netdev
In-Reply-To: <tueoct232007613pm.likjasfdlkjdas987@havoc.gtf.org>
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
diff --git a/drivers/net/ni5010.c b/drivers/net/ni5010.c
index 14a768f..a20005c 100644
--- a/drivers/net/ni5010.c
+++ b/drivers/net/ni5010.c
@@ -518,7 +518,6 @@ static void dump_packet(void *buf, int len)
/* We have a good packet, get it out of the buffer. */
static void ni5010_rx(struct net_device *dev)
{
- struct ni5010_local *lp = netdev_priv(dev);
int ioaddr = dev->base_addr;
unsigned char rcv_stat;
struct sk_buff *skb;
@@ -577,7 +576,6 @@ static void ni5010_rx(struct net_device *dev)
PRINTK2((KERN_DEBUG "%s: Received packet, size=%#4.4x\n",
dev->name, i_pkt_size));
-
}
static int process_xmt_interrupt(struct net_device *dev)
^ permalink raw reply related
* [PATCH 11/12] NET: fix subqueue bugs
From: Jeff Garzik @ 2007-10-23 22:36 UTC (permalink / raw)
To: LKML, davem; +Cc: akpm, netdev
In-Reply-To: <tueoct232007613pm.likjasfdlkjdas987@havoc.gtf.org>
net/sched/sch_prio.c: In function ‘prio_dequeue’:
net/sched/sch_prio.c:139: warning: passing argument 2 of ‘netif_subqueue_stopped’ makes pointer from integer without a cast
net/sched/sch_prio.c: In function ‘rr_dequeue’:
net/sched/sch_prio.c:169: warning: passing argument 2 of ‘netif_subqueue_stopped’ makes pointer from integer without a cast
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
---
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index abd82fc..de89409 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -136,7 +136,7 @@ prio_dequeue(struct Qdisc* sch)
* pulling an skb. This way we avoid excessive requeues
* for slower queues.
*/
- if (!netif_subqueue_stopped(sch->dev, (q->mq ? prio : 0))) {
+ if (!__netif_subqueue_stopped(sch->dev, (q->mq ? prio : 0))) {
qdisc = q->queues[prio];
skb = qdisc->dequeue(qdisc);
if (skb) {
@@ -165,7 +165,7 @@ static struct sk_buff *rr_dequeue(struct Qdisc* sch)
* for slower queues. If the queue is stopped, try the
* next queue.
*/
- if (!netif_subqueue_stopped(sch->dev,
+ if (!__netif_subqueue_stopped(sch->dev,
(q->mq ? q->curband : 0))) {
qdisc = q->queues[q->curband];
skb = qdisc->dequeue(qdisc);
^ permalink raw reply related
* Re: [PATCH 11/12] NET: fix subqueue bugs
From: David Miller @ 2007-10-23 22:38 UTC (permalink / raw)
To: jeff; +Cc: linux-kernel, akpm, netdev
In-Reply-To: <20071023223646.550241F81B7@havoc.gtf.org>
From: Jeff Garzik <jeff@garzik.org>
Date: Tue, 23 Oct 2007 18:36:46 -0400 (EDT)
> net/sched/sch_prio.c: In function ^[$,1rx^[(Bprio_dequeue^[$,1ry^[(B:
> net/sched/sch_prio.c:139: warning: passing argument 2 of ^[$,1rx^[(Bnetif_subqueue_stopped^[$,1ry^[(B makes pointer from integer without a cast
> net/sched/sch_prio.c: In function ^[$,1rx^[(Brr_dequeue^[$,1ry^[(B:
> net/sched/sch_prio.c:169: warning: passing argument 2 of ^[$,1rx^[(Bnetif_subqueue_stopped^[$,1ry^[(B makes pointer from integer without a cast
>
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
I have another copy of this from Pavel earlier in my inbox
and I promise I will integrate his patch today :-)
^ permalink raw reply
* Re: [PATCH 11/12] NET: fix subqueue bugs
From: Jeff Garzik @ 2007-10-23 22:40 UTC (permalink / raw)
To: David Miller; +Cc: linux-kernel, akpm, netdev
In-Reply-To: <20071023.153836.85407923.davem@davemloft.net>
David Miller wrote:
> From: Jeff Garzik <jeff@garzik.org>
> Date: Tue, 23 Oct 2007 18:36:46 -0400 (EDT)
>
>> net/sched/sch_prio.c: In function ^[$,1rxprio_dequeue^[$,1ry:
>> net/sched/sch_prio.c:139: warning: passing argument 2 of ^[$,1rxnetif_subqueue_stopped^[$,1ry makes pointer from integer without a cast
>> net/sched/sch_prio.c: In function ^[$,1rxrr_dequeue^[$,1ry:
>> net/sched/sch_prio.c:169: warning: passing argument 2 of ^[$,1rxnetif_subqueue_stopped^[$,1ry makes pointer from integer without a cast
>>
>> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>
>
> I have another copy of this from Pavel earlier in my inbox
> and I promise I will integrate his patch today :-)
Either way, all good :) Just dumping my fixes from last night.
Jeff
^ permalink raw reply
* Re: [PATCH] Add eeprom_bad_csum_allow module option to e1000.
From: Kok, Auke @ 2007-10-23 23:03 UTC (permalink / raw)
To: Dave Jones, Jeff Garzik, Kok, Auke, Adam Jackson, linux-kernel,
David Miller <davem
In-Reply-To: <20071023212026.GF7793@redhat.com>
Dave Jones wrote:
> On Tue, Oct 23, 2007 at 04:40:01PM -0400, Jeff Garzik wrote:
>
> > > In any case, this patch should not be merged. We often send it around to users to
> > > debug their issue in case it involves eeproms, but merging it will just conceal
> > > the real issue and all of a sudden a flood of people stop reporting *real* issues
> > > to us.
> >
> > Sorry, I disagree. Just as with e100, if there is a clear way the user
> > can recover their setup -- and Adam says his was effective -- I don't
> > see why we should be denying users the ability to use their own hardware.
>
> Indeed. This is a common enough problem that not including it causes more pain
> than its worth. I have two affected boxes myself that I actually thought
> the hardware was dead before I tried ajax's patch.
look: You should have reported this to us and you didn't. Now you are using the
fact that you did not report it as an argument which is out of place.
why do you say it is common? how often have you seen this and not reported it back
to our support? are you willingly trying to frustrate this issue?
Auke
^ permalink raw reply
* Re: [PATCH] Add eeprom_bad_csum_allow module option to e1000.
From: Kok, Auke @ 2007-10-23 23:19 UTC (permalink / raw)
To: David Miller; +Cc: davej, jeff, ajax, linux-kernel, netdev
In-Reply-To: <20071023.145339.55504234.davem@davemloft.net>
David Miller wrote:
> From: Dave Jones <davej@redhat.com>
> Date: Tue, 23 Oct 2007 17:20:26 -0400
>
>> Indeed. This is a common enough problem that not including it causes
>> more pain than its worth. I have two affected boxes myself that I
>> actually thought the hardware was dead before I tried ajax's patch.
>>
>> People aren't going to report this as a bug. They aren't going to
>> try out patches, they're going to do what I did and stick another
>> network card in the box and go on with life.
>>
>> Our users deserve better than this.
>
> Seconded. The resistence to this patch is just flat-out rediculious,
> just like it was in the e100 case.
>
> And I think all of this "e1000 is different!" talk is merely a
> scarecrow for the fact that Intel simply doesn't want this patch
> merged for some other reason.
no, e1000 eeproms contain many timing information and bits crucial to getting the
adapter working in the first place. All of these are documented in our PUBLICALLY
available SDM which is downloadable from our e1000.sf.net website. (e.g. 8254x
sdm, section 5.6, page 98+). For pci-e silicon this gets much more complex.
we haven't even heard from the user what hardware he has nor gotten an eeprom dump
from him.
I'm not hiding anything and you're deliberately creating a negative atmosphere here.
The people who do have eeprom checksum issues have come to us in the past. The
fact that you didn't see them means that they *properly* made it to us.
As a matter of fact I am still working on a permanent solution for bad eeprom
checksums on lenovo T60 laptops. Should I just drop that issue and leave the real
problem unsolved?
This patch only affirms *YOUR* point of view, not that of many customers who have
come to us and received help with many issues. You're completely ignoring that and
that is unfair.
If we want this patch in the kernel in some form that actually shows in a decent
way what a user *should* do and more importantly should *know*, then maybe we can
talk about that.
The patch in question does not add any extra information nor does it do some
sanity checking on the eeprom values or turn off any of the problematic features
that we really should disable. We even have code that allows some of the hardware
to run without a properly setup eeprom in a few hardware cases. And we definately
should print out a lot more warnings to the user that running with garbage eeprom
data is _not_ a good idea.
Auke
^ permalink raw reply
* Re: [PATCH] Add eeprom_bad_csum_allow module option to e1000.
From: Stephen Hemminger @ 2007-10-23 23:53 UTC (permalink / raw)
To: Kok, Auke
Cc: Dave Jones, Jeff Garzik, Kok, Auke, Adam Jackson, linux-kernel,
David Miller, netdev
In-Reply-To: <471E7DCA.9030700@intel.com>
On Tue, 23 Oct 2007 16:03:38 -0700
"Kok, Auke" <auke-jan.h.kok@intel.com> wrote:
> Dave Jones wrote:
> > On Tue, Oct 23, 2007 at 04:40:01PM -0400, Jeff Garzik wrote:
> >
> > > > In any case, this patch should not be merged. We often send it around to users to
> > > > debug their issue in case it involves eeproms, but merging it will just conceal
> > > > the real issue and all of a sudden a flood of people stop reporting *real* issues
> > > > to us.
> > >
> > > Sorry, I disagree. Just as with e100, if there is a clear way the user
> > > can recover their setup -- and Adam says his was effective -- I don't
> > > see why we should be denying users the ability to use their own hardware.
> >
> > Indeed. This is a common enough problem that not including it causes more pain
> > than its worth. I have two affected boxes myself that I actually thought
> > the hardware was dead before I tried ajax's patch.
>
>
> look: You should have reported this to us and you didn't. Now you are using the
> fact that you did not report it as an argument which is out of place.
>
> why do you say it is common? how often have you seen this and not reported it back
> to our support? are you willingly trying to frustrate this issue?
>
>
> Auke
What about a compromise like "ignore_checksum" module option?
That way users with bad checksums wouldn't just ignore the problem (no one reads console logs),
but would have a way to correct the checksum.
There are many reasons would want the ability to fix the problem themselves without
asking Intel.
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: [PATCH 00/13] r8169: pull request for 'upstream-jeff' branch
From: Jeff Garzik @ 2007-10-24 0:13 UTC (permalink / raw)
To: Francois Romieu
Cc: netdev, Andrew Morton, Daniel Drake, Lennert Buytenhek,
Philip Craig, Edward Hsu
In-Reply-To: <20071018204846.GC8594@electric-eye.fr.zoreil.com>
Francois Romieu wrote:
> Please pull from branch 'upstream-jeff' in repository
>
> git://git.kernel.org/pub/scm/linux/kernel/git/romieu/netdev-2.6.git upstream-jeff
pulled, thanks.
and thanks for continuing to CC RealTek.
^ permalink raw reply
* Re: Please pull bug-fixes branch of linux-2.6-mv643xx_eth.git
From: Jeff Garzik @ 2007-10-24 0:15 UTC (permalink / raw)
To: Dale Farnsworth; +Cc: netdev
In-Reply-To: <20071023211317.GA29226@xyzzy.farnsworth.org>
Dale Farnsworth wrote:
> The following changes since commit e8b8c977734193adedf2b0f607d6252c78e86394:
> Linus Torvalds (1):
> Revert "kconfig: tristate choices with mixed tristate and boolean values"
>
> are available in the git repository at:
>
> git://farnsworth.org/dale/linux-2.6-mv643xx_eth.git bug-fixes
pulled
^ permalink raw reply
* Re: Please pull features branch of linux-2.6-mv643xx_eth.git
From: Jeff Garzik @ 2007-10-24 0:16 UTC (permalink / raw)
To: Dale Farnsworth; +Cc: netdev
In-Reply-To: <20071023212016.GA29970@xyzzy.farnsworth.org>
Dale Farnsworth wrote:
> The following changes since commit e8b8c977734193adedf2b0f607d6252c78e86394:
> Linus Torvalds (1):
> Revert "kconfig: tristate choices with mixed tristate and boolean values"
>
> are available in the git repository at:
>
> git://farnsworth.org/dale/linux-2.6-mv643xx_eth.git features
pulled
^ permalink raw reply
* Re: [MIPS] MIPSnet: Delete all the useless debugging printks.
From: Jeff Garzik @ 2007-10-24 0:23 UTC (permalink / raw)
To: Ralf Baechle; +Cc: Andrew Morton, netdev
In-Reply-To: <20071022233526.GA3301@linux-mips.org>
Ralf Baechle wrote:
> Plus minor formatting fixes.
>
> Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
applied
^ permalink raw reply
* Re: [PATCH] sky2: crash on remove
From: Jeff Garzik @ 2007-10-24 0:23 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Andrew Morton, netdev, jkarlson
In-Reply-To: <20071022133909.6a5453c7@freepuppy.rosehill>
Stephen Hemminger wrote:
> Fix off-by one in remove logic that just got introduced.
>
> Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
>
> ---
> This only occurs in new post 2.6.23 code.
applied
^ permalink raw reply
* Re: [PATCH][MIPS] AR7 ethernet
From: Jeff Garzik @ 2007-10-24 0:23 UTC (permalink / raw)
To: Matteo Croce
Cc: linux-mips, Eugene Konev, netdev, davem, kuznet, pekkas, jmorris,
yoshfuji, kaber, Andrew Morton
In-Reply-To: <200710231912.23446.technoboy85@gmail.com>
Matteo Croce wrote:
> Il Monday 15 October 2007 20:24:21 Jeff Garzik ha scritto:
>> applied
>
> Small update to the driver, please apply
>
> Signed-off-by: Matteo Croce <technoboy85@gmail.com>
> Signed-off-by: Eugene Konev <ejka@imfi.kspu.ru>
> Signed-off-by: Felix Fietkau <nbd@openwrt.org>
>
applied
Please provide a useful subject line and changelog for ALL your patches,
as documented by Documentation/SubmittingPatches and
http://linux.yyz.us/patch-format.html
^ permalink raw reply
* Re: [patch 1/2] : remove header_ops bug in qeth driver
From: Jeff Garzik @ 2007-10-24 0:23 UTC (permalink / raw)
To: Ursula Braun
Cc: netdev, linux-s390, frank.blaschka, mschwid2, heicars2, apw,
kamalesh, kaber, shemminger
In-Reply-To: <20071022142044.628766000@linux.vnet.ibm.com>
applied 1-2
^ permalink raw reply
* Re: [PATCH] defxx.c: dfx_bus_init() is __devexit not __devinit
From: Jeff Garzik @ 2007-10-24 0:23 UTC (permalink / raw)
To: Maciej W. Rozycki; +Cc: Andrew Morton, netdev, linux-kernel
In-Reply-To: <Pine.LNX.4.64N.0710221809350.988@blysk.ds.pg.gda.pl>
Maciej W. Rozycki wrote:
> The dfx_bus_uninit() call is called from dfx_unregister() which is
> __devexit and which is ultimately the ->remove call for the device.
>
> Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
> ---
> It should be obvious. Please apply.
>
> Maciej
>
> patch-mips-2.6.23-rc5-20070904-defxx-devexit-0
> diff -up --recursive --new-file linux-mips-2.6.23-rc5-20070904.macro/drivers/net/defxx.c linux-mips-2.6.23-rc5-20070904/drivers/net/defxx.c
> --- linux-mips-2.6.23-rc5-20070904.macro/drivers/net/defxx.c 2007-09-04 04:55:41.000000000 +0000
> +++ linux-mips-2.6.23-rc5-20070904/drivers/net/defxx.c 2007-10-12 00:21:35.000000000 +0000
> @@ -806,7 +806,7 @@ static void __devinit dfx_bus_init(struc
> * Interrupts are disabled at the adapter bus-specific logic.
> */
>
> -static void __devinit dfx_bus_uninit(struct net_device *dev)
> +static void __devexit dfx_bus_uninit(struct net_device *dev)
> {
> DFX_board_t *bp = netdev_priv(dev);
applied
^ permalink raw reply
* Re: [PATCH] pasemi_mac: fix typo
From: Jeff Garzik @ 2007-10-24 0:23 UTC (permalink / raw)
To: Olof Johansson; +Cc: netdev, linuxppc-dev
In-Reply-To: <20071020191003.GB30034@lixom.net>
Olof Johansson wrote:
> Add missing &:
>
> drivers/net/pasemi_mac.c: In function 'pasemi_mac_clean_rx':
> drivers/net/pasemi_mac.c:553: warning: passing argument 1 of 'prefetch'
> makes pointer from integer without a cast
>
>
> Signed-off-by: Olof Johansson <olof@lixom.net>
>
> diff --git a/drivers/net/pasemi_mac.c b/drivers/net/pasemi_mac.c
> index 9f9a421..ab4d309 100644
> --- a/drivers/net/pasemi_mac.c
> +++ b/drivers/net/pasemi_mac.c
> @@ -550,7 +550,7 @@ static int pasemi_mac_clean_rx(struct pasemi_mac *mac, int limit)
>
> n = mac->rx->next_to_clean;
>
> - prefetch(RX_RING(mac, n));
> + prefetch(&RX_RING(mac, n));
>
> for (count = 0; count < limit; count++) {
> macrx = RX_RING(mac, n);
applied
^ permalink raw reply
* Re: [PATCH 1/6] Convert bonding timers to workqueues
From: Jeff Garzik @ 2007-10-24 0:42 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, andy
In-Reply-To: <1192667875793-git-send-email-fubar@us.ibm.com>
Jay Vosburgh wrote:
> Convert bonding timers to workqueues. This converts the various
> monitor functions to run in periodic work queues instead of timers. This
> patch introduces the framework and convers the calls, but does not resolve
> various locking issues, and does not stand alone.
>
> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
> Signed-off-by: Jay Vosburgh <fubar@us.ibm.com>
> ---
> drivers/net/bonding/bond_3ad.c | 6 +-
> drivers/net/bonding/bond_3ad.h | 2 +-
> drivers/net/bonding/bond_alb.c | 6 +-
> drivers/net/bonding/bond_alb.h | 2 +-
> drivers/net/bonding/bond_main.c | 147 ++++++++++++++++++++-----------------
> drivers/net/bonding/bond_sysfs.c | 63 +++++-----------
> drivers/net/bonding/bonding.h | 13 ++--
> 7 files changed, 117 insertions(+), 122 deletions(-)
applied patches 1-6
However, two issues:
1) Credit. Did Andy write these, as the Signed-off-by lines indicate?
If so, you should reflect that with a
From: Andy Gospodarek <andy@greyhouse.net>
as the first line of your email body. Linus's git tool will
automatically notice this special first-line header, and appropriately
credit the right person.
2) vague subject line. _Every_ subject line is copied verbatim into the
kernel changelog, preserved as a permanent one-line summary of the
changeset. As such, with considered in the context of the entire kernel,
Convert more locks to _bh, acquire rtnl, for new locking
tells us nothing at all about the portion of the kernel being modified.
In the future, make sure to include "bonding" somewhere in the subject
line. Generally a "bonding: " prefix would probably be best, but it can
fall elsewhere as your patch #1 subject line indicates.
I hand-edited these subject lines to add "bonding: " prefix before I
applied the patchset.
^ permalink raw reply
* [PATCH] e1000, e1000e valid-addr fixes
From: Jeff Garzik @ 2007-10-24 0:55 UTC (permalink / raw)
To: David Miller; +Cc: davej, auke-jan.h.kok, ajax, linux-kernel, netdev
In-Reply-To: <20071023.145339.55504234.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 579 bytes --]
Actually, looking over the code I see obvious bugs in the logic:
An invalid ethernet address should not cause device loading to fail,
because the user is given the opportunity to supply a MAC address via
userspace (ifconfig or whatever) before the interface goes up.
I just created the attached -bug fix- patch as illustration, though I
have not committed it, waiting for comment.
This patch will make no difference for users hitting invalid-eep-csum
rather than invalid-MAC-addr condition, but it's a problem I noticed
while reviewing Adam's patch in detail.
Jeff
[-- Attachment #2: patch.e1000-addr --]
[-- Type: text/plain, Size: 3740 bytes --]
diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
index f1ce348..8936d48 100644
--- a/drivers/net/e1000/e1000_main.c
+++ b/drivers/net/e1000/e1000_main.c
@@ -1022,11 +1022,6 @@ e1000_probe(struct pci_dev *pdev,
memcpy(netdev->dev_addr, adapter->hw.mac_addr, netdev->addr_len);
memcpy(netdev->perm_addr, adapter->hw.mac_addr, netdev->addr_len);
- if (!is_valid_ether_addr(netdev->perm_addr)) {
- DPRINTK(PROBE, ERR, "Invalid MAC Address\n");
- goto err_eeprom;
- }
-
e1000_get_bus_info(&adapter->hw);
init_timer(&adapter->tx_fifo_stall_timer);
@@ -1134,7 +1129,9 @@ e1000_probe(struct pci_dev *pdev,
"32-bit"));
}
- printk("%s\n", print_mac(mac, netdev->dev_addr));
+ printk("%s%s\n",
+ print_mac(mac, netdev->dev_addr),
+ is_valid_ether_addr(netdev->dev_addr) ? "" : " (INVALID)");
/* reset the hardware with the new settings */
e1000_reset(adapter);
@@ -1396,6 +1393,9 @@ e1000_open(struct net_device *netdev)
struct e1000_adapter *adapter = netdev_priv(netdev);
int err;
+ if (!is_valid_ether_addr(netdev->dev_addr))
+ return -EINVAL;
+
/* disallow open during test */
if (test_bit(__E1000_TESTING, &adapter->flags))
return -EBUSY;
@@ -2377,7 +2377,7 @@ e1000_set_mac(struct net_device *netdev, void *p)
struct sockaddr *addr = p;
if (!is_valid_ether_addr(addr->sa_data))
- return -EADDRNOTAVAIL;
+ return -EINVAL;
/* 82542 2.0 needs to be in reset to write receive address registers */
diff --git a/drivers/net/e1000e/netdev.c b/drivers/net/e1000e/netdev.c
index 033e124..0e3216c 100644
--- a/drivers/net/e1000e/netdev.c
+++ b/drivers/net/e1000e/netdev.c
@@ -2557,6 +2557,9 @@ static int e1000_open(struct net_device *netdev)
struct e1000_hw *hw = &adapter->hw;
int err;
+ if (!is_valid_ether_addr(netdev->dev_addr))
+ return -EINVAL;
+
/* disallow open during test */
if (test_bit(__E1000_TESTING, &adapter->state))
return -EBUSY;
@@ -2670,7 +2673,7 @@ static int e1000_set_mac(struct net_device *netdev, void *p)
struct sockaddr *addr = p;
if (!is_valid_ether_addr(addr->sa_data))
- return -EADDRNOTAVAIL;
+ return -EINVAL;
memcpy(netdev->dev_addr, addr->sa_data, netdev->addr_len);
memcpy(adapter->hw.mac.addr, addr->sa_data, netdev->addr_len);
@@ -3960,14 +3963,16 @@ static void e1000_print_device_info(struct e1000_adapter *adapter)
/* print bus type/speed/width info */
ndev_info(netdev, "(PCI Express:2.5GB/s:%s) "
- "%02x:%02x:%02x:%02x:%02x:%02x\n",
+ "%02x:%02x:%02x:%02x:%02x:%02x%s\n",
/* bus width */
((hw->bus.width == e1000_bus_width_pcie_x4) ? "Width x4" :
"Width x1"),
/* MAC address */
netdev->dev_addr[0], netdev->dev_addr[1],
netdev->dev_addr[2], netdev->dev_addr[3],
- netdev->dev_addr[4], netdev->dev_addr[5]);
+ netdev->dev_addr[4], netdev->dev_addr[5],
+ is_valid_ether_addr(netdev->dev_addr) ?
+ "" : " (INVALID)");
ndev_info(netdev, "Intel(R) PRO/%s Network Connection\n",
(hw->phy.type == e1000_phy_ife)
? "10/100" : "1000");
@@ -4170,16 +4175,6 @@ static int __devinit e1000_probe(struct pci_dev *pdev,
memcpy(netdev->dev_addr, adapter->hw.mac.addr, netdev->addr_len);
memcpy(netdev->perm_addr, adapter->hw.mac.addr, netdev->addr_len);
- if (!is_valid_ether_addr(netdev->perm_addr)) {
- ndev_err(netdev, "Invalid MAC Address: "
- "%02x:%02x:%02x:%02x:%02x:%02x\n",
- netdev->perm_addr[0], netdev->perm_addr[1],
- netdev->perm_addr[2], netdev->perm_addr[3],
- netdev->perm_addr[4], netdev->perm_addr[5]);
- err = -EIO;
- goto err_eeprom;
- }
-
init_timer(&adapter->watchdog_timer);
adapter->watchdog_timer.function = &e1000_watchdog;
adapter->watchdog_timer.data = (unsigned long) adapter;
^ permalink raw reply related
* Re: [PATCH 1/6] Convert bonding timers to workqueues
From: Jay Vosburgh @ 2007-10-24 1:00 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, andy
In-Reply-To: <471E94F3.5050407@garzik.org>
Jeff Garzik <jeff@garzik.org> wrote:
>applied patches 1-6
Thanks.
>However, two issues:
>
>1) Credit. Did Andy write these, as the Signed-off-by lines indicate?
Andy did some of it and I did some of it, so I presume that dual
Signed-off-by is correct (vs. one Signed-off and one Acked).
>2) vague subject line. [...]
Sorry about that; will keep it in mind next time.
-J
---
-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
^ permalink raw reply
* Re: [PATCH] e1000, e1000e valid-addr fixes
From: Jeff Garzik @ 2007-10-24 1:03 UTC (permalink / raw)
To: netdev; +Cc: David Miller, davej, auke-jan.h.kok, ajax, linux-kernel
In-Reply-To: <471E9801.2000006@garzik.org>
Jeff Garzik wrote:
> Actually, looking over the code I see obvious bugs in the logic:
>
> An invalid ethernet address should not cause device loading to fail,
> because the user is given the opportunity to supply a MAC address via
> userspace (ifconfig or whatever) before the interface goes up.
>
> I just created the attached -bug fix- patch as illustration, though I
> have not committed it, waiting for comment.
>
> This patch will make no difference for users hitting invalid-eep-csum
> rather than invalid-MAC-addr condition, but it's a problem I noticed
> while reviewing Adam's patch in detail.
Adding my own comment :)
Does the ethernet stack check is_valid_ether_addr() before permitting
interface-up?
I'm wondering if there is a way to avoid adding
if (!is_valid_ether_addr(dev->dev_addr))
return -EINVAL;
to every ethernet driver's ->open() hook.
Jeff
^ 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