* Re: [PATCH] net: Broadcast ARP packets on link local addresses
From: jamal @ 2006-04-07 13:37 UTC (permalink / raw)
To: Anand Kumria; +Cc: netdev, linux-kernel
In-Reply-To: <pan.2006.04.07.07.20.44.797909@progsoc.org>
On Fri, 2006-07-04 at 17:20 +1000, Anand Kumria wrote:
> On Fri, 31 Mar 2006 15:26:00 -0800, David Daney wrote:
>
> > From: David Daney
> >
> > Greetings,
> >
> > When an internet host joins a network where there is no DHCP server,
> > it may auto-allocate an IP address by the method described in RFC
> > 3927. There are several user space daemons available that implement
> > most of the protocol (zcip, busybox, ...). The kernel's APR driver
> > should function in the normal manner except that it is required to
> > broadcast all ARP packets that it originates in the link local address
> > space (169.254.0.0/16). RFC 3927 section 2.5 explains the requirement.
> >
> > The current ARP code is non-compliant because it does not broadcast
> > some ARP packets as required by RFC 3927.
>
> I haven't seem anyone comment on this,
Theres a lot of comments - check the archives of netdev on the thread
as well as a newer thread under "Broadcast ARP packets on link local
addresses (Version2)"
> but it would be useful to see this
> integrated.
IMO not the way it is defined right now in that patch.
As suggested in the thread, best way is for the kernel to check
if it is link local and do the advert in broadcast instead of unicast.
>
> Something else I've noticed while I've been implementing my zeroconf
> daemon is that the kernel returns link-scoped primary addresses first to
> 'ifconfig'. Unfortunately quite a lot of user-space programs parse its
> output and interpret the address it presents as the primary for the
> specified interface.
>
Not sure i followed.
> Is that a case of user-space breakage that the kernel team would
> ordinarily worry about?
I think user space setting the attribute of the address to be link local
would be a sufficient hint to the kernel to broadcast the arps.
cheers,
jamal
^ permalink raw reply
* Re: [patch] ipv4: initialize arp_tbl rw lock
From: Stephen Hemminger @ 2006-04-07 14:46 UTC (permalink / raw)
To: Heiko Carstens
Cc: Jeff Garzik, Andrew Morton, netdev, linux-kernel, Frank Pavlic
In-Reply-To: <20060407081533.GC11353@osiris.boeblingen.de.ibm.com>
On Fri, 7 Apr 2006 10:15:33 +0200
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>
> The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK
> detects that this lock is not initialized as it is supposed to be.
>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
This is a initialization order problem then, because:
arp_init
neigh_table_init
rwlock_init
does the initialization already. So fix the initialization sequence
of the qeth driver or you will have other problems.
My impression was the -rt folks wanted all lock initializations t be
done at runtime not compile time.
^ permalink raw reply
* Re: Broadcast ARP packets on link local addresses (Version2).
From: David Daney @ 2006-04-07 16:18 UTC (permalink / raw)
To: hadi; +Cc: Janos Farkas, netdev, linux-kernel, pgf, freek
In-Reply-To: <1144416638.5082.33.camel@jzny2>
jamal wrote:
> On Thu, 2006-06-04 at 09:17 -0700, David Daney wrote:
>
>>Janos Farkas wrote:
>
>
>>>Sorry for chiming in this late in the discussion, but... Shouldn't it
>>>be more correct to not depend on the ip address of the used network,
>>>but to use the "scope" parameter of the given address?
>>>
>>
>
> Excellent point! It was bothering me as well but i couldnt express my
> view eloquently as you did.
>
>
>>RFC 3927 specifies the Ethernet arp broadcast behavior for only
>>169.254.0.0/16.
>
>
> Thats besides the point. You could, for example, use 1.1.1.1/24 in your
> network instead of the 10.x or 192.x; and i have seen people use 10.x
> in what appears to be public networks. We dont have speacial checks for
> RFC 1918 IP addresses for example.
>
Following your logic through, It seems that you are advocating
broadcasting *all* ARP packets on *all* link local addresses. That
would mean that on a 192.168.* switched Ethernet network with DHCP that
twice as many ARP packets would be broadcast.
I don't think that is a good idea. On networks with DHCP or statically
allocated addresses, it would be a hard sell to tell people that they
have to broadcast *all* ARP traffic even though there are neither
standards that require nor suggest such action.
The scope parameter, as far as I can tell, is used to make routing
decisions. Overloading it to also implement the RFC 3927 ARP
broadcasting requirement would result in generation of unnecessary
network traffic in configurations that are probably the majority of
Linux deployments.
> 169.254.0.0/16 is by definition link local. I think point made by Janos
> is we should look at the attributes rather than value.
>
The converse is not true. And that is my problem with this idea.
> Have your user space set it to be link local and then fix the kernel if
> it doesnt do the right thing.
>
I could see adding an additional interface attribute that specifies link
local, and then testing that. But that adds substantially more overhead
as you would have to allocate space for the attribute somewhere, and
have extra code to allow setting/querying of the attribute from user space.
The requirement of RFC 3927 is very tightly targeted. My patch does
only what is required, no more. It does not change ARP behavior for
commonly deployed configurations.
>
>> Presumably you could set the scope parameter to local
>>for addresses outside of that range or even for protocols other than
>>Ethernet. Since broadcasting ARP packets usually adversely effects
>>usable network bandwidth, we should probably only do it where it is
>>absolutely required. The overhead of testing the value required by the
>>RFC is quite low (3 machine instructions on i686 is the size of the
>>entire patch), so using some proxy like the scope parameter would not
>>even be a performance win.
>>
>
>
> Again, that is beside the point.
It may be beside your point, I happen to think that it has some relevance.
David Daney
^ permalink raw reply
* Re: [patch] ipv4: initialize arp_tbl rw lock
From: David S. Miller @ 2006-04-07 20:14 UTC (permalink / raw)
To: heiko.carstens; +Cc: jgarzik, akpm, netdev, linux-kernel, fpavlic
In-Reply-To: <20060407081533.GC11353@osiris.boeblingen.de.ibm.com>
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Fri, 7 Apr 2006 10:15:33 +0200
> From: Heiko Carstens <heiko.carstens@de.ibm.com>
>
> The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK
> detects that this lock is not initialized as it is supposed to be.
>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
As Stephen Hemminger pointed out this change is bogus, the
lock is initialized at run time by the ARP layer.
^ permalink raw reply
* Re: Broadcast ARP packets on link local addresses (Version2).
From: Mark Butler @ 2006-04-07 20:38 UTC (permalink / raw)
To: David Daney; +Cc: hadi, Janos Farkas, netdev, linux-kernel, pgf, freek
In-Reply-To: <443690C9.5090500@avtrex.com>
David Daney wrote:
> Following your logic through, It seems that you are advocating
> broadcasting *all* ARP packets on *all* link local addresses. That
> would mean that on a 192.168.* switched Ethernet network with DHCP
> that twice as many ARP packets would be broadcast.
192.168.* addresses are not considered "link local", they are rather
"private" or "site local" addresses.
> The scope parameter, as far as I can tell, is used to make routing
> decisions. Overloading it to also implement the RFC 3927 ARP
> broadcasting requirement would result in generation of unnecessary
> network traffic in configurations that are probably the majority of
> Linux deployments.
No extra network traffic, but there is some measurable overhead to
looking up the scope in the routing table.
One problem is having this type of scheme behave properly by default,
i.e. in the absence of user specified entries. Having to create an
entry for every interface in the system just to get RFC standard
behavior is silly.
- Mark
^ permalink raw reply
* Re: [PATCH 16/21] orinoco_pci: disable device and free IRQ when suspending
From: Francois Romieu @ 2006-04-07 21:24 UTC (permalink / raw)
To: Pavel Roskin
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20060407081051.16107.87289.stgit-fdEtzkpK75rby3iVrkZq2A@public.gmane.org>
Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org> :
[...]
> diff --git a/drivers/net/wireless/orinoco_pci.c b/drivers/net/wireless/orinoco_pci.c
> index 5362c21..e57e92b 100644
> --- a/drivers/net/wireless/orinoco_pci.c
> +++ b/drivers/net/wireless/orinoco_pci.c
> @@ -304,7 +304,9 @@ static int orinoco_pci_suspend(struct pc
>
> orinoco_unlock(priv, &flags);
>
> + free_irq(pdev->irq, dev);
> pci_save_state(pdev);
> + pci_disable_device(pdev);
> pci_set_power_state(pdev, PCI_D3hot);
>
> return 0;
/me stares at the thread behind http://lkml.org/lkml/2005/7/30/143
Imho {free/request}_irq during suspend/resume deserves some
explanation.
--
Ueimor
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH 17/21] orinoco_pci: use pci_iomap() for resources
From: Francois Romieu @ 2006-04-07 21:36 UTC (permalink / raw)
To: Pavel Roskin
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20060407081053.16107.19347.stgit-fdEtzkpK75rby3iVrkZq2A@public.gmane.org>
Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org> :
[...]
> diff --git a/drivers/net/wireless/orinoco_pci.c b/drivers/net/wireless/orinoco_pci.c
> index e57e92b..75df90f 100644
> --- a/drivers/net/wireless/orinoco_pci.c
> +++ b/drivers/net/wireless/orinoco_pci.c
> @@ -170,9 +170,7 @@ static int orinoco_pci_init_one(struct p
> const struct pci_device_id *ent)
> {
> int err = 0;
> - unsigned long pci_iorange;
> - u16 __iomem *pci_ioaddr = NULL;
> - unsigned long pci_iolen;
> + void __iomem *pci_ioaddr = NULL;
> struct orinoco_private *priv = NULL;
> struct orinoco_pci_card *card;
> struct net_device *dev = NULL;
(ok, the useless initializers disappear later in the serie)
[...]
> @@ -208,8 +205,8 @@ static int orinoco_pci_init_one(struct p
> priv = netdev_priv(dev);
> card = priv->card;
> card->pci_ioaddr = pci_ioaddr;
> - dev->mem_start = pci_iorange;
> - dev->mem_end = pci_iorange + pci_iolen - 1;
> + dev->mem_start = pci_resource_start(pdev, 0);
> + dev->mem_end = dev->mem_start + pci_resource_len(pdev, 0) - 1;
Is there a reason why dev->mem_{start/end} should not be removed ?
--
Ueimor
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH 19/21] orinoco: reduce differences between PCI drivers, create orinoco_pci.h
From: Francois Romieu @ 2006-04-07 22:10 UTC (permalink / raw)
To: Pavel Roskin
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20060407081057.16107.82106.stgit-fdEtzkpK75rby3iVrkZq2A@public.gmane.org>
Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org> :
> index 0000000..b05a9a5
> --- /dev/null
> +++ b/drivers/net/wireless/orinoco_pci.h
[...]
> +static int orinoco_pci_resume(struct pci_dev *pdev)
> +{
> + struct net_device *dev = pci_get_drvdata(pdev);
> + struct orinoco_private *priv = netdev_priv(dev);
> + unsigned long flags;
> + int err;
> +
> + pci_set_power_state(pdev, 0);
> + pci_enable_device(pdev);
> + pci_restore_state(pdev);
> +
> + err = request_irq(pdev->irq, orinoco_interrupt, SA_SHIRQ,
> + dev->name, dev);
> + if (err) {
> + printk(KERN_ERR "%s: cannot re-allocate IRQ on resume\n",
> + dev->name);
> + pci_disable_device(pdev);
> + return -EBUSY;
> + }
> +
> + err = orinoco_reinit_firmware(dev);
> + if (err) {
> + printk(KERN_ERR "%s: error %d re-initializing firmware "
> + "on resume\n", dev->name, err);
> + return err;
> + }
> +
> + spin_lock_irqsave(&priv->lock, flags);
Interruptions are enabled. No need to save/restore.
> +
> + netif_device_attach(dev);
> +
> + priv->hw_unavailable--;
> +
> + if (priv->open && (! priv->hw_unavailable)) {
> + err = __orinoco_up(dev);
I wonder if it would be enough to issue hermes_set_irqmask() later
in __orinoco_up() to release this irq disabled section.
--
Ueimor
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH 16/21] orinoco_pci: disable device and free IRQ when suspending
From: Pavel Roskin @ 2006-04-07 22:12 UTC (permalink / raw)
To: Francois Romieu
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20060407212429.GA15720-lmTtMILVy1jWQcoT9B9Ug5SCg42XY1Uw0E9HWUfgJXw@public.gmane.org>
Hello!
On Fri, 2006-04-07 at 23:24 +0200, Francois Romieu wrote:
> Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org> :
> [...]
> > diff --git a/drivers/net/wireless/orinoco_pci.c b/drivers/net/wireless/orinoco_pci.c
> > index 5362c21..e57e92b 100644
> > --- a/drivers/net/wireless/orinoco_pci.c
> > +++ b/drivers/net/wireless/orinoco_pci.c
> > @@ -304,7 +304,9 @@ static int orinoco_pci_suspend(struct pc
> >
> > orinoco_unlock(priv, &flags);
> >
> > + free_irq(pdev->irq, dev);
> > pci_save_state(pdev);
> > + pci_disable_device(pdev);
> > pci_set_power_state(pdev, PCI_D3hot);
> >
> > return 0;
>
> /me stares at the thread behind http://lkml.org/lkml/2005/7/30/143
>
> Imho {free/request}_irq during suspend/resume deserves some
> explanation.
I followed examples from other drivers. The thread in question deals
with the patch where pci_disable_device() precedes free_irq(). Besides,
bridges may need special care because they pass interrupts from other
devices.
I also followed the kernel documentation (Documentation/power/pci.txt),
which says that the driver should free the IRQ on suspend.
If you can suggest an alternative approach, please do so. I don't see
what I can do differently.
--
Regards,
Pavel Roskin
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH 17/21] orinoco_pci: use pci_iomap() for resources
From: Pavel Roskin @ 2006-04-07 22:21 UTC (permalink / raw)
To: Francois Romieu
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20060407213619.GB15720-lmTtMILVy1jWQcoT9B9Ug5SCg42XY1Uw0E9HWUfgJXw@public.gmane.org>
On Fri, 2006-04-07 at 23:36 +0200, Francois Romieu wrote:
> > @@ -208,8 +205,8 @@ static int orinoco_pci_init_one(struct p
> > priv = netdev_priv(dev);
> > card = priv->card;
> > card->pci_ioaddr = pci_ioaddr;
> > - dev->mem_start = pci_iorange;
> > - dev->mem_end = pci_iorange + pci_iolen - 1;
> > + dev->mem_start = pci_resource_start(pdev, 0);
> > + dev->mem_end = dev->mem_start + pci_resource_len(pdev, 0) - 1;
>
> Is there a reason why dev->mem_{start/end} should not be removed ?
Is there a reason why it should? Is it going to be obsolete?
--
Regards,
Pavel Roskin
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH 19/21] orinoco: reduce differences between PCI drivers, create orinoco_pci.h
From: Pavel Roskin @ 2006-04-07 22:43 UTC (permalink / raw)
To: Francois Romieu
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20060407221041.GC15720-lmTtMILVy1jWQcoT9B9Ug5SCg42XY1Uw0E9HWUfgJXw@public.gmane.org>
On Sat, 2006-04-08 at 00:10 +0200, Francois Romieu wrote:
> > + spin_lock_irqsave(&priv->lock, flags);
>
> Interruptions are enabled. No need to save/restore.
Agreed. I have found a few other places where it's also true. I'll fix
it. Actually, I'm going to change the locking to accommodate some USB
devices, so changes will be more widespread.
> > + netif_device_attach(dev);
> > +
> > + priv->hw_unavailable--;
> > +
> > + if (priv->open && (! priv->hw_unavailable)) {
> > + err = __orinoco_up(dev);
>
> I wonder if it would be enough to issue hermes_set_irqmask() later
> in __orinoco_up() to release this irq disabled section.
Maybe, but I prefer not to touch this code, since it's about to undergo
a much more radical rework.
--
Regards,
Pavel Roskin
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH 16/21] orinoco_pci: disable device and free IRQ when suspending
From: Francois Romieu @ 2006-04-07 23:08 UTC (permalink / raw)
To: Pavel Roskin
Cc: torvalds-3NddpPZAyC0, netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <1144447965.5618.8.camel@dv>
Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org> :
> On Fri, 2006-04-07 at 23:24 +0200, Francois Romieu wrote:
> > Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org> :
> > [...]
> > > diff --git a/drivers/net/wireless/orinoco_pci.c b/drivers/net/wireless/orinoco_pci.c
> > > index 5362c21..e57e92b 100644
> > > --- a/drivers/net/wireless/orinoco_pci.c
> > > +++ b/drivers/net/wireless/orinoco_pci.c
> > > @@ -304,7 +304,9 @@ static int orinoco_pci_suspend(struct pc
> > >
> > > orinoco_unlock(priv, &flags);
> > >
> > > + free_irq(pdev->irq, dev);
> > > pci_save_state(pdev);
> > > + pci_disable_device(pdev);
> > > pci_set_power_state(pdev, PCI_D3hot);
> > >
> > > return 0;
> >
> > /me stares at the thread behind http://lkml.org/lkml/2005/7/30/143
> >
> > Imho {free/request}_irq during suspend/resume deserves some
> > explanation.
>
> I followed examples from other drivers.
Yep, that's what I do too.
tg3/sky2/skge do not free_irq() in the suspend path. They disable the
device.
> The thread in question deals with the patch where pci_disable_device()
> precedes free_irq(). Besides, bridges may need special care because
> they pass interrupts from other devices.
> I also followed the kernel documentation (Documentation/power/pci.txt),
> which says that the driver should free the IRQ on suspend.
>
> If you can suggest an alternative approach, please do so. I don't see
> what I can do differently.
Disable the device and avoid free_irq/request_irq altogether ?
The documentation does not require more:
[...]
A driver uses this function to actually transition the device into a low power
state. This should include disabling I/O, IRQs, and bus-mastering, as well as
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
physically transitioning the device to a lower power state; it may also include
calls to pci_enable_wake().
-> free_irq() looks like the heavyweight option.
request_irq() can fail. The reference implementation does not care
(who does ?). Imho it hints that the driver writer should not take
the documentation _too_ literally when it suggests that "disabling
irq == free_irq".
--
Ueimor
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH] PCI Error Recovery: e100 network device driver
From: Linas Vepstas @ 2006-04-07 23:11 UTC (permalink / raw)
To: Greg KH
Cc: netdev, linux-kernel, jesse.brandeburg, linuxppc-dev,
john.ronciak, jeffrey.t.kirsher, linux-pci, Jeff Garzik
In-Reply-To: <20060406224643.GA6278@kroah.com>
On Thu, Apr 06, 2006 at 03:46:43PM -0700, Greg KH wrote:
> On Thu, Apr 06, 2006 at 05:24:00PM -0500, Linas Vepstas wrote:
> > + if(pci_enable_device(pdev)) {
>
> Add a space after "if" and before "(" please.
I guess I'm immune to learning from experience. :-/
Here's a new improved patch.
--linas
[PATCH] PCI Error Recovery: e100 network device driver
Various PCI bus errors can be signaled by newer PCI controllers. This
patch adds the PCI error recovery callbacks to the intel ethernet e100
device driver. The patch has been tested, and appears to work well.
Signed-off-by: Linas Vepstas <linas@linas.org>
Acked-by: Jesse Brandeburg <jesse.brandeburg@intel.com>
----
drivers/net/e100.c | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 75 insertions(+)
Index: linux-2.6.17-rc1/drivers/net/e100.c
===================================================================
--- linux-2.6.17-rc1.orig/drivers/net/e100.c 2006-04-07 16:21:46.000000000 -0500
+++ linux-2.6.17-rc1/drivers/net/e100.c 2006-04-07 18:10:52.411266545 -0500
@@ -2780,6 +2780,80 @@ static void e100_shutdown(struct pci_dev
DPRINTK(PROBE,ERR, "Error enabling wake\n");
}
+/* ------------------ PCI Error Recovery infrastructure -------------- */
+/**
+ * e100_io_error_detected - called when PCI error is detected.
+ * @pdev: Pointer to PCI device
+ * @state: The current pci conneection state
+ */
+static pci_ers_result_t e100_io_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
+{
+ struct net_device *netdev = pci_get_drvdata(pdev);
+
+ /* Similar to calling e100_down(), but avoids adpater I/O. */
+ netdev->stop(netdev);
+
+ /* Detach; put netif into state similar to hotplug unplug. */
+ netif_poll_enable(netdev);
+ netif_device_detach(netdev);
+
+ /* Request a slot reset. */
+ return PCI_ERS_RESULT_NEED_RESET;
+}
+
+/**
+ * e100_io_slot_reset - called after the pci bus has been reset.
+ * @pdev: Pointer to PCI device
+ *
+ * Restart the card from scratch.
+ */
+static pci_ers_result_t e100_io_slot_reset(struct pci_dev *pdev)
+{
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct nic *nic = netdev_priv(netdev);
+
+ if (pci_enable_device(pdev)) {
+ printk(KERN_ERR "e100: Cannot re-enable PCI device after reset.\n");
+ return PCI_ERS_RESULT_DISCONNECT;
+ }
+ pci_set_master(pdev);
+
+ /* Only one device per card can do a reset */
+ if (0 != PCI_FUNC(pdev->devfn))
+ return PCI_ERS_RESULT_RECOVERED;
+ e100_hw_reset(nic);
+ e100_phy_init(nic);
+
+ return PCI_ERS_RESULT_RECOVERED;
+}
+
+/**
+ * e100_io_resume - resume normal operations
+ * @pdev: Pointer to PCI device
+ *
+ * Resume normal operations after an error recovery
+ * sequence has been completed.
+ */
+static void e100_io_resume(struct pci_dev *pdev)
+{
+ struct net_device *netdev = pci_get_drvdata(pdev);
+ struct nic *nic = netdev_priv(netdev);
+
+ /* ack any pending wake events, disable PME */
+ pci_enable_wake(pdev, 0, 0);
+
+ netif_device_attach(netdev);
+ if (netif_running(netdev)) {
+ e100_open(netdev);
+ mod_timer(&nic->watchdog, jiffies);
+ }
+}
+
+static struct pci_error_handlers e100_err_handler = {
+ .error_detected = e100_io_error_detected,
+ .slot_reset = e100_io_slot_reset,
+ .resume = e100_io_resume,
+};
static struct pci_driver e100_driver = {
.name = DRV_NAME,
@@ -2791,6 +2865,7 @@ static struct pci_driver e100_driver = {
.resume = e100_resume,
#endif
.shutdown = e100_shutdown,
+ .err_handler = &e100_err_handler,
};
static int __init e100_init_module(void)
^ permalink raw reply
* Re: [PATCH 17/21] orinoco_pci: use pci_iomap() for resources
From: Francois Romieu @ 2006-04-07 23:38 UTC (permalink / raw)
To: Pavel Roskin
Cc: jgarzik-e+AXbWqSrlAAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <1144448517.5618.14.camel@dv>
Pavel Roskin <proski-mXXj517/zsQ@public.gmane.org> :
> On Fri, 2006-04-07 at 23:36 +0200, Francois Romieu wrote:
>
> > > @@ -208,8 +205,8 @@ static int orinoco_pci_init_one(struct p
> > > priv = netdev_priv(dev);
> > > card = priv->card;
> > > card->pci_ioaddr = pci_ioaddr;
> > > - dev->mem_start = pci_iorange;
> > > - dev->mem_end = pci_iorange + pci_iolen - 1;
> > > + dev->mem_start = pci_resource_start(pdev, 0);
> > > + dev->mem_end = dev->mem_start + pci_resource_len(pdev, 0) - 1;
> >
> > Is there a reason why dev->mem_{start/end} should not be removed ?
>
> Is there a reason why it should? Is it going to be obsolete?
It is slowly obsoleting for a few years (don't laugh...). It is preferred
to store the relevant address in the private part of the (pci) device.
Moderately recent drivers do not use it at all. However it's fairly common
in the setup code of the (legacy) isa devices.
--
Ueimor
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH] Unaligned accesses in the ethernet bridge
From: Stephen Hemminger @ 2006-04-07 23:57 UTC (permalink / raw)
To: Adrian Bunk; +Cc: Peter Chubb, linux-kernel, netdev
In-Reply-To: <20060406203708.GA7118@stusta.de>
On Thu, 6 Apr 2006 22:37:08 +0200
Adrian Bunk <bunk@stusta.de> wrote:
> On Thu, Mar 23, 2006 at 01:06:02PM +1100, Peter Chubb wrote:
> >
> > I see lots of
> > kernel unaligned access to 0xa0000001009dbb6f, ip=0xa000000100811591
> > kernel unaligned access to 0xa0000001009dbb6b, ip=0xa0000001008115c1
> > kernel unaligned access to 0xa0000001009dbb6d, ip=0xa0000001008115f1
> > messages in my logs on IA64 when using the ethernet bridge with 2.6.16.
> >
> >
> > Appended is a patch to fix them.
>
>
> I see this patch already made it into 2.6.17-rc1.
>
> It seems to be a candidate for 2.6.16.3, too?
> If yes, please submit it to stable@kernel.org.
The code that caused this was new in 2.6.17
^ permalink raw reply
* Re: [PATCH] PCI Error Recovery: e100 network device driver
From: Alexey Dobriyan @ 2006-04-08 0:03 UTC (permalink / raw)
To: Linas Vepstas
Cc: Greg KH, Jeff Garzik, netdev, linux-pci, linux-kernel,
linuxppc-dev, john.ronciak, jesse.brandeburg, jeffrey.t.kirsher
In-Reply-To: <20060407231134.GN25225@austin.ibm.com>
On Fri, Apr 07, 2006 at 06:11:34PM -0500, Linas Vepstas wrote:
> --- linux-2.6.17-rc1.orig/drivers/net/e100.c
> +++ linux-2.6.17-rc1/drivers/net/e100.c
> + * @state: The current pci conneection state
connection
> +static pci_ers_result_t e100_io_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
> +{
> + struct net_device *netdev = pci_get_drvdata(pdev);
> +
> + /* Similar to calling e100_down(), but avoids adpater I/O. */
adapter
> +static pci_ers_result_t e100_io_slot_reset(struct pci_dev *pdev)
> +{
> + struct net_device *netdev = pci_get_drvdata(pdev);
> + struct nic *nic = netdev_priv(netdev);
> +
> + if (pci_enable_device(pdev)) {
> + printk(KERN_ERR "e100: Cannot re-enable PCI device after reset.\n");
> + return PCI_ERS_RESULT_DISCONNECT;
> + }
> + pci_set_master(pdev);
> +
> + /* Only one device per card can do a reset */
> + if (0 != PCI_FUNC(pdev->devfn))
Wrong order.
^ permalink raw reply
* Re: [PATCH 17/21] orinoco_pci: use pci_iomap() for resources
From: Pavel Roskin @ 2006-04-08 0:07 UTC (permalink / raw)
To: Francois Romieu
Cc: jgarzik-e+AXbWqSrlAAvxtiuMwx3w, netdev-u79uwXL29TY76Z2rM5mHXA,
orinoco-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f
In-Reply-To: <20060407233819.GB15667-lmTtMILVy1jWQcoT9B9Ug5SCg42XY1Uw0E9HWUfgJXw@public.gmane.org>
Quoting Francois Romieu <romieu-W8zweXLXuWQS+FvcfC7Uqw@public.gmane.org>:
> > > Is there a reason why dev->mem_{start/end} should not be removed ?
> >
> > Is there a reason why it should? Is it going to be obsolete?
>
> It is slowly obsoleting for a few years (don't laugh...). It is preferred
> to store the relevant address in the private part of the (pci) device.
>
> Moderately recent drivers do not use it at all. However it's fairly common
> in the setup code of the (legacy) isa devices.
I agree that many drivers don't use it. But it would be nice to have a document
describing what is going on. On one hand we are adding new information elements
(such as the bus in "ethtool -i"), on the other hand we are removing addresses
from the ifconfig output. Who is deciding which information is useful and
which is not?
How about netdev->irq? Is it going to be obsolete too? Then I can easily
remove orinoco_pci_setup_netdev() with very minimal adjustments.
--
Regards,
Pavel Roskin
-------------------------------------------------------
This SF.Net email is sponsored by xPML, a groundbreaking scripting language
that extends applications into web and mobile media. Attend the live webcast
and join the prime developer group breaking into this new coding territory!
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=110944&bid=241720&dat=121642
^ permalink raw reply
* Re: [PATCH] PCI Error Recovery: e100 network device driver
From: Francois Romieu @ 2006-04-08 8:12 UTC (permalink / raw)
To: Linas Vepstas
Cc: Greg KH, Jeff Garzik, netdev, linux-pci, linux-kernel,
linuxppc-dev, john.ronciak, jesse.brandeburg, jeffrey.t.kirsher
In-Reply-To: <20060407231134.GN25225@austin.ibm.com>
Linas Vepstas <linas@austin.ibm.com> :
> Index: linux-2.6.17-rc1/drivers/net/e100.c
> ===================================================================
> --- linux-2.6.17-rc1.orig/drivers/net/e100.c 2006-04-07 16:21:46.000000000 -0500
> +++ linux-2.6.17-rc1/drivers/net/e100.c 2006-04-07 18:10:52.411266545 -0500
[...]
> +static pci_ers_result_t e100_io_error_detected(struct pci_dev *pdev, pci_channel_state_t state)
80 cols limit.
[...]
> +static pci_ers_result_t e100_io_slot_reset(struct pci_dev *pdev)
> +{
> + struct net_device *netdev = pci_get_drvdata(pdev);
> + struct nic *nic = netdev_priv(netdev);
> +
> + if (pci_enable_device(pdev)) {
> + printk(KERN_ERR "e100: Cannot re-enable PCI device after reset.\n");
- The driver supports {get/set}_msglevel. Please consider using netif_msg_xxx
(see include/linux/netdevice.h).
- s/e100/DRV_NAME/ (or netdev->name, or pci_name(...) depending on the
context).
[...]
> +static struct pci_error_handlers e100_err_handler = {
> + .error_detected = e100_io_error_detected,
> + .slot_reset = e100_io_slot_reset,
> + .resume = e100_io_resume,
> +};
Nit: I'd rather follow the style in the declaration of e100_driver.
--
Ueimor
^ permalink raw reply
* Re: [patch] ipv4: initialize arp_tbl rw lock
From: Heiko Carstens @ 2006-04-08 10:02 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jeff Garzik, Andrew Morton, netdev, linux-kernel, Frank Pavlic,
David S. Miller
In-Reply-To: <20060407074627.2f525b2e@localhost.localdomain>
> > The qeth driver makes use of the arp_tbl rw lock. CONFIG_DEBUG_SPINLOCK
> > detects that this lock is not initialized as it is supposed to be.
>
> This is a initialization order problem then, because:
> arp_init
> neigh_table_init
> rwlock_init
>
> does the initialization already. So fix the initialization sequence
> of the qeth driver or you will have other problems.
>
> My impression was the -rt folks wanted all lock initializations t be
> done at runtime not compile time.
Ok, so the problem seems to be that inet_init gets called after qeth_init.
Looking at the top level Makefile this seems to be true for all network
drivers in drivers/net/ and drivers/s390/net/ since we have
vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y)
The patch below works for me... I guess there must be a better way to make
sure that any networking driver's initcall is made before inet_init?
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index b401942..c5cea07 100644
--- a/Makefile
+++ b/Makefile
@@ -567,7 +567,7 @@ #
# System.map is generated to document addresses of all kernel symbols
vmlinux-init := $(head-y) $(init-y)
-vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y)
+vmlinux-main := $(core-y) $(libs-y) $(net-y) $(drivers-y)
vmlinux-all := $(vmlinux-init) $(vmlinux-main)
vmlinux-lds := arch/$(ARCH)/kernel/vmlinux.lds
^ permalink raw reply related
* Re: [patch] ipv4: initialize arp_tbl rw lock
From: Andrew Morton @ 2006-04-08 10:12 UTC (permalink / raw)
To: Heiko Carstens; +Cc: shemminger, jgarzik, netdev, linux-kernel, fpavlic, davem
In-Reply-To: <20060408100213.GA9412@osiris.boeblingen.de.ibm.com>
Heiko Carstens <heiko.carstens@de.ibm.com> wrote:
>
> Ok, so the problem seems to be that inet_init gets called after qeth_init.
> Looking at the top level Makefile this seems to be true for all network
> drivers in drivers/net/ and drivers/s390/net/ since we have
>
> vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y)
>
> The patch below works for me... I guess there must be a better way to make
> sure that any networking driver's initcall is made before inet_init?
>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
> ---
>
> Makefile | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index b401942..c5cea07 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -567,7 +567,7 @@ #
> # System.map is generated to document addresses of all kernel symbols
>
> vmlinux-init := $(head-y) $(init-y)
> -vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y)
> +vmlinux-main := $(core-y) $(libs-y) $(net-y) $(drivers-y)
<wonders what this will break>
I have a bad feeling that one day we're going to come unstuck with this
practice. Is there anything which dictates that the linker has to lay
sections out in .o-file-order?
Perhaps net initcalls should be using something higher priority than
device_initcall().
^ permalink raw reply
* Re: [patch] ipv4: initialize arp_tbl rw lock
From: David S. Miller @ 2006-04-08 10:14 UTC (permalink / raw)
To: heiko.carstens
Cc: shemminger, jgarzik, akpm, netdev, linux-kernel, fpavlic, davem
In-Reply-To: <20060408100213.GA9412@osiris.boeblingen.de.ibm.com>
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Date: Sat, 8 Apr 2006 12:02:13 +0200
> Ok, so the problem seems to be that inet_init gets called after qeth_init.
> Looking at the top level Makefile this seems to be true for all network
> drivers in drivers/net/ and drivers/s390/net/ since we have
>
> vmlinux-main := $(core-y) $(libs-y) $(drivers-y) $(net-y)
>
> The patch below works for me... I guess there must be a better way to make
> sure that any networking driver's initcall is made before inet_init?
>
> Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
We could make inet_init() a subsystem init but I vaguely recall
that we were doing that at one point and it broke things for
some reason.
Perhaps fs_initcall() would work better. Or if that causes
problems we could create a net_initcall() that sits between
fs_initcall() and device_initcall().
Or any other ideas?
^ permalink raw reply
* Re: [patch] ipv4: initialize arp_tbl rw lock
From: Heiko Carstens @ 2006-04-08 10:42 UTC (permalink / raw)
To: David S. Miller
Cc: shemminger, jgarzik, akpm, netdev, linux-kernel, fpavlic, davem
In-Reply-To: <20060408.031404.111884281.davem@davemloft.net>
> We could make inet_init() a subsystem init but I vaguely recall
> that we were doing that at one point and it broke things for
> some reason.
>
> Perhaps fs_initcall() would work better. Or if that causes
> problems we could create a net_initcall() that sits between
> fs_initcall() and device_initcall().
>
> Or any other ideas?
Just tried fs_initcall() and net_initcall(). Both seem to have some
side effects:
Symptom is that console output sometimes hangs for several seconds at:
"NET: Registered protocol family 2" while all cpus are in cpu_idle().
Heiko
^ permalink raw reply
* Re: Fw: [Bugme-new] [Bug 6349] New: iptables DNAT returns unknown error 4294967295
From: Patrick McHardy @ 2006-04-08 11:51 UTC (permalink / raw)
To: dmb; +Cc: Andrew Morton, netdev, Netfilter Development Mailinglist
In-Reply-To: <20060407142331.26e6b653.akpm@osdl.org>
[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]
Andrew Morton wrote:
>
> Begin forwarded message:
>
> Date: Fri, 7 Apr 2006 06:12:07 -0700
> From: bugme-daemon@bugzilla.kernel.org
> To: bugme-new@lists.osdl.org
> Subject: [Bugme-new] [Bug 6349] New: iptables DNAT returns unknown error 4294967295
>
>
> http://bugzilla.kernel.org/show_bug.cgi?id=6349
>
> Summary: iptables DNAT returns unknown error 4294967295
> Kernel Version: 2.6.17-rc1
> Status: NEW
> Severity: blocking
> Owner: laforge@gnumonks.org
> Submitter: dmb@pochta.ru
>
>
> Problem Description:
> When I try to add the rule with target DNAT to the OUTPUT chain I get the error
> message:
> [root@dbagrii:~]# iptables -t nat -A OUTPUT -p tcp -d 192.168.1.1 --dport 119 -j
> DNAT --to-destination 192.168.1.10:8119
> iptables: Unknown error 4294967295
>
> No rule is added, and the kernel says the message to the syslog:
> Apr 7 15:20:05 dbagrii kernel: ip_tables: DNAT target: bad hook_mask 8
>
> This error appears with iptables tool version 1.3.3 and 1.3.5 i tried to use.
>
> Steps to reproduce:
> Run the iptables tool with this arguments:
> $ iptables -t nat -A OUTPUT -p tcp -d 192.168.1.1 --dport 119 -j DNAT
> --to-destination 192.168.1.10:8119
Please try if this patch helps.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 973 bytes --]
[NETFILTER]: Fix DNAT in LOCAL_OUT
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 0fc7ff57191de5c09afe7db3932009963bba44d6
tree 02bc033088605bfd51fa4ea7ceb57d0aae1db65e
parent 8cc24d7ca2ffc6d9d70e1d94dc8dffbdf677e58f
author Patrick McHardy <kaber@trash.net> Sat, 08 Apr 2006 13:52:18 +0200
committer Patrick McHardy <kaber@trash.net> Sat, 08 Apr 2006 13:52:18 +0200
net/ipv4/netfilter/ip_nat_rule.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/netfilter/ip_nat_rule.c b/net/ipv4/netfilter/ip_nat_rule.c
index efba8c4..250b8ed 100644
--- a/net/ipv4/netfilter/ip_nat_rule.c
+++ b/net/ipv4/netfilter/ip_nat_rule.c
@@ -279,7 +279,7 @@ static struct ipt_target ipt_dnat_reg =
.target = ipt_dnat_target,
.targetsize = sizeof(struct ip_nat_multi_range_compat),
.table = "nat",
- .hooks = 1 << NF_IP_PRE_ROUTING,
+ .hooks = 1 << NF_IP_PRE_ROUTING || 1 << NF_IP_LOCAL_OUT,
.checkentry = ipt_dnat_checkentry,
};
^ permalink raw reply related
* Re: Fw: [Bugme-new] [Bug 6349] New: iptables DNAT returns unknown error 4294967295
From: Patrick McHardy @ 2006-04-08 11:58 UTC (permalink / raw)
To: dmb; +Cc: Andrew Morton, netdev, Netfilter Development Mailinglist
In-Reply-To: <4437A3B9.8000000@trash.net>
[-- Attachment #1: Type: text/plain, Size: 211 bytes --]
Patrick McHardy wrote:
> Please try if this patch helps.
>
> + .hooks = 1 << NF_IP_PRE_ROUTING || 1 << NF_IP_LOCAL_OUT,
I shouldn't send patches before getting out of bed .. :)
Please try this one instead.
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 976 bytes --]
[NETFILTER]: Fix DNAT in LOCAL_OUT
Signed-off-by: Patrick McHardy <kaber@trash.net>
---
commit 2ceefa038e908d5da21aefedae02da4eab1b2787
tree 417f03eca3b97b16a78321ace1556e57f5c60351
parent 8cc24d7ca2ffc6d9d70e1d94dc8dffbdf677e58f
author Patrick McHardy <kaber@trash.net> Sat, 08 Apr 2006 13:59:53 +0200
committer Patrick McHardy <kaber@trash.net> Sat, 08 Apr 2006 13:59:53 +0200
net/ipv4/netfilter/ip_nat_rule.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/ipv4/netfilter/ip_nat_rule.c b/net/ipv4/netfilter/ip_nat_rule.c
index efba8c4..1aba926 100644
--- a/net/ipv4/netfilter/ip_nat_rule.c
+++ b/net/ipv4/netfilter/ip_nat_rule.c
@@ -279,7 +279,7 @@ static struct ipt_target ipt_dnat_reg =
.target = ipt_dnat_target,
.targetsize = sizeof(struct ip_nat_multi_range_compat),
.table = "nat",
- .hooks = 1 << NF_IP_PRE_ROUTING,
+ .hooks = (1 << NF_IP_PRE_ROUTING) | (1 << NF_IP_LOCAL_OUT),
.checkentry = ipt_dnat_checkentry,
};
^ permalink raw reply related
* Re: [patch] ipv4: initialize arp_tbl rw lock
From: Sam Ravnborg @ 2006-04-08 12:14 UTC (permalink / raw)
To: David S. Miller
Cc: heiko.carstens, shemminger, jgarzik, akpm, netdev, linux-kernel,
fpavlic, davem
In-Reply-To: <20060408.031404.111884281.davem@davemloft.net>
On Sat, Apr 08, 2006 at 03:14:04AM -0700, David S. Miller wrote:
> Perhaps fs_initcall() would work better. Or if that causes
> problems we could create a net_initcall() that sits between
> fs_initcall() and device_initcall().
fs_initcall() seems to be used mainly for "init after subsystem" stuff.
Within fs/ only pipe.c uses fs_initcall().
If we are going to overload the usage of fs_initcall() even more then
we should maybe try to rename it?
Sam
^ 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