Netdev List
 help / color / mirror / Atom feed
* [PATCH v11 15/17]An example how to modifiy NIC driver to use napi_gro_frags() interface
From: xiaohui.xin @ 2010-09-25  4:27 UTC (permalink / raw)
  To: netdev, kvm, linux-kernel, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <59d8a50047ee01e26658fd676d26c0162b79e5fd.1285385607.git.xiaohui.xin@intel.com>

From: Xin Xiaohui <xiaohui.xin@intel.com>

This example is made on ixgbe driver.
It provides API is_rx_buffer_mapped_as_page() to indicate
if the driver use napi_gro_frags() interface or not.
The example allocates 2 pages for DMA for one ring descriptor
using netdev_alloc_page(). When packets is coming, using
napi_gro_frags() to allocate skb and to receive the packets.

---
 drivers/net/ixgbe/ixgbe.h      |    3 +
 drivers/net/ixgbe/ixgbe_main.c |  151 ++++++++++++++++++++++++++++++++--------
 2 files changed, 125 insertions(+), 29 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h
index 79c35ae..fceffc5 100644
--- a/drivers/net/ixgbe/ixgbe.h
+++ b/drivers/net/ixgbe/ixgbe.h
@@ -131,6 +131,9 @@ struct ixgbe_rx_buffer {
 	struct page *page;
 	dma_addr_t page_dma;
 	unsigned int page_offset;
+	u16 mapped_as_page;
+	struct page *page_skb;
+	unsigned int page_skb_offset;
 };
 
 struct ixgbe_queue_stats {
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index 6c00ee4..905d6d2 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -688,6 +688,12 @@ static inline void ixgbe_release_rx_desc(struct ixgbe_hw *hw,
 	IXGBE_WRITE_REG(hw, IXGBE_RDT(rx_ring->reg_idx), val);
 }
 
+static bool is_rx_buffer_mapped_as_page(struct ixgbe_rx_buffer *bi,
+					struct net_device *dev)
+{
+	return true;
+}
+
 /**
  * ixgbe_alloc_rx_buffers - Replace used receive buffers; packet split
  * @adapter: address of board private structure
@@ -704,13 +710,17 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
 	i = rx_ring->next_to_use;
 	bi = &rx_ring->rx_buffer_info[i];
 
+
 	while (cleaned_count--) {
 		rx_desc = IXGBE_RX_DESC_ADV(*rx_ring, i);
 
+		bi->mapped_as_page =
+			is_rx_buffer_mapped_as_page(bi, adapter->netdev);
+
 		if (!bi->page_dma &&
 		    (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED)) {
 			if (!bi->page) {
-				bi->page = alloc_page(GFP_ATOMIC);
+				bi->page = netdev_alloc_page(adapter->netdev);
 				if (!bi->page) {
 					adapter->alloc_rx_page_failed++;
 					goto no_buffers;
@@ -727,7 +737,7 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
 			                            PCI_DMA_FROMDEVICE);
 		}
 
-		if (!bi->skb) {
+		if (!bi->mapped_as_page && !bi->skb) {
 			struct sk_buff *skb;
 			/* netdev_alloc_skb reserves 32 bytes up front!! */
 			uint bufsz = rx_ring->rx_buf_len + SMP_CACHE_BYTES;
@@ -747,6 +757,19 @@ static void ixgbe_alloc_rx_buffers(struct ixgbe_adapter *adapter,
 			                         rx_ring->rx_buf_len,
 			                         PCI_DMA_FROMDEVICE);
 		}
+
+		if (bi->mapped_as_page && !bi->page_skb) {
+			bi->page_skb = netdev_alloc_page(adapter->netdev);
+			if (!bi->page_skb) {
+				adapter->alloc_rx_page_failed++;
+				goto no_buffers;
+			}
+			bi->page_skb_offset = 0;
+			bi->dma = pci_map_page(pdev, bi->page_skb,
+					bi->page_skb_offset,
+					(PAGE_SIZE / 2),
+					PCI_DMA_FROMDEVICE);
+		}
 		/* Refresh the desc even if buffer_addrs didn't change because
 		 * each write-back erases this info. */
 		if (rx_ring->flags & IXGBE_RING_RX_PS_ENABLED) {
@@ -823,6 +846,13 @@ struct ixgbe_rsc_cb {
 	dma_addr_t dma;
 };
 
+static bool is_no_buffer(struct ixgbe_rx_buffer *rx_buffer_info)
+{
+	return (!rx_buffer_info->skb ||
+		!rx_buffer_info->page_skb) &&
+		!rx_buffer_info->page;
+}
+
 #define IXGBE_RSC_CB(skb) ((struct ixgbe_rsc_cb *)(skb)->cb)
 
 static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
@@ -832,6 +862,7 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 	struct ixgbe_adapter *adapter = q_vector->adapter;
 	struct net_device *netdev = adapter->netdev;
 	struct pci_dev *pdev = adapter->pdev;
+	struct napi_struct *napi = &q_vector->napi;
 	union ixgbe_adv_rx_desc *rx_desc, *next_rxd;
 	struct ixgbe_rx_buffer *rx_buffer_info, *next_buffer;
 	struct sk_buff *skb;
@@ -868,29 +899,71 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 			len = le16_to_cpu(rx_desc->wb.upper.length);
 		}
 
+		if (is_no_buffer(rx_buffer_info))
+			break;
+
 		cleaned = true;
-		skb = rx_buffer_info->skb;
-		prefetch(skb->data);
-		rx_buffer_info->skb = NULL;
 
-		if (rx_buffer_info->dma) {
-			if ((adapter->flags2 & IXGBE_FLAG2_RSC_ENABLED) &&
-			    (!(staterr & IXGBE_RXD_STAT_EOP)) &&
-				 (!(skb->prev)))
-				/*
-				 * When HWRSC is enabled, delay unmapping
-				 * of the first packet. It carries the
-				 * header information, HW may still
-				 * access the header after the writeback.
-				 * Only unmap it when EOP is reached
-				 */
-				IXGBE_RSC_CB(skb)->dma = rx_buffer_info->dma;
-			else
-				pci_unmap_single(pdev, rx_buffer_info->dma,
-				                 rx_ring->rx_buf_len,
-				                 PCI_DMA_FROMDEVICE);
-			rx_buffer_info->dma = 0;
-			skb_put(skb, len);
+		if (!rx_buffer_info->mapped_as_page) {
+			skb = rx_buffer_info->skb;
+			prefetch(skb->data);
+			rx_buffer_info->skb = NULL;
+
+			if (rx_buffer_info->dma) {
+				if ((adapter->flags2 &
+					IXGBE_FLAG2_RSC_ENABLED) &&
+					(!(staterr & IXGBE_RXD_STAT_EOP)) &&
+					(!(skb->prev)))
+					/*
+					 * When HWRSC is enabled, delay unmapping
+					 * of the first packet. It carries the
+					 * header information, HW may still
+					 * access the header after the writeback.
+					 * Only unmap it when EOP is reached
+					 */
+					IXGBE_RSC_CB(skb)->dma =
+							rx_buffer_info->dma;
+				else
+					pci_unmap_single(pdev,
+							rx_buffer_info->dma,
+							rx_ring->rx_buf_len,
+							PCI_DMA_FROMDEVICE);
+				rx_buffer_info->dma = 0;
+				skb_put(skb, len);
+			}
+		} else {
+			skb = napi_get_frags(napi);
+			prefetch(rx_buffer_info->page_skb_offset);
+			rx_buffer_info->skb = NULL;
+			if (rx_buffer_info->dma) {
+				if ((adapter->flags2 &
+					IXGBE_FLAG2_RSC_ENABLED) &&
+					(!(staterr & IXGBE_RXD_STAT_EOP)) &&
+					(!(skb->prev)))
+					/*
+					 * When HWRSC is enabled, delay unmapping
+					 * of the first packet. It carries the
+					 * header information, HW may still
+					 * access the header after the writeback.
+					 * Only unmap it when EOP is reached
+					 */
+					IXGBE_RSC_CB(skb)->dma =
+						rx_buffer_info->dma;
+				else
+					pci_unmap_page(pdev, rx_buffer_info->dma,
+							PAGE_SIZE / 2,
+							PCI_DMA_FROMDEVICE);
+				rx_buffer_info->dma = 0;
+				skb_fill_page_desc(skb,
+						skb_shinfo(skb)->nr_frags,
+						rx_buffer_info->page_skb,
+						rx_buffer_info->page_skb_offset,
+						len);
+				rx_buffer_info->page_skb = NULL;
+				skb->len += len;
+				skb->data_len += len;
+				skb->truesize += len;
+			}
 		}
 
 		if (upper_len) {
@@ -956,6 +1029,11 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 				rx_buffer_info->dma = next_buffer->dma;
 				next_buffer->skb = skb;
 				next_buffer->dma = 0;
+				if (rx_buffer_info->mapped_as_page) {
+					rx_buffer_info->page_skb =
+							next_buffer->page_skb;
+					next_buffer->page_skb = NULL;
+				}
 			} else {
 				skb->next = next_buffer->skb;
 				skb->next->prev = skb;
@@ -975,7 +1053,8 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 		total_rx_bytes += skb->len;
 		total_rx_packets++;
 
-		skb->protocol = eth_type_trans(skb, adapter->netdev);
+		if (!rx_buffer_info->mapped_as_page)
+			skb->protocol = eth_type_trans(skb, adapter->netdev);
 #ifdef IXGBE_FCOE
 		/* if ddp, not passing to ULD unless for FCP_RSP or error */
 		if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
@@ -984,7 +1063,14 @@ static bool ixgbe_clean_rx_irq(struct ixgbe_q_vector *q_vector,
 				goto next_desc;
 		}
 #endif /* IXGBE_FCOE */
-		ixgbe_receive_skb(q_vector, skb, staterr, rx_ring, rx_desc);
+
+		if (!rx_buffer_info->mapped_as_page)
+			ixgbe_receive_skb(q_vector, skb, staterr,
+						rx_ring, rx_desc);
+		else {
+			skb_record_rx_queue(skb, rx_ring->queue_index);
+			napi_gro_frags(napi);
+		}
 
 next_desc:
 		rx_desc->wb.upper.status_error = 0;
@@ -3131,9 +3217,16 @@ static void ixgbe_clean_rx_ring(struct ixgbe_adapter *adapter,
 
 		rx_buffer_info = &rx_ring->rx_buffer_info[i];
 		if (rx_buffer_info->dma) {
-			pci_unmap_single(pdev, rx_buffer_info->dma,
-			                 rx_ring->rx_buf_len,
-			                 PCI_DMA_FROMDEVICE);
+			if (!rx_buffer_info->mapped_as_page) {
+				pci_unmap_single(pdev, rx_buffer_info->dma,
+						rx_ring->rx_buf_len,
+						PCI_DMA_FROMDEVICE);
+			} else {
+				pci_unmap_page(pdev, rx_buffer_info->dma,
+						PAGE_SIZE / 2,
+						PCI_DMA_FROMDEVICE);
+				rx_buffer_info->page_skb = NULL;
+			}
 			rx_buffer_info->dma = 0;
 		}
 		if (rx_buffer_info->skb) {
@@ -3158,7 +3251,7 @@ static void ixgbe_clean_rx_ring(struct ixgbe_adapter *adapter,
 			               PAGE_SIZE / 2, PCI_DMA_FROMDEVICE);
 			rx_buffer_info->page_dma = 0;
 		}
-		put_page(rx_buffer_info->page);
+		netdev_free_page(adapter->netdev, rx_buffer_info->page);
 		rx_buffer_info->page = NULL;
 		rx_buffer_info->page_offset = 0;
 	}
-- 
1.7.3


^ permalink raw reply related

* [PATCH v11 17/17]add two new ioctls for mp device.
From: xiaohui.xin @ 2010-09-25  4:27 UTC (permalink / raw)
  To: netdev, kvm, linux-kernel, mingo, davem, herbert, jdike; +Cc: Xin Xiaohui
In-Reply-To: <59d8a50047ee01e26658fd676d26c0162b79e5fd.1285385607.git.xiaohui.xin@intel.com>

From: Xin Xiaohui <xiaohui.xin@intel.com>

The patch add two ioctls for mp device.
One is for userspace to query how much memory locked to make mp device
run smoothly. Another one is for userspace to set how much meory locked
it really wants.

---
 drivers/vhost/mpassthru.c |  103 +++++++++++++++++++++++----------------------
 include/linux/mpassthru.h |    2 +
 2 files changed, 54 insertions(+), 51 deletions(-)

diff --git a/drivers/vhost/mpassthru.c b/drivers/vhost/mpassthru.c
index d86d94c..e3a0199 100644
--- a/drivers/vhost/mpassthru.c
+++ b/drivers/vhost/mpassthru.c
@@ -67,6 +67,8 @@ static int debug;
 #define COPY_THRESHOLD (L1_CACHE_BYTES * 4)
 #define COPY_HDR_LEN   (L1_CACHE_BYTES < 64 ? 64 : L1_CACHE_BYTES)
 
+#define DEFAULT_NEED	((8192*2*2)*4096)
+
 struct frag {
 	u16     offset;
 	u16     size;
@@ -110,7 +112,8 @@ struct page_ctor {
 	int			rq_len;
 	spinlock_t		read_lock;
 	/* record the locked pages */
-	int			lock_pages;
+	int			locked_pages;
+	int			cur_pages;
 	struct rlimit		o_rlim;
 	struct net_device	*dev;
 	struct mpassthru_port	port;
@@ -122,6 +125,7 @@ struct mp_struct {
 	struct net_device       *dev;
 	struct page_ctor	*ctor;
 	struct socket           socket;
+	struct task_struct	*user;
 
 #ifdef MPASSTHRU_DEBUG
 	int debug;
@@ -231,7 +235,8 @@ static int page_ctor_attach(struct mp_struct *mp)
 	ctor->port.ctor = page_ctor;
 	ctor->port.sock = &mp->socket;
 	ctor->port.hash = mp_lookup;
-	ctor->lock_pages = 0;
+	ctor->locked_pages = 0;
+	ctor->cur_pages = 0;
 
 	/* locked by mp_mutex */
 	dev->mp_port = &ctor->port;
@@ -264,37 +269,6 @@ struct page_info *info_dequeue(struct page_ctor *ctor)
 	return info;
 }
 
-static int set_memlock_rlimit(struct page_ctor *ctor, int resource,
-			      unsigned long cur, unsigned long max)
-{
-	struct rlimit new_rlim, *old_rlim;
-	int retval;
-
-	if (resource != RLIMIT_MEMLOCK)
-		return -EINVAL;
-	new_rlim.rlim_cur = cur;
-	new_rlim.rlim_max = max;
-
-	old_rlim = current->signal->rlim + resource;
-
-	/* remember the old rlimit value when backend enabled */
-	ctor->o_rlim.rlim_cur = old_rlim->rlim_cur;
-	ctor->o_rlim.rlim_max = old_rlim->rlim_max;
-
-	if ((new_rlim.rlim_max > old_rlim->rlim_max) &&
-			!capable(CAP_SYS_RESOURCE))
-		return -EPERM;
-
-	retval = security_task_setrlimit(resource, &new_rlim);
-	if (retval)
-		return retval;
-
-	task_lock(current->group_leader);
-	*old_rlim = new_rlim;
-	task_unlock(current->group_leader);
-	return 0;
-}
-
 static void relinquish_resource(struct page_ctor *ctor)
 {
 	if (!(ctor->dev->flags & IFF_UP) &&
@@ -323,7 +297,7 @@ static void mp_ki_dtor(struct kiocb *iocb)
 	} else
 		info->ctor->wq_len--;
 	/* Decrement the number of locked pages */
-	info->ctor->lock_pages -= info->pnum;
+	info->ctor->cur_pages -= info->pnum;
 	kmem_cache_free(ext_page_info_cache, info);
 	relinquish_resource(info->ctor);
 
@@ -357,6 +331,7 @@ static int page_ctor_detach(struct mp_struct *mp)
 {
 	struct page_ctor *ctor;
 	struct page_info *info;
+	struct task_struct *tsk = mp->user;
 	int i;
 
 	/* locked by mp_mutex */
@@ -375,9 +350,9 @@ static int page_ctor_detach(struct mp_struct *mp)
 
 	relinquish_resource(ctor);
 
-	set_memlock_rlimit(ctor, RLIMIT_MEMLOCK,
-			   ctor->o_rlim.rlim_cur,
-			   ctor->o_rlim.rlim_max);
+	down_write(&tsk->mm->mmap_sem);
+	tsk->mm->locked_vm -= ctor->locked_pages;
+	up_write(&tsk->mm->mmap_sem);
 
 	/* locked by mp_mutex */
 	ctor->dev->mp_port = NULL;
@@ -514,7 +489,6 @@ static struct page_info *mp_hash_delete(struct page_ctor *ctor,
 {
 	key_mp_t key = mp_hash(info->pages[0], HASH_BUCKETS);
 	struct page_info *tmp = NULL;
-	int i;
 
 	tmp = ctor->hash_table[key];
 	while (tmp) {
@@ -565,14 +539,11 @@ static struct page_info *alloc_page_info(struct page_ctor *ctor,
 	int rc;
 	int i, j, n = 0;
 	int len;
-	unsigned long base, lock_limit;
+	unsigned long base;
 	struct page_info *info = NULL;
 
-	lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
-	lock_limit >>= PAGE_SHIFT;
-
-	if (ctor->lock_pages + count > lock_limit && npages) {
-		printk(KERN_INFO "exceed the locked memory rlimit.");
+	if (ctor->cur_pages + count > ctor->locked_pages) {
+		printk(KERN_INFO "Exceed memory lock rlimt.");
 		return NULL;
 	}
 
@@ -634,7 +605,7 @@ static struct page_info *alloc_page_info(struct page_ctor *ctor,
 			mp_hash_insert(ctor, info->pages[i], info);
 	}
 	/* increment the number of locked pages */
-	ctor->lock_pages += j;
+	ctor->cur_pages += j;
 	return info;
 
 failed:
@@ -1006,12 +977,6 @@ proceed:
 		count--;
 	}
 
-	if (!ctor->lock_pages || !ctor->rq_len) {
-		set_memlock_rlimit(ctor, RLIMIT_MEMLOCK,
-				iocb->ki_user_data * 4096 * 2,
-				iocb->ki_user_data * 4096 * 2);
-	}
-
 	/* Translate address to kernel */
 	info = alloc_page_info(ctor, iocb, iov, count, frags, npages, 0);
 	if (!info)
@@ -1115,8 +1080,10 @@ static long mp_chr_ioctl(struct file *file, unsigned int cmd,
 	struct mp_struct *mp;
 	struct net_device *dev;
 	void __user* argp = (void __user *)arg;
+	unsigned long  __user *limitp = argp;
 	struct ifreq ifr;
 	struct sock *sk;
+	unsigned long limit, locked, lock_limit;
 	int ret;
 
 	ret = -EINVAL;
@@ -1152,6 +1119,7 @@ static long mp_chr_ioctl(struct file *file, unsigned int cmd,
 			goto err_dev_put;
 		}
 		mp->dev = dev;
+		mp->user = current;
 		ret = -ENOMEM;
 
 		sk = sk_alloc(mfile->net, AF_UNSPEC, GFP_KERNEL, &mp_proto);
@@ -1193,6 +1161,39 @@ err_dev_put:
 		ret = do_unbind(mfile);
 		break;
 
+	case MPASSTHRU_SET_MEM_LOCKED:
+		ret = copy_from_user(&limit, limitp, sizeof limit);
+		if (ret < 0)
+			return ret;
+
+		mp = mp_get(mfile);
+		if (!mp)
+			return -ENODEV;
+
+		limit = PAGE_ALIGN(limit) >> PAGE_SHIFT;
+		down_write(&current->mm->mmap_sem);
+		locked = limit + current->mm->locked_vm;
+		lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
+
+		if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) {
+			up_write(&current->mm->mmap_sem);
+			mp_put(mfile);
+			return -ENOMEM;
+		}
+		current->mm->locked_vm = locked;
+		up_write(&current->mm->mmap_sem);
+
+		mutex_lock(&mp_mutex);
+		mp->ctor->locked_pages = limit;
+		mutex_unlock(&mp_mutex);
+
+		mp_put(mfile);
+		return 0;
+
+	case MPASSTHRU_GET_MEM_LOCKED_NEED:
+		limit = DEFAULT_NEED;
+		return copy_to_user(limitp, &limit, sizeof limit);
+
 	default:
 		break;
 	}
diff --git a/include/linux/mpassthru.h b/include/linux/mpassthru.h
index ba8f320..083e9f7 100644
--- a/include/linux/mpassthru.h
+++ b/include/linux/mpassthru.h
@@ -7,6 +7,8 @@
 /* ioctl defines */
 #define MPASSTHRU_BINDDEV      _IOW('M', 213, int)
 #define MPASSTHRU_UNBINDDEV    _IO('M', 214)
+#define MPASSTHRU_SET_MEM_LOCKED	_IOW('M', 215, unsigned long)
+#define MPASSTHRU_GET_MEM_LOCKED_NEED	_IOR('M', 216, unsigned long)
 
 #ifdef __KERNEL__
 #if defined(CONFIG_MEDIATE_PASSTHRU) || defined(CONFIG_MEDIATE_PASSTHRU_MODULE)
-- 
1.7.3

^ permalink raw reply related

* Re: [PATCH] net: fix a lockdep splat
From: David Miller @ 2010-09-25  5:26 UTC (permalink / raw)
  To: penguin-kernel; +Cc: eric.dumazet, jarkao2, linux-fsdevel, netdev
In-Reply-To: <201009231532.JFD65132.OFtFSVFJHQOMLO@I-love.SAKURA.ne.jp>

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Thu, 23 Sep 2010 15:32:23 +0900

> Eric Dumazet wrote:
>> [PATCH v2] net: fix a lockdep splat
> 
> The lockdep warning disappeared for both
> linux-2.6.35.5.tar.bz2 and linux-2.6.36-rc5.tar.bz2 .

Thanks everyone, I've applied Eric's patch to net-2.6


^ permalink raw reply

* Re: [PATCH 2.6.36-rc3 1/1] IPv6: Create temporary address if none exists.
From: David Miller @ 2010-09-25  5:29 UTC (permalink / raw)
  To: gwurster
  Cc: kuznet, pekkas, jmorris, yoshfuji, kaber, shemminger,
	eric.dumazet, herbert, netdev, linux-kernel
In-Reply-To: <20100922160411.GA11185@external.electric.ath.cx>


Your email client is corrupting the patches you are sending.

Long lines are being chopped up, etc.

This makes your patches unusable, turn off all text formatting
in your email client and resend these patches again.

Thanks.

^ permalink raw reply

* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: David Miller @ 2010-09-25  5:37 UTC (permalink / raw)
  To: romieu; +Cc: sgruszka, netdev
In-Reply-To: <20100924222434.GA7743@electric-eye.fr.zoreil.com>

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 25 Sep 2010 00:24:34 +0200

> Stanislaw Gruszka <sgruszka@redhat.com> :
>> Francois Romieu <romieu@fr.zoreil.com> wrote:
> [...]
>> Anyway atomic allocation should not be used in process context.
> 
> What do you mean ? tg3->open() does not seem to bother. It is not alone.

I think this is merely an indication that r8169 is more often
used in systems that actually suspend/resume than tg3 is.  tg3
ought to be doing this too for correctness, as should pretty much
every network driver.

Stanislaw's patches are very reasonable, especially if the problem
is happening.

But yes a "Tested-by: " confirming the fix would really be
appeciated before we apply this.

^ permalink raw reply

* Re: [PATCH] de2104x: disable autonegotiation on broken hardware
From: David Miller @ 2010-09-25  5:41 UTC (permalink / raw)
  To: linux; +Cc: jgarzik, netdev, linux-kernel
In-Reply-To: <201009232318.11541.linux@rainbow-software.org>

From: Ondrej Zary <linux@rainbow-software.org>
Date: Thu, 23 Sep 2010 23:18:08 +0200

> On Thursday 23 September 2010 23:03:13 Jeff Garzik wrote:
>> On Thu, Sep 23, 2010 at 4:59 PM, Ondrej Zary <linux@rainbow-software.org> 
> wrote:
>> > At least on older 21041-AA chips (mine is rev. 11), TP duplex
>> > autonegotiation causes the card not to work at all (link is up but no
>> > packets are transmitted).
>> >
>> > de4x5 disables autonegotiation completely. But it seems to work on newer
>> > (21041-PA rev. 21) so disable it only on rev<20 chips.
>> >
>> > Signed-off-by: Ondrej Zary <linux@rainbow-software.org>
...
>> Interesting...  I never knew about that quirk.
> 
> This errata document says that autonegotiation is somehow broken but it does 
> not specify it further:
> http://ftp.nluug.nl/ftp/ftp/pub/os/NetBSD/misc/dec-docs/ec-qd2ma-te.ps.gz

Applied, thanks guys!

^ permalink raw reply

* Re: [net-next-2.6 PATCH] ixgbevf: Refactor ring parameter re-size
From: David Miller @ 2010-09-25  5:43 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, bphilips, gregory.v.rose
In-Reply-To: <20100923224543.13046.68372.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Thu, 23 Sep 2010 15:46:25 -0700

> From: Greg Rose <gregory.v.rose@intel.com>
> 
> The function to resize the Tx/Rx rings had the potential to
> dereference a NULL pointer and the code would attempt to resize
> the Tx ring even if the Rx ring allocation had failed.  This
> would cause some confusion in the return code semantics.  Fixed
> up to just unwind the allocations if any of them fail and return
> an error.
> 
> Signed-off-by: Greg Rose <gregory.v.rose@intel.com>
> Tested-by: Emil Tantilov <emil.s.tantilov@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied, thanks!

^ permalink raw reply

* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Eric Dumazet @ 2010-09-25  6:06 UTC (permalink / raw)
  To: David Miller; +Cc: romieu, sgruszka, netdev
In-Reply-To: <20100924.223709.233700415.davem@davemloft.net>

Le vendredi 24 septembre 2010 à 22:37 -0700, David Miller a écrit :
> From: Francois Romieu <romieu@fr.zoreil.com>
> Date: Sat, 25 Sep 2010 00:24:34 +0200
> 
> > Stanislaw Gruszka <sgruszka@redhat.com> :
> >> Francois Romieu <romieu@fr.zoreil.com> wrote:
> > [...]
> >> Anyway atomic allocation should not be used in process context.
> > 
> > What do you mean ? tg3->open() does not seem to bother. It is not alone.
> 
> I think this is merely an indication that r8169 is more often
> used in systems that actually suspend/resume than tg3 is.  tg3
> ought to be doing this too for correctness, as should pretty much
> every network driver.
> 

Sure  but most people use MTU=1500, so tg3 works well.

Only r8169 allocates high order pages even with normal MTU

> Stanislaw's patches are very reasonable, especially if the problem
> is happening.
> 
> But yes a "Tested-by: " confirming the fix would really be
> appeciated before we apply this.
> --

Patch solves the suspend/resume, probably, but as soon as we receive
trafic, we can hit the allocation error anyway...




^ permalink raw reply

* Re: ESP trailer_len calculation
From: Herbert Xu @ 2010-09-25  6:23 UTC (permalink / raw)
  To: David Miller; +Cc: kaber, eric.dumazet, netdev
In-Reply-To: <20100924.144044.179940003.davem@davemloft.net>

On Fri, Sep 24, 2010 at 02:40:44PM -0700, David Miller wrote:
> 
> Eric Dumazet and I recently were looking into a strange artifact in
> ESP ->trailer_len calculations.
> 
> Eric was seeing values like "17" which looked strange.
> 
> He foudn that it's because of this line in esp4.c:esp_init_state()
> 
> 	x->props.trailer_len = align + 1 + crypto_aead_authsize(esp->aead);
> 
> which comes from commit:
> 
> commit c5c2523893747f88a83376abad310c8ad13f7197
> Author: Patrick McHardy <kaber@trash.net>
> Date:   Mon Apr 9 11:47:18 2007 -0700
> 
>     [XFRM]: Optimize MTU calculation
> 
> which is based upon discussion threads:
> 
> http://marc.info/?l=linux-netdev&m=115468159401118&w=2
> 
> and
> 
> http://marc.info/?l=linux-netdev&m=117561805827241&w=2
> 
> Even more strange, in the orignal version of this patch the
> calcaluation is actually:
> 
> 	x->props.trailer_len = align - 1 + esp->auth.icv_trunc_len;
> 
> (ie. 'align - 1' instead of 'align + 1')
> 
> It seems that this "- 1 " or "+ 1" term can be completely eliminated,
> unless there are some funny semantics wrt. the padding area of ESP.
> 
> Patrick and Herbert, what do you guys think?

The number 17 does look very strange, however, after going through
the logic it does seem correct.

To calculate the minimum safe trailer length we need to consider
the worst-case scenario, and that is a packet where the payload
just happens to be one byte less than the cipher block size.

ESP always adds two bytes, then pads to at least the cipher
block size, follwed by the authentication value.  So in the
worst case we need to add

	2 + (blocksize - 1) + authlen	=
	blocksize + 1 + authlen

which is exactly what Patrick's patch does.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH 1/1] net-gre: clear skb->queue_mapping in GRE recv path
From: Eric Dumazet @ 2010-09-25  6:46 UTC (permalink / raw)
  To: chavey; +Cc: davem, netdev, Tom Herbert
In-Reply-To: <pvm39syraqr.fsf@chavey.mtv.corp.google.com>

Le vendredi 24 septembre 2010 à 16:18 -0700, chavey@google.com a écrit :
> From: chavey <chavey@google.com>
> Date: Fri, 24 Sep 2010 15:05:02 -0700
> 
> A continuous stream of errors:
> 
>   slbgre0 received packet on queue [123], but number of RX queues is 1
>   net_ratelimit: 155786 callbacks suppressed
> 
> is filling up /var/log/messages.
> 
> This is due to not unsetting skb->queue_mapping in the GRE receive path.
> 
> Signed-off-by: chavey <chavey@google.com>
> ---
>  net/ipv4/ip_gre.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c
> index 945b20a..53bdb97 100644
> --- a/net/ipv4/ip_gre.c
> +++ b/net/ipv4/ip_gre.c
> @@ -641,6 +641,7 @@ static int ipgre_rcv(struct sk_buff *skb)
>  		}
>  
>  		skb_tunnel_rx(skb, tunnel->dev);
> +		skb_set_queue_mapping(skb, 0);
>  
>  		skb_reset_network_header(skb);
>  		ipgre_ecn_decapsulate(iph, skb);


Laurent

Please use you full name in your signature, 

Then, have you seen Tom Herbert patch yesterday ?

http://patchwork.ozlabs.org/patch/65583/

He suggested to place this skb_set_queue_mapping(skb, 0) inside
skb_tunnel_rx(), so that we dont patch every tunnel driver ...

Thanks



^ permalink raw reply

* Re: [PATCH] net: reset skb queue mapping when rx'ing over tunnel
From: Eric Dumazet @ 2010-09-25  6:49 UTC (permalink / raw)
  To: Tom Herbert; +Cc: netdev, davem, chavey
In-Reply-To: <AANLkTimG9ZCZwDyo_9bYURShn7hk6ROzX4-qypo7nG5i@mail.gmail.com>

Le vendredi 24 septembre 2010 à 09:18 -0700, Tom Herbert a écrit :
> > Hmm...
> >
> > This would need to be reverted later when tunnels are updated to be
> > multiqueue aware ? I made an attempt with GRE some days ago.
> >
> > I dont understand why this patch is needed, since get_rps_cpu() has a
> > check anyway
> >
> I think the skb queue_mapping should correspond to a queue in the
> skb's device as an invariant.  In the case that an skb's device is
> change from a multiqueue device to single queue device (like GRE), the
> inconsistency in the queue_mapping is fairly innocuous, we get one
> warning but will pretty much take the unlikely branch for GRE packets
> then on.  But imagine a case where skb's device was change from one
> multiqueue device to another, but the queue mapping was not also
> updated.  This  would cause poor weighting in get_rps_cpus.  For
> example, if the new device had fewer queues than the old one,
> get_rps_cpu will bias toward using queue 0's rps mask.

I believe your patch is fine. Thanks

Acked-by: Eric Dumazet <eric.dumazet@gmail.com>



^ permalink raw reply

* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: David Miller @ 2010-09-25  7:13 UTC (permalink / raw)
  To: eric.dumazet; +Cc: romieu, sgruszka, netdev
In-Reply-To: <1285394797.2478.54.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Sat, 25 Sep 2010 08:06:37 +0200

> Patch solves the suspend/resume, probably, but as soon as we receive
> trafic, we can hit the allocation error anyway...

It allocates 1536 + N, where N can be NET_IP_ALIGN, or some small
value like 8.

This is in the same ballpark as what tg3 allocates for RX buffers.

SLAB/SLUB/whatever just wants multi-order page allocations even
for SKBs which are about this size.

Furthermore, the sleeping allocations we do at ->open() time to
allocate the entire RX ring all at once will buddy up a lot of
pages and make 1-order allocs more likely.

^ permalink raw reply

* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Eric Dumazet @ 2010-09-25  9:12 UTC (permalink / raw)
  To: David Miller; +Cc: romieu, sgruszka, netdev
In-Reply-To: <20100925.001300.193725148.davem@davemloft.net>

Le samedi 25 septembre 2010 à 00:13 -0700, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Sat, 25 Sep 2010 08:06:37 +0200
> 
> > Patch solves the suspend/resume, probably, but as soon as we receive
> > trafic, we can hit the allocation error anyway...
> 
> It allocates 1536 + N, where N can be NET_IP_ALIGN, or some small
> value like 8.
> 
> This is in the same ballpark as what tg3 allocates for RX buffers.
> 
> SLAB/SLUB/whatever just wants multi-order page allocations even
> for SKBs which are about this size.
> 
> Furthermore, the sleeping allocations we do at ->open() time to
> allocate the entire RX ring all at once will buddy up a lot of
> pages and make 1-order allocs more likely.

Yes, I forgot this problem about SLUB (I ended using SLAB on servers
because of this order-3 problem on kmalloc(2048))

bnx2 uses GFP_KERNEL allocations at init time, but tg3 uses GFP_ATOMIC
(because tp->lock is held).

The r8169 current problem is its currently copying all incoming frames.
I guess nobody cares or noticed the performance drop.
(But commit c0cd884a is recent (2.6.34), this is not yet in
production...)





^ permalink raw reply

* de2104x: fix power management
From: Ondrej Zary @ 2010-09-25  9:57 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev, Kernel development list

At least my 21041 cards come out of suspend with bus mastering disabled so
they did not work after resume(no data transferred).
After adding pci_set_master(), the driver oopsed immediately on resume -
because de_clean_rings() is called on suspend but de_init_rings() call
was missing in resume.

Also disable link (reset SIA) before sleep (de4x5 does this too).

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-2.6.36-rc3-/drivers/net/tulip/de2104x.c	2010-09-25 11:27:26.000000000 +0200
+++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c	2010-09-25 11:29:22.000000000 +0200
@@ -1231,6 +1231,7 @@ static void de_adapter_sleep (struct de_
 	if (de->de21040)
 		return;
 
+	dw32(CSR13, 0); /* Reset phy */
 	pci_read_config_dword(de->pdev, PCIPM, &pmctl);
 	pmctl |= PM_Sleep;
 	pci_write_config_dword(de->pdev, PCIPM, pmctl);
@@ -2166,6 +2167,8 @@ static int de_resume (struct pci_dev *pd
 		dev_err(&dev->dev, "pci_enable_device failed in resume\n");
 		goto out;
 	}
+	pci_set_master(pdev);
+	de_init_rings(de);
 	de_init_hw(de);
 out_attach:
 	netif_device_attach(dev);


-- 
Ondrej Zary

^ permalink raw reply

* Re: [rfc] IPVS: Masq local real-servers
From: Julian Anastasov @ 2010-09-25 13:54 UTC (permalink / raw)
  To: Simon Horman
  Cc: lvs-devel, netfilter-devel, netdev, Patrick McHardy,
	Wensong Zhang
In-Reply-To: <20100920090911.GA4646@verge.net.au>


 	Hello,

On Mon, 20 Sep 2010, Simon Horman wrote:

> IPVS has a special Local forwarding mechanism that is used if the
> real-server is a local IP address. Like the Route and Tunnel forwarding
> mechanism Local does not allow port mapping, and thus the port of the
> real-server is always set to be the same as the virtual service.
>
> The Masq forwarding mechanism does allow port mapping, and this causes some
> confusion when the real-server happens to be local.
>
> This patch addresses this confusion by not using the Local forwarding
> mechanism if the masq forwarding mechanism is requested. That is, the masq
> forwarding mechanism will be used, and the real-servers may have a
> different port to the virtual service.

 	Idea is good but there are some things to consider:

- ip_vs_nat_xmit should check the route to real server IP
but must return NF_ACCEPT instead of creating loops by
sending the packets with IP_VS_XMIT. Checks for MTU,
hard_header_len and replacing of skb_dst should be avoided
for the 'rt->rt_flags & RTCF_LOCAL' case. As result,
the packet will be DNAT-ed by IPVS and the conntrack
will be altered to expect reply from the correct local port
and then just like ip_vs_null_xmit the traffic will
reach local stack. 127.0.0.1/8 should be rejected
for the MASQ method because the output routing called
by local stack will not want to send such saddr in replies.

- We have to check if ip_vs_out is prepared to work in
LOCAL_OUT: I hope icmp_send works from this hook.

> Signed-off-by: Simon Horman <horms@verge.net.au>
>
> ---
>
> I considered using Local for the case where the real-server and virtual
> service ports are the same. However, this would require updating the
> real-servers if the port of the virtual-service was changed, however
> editing the forwarding mechanism of a real-server currently isn't
> supported and the extra complexity for an unmeasured performance gain seems
> to be at best left for another patch.
>
> Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c
> ===================================================================
> --- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_core.c	2010-09-19 20:51:30.000000000 +0900
> +++ nf-next-2.6/net/netfilter/ipvs/ip_vs_core.c	2010-09-20 16:30:59.000000000 +0900
> @@ -1496,6 +1496,22 @@ static struct nf_hook_ops ip_vs_ops[] __
> 		.hooknum        = NF_INET_FORWARD,
> 		.priority       = 100,
> 	},

 	Double block?:

> +	/* change source only for local VS/NAT */
> +	{
> +	       .hook           = ip_vs_out,
> +	       .owner          = THIS_MODULE,
> +	       .pf             = PF_INET,
> +	       .hooknum        = NF_INET_LOCAL_OUT,
> +	       .priority       = 100,
> +	},
> +	/* change source only for local VS/NAT */
> +	{
> +	       .hook           = ip_vs_out,
> +	       .owner          = THIS_MODULE,
> +	       .pf             = PF_INET,
> +	       .hooknum        = NF_INET_LOCAL_OUT,
> +	       .priority       = 100,
> +	},
> 	/* After packet filtering (but before ip_vs_out_icmp), catch icmp
> 	 * destined for 0.0.0.0/0, which is for incoming IPVS connections */
> 	{
> Index: nf-next-2.6/net/netfilter/ipvs/ip_vs_ctl.c
> ===================================================================
> --- nf-next-2.6.orig/net/netfilter/ipvs/ip_vs_ctl.c	2010-09-20 15:07:27.000000000 +0900
> +++ nf-next-2.6/net/netfilter/ipvs/ip_vs_ctl.c	2010-09-20 17:45:46.000000000 +0900
> @@ -766,7 +766,7 @@ ip_vs_zero_stats(struct ip_vs_stats *sta
>  *	Update a destination in the given service
>  */
> static void

 	'_' in __ip_vs_update_dest was lost?:

> -__ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
> +_ip_vs_update_dest(struct ip_vs_service *svc, struct ip_vs_dest *dest,
> 		    struct ip_vs_dest_user_kern *udest, int add)
> {
> 	int conn_flags;
> @@ -777,18 +777,22 @@ __ip_vs_update_dest(struct ip_vs_service
> 	conn_flags |= IP_VS_CONN_F_INACTIVE;
>
> 	/* check if local node and update the flags */

 	I think, it should work for svc->fwmark too, eg.
consolidating traffic for many virtual IPs to single
listening socket.

> +	if ((conn_flags & IP_VS_CONN_F_FWD_MASK) != IP_VS_CONN_F_MASQ ||
> +	    svc->fwmark) {
> #ifdef CONFIG_IP_VS_IPV6
> -	if (svc->af == AF_INET6) {
> -		if (__ip_vs_addr_is_local_v6(&udest->addr.in6)) {
> -			conn_flags = (conn_flags & ~IP_VS_CONN_F_FWD_MASK)
> -				| IP_VS_CONN_F_LOCALNODE;
> -		}
> -	} else
> +		if (svc->af == AF_INET6) {
> +			if (__ip_vs_addr_is_local_v6(&udest->addr.in6)) {
> +				conn_flags = (conn_flags &
> +					      ~IP_VS_CONN_F_FWD_MASK) |
> +					IP_VS_CONN_F_LOCALNODE;
> +			}
> +		} else
> #endif
> 		if (inet_addr_type(&init_net, udest->addr.ip) == RTN_LOCAL) {
> 			conn_flags = (conn_flags & ~IP_VS_CONN_F_FWD_MASK)
> 				| IP_VS_CONN_F_LOCALNODE;
> 		}
> +	}
>
> 	/* set the IP_VS_CONN_F_NOOUTPUT flag if not masquerading/NAT */
> 	if ((conn_flags & IP_VS_CONN_F_FWD_MASK) != IP_VS_CONN_F_MASQ) {
>

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: de2104x: fix power management
From: Jeff Garzik @ 2010-09-25 17:16 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: netdev, Kernel development list
In-Reply-To: <201009251157.07197.linux@rainbow-software.org>

On 09/25/2010 05:57 AM, Ondrej Zary wrote:
> At least my 21041 cards come out of suspend with bus mastering disabled so
> they did not work after resume(no data transferred).
> After adding pci_set_master(), the driver oopsed immediately on resume -
> because de_clean_rings() is called on suspend but de_init_rings() call
> was missing in resume.
>
> Also disable link (reset SIA) before sleep (de4x5 does this too).
>
> Signed-off-by: Ondrej Zary<linux@rainbow-software.org>
>
> --- linux-2.6.36-rc3-/drivers/net/tulip/de2104x.c	2010-09-25 11:27:26.000000000 +0200
> +++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c	2010-09-25 11:29:22.000000000 +0200
> @@ -1231,6 +1231,7 @@ static void de_adapter_sleep (struct de_
>   	if (de->de21040)
>   		return;
>
> +	dw32(CSR13, 0); /* Reset phy */
>   	pci_read_config_dword(de->pdev, PCIPM,&pmctl);
>   	pmctl |= PM_Sleep;
>   	pci_write_config_dword(de->pdev, PCIPM, pmctl);
> @@ -2166,6 +2167,8 @@ static int de_resume (struct pci_dev *pd
>   		dev_err(&dev->dev, "pci_enable_device failed in resume\n");
>   		goto out;
>   	}
> +	pci_set_master(pdev);
> +	de_init_rings(de);
>   	de_init_hw(de);
>   out_attach:
>   	netif_device_attach(dev);

Acked-by: Jeff Garzik <jgarzik@redhat.com>

IMO suspend/resume could use another look; if netif_running is false 
(interface is down), de2104x does not put the PCI device to sleep, which 
seems strange.

I also wonder if rtnl_lock() couldn't be eliminated, as other net 
drivers don't need it in suspend/resume.


^ permalink raw reply

* [PATCH] de2104x: fix TP link detection
From: Ondrej Zary @ 2010-09-25 20:39 UTC (permalink / raw)
  To: jgarzik; +Cc: netdev, Kernel development list

Compex FreedomLine 32 PnP-PCI2 cards have only TP and BNC connectors but the
SROM contains AUI port too. When TP loses link, the driver switches to
non-existing AUI port (which reports that carrier is always present).

Connecting TP back generates LinkPass interrupt but de_media_interrupt() is
broken - it only updates the link state of currently connected media, ignoring
the fact that LinkPass and LinkFail bits of MacStatus register belong to the
TP port only (the chip documentation says that).

This patch changes de_media_interrupt() to switch media to TP when link goes
up (and media type is not locked) and also to update the link state only when
the TP port is used.

Also the NonselPortActive (and also SelPortActive) bits of SIAStatus register
need to be cleared (by writing 1) after reading or they're useless.

Signed-off-by: Ondrej Zary <linux@rainbow-software.org>

--- linux-2.6.36-rc3-/drivers/net/tulip/de2104x.c	2010-09-25 21:19:53.000000000 +0200
+++ linux-2.6.36-rc3/drivers/net/tulip/de2104x.c	2010-09-25 21:22:11.000000000 +0200
@@ -243,6 +243,7 @@ enum {
 	NWayState		= (1 << 14) | (1 << 13) | (1 << 12),
 	NWayRestart		= (1 << 12),
 	NonselPortActive	= (1 << 9),
+	SelPortActive		= (1 << 8),
 	LinkFailStatus		= (1 << 2),
 	NetCxnErr		= (1 << 1),
 };
@@ -1066,6 +1067,9 @@ static void de21041_media_timer (unsigne
 	unsigned int carrier;
 	unsigned long flags;
 
+	/* clear port active bits */
+	dw32(SIAStatus, NonselPortActive | SelPortActive);
+
 	carrier = (status & NetCxnErr) ? 0 : 1;
 
 	if (carrier) {
@@ -1160,14 +1164,29 @@ no_link_yet:
 static void de_media_interrupt (struct de_private *de, u32 status)
 {
 	if (status & LinkPass) {
+		/* Ignore if current media is AUI or BNC and we can't use TP */
+		if ((de->media_type == DE_MEDIA_AUI ||
+		     de->media_type == DE_MEDIA_BNC) &&
+		    (de->media_lock ||
+		     !de_ok_to_advertise(de, DE_MEDIA_TP_AUTO)))
+			return;
+		/* If current media is not TP, change it to TP */
+		if ((de->media_type == DE_MEDIA_AUI ||
+		     de->media_type == DE_MEDIA_BNC)) {
+			de->media_type = DE_MEDIA_TP_AUTO;
+			de_stop_rxtx(de);
+			de_set_media(de);
+			de_start_rxtx(de);
+		}
 		de_link_up(de);
 		mod_timer(&de->media_timer, jiffies + DE_TIMER_LINK);
 		return;
 	}
 
 	BUG_ON(!(status & LinkFail));
-
-	if (netif_carrier_ok(de->dev)) {
+	/* Mark the link as down only if current media is TP */
+	if (netif_carrier_ok(de->dev) && de->media_type != DE_MEDIA_AUI &&
+	    de->media_type != DE_MEDIA_BNC) {
 		de_link_down(de);
 		mod_timer(&de->media_timer, jiffies + DE_TIMER_NO_LINK);
 	}


-- 
Ondrej Zary

^ permalink raw reply

* Re: [PATCH] de2104x: fix TP link detection
From: Jeff Garzik @ 2010-09-25 20:58 UTC (permalink / raw)
  To: Ondrej Zary; +Cc: netdev, Kernel development list
In-Reply-To: <201009252239.21376.linux@rainbow-software.org>

On Sat, Sep 25, 2010 at 4:39 PM, Ondrej Zary <linux@rainbow-software.org> wrote:
> @@ -1160,14 +1164,29 @@ no_link_yet:
>  static void de_media_interrupt (struct de_private *de, u32 status)
>  {
>        if (status & LinkPass) {
> +               /* Ignore if current media is AUI or BNC and we can't use TP */
> +               if ((de->media_type == DE_MEDIA_AUI ||
> +                    de->media_type == DE_MEDIA_BNC) &&
> +                   (de->media_lock ||
> +                    !de_ok_to_advertise(de, DE_MEDIA_TP_AUTO)))
> +                       return;
> +               /* If current media is not TP, change it to TP */
> +               if ((de->media_type == DE_MEDIA_AUI ||
> +                    de->media_type == DE_MEDIA_BNC)) {
> +                       de->media_type = DE_MEDIA_TP_AUTO;
> +                       de_stop_rxtx(de);
> +                       de_set_media(de);
> +                       de_start_rxtx(de);
> +               }
>                de_link_up(de);
>                mod_timer(&de->media_timer, jiffies + DE_TIMER_LINK);
>                return;
>        }
>
>        BUG_ON(!(status & LinkFail));
> -
> -       if (netif_carrier_ok(de->dev)) {
> +       /* Mark the link as down only if current media is TP */
> +       if (netif_carrier_ok(de->dev) && de->media_type != DE_MEDIA_AUI &&
> +           de->media_type != DE_MEDIA_BNC) {
>                de_link_down(de);
>                mod_timer(&de->media_timer, jiffies + DE_TIMER_NO_LINK);
>        }

Acked-by: Jeff Garzik <jgarzik@redhat.com>

^ permalink raw reply

* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Ben Hutchings @ 2010-09-25 23:33 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, romieu, sgruszka, netdev
In-Reply-To: <1285405945.2478.255.camel@edumazet-laptop>

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

On Sat, 2010-09-25 at 11:12 +0200, Eric Dumazet wrote:
[...]
> The r8169 current problem is its currently copying all incoming frames.
> I guess nobody cares or noticed the performance drop.
> (But commit c0cd884a is recent (2.6.34), this is not yet in
> production...)

Since that was a security fix it was backported to 2.6.32.12 and
probably most distribution kernels.  So yes it is in production.

Ben.

-- 
Ben Hutchings
Once a job is fouled up, anything done to improve it makes it worse.

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

^ permalink raw reply

* Re: Deadlock in Bluetooth code in 2.6.36
From: Gustavo F. Padovan @ 2010-09-26  0:14 UTC (permalink / raw)
  To: David Miller
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	marcel-kz+m5ild9QBg9hUCZPvPmw, mathewm-sgV2jX0FEOL9JmXXK+q4OQ
In-Reply-To: <20100924.211824.242130957.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>

* David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> [2010-09-24 21:18:24 -0700]:

> From: "Gustavo F. Padovan" <padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
> Date: Tue, 21 Sep 2010 18:20:12 -0300
> 
> > My questions here is on how to fix this properly. Maybe
> > sock_alloc_send_skb() should not be used with SOCK_STREAM and reliable
> > protocols and I'm not aware of that? And should I use something like
> > sk_stream_alloc_skb() instead?
> > 
> > Any comments are welcome. With lucky we can fix that for 2.6.36 and
> > together with others fixes we have queued deliver a fully functional
> > L2CAP layer on 2.6.36.
> 
> Use sock_alloc_send_skb() as you do now, but if it fails wait for socket
> space to become available just like TCP does, then loop back and try to
> allocate again if the space-wait doesn't return an error.

sock_alloc_send_skb() doesn't fail when there is no space available
it sleeps and try again later. That is the problem. So if
sock_alloc_send_skb() is called with the socket lock held potentially we
can have a starvation in the backlog queue and then the deadlock due to
be unable to read the acknowledgments packets residing in the backlog
queue.

Our current solution is release the lock before call
sock_alloc_send_skb() and acquire it again when sock_alloc_send_skb()
returns. Patch follows:

------
Bluetooth: Fix deadlock in the ERTM logic                        
                                                                                      
The Enhanced Retransmission Mode(ERTM) is a reliable mode of operation               
of the Bluetooth L2CAP layer. Think on it like a simplified version of                
TCP.                                                                                  
The problem we were facing here was a deadlock. ERTM uses a backlog                   
queue to queue incomimg packets while the user is helding the lock. At                
some moment the sk_sndbuf can be exceeded and we can't alloc new skbs                 
then the code sleep with the lock to wait for memory, that stalls the                 
ERTM connection once we can't read the acknowledgements packets in the                
backlog queue to free memory and make the allocation of outcoming skb
successful.

This patch actually affect all users of bt_skb_send_alloc(), i.e., all
L2CAP modes and SCO.

Signed-off-by: Gustavo F. Padovan <padovan-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
Signed-off-by: Ulisses Furquim <ulisses-Y3ZbgMPKUGA34EUeqzHoZw@public.gmane.org>
---
 include/net/bluetooth/bluetooth.h |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 27a902d..118b69b 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -161,10 +161,21 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk, unsigned long l
 {
        struct sk_buff *skb;
 
+       release_sock(sk);
        if ((skb = sock_alloc_send_skb(sk, len + BT_SKB_RESERVE, nb, err))) {
                skb_reserve(skb, BT_SKB_RESERVE);
                bt_cb(skb)->incoming  = 0;
        }
+       lock_sock(sk);
+
+       if (*err)
+               return NULL;
+
+       *err = sock_error(sk);
+       if (*err) {
+               kfree_skb(skb);
+               return NULL;
+       }
 
        return skb;
 }
-- 
1.7.2.2

-- 
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

^ permalink raw reply related

* Re: [PATCH 1/2 -next] r8169: allocate with GFP_KERNEL flag when able to sleep
From: Eric Dumazet @ 2010-09-26  6:09 UTC (permalink / raw)
  To: Ben Hutchings; +Cc: David Miller, romieu, sgruszka, netdev
In-Reply-To: <1285457597.2697.183.camel@localhost>

Le dimanche 26 septembre 2010 à 00:33 +0100, Ben Hutchings a écrit :
> On Sat, 2010-09-25 at 11:12 +0200, Eric Dumazet wrote:
> [...]
> > The r8169 current problem is its currently copying all incoming frames.
> > I guess nobody cares or noticed the performance drop.
> > (But commit c0cd884a is recent (2.6.34), this is not yet in
> > production...)
> 
> Since that was a security fix it was backported to 2.6.32.12 and
> probably most distribution kernels.  So yes it is in production.

Maybe in your company, not a single machine in mine ;)

Most linux servers in production run much older kernels.

rhel 5 -> 2.6.18
debian 5 -> 2.6.26.x




^ permalink raw reply

* [PATCH 1/9] ibm_newemac: use free_netdev(netdev) instead of kfree()
From: Vasiliy Kulikov @ 2010-09-26  9:58 UTC (permalink / raw)
  To: kernel-janitors-u79uwXL29TY76Z2rM5mHXA
  Cc: Jiri Pirko, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, David S. Miller

Freeing netdev without free_netdev() leads to net, tx leaks.
I might lead to dereferencing freed pointer.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

@@
struct net_device* dev;
@@

-kfree(dev)
+free_netdev(dev)
---
 drivers/net/ibm_newemac/core.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c
index 3506fd6..519e19e 100644
--- a/drivers/net/ibm_newemac/core.c
+++ b/drivers/net/ibm_newemac/core.c
@@ -2928,7 +2928,7 @@ static int __devinit emac_probe(struct platform_device *ofdev,
 	if (dev->emac_irq != NO_IRQ)
 		irq_dispose_mapping(dev->emac_irq);
  err_free:
-	kfree(ndev);
+	free_netdev(ndev);
  err_gone:
 	/* if we were on the bootlist, remove us as we won't show up and
 	 * wake up all waiters to notify them in case they were waiting
@@ -2971,7 +2971,7 @@ static int __devexit emac_remove(struct platform_device *ofdev)
 	if (dev->emac_irq != NO_IRQ)
 		irq_dispose_mapping(dev->emac_irq);
 
-	kfree(dev->ndev);
+	free_netdev(dev->ndev);
 
 	return 0;
 }
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 2/9] rionet: use free_netdev(netdev) instead of kfree()
From: Vasiliy Kulikov @ 2010-09-26  9:58 UTC (permalink / raw)
  To: kernel-janitors; +Cc: Tejun Heo, netdev, linux-kernel

Freeing netdev without free_netdev() leads to net, tx leaks.
I might lead to dereferencing freed pointer.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

@@
struct net_device* dev;
@@

-kfree(dev)
+free_netdev(dev)
---
 drivers/net/rionet.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/rionet.c b/drivers/net/rionet.c
index 07eb884..44150f2 100644
--- a/drivers/net/rionet.c
+++ b/drivers/net/rionet.c
@@ -384,7 +384,7 @@ static void rionet_remove(struct rio_dev *rdev)
 	free_pages((unsigned long)rionet_active, rdev->net->hport->sys_size ?
 					__ilog2(sizeof(void *)) + 4 : 0);
 	unregister_netdev(ndev);
-	kfree(ndev);
+	free_netdev(ndev);
 
 	list_for_each_entry_safe(peer, tmp, &rionet_peers, node) {
 		list_del(&peer->node);
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 3/9] sgiseeq: use free_netdev(netdev) instead of kfree()
From: Vasiliy Kulikov @ 2010-09-26  9:58 UTC (permalink / raw)
  To: kernel-janitors
  Cc: David S. Miller, Eric Dumazet, Uwe Kleine-König,
	Julia Lawall, Tejun Heo, netdev, linux-kernel

Freeing netdev without free_netdev() leads to net, tx leaks.
I might lead to dereferencing freed pointer.

The semantic match that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

@@
struct net_device* dev;
@@

-kfree(dev)
+free_netdev(dev)
---
 drivers/net/sgiseeq.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/sgiseeq.c b/drivers/net/sgiseeq.c
index cc4bd8c..9265315 100644
--- a/drivers/net/sgiseeq.c
+++ b/drivers/net/sgiseeq.c
@@ -804,7 +804,7 @@ static int __devinit sgiseeq_probe(struct platform_device *pdev)
 err_out_free_page:
 	free_page((unsigned long) sp->srings);
 err_out_free_dev:
-	kfree(dev);
+	free_netdev(dev);
 
 err_out:
 	return err;
-- 
1.7.0.4

^ permalink raw reply related

* Re: kvm networking todo wiki
From: Michael S. Tsirkin @ 2010-09-26 11:21 UTC (permalink / raw)
  To: Sridhar Samudrala
  Cc: David Stevens, sri, Anthony Liguori, Rusty Russell,
	Krishna Kumar2, Shirley Ma, Xin, Xiaohui, jdike, herbert, lmr,
	akong, kvm, qemu-devel, netdev
In-Reply-To: <4C9A873E.6010403@us.ibm.com>

On Wed, Sep 22, 2010 at 03:46:22PM -0700, Sridhar Samudrala wrote:
> On Tue, 2010-09-21 at 18:11 +0200, Michael S. Tsirkin wrote:
> 
>     I've put up a wiki page with a kvm networking todo list,
>     mainly to avoid effort duplication, but also in the hope
>     to draw attention to what I think we should try addressing
>     in KVM:
> 
>     http://www.linux-kvm.org/page/NetworkingTodo
> 
>     This page could cover all networking related activity in KVM,
>     currently most info is related to virtio-net.
> 
>     Note: if there's no developer listed for an item,
>     this just means I don't know of anyone actively working
>     on an issue at the moment, not that no one intends to.
> 
>     I would appreciate it if others working on one of the items on this list
>     would add their names so we can communicate better.  If others like this
>     wiki page, please go ahead and add stuff you are working on if any.
> 
>     It would be especially nice to add autotest projects:
>     there is just a short test matrix and a catch-all
>     'Cover test matrix with autotest', currently.
> 
>     Currently there are some links to Red Hat bugzilla entries,
>     feel free to add links to other bugzillas.
> 
> 
> Thanks for capturing these items. It is really useful.
> 
> Another item that is missing is
> - support assigning SR-IOV VF to a guest via tap/macvtap
> 
> Currently, this requires
>  - VF to be put in promiscuous mode when using a bridge/tap
>  - add a new mac address to VF when using macvtap.
> 
> I don't think any of the VF drivers provide these capabilities
> at this time.
> 
> -Sridhar

I think this is part of what is needed for the work item:
    *  guest programmable mac/vlan filtering with macvtap 
If yes pls add this detail if not add another item.
More importantly: anyone's going to work on this?


-- 
MST


^ 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