Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 11/12] [IPSEC]: Reinject packet instead of calling netfilter directly on input
From: Herbert Xu @ 2007-10-16 15:12 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki / 吉藤英明
  Cc: davem, netdev, kaber, kozakai
In-Reply-To: <20071017.000547.102039099.yoshfuji@linux-ipv6.org>

On Wed, Oct 17, 2007 at 12:05:47AM +0900, YOSHIFUJI Hideaki / 吉藤英明 wrote:
> 
> I think with this change, we parse extension headers, twice.
> We really do not want to do this.

Good point.  I'll need to think of some other way to do this then.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 7/12] [IPSEC]: Remove xfrmX_tunnel_check_size
From: Herbert Xu @ 2007-10-16 15:17 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David S. Miller, netdev
In-Reply-To: <4714CCCC.2040504@trash.net>

On Tue, Oct 16, 2007 at 04:38:04PM +0200, Patrick McHardy wrote:
>
> Thats true, but for the first case we actually have something in the
> stack doing that, which is NAT and routing by fwmark. Maybe netfilter
> should just send an ICMP error back, that would also solve the problem
> of silently dropped packets when rerouting to an unreachable
> destination.

Crap, NAT is now bane :)

OK Dave, please scratch everything starting from patch 7.
The first 6 patches should be OK though, unless something
else comes up :)

Patrick, my plan to solve this is to move the POST_ROUTING
calls up one-level.  So we'd call them from ip_forward_finish
and where we currently call dst_output.

Let me play with this and see how it turns out.

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* [PATCH 1/2] net: Let inet_diag and friends autoload
From: Jean Delvare @ 2007-10-16 15:28 UTC (permalink / raw)
  To: netdev; +Cc: Alexey Kuznetsov

By adding module aliases to inet_diag, tcp_diag and dccp_diag, we let
them load automatically as needed. This makes tools like "ss" run
faster.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
---
The alias naming scheme for tcp_diag and dccp_diag follows what was
done for dccp_ipv4 and dccp_ipv6.

 include/linux/net.h  |    4 ++++
 net/dccp/diag.c      |    1 +
 net/ipv4/inet_diag.c |    7 +++++++
 net/ipv4/tcp_diag.c  |    1 +
 4 files changed, 13 insertions(+)

--- linux-2.6.23-rc0.orig/net/ipv4/inet_diag.c	2007-10-16 13:32:13.000000000 +0200
+++ linux-2.6.23-rc0/net/ipv4/inet_diag.c	2007-10-16 13:46:34.000000000 +0200
@@ -815,6 +815,12 @@ static int inet_diag_rcv_msg(struct sk_b
 	    nlmsg_len(nlh) < hdrlen)
 		return -EINVAL;
 
+#ifdef CONFIG_KMOD
+	if (inet_diag_table[nlh->nlmsg_type] == NULL)
+		request_module("net-pf-%d-proto-%d-type-%d", PF_NETLINK,
+			       NETLINK_INET_DIAG, nlh->nlmsg_type);
+#endif
+
 	if (inet_diag_table[nlh->nlmsg_type] == NULL)
 		return -ENOENT;
 
@@ -914,3 +920,4 @@ static void __exit inet_diag_exit(void)
 module_init(inet_diag_init);
 module_exit(inet_diag_exit);
 MODULE_LICENSE("GPL");
+MODULE_ALIAS_NET_PF_PROTO(PF_NETLINK, NETLINK_INET_DIAG);
--- linux-2.6.23-rc0.orig/include/linux/net.h	2007-10-15 21:39:08.000000000 +0200
+++ linux-2.6.23-rc0/include/linux/net.h	2007-10-16 14:31:02.000000000 +0200
@@ -313,6 +313,10 @@ static const struct proto_ops name##_ops
 #define MODULE_ALIAS_NET_PF_PROTO(pf, proto) \
 	MODULE_ALIAS("net-pf-" __stringify(pf) "-proto-" __stringify(proto))
 
+#define MODULE_ALIAS_NET_PF_PROTO_TYPE(pf, proto, type) \
+	MODULE_ALIAS("net-pf-" __stringify(pf) "-proto-" __stringify(proto) \
+		     "-type-" __stringify(type))
+
 #ifdef CONFIG_SYSCTL
 #include <linux/sysctl.h>
 extern ctl_table net_table[];
--- linux-2.6.23-rc0.orig/net/ipv4/tcp_diag.c	2007-10-16 13:32:13.000000000 +0200
+++ linux-2.6.23-rc0/net/ipv4/tcp_diag.c	2007-10-16 13:51:09.000000000 +0200
@@ -56,3 +56,4 @@ static void __exit tcp_diag_exit(void)
 module_init(tcp_diag_init);
 module_exit(tcp_diag_exit);
 MODULE_LICENSE("GPL");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_INET_DIAG, TCPDIAG_GETSOCK);
--- linux-2.6.23-rc0.orig/net/dccp/diag.c	2007-02-04 19:44:54.000000000 +0100
+++ linux-2.6.23-rc0/net/dccp/diag.c	2007-10-16 14:27:10.000000000 +0200
@@ -68,3 +68,4 @@ module_exit(dccp_diag_fini);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Arnaldo Carvalho de Melo <acme@mandriva.com>");
 MODULE_DESCRIPTION("DCCP inet_diag handler");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_NETLINK, NETLINK_INET_DIAG, DCCPDIAG_GETSOCK);

-- 
Jean Delvare
Suse L3

^ permalink raw reply

* Re: [PATCH 4/5] ibmebus: Move to of_device and of_platform_driver, match eHCA and eHEA drivers
From: Stephen Rothwell @ 2007-10-16 15:27 UTC (permalink / raw)
  To: Joachim Fenkes
  Cc: Jeff Garzik, Thomas Q Klein, Jan-Bernd Themann, netdev,
	Paul Mackerras, LKML, LinuxPPC-Dev, Christoph Raisch, Marcus Eder,
	Paul Mackerras, Stefan Roscher
In-Reply-To: <200710162320.00630.fenkes@de.ibm.com>

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

On Tue, 16 Oct 2007 23:20:00 +0200 Joachim Fenkes <fenkes@de.ibm.com> wrote:
>

One small change - I intend to remove the name and owner fields from
struct of_platform_driver, so you should not bother initialising the name
field and just initialise the name field of the embedded struct
device_driver instead.  This, of course, means that you don't need

	drv->driver.name = drv->name;

in ibmebus_register_driver.

Sorry for not picking this up earlier.

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply

* [PATCH 2/2] net: Use MODULE_ALIAS_NET_PF_PROTO_TYPE where possible
From: Jean Delvare @ 2007-10-16 15:30 UTC (permalink / raw)
  To: netdev; +Cc: Arnaldo Carvalho de Melo

Now that we have this new MODULE_ALIAS_NET_PF_PROTO_TYPE macro, use it
where possible.

Signed-off-by: Jean Delvare <jdelvare@suse.de>
Cc: Arnaldo Carvalho de Melo <acme@ghostprotocols.net>
---
 net/dccp/ipv4.c |    4 ++--
 net/dccp/ipv6.c |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

--- linux-2.6.23-rc0.orig/net/dccp/ipv4.c	2007-10-15 21:39:08.000000000 +0200
+++ linux-2.6.23-rc0/net/dccp/ipv4.c	2007-10-16 14:36:54.000000000 +0200
@@ -1037,8 +1037,8 @@ module_exit(dccp_v4_exit);
  * values directly, Also cover the case where the protocol is not specified,
  * i.e. net-pf-PF_INET-proto-0-type-SOCK_DCCP
  */
-MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-33-type-6");
-MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-0-type-6");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_INET, 33, 6);
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_INET, 0, 6);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Arnaldo Carvalho de Melo <acme@mandriva.com>");
 MODULE_DESCRIPTION("DCCP - Datagram Congestion Controlled Protocol");
--- linux-2.6.23-rc0.orig/net/dccp/ipv6.c	2007-10-15 21:39:08.000000000 +0200
+++ linux-2.6.23-rc0/net/dccp/ipv6.c	2007-10-16 14:37:06.000000000 +0200
@@ -1220,8 +1220,8 @@ module_exit(dccp_v6_exit);
  * values directly, Also cover the case where the protocol is not specified,
  * i.e. net-pf-PF_INET6-proto-0-type-SOCK_DCCP
  */
-MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-33-type-6");
-MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-0-type-6");
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_INET6, 33, 6);
+MODULE_ALIAS_NET_PF_PROTO_TYPE(PF_INET6, 0, 6);
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Arnaldo Carvalho de Melo <acme@mandriva.com>");
 MODULE_DESCRIPTION("DCCPv6 - Datagram Congestion Controlled Protocol");

-- 
Jean Delvare
Suse L3

^ permalink raw reply

* Re: e100 problems in .23rc8 ?
From: Eric Sandeen @ 2007-10-16 15:47 UTC (permalink / raw)
  To: Herbert Xu; +Cc: David Mack, davej, auke-jan.h.kok, netdev
In-Reply-To: <20071016143505.GA12640@gondor.apana.org.au>

Herbert Xu wrote:
> On Tue, Oct 16, 2007 at 09:33:15AM -0500, Eric Sandeen wrote:
>> Hm... running 2.6.23-6.fc8, I've been through 30+ reboot cycles without
>> a problem.  Before, I'd oops every 5 or so times I booted...
>>
>> I now have another NIC in the box, disabled; I don't think that should
>> be affecting anything?
> 
> Well the original problem was caused by spurious interrupts on
> the IRQ line where your e100 is so it could well be sporadic.

Hah, well, I took the other NIC out and it didn't survive more than a
couple reboots on that kernel.

Now that I know I can still hit it, I'll do any testing that's needed.

Thanks,

-Eric

^ permalink raw reply

* Re: new NAPI interface broken
From: Arjan van de Ven @ 2007-10-16 15:56 UTC (permalink / raw)
  To: David Miller; +Cc: benh, ossthema, shemminger, netdev, themann, raisch
In-Reply-To: <20071016.013146.28785302.davem@davemloft.net>

David Miller wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Date: Tue, 16 Oct 2007 18:28:56 +1000
> 
>> Allright, so that's an out of tree userland thingy... (which may well
>> work on ppc too I suppose). Definitely not installed by default by my
>> distro so IRQs from the network cards on all x86's using ubuntu gutsy at
>> least are spread to all CPUs :-)
> 
> But the thing does treat network interfaces differently from
> other devices.
> 
and it works on various architectures....
The in-kernel x86 thing is going away (it's actually highly 
suboptimal)....

Yes it's done in userland, this is the right place so far.... out of 
tree... well... there's no good place for such userspace tools in the 
kernel tree currently otherwise I'd love to have it there.
If your distro doesn't install this by default, please file a bug 
against the distro; I know we (Intel) worked with Fedora, RHEL, SuSE 
and Ubuntu to get it included.... maybe others don't?

^ permalink raw reply

* Re: netif_napi_add vs. multiple netdev's
From: Stephen Hemminger @ 2007-10-16 16:10 UTC (permalink / raw)
  To: benh; +Cc: netdev
In-Reply-To: <1192513633.19073.21.camel@pasglop>

On Tue, 16 Oct 2007 15:47:13 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> Hi Stehphen !
> 
> The new netif_napi_add() function takes a netdev argument. In the EMAC
> case, there is one NAPI instance working on behalf of multiple netdev's,
> so that isn't very useful. For my EMAC patch (just posted to you & the
> list), I'm not passing NULL, but I'm wondering what would be a good way
> to handle netpoll here...

The problem is that netpoll only has a handle to the device being used
as a netconsole, but may need to activate the NAPI poll routine to
cleanup state. Perhaps teaching netpoll about N to 1 mappings would
be better.

> 
> The way it's currently implemented, there's a list of NAPI's attached to
> the netdev, so obviously, that won't work for my usage scenario.

sky2 driver has potentially 2 ports for 1 NAPI instance.

> I'm not sure what's the best data structure that would be suitable for
> both N ndev's for 1 NAPI and 1 ndev for N NAPI's though... I could
> allocate "stub" list heads and queue those up, but that's a bit gross...

The new interface doesn't change anything. The old code had the
same problem vis-a-vis netpoll.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply

* RE: e100 problems in .23rc8 ?
From: David Mack @ 2007-10-16 16:39 UTC (permalink / raw)
  To: Herbert Xu, Eric Sandeen; +Cc: davej, auke-jan.h.kok, netdev
In-Reply-To: <20071016143505.GA12640@gondor.apana.org.au>

My problem is anything but sporadic. I have succeeded in booting a
2.6.23-based kernel exactly once since the roll toward F8 started early
last summer.

Dave

> -----Original Message-----
> From: Herbert Xu [mailto:herbert@gondor.apana.org.au] On 
> Behalf Of Herbert Xu
> Sent: Tuesday, October 16, 2007 7:35 AM
> To: Eric Sandeen
> Cc: David Mack; davej@redhat.com; auke-jan.h.kok@intel.com; 
> netdev@vger.kernel.org
> Subject: Re: e100 problems in .23rc8 ?
> 
> On Tue, Oct 16, 2007 at 09:33:15AM -0500, Eric Sandeen wrote:
> > 
> > Hm... running 2.6.23-6.fc8, I've been through 30+ reboot 
> cycles without
> > a problem.  Before, I'd oops every 5 or so times I booted...
> > 
> > I now have another NIC in the box, disabled; I don't think 
> that should
> > be affecting anything?
> 
> Well the original problem was caused by spurious interrupts on
> the IRQ line where your e100 is so it could well be sporadic.
> 
> Cheers,
> -- 
> Visit Openswan at http://www.openswan.org/
> Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> 

^ permalink raw reply

* [PATCH] Disable polling in rtl8169_suspend() to prevent race with rtl8169_interrupt()
From: Karsten Wiese @ 2007-10-16 16:46 UTC (permalink / raw)
  To: romieu, netdev

Hi,

Sometimes (~ 1 in 10) after suspend to disk my r8169 nic doesn't work and a
"r8169: eth0: link down" gets mingled up with those suspend messages.
First tests show nothing bad, so maybe this patch really helps.
Will post again, if it doesn't.

      Karsten
------------------>

Disable polling in rtl8169_suspend() to prevent race with rtl8169_interrupt()

rtl8169_interrupt() is called during polling.
Without patch, rtl8169_interrupt() could run after rtl8169_asic_down(),
causing a disabled link state being saved later in rtl8169_suspend() by
pci_save_state().
netif_stop_queue() can go as it is called by netif_device_detach().

Signed-off-by: Karsten Wiese <fzu@wemgehoertderstaat.de>
---
 drivers/net/r8169.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index c76dd29..fee6c8e 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -3024,7 +3024,7 @@ static int rtl8169_suspend(struct pci_dev *pdev, pm_message_t state)
 		goto out_pci_suspend;
 
 	netif_device_detach(dev);
-	netif_stop_queue(dev);
+	netif_poll_disable(dev);
 
 	spin_lock_irq(&tp->lock);
 
@@ -3054,6 +3054,7 @@ static int rtl8169_resume(struct pci_dev *pdev)
 	if (!netif_running(dev))
 		goto out;
 
+	netif_poll_enable(dev);
 	netif_device_attach(dev);
 
 	rtl8169_schedule_work(dev, rtl8169_reset_task);
-- 
1.5.2.4


^ permalink raw reply related

* Multicast problem
From: Joakim Tjernlund @ 2007-10-16 16:49 UTC (permalink / raw)
  To: Netdev

I can't get my PowerPC 83xx(eth driver ucc_qeth) board to reply on a
multcast ping:
  ping -t 1 -c 2 224.0.0.1
unless I do ifconfig eth1 promisc and
echo 0 > /proc/sys/net/ipv4/icmp_echo_ignore_broadcasts

I guess I am missing something, but what?

Kernel: 2.6.23

 Jocke


^ permalink raw reply

* Re: Multicast problem
From: David Stevens @ 2007-10-16 17:04 UTC (permalink / raw)
  To: joakim.tjernlund; +Cc: Netdev
In-Reply-To: <1192553387.30006.29.camel@gentoo-jocke.transmode.se>

If you have icmp_echo_ignore_broadcasts set to 1, it won't respond to
multicasts. That should be all you need to do for 224.0.0.1.

Some smart switches rely on IGMP snooping to determine group membership,
and some of those don't understand IGMPv3, so if you have no v1 or v2 
queriers
on the network, you can force the IGMP version to be 2 instead of the 
default 3
by using a sysctl. But that's not the issue for 224.0.0.1, since the 
all-hosts group
is not reported via IGMP.

You might check the contents of /proc/net/igmp and /proc/net/dev_mcast to 
make
sure they have entries for the group. If so, I'd guess it might be a 
driver or device issue.

                                                        +-DLS


^ permalink raw reply

* [PATCH] gianfar: fix compile errors and warnings
From: Grant Likely @ 2007-10-16 17:08 UTC (permalink / raw)
  To: jeff, Li Yang, linux-kernel, netdev

From: Grant Likely <grant.likely@secretlab.ca>

Fixes compile error from commit 09f75cd7bf13720738e6a196cc0107ce9a5bd5a0.
Also eliminates an unused variable and eliminates an uninitialized variable
warning.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
---

 drivers/net/gianfar.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 558440c..0a2f2b3 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -696,7 +696,7 @@ int startup_gfar(struct net_device *dev)
 {
 	struct txbd8 *txbdp;
 	struct rxbd8 *rxbdp;
-	dma_addr_t addr;
+	dma_addr_t addr = 0;
 	unsigned long vaddr;
 	int i;
 	struct gfar_private *priv = netdev_priv(dev);
@@ -1237,8 +1237,6 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
  * starting over will fix the problem. */
 static void gfar_timeout(struct net_device *dev)
 {
-	struct gfar_private *priv = netdev_priv(dev);
-
 	dev->stats.tx_errors++;
 
 	if (dev->flags & IFF_UP) {
@@ -1346,7 +1344,7 @@ struct sk_buff * gfar_new_skb(struct net_device *dev, struct rxbd8 *bdp)
 
 static inline void count_errors(unsigned short status, struct gfar_private *priv)
 {
-	struct net_device_stats *stats = &dev->stats;
+	struct net_device_stats *stats = &priv->dev->stats;
 	struct gfar_extra_stats *estats = &priv->extra_stats;
 
 	/* If the packet was truncated, none of the other errors


^ permalink raw reply related

* Re: [PATCH] PHYLIB: IRQ event workqueue handling fixes
From: Maciej W. Rozycki @ 2007-10-16 17:19 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Andy Fleming, Andrew Morton, Jeff Garzik, netdev, linux-kernel
In-Reply-To: <20071016062108.GB1000@ff.dom.local>

On Tue, 16 Oct 2007, Jarek Poplawski wrote:

> Yes, it's all right here. Sorry for bothering - I should've found this
> by myself.

 Ah, no problem -- even with the right keys you may sometimes get lost in 
the results.

> I've still some doubts about this possible enable_irq() after
> free_irq(). If it's the only handler the status would be changed again
> and at least some of this code in check_irq_resend() would be run, but
> I can miss something again or/and this doesn't matter, as well.

 Well, enable_irq() and disable_irq() themselves are nesting, so they are 
not a problem.  OTOH, free_irq() does not seem to maintain the depth count 
correctly, which looks like a bug to me and which could trigger regardless 
of whether flush_scheduled_work() or cancel_work_sync() was called.

 The reason is CONFIG_DEBUG_SHIRQ which makes a simulated interrupt event 
be sent at the end of free_irq().  It looks like a problem that is 
complementary to one I signalled here:

http://lkml.org/lkml/2007/9/12/82

with respect to request_irq(), where, similarly, such an interrupt event 
is sent at the beginning.  It looks like nobody was concerned back then, 
but perhaps it is time to do a better investigation now and propose a 
solution.

 I'll think about it and thanks for your inquisitiveness that has led to 
these conclusions.

  Maciej

^ permalink raw reply

* Re: MSI interrupts and disable_irq
From: Yinghai Lu @ 2007-10-16 17:23 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Manfred Spraul, Ayaz Abdulla, nedev, Linux Kernel Mailing List,
	David Miller, Andrew Morton
In-Reply-To: <4713E713.9060702@pobox.com>

On 10/15/07, Jeff Garzik <jgarzik@pobox.com> wrote:
> Manfred Spraul wrote:
> > Jeff Garzik wrote:
> >>
> >> I think the scenario you outline is an illustration of the approach's
> >> fragility:  disable_irq() is a heavy hammer that originated with INTx,
> >> and it relies on a chip-specific disable method (kernel/irq/manage.c)
> >> that practically guarantees behavior will vary across MSI/INTx/etc.
> >>
> > I checked the code: IRQ_DISABLE is implemented in software, i.e.
> > handle_level_irq() only calls handle_IRQ_event() [and then the nic irq
> > handler] if IRQ_DISABLE is not set.
> > OTHO: The last trace looks as if nv_do_nic_poll() is interrupted by an irq.
> >
> > Perhaps something corrupts dev->irq? The irq is requested with
> >    request_irq(np->pci_dev->irq, handler, IRQF_SHARED, dev->name, dev)
> > and disabled with
> >    disable_irq_lockdep(dev->irq);
> >
> > Someone around with a MSI capable board? The forcedeth driver does
> >    dev->irq = pci_dev->irq
> > in nv_probe(), especially before pci_enable_msi().
> > Does pci_enable_msi() change pci_dev->irq? Then we would disable the
> > wrong interrupt....
>
> Remember, fundamentally MSI-X is a one-to-many relationship, when you
> consider a single PCI device might have multiple vectors.

msi-x is using other entry

               if (np->msi_flags & NV_MSI_X_ENABLED)

enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);

YH

^ permalink raw reply

* [PATCH] [net] fix gianfar (compile and warning)
From: Sebastian Siewior @ 2007-10-16 17:25 UTC (permalink / raw)
  To: jeff; +Cc: Li Yang, netdev, Grant Likely

Currently it does not compile because a not declared variable is used.
struct net_device_stats in driver's private struct is also unsued.
This patch uses (hopefully) the right stats.

Cc: Li Yang <leoli@freescale.com>
Signed-off-by: Sebastian Siewior <bigeasy@linutronix.de>
---
 drivers/net/gianfar.c |    7 +++----
 drivers/net/gianfar.h |    1 -
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
index 558440c..cc288d8 100644
--- a/drivers/net/gianfar.c
+++ b/drivers/net/gianfar.c
@@ -1237,8 +1237,6 @@ static int gfar_change_mtu(struct net_device *dev, int new_mtu)
  * starting over will fix the problem. */
 static void gfar_timeout(struct net_device *dev)
 {
-	struct gfar_private *priv = netdev_priv(dev);
-
 	dev->stats.tx_errors++;
 
 	if (dev->flags & IFF_UP) {
@@ -1344,8 +1342,9 @@ struct sk_buff * gfar_new_skb(struct net_device *dev, struct rxbd8 *bdp)
 	return skb;
 }
 
-static inline void count_errors(unsigned short status, struct gfar_private *priv)
+static inline void count_errors(unsigned short status, struct net_device *dev)
 {
+	struct gfar_private *priv = netdev_priv(dev);
 	struct net_device_stats *stats = &dev->stats;
 	struct gfar_extra_stats *estats = &priv->extra_stats;
 
@@ -1539,7 +1538,7 @@ int gfar_clean_rx_ring(struct net_device *dev, int rx_work_limit)
 
 			dev->stats.rx_bytes += pkt_len;
 		} else {
-			count_errors(bdp->status, priv);
+			count_errors(bdp->status, dev);
 
 			if (skb)
 				dev_kfree_skb_any(skb);
diff --git a/drivers/net/gianfar.h b/drivers/net/gianfar.h
index c16cc8b..46cd773 100644
--- a/drivers/net/gianfar.h
+++ b/drivers/net/gianfar.h
@@ -749,7 +749,6 @@ struct gfar_private {
 	uint32_t msg_enable;
 
 	/* Network Statistics */
-	struct net_device_stats stats;
 	struct gfar_extra_stats extra_stats;
 };
 
-- 
1.5.3.4


^ permalink raw reply related

* Re: r8169 & TX offload
From: Francois Romieu @ 2007-10-16 17:25 UTC (permalink / raw)
  To: Vladislav Bolkhovitin; +Cc: LKML, netdev
In-Reply-To: <4714D785.3000402@vlnb.net>

(please remove l-k from the Cc:, this ought to be sent to netdev)

Vladislav Bolkhovitin <vst@vlnb.net> :
>
> I've recently bought Realtek RTL8169S-32 chip based Gigabit Ethernet card:
> 
> # lspci -vv
[...]
> I've noticed that all TX offload features (tx-checksumming, 
> scatter-gather and TSO) are disabled by default and should be manually 
> enabled by ethtool. I wonder, is there any particular reason for that? 

As far as I can tell, it is mostly a lack of pressure from the users and
my desire to avoid a change of behavior.

> Why they are not enabled by default as it was done for e1000 or tg3? Are 
> there any hidden drawbacks in enabling them?

None that I know of. I had not noticed a huge difference and people did
not comment a lot either...

> Those offload options are definitely work. They give in my setup (32-bit 
> 66MHz PCI, Xeon 1.7GHz CPU, open-iscsi) CPU offload from 75% to 45% and 
> data write throughput improvement from 55MB/s to 59MB/s.

...but your datapoint is welcome.

-- 
Ueimor

^ permalink raw reply

* RE: Multicast problem
From: Joakim Tjernlund @ 2007-10-16 17:31 UTC (permalink / raw)
  To: 'David Stevens'; +Cc: 'Netdev'
In-Reply-To: <OFB45E9AE1.F95FB289-ON88257376.005D25C3-88257376.005DB70D@us.ibm.com>

> -----Original Message-----
> From: David Stevens [mailto:dlstevens@us.ibm.com] 
> Sent: den 16 oktober 2007 19:05
> To: joakim.tjernlund@transmode.se
> Cc: Netdev
> Subject: Re: Multicast problem
> 
> If you have icmp_echo_ignore_broadcasts set to 1, it won't respond to
> multicasts. That should be all you need to do for 224.0.0.1.
> 
> Some smart switches rely on IGMP snooping to determine group 
> membership,
> and some of those don't understand IGMPv3, so if you have no v1 or v2 
> queriers
> on the network, you can force the IGMP version to be 2 instead of the 
> default 3
> by using a sysctl.

 Tried forcing igmp from 3 to 2, didn't help

> But that's not the issue for 224.0.0.1, since the 
> all-hosts group
> is not reported via IGMP.
> 
> You might check the contents of /proc/net/igmp and 
> /proc/net/dev_mcast to

dev_mcast and igmp looks:
root@Challenger:/proc/net# m dev_mcast
1    eth0            1     0     01005e000001
2    eth1            1     0     01005e000001

root@Challenger:/proc/net# m igmp
Idx     Device    : Count Querier       Group    Users Timer    Reporter
1       eth0      :     1      V3
                                E0000001     1 0:00000000               0
2       eth1      :     1      V2
                                E0000001     1 0:00000000               0
4       lo        :     0      V3
                                E0000001     1 0:00000000               0

I can't tell if that's OK or not.

 
> make
> sure they have entries for the group. If so, I'd guess it might be a 
> driver or device issue.

Could it be some missing kernel config? This is a custom board so I could
very well miss something I kernel config or user space config.

Thanks for your help, if you have any more ideas you are most welcome
to share :)

 Jocke
> 
>                                                         +-DLS


^ permalink raw reply

* Re: MSI interrupts and disable_irq
From: Jeff Garzik @ 2007-10-16 17:39 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Manfred Spraul, Ayaz Abdulla, nedev, Linux Kernel Mailing List,
	David Miller, Andrew Morton
In-Reply-To: <86802c440710161023p3fbd6832gdb6d10a1b5e9d932@mail.gmail.com>

Yinghai Lu wrote:
> On 10/15/07, Jeff Garzik <jgarzik@pobox.com> wrote:
>> Manfred Spraul wrote:
>>> Jeff Garzik wrote:
>>>> I think the scenario you outline is an illustration of the approach's
>>>> fragility:  disable_irq() is a heavy hammer that originated with INTx,
>>>> and it relies on a chip-specific disable method (kernel/irq/manage.c)
>>>> that practically guarantees behavior will vary across MSI/INTx/etc.
>>>>
>>> I checked the code: IRQ_DISABLE is implemented in software, i.e.
>>> handle_level_irq() only calls handle_IRQ_event() [and then the nic irq
>>> handler] if IRQ_DISABLE is not set.
>>> OTHO: The last trace looks as if nv_do_nic_poll() is interrupted by an irq.
>>>
>>> Perhaps something corrupts dev->irq? The irq is requested with
>>>    request_irq(np->pci_dev->irq, handler, IRQF_SHARED, dev->name, dev)
>>> and disabled with
>>>    disable_irq_lockdep(dev->irq);
>>>
>>> Someone around with a MSI capable board? The forcedeth driver does
>>>    dev->irq = pci_dev->irq
>>> in nv_probe(), especially before pci_enable_msi().
>>> Does pci_enable_msi() change pci_dev->irq? Then we would disable the
>>> wrong interrupt....
>> Remember, fundamentally MSI-X is a one-to-many relationship, when you
>> consider a single PCI device might have multiple vectors.
> 
> msi-x is using other entry
> 
>                if (np->msi_flags & NV_MSI_X_ENABLED)
> 
> enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);

Correct, but the overall point was that MSI-X conceptually conflicts 
with the existing "lockless" disable_irq() schedule, which was written 
when there was a one-one relationship between irq, PCI device, and work 
to be done.

	Jeff




^ permalink raw reply

* Re: MSI interrupts and disable_irq
From: Yinghai Lu @ 2007-10-16 17:59 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Manfred Spraul, Ayaz Abdulla, nedev, Linux Kernel Mailing List,
	David Miller, Andrew Morton
In-Reply-To: <4714F736.5000302@pobox.com>

On 10/16/07, Jeff Garzik <jgarzik@pobox.com> wrote:
> Yinghai Lu wrote:
> > On 10/15/07, Jeff Garzik <jgarzik@pobox.com> wrote:
> >> Manfred Spraul wrote:
> >>> Jeff Garzik wrote:
> >>>> I think the scenario you outline is an illustration of the approach's
> >>>> fragility:  disable_irq() is a heavy hammer that originated with INTx,
> >>>> and it relies on a chip-specific disable method (kernel/irq/manage.c)
> >>>> that practically guarantees behavior will vary across MSI/INTx/etc.
> >>>>
> >>> I checked the code: IRQ_DISABLE is implemented in software, i.e.
> >>> handle_level_irq() only calls handle_IRQ_event() [and then the nic irq
> >>> handler] if IRQ_DISABLE is not set.
> >>> OTHO: The last trace looks as if nv_do_nic_poll() is interrupted by an irq.
> >>>
> >>> Perhaps something corrupts dev->irq? The irq is requested with
> >>>    request_irq(np->pci_dev->irq, handler, IRQF_SHARED, dev->name, dev)
> >>> and disabled with
> >>>    disable_irq_lockdep(dev->irq);
> >>>
> >>> Someone around with a MSI capable board? The forcedeth driver does
> >>>    dev->irq = pci_dev->irq
> >>> in nv_probe(), especially before pci_enable_msi().
> >>> Does pci_enable_msi() change pci_dev->irq? Then we would disable the
> >>> wrong interrupt....
> >> Remember, fundamentally MSI-X is a one-to-many relationship, when you
> >> consider a single PCI device might have multiple vectors.
> >
> > msi-x is using other entry
> >
> >                if (np->msi_flags & NV_MSI_X_ENABLED)
> >
> > enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
>
> Correct, but the overall point was that MSI-X conceptually conflicts
> with the existing "lockless" disable_irq() schedule, which was written
> when there was a one-one relationship between irq, PCI device, and work
> to be done.

Can I use your new driver with RHEL 5 or RHEL 5.1?

YH

^ permalink raw reply

* Re: MSI interrupts and disable_irq
From: Yinghai Lu @ 2007-10-16 18:01 UTC (permalink / raw)
  To: Jeff Garzik
  Cc: Manfred Spraul, Ayaz Abdulla, nedev, Linux Kernel Mailing List,
	David Miller, Andrew Morton
In-Reply-To: <4714F736.5000302@pobox.com>

On 10/16/07, Jeff Garzik <jgarzik@pobox.com> wrote:
> Yinghai Lu wrote:
> > On 10/15/07, Jeff Garzik <jgarzik@pobox.com> wrote:
> >> Manfred Spraul wrote:
> >>> Jeff Garzik wrote:
> >>>> I think the scenario you outline is an illustration of the approach's
> >>>> fragility:  disable_irq() is a heavy hammer that originated with INTx,
> >>>> and it relies on a chip-specific disable method (kernel/irq/manage.c)
> >>>> that practically guarantees behavior will vary across MSI/INTx/etc.
> >>>>
> >>> I checked the code: IRQ_DISABLE is implemented in software, i.e.
> >>> handle_level_irq() only calls handle_IRQ_event() [and then the nic irq
> >>> handler] if IRQ_DISABLE is not set.
> >>> OTHO: The last trace looks as if nv_do_nic_poll() is interrupted by an irq.
> >>>
> >>> Perhaps something corrupts dev->irq? The irq is requested with
> >>>    request_irq(np->pci_dev->irq, handler, IRQF_SHARED, dev->name, dev)
> >>> and disabled with
> >>>    disable_irq_lockdep(dev->irq);
> >>>
> >>> Someone around with a MSI capable board? The forcedeth driver does
> >>>    dev->irq = pci_dev->irq
> >>> in nv_probe(), especially before pci_enable_msi().
> >>> Does pci_enable_msi() change pci_dev->irq? Then we would disable the
> >>> wrong interrupt....
> >> Remember, fundamentally MSI-X is a one-to-many relationship, when you
> >> consider a single PCI device might have multiple vectors.
> >
> > msi-x is using other entry
> >
> >                if (np->msi_flags & NV_MSI_X_ENABLED)
> >
> > enable_irq_lockdep(np->msi_x_entry[NV_MSI_X_VECTOR_ALL].vector);
>
> Correct, but the overall point was that MSI-X conceptually conflicts
> with the existing "lockless" disable_irq() schedule, which was written
> when there was a one-one relationship between irq, PCI device, and work
> to be done.

at this point, nic in mcp55 is using msi or INTx.

YH

^ permalink raw reply

* Re: [PATCH/RFC] net: Add __napi_sycnhronize() to sync with napi poll
From: Stephen Hemminger @ 2007-10-16 18:53 UTC (permalink / raw)
  To: benh; +Cc: netdev, David S. Miller, Roland Dreier, linuxppc-dev list
In-Reply-To: <1192513792.19073.23.camel@pasglop>

On Tue, 16 Oct 2007 15:49:52 +1000
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> net: Add __napi_sycnhronize() to sync with napi poll
> 
> The EMAC driver which needs to handle multiple devices with one
> NAPI instance implements its own per-channel disable bit. However,
> when setting such a bit, it needs to synchronize with the poller
> (that is make sure that any pending poller instance has completed,
> or is started late enough to see that disable bit).
> 
> This implements a low level __napi_synchronize() function to acheive
> that. The underscores are to emphasis the low level aspect of it and
> to discourage driver writers who don't know what they are doing to
> use it (to please DaveM :-)
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
> 
> (Use correct address for Stephen this time)
> 
> If the approach is accepted, I would like to have this merged now
> so the EMAC patch to make it work again can follow :-)
> 
> Note: I use msleep_interruptible(1); just like napi_disable(). However
> I'm not too happy that the "hot" loop that results of a pending signal
> here will spin without even a cpu_relax ... what do you guys think would
> be the best way to handle this ?


So this is really just like synchronize_irq()?  Using msleep is bogus
because you want to spin, you are only waiting for a softirq on the other
cpu to finish. If you wait for a whole millisecond and sleep that
is far longer than the napi routine should take.

You could even optimize it like synchronize_irq() for the non-SMP case.



-- 
Stephen Hemminger <shemminger@linux-foundation.org>

^ permalink raw reply

* RE: Multicast problem
From: Joakim Tjernlund @ 2007-10-16 18:54 UTC (permalink / raw)
  To: 'David Stevens'; +Cc: 'Netdev'
In-Reply-To: <OFB45E9AE1.F95FB289-ON88257376.005D25C3-88257376.005DB70D@us.ibm.com>

> -----Original Message-----
> From: David Stevens [mailto:dlstevens@us.ibm.com] 
> Sent: den 16 oktober 2007 19:05
> To: joakim.tjernlund@transmode.se
> Cc: Netdev
> Subject: Re: Multicast problem
> 
> If you have icmp_echo_ignore_broadcasts set to 1, it won't respond to
> multicasts. That should be all you need to do for 224.0.0.1.

hmm maybe I misunderstand here, but the we are trying to make OSPF work
in 2.6 and the problem appears to be that our board does not pick up
multicasts.
Is icmp_echo_ignore_broadcasts=0 required to make OSPF work in 2.6?

 Jocke


^ permalink raw reply

* [PATCH 0/7] skge: patches for 2.6.24
From: Stephen Hemminger @ 2007-10-16 19:15 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev

The first three are bug fixes, followed by cleanup
and addition of new eeprom and debug interface.

Patches are against current linux-2.6 git tree which
is at pre 2.6.24 state.

-- 
Stephen Hemminger <shemminger@linux-foundation.org>


^ permalink raw reply

* [PATCH 1/7] skge: fix ram buffer size calculation
From: Stephen Hemminger @ 2007-10-16 19:15 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20071016191548.447710749@linux-foundation.org>

[-- Attachment #1: skge-ram-buffer-fix.patch --]
[-- Type: text/plain, Size: 3525 bytes --]

This fixes problems with transmit hangs on older fiber based SysKonnect boards.

Adjust ram buffer sizing calculation to make it correct on all boards
and make it like the code in sky2 driver. 

Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>

--- a/drivers/net/skge.c	2007-10-16 10:55:13.000000000 -0700
+++ b/drivers/net/skge.c	2007-10-16 10:55:33.000000000 -0700
@@ -2403,32 +2403,31 @@ static int skge_ioctl(struct net_device 
 	return err;
 }
 
-static void skge_ramset(struct skge_hw *hw, u16 q, u32 start, size_t len)
+/* Assign Ram Buffer allocation to queue */
+static void skge_ramset(struct skge_hw *hw, u16 q, u32 start, u32 space)
 {
 	u32 end;
 
-	start /= 8;
-	len /= 8;
-	end = start + len - 1;
+	/* convert from K bytes to qwords used for hw register */
+	start *= 1024/8;
+	space *= 1024/8;
+	end = start + space - 1;
 
 	skge_write8(hw, RB_ADDR(q, RB_CTRL), RB_RST_CLR);
 	skge_write32(hw, RB_ADDR(q, RB_START), start);
+	skge_write32(hw, RB_ADDR(q, RB_END), end);
 	skge_write32(hw, RB_ADDR(q, RB_WP), start);
 	skge_write32(hw, RB_ADDR(q, RB_RP), start);
-	skge_write32(hw, RB_ADDR(q, RB_END), end);
 
 	if (q == Q_R1 || q == Q_R2) {
+		u32 tp = space - space/4;
+
 		/* Set thresholds on receive queue's */
-		skge_write32(hw, RB_ADDR(q, RB_RX_UTPP),
-			     start + (2*len)/3);
-		skge_write32(hw, RB_ADDR(q, RB_RX_LTPP),
-			     start + (len/3));
-	} else {
-		/* Enable store & forward on Tx queue's because
-		 * Tx FIFO is only 4K on Genesis and 1K on Yukon
-		 */
+		skge_write32(hw, RB_ADDR(q, RB_RX_UTPP), tp);
+		skge_write32(hw, RB_ADDR(q, RB_RX_LTPP), space/4);
+	} else if (hw->chip_id != CHIP_ID_GENESIS)
+		/* Genesis Tx Fifo is too small for normal store/forward */
 		skge_write8(hw, RB_ADDR(q, RB_CTRL), RB_ENA_STFWD);
-	}
 
 	skge_write8(hw, RB_ADDR(q, RB_CTRL), RB_ENA_OP_MD);
 }
@@ -2456,7 +2455,7 @@ static int skge_up(struct net_device *de
 	struct skge_port *skge = netdev_priv(dev);
 	struct skge_hw *hw = skge->hw;
 	int port = skge->port;
-	u32 chunk, ram_addr;
+	u32 ramaddr, ramsize, rxspace;
 	size_t rx_size, tx_size;
 	int err;
 
@@ -2511,14 +2510,15 @@ static int skge_up(struct net_device *de
 	spin_unlock_bh(&hw->phy_lock);
 
 	/* Configure RAMbuffers */
-	chunk = hw->ram_size / ((hw->ports + 1)*2);
-	ram_addr = hw->ram_offset + 2 * chunk * port;
+	ramsize = (hw->ram_size - hw->ram_offset) / hw->ports;
+	ramaddr = hw->ram_offset + port * ramsize;
+	rxspace = 8 + (2*(ramsize - 16))/3;
 
-	skge_ramset(hw, rxqaddr[port], ram_addr, chunk);
-	skge_qset(skge, rxqaddr[port], skge->rx_ring.to_clean);
+	skge_ramset(hw, rxqaddr[port], ramaddr, rxspace);
+	skge_ramset(hw, txqaddr[port], ramaddr + rxspace, ramsize - rxspace);
 
+	skge_qset(skge, rxqaddr[port], skge->rx_ring.to_clean);
 	BUG_ON(skge->tx_ring.to_use != skge->tx_ring.to_clean);
-	skge_ramset(hw, txqaddr[port], ram_addr+chunk, chunk);
 	skge_qset(skge, txqaddr[port], skge->tx_ring.to_use);
 
 	/* Start receiver BMU */
@@ -3450,15 +3450,12 @@ static int skge_reset(struct skge_hw *hw
 	if (hw->chip_id == CHIP_ID_GENESIS) {
 		if (t8 == 3) {
 			/* special case: 4 x 64k x 36, offset = 0x80000 */
-			hw->ram_size = 0x100000;
-			hw->ram_offset = 0x80000;
+			hw->ram_size = 1024;
+			hw->ram_offset = 512;
 		} else
 			hw->ram_size = t8 * 512;
-	}
-	else if (t8 == 0)
-		hw->ram_size = 0x20000;
-	else
-		hw->ram_size = t8 * 4096;
+	} else /* Yukon */
+		hw->ram_size = t8 ? t8 * 4 : 128;
 
 	hw->intr_mask = IS_HW_ERR;
 

-- 
Stephen Hemminger <shemminger@linux-foundation.org>


^ 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