* [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: Jarek Poplawski @ 2010-01-27 9:59 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100126103524.GB7780@ff.dom.local>
While looking at the "Deadlock in IPv6 code while garbage collection
on the rwlock protecting the routing tree" reported by Srinivas
Akkipeddi I found there is possible a NULL pointer dereference in
icmp6_dst_alloc().
Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
---
net/ipv6/route.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c2bd74c..93c3b20 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -973,7 +973,7 @@ struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
rt = ip6_dst_alloc(&net->ipv6.ip6_dst_ops);
if (unlikely(rt == NULL)) {
in6_dev_put(idev);
- goto out;
+ return NULL;
}
dev_hold(dev);
@@ -1009,7 +1009,6 @@ struct dst_entry *icmp6_dst_alloc(struct net_device *dev,
fib6_force_start_gc(net);
-out:
return &rt->u.dst;
}
^ permalink raw reply related
* [PATCH 1/3] net: maintain namespace isolation between vlan and real device
From: Arnd Bergmann @ 2010-01-27 10:05 UTC (permalink / raw)
To: David Miller
Cc: Stephen Hemminger, Patrick McHardy, Michael S. Tsirkin,
Herbert Xu, Or Gerlitz, netdev, bridge, linux-kernel
In-Reply-To: <201001271104.20607.arnd@arndb.de>
In the vlan and macvlan drivers, the start_xmit function forwards
data to the dev_queue_xmit function for another device, which may
potentially belong to a different namespace.
To make sure that classification stays within a single namespace,
this resets the potentially critical fields.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/net/macvlan.c | 2 +-
include/linux/netdevice.h | 9 +++++++++
net/8021q/vlan_dev.c | 2 +-
net/core/dev.c | 35 +++++++++++++++++++++++++++++++----
4 files changed, 42 insertions(+), 6 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index bad1303..e0436fd 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -269,7 +269,7 @@ static int macvlan_queue_xmit(struct sk_buff *skb, struct net_device *dev)
}
xmit_world:
- skb->dev = vlan->lowerdev;
+ skb_set_dev(skb, vlan->lowerdev);
return dev_queue_xmit(skb);
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 93a32a5..622ba5a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1004,6 +1004,15 @@ static inline bool netdev_uses_dsa_tags(struct net_device *dev)
return 0;
}
+#ifndef CONFIG_NET_NS
+static inline void skb_set_dev(struct sk_buff *skb, struct net_device *dev)
+{
+ skb->dev = dev;
+}
+#else /* CONFIG_NET_NS */
+void skb_set_dev(struct sk_buff *skb, struct net_device *dev);
+#endif
+
static inline bool netdev_uses_trailer_tags(struct net_device *dev)
{
#ifdef CONFIG_NET_DSA_TAG_TRAILER
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 77a49ff..95034a8 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -322,7 +322,7 @@ static netdev_tx_t vlan_dev_hard_start_xmit(struct sk_buff *skb,
}
- skb->dev = vlan_dev_info(dev)->real_dev;
+ skb_set_dev(skb, vlan_dev_info(dev)->real_dev);
len = skb->len;
ret = dev_queue_xmit(skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index 2cba5c5..e80403a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1448,13 +1448,10 @@ int dev_forward_skb(struct net_device *dev, struct sk_buff *skb)
if (skb->len > (dev->mtu + dev->hard_header_len))
return NET_RX_DROP;
- skb_dst_drop(skb);
+ skb_set_dev(skb, dev);
skb->tstamp.tv64 = 0;
skb->pkt_type = PACKET_HOST;
skb->protocol = eth_type_trans(skb, dev);
- skb->mark = 0;
- secpath_reset(skb);
- nf_reset(skb);
return netif_rx(skb);
}
EXPORT_SYMBOL_GPL(dev_forward_skb);
@@ -1614,6 +1611,36 @@ static bool dev_can_checksum(struct net_device *dev, struct sk_buff *skb)
return false;
}
+/**
+ * skb_dev_set -- assign a buffer to a new device
+ * @skb: buffer for the new device
+ * @dev: network device
+ *
+ * If an skb is owned by a device already, we have to reset
+ * all data private to the namespace a device belongs to
+ * before assigning it a new device.
+ */
+#ifdef CONFIG_NET_NS
+void skb_set_dev(struct sk_buff *skb, struct net_device *dev)
+{
+ skb_dst_drop(skb);
+ if (skb->dev && !net_eq(dev_net(skb->dev), dev_net(dev))) {
+ secpath_reset(skb);
+ nf_reset(skb);
+ skb_init_secmark(skb);
+ skb->mark = 0;
+ skb->priority = 0;
+ skb->nf_trace = 0;
+ skb->ipvs_property = 0;
+#ifdef CONFIG_NET_SCHED
+ skb->tc_index = 0;
+#endif
+ }
+ skb->dev = dev;
+}
+EXPORT_SYMBOL(skb_set_dev);
+#endif /* CONFIG_NET_NS */
+
/*
* Invalidate hardware checksum when packet is to be mangled, and
* complete checksum manually on outgoing path.
--
1.6.3.3
^ permalink raw reply related
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: David Miller @ 2010-01-27 10:14 UTC (permalink / raw)
To: jarkao2; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127095952.GA11635@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Wed, 27 Jan 2010 09:59:52 +0000
> While looking at the "Deadlock in IPv6 code while garbage collection
> on the rwlock protecting the routing tree" reported by Srinivas
> Akkipeddi I found there is possible a NULL pointer dereference in
> icmp6_dst_alloc().
>
> Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
Jarek, if rt is NULL so is &rt->u.dst
That's two patches trying to fix this same non-problem in
two different places today :-)
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: Jarek Poplawski @ 2010-01-27 10:34 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127.021424.263649631.davem@davemloft.net>
On Wed, Jan 27, 2010 at 02:14:24AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Wed, 27 Jan 2010 09:59:52 +0000
>
> > While looking at the "Deadlock in IPv6 code while garbage collection
> > on the rwlock protecting the routing tree" reported by Srinivas
> > Akkipeddi I found there is possible a NULL pointer dereference in
> > icmp6_dst_alloc().
> >
> > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
>
> Jarek, if rt is NULL so is &rt->u.dst
>
> That's two patches trying to fix this same non-problem in
> two different places today :-)
Yes, great joke! :-\
(Btw, maybe it's a sign this might be more readable?)
Thanks,
Jarek P.
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: David Miller @ 2010-01-27 11:04 UTC (permalink / raw)
To: jarkao2; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127103417.GA15206@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Wed, 27 Jan 2010 10:34:17 +0000
> (Btw, maybe it's a sign this might be more readable?)
I welcome suggests that, tree wide, would make this harder
to misinterpret.
^ permalink raw reply
* Re: [PATCH 1/1 V4] net: Add Aeroflex Gaisler 10/100/1G Ethernet MAC driver
From: Kristoffer Glembo @ 2010-01-27 11:01 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, davem
In-Reply-To: <20100125083131.568abc34@nehalam>
Hi,
Stephen Hemminger wrote:
> On Mon, 25 Jan 2010 12:01:12 +0100
> Kristoffer Glembo <kristoffer@gaisler.com> wrote:
>
>> +/* Accept MAC address of the form macaddr=0x08,0x00,0x20,0x30,0x40,0x50 */
>> +static int macaddr[6];
>> +module_param_array(macaddr, int, NULL, 0);
>> +MODULE_PARM_DESC(macaddr, "GRETH Ethernet MAC address");
>
> Don't you want an array of uchar here not int?
>
I copied this approach from the sunhme driver and it works fine. Any special
reason to change it?
I have incorporated the rest of your feedback including ethtool support.
Thanks a lot for the review!
Best regards,
Kristoffer Glembo
^ permalink raw reply
* Re: [PATCH net-next-2.6] packet: Add GSO/checksum offload support to af_packet sockets
From: Michael S. Tsirkin @ 2010-01-27 11:42 UTC (permalink / raw)
To: Sridhar Samudrala; +Cc: David Miller, Rusty Russell, Herbert Xu, netdev
In-Reply-To: <1264537819.24933.122.camel@w-sridhar.beaverton.ibm.com>
On Tue, Jan 26, 2010 at 12:30:19PM -0800, Sridhar Samudrala wrote:
> This patch adds GSO/checksum offload to af_packet sockets using
> virtio_net_hdr. Based on Rusty's patch to add this support to tun.
> It allows GSO/checksum offload to be enabled when using raw socket
> backend with virtio_net.
> Adds PACKET_VNET_HDR socket option to prepend virtio_net_hdr in the
> receive path and process/skip virtio_net_hdr in the send path. This
> option is only allowed with SOCK_RAW sockets attached to ethernet
> type devices.
>
> Signed-off-by: Sridhar Samudrala <sri@us.ibm.com>
So the main issue with this implemenation is that it silently fails for
non-ethernet protocols. It would be better to detect unsupported
protocols and return an error to user. This is same issue that was
pointed out by DaveM with my earlier attempt to solve a different
(related) problem:
http://lkml.org/lkml/2010/1/5/474
For an incomplete prototype attempting to solve the issue in a generic way:
http://lkml.org/lkml/2010/1/6/56
A couple of additional comments below.
> diff --git a/include/linux/if_packet.h b/include/linux/if_packet.h
> index 4021d47..aa57a5f 100644
> --- a/include/linux/if_packet.h
> +++ b/include/linux/if_packet.h
> @@ -46,6 +46,7 @@ struct sockaddr_ll {
> #define PACKET_RESERVE 12
> #define PACKET_TX_RING 13
> #define PACKET_LOSS 14
> +#define PACKET_VNET_HDR 15
>
> struct tpacket_stats {
> unsigned int tp_packets;
> diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
> index 53633c5..36d5360 100644
> --- a/net/packet/af_packet.c
> +++ b/net/packet/af_packet.c
> @@ -80,6 +80,8 @@
> #include <linux/init.h>
> #include <linux/mutex.h>
> #include <linux/if_vlan.h>
> +#include <linux/virtio_net.h>
> +#include <linux/if_arp.h>
>
> #ifdef CONFIG_INET
> #include <net/inet_common.h>
> @@ -193,7 +195,8 @@ struct packet_sock {
> struct mutex pg_vec_lock;
> unsigned int running:1, /* prot_hook is attached*/
> auxdata:1,
> - origdev:1;
> + origdev:1,
> + vnet_hdr:1;
> int ifindex; /* bound device */
> __be16 num;
> struct packet_mclist *mclist;
> @@ -1056,6 +1059,30 @@ out:
> }
> #endif
>
> +static inline struct sk_buff *packet_alloc_skb(struct sock *sk, size_t prepad,
> + size_t reserve, size_t len,
> + size_t linear, int noblock,
> + int *err)
> +{
> + struct sk_buff *skb;
> +
> + /* Under a page? Don't bother with paged skb. */
> + if (prepad + len < PAGE_SIZE || !linear)
> + linear = len;
> +
> + skb = sock_alloc_send_pskb(sk, prepad + linear, len - linear, noblock,
> + err);
> + if (!skb)
> + return NULL;
> +
> + skb_reserve(skb, reserve);
> + skb_put(skb, linear);
> + skb->data_len = len - linear;
> + skb->len += len - linear;
> +
> + return skb;
> +}
> +
> static int packet_snd(struct socket *sock,
> struct msghdr *msg, size_t len)
> {
> @@ -1066,14 +1093,15 @@ static int packet_snd(struct socket *sock,
> __be16 proto;
> unsigned char *addr;
> int ifindex, err, reserve = 0;
> + struct virtio_net_hdr vnethdr = { 0 };
> + int offset = 0;
> + struct packet_sock *po = pkt_sk(sk);
>
> /*
> * Get and verify the address.
> */
>
> if (saddr == NULL) {
> - struct packet_sock *po = pkt_sk(sk);
> -
> ifindex = po->ifindex;
> proto = po->num;
> addr = NULL;
> @@ -1100,25 +1128,52 @@ static int packet_snd(struct socket *sock,
> if (!(dev->flags & IFF_UP))
> goto out_unlock;
>
> - err = -EMSGSIZE;
> - if (len > dev->mtu+reserve)
> - goto out_unlock;
> + if (po->vnet_hdr) {
> + err = -EINVAL;
> + if (dev->type != ARPHRD_ETHER)
> + goto out_unlock;
> +
> + if (len < sizeof(vnethdr))
> + goto out_unlock;
>
> - skb = sock_alloc_send_skb(sk, len + LL_ALLOCATED_SPACE(dev),
> - msg->msg_flags & MSG_DONTWAIT, &err);
> + len -= sizeof(vnethdr);
> +
> + err = -EFAULT;
> + if (memcpy_fromiovec((void *)&vnethdr, msg->msg_iov,
> + sizeof(vnethdr)))
> + goto out_unlock;
> +
> + if ((vnethdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) &&
> + (vnethdr.csum_start + vnethdr.csum_offset + 2 >
> + vnethdr.hdr_len))
> + vnethdr.hdr_len = vnethdr.csum_start +
> + vnethdr.csum_offset + 2;
> +
> + err = -EINVAL;
> + if (vnethdr.hdr_len > len)
> + goto out_unlock;
> + } else {
> + err = -EMSGSIZE;
> + if (len > dev->mtu+reserve)
> + goto out_unlock;
IMO we should always perform the length check if GSO is off.
> + }
> +
> + err = -ENOBUFS;
> + skb = packet_alloc_skb(sk, LL_ALLOCATED_SPACE(dev),
> + LL_RESERVED_SPACE(dev), len, vnethdr.hdr_len,
> + msg->msg_flags & MSG_DONTWAIT, &err);
> if (skb == NULL)
> goto out_unlock;
>
> - skb_reserve(skb, LL_RESERVED_SPACE(dev));
> - skb_reset_network_header(skb);
> + skb_set_network_header(skb, reserve);
I think the above is wrong for vlans?
>
> err = -EINVAL;
> if (sock->type == SOCK_DGRAM &&
> - dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len) < 0)
> + (offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len)) < 0)
> goto out_free;
>
> /* Returns -EFAULT on error */
> - err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
> + err = skb_copy_datagram_from_iovec(skb, offset, msg->msg_iov, 0, len);
> if (err)
> goto out_free;
>
> @@ -1127,6 +1182,51 @@ static int packet_snd(struct socket *sock,
> skb->priority = sk->sk_priority;
> skb->mark = sk->sk_mark;
>
> + if (po->vnet_hdr) {
> + skb_reset_mac_header(skb);
> + skb->protocol = eth_hdr(skb)->h_proto;
> +
Is this also broken for vlans?
> + if (vnethdr.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
> + if (!skb_partial_csum_set(skb, vnethdr.csum_start,
> + vnethdr.csum_offset)) {
> + err = -EINVAL;
> + goto out_free;
> + }
> + }
> +
> + if (vnethdr.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
> + switch (vnethdr.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
> + case VIRTIO_NET_HDR_GSO_TCPV4:
> + skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
> + break;
> + case VIRTIO_NET_HDR_GSO_TCPV6:
> + skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
> + break;
> + case VIRTIO_NET_HDR_GSO_UDP:
> + skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
> + break;
> + default:
> + err = -EINVAL;
> + goto out_free;
> + }
> +
> + if (vnethdr.gso_type & VIRTIO_NET_HDR_GSO_ECN)
> + skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
> +
> + skb_shinfo(skb)->gso_size = vnethdr.gso_size;
> + if (skb_shinfo(skb)->gso_size == 0) {
> + err = -EINVAL;
> + goto out_free;
> + }
> +
> + /* Header must be checked, and gso_segs computed. */
> + skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
> + skb_shinfo(skb)->gso_segs = 0;
> + }
> +
> + len += sizeof(vnethdr);
> + }
> +
> /*
> * Now send it
> */
> @@ -1420,6 +1520,7 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
> struct sk_buff *skb;
> int copied, err;
> struct sockaddr_ll *sll;
> + int vnet_hdr_len = 0;
>
> err = -EINVAL;
> if (flags & ~(MSG_PEEK|MSG_DONTWAIT|MSG_TRUNC|MSG_CMSG_COMPAT))
> @@ -1451,6 +1552,44 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
> if (skb == NULL)
> goto out;
>
> + if (pkt_sk(sk)->vnet_hdr) {
> + struct virtio_net_hdr vnethdr = { 0 };
> +
> + vnet_hdr_len = sizeof(vnethdr);
> + if ((len -= vnet_hdr_len) < 0)
> + return -EINVAL;
> +
> + if (skb_is_gso(skb)) {
> + struct skb_shared_info *sinfo = skb_shinfo(skb);
> +
> + /* This is a hint as to how much should be linear. */
> + vnethdr.hdr_len = skb_headlen(skb);
> + vnethdr.gso_size = sinfo->gso_size;
> + if (sinfo->gso_type & SKB_GSO_TCPV4)
> + vnethdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
> + else if (sinfo->gso_type & SKB_GSO_TCPV6)
> + vnethdr.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
> + else if (sinfo->gso_type & SKB_GSO_UDP)
> + vnethdr.gso_type = VIRTIO_NET_HDR_GSO_UDP;
> + else
> + BUG();
Is there any chance this can get SKB_GSO_FCOE by binding to
an appropriate interface? Maybe we don't want to BUG().
> + if (sinfo->gso_type & SKB_GSO_TCP_ECN)
> + vnethdr.gso_type |= VIRTIO_NET_HDR_GSO_ECN;
> + } else
> + vnethdr.gso_type = VIRTIO_NET_HDR_GSO_NONE;
> +
> + if (skb->ip_summed == CHECKSUM_PARTIAL) {
> + vnethdr.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
> + vnethdr.csum_start = skb->csum_start - skb_headroom(skb);
> + vnethdr.csum_offset = skb->csum_offset;
> + } /* else everything is zero */
> +
> + if (unlikely(memcpy_toiovec(msg->msg_iov, (void *)&vnethdr,
> + sizeof(vnethdr)))) {
> + return -EFAULT;
> + }
> + }
> +
> /*
> * If the address length field is there to be filled in, we fill
> * it in now.
> @@ -1502,7 +1641,7 @@ static int packet_recvmsg(struct kiocb *iocb, struct socket *sock,
> * Free or return the buffer as appropriate. Again this
> * hides all the races and re-entrancy issues from us.
> */
> - err = (flags&MSG_TRUNC) ? skb->len : copied;
> + err = vnet_hdr_len + ((flags&MSG_TRUNC) ? skb->len : copied);
>
> out_free:
> skb_free_datagram(sk, skb);
> @@ -1826,6 +1965,22 @@ packet_setsockopt(struct socket *sock, int level, int optname, char __user *optv
> po->origdev = !!val;
> return 0;
> }
> + case PACKET_VNET_HDR:
> + {
> + int val;
> +
> + if (sock->type != SOCK_RAW)
> + return -EINVAL;
> + if (po->rx_ring.pg_vec || po->tx_ring.pg_vec)
> + return -EBUSY;
Another way to get a broken ring + vnet hdr configuration
would be to enable vnet hdr first and mmap second.
I think we need to guard against this as well, by checking vnet_hdr
when tx/rx ring is enabled.
> + if (optlen < sizeof(val))
> + return -EINVAL;
> + if (copy_from_user(&val, optval, sizeof(val)))
> + return -EFAULT;
> +
> + po->vnet_hdr = !!val;
> + return 0;
> + }
> default:
> return -ENOPROTOOPT;
> }
> @@ -1876,6 +2031,13 @@ static int packet_getsockopt(struct socket *sock, int level, int optname,
>
> data = &val;
> break;
> + case PACKET_VNET_HDR:
> + if (len > sizeof(int))
> + len = sizeof(int);
> + val = po->vnet_hdr;
> +
> + data = &val;
> + break;
> #ifdef CONFIG_PACKET_MMAP
> case PACKET_VERSION:
> if (len > sizeof(int))
>
^ permalink raw reply
* Re: CBQ broken in 2.6
From: David Miller @ 2010-01-27 11:56 UTC (permalink / raw)
To: anton.ivanov; +Cc: linux-kernel, netdev
In-Reply-To: <1264590347.29501.11.camel@tranquility.sigsegv.cx>
From: Anton Ivanov <anton.ivanov@kot-begemot.co.uk>
Date: Wed, 27 Jan 2010 11:05:47 +0000
> I have been looking at CBQ being broken in 2.6 (Debian BUG 534430).
Please direct your question to netdev@vger.kernel.org which is
where the networking experts hang out.
Added to CC:
> The bug is very simple and it makes it unusable for all practical
> purposes - BOUNDED classes always borrow from their parent. As a result
> it impossible to:
>
> 1. Set and enforce a bandwidth limit to a class with a CBQ tree
> 2. Use CBQ to do QoS at any place but the actual link. For example - put
> a linux box after behind a DSL modem and tell it to QoS at the bandwidth
> at which the modem is configured.
>
> I have now looked through the code of cbq_sched.c (in 2.6.33-rc5 and
> 2.6.26 as shipped by Debian).
I think you mean net/sched/sch_cbq.c, there is no cbq_sched.c
anywhere in the source tree.
> In both pieces of code the variable in the class structure for CBQ
> cl->borrow is used to signal if borrowing is allowed. If borrowing is
> not allowed (BOUNDED class) it is set to NULL.
>
> However the routine which parses the class options and defines the class
> cbq_change_class() sets it to the parent class and never resets it to
> NULL for bounded classes.
>
> Am I missing something here?
>
> Please cc me as I am not subscribed to the list.
>
> Best Regards,
>
> --
> Understanding is a three-edged sword:
> your side, their side, and the truth. --Kosh Naranek
>
> A. R. Ivanov
> E-mail: anton.ivanov@kot-begemot.co.uk
> WWW: http://www.kot-begemot.co.uk/
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: Jarek Poplawski @ 2010-01-27 12:17 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127.030416.179345234.davem@davemloft.net>
On Wed, Jan 27, 2010 at 03:04:16AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Wed, 27 Jan 2010 10:34:17 +0000
>
> > (Btw, maybe it's a sign this might be more readable?)
>
> I welcome suggests that, tree wide, would make this harder
> to misinterpret.
If there is nothing better I still think this patch is right, except
better subject (Fix impossible NULL dereference in ... ;-).
Jarek P.
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: David Miller @ 2010-01-27 12:23 UTC (permalink / raw)
To: jarkao2; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127121746.GA15611@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Wed, 27 Jan 2010 12:17:46 +0000
> If there is nothing better I still think this patch is right, except
> better subject (Fix impossible NULL dereference in ... ;-).
Sorry, I'm not applying it, just like I didn't apply Krishna's similar
patch he posted earlier today.
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: Atis Elsts @ 2010-01-27 12:35 UTC (permalink / raw)
To: Jarek Poplawski; +Cc: David Miller, shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127121746.GA15611@ff.dom.local>
> On Wed, Jan 27, 2010 at 03:04:16AM -0800, David Miller wrote:
>> From: Jarek Poplawski <jarkao2@gmail.com>
>> Date: Wed, 27 Jan 2010 10:34:17 +0000
>>
>> > (Btw, maybe it's a sign this might be more readable?)
>>
>> I welcome suggests that, tree wide, would make this harder
>> to misinterpret.
How about adding static inline functions for accessing the dst_entry
field in struct route via a cast? That way it would be immediately
obvious that invalid memory access is not going to happen. Like this:
static inline struct dst_entry *route_dst(struct rtable *rt)
{
return (struct dst_entry *)rt;
}
static inline struct dst_entry *route6_dst(struct rt6_info *rt)
{
return (struct dst_entry *)rt;
}
and so on...
Atis
^ permalink raw reply
* Re: [PATCH] tcp: fix ICMP-RTO war
From: Ilpo Järvinen @ 2010-01-27 12:36 UTC (permalink / raw)
To: Damian Lukowski
Cc: Denys Fedoryshchenko, Netdev, David Miller, Alexey Kuznetsov
In-Reply-To: <4B5D8ABB.8030906@tvk.rwth-aachen.de>
On Mon, 25 Jan 2010, Damian Lukowski wrote:
> considering Denys' latest tests, I think we should bound
> at TCP_RTO_MIN inside __tcp_set_rto().
> Look at the following piece:
> > [ 604.193389] rto: 200 (0 >> 3 + 0, 32) time: 304193 sent: 304091 pen: 1 307291 rem: 98
> > [ 604.193518] lower bound violation: 0 code 1 sk_state 1
> > [ 604.193589] rto: 200 (0 >> 3 + 0, 31) time: 304193 sent: 304091 pen: 1 304291 rem: 98
> > [ 604.193706] lower bound violation: 0 code 1 sk_state 1
> > [ 604.193776] rto: 200 (0 >> 3 + 0, 30) time: 304193 sent: 304091 pen: 1 304291 rem: 98
> > [ 607.341327] lower bound violation: 0 code 1 sk_state 1
> > [ 607.341412] rto: 200 (0 >> 3 + 0, 33) time: 307341 sent: 307091 pen: 1 310291 rem: 0
>
> We have a burst of three incoming ICMPs, not triggering retransmissions because
> of rem > 0. Nevertheless, there is an increase of icsk_backoff by four
> within 3100ms, with no ICMPs in between.
> For me, this is explainable by the broken mdev/rtt issue together with
> bursty ICMP replies.
Unless they are for a different connection? We might have to print sk (%p)
in all those printouts to be sure which maps to which. If a peer becomes
unreachable, it may well have multiple connections open (this was a
proxy, iirc?).
--
i.
^ permalink raw reply
* Re: [PATCH] tcp: fix ICMP-RTO war
From: Ilpo Järvinen @ 2010-01-27 12:41 UTC (permalink / raw)
To: David Miller; +Cc: damian, denys, Netdev, Alexey Kuznetsov
In-Reply-To: <20100125.234557.00486980.davem@davemloft.net>
On Mon, 25 Jan 2010, David Miller wrote:
> From: Damian Lukowski <damian@tvk.rwth-aachen.de>
> Date: Mon, 25 Jan 2010 16:07:43 +0100
>
> > @@ -530,7 +530,11 @@ static inline void tcp_bound_rto(const struct sock *sk)
> >
> > static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
> > {
> > - return (tp->srtt >> 3) + tp->rttvar;
> > + u32 rto = (tp->srtt >> 3) + tp->rttvar;
> > + if (unlikely(rto < TCP_RTO_MIN))
> > + return TCP_RTO_MIN;
> > + else
> > + return rto;
> > }
>
> The min RTO is now a runtime variable, TCP_RTO_MIN is merely the
> default, so we should use tcp_rto_min() for obtaining that value.
>
> And if we make this change, we might want to delete the comment in
> tcp_set_rto() which claims:
>
> /* NOTE: clamping at TCP_RTO_MIN is not required, current algo
> * guarantees that rto is higher.
> */
> tcp_bound_rto(sk);
>
> And we have shown here at least one case where that is not true.
> :-)
I went through some history, it seems that this comment about the lower
bound originates from Alexey [1]:
commit 893e1302654a9bcdc0e7d9ac95159657a8f5e0e8
Author: davem <davem>
Date: Wed Dec 13 04:10:12 2000 +0000
Fix numerous RTO bugs.
Withdraw CWR when DSACKs are seen.
Do not allow recvmsg on unbound socket, it is
senseless.
All from Alexey.
> I've looked at Denys's traces and your analysis, and I still
> can't figure out who the true culprit is that lets us get into
> such a state that RTO is evaluated so low...
--
i.
[1] http://git.kernel.org/?p=linux/kernel/git/davem/netdev-vger-cvs.git;a=commit;h=893e1302654a9bcdc0e7d9ac95159657a8f5e0e8
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: Jarek Poplawski @ 2010-01-27 12:43 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127.042336.52877358.davem@davemloft.net>
On Wed, Jan 27, 2010 at 04:23:36AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Wed, 27 Jan 2010 12:17:46 +0000
>
> > If there is nothing better I still think this patch is right, except
> > better subject (Fix impossible NULL dereference in ... ;-).
>
> Sorry, I'm not applying it, just like I didn't apply Krishna's similar
> patch he posted earlier today.
I've understood your decision and I'm OK with that; I'm only not sure
there is a better way (comments?) to make this code readable.
Jarek P.
^ permalink raw reply
* Re: CBQ broken in 2.6
From: Anton Ivanov @ 2010-01-27 12:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100127.035635.173712880.davem@davemloft.net>
Thanks it is indeed sch_cbq.c,
I have been going through the code in the meantime and found a place
where cl->tparent can be modified further down in the routine in
question. There is an invocation of cbq_set_lss() around the end of
cbq_change_class() which can do that.
I am rebuilding the kernel for my CBQ box with a few printks at the
moment to see if it modified there or not.
In any case here is the tell-tale symptom:
class cbq 1:16 parent 1: leaf 76: rate 5600Kbit (bounded,isolated) prio
2
Sent 162051 bytes 925 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
borrowed 201 overactions 0 avgidle 78 undertime 0
That is a bounded class. Its borrowed should be always 0 no matter what.
That is basically a broken CBQ implementation.
An elementary network test suite shows the same result - it is being
allowed to borrow.
I am happy to send the whole config if necessary if someone wants to
look at it.
Brgds,
[snip]
--
Understanding is a three-edged sword:
your side, their side, and the truth. --Kosh Naranek
A. R. Ivanov
E-mail: anton.ivanov@kot-begemot.co.uk
WWW: http://www.kot-begemot.co.uk/
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: David Miller @ 2010-01-27 13:14 UTC (permalink / raw)
To: jarkao2; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127124331.GB15611@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Wed, 27 Jan 2010 12:43:31 +0000
> I'm only not sure there is a better way (comments?) to make this
> code readable.
You can make it more readable and even validate the invariant at the
same time by adding some kind of inline function if you wish.
static inline struct dst_entry *rtable_to_dst_entry(struct rtable *rt)
{
BUILD_BUG_ON(rt != &rt->u.dst);
/* Because "->u.dst" is the first member of struct rtable,
* this dereference is safe even when rt is NULL.
*/
return &rt->u.dst;
}
Something like that.
^ permalink raw reply
* Re: [PATCH] l2tp: Fix a UDP socket reference count bug in the pppol2tp driver
From: James Chapman @ 2010-01-27 13:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100123.015511.83860202.davem@davemloft.net>
David Miller wrote:
> From: James Chapman <jchapman@katalix.com>
> Date: Thu, 21 Jan 2010 16:10:09 +0000
>
>> The bug can cause a kernel stack trace when a tunnel socket is closed.
>>
>> WARNING: at include/net/sock.h:435 udp_lib_unhash+0x117/0x120()
>> Pid: 1086, comm: openl2tpd Not tainted 2.6.33-rc1 #8
>> Call Trace:
>
> This fix doesn't look right at all.
>
> You grab one reference in connect() and then drop a reference
> every single recvmsg() call.
No, one ref is grabbed when the UDP socket is prepared for L2TP. Another
ref is grabbed while processing a skb in the receive path.
> recvmsg() calls to connect() would be many to one, so I can't
> see how this reference counting scheme could possibly work.
Perhaps you missed the sock_hold() in pppol2tp_sock_to_tunnel(), which
is called for every received skb in pppol2tp_recv_core()?
When userspace closes all session sockets in the tunnel, including the
special tunnel pppol2tp socket which has session_id==0, the ref on the
UDP tunnel socket is dropped, which allows it to be released.
> Why don't you describe the exact sequence of events that lead
> to the trace, so we can figure out how to correct this
> properly?
A way to reproduce the issue is to prepare the UDP socket for L2TP (by
opening a tunnel pppol2tp socket) and then close it before any L2TP
sessions are added to it. The sequence is
Create UDP socket
Create tunnel pppol2tp socket to prepare UDP socket for L2TP
pppol2tp_connect: session_id=0, peer_session_id=0
L2TP SCCRP control frame received (tunnel_id==0)
pppol2tp_recv_core: sock_hold()
pppol2tp_recv_core: sock_put
L2TP ZLB control frame received (tunnel_id=nnn)
pppol2tp_recv_core: sock_hold()
pppol2tp_recv_core: sock_put
Close tunnel management socket
pppol2tp_release: session_id=0, peer_session_id=0
Close UDP socket
udp_lib_close: BUG
The addition of sock_hold() in pppol2tp_connect() solves the problem.
For data frames, two sock_put() calls were added to plug a refcnt leak
per received data frame. The ref that is grabbed at the top of
pppol2tp_recv_core() must always be released, but this wasn't done for
accepted data frames or data frames discarded because of bad UDP
checksums. This leak meant that any UDP socket that had passed L2TP data
traffic (i.e. L2TP data frames, not just L2TP control frames) using
pppol2tp would not be released by the kernel.
Does the above help?
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ permalink raw reply
* Re: Network QoS support in applications
From: Benny Amorsen @ 2010-01-27 13:24 UTC (permalink / raw)
To: Dunc; +Cc: Kalle Valo, David Miller, kaber, netdev, linux-wireless
In-Reply-To: <4B5EF5DF.2070005@lemonia.org>
Dunc <dunc@lemonia.org> writes:
> If applications set the QoS values, the who's to stop someone (for
> example) writing a bittorrent client that marks all packets for the
> highest priority as if they were VoIP or something?
Nothing, but nothing stops them from writing a bittorrent client which
does "optimistic ACK" either. Yet noone seems to bother.
Also, bittorrent marked as EF could easily get hit by a "don't queue
voice packets, just drop instead" policy. Late voice packets are useless
and might as well be dropped, but TCP streams like bittorrent react
badly to packet loss.
All in all there is little incentive for people to game the system.
Should it happen anyway, network administrators have plentiful tools for
fixing it, up to and including ip link set dev whatever down.
/Benny
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: Jarek Poplawski @ 2010-01-27 13:28 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127.051410.74328462.davem@davemloft.net>
On Wed, Jan 27, 2010 at 05:14:10AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Wed, 27 Jan 2010 12:43:31 +0000
>
> > I'm only not sure there is a better way (comments?) to make this
> > code readable.
>
> You can make it more readable and even validate the invariant at the
> same time by adding some kind of inline function if you wish.
>
> static inline struct dst_entry *rtable_to_dst_entry(struct rtable *rt)
> {
> BUILD_BUG_ON(rt != &rt->u.dst);
>
> /* Because "->u.dst" is the first member of struct rtable,
> * this dereference is safe even when rt is NULL.
> */
> return &rt->u.dst;
> }
>
> Something like that.
Yes, it looks the most readable to me.
Thanks,
Jarek P.
^ permalink raw reply
* Re: [PATCH 2/4] ar7: implement clock API
From: Ralf Baechle @ 2010-01-27 13:54 UTC (permalink / raw)
To: Florian Fainelli; +Cc: linux-mips, Wim Van Sebroeck, netdev, David Miller
In-Reply-To: <201001270910.06224.florian@openwrt.org>
On Wed, Jan 27, 2010 at 09:10:06AM +0100, Florian Fainelli wrote:
> This patch makes the ar7 clock code implement the
> Linux clk API. Drivers using the various clocks
> available in the SoC are updated accordingly.
>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> Acked-by: Wim Van Sebroeck <wim@iguana.be>
Thanks; queued for 2.6.34.
Ralf
^ permalink raw reply
* Re: [PATCH] tcp: fix ICMP-RTO war
From: Denys Fedoryshchenko @ 2010-01-27 13:56 UTC (permalink / raw)
To: Ilpo Järvinen, Netdev, Damian Lukowski, David Miller
On Wednesday 27 January 2010 14:36:18 you wrote:
> Unless they are for a different connection? We might have to print sk (%p)
> in all those printouts to be sure which maps to which. If a peer becomes
> unreachable, it may well have multiple connections open (this was a
> proxy, iirc?).
>
Ok i will try to do that today.
Most probably different connections, on this proxy i have 10-15k established
connections at peak time.
Also about estimating rtt, maybe there is something wrong in idea. Just what
will happen if ip visible for proxy have multiple people behind? It can be
small router for hotspot and 10-20 people behind it, some with very large
rtt, some with very small.
And some of them getting disconnected (out of range from wireless, for
example), and thats most probably why host unreachable sent... at same time
proxy have established and running tcp connections with other people on same
router (means for proxy same ip).
Sorry, again to Ilpo Järvinen, by default i have reply, instead of reply all.
^ permalink raw reply
* [PATCH] sctp: IPsec rules are ineffective with ipv6
From: Nicolas Dichtel @ 2010-01-27 14:12 UTC (permalink / raw)
To: netdev, Vlad Yasevich, linux-sctp
[-- Attachment #1: Type: text/plain, Size: 165 bytes --]
xfrm_lookup() is missing in sctp_v6_xmit(), add it.
Signed-off-by: Junwei Zhang <junwei.zhang@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
[-- Attachment #2: x.diff --]
[-- Type: text/x-diff, Size: 708 bytes --]
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index cc50fbe..f24e23c 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -197,8 +197,10 @@ out:
static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
{
struct sock *sk = skb->sk;
+ struct dst_entry *dst = skb_dst(skb);
struct ipv6_pinfo *np = inet6_sk(sk);
struct flowi fl;
+ int err;
memset(&fl, 0, sizeof(fl));
@@ -231,6 +233,9 @@ static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
if (!(transport->param_flags & SPP_PMTUD_ENABLE))
skb->local_df = 1;
+ if ((err = xfrm_lookup(sock_net(sk), &dst, &fl, sk, 0)) < 0)
+ return err;
+
return ip6_xmit(sk, skb, &fl, np->opt, 0);
}
^ permalink raw reply related
* occasional netdev watchdog timeouts with sungem on Apple G5
From: Mikael Pettersson @ 2010-01-27 14:21 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
For a few months now I've been getting very occasional netdev
watchdog timeouts on the built-in sungem ethernet in my G5. I
can't say exactly when it started, but probably post-2.6.30.
The kernel always recovers so it's not a big problem, but I find it
a bit alarming since I never see this with any other ethernet driver.
dmesg from 2.6.33-rc4:
sungem.c:v0.98 8/24/03 David S. Miller (davem@redhat.com)
PHY ID: 2062e0, addr: 1
eth0: Sun GEM (PCI) 10/100/1000BaseT Ethernet 00:0a:95:aa:66:50
eth0: Found BCM5421-K2 PHY
...
eth0: Link is up at 100 Mbps, full-duplex.
...
eth0: Pause is enabled (rxfifo: 10240 off: 7168 on: 5632)
...
NETDEV WATCHDOG: eth0 (gem): transmit queue 0 timed out
------------[ cut here ]------------
Badness at net/sched/sch_generic.c:255
NIP: c0000000002812ac LR: c0000000002812a8 CTR: 0000000000000001
REGS: c0000001f3513750 TRAP: 0700 Not tainted (2.6.33-rc4)
MSR: 9000000000029032 <EE,ME,CE,IR,DR> CR: 28000484 XER: 20000000
TASK = c0000001f6390000[2856] 'sshd' THREAD: c0000001f3510000 CPU: 1
GPR00: c0000000002812a8 c0000001f35139d0 c000000000457ed0 000000000000003e
GPR04: 0000000000000000 ffffffffffffffff 0000000000000000 000000000001ffff
GPR08: c0000000004788b8 c0000000003c8f08 c0000000004788b8 0000000008bb9000
GPR12: 0000000028000482 c000000000474680 0000000020103670 00000000806595c4
GPR16: 00000000631b65ac 0000000000000100 c00000000044a740 c00000000045d300
GPR20: c0000001f60d5020 c0000001f60d5420 c0000001f60d5820 ffffffffffffffff
GPR24: 0000000000000001 0000000000000000 0000000000000000 0000000000000002
GPR28: 0000000000000001 0000000000000000 c000000000412258 c0000001f4750000
NIP [c0000000002812ac] .dev_watchdog+0x1d0/0x310
LR [c0000000002812a8] .dev_watchdog+0x1cc/0x310
Call Trace:
[c0000001f35139d0] [c0000000002812a8] .dev_watchdog+0x1cc/0x310 (unreliable)
[c0000001f3513ad0] [c000000000054890] .run_timer_softirq+0x1c4/0x2a0
[c0000001f3513bd0] [c00000000004e870] .__do_softirq+0xf0/0x1d0
[c0000001f3513ca0] [c00000000000bbc0] .do_softirq+0x50/0x94
[c0000001f3513d20] [c00000000004e5c0] .irq_exit+0x54/0xb0
[c0000001f3513da0] [c00000000001be6c] .timer_interrupt+0xb4/0xe0
[c0000001f3513e30] [c000000000003610] decrementer_common+0x110/0x180
Instruction dump:
41fe0040 38810070 7fe3fb78 38a00040 4bfe96b1 60000000 7fa6eb78 7fe4fb78
7c651b78 e87e8028 48054ee5 60000000 <0fe00000> e93e8020 38000001 98090000
eth0: transmit timed out, resetting
eth0: TX_STATE[003ffc05:00000001:0000001f]
eth0: RX_STATE[0100c805:00000001:00000021]
eth0: Link is up at 1000 Mbps, full-duplex.
eth0: Pause is enabled (rxfifo: 10240 off: 7168 on: 5632)
eth0: transmit timed out, resetting
eth0: TX_STATE[00000001:00000000:00000001]
eth0: RX_STATE[00064801:00000000:00000001]
eth0: Link is up at 100 Mbps, full-duplex.
eth0: Pause is enabled (rxfifo: 10240 off: 7168 on: 5632)
eth0: transmit timed out, resetting
eth0: TX_STATE[003ffc05:00000001:0000001f]
eth0: RX_STATE[0100c805:00000001:00000021]
eth0: Link is up at 100 Mbps, full-duplex.
eth0: Pause is enabled (rxfifo: 10240 off: 7168 on: 5632)
lspci -v:
0000:f0:0b.0 Host bridge: Apple Computer Inc. U3 AGP
Flags: bus master, 66MHz, medium devsel, latency 16
Capabilities: <access denied>
0000:f0:10.0 VGA compatible controller: nVidia Corporation NV34 [GeForce FX 5200 Ultra] (rev a1) (prog-if 00 [VGA controller])
Subsystem: nVidia Corporation Unknown device 0010
Flags: bus master, 66MHz, medium devsel, latency 16, IRQ 48
Memory at a1000000 (32-bit, non-prefetchable) [size=16M]
Memory at a8000000 (32-bit, prefetchable) [size=128M]
Expansion ROM at a0000000 [disabled] [size=128K]
Capabilities: <access denied>
0001:00:00.0 Host bridge: Apple Computer Inc. Unknown device 004a
Flags: bus master, fast devsel, latency 0
Capabilities: <access denied>
0001:00:01.0 PCI bridge: Advanced Micro Devices [AMD] AMD-8131 PCI-X Bridge (rev 12) (prog-if 00 [Normal decode])
Flags: bus master, 66MHz, medium devsel, latency 32
Bus: primary=00, secondary=06, subordinate=06, sec-latency=32
Memory behind bridge: 90000000-900fffff
Capabilities: <access denied>
0001:00:02.0 PCI bridge: Advanced Micro Devices [AMD] AMD-8131 PCI-X Bridge (rev 12) (prog-if 00 [Normal decode])
Flags: bus master, 66MHz, medium devsel, latency 32
Bus: primary=00, secondary=07, subordinate=07, sec-latency=32
Capabilities: <access denied>
0001:00:03.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=01, subordinate=01, sec-latency=32
Memory behind bridge: 80000000-800fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:00:04.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=02, subordinate=02, sec-latency=32
Memory behind bridge: 80100000-801fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:00:05.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=03, subordinate=03, sec-latency=32
Memory behind bridge: 80200000-802fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:00:06.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=04, subordinate=04, sec-latency=32
Memory behind bridge: 80300000-805fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:00:07.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=05, subordinate=05, sec-latency=32
Memory behind bridge: 80600000-806fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:01:07.0 Class ff00: Apple Computer Inc. K2 KeyLargo Mac/IO (rev 20)
Flags: bus master, medium devsel, latency 16
Memory at 80000000 (32-bit, non-prefetchable) [size=512K]
0001:01:08.0 USB Controller: Apple Computer Inc. K2 KeyLargo USB (prog-if 10 [OHCI])
Flags: bus master, medium devsel, latency 16, IRQ 27
Memory at 80081000 (32-bit, non-prefetchable) [size=4K]
0001:01:09.0 USB Controller: Apple Computer Inc. K2 KeyLargo USB (prog-if 10 [OHCI])
Flags: bus master, medium devsel, latency 16, IRQ 28
Memory at 80080000 (32-bit, non-prefetchable) [size=4K]
0001:02:0b.0 USB Controller: NEC Corporation USB (rev 43) (prog-if 10 [OHCI])
Subsystem: NEC Corporation Hama USB 2.0 CardBus
Flags: bus master, medium devsel, latency 16, IRQ 63
Memory at 80102000 (32-bit, non-prefetchable) [size=4K]
Capabilities: <access denied>
0001:02:0b.1 USB Controller: NEC Corporation USB (rev 43) (prog-if 10 [OHCI])
Subsystem: NEC Corporation Hama USB 2.0 CardBus
Flags: bus master, medium devsel, latency 16, IRQ 63
Memory at 80101000 (32-bit, non-prefetchable) [size=4K]
Capabilities: <access denied>
0001:02:0b.2 USB Controller: NEC Corporation USB 2.0 (rev 04) (prog-if 20 [EHCI])
Subsystem: NEC Corporation USB 2.0
Flags: bus master, medium devsel, latency 16, IRQ 63
Memory at 80100000 (32-bit, non-prefetchable) [size=256]
Capabilities: <access denied>
0001:03:0d.0 Class ff00: Apple Computer Inc. K2 ATA/100
Flags: bus master, medium devsel, latency 32, IRQ 39
Memory at 80204000 (32-bit, non-prefetchable) [size=16K]
0001:03:0e.0 FireWire (IEEE 1394): Apple Computer Inc. K2 FireWire (prog-if 10 [OHCI])
Subsystem: Apple Computer Inc. Unknown device 5811
Flags: bus master, medium devsel, latency 248, IRQ 40
Memory at 80200000 (32-bit, non-prefetchable) [size=4K]
Capabilities: <access denied>
0001:04:0f.0 Ethernet controller: Apple Computer Inc. K2 GMAC (Sun GEM)
Flags: bus master, 66MHz, slow devsel, latency 16, IRQ 41
Memory at 80400000 (32-bit, non-prefetchable) [size=2M]
Expansion ROM at 80300000 [disabled] [size=1M]
0001:05:0c.0 IDE interface: Broadcom K2 SATA (prog-if 8f [Master SecP SecO PriP PriO])
Subsystem: Broadcom K2 SATA
Flags: bus master, medium devsel, latency 16, IRQ 16
I/O ports at <unassigned> [disabled]
I/O ports at <unassigned> [disabled]
I/O ports at <unassigned> [disabled]
I/O ports at <unassigned> [disabled]
I/O ports at <unassigned> [disabled]
Memory at 80600000 (32-bit, non-prefetchable) [size=8K]
0001:06:03.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5701 Gigabit Ethernet (rev 15)
Subsystem: Compaq Computer Corporation NC7770 Gigabit Server Adapter (PCI-X, 10/100/1000-T)
Flags: bus master, 66MHz, medium devsel, latency 16, IRQ 53
Memory at 90010000 (64-bit, non-prefetchable) [size=64K]
[virtual] Expansion ROM at 90000000 [disabled] [size=64K]
Capabilities: <access denied>
Just FYI, in case a pattern emerges etc.
/Mikael
^ permalink raw reply
* [PATCH 1/3] sky2: Factor out code to calculate packet sizes
From: Mike McCormack @ 2010-01-27 15:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Move code to calculate receive threshold and packet size out of
sky2_rx_start() so that is can be called from elsewhere easily.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
drivers/net/sky2.c | 57 +++++++++++++++++++++++++++++++++------------------
1 files changed, 37 insertions(+), 20 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 0b47c7f..2061eb8 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1064,6 +1064,40 @@ static inline struct sky2_rx_le *sky2_next_rx(struct sky2_port *sky2)
return le;
}
+static unsigned sky2_get_rx_threshold(struct sky2_port* sky2)
+{
+ unsigned size;
+
+ /* Space needed for frame data + headers rounded up */
+ size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
+
+ /* Stopping point for hardware truncation */
+ return (size - 8) / sizeof(u32);
+}
+
+static unsigned sky2_get_rx_data_size(struct sky2_port* sky2)
+{
+ struct rx_ring_info *re;
+ unsigned size;
+
+ /* Space needed for frame data + headers rounded up */
+ size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
+
+ sky2->rx_nfrags = size >> PAGE_SHIFT;
+ BUG_ON(sky2->rx_nfrags > ARRAY_SIZE(re->frag_addr));
+
+ /* Compute residue after pages */
+ size -= sky2->rx_nfrags << PAGE_SHIFT;
+
+ /* Optimize to handle small packets and headers */
+ if (size < copybreak)
+ size = copybreak;
+ if (size < ETH_HLEN)
+ size = ETH_HLEN;
+
+ return size;
+}
+
/* Build description to hardware for one receive segment */
static void sky2_rx_add(struct sky2_port *sky2, u8 op,
dma_addr_t map, unsigned len)
@@ -1337,7 +1371,7 @@ static int sky2_rx_start(struct sky2_port *sky2)
struct sky2_hw *hw = sky2->hw;
struct rx_ring_info *re;
unsigned rxq = rxqaddr[sky2->port];
- unsigned i, size, thresh;
+ unsigned i, thresh;
sky2->rx_put = sky2->rx_next = 0;
sky2_qset(hw, rxq);
@@ -1358,25 +1392,7 @@ static int sky2_rx_start(struct sky2_port *sky2)
if (!(hw->flags & SKY2_HW_NEW_LE))
rx_set_checksum(sky2);
- /* Space needed for frame data + headers rounded up */
- size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
-
- /* Stopping point for hardware truncation */
- thresh = (size - 8) / sizeof(u32);
-
- sky2->rx_nfrags = size >> PAGE_SHIFT;
- BUG_ON(sky2->rx_nfrags > ARRAY_SIZE(re->frag_addr));
-
- /* Compute residue after pages */
- size -= sky2->rx_nfrags << PAGE_SHIFT;
-
- /* Optimize to handle small packets and headers */
- if (size < copybreak)
- size = copybreak;
- if (size < ETH_HLEN)
- size = ETH_HLEN;
-
- sky2->rx_data_size = size;
+ sky2->rx_data_size = sky2_get_rx_data_size(sky2);
/* Fill Rx ring */
for (i = 0; i < sky2->rx_pending; i++) {
@@ -1401,6 +1417,7 @@ static int sky2_rx_start(struct sky2_port *sky2)
* the register is limited to 9 bits, so if you do frames > 2052
* you better get the MTU right!
*/
+ thresh = sky2_get_rx_threshold(sky2);
if (thresh > 0x1ff)
sky2_write32(hw, SK_REG(sky2->port, RX_GMF_CTRL_T), RX_TRUNC_OFF);
else {
--
1.5.6.5
^ permalink raw reply related
* [PATCH 2/3] sky2: Allocate initial skbs in sky2_alloc_buffers
From: Mike McCormack @ 2010-01-27 15:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Allocating everything in one place means there's a single point
of failure in sky2_up, and sky2_rx_start can no longer fail.
This also fixes a memory leak in the case that sky2_rx_start
fails in the middle of allocating skbs, since any allocated
skbs will not be free'd in sky2_up's failure path.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
drivers/net/sky2.c | 42 +++++++++++++++++++++++-------------------
1 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 2061eb8..a967912 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1358,7 +1358,7 @@ static inline void sky2_rx_update(struct sky2_port *sky2, unsigned rxq)
}
/*
- * Allocate and setup receiver buffer pool.
+ * Setup receiver buffer pool.
* Normal case this ends up creating one list element for skb
* in the receive ring. Worst case if using large MTU and each
* allocation falls on a different 64 bit region, that results
@@ -1392,22 +1392,10 @@ static int sky2_rx_start(struct sky2_port *sky2)
if (!(hw->flags & SKY2_HW_NEW_LE))
rx_set_checksum(sky2);
- sky2->rx_data_size = sky2_get_rx_data_size(sky2);
- /* Fill Rx ring */
+ /* submit Rx ring */
for (i = 0; i < sky2->rx_pending; i++) {
re = sky2->rx_ring + i;
-
- re->skb = sky2_rx_alloc(sky2);
- if (!re->skb)
- goto nomem;
-
- if (sky2_rx_map_skb(hw->pdev, re, sky2->rx_data_size)) {
- dev_kfree_skb(re->skb);
- re->skb = NULL;
- goto nomem;
- }
-
sky2_rx_submit(sky2, re);
}
@@ -1453,14 +1441,12 @@ static int sky2_rx_start(struct sky2_port *sky2)
return 0;
-nomem:
- sky2_rx_clean(sky2);
- return -ENOMEM;
}
static int sky2_alloc_buffers(struct sky2_port *sky2)
{
struct sky2_hw *hw = sky2->hw;
+ unsigned i;
/* must be power of 2 */
sky2->tx_le = pci_alloc_consistent(hw->pdev,
@@ -1486,6 +1472,24 @@ static int sky2_alloc_buffers(struct sky2_port *sky2)
if (!sky2->rx_ring)
goto nomem;
+ sky2->rx_data_size = sky2_get_rx_data_size(sky2);
+
+ /* Fill Rx ring */
+ for (i = 0; i < sky2->rx_pending; i++) {
+ struct rx_ring_info *re = sky2->rx_ring + i;
+
+ re->skb = sky2_rx_alloc(sky2);
+ if (!re->skb)
+ goto nomem;
+
+ if (sky2_rx_map_skb(hw->pdev, re, sky2->rx_data_size)) {
+ dev_kfree_skb(re->skb);
+ re->skb = NULL;
+ goto nomem;
+ }
+ }
+
+
return 0;
nomem:
return -ENOMEM;
@@ -1495,6 +1499,8 @@ static void sky2_free_buffers(struct sky2_port *sky2)
{
struct sky2_hw *hw = sky2->hw;
+ sky2_rx_clean(sky2);
+
if (sky2->rx_le) {
pci_free_consistent(hw->pdev, RX_LE_BYTES,
sky2->rx_le, sky2->rx_le_map);
@@ -1953,8 +1959,6 @@ static int sky2_down(struct net_device *dev)
/* Free any pending frames stuck in HW queue */
sky2_tx_complete(sky2, sky2->tx_prod);
- sky2_rx_clean(sky2);
-
sky2_free_buffers(sky2);
return 0;
--
1.5.6.5
^ permalink raw reply related
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