Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Russell Stuart @ 2006-03-14  0:31 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, hadi, lartc
In-Reply-To: <20060313135048.27b09fba@localhost.localdomain>

On Mon, 2006-03-13 at 13:50 -0800, Stephen Hemminger wrote:
> On Tue, 14 Mar 2006 07:43:57 +1000
> Russell Stuart <russell-lartc@stuart.id.au> wrote:
> 
> > On Mon, 2006-03-13 at 10:04 -0800, Stephen Hemminger wrote:
> > > The memset fix is in current CVS. I just wasn't going to take the
> > > patch that looked at utsname to decide what hash to use.
> > 
> > Stephen, could you describe your objections to it please?
> > 
> 
> Because it means that the API for netlink isn't being used properly.

Do you mean the binary API between the kernel and the 
applications has been broken; this is bad as it 
transgresses some unwritten law; and the patch is
bad because it hides this problem rather than addressing
it?

Does the fact that the binary API changed during a
major kernel release (between 2.4 and 2.6) give us some 
wriggle room here?

Anyway, jokes aside, the situation we have now is the 
current "tc" doesn't work with the current kernel.  This 
situation can't be allowed to continue, I presume.  Ergo, 
here is a list of things we could do to fix this.  All 
you (plural) have to do is choose one, or some 
combination:

1.  Change the hashing algorithm back to the 2.4 way.
    (IMHO, the 2.4 algorithm was better.)  Disadvantage:
    anybody who had a u32 filter that hashed on more
    than one non-zero byte will have their u32 filters
    suddenly break.  However, since how the hashing
    algorithm was never documented beyond "HOWTO's"
    that showed how to hash on one byte, and every
    example of hashing I have seen has been a copy &
    paste of said HOWTO's, my guess is there are stuff
    all people who will be effected.  Another disadvantage:
    people who are using older 2.6 kernels (eg me on
    my production machines) won't have the problem fixed.

2.  Change the hashing algorithm in "tc" to match the 
    current kernel.  Disadvantage: "tc" will no longer 
    work with 2.4 kernels.

3.  Change the hashing algorithm in "tc" to match the
    current kernel, and change the 2.4 kernel's hashing
    algorithm to match the 2.6 kernel.  This is Jamal's
    proposed solution.  Disadvantage: 2.4 is a "stable" 
    kernel, produced when we made a real effort to 
    distinguish between between stable and development 
    kernels.  This change would break API compatibility 
    in a stable series.  Yuk!

4.  Make "tc" look at the kernel version, and choose
    the appropriate hashing function.  This was my
    solution, and everybody seems to hate it bar me
    :(.  Disadvantages: None, other than it hides a
    violation of an "unwritten law".

5.  A combination of 1 & 4.  Change the hashing in 2.6 
    algorithm back to what it was in 2.4, and hide the 
    change by making "tc" check the kernel version and 
    choose the matching hash.    Disadvantages: None, 
    other than now we have wilfully broken the unwritten
    law twice.

My personal preference is to have a "tc" in CVS that
works with _all_ kernel versions.  Yes - the netlink
interface has been broken.  But is was done, and now 
can't be undone.  No matter why we do, there will
forever more be kernel versions with incompatible
netlink interfaces.  We can't fix it.  We just have
to limit the damage.

^ permalink raw reply

* RE: Router stops routing after changing MAC Address
From: linux-os (Dick Johnson) @ 2006-03-13 22:35 UTC (permalink / raw)
  To: Greg Scott
  Cc: Rick Jones, Chuck Ebbert, linux-kernel, netdev, Bart Samwel,
	Alan Cox, Simon Mackinlay
In-Reply-To: <925A849792280C4E80C5461017A4B8A20321F9@mail733.InfraSupportEtc.com>


On Mon, 13 Mar 2006, Greg Scott wrote:

> Yup.
>
> I had a situation 2 weeks ago where a customer connected a system to the
> Internet with an IP Address he should not have used.  And the little
> Cisco router on the frontend dutifully recorded it in its ARP cache -
> forever, with no TTL!  This took down their webmail for most of a day
> until we finally had to cycle the power on that nasty little Cisco 678.
>
> Bigger routers do it too.  I've had several situations over the years
> where I replaced an older firewall with a newer one with the same IP
> Addresses.  All the internal servers find it soon enough.  But I've
> waited literally hours for the routers to finally purge their ARP caches
> so they would see my replacement systems - often with the customer
> looking over my shoulders getting more and more nervous by the minute.
>
> And sometimes the routers are not accessible - you can't cycle them even
> if you had permission.  Consider the cases of bridged DSL service -

Bzzzzst... Not! There are not any MAC addresses associated with any
of the intercity links, usually not even in WANs!  MAC is for
Ethernet! Once you go to fiber, ATM, T-N, etc., there are no
MAC addresses. That's why there are bridges and routers, you
got to "connect" your tiny time-slot to your LAN and that
first device contains the MAC address that all your other stuff
talks to.

> where the real router could be on the other side of the country.  Try
> calling an ISP and asking the tech on the other end to purge an ARP
> cache on a router.  So the same IP Addresses but different MAC
> addresses, all you can do is wait for the passage of (lots of) time.
> That happened to me in my own network once.  I accidently took down my
> email server for something like 4 hours one time when I got careless.
>
>> Indeed, there is a large onus on the software doing the MAC
>> override to make sure it does not break the required uniqueness.
>> Just as if one were using locally administered MAC addresses.
>
> Yes.  My 12:34:56 OUI scheme will work for this project but it is
> definitely not good for the long term.  I really really hope I have to
> spend some money with the IEEE soon to support lots and lots of
> rollouts.  :)
>
> - Greg Scott
>
>
>
> -----Original Message-----
> From: Rick Jones [mailto:rick.jones2@hp.com]
> Sent: Monday, March 13, 2006 3:50 PM
> To: linux-os (Dick Johnson)
> Cc: Greg Scott; Chuck Ebbert; linux-kernel; netdev@vger.kernel.org; Bart
> Samwel; Alan Cox; Simon Mackinlay
> Subject: Re: Router stops routing after changing MAC Address
>
> > Anyway, if the device fails, you have
>> routers and hosts ARPing the interface, trying to establish a route
>> anyway.
>
> But only after what may be a much longer time than the customer is
> willing to accept or able to configure.  I know of a number of HA
> situations where the "new" device is given the "old" MAC just to avoid
> that speicific situation of ARP caches not being updated except after
> quite some time.  Not necessarily on the end-systems, the issue can be
> with intermediate devices (routers).
>
> And if one has to work with static ARP entries to deal (however
> imperfectly) with ARP poisioning or whatnot...
>
> Indeed, there is a large onus on the software doing the MAC override to
> make sure it does not break the required uniqueness.  Just as if one
> were using locally administered MAC addresses.
>
> rick jones
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
>

Cheers,
Dick Johnson
Penguin : Linux version 2.6.15.4 on an i686 machine (5589.54 BogoMips).
Warning : 98.36% of all statistics are fiction, book release in April.
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

^ permalink raw reply

* [2.6 patch] hostap_{pci,plx}.c: fix memory leaks
From: Adrian Bunk @ 2006-03-13 22:28 UTC (permalink / raw)
  To: jkmaline; +Cc: hostap, netdev, linux-kernel

This patch fixes two memotry leaks spotted by the Coverity checker.


Signed-off-by: Adrian Bunk <bunk@stusta.de>

---

 drivers/net/wireless/hostap/hostap_pci.c |    6 +++---
 drivers/net/wireless/hostap/hostap_plx.c |    6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

--- linux-2.6.16-rc6-mm1-full/drivers/net/wireless/hostap/hostap_pci.c.old	2006-03-13 22:34:30.000000000 +0100
+++ linux-2.6.16-rc6-mm1-full/drivers/net/wireless/hostap/hostap_pci.c	2006-03-13 22:37:57.000000000 +0100
@@ -301,14 +301,14 @@ static int prism2_pci_probe(struct pci_d
 	struct hostap_interface *iface;
 	struct hostap_pci_priv *hw_priv;
 
+	if (pci_enable_device(pdev))
+		return -EIO;
+
 	hw_priv = kmalloc(sizeof(*hw_priv), GFP_KERNEL);
 	if (hw_priv == NULL)
 		return -ENOMEM;
 	memset(hw_priv, 0, sizeof(*hw_priv));
 
-	if (pci_enable_device(pdev))
-		return -EIO;
-
 	phymem = pci_resource_start(pdev, 0);
 
 	if (!request_mem_region(phymem, pci_resource_len(pdev, 0), "Prism2")) {
--- linux-2.6.16-rc6-mm1-full/drivers/net/wireless/hostap/hostap_plx.c.old	2006-03-13 22:39:40.000000000 +0100
+++ linux-2.6.16-rc6-mm1-full/drivers/net/wireless/hostap/hostap_plx.c	2006-03-13 22:40:09.000000000 +0100
@@ -446,14 +446,14 @@ static int prism2_plx_probe(struct pci_d
 	int tmd7160;
 	struct hostap_plx_priv *hw_priv;
 
+	if (pci_enable_device(pdev))
+		return -EIO;
+
 	hw_priv = kmalloc(sizeof(*hw_priv), GFP_KERNEL);
 	if (hw_priv == NULL)
 		return -ENOMEM;
 	memset(hw_priv, 0, sizeof(*hw_priv));
 
-	if (pci_enable_device(pdev))
-		return -EIO;
-
 	/* National Datacomm NCP130 based on TMD7160, not PLX9052. */
 	tmd7160 = (pdev->vendor == 0x15e8) && (pdev->device == 0x0131);
 

^ permalink raw reply

* RE: Router stops routing after changing MAC Address
From: Greg Scott @ 2006-03-13 22:15 UTC (permalink / raw)
  To: Rick Jones, linux-os (Dick Johnson)
  Cc: Chuck Ebbert, linux-kernel, netdev, Bart Samwel, Alan Cox,
	Simon Mackinlay

Yup.

I had a situation 2 weeks ago where a customer connected a system to the
Internet with an IP Address he should not have used.  And the little
Cisco router on the frontend dutifully recorded it in its ARP cache -
forever, with no TTL!  This took down their webmail for most of a day
until we finally had to cycle the power on that nasty little Cisco 678.

Bigger routers do it too.  I've had several situations over the years
where I replaced an older firewall with a newer one with the same IP
Addresses.  All the internal servers find it soon enough.  But I've
waited literally hours for the routers to finally purge their ARP caches
so they would see my replacement systems - often with the customer
looking over my shoulders getting more and more nervous by the minute.  

And sometimes the routers are not accessible - you can't cycle them even
if you had permission.  Consider the cases of bridged DSL service -
where the real router could be on the other side of the country.  Try
calling an ISP and asking the tech on the other end to purge an ARP
cache on a router.  So the same IP Addresses but different MAC
addresses, all you can do is wait for the passage of (lots of) time.
That happened to me in my own network once.  I accidently took down my
email server for something like 4 hours one time when I got careless.  

> Indeed, there is a large onus on the software doing the MAC 
> override to make sure it does not break the required uniqueness.  
> Just as if one were using locally administered MAC addresses.

Yes.  My 12:34:56 OUI scheme will work for this project but it is
definitely not good for the long term.  I really really hope I have to
spend some money with the IEEE soon to support lots and lots of
rollouts.  :)

- Greg Scott



-----Original Message-----
From: Rick Jones [mailto:rick.jones2@hp.com] 
Sent: Monday, March 13, 2006 3:50 PM
To: linux-os (Dick Johnson)
Cc: Greg Scott; Chuck Ebbert; linux-kernel; netdev@vger.kernel.org; Bart
Samwel; Alan Cox; Simon Mackinlay
Subject: Re: Router stops routing after changing MAC Address

 > Anyway, if the device fails, you have
> routers and hosts ARPing the interface, trying to establish a route 
> anyway.

But only after what may be a much longer time than the customer is
willing to accept or able to configure.  I know of a number of HA
situations where the "new" device is given the "old" MAC just to avoid
that speicific situation of ARP caches not being updated except after
quite some time.  Not necessarily on the end-systems, the issue can be
with intermediate devices (routers).

And if one has to work with static ARP entries to deal (however
imperfectly) with ARP poisioning or whatnot...

Indeed, there is a large onus on the software doing the MAC override to
make sure it does not break the required uniqueness.  Just as if one
were using locally administered MAC addresses.

rick jones

^ permalink raw reply

* Re: [PATCH] scm: fold __scm_send() into scm_send()
From: Andrew Morton @ 2006-03-13 22:13 UTC (permalink / raw)
  To: Benjamin LaHaise; +Cc: ioe-lkml, davem, linux-kernel, netdev
In-Reply-To: <20060313202242.GC27761@kvack.org>

Benjamin LaHaise <bcrl@kvack.org> wrote:
>
> On Mon, Mar 13, 2006 at 09:05:31PM +0100, Ingo Oeser wrote:
> > From: Ingo Oeser <ioe-lkml@rameria.de>
> > 
> > Fold __scm_send() into scm_send() and remove that interface completly
> > from the kernel.
> 
> Whoa, what are you doing here?
>

scm_send() and scm_recv() are already uninlined in Dave's tree - this patch
does further consolidation.

>  Uninlining scm_send() is a Bad Thing to do 
> given that scm_send() is in the AF_UNIX hot path.

scm_send() and scm_recv() are in _two_ AF_UNIX hotpaths...

^ permalink raw reply

* RE: [PATCH 3/8] [I/OAT] Setup the networking subsystem as a DMA client
From: Leech, Christopher @ 2006-03-13 22:12 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel, netdev

> >  +#ifdef CONFIG_NET_DMA
> >  +#include <linux/dmaengine.h>
> >  +#endif
> 
> There are still a number of instances of this in the patch 
> series.  Did you
> decide to keep the ifdefs in there for some reason?

No, I just missed them in header files.  Thanks. 

^ permalink raw reply

* Re: Router stops routing after changing MAC Address
From: Randy.Dunlap @ 2006-03-13 22:10 UTC (permalink / raw)
  To: linux-os (Dick Johnson)
  Cc: shemminger, GregScott, 76306.1226, linux-kernel, davem, netdev,
	bart, alan, smackinlay
In-Reply-To: <Pine.LNX.4.61.0603131513380.5373@chaos.analogic.com>

On Mon, 13 Mar 2006 15:27:26 -0500 linux-os \(Dick Johnson\) wrote:

> 
> On Mon, 13 Mar 2006, Stephen Hemminger wrote:
> 
> > There still is a bug in the 3c59x driver.  It doesn't include any code
> > to handle changing the mac address.  It will work if you take the device
> > down, change address, then bring it up. But you shouldn't have to do that.
> >
> > Also, if the driver handles setting mac address, it could have prevented
> > you from using a multicast address.
> >
> > Something like this is needed (untested, I don't have that hardware).
> >
[cut patch]

> Actually, it doesn't make any difference. Changing the IEEE station
> (physical) address is not an allowed procedure even though hooks are
> available in many drivers to do this. According to the IEEE 802
> physical media specification, this 48-bit address must be unique and
> must be one of a group assigned by IEEE. Failure to follow this
> simple protocol can (will) cause an entire network to fail. If
> you don't care, then you certainly don't care about multicast
> bits either, basically let them set it to all ones as well.

They used to allow "Locally Administered Addresses."  Hrm,
google still finds 18,000 hits for that phrase.  Is that now
outlawed?

Even ieee.org has hit(s) for it:
http://standards.ieee.org/regauth/groupmac/tutorial.html

http://en.wikipedia.org/wiki/MAC_address
http://www.mynetwatchman.com/pckidiot/chap04.htm

---
~Randy
You can't do anything without having to do something else first.
-- Belefant's Law

^ permalink raw reply

* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Stephen Hemminger @ 2006-03-13 21:50 UTC (permalink / raw)
  To: Russell Stuart; +Cc: netdev, hadi, lartc
In-Reply-To: <1142286237.17608.7.camel@ras.pc.brisbane.lube>

On Tue, 14 Mar 2006 07:43:57 +1000
Russell Stuart <russell-lartc@stuart.id.au> wrote:

> On Mon, 2006-03-13 at 10:04 -0800, Stephen Hemminger wrote:
> > The memset fix is in current CVS. I just wasn't going to take the
> > patch that looked at utsname to decide what hash to use.
> 
> Stephen, could you describe your objections to it please?
> 

Because it means that the API for netlink isn't being used properly.

^ permalink raw reply

* Re: Router stops routing after changing MAC Address
From: Rick Jones @ 2006-03-13 21:50 UTC (permalink / raw)
  To: linux-os (Dick Johnson)
  Cc: Greg Scott, Chuck Ebbert, linux-kernel, netdev, Bart Samwel,
	Alan Cox, Simon Mackinlay
In-Reply-To: <Pine.LNX.4.61.0603131636470.5608@chaos.analogic.com>

 > Anyway, if the device fails, you have
> routers and hosts ARPing the interface, trying to establish a
> route anyway.

But only after what may be a much longer time than the customer is 
willing to accept or able to configure.  I know of a number of HA 
situations where the "new" device is given the "old" MAC just to avoid 
that speicific situation of ARP caches not being updated except after 
quite some time.  Not necessarily on the end-systems, the issue can be 
with intermediate devices (routers).

And if one has to work with static ARP entries to deal (however 
imperfectly) with ARP poisioning or whatnot...

Indeed, there is a large onus on the software doing the MAC override to 
make sure it does not break the required uniqueness.  Just as if one 
were using locally administered MAC addresses.

rick jones

^ permalink raw reply

* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Russell Stuart @ 2006-03-13 21:43 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, hadi, lartc
In-Reply-To: <20060313100421.7df7f9ed@localhost.localdomain>

On Mon, 2006-03-13 at 10:04 -0800, Stephen Hemminger wrote:
> The memset fix is in current CVS. I just wasn't going to take the
> patch that looked at utsname to decide what hash to use.

Stephen, could you describe your objections to it please?

^ permalink raw reply

* RE: Router stops routing after changing MAC Address
From: linux-os (Dick Johnson) @ 2006-03-13 21:39 UTC (permalink / raw)
  To: Greg Scott
  Cc: Stephen Hemminger, Chuck Ebbert, linux-kernel, David S. Miller,
	netdev, Bart Samwel, Alan Cox, Simon Mackinlay
In-Reply-To: <925A849792280C4E80C5461017A4B8A20321F5@mail733.InfraSupportEtc.com>


On Mon, 13 Mar 2006, Greg Scott wrote:

> But in a failover scenario you want two devices to have the same IEEE
> (station) Address (or MAC Address or hardware address).  So many names
> for the same thing!
>
> When the primary unit fails, you want the backup unit to completely
> assume the failed unit's identity - right down to the MAC Address.  The
> other way to do it using gratuitous ARPs is not good enough because some
> cheap router someplace with an ARP cache of several hours will not
> listen and will never update its own ARP cache.
>
> I like to think of this as bending the rules a little bit, not really
> breaking them.  :)
>
> - Greg
>

Top posting, NotGood(tm). Anyway, if the device fails, you have
routers and hosts ARPing the interface, trying to establish a
route anyway.

>
>
>> Actually, it doesn't make any difference. Changing the IEEE station
>> (physical) address is not an allowed procedure even though hooks are
>> available in many drivers to do this. According to the IEEE 802
>> physical media specification, this 48-bit address must be unique
>> and must be one of a group assigned by IEEE. Failure to follow this
>> simple protocol can (will) cause an entire network to fail. If you
>> don't care, then you certainly don't care about multicast bits either,
>> basically let them set it to all ones as well.
>
>> Cheers,
>> Dick Johnson
>> Penguin : Linux version 2.6.15.4 on an i686 machine (5589.54 BogoMips).
>> Warning : 98.36% of all statistics are fiction, book release in April.
>

Cheers,
Dick Johnson
Penguin : Linux version 2.6.15.4 on an i686 machine (5589.54 BogoMips).
Warning : 98.36% of all statistics are fiction, book release in April.
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

^ permalink raw reply

* [-mm patch] make drivers/net/tg3.c:tg3_request_irq()
From: Adrian Bunk @ 2006-03-13 21:26 UTC (permalink / raw)
  To: Andrew Morton, davem; +Cc: linux-kernel, netdev, jgarzik
In-Reply-To: <20060312031036.3a382581.akpm@osdl.org>

On Sun, Mar 12, 2006 at 03:10:36AM -0800, Andrew Morton wrote:
>...
> Changes since 2.6.16-rc5-mm3:
>...
>  git-net.patch
>...
>  git trees
>...


This patch makes the needlessly global function tg3_request_irq() 
static.


Signed-off-by: Adrian Bunk <bunk@stusta.de>

--- linux-2.6.16-rc6-mm1-full/drivers/net/tg3.c.old	2006-03-13 21:13:31.000000000 +0100
+++ linux-2.6.16-rc6-mm1-full/drivers/net/tg3.c	2006-03-13 21:14:26.000000000 +0100
@@ -6531,7 +6531,7 @@
 	add_timer(&tp->timer);
 }
 
-int tg3_request_irq(struct tg3 *tp)
+static int tg3_request_irq(struct tg3 *tp)
 {
 	irqreturn_t (*fn)(int, void *, struct pt_regs *);
 	unsigned long flags;

^ permalink raw reply

* RE: Router stops routing after changing MAC Address
From: Greg Scott @ 2006-03-13 20:57 UTC (permalink / raw)
  To: linux-os (Dick Johnson), Stephen Hemminger
  Cc: Chuck Ebbert, linux-kernel, David S. Miller, netdev, Bart Samwel,
	Alan Cox, Simon Mackinlay

But in a failover scenario you want two devices to have the same IEEE
(station) Address (or MAC Address or hardware address).  So many names
for the same thing!  

When the primary unit fails, you want the backup unit to completely
assume the failed unit's identity - right down to the MAC Address.  The
other way to do it using gratuitous ARPs is not good enough because some
cheap router someplace with an ARP cache of several hours will not
listen and will never update its own ARP cache.  

I like to think of this as bending the rules a little bit, not really
breaking them.  :)

- Greg



>Actually, it doesn't make any difference. Changing the IEEE station
>(physical) address is not an allowed procedure even though hooks are 
>available in many drivers to do this. According to the IEEE 802 
>physical media specification, this 48-bit address must be unique 
>and must be one of a group assigned by IEEE. Failure to follow this 
>simple protocol can (will) cause an entire network to fail. If you 
>don't care, then you certainly don't care about multicast bits either, 
>basically let them set it to all ones as well.

>Cheers,
>Dick Johnson
>Penguin : Linux version 2.6.15.4 on an i686 machine (5589.54 BogoMips).
>Warning : 98.36% of all statistics are fiction, book release in April.

^ permalink raw reply

* Re: Router stops routing after changing MAC Address
From: linux-os (Dick Johnson) @ 2006-03-13 20:27 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Greg Scott, Chuck Ebbert, linux-kernel, David S. Miller, netdev,
	Bart Samwel, Alan Cox, Simon Mackinlay
In-Reply-To: <20060313100041.212cee08@localhost.localdomain>


On Mon, 13 Mar 2006, Stephen Hemminger wrote:

> There still is a bug in the 3c59x driver.  It doesn't include any code
> to handle changing the mac address.  It will work if you take the device
> down, change address, then bring it up. But you shouldn't have to do that.
>
> Also, if the driver handles setting mac address, it could have prevented
> you from using a multicast address.
>
> Something like this is needed (untested, I don't have that hardware).
>
>
> --- linux-2.6/drivers/net/3c59x.c.orig	2006-03-13 09:58:25.000000000 -0800
> +++ linux-2.6/drivers/net/3c59x.c	2006-03-13 09:52:47.000000000 -0800
> @@ -895,6 +895,7 @@ static void dump_tx_ring(struct net_devi
> static void update_stats(void __iomem *ioaddr, struct net_device *dev);
> static struct net_device_stats *vortex_get_stats(struct net_device *dev);
> static void set_rx_mode(struct net_device *dev);
> +static int set_rx_address(struct net_device *dev, void *addr);
> #ifdef CONFIG_PCI
> static int vortex_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
> #endif
> @@ -1563,6 +1564,7 @@ static int __devinit vortex_probe1(struc
> #endif
> 	dev->ethtool_ops = &vortex_ethtool_ops;
> 	dev->set_multicast_list = set_rx_mode;
> +	dev->set_mac_address = set_rx_address;
> 	dev->tx_timeout = vortex_tx_timeout;
> 	dev->watchdog_timeo = (watchdog * HZ) / 1000;
> #ifdef CONFIG_NET_POLL_CONTROLLER
> @@ -3150,6 +3152,27 @@ static void set_rx_mode(struct net_devic
> 	iowrite16(new_mode, ioaddr + EL3_CMD);
> }
>
> +
> +static int set_rx_address(struct net_device *dev, void *p)
> +{
> +	struct vortex_private *vp = netdev_priv(dev);
> +	void __iomem *ioaddr = vp->ioaddr;
> +	const struct sockaddr *addr = p;
> +
> +	if (!is_valid_ether_addr(addr->sa_data))
> +		return -EADDRNOTAVAIL;
> +
> +	spin_lock_bh(&vp->lock);
> +	memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
> +
> +	EL3WINDOW(2);
> +	for (i = 0; i < ETH_ALEN; i++)
> +		iowrite8(dev->dev_addr[i], ioaddr + i);
> +	spin_unlock_bh(&vp->lock);
> +
> +	return 0;
> +}
> +
> #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
> /* Setup the card so that it can receive frames with an 802.1q VLAN tag.
>    Note that this must be done after each RxReset due to some backwards
> -

Actually, it doesn't make any difference. Changing the IEEE station
(physical) address is not an allowed procedure even though hooks are
available in many drivers to do this. According to the IEEE 802
physical media specification, this 48-bit address must be unique and
must be one of a group assigned by IEEE. Failure to follow this
simple protocol can (will) cause an entire network to fail. If
you don't care, then you certainly don't care about multicast
bits either, basically let them set it to all ones as well.

Cheers,
Dick Johnson
Penguin : Linux version 2.6.15.4 on an i686 machine (5589.54 BogoMips).
Warning : 98.36% of all statistics are fiction, book release in April.
_
\x1a\x04

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

^ permalink raw reply

* Re: [PATCH] scm: fold __scm_send() into scm_send()
From: Benjamin LaHaise @ 2006-03-13 20:22 UTC (permalink / raw)
  To: Ingo Oeser; +Cc: David S. Miller, linux-kernel, akpm, netdev
In-Reply-To: <200603132105.32794.ioe-lkml@rameria.de>

On Mon, Mar 13, 2006 at 09:05:31PM +0100, Ingo Oeser wrote:
> From: Ingo Oeser <ioe-lkml@rameria.de>
> 
> Fold __scm_send() into scm_send() and remove that interface completly
> from the kernel.

Whoa, what are you doing here?  Uninlining scm_send() is a Bad Thing to do 
given that scm_send() is in the AF_UNIX hot path.

		-ben
-- 
"Time is of no importance, Mr. President, only life is important."
Don't Email: <dont@kvack.org>.

^ permalink raw reply

* [PATCH] scm: fold __scm_send() into scm_send()
From: Ingo Oeser @ 2006-03-13 20:05 UTC (permalink / raw)
  To: David S. Miller; +Cc: linux-kernel, akpm, netdev
In-Reply-To: <20060312.180802.13404061.davem@davemloft.net>

From: Ingo Oeser <ioe-lkml@rameria.de>

Fold __scm_send() into scm_send() and remove that interface completly
from the kernel.

Signed-off-by: Ingo Oeser <ioe-klml@rameria.de>
---
Inspired by the patch to inline scm_send()
I did the next logical step :-)

Regards

Ingo Oeser

diff --git a/include/net/scm.h b/include/net/scm.h
index eb44e5a..ec8b891 100644
--- a/include/net/scm.h
+++ b/include/net/scm.h
@@ -26,11 +26,9 @@ struct scm_cookie
 
 extern void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm);
 extern void scm_detach_fds_compat(struct msghdr *msg, struct scm_cookie *scm);
-extern int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm);
 extern void __scm_destroy(struct scm_cookie *scm);
 extern struct scm_fp_list * scm_fp_dup(struct scm_fp_list *fpl);
-extern int scm_send(struct socket *sock, struct msghdr *msg,
-			struct scm_cookie *scm);
+extern int scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm);
 extern void scm_recv(struct socket *sock, struct msghdr *msg,
 		struct scm_cookie *scm, int flags);
 
diff --git a/net/core/scm.c b/net/core/scm.c
index b6dee90..6adbe60 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -110,10 +110,21 @@ void __scm_destroy(struct scm_cookie *sc
 	}
 }
 
-int __scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *p)
+int scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm)
 {
 	struct cmsghdr *cmsg;
 	int err;
+	struct task_struct *tsk = current;
+	scm->creds = (struct ucred) {
+		.uid = tsk->uid,
+		.gid = tsk->gid,
+		.pid = tsk->tgid
+	};
+	scm->fp = NULL;
+	scm->sid = security_sk_sid(sock->sk, NULL, 0);
+	scm->seq = 0;
+	if (msg->msg_controllen <= 0)
+		return 0;
 
 	for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg))
 	{
@@ -136,15 +147,15 @@ int __scm_send(struct socket *sock, stru
 		switch (cmsg->cmsg_type)
 		{
 		case SCM_RIGHTS:
-			err=scm_fp_copy(cmsg, &p->fp);
+			err=scm_fp_copy(cmsg, &scm->fp);
 			if (err<0)
 				goto error;
 			break;
 		case SCM_CREDENTIALS:
 			if (cmsg->cmsg_len != CMSG_LEN(sizeof(struct ucred)))
 				goto error;
-			memcpy(&p->creds, CMSG_DATA(cmsg), sizeof(struct ucred));
-			err = scm_check_creds(&p->creds);
+			memcpy(&scm->creds, CMSG_DATA(cmsg), sizeof(struct ucred));
+			err = scm_check_creds(&scm->creds);
 			if (err)
 				goto error;
 			break;
@@ -153,15 +164,15 @@ int __scm_send(struct socket *sock, stru
 		}
 	}
 
-	if (p->fp && !p->fp->count)
+	if (scm->fp && !scm->fp->count)
 	{
-		kfree(p->fp);
-		p->fp = NULL;
+		kfree(scm->fp);
+		scm->fp = NULL;
 	}
 	return 0;
 	
 error:
-	scm_destroy(p);
+	scm_destroy(scm);
 	return err;
 }
 
@@ -284,22 +295,6 @@ struct scm_fp_list *scm_fp_dup(struct sc
 	return new_fpl;
 }
 
-int scm_send(struct socket *sock, struct msghdr *msg, struct scm_cookie *scm)
-{
-	struct task_struct *p = current;
-	scm->creds = (struct ucred) {
-		.uid = p->uid,
-		.gid = p->gid,
-		.pid = p->tgid
-	};
-	scm->fp = NULL;
-	scm->sid = security_sk_sid(sock->sk, NULL, 0);
-	scm->seq = 0;
-	if (msg->msg_controllen <= 0)
-		return 0;
-	return __scm_send(sock, msg, scm);
-}
-
 void scm_recv(struct socket *sock, struct msghdr *msg,
 		struct scm_cookie *scm, int flags)
 {
@@ -332,7 +326,6 @@ void scm_recv(struct socket *sock, struc
 }
 
 EXPORT_SYMBOL(__scm_destroy);
-EXPORT_SYMBOL(__scm_send);
 EXPORT_SYMBOL(scm_send);
 EXPORT_SYMBOL(scm_recv);
 EXPORT_SYMBOL(put_cmsg);

^ permalink raw reply related

* Re: drivers/net/chelsio/sge.c: two array overflows
From: Scott Bardone @ 2006-03-13 19:31 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: maintainers, jgarzik, netdev, linux-kernel
In-Reply-To: <20060311013720.GG21864@stusta.de>

[-- Attachment #1: Type: text/plain, Size: 731 bytes --]

Adrian,

This is a bug. The array should contain 2 elements.

Attached is a patch which fixes it.
Thanks.

Signed-off-by: Scott Bardone <sbardone@chelsio.com>


Adrian Bunk wrote:
> The Coverity checker spotted the following two array overflows in 
> drivers/net/chelsio/sge.c (in both cases, the arrays contain 3 elements):
> 
> <--  snip  -->
> 
> ...
> static void restart_tx_queues(struct sge *sge)
> {
> ...
>                                 sge->stats.cmdQ_restarted[3]++;
> ...
> static int t1_sge_tx(struct sk_buff *skb, struct adapter *adapter,
>                      unsigned int qid, struct net_device *dev)
> {
> ...
>                         sge->stats.cmdQ_full[3]++;
> ...
> 
> <--  snip  -->
> 
> 
> cu
> Adrian
> 

[-- Attachment #2: sge.patch --]
[-- Type: text/x-patch, Size: 907 bytes --]

--- sge.c	2006-02-17 14:23:45.000000000 -0800
+++ sge.fix.c	2006-03-13 10:51:24.000000000 -0800
@@ -1021,7 +1021,7 @@
 			if (test_and_clear_bit(nd->if_port,
 					       &sge->stopped_tx_queues) &&
 			    netif_running(nd)) {
-				sge->stats.cmdQ_restarted[3]++;
+				sge->stats.cmdQ_restarted[2]++;
 				netif_wake_queue(nd);
 			}
 		}
@@ -1350,7 +1350,7 @@
 	 	if (unlikely(credits < count)) {
 			netif_stop_queue(dev);
 			set_bit(dev->if_port, &sge->stopped_tx_queues);
-			sge->stats.cmdQ_full[3]++;
+			sge->stats.cmdQ_full[2]++;
 			spin_unlock(&q->lock);
 			if (!netif_queue_stopped(dev))
 				CH_ERR("%s: Tx ring full while queue awake!\n",
@@ -1358,7 +1358,7 @@
 			return NETDEV_TX_BUSY;
 		}
 		if (unlikely(credits - count < q->stop_thres)) {
-			sge->stats.cmdQ_full[3]++;
+			sge->stats.cmdQ_full[2]++;
 			netif_stop_queue(dev);
 			set_bit(dev->if_port, &sge->stopped_tx_queues);
 		}

^ permalink raw reply

* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Patrick McHardy @ 2006-03-13 19:02 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, hadi, lartc
In-Reply-To: <20060313100421.7df7f9ed@localhost.localdomain>

Stephen Hemminger wrote:
>>BTW, running valgrind on tc shows lots of uses of uninitialized values,
>>it seems like a good idea if someone would go over these and fix them
>>up.
> 
> 
> If we had a test script of commands (code coverage), that would help.

Actually the Coverity scanner is quite good at spotting these problems,
so I've requested iproute to be added to the list of regulary scanned
projects.

^ permalink raw reply

* Re: [patch 1/4] natsemi: Add support for using MII port with no PHY
From: thockin @ 2006-03-13 18:41 UTC (permalink / raw)
  To: Jeff Garzik, netdev, linux-kernel
In-Reply-To: <20060313182331.GA19014@sirena.org.uk>

On Mon, Mar 13, 2006 at 06:23:31PM +0000, Mark Brown wrote:
> On Sun, Mar 12, 2006 at 01:41:13PM -0800, thockin@hockin.org wrote:
> 
> > Not that my opinion should hold much weight, having been absent from the
> > driver for some time, but yuck.  Is there no better way to do this thatn
> > sprinkling poo all over it?
> 
> The changes are mostly isolated into check_link(), the fact that half
> the function gets placed inside a conditional but diff sees it as a
> bunch of smaller changes makes the changes look a lot more invasive than
> they actually are.  I guess that could be helped by splitting the PHY
> access code out of check_link() into check_phy_status() or something but
> I'm not sure how much that really helps.

It's not terribly offensive it just seems like a hack. :)  I'm not sure I
really understand the reasoning, so I can't offer anythign better or more
general purpose.

^ permalink raw reply

* Re: [patch 1/4] natsemi: Add support for using MII port with no PHY
From: Mark Brown @ 2006-03-13 18:23 UTC (permalink / raw)
  To: thockin; +Cc: Jeff Garzik, netdev, linux-kernel
In-Reply-To: <20060312214113.GA15071@hockin.org>

On Sun, Mar 12, 2006 at 01:41:13PM -0800, thockin@hockin.org wrote:

> Not that my opinion should hold much weight, having been absent from the
> driver for some time, but yuck.  Is there no better way to do this thatn
> sprinkling poo all over it?

The changes are mostly isolated into check_link(), the fact that half
the function gets placed inside a conditional but diff sees it as a
bunch of smaller changes makes the changes look a lot more invasive than
they actually are.  I guess that could be helped by splitting the PHY
access code out of check_link() into check_phy_status() or something but
I'm not sure how much that really helps.

-- 
"You grabbed my hand and we fell into it, like a daydream - or a fever."

^ permalink raw reply

* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Patrick McHardy @ 2006-03-13 18:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, hadi, lartc
In-Reply-To: <20060313100421.7df7f9ed@localhost.localdomain>

Stephen Hemminger wrote:
> The memset fix is in current CVS. I just wasn't going to take the
> patch that looked at utsname to decide what hash to use.

Yes, that sucks. We have another incompatibility, old iproute versions
(like the one shipped by Debian) show garbage statistics for HFSC. I
think it still works properly, but I wasn't able to track down the
cause. I have a feeling its somehow related to the gen_stats changes.

>>BTW, running valgrind on tc shows lots of uses of uninitialized values,
>>it seems like a good idea if someone would go over these and fix them
>>up.
> 
> 
> If we had a test script of commands (code coverage), that would help.

Getting good coverage looks like a lot of work, but I'll put it on
my TODO list for a boring day :)

^ permalink raw reply

* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Stephen Hemminger @ 2006-03-13 18:04 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netdev, hadi, lartc
In-Reply-To: <4415B217.30507@trash.net>

On Mon, 13 Mar 2006 18:55:35 +0100
Patrick McHardy <kaber@trash.net> wrote:

> jamal wrote:
> > On Mon, 2006-13-03 at 14:44 +1000, Russell Stuart wrote: 
> > 
> >>You are wrong on both counts.
> > 
> > 
> > I am wrong on why it is being rejected - but what you are seeing is
> > worse than i thought initially. 
> > 
> > Lets put it this way:
> > The only you will _ever_ get that message is if you had made a syntax
> > error (which you did not). Please look at the code on where that message
> > appears and:
> > 
> > a) tell me how you would have got that message to begin with using
> > perfectly legal syntax.
> > a) tell me how a memset would have fixed that.
> 
> He already told you, pack_key expects the selector to be initialized,
> otherwise nkeys might contain a value >= 128, which would cause
> exactly this error, if a matching key is not found within the
> uninitialized memory by accident.
> 
> > Just send the memset fix to Stephen with a different reason. Your
> > current reason is _wrong_ and i really dont have much time to have this
> > kind of discussion.  
> > If you had said "I added that memset there because it looks like the
> > right thing to do" then we would not have had this discussion.
> > 
> > You made claims you fixed a bug. It cant possibly be the bug you fixed.
> > Was it some other bug perhaps and you mixed up the two?

The memset fix is in current CVS. I just wasn't going to take the
patch that looked at utsname to decide what hash to use.

> The patch as well as the description are perfectly fine.
> 
> BTW, running valgrind on tc shows lots of uses of uninitialized values,
> it seems like a good idea if someone would go over these and fix them
> up.

If we had a test script of commands (code coverage), that would help.

^ permalink raw reply

* Re: Router stops routing after changing MAC Address
From: Stephen Hemminger @ 2006-03-13 18:00 UTC (permalink / raw)
  To: Greg Scott
  Cc: Chuck Ebbert, linux-kernel, David S. Miller, netdev, Bart Samwel,
	Alan Cox, Simon Mackinlay
In-Reply-To: <925A849792280C4E80C5461017A4B8A20321F2@mail733.InfraSupportEtc.com>

There still is a bug in the 3c59x driver.  It doesn't include any code
to handle changing the mac address.  It will work if you take the device
down, change address, then bring it up. But you shouldn't have to do that.

Also, if the driver handles setting mac address, it could have prevented
you from using a multicast address.

Something like this is needed (untested, I don't have that hardware).


--- linux-2.6/drivers/net/3c59x.c.orig	2006-03-13 09:58:25.000000000 -0800
+++ linux-2.6/drivers/net/3c59x.c	2006-03-13 09:52:47.000000000 -0800
@@ -895,6 +895,7 @@ static void dump_tx_ring(struct net_devi
 static void update_stats(void __iomem *ioaddr, struct net_device *dev);
 static struct net_device_stats *vortex_get_stats(struct net_device *dev);
 static void set_rx_mode(struct net_device *dev);
+static int set_rx_address(struct net_device *dev, void *addr);
 #ifdef CONFIG_PCI
 static int vortex_ioctl(struct net_device *dev, struct ifreq *rq, int cmd);
 #endif
@@ -1563,6 +1564,7 @@ static int __devinit vortex_probe1(struc
 #endif
 	dev->ethtool_ops = &vortex_ethtool_ops;
 	dev->set_multicast_list = set_rx_mode;
+	dev->set_mac_address = set_rx_address;
 	dev->tx_timeout = vortex_tx_timeout;
 	dev->watchdog_timeo = (watchdog * HZ) / 1000;
 #ifdef CONFIG_NET_POLL_CONTROLLER
@@ -3150,6 +3152,27 @@ static void set_rx_mode(struct net_devic
 	iowrite16(new_mode, ioaddr + EL3_CMD);
 }
 
+
+static int set_rx_address(struct net_device *dev, void *p)
+{
+	struct vortex_private *vp = netdev_priv(dev);
+	void __iomem *ioaddr = vp->ioaddr;
+	const struct sockaddr *addr = p;
+
+	if (!is_valid_ether_addr(addr->sa_data))
+		return -EADDRNOTAVAIL;
+
+	spin_lock_bh(&vp->lock);
+	memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
+
+	EL3WINDOW(2);
+	for (i = 0; i < ETH_ALEN; i++)
+		iowrite8(dev->dev_addr[i], ioaddr + i);
+	spin_unlock_bh(&vp->lock);
+	
+	return 0;
+}
+
 #if defined(CONFIG_VLAN_8021Q) || defined(CONFIG_VLAN_8021Q_MODULE)
 /* Setup the card so that it can receive frames with an 802.1q VLAN tag.
    Note that this must be done after each RxReset due to some backwards

^ permalink raw reply

* Re: [PATCH] TC: bug fixes to the "sample" clause
From: Patrick McHardy @ 2006-03-13 17:55 UTC (permalink / raw)
  To: hadi; +Cc: netdev, lartc
In-Reply-To: <1142269090.5242.29.camel@jzny2>

jamal wrote:
> On Mon, 2006-13-03 at 14:44 +1000, Russell Stuart wrote: 
> 
>>You are wrong on both counts.
> 
> 
> I am wrong on why it is being rejected - but what you are seeing is
> worse than i thought initially. 
> 
> Lets put it this way:
> The only you will _ever_ get that message is if you had made a syntax
> error (which you did not). Please look at the code on where that message
> appears and:
> 
> a) tell me how you would have got that message to begin with using
> perfectly legal syntax.
> a) tell me how a memset would have fixed that.

He already told you, pack_key expects the selector to be initialized,
otherwise nkeys might contain a value >= 128, which would cause
exactly this error, if a matching key is not found within the
uninitialized memory by accident.

> Just send the memset fix to Stephen with a different reason. Your
> current reason is _wrong_ and i really dont have much time to have this
> kind of discussion.  
> If you had said "I added that memset there because it looks like the
> right thing to do" then we would not have had this discussion.
> 
> You made claims you fixed a bug. It cant possibly be the bug you fixed.
> Was it some other bug perhaps and you mixed up the two?

The patch as well as the description are perfectly fine.

BTW, running valgrind on tc shows lots of uses of uninitialized values,
it seems like a good idea if someone would go over these and fix them
up.

^ permalink raw reply

* Re: [PATCH 5/6 v2] IB: IP address based RDMA connection manager
From: Roland Dreier @ 2006-03-13 17:26 UTC (permalink / raw)
  To: Sean Hefty; +Cc: netdev, linux-kernel, openib-general
In-Reply-To: <ORSMSX401FRaqbC8wSA0000001e@orsmsx401.amr.corp.intel.com>

    Sean> It's dropping the reference on cma_dev, as opposed to
    Sean> id_priv.

Duh, sorry.

 - R.

^ permalink raw reply


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