* Re: [PATCH 4/5] ibmebus: Move to of_device and of_platform_driver, match eHCA and eHEA drivers
From: Jeff Garzik @ 2007-10-16 15:00 UTC (permalink / raw)
To: Joachim Fenkes
Cc: Jan-Bernd Themann, Paul Mackerras, Christoph Raisch,
Hoang-Nam Nguyen, LKML, LinuxPPC-Dev, Paul Mackerras,
Stefan Roscher, Thomas Q Klein, netdev, Marcus Eder
In-Reply-To: <200710162320.00630.fenkes@de.ibm.com>
Joachim Fenkes wrote:
> On Tuesday 09 October 2007 10:21, Jan-Bernd Themann wrote:
>> Roland Dreier <rdreier@cisco.com> wrote on 03.10.2007 20:05:44:
>>> > > Replace struct ibmebus_dev and struct ibmebus_driver with struct
> of_device
>>> > > and struct of_platform_driver, respectively. Match the external
> ibmebus
>>> > > interface and drivers using it.
>>> > >
>>> > > Signed-off-by: Joachim Fenkes <fenkes@de.ibm.com>
>>> >
>>> > This is somewhat difficult as this patch touches files that are the
>>> > responsibility of three different maintainers. �Is it possible to
>>> > split the patch into three, one for each maintainer (possibly by
>>> > keeping both old and new interfaces around for a little while)?
>>> >
>>> > If not, then you need to get an Acked-by and an agreement that this
>>> > change can go via the powerpc.git tree from Roland Dreier and Jeff
>>> > Garzik.
>>>
>>> I don't see anything objectionable in the infiniband parts of the
>>> patch -- I don't have any way to test the changes but it all looks
>>> like a straightforward conversion to a new platform API. So:
>>>
>>> Acked-by: Roland Dreier <rolandd@cisco.com>
>>>
>>> - R.
>> Looks good from eHEA driver perspective.
>>
>> Acked-by: Jan-Bernd Themann <themann@de.ibm.com>
>
> Jeff, do you have any objections against this patch going into the kernel
> via Paul's powerpc.git tree? It touches only a few lines of ehea which are
> specific to the bus interface changes.
>
> You can see the full patch here:
> http://patchwork.ozlabs.org/linuxppc/patch?id=13750
>
> If you have no objections, please ack the patch so Paul can include it.
Fine with me...
Jeff
^ permalink raw reply
* Re: [PATCH 11/12] [IPSEC]: Reinject packet instead of calling netfilter directly on input
From: YOSHIFUJI Hideaki / 吉藤英明 @ 2007-10-16 15:05 UTC (permalink / raw)
To: herbert; +Cc: davem, netdev, kaber, yoshfuji, kozakai
In-Reply-To: <E1IhnTz-0003HT-00@gondolin.me.apana.org.au>
Herbert,
I think with this change, we parse extension headers, twice.
We really do not want to do this.
--yoshfuji
In article <E1IhnTz-0003HT-00@gondolin.me.apana.org.au> (at Tue, 16 Oct 2007 22:33:19 +0800), Herbert Xu <herbert@gondor.apana.org.au> says:
> [IPSEC]: Reinject packet instead of calling netfilter directly on input
>
> Currently we call netfilter directly on input after a series of transport
> mode transforms (and BEET but that's a separate bug). This is inconsistent
> because other parts of the stack such AF_PACKET cannot see the decapsulated
> packet. In fact this is a common complaint about the Linux IPsec stack.
>
> Another problem is that there is a potential for stack overflow if we
> encounter a DNAT rule which turns a foreign packet into a local one that
> contains another transport mode SA.
>
> This patch introduces a major behavioural change by reinjecting the
> packet instead of calling netfilter directly.
>
> This solves both of the aformentioned problems.
>
> It is still inconsistent with how we do things on output since we don't
> pass things through AF_PACKET there either but the same inconsistency
> exists for tunnel mode too so it's not a new problem.
>
> To make things easier I've added a new function called netif_rerx which
> resets netfilter and the dst before reinjecting the packet using netif_rx.
> This can be used by other tunnel code as well.
>
> I haven't added a reinject function for RO mode since it can never be
> called on that path and if it does we want to know about it through an
> OOPS.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> ---
>
> include/linux/netdevice.h | 1 +
> include/net/xfrm.h | 8 ++++++++
> net/core/dev.c | 12 ++++++++++++
> net/ipv4/xfrm4_input.c | 24 ++----------------------
> net/ipv4/xfrm4_mode_beet.c | 7 +++++++
> net/ipv4/xfrm4_mode_transport.c | 11 +++++++++++
> net/ipv4/xfrm4_mode_tunnel.c | 7 +++++++
> net/ipv6/xfrm6_input.c | 23 ++---------------------
> net/ipv6/xfrm6_mode_beet.c | 7 +++++++
> net/ipv6/xfrm6_mode_transport.c | 10 ++++++++++
> net/ipv6/xfrm6_mode_tunnel.c | 7 +++++++
> 11 files changed, 74 insertions(+), 43 deletions(-)
>
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 39dd83b..097f911 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -1039,6 +1039,7 @@ extern void dev_kfree_skb_any(struct sk_buff *skb);
> #define HAVE_NETIF_RX 1
> extern int netif_rx(struct sk_buff *skb);
> extern int netif_rx_ni(struct sk_buff *skb);
> +extern int netif_rerx(struct sk_buff *skb);
> #define HAVE_NETIF_RECEIVE_SKB 1
> extern int netif_receive_skb(struct sk_buff *skb);
> extern int dev_valid_name(const char *name);
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index a9e8247..e5ae5fa 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -311,6 +311,14 @@ struct xfrm_mode {
> */
> int (*output)(struct xfrm_state *x,struct sk_buff *skb);
>
> + /*
> + * Reinject packet into stack.
> + *
> + * On entry, the packet is in the state as on exit from the
> + * input function above.
> + */
> + int (*reinject)(struct xfrm_state *x,struct sk_buff *skb);
> +
> struct xfrm_state_afinfo *afinfo;
> struct module *owner;
> unsigned int encap;
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 38b03da..b753ec8 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -1808,6 +1808,18 @@ int netif_rx_ni(struct sk_buff *skb)
>
> EXPORT_SYMBOL(netif_rx_ni);
>
> +/* Reinject a packet that has previously been processed, e.g., by tunneling. */
> +int netif_rerx(struct sk_buff *skb)
> +{
> + nf_reset(skb);
> +
> + dst_release(skb->dst);
> + skb->dst = NULL;
> +
> + return netif_rx(skb);
> +}
> +EXPORT_SYMBOL(netif_rerx);
> +
> static inline struct net_device *skb_bond(struct sk_buff *skb)
> {
> struct net_device *dev = skb->dev;
> diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
> index 5cb0b59..f5576d5 100644
> --- a/net/ipv4/xfrm4_input.c
> +++ b/net/ipv4/xfrm4_input.c
> @@ -41,7 +41,6 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
> struct xfrm_state *xfrm_vec[XFRM_MAX_DEPTH];
> struct xfrm_state *x;
> int xfrm_nr = 0;
> - int decaps = 0;
> unsigned int nhoff = offsetof(struct iphdr, protocol);
>
> seq = 0;
> @@ -95,7 +94,6 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
> goto drop;
>
> if (x->props.mode == XFRM_MODE_TUNNEL) {
> - decaps = 1;
> break;
> }
>
> @@ -122,26 +120,8 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
> xfrm_nr * sizeof(xfrm_vec[0]));
> skb->sp->len += xfrm_nr;
>
> - nf_reset(skb);
> -
> - if (decaps) {
> - dst_release(skb->dst);
> - skb->dst = NULL;
> - netif_rx(skb);
> - return 0;
> - } else {
> -#ifdef CONFIG_NETFILTER
> - __skb_push(skb, skb->data - skb_network_header(skb));
> - ip_hdr(skb)->tot_len = htons(skb->len);
> - ip_send_check(ip_hdr(skb));
> -
> - NF_HOOK(PF_INET, NF_IP_PRE_ROUTING, skb, skb->dev, NULL,
> - xfrm4_rcv_encap_finish);
> - return 0;
> -#else
> - return -ip_hdr(skb)->protocol;
> -#endif
> - }
> + x->mode->reinject(x, skb);
> + return 0;
>
> drop_unlock:
> spin_unlock(&x->lock);
> diff --git a/net/ipv4/xfrm4_mode_beet.c b/net/ipv4/xfrm4_mode_beet.c
> index 73d2338..012ae98 100644
> --- a/net/ipv4/xfrm4_mode_beet.c
> +++ b/net/ipv4/xfrm4_mode_beet.c
> @@ -11,6 +11,7 @@
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/netdevice.h>
> #include <linux/skbuff.h>
> #include <linux/stringify.h>
> #include <net/dst.h>
> @@ -109,9 +110,15 @@ out:
> return err;
> }
>
> +static int xfrm4_beet_reinject(struct xfrm_state *x, struct sk_buff *skb)
> +{
> + return netif_rerx(skb);
> +}
> +
> static struct xfrm_mode xfrm4_beet_mode = {
> .input = xfrm4_beet_input,
> .output = xfrm4_beet_output,
> + .reinject = xfrm4_beet_reinject,
> .owner = THIS_MODULE,
> .encap = XFRM_MODE_BEET,
> };
> diff --git a/net/ipv4/xfrm4_mode_transport.c b/net/ipv4/xfrm4_mode_transport.c
> index fd840c7..602418b 100644
> --- a/net/ipv4/xfrm4_mode_transport.c
> +++ b/net/ipv4/xfrm4_mode_transport.c
> @@ -7,6 +7,7 @@
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/netdevice.h>
> #include <linux/skbuff.h>
> #include <linux/stringify.h>
> #include <net/dst.h>
> @@ -54,9 +55,19 @@ static int xfrm4_transport_input(struct xfrm_state *x, struct sk_buff *skb)
> return 0;
> }
>
> +static int xfrm4_transport_reinject(struct xfrm_state *x, struct sk_buff *skb)
> +{
> + __skb_push(skb, skb->data - skb_network_header(skb));
> + ip_hdr(skb)->tot_len = htons(skb->len);
> + ip_send_check(ip_hdr(skb));
> +
> + return netif_rerx(skb);
> +}
> +
> static struct xfrm_mode xfrm4_transport_mode = {
> .input = xfrm4_transport_input,
> .output = xfrm4_transport_output,
> + .reinject = xfrm4_transport_reinject,
> .owner = THIS_MODULE,
> .encap = XFRM_MODE_TRANSPORT,
> };
> diff --git a/net/ipv4/xfrm4_mode_tunnel.c b/net/ipv4/xfrm4_mode_tunnel.c
> index 1ae9d32..780908a 100644
> --- a/net/ipv4/xfrm4_mode_tunnel.c
> +++ b/net/ipv4/xfrm4_mode_tunnel.c
> @@ -7,6 +7,7 @@
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/netdevice.h>
> #include <linux/skbuff.h>
> #include <linux/stringify.h>
> #include <net/dst.h>
> @@ -134,9 +135,15 @@ out:
> return err;
> }
>
> +static int xfrm4_tunnel_reinject(struct xfrm_state *x, struct sk_buff *skb)
> +{
> + return netif_rerx(skb);
> +}
> +
> static struct xfrm_mode xfrm4_tunnel_mode = {
> .input = xfrm4_tunnel_input,
> .output = xfrm4_tunnel_output,
> + .reinject = xfrm4_tunnel_reinject,
> .owner = THIS_MODULE,
> .encap = XFRM_MODE_TUNNEL,
> };
> diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
> index b1201c3..1347e0a 100644
> --- a/net/ipv6/xfrm6_input.c
> +++ b/net/ipv6/xfrm6_input.c
> @@ -23,7 +23,6 @@ int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi)
> struct xfrm_state *xfrm_vec[XFRM_MAX_DEPTH];
> struct xfrm_state *x;
> int xfrm_nr = 0;
> - int decaps = 0;
> unsigned int nhoff;
>
> nhoff = IP6CB(skb)->nhoff;
> @@ -72,7 +71,6 @@ int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi)
> goto drop;
>
> if (x->props.mode == XFRM_MODE_TUNNEL) { /* XXX */
> - decaps = 1;
> break;
> }
>
> @@ -98,25 +96,8 @@ int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi)
> xfrm_nr * sizeof(xfrm_vec[0]));
> skb->sp->len += xfrm_nr;
>
> - nf_reset(skb);
> -
> - if (decaps) {
> - dst_release(skb->dst);
> - skb->dst = NULL;
> - netif_rx(skb);
> - return -1;
> - } else {
> -#ifdef CONFIG_NETFILTER
> - ipv6_hdr(skb)->payload_len = htons(skb->len);
> - __skb_push(skb, skb->data - skb_network_header(skb));
> -
> - NF_HOOK(PF_INET6, NF_IP6_PRE_ROUTING, skb, skb->dev, NULL,
> - ip6_rcv_finish);
> - return -1;
> -#else
> - return 1;
> -#endif
> - }
> + x->mode->reinject(x, skb);
> + return -1;
>
> drop_unlock:
> spin_unlock(&x->lock);
> diff --git a/net/ipv6/xfrm6_mode_beet.c b/net/ipv6/xfrm6_mode_beet.c
> index 13bb1e8..17622cf 100644
> --- a/net/ipv6/xfrm6_mode_beet.c
> +++ b/net/ipv6/xfrm6_mode_beet.c
> @@ -11,6 +11,7 @@
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/netdevice.h>
> #include <linux/skbuff.h>
> #include <linux/stringify.h>
> #include <net/dsfield.h>
> @@ -74,9 +75,15 @@ out:
> return err;
> }
>
> +static int xfrm6_beet_reinject(struct xfrm_state *x, struct sk_buff *skb)
> +{
> + return netif_rerx(skb);
> +}
> +
> static struct xfrm_mode xfrm6_beet_mode = {
> .input = xfrm6_beet_input,
> .output = xfrm6_beet_output,
> + .reinject = xfrm6_beet_reinject,
> .owner = THIS_MODULE,
> .encap = XFRM_MODE_BEET,
> };
> diff --git a/net/ipv6/xfrm6_mode_transport.c b/net/ipv6/xfrm6_mode_transport.c
> index 4e34410..e165442 100644
> --- a/net/ipv6/xfrm6_mode_transport.c
> +++ b/net/ipv6/xfrm6_mode_transport.c
> @@ -8,6 +8,7 @@
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/netdevice.h>
> #include <linux/skbuff.h>
> #include <linux/stringify.h>
> #include <net/dst.h>
> @@ -59,9 +60,18 @@ static int xfrm6_transport_input(struct xfrm_state *x, struct sk_buff *skb)
> return 0;
> }
>
> +static int xfrm6_transport_reinject(struct xfrm_state *x, struct sk_buff *skb)
> +{
> + ipv6_hdr(skb)->payload_len = htons(skb->len);
> + __skb_push(skb, skb->data - skb_network_header(skb));
> +
> + return netif_rerx(skb);
> +}
> +
> static struct xfrm_mode xfrm6_transport_mode = {
> .input = xfrm6_transport_input,
> .output = xfrm6_transport_output,
> + .reinject = xfrm6_transport_reinject,
> .owner = THIS_MODULE,
> .encap = XFRM_MODE_TRANSPORT,
> };
> diff --git a/net/ipv6/xfrm6_mode_tunnel.c b/net/ipv6/xfrm6_mode_tunnel.c
> index ea22838..1329d6a 100644
> --- a/net/ipv6/xfrm6_mode_tunnel.c
> +++ b/net/ipv6/xfrm6_mode_tunnel.c
> @@ -8,6 +8,7 @@
> #include <linux/init.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> +#include <linux/netdevice.h>
> #include <linux/skbuff.h>
> #include <linux/stringify.h>
> #include <net/dsfield.h>
> @@ -113,9 +114,15 @@ out:
> return err;
> }
>
> +static int xfrm6_tunnel_reinject(struct xfrm_state *x, struct sk_buff *skb)
> +{
> + return netif_rerx(skb);
> +}
> +
> static struct xfrm_mode xfrm6_tunnel_mode = {
> .input = xfrm6_tunnel_input,
> .output = xfrm6_tunnel_output,
> + .reinject = xfrm6_tunnel_reinject,
> .owner = THIS_MODULE,
> .encap = XFRM_MODE_TUNNEL,
> };
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 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
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