Netdev List
 help / color / mirror / Atom feed
* Re: Gianfar driver failing on MPC8641D based board
From: Anton Vorontsov @ 2010-02-26 14:35 UTC (permalink / raw)
  To: Martyn Welch
  Cc: linuxppc-dev list, netdev, linux-kernel, Paul Gortmaker,
	Sandeep Gopalpet, davem
In-Reply-To: <4B87B937.6010204@ge.com>

On Fri, Feb 26, 2010 at 12:06:15PM +0000, Martyn Welch wrote:
> Anton Vorontsov wrote:
> > On Thu, Feb 25, 2010 at 07:53:30PM -0500, Paul Gortmaker wrote:
> > [...]
> >   
> >> I was able to reproduce it on an 8641D and bisected it down to this:
> >>
> >> -----------
> >> commit a3bc1f11e9b867a4f49505ecac486a33af248b2e
> >> Author: Anton Vorontsov <avorontsov@ru.mvista.com>
> >> Date:   Tue Nov 10 14:11:10 2009 +0000
> >>
> >>     gianfar: Revive SKB recycling
> >>     
> >
> > Thanks for the bisect. I have a guess why tx hangs in
> > SMP case. Could anyone try the patch down below?
> >   
> 
> Yup, no problem. I'm afraid it doesn't resolve the problem for me.

Hm.. I found a p2020 board and I was able to reproduce the issue.
The patch down below fixed it completely for me... hm.

I'll look further, thanks!

> > [...]
> >   
> >> ...which probably explains why you weren't seeing it on non-SMP.
> >> I'd imagine it would show up on any of the e500mc boards too.
> >>     
> >
> > Yeah.. Pity, I don't have SMP boards anymore. I'll try
> > to get one though.
> >
> >
> > diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c
> > index 8bd3c9f..3ff3bd0 100644
> > --- a/drivers/net/gianfar.c
> > +++ b/drivers/net/gianfar.c
> > @@ -2614,6 +2614,8 @@ static int gfar_poll(struct napi_struct *napi, int budget)
> >  			tx_queue = priv->tx_queue[rx_queue->qindex];
> >  
> >  			tx_cleaned += gfar_clean_tx_ring(tx_queue);
> > +			if (!tx_cleaned && !tx_queue->num_txbdfree)
> > +				tx_cleaned += 1; /* don't complete napi */
> >  			rx_cleaned_per_queue = gfar_clean_rx_ring(rx_queue,
> >  							budget_per_queue);
> >  			rx_cleaned += rx_cleaned_per_queue;
> >   

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply

* [PATCH] Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo and return it
From: Jeff Garzik @ 2010-02-26 13:44 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, Peter P Waskiewicz Jr
In-Reply-To: <20100226115355.20213.59254.stgit@localhost.localdomain>

On Fri, Feb 26, 2010 at 03:54:20AM -0800, Jeff Kirsher wrote:
> From: Peter Waskiewicz <peter.p.waskiewicz.jr@intel.com>
> 
> The drvinfo struct should include the number of strings that
> get_rx_ntuple will return.  It will be variable if an underlying
> driver implements its own get_rx_ntuple routine, so userspace
> needs to know how much data is coming.
> 
> Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

As noted in the other email, your patch breaks ABI.  The proper path is
to decrease the size of reserved struct member, and NOT shift the offset
of other members.



However, perhaps consider the following patch for returning n-tuple
count, for four reasons:

1) space in ethtool_drvinfo is limited

2) the patch below permits trivial string set addition, without
   ABI changes beyond adding a new ETH_SS_xxx constant.

3) the patch below permits direct access to ops->get_sset_count(),
   rather than implicit access via ethtool_drvinfo

4) ethtool_drvinfo interface does not permit indication of 
   ops->get_sset_count() failure, versus returning zero value.  The
   patch below does so, via output sset_mask.

WARNING: this patch is compile-tested only.

NOTE: I added a cosmetic fix to ETHTOOL_[GS]RXNTUPLE constants, making
their indentation consistent with the rest of the list of constants.

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

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index f7992a2..04ecf18 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -254,6 +254,17 @@ struct ethtool_gstrings {
 	__u8	data[0];
 };
 
+struct ethtool_sset_info {
+	__u32	cmd;		/* ETHTOOL_GSSET_INFO */
+	__u32	reserved;
+	__u64	sset_mask;	/* input: each bit selects an sset to query */
+				/* output: each bit a returned sset */
+	__u32	data[0];	/* ETH_SS_xxx count, in order, based on bits
+				   in sset_mask.  One bit implies one
+				   __u32, two bits implies two
+				   __u32's, etc. */
+};
+
 enum ethtool_test_flags {
 	ETH_TEST_FL_OFFLINE	= (1 << 0),	/* online / offline */
 	ETH_TEST_FL_FAILED	= (1 << 1),	/* test passed / failed */
@@ -607,9 +618,9 @@ struct ethtool_ops {
 #define	ETHTOOL_SRXCLSRLINS	0x00000032 /* Insert RX classification rule */
 #define	ETHTOOL_FLASHDEV	0x00000033 /* Flash firmware to device */
 #define	ETHTOOL_RESET		0x00000034 /* Reset hardware */
-
-#define ETHTOOL_SRXNTUPLE	0x00000035 /* Add an n-tuple filter to device */
-#define ETHTOOL_GRXNTUPLE	0x00000036 /* Get n-tuple filters from device */
+#define	ETHTOOL_SRXNTUPLE	0x00000035 /* Add an n-tuple filter to device */
+#define	ETHTOOL_GRXNTUPLE	0x00000036 /* Get n-tuple filters from device */
+#define	ETHTOOL_GSSET_INFO	0x00000037 /* Get string set info */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1c94f48..c019f92 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -241,6 +241,69 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
 /*
  * noinline attribute so that gcc doesnt use too much stack in dev_ethtool()
  */
+static noinline int ethtool_get_sset_info(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_sset_info info;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	u64 sset_mask;
+	int i, idx = 0, n_bits = 0, ret, rc;
+	u32 *info_buf = NULL;
+
+	if (!ops->get_sset_count)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&info, useraddr, sizeof(info)))
+		return -EFAULT;
+
+	/* store copy of mask, because we zero struct later on */
+	sset_mask = info.sset_mask;
+	if (!sset_mask)
+		return 0;
+
+	/* calculate size of return buffer */
+	for (i = 0; i < 64; i++)
+		if (sset_mask & (1ULL << i))
+			n_bits++;
+
+	memset(&info, 0, sizeof(info));
+	info.cmd = ETHTOOL_GSSET_INFO;
+
+	info_buf = kzalloc(n_bits * sizeof(u32), GFP_USER);
+	if (!info_buf)
+		return -ENOMEM;
+
+	/* fill return buffer based on input bitmask and successful 
+	 * get_sset_count return
+	 */
+	for (i = 0; i < 64; i++) {
+		if (!(sset_mask & (1ULL << i)))
+			continue;
+
+		rc = ops->get_sset_count(dev, i);
+		if (rc >= 0) {
+			info.sset_mask |= (1ULL << i);
+			info_buf[idx++] = rc;
+		}
+	}
+
+	ret = -EFAULT;
+	if (copy_to_user(useraddr, &info, sizeof(info)))
+		goto out;
+
+	useraddr += offsetof(struct ethtool_sset_info, data);
+	if (copy_to_user(useraddr, info_buf, idx * sizeof(u32)))
+		goto out;
+
+	ret = 0;
+
+out:
+	kfree(info_buf);
+	return ret;
+}
+
+/*
+ * noinline attribute so that gcc doesnt use too much stack in dev_ethtool()
+ */
 static noinline int ethtool_set_rxnfc(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_rxnfc cmd;
@@ -1472,6 +1535,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GRXNTUPLE:
 		rc = ethtool_get_rx_ntuple(dev, useraddr);
 		break;
+	case ETHTOOL_GSSET_INFO:
+		rc = ethtool_get_sset_info(dev, useraddr);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}

^ permalink raw reply related

* Re: [net-next-2.6 PATCH 3/3] ixgbe: Do not allocate too many netdev txqueues
From: Eric Dumazet @ 2010-02-26 14:04 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, John Fastabend
In-Reply-To: <20100226091519.19796.82441.stgit@localhost.localdomain>

Le vendredi 26 février 2010 à 01:15 -0800, Jeff Kirsher a écrit :
> From: John Fastabend <john.r.fastabend@intel.com>
> 
> Instead of allocating 128 struct netdev_queue per device, use the
> minimum value between 128 and the number of possible txq's, to
> reduce ram usage and "tc -s -d class shod dev .." output.
> 
> This patch fixes Eric Dumazet's patch to set the TX queues to
> the correct minimum.
> 
> Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
> 
>  drivers/net/ixgbe/ixgbe_main.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
> index 4a01022..a961da2 100644
> --- a/drivers/net/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ixgbe/ixgbe_main.c
> @@ -5996,6 +5996,7 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
>  	const struct ixgbe_info *ii = ixgbe_info_tbl[ent->driver_data];
>  	static int cards_found;
>  	int i, err, pci_using_dac;
> +	unsigned int indices = num_possible_cpus();
>  #ifdef IXGBE_FCOE
>  	u16 device_caps;
>  #endif
> @@ -6034,7 +6035,18 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
>  	pci_set_master(pdev);
>  	pci_save_state(pdev);
>  
> -	netdev = alloc_etherdev_mq(sizeof(struct ixgbe_adapter), MAX_TX_QUEUES);
> +	if (ii->mac == ixgbe_mac_82598EB)
> +		indices = min_t(unsigned int, indices, IXGBE_MAX_RSS_INDICES);
> +	else
> +		indices = min_t(unsigned int, indices, IXGBE_MAX_FDIR_INDICES);
> +
> +	indices = max_t(unsigned int, indices, IXGBE_MAX_DCB_INDICES);
> +#ifdef IXGBE_FCOE
> +	indices += min_t(unsigned int, num_possible_cpus(),
> +			 IXGBE_MAX_FCOE_INDICES);
> +#endif
> +	indices = min_t(unsigned int, indices, MAX_TX_QUEUES);
> +	netdev = alloc_etherdev_mq(sizeof(struct ixgbe_adapter), indices);
>  	if (!netdev) {
>  		err = -ENOMEM;
>  		goto err_alloc_etherdev;
> 

Thanks Jeff, but what is the reason for limiting to MAX_TX_QUEUES ?
Is it a hardware issue ?



^ permalink raw reply

* Re: Advice on RCU for IPVS
From: Eric Dumazet @ 2010-02-26 14:00 UTC (permalink / raw)
  To: Simon Horman; +Cc: netdev, lvs-devel
In-Reply-To: <20100226031812.GC6111@verge.net.au>

Le vendredi 26 février 2010 à 14:18 +1100, Simon Horman a écrit :
> Hi,
> 
> I have been looking at converting the rwlocks in IPVS over to use RCU.
> A problem that I am facing is that the lblcr scheduler uses
> a write lock on list A and then taking a write lock on list B.
> Where list B is basically part of one of the elements of list A.
> 
> This problem is present in ip_vs_lblcr_schedule() and
> the key code looks like this.
> 
> 
> 	/* First look in our cache */
> 	read_lock(&svc->sched_lock);
> 	en = ip_vs_lblcr_get(svc->af, tbl, &iph.daddr);
> 	if (en) {
> 		...
> 
> 		/* Get the least loaded destination */
> 		read_lock(&en->set.lock);
> 		dest = ip_vs_dest_set_min(&en->set);
> 		read_unlock(&en->set.lock);
> 
> 		...
> 
> 			write_lock(&en->set.lock);
> 			m = ip_vs_dest_set_max(&en->set);
> 			if (m)
> 				ip_vs_dest_set_erase(&en->set, m);
> 			write_unlock(&en->set.lock);
> 
> 		...
> 
> 		/* Update our cache entry */
> 		write_lock(&en->set.lock);
> 		ip_vs_dest_set_insert(&en->set, dest);
> 		write_unlock(&en->set.lock);
> 	}
> 	read_unlock(&svc->sched_lock);
> 
> dest is referenced counted and doesn't seem to need to be guarded
> by svc->sched_lock.
> 
> It seems to me that this is quite difficult to convert over to RCU
> as there are write-side critical sections inside a read-side critical
> section.
> 
> I investigated reference counting the return value of
> ip_vs_lblcr_get() or the return value of ip_vs_dest_set_max() and
> ip_vs_dest_set_insert(). But this seems to be difficult,
> especially at rmmod time.
> 
> I also considered just making the whole thing a write-side critical section.
> Which seems to be somewhat of a sledge-hammer and result in
> a critical section that is much larger than I would like. Though
> no bigger than the existing area covered by the read-lock on
> svc->sched_lock.
> 
> Any suggestions would be appreciated.

The code you copy/pasted seems really complex, I would suggest to make
it as simple as possible (using spinlocks for example instead of
rwlocks) before considering RCU conversion.




^ permalink raw reply

* Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo and return it
From: Jeff Garzik @ 2010-02-26 13:59 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, Peter P Waskiewicz Jr
In-Reply-To: <4B87D31B.4000001@garzik.org>

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

On 02/26/2010 08:56 AM, Jeff Garzik wrote:
> However, perhaps consider the following patch for returning n-tuple
> count, for four reasons:
>
> 1) space in ethtool_drvinfo is limited
>
> 2) the patch below permits trivial string set addition, without
> ABI changes beyond adding a new ETH_SS_xxx constant.
>
> 3) the patch below permits direct access to ops->get_sset_count(),
> rather than implicit access via ethtool_drvinfo
>
> 4) ethtool_drvinfo interface does not permit indication of
> ops->get_sset_count() failure, versus returning zero value. The
> patch below does so, via output sset_mask.
>
> WARNING: this patch is compile-tested only.
>
> NOTE: I added a cosmetic fix to ETHTOOL_[GS]RXNTUPLE constants, making
> their indentation consistent with the rest of the list of constants.
>
> Signed-off-by: Jeff Garzik <jgarzik@redhat.com>


With, perhaps, a note like the attached as a reminder to folks about 
future additions.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 455 bytes --]

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 31b1edd..e376e8c 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -212,6 +212,9 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
 	info.cmd = ETHTOOL_GDRVINFO;
 	ops->get_drvinfo(dev, &info);
 
+	/* this method of obtaining string set info is deprecated;
+	 * consider using ETHTOOL_GSSET_INFO instead
+	 */
 	if (ops->get_sset_count) {
 		int rc;
 

^ permalink raw reply related

* Re: [RFC] IPVS: Convert connection table lock over to RCU
From: Eric Dumazet @ 2010-02-26 13:57 UTC (permalink / raw)
  To: Simon Horman
  Cc: netdev, lvs-devel, Wensong Zhang, Julian Anastasov,
	Patrick McHardy, David S. Miller
In-Reply-To: <20100226030054.GA6111@verge.net.au>

Le vendredi 26 février 2010 à 14:00 +1100, Simon Horman a écrit :
> Signed-off-by: Simon Horman <horms@verge.net.au>
> 
> --- 
> 
> This seems to be a fairly clean conversion to me. But its my journey
> into the world of RCU, so I would appreciate a careful review.
> 
> I have deliberately introduced some noise into this patch
> in the form of changing the name of some global variables and functions.
> This is in order to clearly highlight changes at the call-sites.
> 
> The table of 16 locks (4 bits) used for the connection table seems
> to be somewhat arbitrary to me, this patch intentionally leaves
> that as is.
> 
> Index: net-next-2.6/net/netfilter/ipvs/ip_vs_conn.c
> ===================================================================
> --- net-next-2.6.orig/net/netfilter/ipvs/ip_vs_conn.c	2010-02-26 10:42:16.000000000 +1100
> +++ net-next-2.6/net/netfilter/ipvs/ip_vs_conn.c	2010-02-26 10:52:32.000000000 +1100
> @@ -35,6 +35,8 @@
>  #include <linux/seq_file.h>
>  #include <linux/jhash.h>
>  #include <linux/random.h>
> +#include <linux/spinlock.h>
> +#include <linux/rculist.h>
>  
>  #include <net/net_namespace.h>
>  #include <net/ip_vs.h>
> @@ -75,57 +77,37 @@ static unsigned int ip_vs_conn_rnd;
>  /*
>   *  Fine locking granularity for big connection hash table
>   */
> -#define CT_LOCKARRAY_BITS  4
> -#define CT_LOCKARRAY_SIZE  (1<<CT_LOCKARRAY_BITS)
> -#define CT_LOCKARRAY_MASK  (CT_LOCKARRAY_SIZE-1)
> +#define CT_MUTEX_BITS  4
> +#define CT_MUTEX_SIZE  (1<<CT_MUTEX_BITS)
> +#define CT_MUTEX_MASK  (CT_MUTEX_SIZE-1)
>  
> -struct ip_vs_aligned_lock
> +struct ip_vs_aligned_spinlock
>  {
> -	rwlock_t	l;
> +	spinlock_t	l;
>  } __attribute__((__aligned__(SMP_CACHE_BYTES)));
>  
> -/* lock array for conn table */
> -static struct ip_vs_aligned_lock
> -__ip_vs_conntbl_lock_array[CT_LOCKARRAY_SIZE] __cacheline_aligned;
> +/* mutex array for connection table */
> +static struct ip_vs_aligned_spinlock
> +__ip_vs_conntbl_mutex[CT_MUTEX_SIZE] __cacheline_aligned;
>  
> -static inline void ct_read_lock(unsigned key)
> +static inline void ct_mutex_lock(unsigned key)
>  {
> -	read_lock(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
> +	spin_lock(&__ip_vs_conntbl_mutex[key&CT_MUTEX_MASK].l);
>  }
>  
> -static inline void ct_read_unlock(unsigned key)
> +static inline void ct_mutex_unlock(unsigned key)
>  {
> -	read_unlock(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
> +	spin_unlock(&__ip_vs_conntbl_mutex[key&CT_MUTEX_MASK].l);
>  }
>  
> -static inline void ct_write_lock(unsigned key)
> +static inline void ct_mutex_lock_bh(unsigned key)
>  {
> -	write_lock(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
> +	spin_lock_bh(&__ip_vs_conntbl_mutex[key&CT_MUTEX_MASK].l);
>  }
>  
> -static inline void ct_write_unlock(unsigned key)
> +static inline void ct_mutex_unlock_bh(unsigned key)
>  {
> -	write_unlock(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
> -}
> -
> -static inline void ct_read_lock_bh(unsigned key)
> -{
> -	read_lock_bh(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
> -}
> -
> -static inline void ct_read_unlock_bh(unsigned key)
> -{
> -	read_unlock_bh(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
> -}
> -
> -static inline void ct_write_lock_bh(unsigned key)
> -{
> -	write_lock_bh(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
> -}
> -
> -static inline void ct_write_unlock_bh(unsigned key)
> -{
> -	write_unlock_bh(&__ip_vs_conntbl_lock_array[key&CT_LOCKARRAY_MASK].l);
> +	spin_unlock_bh(&__ip_vs_conntbl_mutex[key&CT_MUTEX_MASK].l);
>  }
>  
> 
> @@ -155,27 +137,27 @@ static unsigned int ip_vs_conn_hashkey(i
>  static inline int ip_vs_conn_hash(struct ip_vs_conn *cp)
>  {
>  	unsigned hash;
> -	int ret;
>  
>  	/* Hash by protocol, client address and port */
>  	hash = ip_vs_conn_hashkey(cp->af, cp->protocol, &cp->caddr, cp->cport);
>  
> -	ct_write_lock(hash);
> +	ct_mutex_lock(hash);
>  
>  	if (!(cp->flags & IP_VS_CONN_F_HASHED)) {
> -		list_add(&cp->c_list, &ip_vs_conn_tab[hash]);
> +		list_add_rcu(&cp->c_list, &ip_vs_conn_tab[hash]);
>  		cp->flags |= IP_VS_CONN_F_HASHED;
>  		atomic_inc(&cp->refcnt);
> -		ret = 1;
> -	} else {
> -		pr_err("%s(): request for already hashed, called from %pF\n",
> -		       __func__, __builtin_return_address(0));
> -		ret = 0;
> +		ct_mutex_unlock(hash);
> +		synchronize_rcu();

Why is synchronize_rcu() necessary here ?

When adding a new item in a list, you dont need any rcu grace period.

> +		return 1;
>  	}
>  
> -	ct_write_unlock(hash);
> +	ct_mutex_unlock(hash);
>  
> -	return ret;
> +	pr_err("%s(): request for already hashed, called from %pF\n",
> +	       __func__, __builtin_return_address(0));
> +
> +	return 0;
>  }
>  
> 
> @@ -186,24 +168,24 @@ static inline int ip_vs_conn_hash(struct
>  static inline int ip_vs_conn_unhash(struct ip_vs_conn *cp)
>  {
>  	unsigned hash;
> -	int ret;
>  
>  	/* unhash it and decrease its reference counter */
>  	hash = ip_vs_conn_hashkey(cp->af, cp->protocol, &cp->caddr, cp->cport);
>  
> -	ct_write_lock(hash);
> +	ct_mutex_lock(hash);
>  
>  	if (cp->flags & IP_VS_CONN_F_HASHED) {
> -		list_del(&cp->c_list);
> +		list_del_rcu(&cp->c_list);
>  		cp->flags &= ~IP_VS_CONN_F_HASHED;
>  		atomic_dec(&cp->refcnt);
> -		ret = 1;
> -	} else
> -		ret = 0;
> +		ct_mutex_unlock(hash);
> +		synchronize_rcu();

Are you sure we can afford a synchronize_rcu() call here ?

This is a very long primitive, and I bet this is not acceptable for IPVS
use case.

> +		return 1;
> +	}
>  
> -	ct_write_unlock(hash);
> +	ct_mutex_unlock(hash);
>  
> -	return ret;
> +	return 0;
>  }
>  
> 
> @@ -222,9 +204,9 @@ static inline struct ip_vs_conn *__ip_vs
>  
>  	hash = ip_vs_conn_hashkey(af, protocol, s_addr, s_port);
>  
> -	ct_read_lock(hash);
> +	rcu_read_lock();
>  
> -	list_for_each_entry(cp, &ip_vs_conn_tab[hash], c_list) {
> +	list_for_each_entry_rcu(cp, &ip_vs_conn_tab[hash], c_list) {
>  		if (cp->af == af &&
>  		    ip_vs_addr_equal(af, s_addr, &cp->caddr) &&
>  		    ip_vs_addr_equal(af, d_addr, &cp->vaddr) &&
> @@ -233,12 +215,12 @@ static inline struct ip_vs_conn *__ip_vs
>  		    protocol == cp->protocol) {
>  			/* HIT */
>  			atomic_inc(&cp->refcnt);
> -			ct_read_unlock(hash);
> +			rcu_read_unlock();
>  			return cp;
>  		}
>  	}
>  
> -	ct_read_unlock(hash);
> +	rcu_read_unlock();
>  
>  	return NULL;
>  }
> @@ -273,9 +255,9 @@ struct ip_vs_conn *ip_vs_ct_in_get
>  
>  	hash = ip_vs_conn_hashkey(af, protocol, s_addr, s_port);
>  
> -	ct_read_lock(hash);
> +	rcu_read_lock();
>  
> -	list_for_each_entry(cp, &ip_vs_conn_tab[hash], c_list) {
> +	list_for_each_entry_rcu(cp, &ip_vs_conn_tab[hash], c_list) {
>  		if (cp->af == af &&
>  		    ip_vs_addr_equal(af, s_addr, &cp->caddr) &&
>  		    /* protocol should only be IPPROTO_IP if
> @@ -293,7 +275,7 @@ struct ip_vs_conn *ip_vs_ct_in_get
>  	cp = NULL;
>  
>    out:
> -	ct_read_unlock(hash);
> +	rcu_read_unlock();
>  
>  	IP_VS_DBG_BUF(9, "template lookup/in %s %s:%d->%s:%d %s\n",
>  		      ip_vs_proto_name(protocol),
> @@ -322,9 +304,9 @@ struct ip_vs_conn *ip_vs_conn_out_get
>  	 */
>  	hash = ip_vs_conn_hashkey(af, protocol, d_addr, d_port);
>  
> -	ct_read_lock(hash);
> +	rcu_read_lock();
>  
> -	list_for_each_entry(cp, &ip_vs_conn_tab[hash], c_list) {
> +	list_for_each_entry_rcu(cp, &ip_vs_conn_tab[hash], c_list) {
>  		if (cp->af == af &&
>  		    ip_vs_addr_equal(af, d_addr, &cp->caddr) &&
>  		    ip_vs_addr_equal(af, s_addr, &cp->daddr) &&
> @@ -337,7 +319,7 @@ struct ip_vs_conn *ip_vs_conn_out_get
>  		}
>  	}
>  
> -	ct_read_unlock(hash);
> +	rcu_read_unlock();
>  
>  	IP_VS_DBG_BUF(9, "lookup/out %s %s:%d->%s:%d %s\n",
>  		      ip_vs_proto_name(protocol),
> @@ -776,14 +758,16 @@ static void *ip_vs_conn_array(struct seq
>  	struct ip_vs_conn *cp;
>  
>  	for (idx = 0; idx < ip_vs_conn_tab_size; idx++) {
> -		ct_read_lock_bh(idx);
> -		list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
> +		rcu_read_lock_bh();
> +		list_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
>  			if (pos-- == 0) {
>  				seq->private = &ip_vs_conn_tab[idx];
> +				/* N.B: no rcu_read_unlock_bh() here
> +				 *      Seems really horrible :-( */
>  				return cp;
>  			}
>  		}
> -		ct_read_unlock_bh(idx);
> +		rcu_read_unlock_bh();
>  	}
>  
>  	return NULL;
> @@ -807,19 +791,22 @@ static void *ip_vs_conn_seq_next(struct
>  
>  	/* more on same hash chain? */
>  	if ((e = cp->c_list.next) != l)
> -		return list_entry(e, struct ip_vs_conn, c_list);
> +		return list_entry_rcu(e, struct ip_vs_conn, c_list);
>  
>  	idx = l - ip_vs_conn_tab;
> -	ct_read_unlock_bh(idx);
> +	rcu_read_unlock_bh();
>  
>  	while (++idx < ip_vs_conn_tab_size) {
> -		ct_read_lock_bh(idx);
> -		list_for_each_entry(cp, &ip_vs_conn_tab[idx], c_list) {
> +		rcu_read_lock_bh();
> +		list_for_each_entry_rcu(cp, &ip_vs_conn_tab[idx], c_list) {
>  			seq->private = &ip_vs_conn_tab[idx];
> +			/* N.B: no rcu_read_unlock_bh() here
> +			 *      Seems really horrible :-( */

... if you add a comment, please write why you need to keep rcu locked
... or dont add a comment, since this construct is quite common.




^ permalink raw reply

* Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo and return it
From: Jeff Garzik @ 2010-02-26 13:56 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, netdev, gospo, Peter P Waskiewicz Jr
In-Reply-To: <20100226115355.20213.59254.stgit@localhost.localdomain>

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

On 02/26/2010 06:54 AM, Jeff Kirsher wrote:
> From: Peter Waskiewicz<peter.p.waskiewicz.jr@intel.com>
>
> The drvinfo struct should include the number of strings that
> get_rx_ntuple will return.  It will be variable if an underlying
> driver implements its own get_rx_ntuple routine, so userspace
> needs to know how much data is coming.
>
> Signed-off-by: Peter P Waskiewicz Jr<peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
> ---
>
>   include/linux/ethtool.h |    1 +
>   net/core/ethtool.c      |    3 +++
>   2 files changed, 4 insertions(+), 0 deletions(-)

(resending reply, standard patch-sending box is having trouble sending 
to vger)


As noted in the other email, your patch breaks ABI.  The proper path is
to decrease the size of reserved struct member, and NOT shift the offset
of other members.



However, perhaps consider the following patch for returning n-tuple
count, for four reasons:

1) space in ethtool_drvinfo is limited

2) the patch below permits trivial string set addition, without
    ABI changes beyond adding a new ETH_SS_xxx constant.

3) the patch below permits direct access to ops->get_sset_count(),
    rather than implicit access via ethtool_drvinfo

4) ethtool_drvinfo interface does not permit indication of
    ops->get_sset_count() failure, versus returning zero value.  The
    patch below does so, via output sset_mask.

WARNING: this patch is compile-tested only.

NOTE: I added a cosmetic fix to ETHTOOL_[GS]RXNTUPLE constants, making
their indentation consistent with the rest of the list of constants.

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



[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 3582 bytes --]

diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
index f7992a2..04ecf18 100644
--- a/include/linux/ethtool.h
+++ b/include/linux/ethtool.h
@@ -254,6 +254,17 @@ struct ethtool_gstrings {
 	__u8	data[0];
 };
 
+struct ethtool_sset_info {
+	__u32	cmd;		/* ETHTOOL_GSSET_INFO */
+	__u32	reserved;
+	__u64	sset_mask;	/* input: each bit selects an sset to query */
+				/* output: each bit a returned sset */
+	__u32	data[0];	/* ETH_SS_xxx count, in order, based on bits
+				   in sset_mask.  One bit implies one
+				   __u32, two bits implies two
+				   __u32's, etc. */
+};
+
 enum ethtool_test_flags {
 	ETH_TEST_FL_OFFLINE	= (1 << 0),	/* online / offline */
 	ETH_TEST_FL_FAILED	= (1 << 1),	/* test passed / failed */
@@ -607,9 +618,9 @@ struct ethtool_ops {
 #define	ETHTOOL_SRXCLSRLINS	0x00000032 /* Insert RX classification rule */
 #define	ETHTOOL_FLASHDEV	0x00000033 /* Flash firmware to device */
 #define	ETHTOOL_RESET		0x00000034 /* Reset hardware */
-
-#define ETHTOOL_SRXNTUPLE	0x00000035 /* Add an n-tuple filter to device */
-#define ETHTOOL_GRXNTUPLE	0x00000036 /* Get n-tuple filters from device */
+#define	ETHTOOL_SRXNTUPLE	0x00000035 /* Add an n-tuple filter to device */
+#define	ETHTOOL_GRXNTUPLE	0x00000036 /* Get n-tuple filters from device */
+#define	ETHTOOL_GSSET_INFO	0x00000037 /* Get string set info */
 
 /* compatibility with older code */
 #define SPARC_ETH_GSET		ETHTOOL_GSET
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 1c94f48..c019f92 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -241,6 +241,69 @@ static noinline int ethtool_get_drvinfo(struct net_device *dev, void __user *use
 /*
  * noinline attribute so that gcc doesnt use too much stack in dev_ethtool()
  */
+static noinline int ethtool_get_sset_info(struct net_device *dev, void __user *useraddr)
+{
+	struct ethtool_sset_info info;
+	const struct ethtool_ops *ops = dev->ethtool_ops;
+	u64 sset_mask;
+	int i, idx = 0, n_bits = 0, ret, rc;
+	u32 *info_buf = NULL;
+
+	if (!ops->get_sset_count)
+		return -EOPNOTSUPP;
+
+	if (copy_from_user(&info, useraddr, sizeof(info)))
+		return -EFAULT;
+
+	/* store copy of mask, because we zero struct later on */
+	sset_mask = info.sset_mask;
+	if (!sset_mask)
+		return 0;
+
+	/* calculate size of return buffer */
+	for (i = 0; i < 64; i++)
+		if (sset_mask & (1ULL << i))
+			n_bits++;
+
+	memset(&info, 0, sizeof(info));
+	info.cmd = ETHTOOL_GSSET_INFO;
+
+	info_buf = kzalloc(n_bits * sizeof(u32), GFP_USER);
+	if (!info_buf)
+		return -ENOMEM;
+
+	/* fill return buffer based on input bitmask and successful 
+	 * get_sset_count return
+	 */
+	for (i = 0; i < 64; i++) {
+		if (!(sset_mask & (1ULL << i)))
+			continue;
+
+		rc = ops->get_sset_count(dev, i);
+		if (rc >= 0) {
+			info.sset_mask |= (1ULL << i);
+			info_buf[idx++] = rc;
+		}
+	}
+
+	ret = -EFAULT;
+	if (copy_to_user(useraddr, &info, sizeof(info)))
+		goto out;
+
+	useraddr += offsetof(struct ethtool_sset_info, data);
+	if (copy_to_user(useraddr, info_buf, idx * sizeof(u32)))
+		goto out;
+
+	ret = 0;
+
+out:
+	kfree(info_buf);
+	return ret;
+}
+
+/*
+ * noinline attribute so that gcc doesnt use too much stack in dev_ethtool()
+ */
 static noinline int ethtool_set_rxnfc(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_rxnfc cmd;
@@ -1472,6 +1535,9 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 	case ETHTOOL_GRXNTUPLE:
 		rc = ethtool_get_rx_ntuple(dev, useraddr);
 		break;
+	case ETHTOOL_GSSET_INFO:
+		rc = ethtool_get_sset_info(dev, useraddr);
+		break;
 	default:
 		rc = -EOPNOTSUPP;
 	}

^ permalink raw reply related

* Re: [ethtool PATCH 2/2] ethtool: Update manpage with n-tuple information
From: Jeff Garzik @ 2010-02-26 13:51 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: netdev, gospo, Peter P Waskiewicz Jr
In-Reply-To: <20100226115603.20259.93232.stgit@localhost.localdomain>

On 02/26/2010 06:56 AM, Jeff Kirsher wrote:
> From: Peter Waskiewicz<peter.p.waskiewicz.jr@intel.com>
>
> Add n-tuple information to the ethtool manpage.
>
> Signed-off-by: Peter P Waskiewicz Jr<peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
> ---
>
>   ethtool.8 |  102 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 files changed, 102 insertions(+), 0 deletions(-)

applied



^ permalink raw reply

* Re: [net-next-2.6 PATCH] Preserve queue mapping with bonding and VLAN devices
From: David Miller @ 2010-02-26 13:50 UTC (permalink / raw)
  To: sysoleg; +Cc: bhutchings, netdev
In-Reply-To: <68631266953760@webmail134.yandex.ru>

From: "\"Oleg A. Arkhangelsky\"" <sysoleg@yandex.ru>
Date: Tue, 23 Feb 2010 22:36:00 +0300

> 23.02.10, 15:59, "Ben Hutchings" <bhutchings@solarflare.com>:
> 
>>  The queue mapping will normally be the same, only no longer biased by 1.
>>  So I think a better solution would be to maintain that bias on TX as
>>  well, or to remove the bias and reserve -1 for unknown RX queue.
> 
> Second try. Not tested but looks OK.
> 
> Must be applied with "[net-next-2.6 PATCH] Multiqueue support for bonding devices"
> 
> Forwarded packet goes through dev_queue_xmit() more that once when using bonding
> or 802.1q VLAN devices, so we've lost rx-tx queue mapping index for real devices.
> This is because initial queue index value (as it recorded by skb_record_tx_queue())
> is overwritten by skb_set_queue_mapping().
> 
> Signed-off-by: Oleg A. Arkhangelsky <sysoleg@yandex.ru> 

Your patches are fine, but they are whitespace corrupted by your
email client (tabs turned into spaces, etc.) so they won't apply
properly.

Please fix this up and resend your patches.

Thanks!

^ permalink raw reply

* Re: [PATCH v3] can: ti hecc module : add platform specific initialization callback.
From: David Miller @ 2010-02-26 13:22 UTC (permalink / raw)
  To: srk-l0cyMroinI0
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, mkl-bIcnvbaLZ9MEGnE8C9+IrQ
In-Reply-To: <1267190078-22328-1-git-send-email-srk-l0cyMroinI0@public.gmane.org>

From: Sriramakrishnan <srk-l0cyMroinI0@public.gmane.org>
Date: Fri, 26 Feb 2010 18:44:38 +0530

> CAN module on AM3517 requires programming of IO expander as part
> of init sequence - to enable CAN PHY. Added platform specific
> callback to handle phy control(switch on /off).
> 
> Signed-off-by: Sriramakrishnan <srk-l0cyMroinI0@public.gmane.org>

Applied, thanks.

^ permalink raw reply

* [PATCH v3] can: ti hecc module : add platform specific initialization callback.
From: Sriramakrishnan @ 2010-02-26 13:14 UTC (permalink / raw)
  To: netdev; +Cc: socketcan-core, mkl, Sriramakrishnan

CAN module on AM3517 requires programming of IO expander as part
of init sequence - to enable CAN PHY. Added platform specific
callback to handle phy control(switch on /off).

Signed-off-by: Sriramakrishnan <srk@ti.com>
---
Addressed review comments and also added conditional compile
macros to gaurd against multiple inclusion of header file.

 drivers/net/can/ti_hecc.c            |   17 ++++++++++++++++-
 include/linux/can/platform/ti_hecc.h |    8 ++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/net/can/ti_hecc.c b/drivers/net/can/ti_hecc.c
index df27d82..ceba29e 100644
--- a/drivers/net/can/ti_hecc.c
+++ b/drivers/net/can/ti_hecc.c
@@ -28,9 +28,11 @@
  *         .mbx_offset             = 0x2000,
  *         .int_line               = 0,
  *         .revision               = 1,
+ *         .transceiver_switch     = hecc_phy_control,
  * };
  *
- * Please see include/can/platform/ti_hecc.h for description of above fields
+ * Please see include/linux/can/platform/ti_hecc.h for description of
+ * above fields.
  *
  */
 
@@ -220,6 +222,7 @@ struct ti_hecc_priv {
 	u32 tx_head;
 	u32 tx_tail;
 	u32 rx_next;
+	void (*transceiver_switch)(int);
 };
 
 static inline int get_tx_head_mb(struct ti_hecc_priv *priv)
@@ -317,6 +320,13 @@ static int ti_hecc_set_btc(struct ti_hecc_priv *priv)
 	return 0;
 }
 
+static void ti_hecc_transceiver_switch(const struct ti_hecc_priv *priv,
+					int on)
+{
+	if (priv->transceiver_switch)
+		priv->transceiver_switch(on);
+}
+
 static void ti_hecc_reset(struct net_device *ndev)
 {
 	u32 cnt;
@@ -816,10 +826,13 @@ static int ti_hecc_open(struct net_device *ndev)
 		return err;
 	}
 
+	ti_hecc_transceiver_switch(priv, 1);
+
 	/* Open common can device */
 	err = open_candev(ndev);
 	if (err) {
 		dev_err(ndev->dev.parent, "open_candev() failed %d\n", err);
+		ti_hecc_transceiver_switch(priv, 0);
 		free_irq(ndev->irq, ndev);
 		return err;
 	}
@@ -840,6 +853,7 @@ static int ti_hecc_close(struct net_device *ndev)
 	ti_hecc_stop(ndev);
 	free_irq(ndev->irq, ndev);
 	close_candev(ndev);
+	ti_hecc_transceiver_switch(priv, 0);
 
 	return 0;
 }
@@ -901,6 +915,7 @@ static int ti_hecc_probe(struct platform_device *pdev)
 	priv->hecc_ram_offset = pdata->hecc_ram_offset;
 	priv->mbx_offset = pdata->mbx_offset;
 	priv->int_line = pdata->int_line;
+	priv->transceiver_switch = pdata->transceiver_switch;
 
 	priv->can.bittiming_const = &ti_hecc_bittiming_const;
 	priv->can.do_set_mode = ti_hecc_do_set_mode;
diff --git a/include/linux/can/platform/ti_hecc.h b/include/linux/can/platform/ti_hecc.h
index 4688c7b..af17cb3 100644
--- a/include/linux/can/platform/ti_hecc.h
+++ b/include/linux/can/platform/ti_hecc.h
@@ -1,3 +1,6 @@
+#ifndef __CAN_PLATFORM_TI_HECC_H__
+#define __CAN_PLATFORM_TI_HECC_H__
+
 /*
  * TI HECC (High End CAN Controller) driver platform header
  *
@@ -23,6 +26,7 @@
  * @mbx_offset:		Mailbox RAM offset
  * @int_line:		Interrupt line to use - 0 or 1
  * @version:		version for future use
+ * @transceiver_switch:	platform specific callback fn for transceiver control
  *
  * Platform data structure to get all platform specific settings.
  * this structure also accounts the fact that the IP may have different
@@ -35,6 +39,6 @@ struct ti_hecc_platform_data {
 	u32 mbx_offset;
 	u32 int_line;
 	u32 version;
+	void (*transceiver_switch) (int);
 };
-
-
+#endif
-- 
1.6.2.4


^ permalink raw reply related

* Re: [RFC PATCH] accounting for socket backlog
From: Eric Dumazet @ 2010-02-26 13:12 UTC (permalink / raw)
  To: Zhu Yi; +Cc: netdev@vger.kernel.org, Shi, Alex
In-Reply-To: <1267151683.16986.1646.camel@debian>

Le vendredi 26 février 2010 à 10:34 +0800, Zhu Yi a écrit :
> On Thu, 2010-02-25 at 19:24 +0800, Eric Dumazet wrote:
> > - What about other protocols that also use a backlog ?
> 
> I don't think protocols with flow/congestion control capability have
> such issue. We have tested TCP is immune. Other current backlog users
> are dccp, sctp, tipc, x.25 and llc. We didn't test all of them. But
> looks like only llc here is possible but unlikely?
> 

I looked again for TCP case and I see same problem than UDP. If socket
is locked by user, and a softirq flood happens, you can fill backlog too
with frames (any frame with correct tuple, even with bad sequences), and
exhaust memory.


> > - __release_sock() could run forever with no preemption, even with a
> > limit on backlog. 
> 
> Yes, but there is no critical impact like memory exhausted for this
> case.
> 

Well, if you have one processor, and a process doesnt want to yield the
cpu (apart of sofirq of course that is filling the backlog while your
process tries to empty it), your machine is dead. This is critical
too :)




^ permalink raw reply

* Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo and return it
From: David Miller @ 2010-02-26 13:11 UTC (permalink / raw)
  To: jeff; +Cc: jeffrey.t.kirsher, netdev, gospo, peter.p.waskiewicz.jr
In-Reply-To: <4B87C70F.8060403@garzik.org>

From: Jeff Garzik <jeff@garzik.org>
Date: Fri, 26 Feb 2010 08:05:19 -0500

> NAK.  Did you even read the patch?

I did, brain doesn't work sometimes :-)

> We don't increase the size of struct ethtool_drvinfo, _especially_ by
> sticking struct members into the middle of the struct.
> 
> What do you think 'reserved' is for???

Right, I'll revert, thanks.

^ permalink raw reply

* Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo and return it
From: Jeff Garzik @ 2010-02-26 13:05 UTC (permalink / raw)
  To: David Miller; +Cc: jeffrey.t.kirsher, netdev, gospo, peter.p.waskiewicz.jr
In-Reply-To: <20100226.042027.250812962.davem@davemloft.net>

On 02/26/2010 07:20 AM, David Miller wrote:
> From: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
> Date: Fri, 26 Feb 2010 03:54:20 -0800
>
>> From: Peter Waskiewicz<peter.p.waskiewicz.jr@intel.com>
>>
>> The drvinfo struct should include the number of strings that
>> get_rx_ntuple will return.  It will be variable if an underlying
>> driver implements its own get_rx_ntuple routine, so userspace
>> needs to know how much data is coming.
>>
>> Signed-off-by: Peter P Waskiewicz Jr<peter.p.waskiewicz.jr@intel.com>
>> Signed-off-by: Jeff Kirsher<jeffrey.t.kirsher@intel.com>
>
> Applied.

NAK.  Did you even read the patch?

We don't increase the size of struct ethtool_drvinfo, _especially_ by 
sticking struct members into the middle of the struct.

What do you think 'reserved' is for???

	Jeff




^ permalink raw reply

* Re: [PATCH net-next] ipv6: Use 1280 as min MTU for ipv6 forwarding
From: David Miller @ 2010-02-26 12:34 UTC (permalink / raw)
  To: uweber; +Cc: netdev
In-Reply-To: <20100226123244.GB9114@babylon>

From: Ulrich Weber <uweber@astaro.com>
Date: Fri, 26 Feb 2010 13:32:44 +0100

> Clients will set their MTU to 1280 if they receive a
> ICMPV6_PKT_TOOBIG message with an MTU less than 1280.
> 
> To allow encapsulating of packets over a 1280 link
> we should always accept packets with a size of 1280
> for forwarding even if the path has a lower MTU and
> fragment the encapsulated packets afterwards.
> 
> In case a forwarded packet is not going to be encapsulated
> a ICMPV6_PKT_TOOBIG msg will still be send by ip6_fragment()
> with the correct MTU.
> 
> Signed-off-by: Ulrich Weber <uweber@astaro.com>

Applied, thanks for fixing up this patch.

^ permalink raw reply

* [PATCH net-next] ipv6: Use 1280 as min MTU for ipv6 forwarding
From: Ulrich Weber @ 2010-02-26 12:32 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <20100226.035947.242568281.davem@davemloft.net>

Clients will set their MTU to 1280 if they receive a
ICMPV6_PKT_TOOBIG message with an MTU less than 1280.

To allow encapsulating of packets over a 1280 link
we should always accept packets with a size of 1280
for forwarding even if the path has a lower MTU and
fragment the encapsulated packets afterwards.

In case a forwarded packet is not going to be encapsulated
a ICMPV6_PKT_TOOBIG msg will still be send by ip6_fragment()
with the correct MTU.

Signed-off-by: Ulrich Weber <uweber@astaro.com>
---
 net/ipv6/ip6_output.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 1a5fe9a..dabf108 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -402,6 +402,7 @@ int ip6_forward(struct sk_buff *skb)
 	struct ipv6hdr *hdr = ipv6_hdr(skb);
 	struct inet6_skb_parm *opt = IP6CB(skb);
 	struct net *net = dev_net(dst->dev);
+	u32 mtu;
 
 	if (net->ipv6.devconf_all->forwarding == 0)
 		goto error;
@@ -508,10 +509,14 @@ int ip6_forward(struct sk_buff *skb)
 		}
 	}
 
-	if (skb->len > dst_mtu(dst)) {
+	mtu = dst_mtu(dst);
+	if (mtu < IPV6_MIN_MTU)
+		mtu = IPV6_MIN_MTU;
+
+	if (skb->len > mtu) {
 		/* Again, force OUTPUT device used as source address */
 		skb->dev = dst->dev;
-		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, dst_mtu(dst));
+		icmpv6_send(skb, ICMPV6_PKT_TOOBIG, 0, mtu);
 		IP6_INC_STATS_BH(net,
 				 ip6_dst_idev(dst), IPSTATS_MIB_INTOOBIGERRORS);
 		IP6_INC_STATS_BH(net,
@@ -621,8 +626,7 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 	mtu = ip6_skb_dst_mtu(skb);
 
 	/* We must not fragment if the socket is set to force MTU discovery
-	 * or if the skb it not generated by a local socket.  (This last
-	 * check should be redundant, but it's free.)
+	 * or if the skb it not generated by a local socket.
 	 */
 	if (!skb->local_df) {
 		skb->dev = skb_dst(skb)->dev;
-- 
1.6.3.3


^ permalink raw reply related

* Re: [PATCHv2] can: ti hecc module : add platform specific initialization callback.
From: David Miller @ 2010-02-26 12:32 UTC (permalink / raw)
  To: mkl-bIcnvbaLZ9MEGnE8C9+IrQ
  Cc: srk-l0cyMroinI0, socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4B86BAB8.8090603-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

From: Marc Kleine-Budde <mkl-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Date: Thu, 25 Feb 2010 19:00:24 +0100

> Sriramakrishnan wrote:
>> CAN module on AM3517 requires programming of IO expander as part
>> of init sequence - to enable CAN PHY. Added platform specific
>> callback to handle phy control(switch on /off).
>> 
>> Signed-off-by: Sriramakrishnan <srk-l0cyMroinI0@public.gmane.org>
> 
> looks good, just minor nitpicking inline

Please address the feedback and resubmit this patch.

Thank you.

^ permalink raw reply

* Re: Bridge: request for the via_phys_dev feature discussion
From: David Miller @ 2010-02-26 12:30 UTC (permalink / raw)
  To: xemul; +Cc: shemminger, netdev
In-Reply-To: <4B7D30B8.2050400@openvz.org>

From: Pavel Emelyanov <xemul@openvz.org>
Date: Thu, 18 Feb 2010 15:21:12 +0300

> In simple situation containers users want to bridge their containers through
> veth device with the host eth0 adapter. But after one add his eth0 to bridge
> he need to
> a) reconfigure everything including IP addresses, routing tables and
>    netfilter rules
> b) recreate connections that were bound to eth0
> 
> That's OK if one setup the box from the scratch and adds eth0 to bridge from
> the very beginning, but for those who start using containers on pre-configured
> boxes or for those, who decided to switch to bridge+veth from some other 
> virtual device (e.g. macvlan or venet device in OpenVZ) this becomes real pain.
> 
> I don't insist you accept the patches Cyrill proposed, I don't even insist
> we rework them keeping the idea intact. I just want to know your opinion about
> how to solve the above problem better.

Stephen, please work with Pavel on finding a solution for this issue
that works for you.

Thanks.

^ permalink raw reply

* Re: [net-next-2.6 PATCH] infiniband: convert to use netdev_for_each_mc_addr
From: David Miller @ 2010-02-26 12:22 UTC (permalink / raw)
  To: jpirko-H+wXaHxf7aLQT0dZR+AlfA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	faisal.latif-ral2JQCrhuEAvxtiuMwx3w,
	chien.tin.tung-ral2JQCrhuEAvxtiuMwx3w,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20100224151108.GD2663-YzwxZg+R7evMbnheQZGK0N5OCZ2W11yPFxja6HXR22MAvxtiuMwx3w@public.gmane.org>

From: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Wed, 24 Feb 2010 16:11:08 +0100

> Due to the loop complexicity in nes_nic.c, I'm using char* to copy mc addresses
> to it.
> 
> Signed-off-by: Jiri Pirko <jpirko-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [net-next-2.6 PATCH] infiniband: convert to use netdev_for_each_mc_addr
From: David Miller @ 2010-02-26 12:22 UTC (permalink / raw)
  To: jpirko; +Cc: ogerlitz, netdev, jgunthorpe, linux-rdma
In-Reply-To: <20100225084915.GA3171@psychotron.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Thu, 25 Feb 2010 09:49:15 +0100

> Subject: [net-next-2.6 PATCH] net: add addr len check to dev_mc_add
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH] ethtool: Add n-tuple string length to drvinfo and return it
From: David Miller @ 2010-02-26 12:20 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, peter.p.waskiewicz.jr
In-Reply-To: <20100226115355.20213.59254.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Fri, 26 Feb 2010 03:54:20 -0800

> From: Peter Waskiewicz <peter.p.waskiewicz.jr@intel.com>
> 
> The drvinfo struct should include the number of strings that
> get_rx_ntuple will return.  It will be variable if an underlying
> driver implements its own get_rx_ntuple routine, so userspace
> needs to know how much data is coming.
> 
> Signed-off-by: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [PATCH] greth: fall through to common return statement on error
From: David Miller @ 2010-02-26 12:20 UTC (permalink / raw)
  To: horms; +Cc: netdev, kristoffer, jpirko, kirjanov
In-Reply-To: <20100225235019.GA26133@verge.net.au>

From: Simon Horman <horms@verge.net.au>
Date: Fri, 26 Feb 2010 10:50:19 +1100

> There doesn't seem to be any reason to explicitly return
> NETDEV_TX_OK as err is set to NETDEV_TX_OK in all cases that
> reach this point.
> 
> Signed-off-by: Simon Horman <horms@verge.net.au>

Looks fine, applied.

^ permalink raw reply

* Re: [PATCH] netdev: use list_first_entry macro
From: David Miller @ 2010-02-26 12:20 UTC (permalink / raw)
  To: shemminger; +Cc: netdev
In-Reply-To: <20100224160138.3a8de8de@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 24 Feb 2010 16:01:38 -0800

> Use list_first_entry macro; no longer any need to use 
> 'next' directly in list to find first entry.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH] rtnetlink: clean up SR-IOV config interface
From: David Miller @ 2010-02-26 12:19 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: netdev, gospo, mitch.a.williams
In-Reply-To: <20100225075950.18665.50738.stgit@localhost.localdomain>

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Wed, 24 Feb 2010 23:59:56 -0800

> From: Williams, Mitch A <mitch.a.williams@intel.com>
> 
> This patch consists of a few minor cleanups to the SR-IOV
> configurion code in rtnetlink.
> - Remove unneccesary lock
> - Remove unneccesary casts
> - Return correct error code for no driver support
> 
> These changes are based on comments from Patrick McHardy
> 
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>

Applied.

^ permalink raw reply

* Re: [net-next-2.6 PATCH] af_packet: do not accept mc address smaller then dev->addr_len in packet_mc_add()
From: David Miller @ 2010-02-26 12:19 UTC (permalink / raw)
  To: jpirko; +Cc: netdev
In-Reply-To: <20100225095703.GB2667@psychotron.lab.eng.brq.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Thu, 25 Feb 2010 10:57:04 +0100

> There is no point of accepting an address of smaller length than dev->addr_len
> here. Therefore change this for stonger check.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>

Applied.

^ 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