Netdev List
 help / color / mirror / Atom feed
* [PATCH] Au1000 eth : fix ioctl handling
From: Florian Fainelli @ 2007-07-24  9:15 UTC (permalink / raw)
  To: netdev


[-- Attachment #1.1: Type: text/plain, Size: 167 bytes --]

Hi all,

This patch fixes the handling of unsupported ioctls with the au1000_eth 
driver.

Signed-off-by: Florian Fainelli <florian.fainelli@telecomint.eu>
-- 

[-- Attachment #1.2: au1000_eth_ioctl.patch --]
[-- Type: text/plain, Size: 818 bytes --]

diff --git a/drivers/net/au1000_eth.c b/drivers/net/au1000_eth.c
index c27cfce..99a1c61 100644
--- a/drivers/net/au1000_eth.c
+++ b/drivers/net/au1000_eth.c
@@ -1316,12 +1316,20 @@ static void set_rx_mode(struct net_device *dev)
 static int au1000_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 {
 	struct au1000_private *aup = (struct au1000_private *)dev->priv;
+	struct mii_ioctl_data *data = if_mii(rq);
+	int rc = -EOPNOTSUPP;
 
 	if (!netif_running(dev)) return -EINVAL;
 
 	if (!aup->phy_dev) return -EINVAL; // PHY not controllable
 
-	return phy_mii_ioctl(aup->phy_dev, if_mii(rq), cmd);
+	switch (cmd) {
+	default:
+		rc = phy_mii_ioctl(aup->phy_dev, data, cmd);
+		break;
+	}
+
+	return rc;
 }
 
 static struct net_device_stats *au1000_get_stats(struct net_device *dev)

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply related

* Re: Races in net_rx_action vs netpoll?
From: Jarek Poplawski @ 2007-07-24 10:26 UTC (permalink / raw)
  To: David Miller; +Cc: shemminger, okir, netdev
In-Reply-To: <20070722.000507.26966710.davem@davemloft.net>

On 22-07-2007 09:05, David Miller wrote:
> From: Stephen Hemminger <shemminger@linux-foundation.org>
> Date: Thu, 19 Jul 2007 17:27:47 +0100
> 
>> Please revisit the requirements that netconsole needs and redesign
>> it from scratch.  The existing code is causing too much breakage.
>>
>> Can it be done without breaking the semantics of network devices, or
>> should we rewrite the driver interface to take have a different
>> interface like netdev_sync_send_skb() that is slow, synchronous and
>> non-interrupt (ie polls for completion).  Of course, then people
>> will complain that netconsole traffic slows the machine down.  for
>> completion.
> 
> I couldn't agree more.
> 
> Since netpoll runs outside of all of the normal netdevice locking
> rules, only the people using netpoll hit all the bugs.  That means
> most of us do not test out these code path, which is bad.
> 
> So, if anything, a more integrated implementation is essential.
> 

But, IMHO, until this will be done some simpler measures could
do be done to make poll_napi less dangerous (as a matter of fact
I wonder why oopses observed & diagnosed by Olaf are so rare).

Current locking with netpoll_poll_lock is mainly misleading.
It seems somebody who planned napi didn't even think such helpers
as poll_napi are possible on other cpus and somebody doing netpoll
didn't want to show this all per cpu data needs full locking anyway.
But since it's like this there is no reason to invent a wheel again
and "normal" locking should be done: so global (not per device)
spin_lock (#ifdef CONFIG_NETPOLL only) held during all net_rx_action
and spin_trylock similarly in poll_napi (with STATE_RX_SCHED
re-checking) should be minimum needed here.

Regards,
Jarek P.

^ permalink raw reply

* Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
From: Mike Galbraith @ 2007-07-24 10:01 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Alexey Dobriyan, Linus Torvalds, linux-kernel, netdev
In-Reply-To: <20070723132431.42afbae8.akpm@linux-foundation.org>

On Mon, 2007-07-23 at 13:24 -0700, Andrew Morton wrote:

> You're using DEBUG_PAGEALLOC, but I was not, so I think we can rule that out.

My box bugged during boot the first time I booted 23-rc1, but nothing
made it to the console, and I didn't have a serial console running.  I
didn't have DEBUG_PAGEALLOC or friends set.

> I haven't worked out where that kmap_atomic() call is coming from yet. 
> Both traces point up into the page allocator, but I _think_ that's stack
> gunk.

I just enabled all debug options, and was just rewarded with the below.

[  119.079531] eth1: link up, 100Mbps, full-duplex, lpa 0x45E1
[  119.558867] ------------[ cut here ]------------
[  119.572197] kernel BUG at arch/i386/mm/highmem.c:38!
[  119.585804] invalid opcode: 0000 [#1]
[  119.598013] PREEMPT SMP DEBUG_PAGEALLOC
[  119.610103] Modules linked in: edd button battery ac ip6t_REJECT xt_tcpudp ipt_REJECT xt_state iptable_mangle iptable_nat nf_nat iptable_filter ip6table_mangle nf_conntrack_ipv4 nf_conntrack nfnetlink ip_tables ip6table_filter ip6_tables x_tables nls_iso8859_1 nls_cp437 nls_utf8 snd_intel8x0 snd_ac97_codec ac97_bus snd_mpu401 snd_pcm prism54 snd_timer snd_mpu401_uart snd_rawmidi snd_seq_device snd intel_agp agpgart soundcore snd_page_alloc i2c_i801 fan thermal processor
[  119.698063] CPU:    1
[  119.698065] EIP:    0060:[<c011cd2d>]    Not tainted VLI
[  119.698067] EFLAGS: 00010006   (2.6.23-rc1-smp #75)
[  119.736358] EIP is at kmap_atomic_prot+0xa7/0xab
[  119.749647] eax: 3d07f163   ebx: c166db80   ecx: c0750e60   edx: 00000007
[  119.765417] esi: 00000022   edi: 00000163   ebp: c069dcd4   esp: c069dcc8
[  119.781273] ds: 007b   es: 007b   fs: 00d8  gs: 0033  ss: 0068
[  119.796378] Process udevd (pid: 4775, ti=c069d000 task=f31aea60 task.ti=f477d000)
[  119.804068] Stack: c166db80 00000000 c166db80 c069dcdc c011cd3f c069dd40 c015b6e0 00000001 
[  119.822272]        00000044 00000163 00000000 00000001 c165f4e0 00000001 c165f4e0 00000001 
[  119.840762]        00000000 00028020 c061e71c c166db80 00000046 00000080 00000001 c011e4de 
[  119.859389] Call Trace:
[  119.881302]  [<c0105144>] show_trace_log_lvl+0x1a/0x30
[  119.896319]  [<c01051ff>] show_stack_log_lvl+0xa5/0xca
[  119.911171]  [<c0105420>] show_registers+0x1fc/0x343
[  119.925756]  [<c0105689>] die+0x122/0x249
[  119.939241]  [<c0105834>] do_trap+0x84/0xad
[  119.952897]  [<c0105b1c>] do_invalid_op+0x88/0x92
[  119.967118]  [<c04cf3c2>] error_code+0x72/0x78
[  119.980948]  [<c011cd3f>] kmap_atomic+0xe/0x10
[  119.994642]  [<c015b6e0>] get_page_from_freelist+0x39e/0x45e
[  120.009485]  [<c015b7fb>] __alloc_pages+0x5b/0x2db
[  120.023342]  [<c0172872>] cache_alloc_refill+0x380/0x6f2
[  120.037623]  [<c0172e7a>] kmem_cache_alloc+0xa1/0xa5
[  120.051426]  [<c03fb397>] neigh_create+0x5f/0x506
[  120.064894]  [<c046e25d>] ndisc_dst_alloc+0x122/0x151
[  120.078769]  [<c0471b0b>] __ndisc_send+0x8d/0x4fa
[  120.092340]  [<c0472915>] ndisc_send_ns+0x5f/0x7d
[  120.105848]  [<c0469ff5>] addrconf_dad_timer+0xdb/0xe0
[  120.119758]  [<c012f8a0>] run_timer_softirq+0x130/0x191
[  120.133717]  [<c012c06d>] __do_softirq+0x76/0xe4
[  120.147475]  [<c0106b48>] do_softirq+0x63/0xac
[  120.147488]  [<c012bff5>] 

(gdb) list *neigh_create+0x5f
0xc03fb397 is in neigh_create (include/linux/slab.h:259).
254     /*
255      * Shortcuts
256      */
257     static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
258     {
259             return kmem_cache_alloc(k, flags | __GFP_ZERO);
260     }
261
262     /**
263      * kzalloc - allocate memory. The memory is set to zero.
(gdb) list *kmem_cache_alloc+0xa1
0xc0172e7a is in kmem_cache_alloc (mm/slab.c:3176).
3171                    STATS_INC_ALLOCHIT(cachep);
3172                    ac->touched = 1;
3173                    objp = ac->entry[--ac->avail];
3174            } else {
3175                    STATS_INC_ALLOCMISS(cachep);
3176                    objp = cache_alloc_refill(cachep, flags);
3177            }
3178            return objp;
3179    }
3180
(gdb) list *cache_alloc_refill+0x380
0xc0172872 is in cache_alloc_refill (include/linux/gfp.h:154).
149
150             /* Unknown node is current node */
151             if (nid < 0)
152                     nid = numa_node_id();
153
154             return __alloc_pages(gfp_mask, order,
155                     NODE_DATA(nid)->node_zonelists + gfp_zone(gfp_mask));
156     }
157
158     #ifdef CONFIG_NUMA
(gdb) list *__alloc_pages+0x5b
0xc015b7fb is in __alloc_pages (mm/page_alloc.c:1248).
1243            if (unlikely(*z == NULL)) {
1244                    /* Should this ever happen?? */
1245                    return NULL;
1246            }
1247
1248            page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, order,
1249                                    zonelist, ALLOC_WMARK_LOW|ALLOC_CPUSET);
1250            if (page)
1251                    goto got_pg;
1252
(gdb) list *get_page_from_freelist+0x39e
0xc015b6e0 is in get_page_from_freelist (include/linux/highmem.h:122).
117             return __alloc_zeroed_user_highpage(__GFP_MOVABLE, vma, vaddr);
118     }
119
120     static inline void clear_highpage(struct page *page)
121     {
122             void *kaddr = kmap_atomic(page, KM_USER0);
123             clear_page(kaddr);
124             kunmap_atomic(kaddr, KM_USER0);
125     }
126




^ permalink raw reply

* Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
From: Mike Galbraith @ 2007-07-24 10:37 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Alexey Dobriyan, Linus Torvalds, linux-kernel, netdev
In-Reply-To: <1185271269.6479.7.camel@Homer.simpson.net>

On Tue, 2007-07-24 at 12:01 +0200, Mike Galbraith wrote:
> On Mon, 2007-07-23 at 13:24 -0700, Andrew Morton wrote:
> 
> > You're using DEBUG_PAGEALLOC, but I was not, so I think we can rule that out.
> 
> My box bugged during boot the first time I booted 23-rc1, but nothing
> made it to the console, and I didn't have a serial console running.  I
> didn't have DEBUG_PAGEALLOC or friends set.
> 
> > I haven't worked out where that kmap_atomic() call is coming from yet. 
> > Both traces point up into the page allocator, but I _think_ that's stack
> > gunk.
> 
> I just enabled all debug options, and was just rewarded with the below.

Hm.  I just also experienced filesystem corruption when I tried to send
from that kernel, and it bugged in the process.  My mount table ended up
in /etc/resolv.conf along with some binary goop, making nscd rather
unhappy after reboot.  fsck time.

	.Mike


^ permalink raw reply

* Re: [GENETLINK]: Question: global lock (genl_mutex) possible refinement?
From: Richard MUSIL @ 2007-07-24 11:09 UTC (permalink / raw)
  To: Thomas Graf; +Cc: Patrick McHardy, netdev
In-Reply-To: <20070724093558.GB9285@postel.suug.ch>

Thomas Graf wrote:
> Please provide a new overall patch which is not based on your
> initial patch so I can review your idea properly.

Here it goes (merging two previous patches). I have diffed
against v2.6.22, which I am using currently as my base:

 include/net/genetlink.h |    1 +
 net/netlink/genetlink.c |  106 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 80 insertions(+), 27 deletions(-)

diff --git a/include/net/genetlink.h b/include/net/genetlink.h
index b6eaca1..681ad13 100644
--- a/include/net/genetlink.h
+++ b/include/net/genetlink.h
@@ -25,6 +25,7 @@ struct genl_family
 	struct nlattr **	attrbuf;	/* private */
 	struct list_head	ops_list;	/* private */
 	struct list_head	family_list;	/* private */
+	struct mutex		lock;		/* private */
 };
 
 /**
diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index b9ab62f..0104267 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -38,6 +38,32 @@ static void genl_unlock(void)
 		genl_sock->sk_data_ready(genl_sock, 0);
 }
 
+static DEFINE_MUTEX(genl_fam_mutex);	/* serialization for family list management */
+
+static inline void genl_fam_lock(struct genl_family *family)
+{
+	mutex_lock(&genl_fam_mutex);
+	if (family)
+		mutex_lock(&family->lock);
+}
+
+static inline void genl_fam_unlock(struct genl_family *family)
+{
+	if (family)
+		mutex_unlock(&family->lock);
+	mutex_unlock(&genl_fam_mutex);
+}
+
+static inline void genl_onefam_lock(struct genl_family *family)
+{
+	mutex_lock(&family->lock);
+}
+
+static inline void genl_onefam_unlock(struct genl_family *family)
+{
+	mutex_unlock(&family->lock);
+}
+
 #define GENL_FAM_TAB_SIZE	16
 #define GENL_FAM_TAB_MASK	(GENL_FAM_TAB_SIZE - 1)
 
@@ -150,9 +176,9 @@ int genl_register_ops(struct genl_family *family, struct genl_ops *ops)
 	if (ops->policy)
 		ops->flags |= GENL_CMD_CAP_HASPOL;
 
-	genl_lock();
+	genl_fam_lock(family);
 	list_add_tail(&ops->ops_list, &family->ops_list);
-	genl_unlock();
+	genl_fam_unlock(family);
 
 	genl_ctrl_event(CTRL_CMD_NEWOPS, ops);
 	err = 0;
@@ -180,16 +206,16 @@ int genl_unregister_ops(struct genl_family *family, struct genl_ops *ops)
 {
 	struct genl_ops *rc;
 
-	genl_lock();
+	genl_fam_lock(family);
 	list_for_each_entry(rc, &family->ops_list, ops_list) {
 		if (rc == ops) {
 			list_del(&ops->ops_list);
-			genl_unlock();
+			genl_fam_unlock(family);
 			genl_ctrl_event(CTRL_CMD_DELOPS, ops);
 			return 0;
 		}
 	}
-	genl_unlock();
+	genl_fam_unlock(family);
 
 	return -ENOENT;
 }
@@ -216,8 +242,9 @@ int genl_register_family(struct genl_family *family)
 		goto errout;
 
 	INIT_LIST_HEAD(&family->ops_list);
+	mutex_init(&family->lock);
 
-	genl_lock();
+	genl_fam_lock(family);
 
 	if (genl_family_find_byname(family->name)) {
 		err = -EEXIST;
@@ -251,14 +278,14 @@ int genl_register_family(struct genl_family *family)
 		family->attrbuf = NULL;
 
 	list_add_tail(&family->family_list, genl_family_chain(family->id));
-	genl_unlock();
+	genl_fam_unlock(family);
 
 	genl_ctrl_event(CTRL_CMD_NEWFAMILY, family);
 
 	return 0;
 
 errout_locked:
-	genl_unlock();
+	genl_fam_unlock(family);
 errout:
 	return err;
 }
@@ -275,7 +302,7 @@ int genl_unregister_family(struct genl_family *family)
 {
 	struct genl_family *rc;
 
-	genl_lock();
+	genl_fam_lock(family);
 
 	list_for_each_entry(rc, genl_family_chain(family->id), family_list) {
 		if (family->id != rc->id || strcmp(rc->name, family->name))
@@ -283,14 +310,16 @@ int genl_unregister_family(struct genl_family *family)
 
 		list_del(&rc->family_list);
 		INIT_LIST_HEAD(&family->ops_list);
-		genl_unlock();
+
+		genl_fam_unlock(family);
+		mutex_destroy(&family->lock);
 
 		kfree(family->attrbuf);
 		genl_ctrl_event(CTRL_CMD_DELFAMILY, family);
 		return 0;
 	}
 
-	genl_unlock();
+	genl_fam_unlock(family);
 
 	return -ENOENT;
 }
@@ -303,38 +332,57 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	struct genlmsghdr *hdr = nlmsg_data(nlh);
 	int hdrlen, err;
 
+	genl_fam_lock(NULL);
 	family = genl_family_find_byid(nlh->nlmsg_type);
-	if (family == NULL)
+	if (family == NULL) {
+		genl_fam_unlock(NULL);
 		return -ENOENT;
+	}
+
+	/* get particular family lock, but release global family lock
+	 * so registering operations for other families are possible */
+	genl_onefam_lock(family);
+	genl_fam_unlock(NULL);
 
 	hdrlen = GENL_HDRLEN + family->hdrsize;
-	if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen))
-		return -EINVAL;
+	if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen)) {
+		err = -EINVAL;
+		goto unlock_out;
+	}
 
 	ops = genl_get_cmd(hdr->cmd, family);
-	if (ops == NULL)
-		return -EOPNOTSUPP;
+	if (ops == NULL) {
+		err = -EOPNOTSUPP;
+		goto unlock_out;
+	}
 
 	if ((ops->flags & GENL_ADMIN_PERM) &&
-	    security_netlink_recv(skb, CAP_NET_ADMIN))
-		return -EPERM;
+	    security_netlink_recv(skb, CAP_NET_ADMIN)) {
+		err = -EPERM;
+		goto unlock_out;
+	}
 
 	if (nlh->nlmsg_flags & NLM_F_DUMP) {
-		if (ops->dumpit == NULL)
-			return -EOPNOTSUPP;
+		if (ops->dumpit == NULL) {
+			err = -EOPNOTSUPP;
+			goto unlock_out;
+		}
 
-		return netlink_dump_start(genl_sock, skb, nlh,
+		err = netlink_dump_start(genl_sock, skb, nlh,
 					  ops->dumpit, ops->done);
+		goto unlock_out;
 	}
 
-	if (ops->doit == NULL)
-		return -EOPNOTSUPP;
+	if (ops->doit == NULL) {
+		err = -EOPNOTSUPP;
+		goto unlock_out;
+	}
 
 	if (family->attrbuf) {
 		err = nlmsg_parse(nlh, hdrlen, family->attrbuf, family->maxattr,
 				  ops->policy);
 		if (err < 0)
-			return err;
+			goto unlock_out;
 	}
 
 	info.snd_seq = nlh->nlmsg_seq;
@@ -344,7 +392,11 @@ static int genl_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 	info.userhdr = nlmsg_data(nlh) + GENL_HDRLEN;
 	info.attrs = family->attrbuf;
 
-	return ops->doit(skb, &info);
+	err = ops->doit(skb, &info);
+
+unlock_out:
+	genl_onefam_unlock(family);
+	return err;
 }
 
 static void genl_rcv(struct sock *sk, int len)
@@ -425,7 +477,7 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb)
 	int fams_to_skip = cb->args[1];
 
 	if (chains_to_skip != 0)
-		genl_lock();
+		genl_fam_lock(NULL);
 
 	for (i = 0; i < GENL_FAM_TAB_SIZE; i++) {
 		if (i < chains_to_skip)
@@ -445,7 +497,7 @@ static int ctrl_dumpfamily(struct sk_buff *skb, struct netlink_callback *cb)
 
 errout:
 	if (chains_to_skip != 0)
-		genl_unlock();
+		genl_fam_unlock(NULL);
 
 	cb->args[0] = i;
 	cb->args[1] = n;


^ permalink raw reply related

* [PATCH] virtio_net.c gso & feature support
From: Rusty Russell @ 2007-07-24 11:56 UTC (permalink / raw)
  To: virtualization; +Cc: Christian Borntraeger, Herbert Xu, netdev

Feedback welcome, as always!

(There's been talk of a virtualization git tree, in which case there'll
be a decent home for these patches soon).

Cheers,
Rusty.
==
Add feature and GSO support to virtio net driver.

If you don't do GSO, you can simply ignore the first sg element of
every outgoing packet, and tack a dummy one on every incoming.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 drivers/net/virtio_net.c   |  130 ++++++++++++++++++++++++++++++++++++++++----
 include/linux/virtio_net.h |   25 +++++++-
 2 files changed, 143 insertions(+), 12 deletions(-)

===================================================================
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -16,12 +16,13 @@
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
-//#define DEBUG
+#define DEBUG
 #include <linux/netdevice.h>
 #include <linux/etherdevice.h>
 #include <linux/module.h>
 #include <linux/virtio.h>
 #include <linux/scatterlist.h>
+#include <linux/virtio_net.h>
 
 /* FIXME: Make dynamic */
 #define MAX_PACKET_LEN (ETH_HLEN+ETH_DATA_LEN)
@@ -40,6 +41,18 @@ struct virtnet_info
 	struct sk_buff_head send;
 };
 
+static inline struct virtio_net_hdr *skb_vnet_hdr(struct sk_buff *skb)
+{
+	return (struct virtio_net_hdr *)skb->cb;
+}
+
+static void vnet_hdr_to_sg(struct scatterlist *sg, struct sk_buff *skb)
+{
+	sg->page = virt_to_page(skb_vnet_hdr(skb));
+	sg->offset = offset_in_page(skb_vnet_hdr(skb));
+	sg->length = sizeof(struct virtio_net_hdr);
+}
+
 static bool skb_xmit_done(struct virtqueue *vq)
 {
 	struct virtnet_info *vi = vq->priv;
@@ -52,12 +65,14 @@ static void receive_skb(struct net_devic
 static void receive_skb(struct net_device *dev, struct sk_buff *skb,
 			unsigned len)
 {
-	if (unlikely(len < ETH_HLEN)) {
+	struct virtio_net_hdr *hdr = skb_vnet_hdr(skb);
+
+	if (unlikely(len < sizeof(struct virtio_net_hdr) + ETH_HLEN)) {
 		pr_debug("%s: short packet %i\n", dev->name, len);
 		dev->stats.rx_length_errors++;
-		dev_kfree_skb(skb);
-		return;
-	}
+		goto drop;
+	}
+	len -= sizeof(struct virtio_net_hdr);
 	BUG_ON(len > MAX_PACKET_LEN);
 
 	skb_trim(skb, len);
@@ -66,13 +81,70 @@ static void receive_skb(struct net_devic
 		 ntohs(skb->protocol), skb->len, skb->pkt_type);
 	dev->stats.rx_bytes += skb->len;
 	dev->stats.rx_packets++;
+
+	if (hdr->flags & VIRTIO_NET_F_NEEDS_CSUM) {
+		pr_debug("Needs csum!\n");
+		skb->ip_summed = CHECKSUM_PARTIAL;
+		skb->csum_start = hdr->csum_start;
+		skb->csum_offset = hdr->csum_offset;
+		if (skb->csum_start > skb->len - 2
+		    || skb->csum_offset > skb->len - 2) {
+			if (net_ratelimit())
+				printk(KERN_WARNING "%s: csum=%u/%u len=%u\n",
+				       dev->name, skb->csum_start,
+				       skb->csum_offset, skb->len);
+			goto frame_err;
+		}
+	}
+
+	if (hdr->gso_type != VIRTIO_NET_GSO_NONE) {
+		pr_debug("GSO!\n");
+		switch (hdr->gso_type) {
+		case VIRTIO_NET_GSO_TCP:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+			break;
+		case VIRTIO_NET_GSO_TCP_ECN:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCP_ECN;
+			break;
+		case VIRTIO_NET_GSO_UDP:
+			skb_shinfo(skb)->gso_type = SKB_GSO_UDP;
+			break;
+		case VIRTIO_NET_GSO_TCPV6:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+			break;
+		default:
+			if (net_ratelimit())
+				printk(KERN_WARNING "%s: bad gso type %u.\n",
+				       dev->name, hdr->gso_type);
+			goto frame_err;
+		}
+
+		skb_shinfo(skb)->gso_size = hdr->gso_size;
+		if (skb_shinfo(skb)->gso_size == 0) {
+			if (net_ratelimit())
+				printk(KERN_WARNING "%s: zero gso size.\n",
+				       dev->name);
+			goto frame_err;
+		}
+
+		/* Header must be checked, and gso_segs computed. */
+		skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
+		skb_shinfo(skb)->gso_segs = 0;
+	}
+
 	netif_receive_skb(skb);
+	return;
+
+frame_err:
+	dev->stats.rx_frame_errors++;
+drop:
+	dev_kfree_skb(skb);
 }
 
 static void try_fill_recv(struct virtnet_info *vi)
 {
 	struct sk_buff *skb;
-	struct scatterlist sg[MAX_SKB_FRAGS];
+	struct scatterlist sg[1+MAX_SKB_FRAGS];
 	int num, err;
 
 	for (;;) {
@@ -81,7 +153,8 @@ static void try_fill_recv(struct virtnet
 			break;
 
 		skb_put(skb, MAX_PACKET_LEN);
-		num = skb_to_sgvec(skb, sg, 0, skb->len);
+		vnet_hdr_to_sg(sg, skb);
+		num = skb_to_sgvec(skb, sg+1, 0, skb->len) + 1;
 		skb_queue_head(&vi->recv, skb);
 
 		err = vi->vq_recv->ops->add_buf(vi->vq_recv, sg, 0, num, skb);
@@ -161,7 +234,8 @@ static int start_xmit(struct sk_buff *sk
 {
 	struct virtnet_info *vi = netdev_priv(dev);
 	int num, err;
-	struct scatterlist sg[MAX_SKB_FRAGS];
+	struct scatterlist sg[1+MAX_SKB_FRAGS];
+	struct virtio_net_hdr *hdr;
 	const unsigned char *dest = ((struct ethhdr *)skb->data)->h_dest;
 
 	pr_debug("%s: xmit %p %02x:%02x:%02x:%02x:%02x:%02x\n",
@@ -170,7 +244,41 @@ static int start_xmit(struct sk_buff *sk
 
 	free_old_xmit_skbs(vi);
 
-	num = skb_to_sgvec(skb, sg, 0, skb->len);
+	/* Encode metadata header at front. */
+	hdr = skb_vnet_hdr(skb);
+	if (skb->ip_summed == CHECKSUM_PARTIAL) {
+		hdr->flags = VIRTIO_NET_F_NEEDS_CSUM;
+		hdr->csum_start = skb->csum_start - skb_headroom(skb);
+		hdr->csum_offset = skb->csum_offset;
+	} else {
+		hdr->flags = 0;
+		hdr->csum_offset = hdr->csum_start = 0;
+	}
+
+	if (skb_is_gso(skb)) {
+		printk("xmit: gso size %u len %u (%u/%u/%u)\n",
+		       skb_shinfo(skb)->gso_size, skb->len,
+		       skb_transport_header(skb) - skb->head,
+		       skb_network_header(skb) - skb->head,
+		       skb->data - skb->head);
+		hdr->gso_size = skb_shinfo(skb)->gso_size;
+		if (skb_shinfo(skb)->gso_type & SKB_GSO_TCP_ECN)
+			hdr->gso_type = VIRTIO_NET_GSO_TCP_ECN;
+		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
+			hdr->gso_type = VIRTIO_NET_GSO_TCP;
+		else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
+			hdr->gso_type = VIRTIO_NET_GSO_TCPV6;
+		else if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP)
+			hdr->gso_type = VIRTIO_NET_GSO_UDP;
+		else
+			BUG();
+	} else {
+		hdr->gso_type = VIRTIO_NET_GSO_NONE;
+		hdr->gso_size = 0;
+	}
+
+	vnet_hdr_to_sg(sg, skb);
+	num = skb_to_sgvec(skb, sg+1, 0, skb->len) + 1;
 	__skb_queue_head(&vi->send, skb);
 	err = vi->vq_send->ops->add_buf(vi->vq_send, sg, num, 0, skb);
 	if (err) {
@@ -220,7 +328,8 @@ struct net_device *virtnet_probe(struct 
 struct net_device *virtnet_probe(struct virtqueue *vq_recv,
 				 struct virtqueue *vq_send,
 				 struct device *device,
-				 const u8 mac[ETH_ALEN])
+				 const u8 mac[ETH_ALEN],
+				 unsigned long features)
 {
 	int err;
 	struct net_device *dev;
@@ -239,6 +348,7 @@ struct net_device *virtnet_probe(struct 
 	dev->poll = virtnet_poll;
 	dev->hard_start_xmit = start_xmit;
 	dev->weight = 16;
+	dev->features = features;
 	SET_NETDEV_DEV(dev, device);
 
 	vi = netdev_priv(dev);
===================================================================
--- a/include/linux/virtio_net.h
+++ b/include/linux/virtio_net.h
@@ -1,6 +1,26 @@
 #ifndef _LINUX_VIRTIO_NET_H
 #define _LINUX_VIRTIO_NET_H
 #include <linux/types.h>
+
+/* This is the first element of the scatter-gather list.
+ * If you don't specify GSO or CSUM features, you can simply ignore the
+ * header. */
+struct virtio_net_hdr
+{
+#define VIRTIO_NET_F_NEEDS_CSUM	1	// Use csum_start, csum_offset
+      __u16 flags;
+#define VIRTIO_NET_GSO_NONE	0	// Not a GSO frame
+#define VIRTIO_NET_GSO_TCP	1	// GSO frame, IPv4 TCP (TSO)
+#define VIRTIO_NET_GSO_TCP_ECN	2	// GSO frame, IPv4 TCP w/ ECN
+#define VIRTIO_NET_GSO_UDP	3	// GSO frame, IPv4 UDP (UFO)
+#define VIRTIO_NET_GSO_TCPV6	4	// GSO frame, IPv6 TCP
+      __u16 gso_type;
+      __u16 gso_size;
+      __u16 csum_start;
+      __u16 csum_offset;
+};
+
+#ifdef __KERNEL__
 #include <linux/etherdevice.h>
 struct device;
 struct net_device;
@@ -9,7 +29,8 @@ struct net_device *virtnet_probe(struct 
 struct net_device *virtnet_probe(struct virtqueue *vq_recv,
 				 struct virtqueue *vq_send,
 				 struct device *dev,
-				 const u8 mac[ETH_ALEN]);
+				 const u8 mac[ETH_ALEN],
+				 unsigned long features);
 void virtnet_remove(struct net_device *dev);
-
+#endif /* __KERNEL__ */
 #endif /* _LINUX_VIRTIO_NET_H */



^ permalink raw reply

* Re: [PATCH] [2.6.22] Fix a potential NULL pointer dereference in write_bulk_callback() in drivers/net/usb/pegasus.c
From: Petko Manolov @ 2007-07-24 12:57 UTC (permalink / raw)
  To: Micah Gruber; +Cc: petkan, linux-kernel, netdev, jgarzik
In-Reply-To: <46A46160.9080508@gmail.com>

ACK :-)


cheers,
Petko


On Mon, 23 Jul 2007, Micah Gruber wrote:

> This patch fixes a potential null dereference bug where we dereference 
> pegasus before a null check. This patch simply moves the dereferencing after 
> the null check.
>
> Signed-off-by: Micah Gruber <micah.gruber@gmail.com>
>
> ---
>
> --- a/drivers/net/usb/pegasus.c
> +++ b/drivers/net/usb/pegasus.c
> @@ -768,11 +768,13 @@
> static void write_bulk_callback(struct urb *urb)
> {
>       pegasus_t *pegasus = urb->context;
> -       struct net_device *net = pegasus->net;
> +       struct net_device *net;
>
>       if (!pegasus)
>               return;
>
> +       net = pegasus->net;
> +
>       if (!netif_device_present(net) || !netif_running(net))
>               return;
>

^ permalink raw reply

* Re: [PATCH 2.6.22] TCP: Make TCP_RTO_MAX a variable (take 2)
From: OBATA Noboru @ 2007-07-24 13:30 UTC (permalink / raw)
  To: rick.jones2; +Cc: shemminger, davem, yoshfuji, netdev
In-Reply-To: <4696AAD2.9030405@hp.com>

From: Rick Jones <rick.jones2@hp.com>
Subject: Re: [PATCH 2.6.22] TCP: Make TCP_RTO_MAX a variable (take 2)
Date: Thu, 12 Jul 2007 15:27:30 -0700

> > So the problem is that RTO can grows to be twice the failover detection
> > time.  So back to the original mail, the scenario has a switch with failover
> > detection of 20seconds.  Worst case TCP RTO could grow to 40 seconds.
> > 
> > Going back in archive to original mail:
> > 
> > 
> >>Background
> >>==========
> >>
> >>When designing a TCP/IP based network system on failover-capable
> >>network devices, people want to set timeouts hierarchically in
> >>three layers, network device layer, TCP layer, and application
> >>layer (bottom-up order), such that:
> >>
> >>1. Network device layer detects a failure first and switch to a
> >>   backup device (say, in 20sec).
> >>
> >>2. TCP layer timeout & retransmission comes next, _hopefully_
> >>   before the application layer timeout.
> >>
> >>3. Application layer detects a network failure last (by, say,
> >>   30sec timeout) and may trigger a system-level failover.
> > 
> > 
> > Sounds like the solution is to make the switch failover detection faster.
> > If you get switch failover down to 5sec then TCP RTO shouldn't be bigger
> > than 10sec, and application will survive.
> 
> That may indeed be the best solution, we'll have to wait to hear if 
> there is any freedom there.  When this sort of thing has crossed my path 
> in other contexts, the general answer is that the device failover time 
> is fixed, and the application layer time is similarly constrained by 
> end-user expectation/requirement.  Often as not, layer 8 and 9 issues 
> tend to dominate and expect to trump (in this case layer 4 issues).

I agree that application will survive if a user makes the
application timeout twice the failover timeout.  But I'm afraid
there is no such freedom.

Basically, to minimize downtime, shorter timeouts are preferred
as long as the probability of mis-detection is kept low at a
certain level.

In practice, failover timeouts for bonding, switches, or routers
are determined by heuristics.  Users know what timeout values and
retry counts of probe packets are suitable for detecting failure
of a certain combination of network equipments. (e.g., 5sec
timeout, retries 4 times)  Shorter is better.

And application timeout (or system timeout) is given as an
end-user requirement.  There is little change of negotiation,
really.  And again shorter (than requirement, if possible) is
better.

Regards,

-- 
OBATA Noboru (noboru.obata.ar@hitachi.com)

^ permalink raw reply

* Re: [PATCH 2.6.22] TCP: Make TCP_RTO_MAX a variable (take 2)
From: OBATA Noboru @ 2007-07-24 13:35 UTC (permalink / raw)
  To: rick.jones2; +Cc: davem, shemminger, yoshfuji, netdev
In-Reply-To: <46969460.3020604@hp.com>

From: Rick Jones <rick.jones2@hp.com>
Subject: Re: [PATCH 2.6.22] TCP: Make TCP_RTO_MAX a variable (take 2)
Date: Thu, 12 Jul 2007 13:51:44 -0700

> > TCP's timeouts are perfectly fine, and the only thing you
> > might be showing above is that the application timeouts
> > are too short or that TCP needs notifications.
> 
> The application timeouts are probably being driven by external desires 
> for a given recovery time.

Agreed.

> TCP notifications don't solve anything unless the links in question are 
> local to the machine on which the TCP endpoint resides.

Agreed.  Thank you for a good explanation.

My original discussion using Dom-0 and Dom-U might be
misleading, but I was trying to say:

* Network failure and recovery(failover) are not necessarily
  visible locally.

  ** Dom-0 vs. Dom-U discussion is just an example of the case
     where a network failure is not visible locally.

  ** For another example, network switches or routers sitting
     somewhere in the middle of route are often duplicated with
     active-standby setting today.

* Quick response (retransmission) of TCP upon a recovery of such
  invisible devices as well is desired.

* If the failure and recovery are not visible locally, TCP
  notifications do not help.

Regards,

-- 
OBATA Noboru (noboru.obata.ar@hitachi.com)

^ permalink raw reply

* Re: [PATCH 2/4] Add new timeval_to_sec function
From: Patrick McHardy @ 2007-07-24 13:43 UTC (permalink / raw)
  To: Varun Chandramohan
  Cc: Oliver Hartkopp, netdev, sri, dlstevens, varuncha,
	Thomas Gleixner
In-Reply-To: <46A59EDC.4060906@linux.vnet.ibm.com>

Varun Chandramohan wrote:
> Oliver Hartkopp wrote:
> 
>>>>I don't think you should round down timeout values.
>>>>  
>>>>    
>>>>      
>>>
>>>Can you elaborate on that? As per the RFC of MIB ,we need only seconds
>>>granularity. Taking that as the case i dont understand why round down
>>>should not be done?
>>>  
>>>    
>>
>>When you like to create any timeout based on your calculated value, you
>>might run into the problem that your calculated value is set to _zero_
>>even if there was "some time" before the conversion. This might probably
>>not what you indented to get.
>>
>>So what about rounding up with
>>
>>return (tv->tv_sec + (tv->tv_usec + 999999)/1000000);
>>
>>???
>>
>>  
> 
> This can done.  Is this what you were ref to me, Patrick?


Yes, timeouts should usually be at least as long as specified.

^ permalink raw reply

* Re: [PATCH 2/4] Add new timeval_to_sec function
From: Varun Chandramohan @ 2007-07-24 13:50 UTC (permalink / raw)
  To: Patrick McHardy
  Cc: Oliver Hartkopp, netdev, sri, dlstevens, varuncha,
	Thomas Gleixner
In-Reply-To: <46A6021B.4070606@trash.net>

Patrick McHardy wrote:
> Varun Chandramohan wrote:
>   
>> Oliver Hartkopp wrote:
>>
>>     
>>>>> I don't think you should round down timeout values.
>>>>>  
>>>>>    
>>>>>      
>>>>>           
>>>> Can you elaborate on that? As per the RFC of MIB ,we need only seconds
>>>> granularity. Taking that as the case i dont understand why round down
>>>> should not be done?
>>>>  
>>>>    
>>>>         
>>> When you like to create any timeout based on your calculated value, you
>>> might run into the problem that your calculated value is set to _zero_
>>> even if there was "some time" before the conversion. This might probably
>>> not what you indented to get.
>>>
>>> So what about rounding up with
>>>
>>> return (tv->tv_sec + (tv->tv_usec + 999999)/1000000);
>>>
>>> ???
>>>
>>>  
>>>       
>> This can done.  Is this what you were ref to me, Patrick?
>>     
>
>
> Yes, timeouts should usually be at least as long as specified.
>   
Thanks Patrick and Oliver, ill round it up. :-)


^ permalink raw reply

* Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
From: Dan Williams @ 2007-07-24 13:55 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Andrew Morton, Alexey Dobriyan, Linus Torvalds, linux-kernel,
	netdev, mark.fasheh
In-Reply-To: <20070724082207.GN3287@kernel.dk>

On 7/24/07, Jens Axboe <jens.axboe@oracle.com> wrote:
> What about the new async crypto stuff? I've been looking, but is it
> guarenteed that async_memcpy() runs in process context with interrupts
> enabled always? If not, there's a km type bug there.
>

Currently the only user is the MD raid456 driver, and yes, it only
performs copies from the handle_stripe routine which is always run in
process context with interrupts enabled.  However this is not
documented.  Would it be advisable to add a WARN_ON for this
condition?

> In general, I think the highmem stuff could do with more safety checks:
>
> - People ALWAYS get the atomic unmaps wrong, passing in the page instead
>   of the address. I've seen tons of these. And since kunmap_atomic()
>   takes a void pointer, nobody notices until it goes boom.
> - People easily get the km type wrong - they use KM_USERx in interrupt
>   context, or one of the irq variants without disabling interrupts.
>
> If we could just catch these two types of bugs, we've got a lot of these
> problems covered.
>
>
> --
> Jens Axboe
>
--
Dan

^ permalink raw reply

* Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
From: Dan Williams @ 2007-07-24 14:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Jens Axboe, Alexey Dobriyan, Linus Torvalds, linux-kernel, netdev,
	mark.fasheh, Nelson, Shannon
In-Reply-To: <20070724013455.691e6752.akpm@linux-foundation.org>

On 7/24/07, Andrew Morton <akpm@linux-foundation.org> wrote:
[...]
> > What about the new async crypto stuff? I've been looking, but is it
> > guarenteed that async_memcpy() runs in process context with interrupts
> > enabled always? If not, there's a km type bug there.
>
> I think Shannon maintains that now.
>

I am looking after the async_tx API, I will send a patch to update
MAINTAINERS shortly.

--
Dan

^ permalink raw reply

* Re: [PATCH net-2.6.22-rc7] xfrm state selection update to use inner addresses
From: Patrick McHardy @ 2007-07-24 14:31 UTC (permalink / raw)
  To: joakim.koskela; +Cc: netdev, David Miller
In-Reply-To: <200707231119.25457.joakim.koskela@hiit.fi>

Joakim Koskela wrote:
> This patch modifies the xfrm state selection logic to use the inner
> addresses where the outer have been (incorrectly) used. This is
> required for beet mode in general and interfamily setups in both
> tunnel and beet mode.


Looks good.

Acked-by: Patrick McHardy <kaber@trash.net>

^ permalink raw reply

* Re: [PATCH]: revised make xfrm_audit_log more generic patch
From: Steve Grubb @ 2007-07-24 15:04 UTC (permalink / raw)
  To: Joy Latten; +Cc: netdev, linux-audit, davem
In-Reply-To: <200707232146.l6NLk50u001083@faith.austin.ibm.com>

Hi,

I think we need just one other minor tweak.

On Monday 23 July 2007 17:46:05 Joy Latten wrote:
> @@ -2154,44 +2168,23 @@ EXPORT_SYMBOL(xfrm_bundle_ok);
>  /* Audit addition and deletion of SAs and ipsec policy */
>  
>  void xfrm_audit_log(uid_t auid, u32 sid, int type, int result,
> -                   struct xfrm_policy *xp, struct xfrm_state *x)
> +                    u16 family, xfrm_address_t saddr, xfrm_address_t
> daddr, +                    __be32 spi, __be32 flowlabel, struct
> xfrm_sec_ctx *sctx, +                    char *buf)
>  {
> -
>         char *secctx;
>         u32 secctx_len;
> -       struct xfrm_sec_ctx *sctx = NULL;
>         struct audit_buffer *audit_buf;
> -       int family;
>         extern int audit_enabled;
>  
>         if (audit_enabled == 0)
>                 return;
>  
> -       BUG_ON((type == AUDIT_MAC_IPSEC_ADDSA ||
> -               type == AUDIT_MAC_IPSEC_DELSA) && !x);
> -       BUG_ON((type == AUDIT_MAC_IPSEC_ADDSPD ||
> -               type == AUDIT_MAC_IPSEC_DELSPD) && !xp);
> -
>         audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC,
> type); if (audit_buf == NULL)
>                 return;
>  
> -       switch(type) {
> -       case AUDIT_MAC_IPSEC_ADDSA:
> -               audit_log_format(audit_buf, "SAD add: auid=%u", auid);
> -               break;
> -       case AUDIT_MAC_IPSEC_DELSA:
> -               audit_log_format(audit_buf, "SAD delete: auid=%u", auid);
> -               break;
> -       case AUDIT_MAC_IPSEC_ADDSPD:
> -               audit_log_format(audit_buf, "SPD add: auid=%u", auid);
> -               break;
> -       case AUDIT_MAC_IPSEC_DELSPD:
> -               audit_log_format(audit_buf, "SPD delete: auid=%u", auid);
> -               break;
> -       default:
> -               return;
> -       }
> +       audit_log_format(audit_buf, "%s: auid=%u", buf, auid);
>  
>         if (sid != 0 &&
>                 security_secid_to_secctx(sid, &secctx, &secctx_len) == 0)

The operation in buf will not be parsed by the user space tools. Let's 
use "op=%s " where you have "%s: " above. Audit record fields are name=value 
and fields separated by spaces. "op" is what we are using in other places to 
mean operation. 

I know its a change from the records above, but we previously had some detail 
about what operation was being performed by the record type and this did not 
matter so much. Now that we only have one event type, the meaning of the 
event being recorded needs to be parsable and in a field. 

It also wouldn't hurt to change the text being sent to this function to have a 
hyphen instead of a space, so "SPD delete" becomes "SPD-delete". This keeps 
the parser happy.

This patch otherwise looks good.

-Steve

^ permalink raw reply

* [PATCH] e1000: repost work around 82571 completion timout on pseries HW
From: Andy Gospodarek @ 2007-07-24 15:19 UTC (permalink / raw)
  To: netdev, Auke Kok


There seems to have been some discussion about a patch like this in the
past but I still haven't noticed any platforms fixes or noticed that
this got included, so I'd like to propose this modified version.

Thoughts?

Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
---

 e1000_hw.c |    7 +++++++
 e1000_hw.h |    1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
index 9be4469..7c75b8b 100644
--- a/drivers/net/e1000/e1000_hw.c
+++ b/drivers/net/e1000/e1000_hw.c
@@ -1030,6 +1030,13 @@ e1000_init_hw(struct e1000_hw *hw)
         break;
     }
 
+#if defined(CONFIG_PPC_PSERIES)
+    if (hw->mac_type == e1000_82571) {
+        uint32_t gcr = E1000_READ_REG(hw, GCR);
+        gcr |= E1000_GCR_DISABLE_TIMEOUT_MECHANISM;
+        E1000_WRITE_REG(hw, GCR, gcr);
+    }
+#endif 
 
     if (hw->mac_type == e1000_82573) {
         uint32_t gcr = E1000_READ_REG(hw, GCR);
diff --git a/drivers/net/e1000/e1000_hw.h b/drivers/net/e1000/e1000_hw.h
index bd000b8..71f4e2d 100644
--- a/drivers/net/e1000/e1000_hw.h
+++ b/drivers/net/e1000/e1000_hw.h
@@ -2211,6 +2211,7 @@ struct e1000_host_command_info {
 #define PCI_EX_82566_SNOOP_ALL PCI_EX_NO_SNOOP_ALL
 
 #define E1000_GCR_L1_ACT_WITHOUT_L0S_RX 0x08000000
+#define E1000_GCR_DISABLE_TIMEOUT_MECHANISM 0x80000000
 /* Function Active and Power State to MNG */
 #define E1000_FACTPS_FUNC0_POWER_STATE_MASK         0x00000003
 #define E1000_FACTPS_LAN0_VALID                     0x00000004

^ permalink raw reply related

* Re: [PATCH][30/37] Clean up duplicate includes in net/netfilter/
From: Patrick McHardy @ 2007-07-24 15:30 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: James Morris, Rusty Russell, Bart De Schuymer, Harald Welte,
	netdev, netfilter-devel, Linux Kernel Mailing List,
	Netfilter Core Team, Andrew Morton, David S Miller
In-Reply-To: <200707211703.47827.jesper.juhl@gmail.com>

Jesper Juhl wrote:
> This patch cleans up duplicate includes in
> 	net/netfilter/


I've queued this one and the bridge-netfilter patch (27/37), thanks
Jesper.

^ permalink raw reply

* Re: [PATCH] e1000: repost work around 82571 completion timout on pseries HW
From: Kok, Auke @ 2007-07-24 15:31 UTC (permalink / raw)
  To: Andy Gospodarek; +Cc: netdev
In-Reply-To: <20070724151912.GA2293@gospo.rdu.redhat.com>

Andy Gospodarek wrote:
> There seems to have been some discussion about a patch like this in the
> past but I still haven't noticed any platforms fixes or noticed that
> this got included, so I'd like to propose this modified version.
> 
> Thoughts?

I've written a completely new version for IBM based on the response that 
implements this as a pci quirk (to tag the chipset, not the device).

However IBM has spotted an issue due to firmware hiding the id for linux of the 
bridge involved (on pseries), and they're working on an alternative to fix that.

So, the patch will look different, and I'll post it as soon as we have a version 
for both pseries and xseries that works.

Auke


> 
> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
> ---
> 
>  e1000_hw.c |    7 +++++++
>  e1000_hw.h |    1 +
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/net/e1000/e1000_hw.c b/drivers/net/e1000/e1000_hw.c
> index 9be4469..7c75b8b 100644
> --- a/drivers/net/e1000/e1000_hw.c
> +++ b/drivers/net/e1000/e1000_hw.c
> @@ -1030,6 +1030,13 @@ e1000_init_hw(struct e1000_hw *hw)
>          break;
>      }
>  
> +#if defined(CONFIG_PPC_PSERIES)
> +    if (hw->mac_type == e1000_82571) {
> +        uint32_t gcr = E1000_READ_REG(hw, GCR);
> +        gcr |= E1000_GCR_DISABLE_TIMEOUT_MECHANISM;
> +        E1000_WRITE_REG(hw, GCR, gcr);
> +    }
> +#endif 
>  
>      if (hw->mac_type == e1000_82573) {
>          uint32_t gcr = E1000_READ_REG(hw, GCR);
> diff --git a/drivers/net/e1000/e1000_hw.h b/drivers/net/e1000/e1000_hw.h
> index bd000b8..71f4e2d 100644
> --- a/drivers/net/e1000/e1000_hw.h
> +++ b/drivers/net/e1000/e1000_hw.h
> @@ -2211,6 +2211,7 @@ struct e1000_host_command_info {
>  #define PCI_EX_82566_SNOOP_ALL PCI_EX_NO_SNOOP_ALL
>  
>  #define E1000_GCR_L1_ACT_WITHOUT_L0S_RX 0x08000000
> +#define E1000_GCR_DISABLE_TIMEOUT_MECHANISM 0x80000000
>  /* Function Active and Power State to MNG */
>  #define E1000_FACTPS_FUNC0_POWER_STATE_MASK         0x00000003
>  #define E1000_FACTPS_LAN0_VALID                     0x00000004

^ permalink raw reply

* Re: Oops in xfrm_bundle_ok
From: Ken-ichirou MATSUZAWA @ 2007-07-24 15:51 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20070705.024710.74752219.chamas@h4.dion.ne.jp>

 Hello,

From: Ken-ichirou MATSUZAWA <chamas@h4.dion.ne.jp>
Subject: Oops in xfrm_bundle_ok
Date: Thu, 05 Jul 2007 02:47:10 +0900 (JST)

> I got Oops like below. I glanced xfrm_bundle_ok() in
> xfrm_policy.c and __xfrm4.bundle_create() in xfrm4_policy.c.

It seems this was fixed by below, thanks a lot.

> commit bd0bf0765ea1fba80d7085e1f0375ec045631dc1
> Author: Patrick McHardy <kaber@trash.net>
> Date:   Wed Jul 18 01:55:52 2007 -0700
>
>     [XFRM]: Fix crash introduced by struct dst_entry reordering
....

@@ -2141,7 +2141,7 @@ int xfrm_bundle_ok(struct xfrm_policy *pol, struct xfrm_dst *first,
 		if (last == first)
 			break;
 
-		last = last->u.next;
+		last = (struct xfrm_dst *)last->u.dst.next;
 		last->child_mtu_cached = mtu;
 	}

^ permalink raw reply

* Re: [PATCH]: revised make xfrm_audit_log more generic patch
From: Joy Latten @ 2007-07-24 16:17 UTC (permalink / raw)
  To: Steve Grubb; +Cc: netdev, davem, linux-audit
In-Reply-To: <200707241104.56310.sgrubb@redhat.com>

On Tue, 2007-07-24 at 11:04 -0400, Steve Grubb wrote:

> > +       audit_log_format(audit_buf, "%s: auid=%u", buf, auid);
> >  
> >         if (sid != 0 &&
> >                 security_secid_to_secctx(sid, &secctx, &secctx_len) == 0)
> 
> The operation in buf will not be parsed by the user space tools. Let's 
> use "op=%s " where you have "%s: " above. Audit record fields are name=value 
> and fields separated by spaces. "op" is what we are using in other places to 
> mean operation. 
> 
> I know its a change from the records above, but we previously had some detail 
> about what operation was being performed by the record type and this did not 
> matter so much. Now that we only have one event type, the meaning of the 
> event being recorded needs to be parsable and in a field. 
> 
> It also wouldn't hurt to change the text being sent to this function to have a 
> hyphen instead of a space, so "SPD delete" becomes "SPD-delete". This keeps 
> the parser happy.
> 
> This patch otherwise looks good.

Sounds good. I will make the changes and resend. 
Thanks!!

Joy

^ permalink raw reply

* Re: 2.6.23-rc1: BUG_ON in kmap_atomic_prot()
From: Andrew Morton @ 2007-07-24 16:28 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: Alexey Dobriyan, Linus Torvalds, linux-kernel, netdev,
	Christoph Lameter
In-Reply-To: <1185271269.6479.7.camel@Homer.simpson.net>

On Tue, 24 Jul 2007 12:01:09 +0200 Mike Galbraith <efault@gmx.de> wrote:

> On Mon, 2007-07-23 at 13:24 -0700, Andrew Morton wrote:
> 
> > You're using DEBUG_PAGEALLOC, but I was not, so I think we can rule that out.
> 
> My box bugged during boot the first time I booted 23-rc1, but nothing
> made it to the console, and I didn't have a serial console running.  I
> didn't have DEBUG_PAGEALLOC or friends set.
> 
> > I haven't worked out where that kmap_atomic() call is coming from yet. 
> > Both traces point up into the page allocator, but I _think_ that's stack
> > gunk.
> 
> I just enabled all debug options, and was just rewarded with the below.

doh.  It's a slab bug.

> [  119.079531] eth1: link up, 100Mbps, full-duplex, lpa 0x45E1
> [  119.558867] ------------[ cut here ]------------
> [  119.572197] kernel BUG at arch/i386/mm/highmem.c:38!
> [  119.585804] invalid opcode: 0000 [#1]
> [  119.598013] PREEMPT SMP DEBUG_PAGEALLOC
> [  119.610103] Modules linked in: edd button battery ac ip6t_REJECT xt_tcpudp ipt_REJECT xt_state iptable_mangle iptable_nat nf_nat iptable_filter ip6table_mangle nf_conntrack_ipv4 nf_conntrack nfnetlink ip_tables ip6table_filter ip6_tables x_tables nls_iso8859_1 nls_cp437 nls_utf8 snd_intel8x0 snd_ac97_codec ac97_bus snd_mpu401 snd_pcm prism54 snd_timer snd_mpu401_uart snd_rawmidi snd_seq_device snd intel_agp agpgart soundcore snd_page_alloc i2c_i801 fan thermal processor
> [  119.698063] CPU:    1
> [  119.698065] EIP:    0060:[<c011cd2d>]    Not tainted VLI
> [  119.698067] EFLAGS: 00010006   (2.6.23-rc1-smp #75)
> [  119.736358] EIP is at kmap_atomic_prot+0xa7/0xab
> [  119.749647] eax: 3d07f163   ebx: c166db80   ecx: c0750e60   edx: 00000007
> [  119.765417] esi: 00000022   edi: 00000163   ebp: c069dcd4   esp: c069dcc8
> [  119.781273] ds: 007b   es: 007b   fs: 00d8  gs: 0033  ss: 0068
> [  119.796378] Process udevd (pid: 4775, ti=c069d000 task=f31aea60 task.ti=f477d000)
> [  119.804068] Stack: c166db80 00000000 c166db80 c069dcdc c011cd3f c069dd40 c015b6e0 00000001 
> [  119.822272]        00000044 00000163 00000000 00000001 c165f4e0 00000001 c165f4e0 00000001 
> [  119.840762]        00000000 00028020 c061e71c c166db80 00000046 00000080 00000001 c011e4de 
> [  119.859389] Call Trace:
> [  119.881302]  [<c0105144>] show_trace_log_lvl+0x1a/0x30
> [  119.896319]  [<c01051ff>] show_stack_log_lvl+0xa5/0xca
> [  119.911171]  [<c0105420>] show_registers+0x1fc/0x343
> [  119.925756]  [<c0105689>] die+0x122/0x249
> [  119.939241]  [<c0105834>] do_trap+0x84/0xad
> [  119.952897]  [<c0105b1c>] do_invalid_op+0x88/0x92
> [  119.967118]  [<c04cf3c2>] error_code+0x72/0x78
> [  119.980948]  [<c011cd3f>] kmap_atomic+0xe/0x10
> [  119.994642]  [<c015b6e0>] get_page_from_freelist+0x39e/0x45e
> [  120.009485]  [<c015b7fb>] __alloc_pages+0x5b/0x2db
> [  120.023342]  [<c0172872>] cache_alloc_refill+0x380/0x6f2
> [  120.037623]  [<c0172e7a>] kmem_cache_alloc+0xa1/0xa5
> [  120.051426]  [<c03fb397>] neigh_create+0x5f/0x506
> [  120.064894]  [<c046e25d>] ndisc_dst_alloc+0x122/0x151
> [  120.078769]  [<c0471b0b>] __ndisc_send+0x8d/0x4fa
> [  120.092340]  [<c0472915>] ndisc_send_ns+0x5f/0x7d
> [  120.105848]  [<c0469ff5>] addrconf_dad_timer+0xdb/0xe0
> [  120.119758]  [<c012f8a0>] run_timer_softirq+0x130/0x191
> [  120.133717]  [<c012c06d>] __do_softirq+0x76/0xe4
> [  120.147475]  [<c0106b48>] do_softirq+0x63/0xac
> [  120.147488]  [<c012bff5>] 


> (gdb) list *neigh_create+0x5f
> 0xc03fb397 is in neigh_create (include/linux/slab.h:259).
> 254     /*
> 255      * Shortcuts
> 256      */
> 257     static inline void *kmem_cache_zalloc(struct kmem_cache *k, gfp_t flags)
> 258     {
> 259             return kmem_cache_alloc(k, flags | __GFP_ZERO);
> 260     }

See, networking's kmem_cache_alloc(..., __GFP_ZERO) ended up calling into
the page allocator with __GFP_ZERO.  This is the bug - slab isn't supposed
to do that: the __GFP_ZERO is supposed to be removed.

Now, it's not a highmem page, so prep_zero_page() won't actually establish
a kmap, but it will check that the kmap slot is presently unused on this
CPU.

But networking calls in here from softirq context (illegal for KM_USER0)
and sometimes that KM_USER0 slot *will* be in use, so kmap_atomic_prot()
will go BUG.

I must say it's really really scary that such a low-level function as
prep_zero_page() is using KM_USER0.  I don't think it has enough debugging
checks in there to prevent Bad Stuff from going undetected.

I guess this was the bug:

--- a/mm/slab.c~a
+++ a/mm/slab.c
@@ -2776,7 +2776,7 @@ static int cache_grow(struct kmem_cache 
 	 * 'nodeid'.
 	 */
 	if (!objp)
-		objp = kmem_getpages(cachep, flags, nodeid);
+		objp = kmem_getpages(cachep, local_flags, nodeid);
 	if (!objp)
 		goto failed;
 
_


I don't see why you later got fs corruption - afacit we won't actually
modify the KM_USER0 slot in this scenario.


> 262     /**
> 263      * kzalloc - allocate memory. The memory is set to zero.
> (gdb) list *kmem_cache_alloc+0xa1
> 0xc0172e7a is in kmem_cache_alloc (mm/slab.c:3176).
> 3171                    STATS_INC_ALLOCHIT(cachep);
> 3172                    ac->touched = 1;
> 3173                    objp = ac->entry[--ac->avail];
> 3174            } else {
> 3175                    STATS_INC_ALLOCMISS(cachep);
> 3176                    objp = cache_alloc_refill(cachep, flags);
> 3177            }
> 3178            return objp;
> 3179    }
> 3180
> (gdb) list *cache_alloc_refill+0x380
> 0xc0172872 is in cache_alloc_refill (include/linux/gfp.h:154).
> 149
> 150             /* Unknown node is current node */
> 151             if (nid < 0)
> 152                     nid = numa_node_id();
> 153
> 154             return __alloc_pages(gfp_mask, order,
> 155                     NODE_DATA(nid)->node_zonelists + gfp_zone(gfp_mask));
> 156     }
> 157
> 158     #ifdef CONFIG_NUMA
> (gdb) list *__alloc_pages+0x5b
> 0xc015b7fb is in __alloc_pages (mm/page_alloc.c:1248).
> 1243            if (unlikely(*z == NULL)) {
> 1244                    /* Should this ever happen?? */
> 1245                    return NULL;
> 1246            }
> 1247
> 1248            page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, order,
> 1249                                    zonelist, ALLOC_WMARK_LOW|ALLOC_CPUSET);
> 1250            if (page)
> 1251                    goto got_pg;
> 1252
> (gdb) list *get_page_from_freelist+0x39e
> 0xc015b6e0 is in get_page_from_freelist (include/linux/highmem.h:122).
> 117             return __alloc_zeroed_user_highpage(__GFP_MOVABLE, vma, vaddr);
> 118     }
> 119
> 120     static inline void clear_highpage(struct page *page)
> 121     {
> 122             void *kaddr = kmap_atomic(page, KM_USER0);
> 123             clear_page(kaddr);
> 124             kunmap_atomic(kaddr, KM_USER0);
> 125     }
> 126
> 
> 

^ permalink raw reply

* Re: [PATCH] e1000: repost work around 82571 completion timout on pseries HW
From: Andy Gospodarek @ 2007-07-24 16:29 UTC (permalink / raw)
  To: Kok, Auke; +Cc: Andy Gospodarek, netdev
In-Reply-To: <46A61B3C.5070808@intel.com>

On Tue, Jul 24, 2007 at 08:31:08AM -0700, Kok, Auke wrote:
> Andy Gospodarek wrote:
> >There seems to have been some discussion about a patch like this in the
> >past but I still haven't noticed any platforms fixes or noticed that
> >this got included, so I'd like to propose this modified version.
> >
> >Thoughts?
> 
> I've written a completely new version for IBM based on the response that 
> implements this as a pci quirk (to tag the chipset, not the device).
> 
> However IBM has spotted an issue due to firmware hiding the id for linux of 
> the bridge involved (on pseries), and they're working on an alternative to 
> fix that.
> 
> So, the patch will look different, and I'll post it as soon as we have a 
> version for both pseries and xseries that works.
> 
> Auke
> 

Sounds great.  I just wanted to make sure this didn't slip through the
cracks.



^ permalink raw reply

* Re: [GENETLINK]: Fix race in genl_unregister_mc_groups()
From: Brian Haley @ 2007-07-24 16:14 UTC (permalink / raw)
  To: Thomas Graf; +Cc: davem, johannes, netdev
In-Reply-To: <20070724094557.GE9285@postel.suug.ch>

Thomas Graf wrote:
> @@ -217,14 +229,8 @@ EXPORT_SYMBOL(genl_register_mc_group);
>  void genl_unregister_mc_group(struct genl_family *family,
>  			      struct genl_multicast_group *grp)
>  {
> -	BUG_ON(grp->family != family);
>  	genl_lock();
> -	netlink_clear_multicast_users(genl_sock, grp->id);
> -	clear_bit(grp->id, mc_groups);
> -	list_del(&grp->list);
> -	genl_ctrl_event(CTRL_CMD_DELMCAST_GRP, grp);
> -	grp->id = 0;
> -	grp->family = NULL;
> +	genl_unregister_mc_group(family, grp);
>  	genl_unlock();
>  }

Shouldn't this be __genl_unregister_mc_group(family, grp) ?

-Brian

^ permalink raw reply

* Re: [PATCH]: revised make xfrm_audit_log more generic patch
From: Joy Latten @ 2007-07-24 16:33 UTC (permalink / raw)
  To: Steve Grubb; +Cc: netdev, davem, linux-audit
In-Reply-To: <200707241104.56310.sgrubb@redhat.com>

On Tue, 2007-07-24 at 11:04 -0400, Steve Grubb wrote:

> It also wouldn't hurt to change the text being sent to this function to have a 
> hyphen instead of a space, so "SPD delete" becomes "SPD-delete". This keeps 
> the parser happy.
> 
Steve, more for my education, should all entries have this sort of
syntax, that is, a hyphen in it? I imagine some entries might be a 
bit more wordy and so I was wondering ahead of time how to do it.

Thanks!!

Joy

^ permalink raw reply

* Re: [PATCH 0/2] netxen: bug fixes for multiport adapters
From: Dhananjay Phadke @ 2007-07-24 16:57 UTC (permalink / raw)
  To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20070720190749.535704712@netxen.com>

Jeff,

Any chance of these patches getting committed soon?

Thanks,
-Dhananjay Phadke

On 7/21/07, dhananjay@netxen.com <dhananjay@netxen.com> wrote:
> These patches include fix for problem with 2nd port of multiport adapters
> on IBM blades. Also improves interrupt handling for multiport adapters
> avoiding interrupt flood after interrupt is down.
>
> Generated against upstream-fixes.
>
>  drivers/net/netxen/netxen_nic.h          |    3 +-
>  drivers/net/netxen/netxen_nic_main.c     |   85 +++++++++++++++---------------
>  2 files changed, 43 insertions(+), 45 deletions(-)
>
> --
> -
> 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


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