Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH 00/12] sfc changes for 2.6.36
From: Ben Hutchings @ 2010-06-02 13:52 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <20100602.022144.63047920.davem@davemloft.net>

On Wed, 2010-06-02 at 02:21 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Tue, 01 Jun 2010 22:16:07 +0100
> 
> > The major feature here is RX buffer recycling, which improves
> > performance on networks with heavy multicast traffic.  Other than that,
> > there are various bug fixes and cleanup.
> > 
> > Ben.
> > 
> > Ben Hutchings (3):
> >   sfc: Rename struct efx_mcdi_phy_cfg to efx_mcdi_phy_data
> >   sfc: Only count bad packets in rx_errors
> >   sfc: Get port number from CS_PORT_NUM, not PCI function number
> > 
> > Steve Hodgson (9):
> >   sfc: Reschedule any resets scheduled inside efx_pm_freeze()
> >   sfc: Workaround flush failures on Falcon B0
> >   sfc: Synchronise link_advertising and wanted_fc on Siena
> >   sfc: Wait for the link to stay up before running loopback selftest
> >   sfc: Allow DRV_GEN events to be used outside of selftests
> >   sfc: Remove efx_rx_queue::add_lock
> >   sfc: Support only two rx buffers per page
> >   sfc: Recycle discarded rx buffers back onto the queue
> >   sfc: Allow shared pages to be recycled
> 
> ALl applied, thanks Ben.

Did you apply patch 12 "sfc: Get port number from CS_PORT_NUM, not PCI
function number" to net-2.6?

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* Re: [PATCH 00/12] sfc changes for 2.6.36
From: David Miller @ 2010-06-02 13:57 UTC (permalink / raw)
  To: bhutchings; +Cc: netdev, linux-net-drivers
In-Reply-To: <1275486746.2870.31.camel@localhost>

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Wed, 02 Jun 2010 14:52:26 +0100

> Did you apply patch 12 "sfc: Get port number from CS_PORT_NUM, not PCI
> function number" to net-2.6?

When you mix bug fixes and cleanups, I'm going to apply it all to
net-next-2.6 You didn't even specify an intended destination in your
subject lines, so you leave it entirely open for interpretation and
my choice.

So, if you want a bug fix added to net-2.6, tossing it into a series
which is not wholly appropriate for net-next-2.6 is not that way to
accomplish that.

The way to do it is:

1) Submit the bug fix, explicitly state "[PATCH net-2.6 xxx] "
   in your subject line.

2) Prepare your net-next-2.6 changes on top of that, and do one
   of two things:
   a) Explicitly state in your patch series in the "[PATCH net-next-2.6 0/xxx] "
      posting "this depends upon the bug fixes posted in the series
      XXX posted earlier.
   b) Wait for the bug fix to reach net-next-2.6 when I do a merge,
      then post your series.

Otherwise you make life way to miserable and complicated for me,
the one who has to review and integrate all of your work.

Thanks.

^ permalink raw reply

* [PATCH v2] cls_u32: use skb_header_pointer() to dereference data safely
From: Changli Gao @ 2010-06-02 14:00 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

use skb_header_pointer() to dereference data safely

the original skb->data dereference isn't safe, as there isn't any skb->len or
skb_is_nonlinear() check. skb_header_pointer() is used instead in this patch.
And when the skb isn't long enough, we terminate the function u32_classify()
immediately with -1. Unaligned access is also fixed.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/cls_u32.c |   51 ++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 38 insertions(+), 13 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 9627542..e6dba22 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -41,6 +41,7 @@
 #include <net/netlink.h>
 #include <net/act_api.h>
 #include <net/pkt_cls.h>
+#include <asm/unaligned.h>
 
 struct tc_u_knode
 {
@@ -98,11 +99,11 @@ static int u32_classify(struct sk_buff *skb, struct tcf_proto *tp, struct tcf_re
 {
 	struct {
 		struct tc_u_knode *knode;
-		u8		  *ptr;
+		unsigned int	  off;
 	} stack[TC_U32_MAXDEPTH];
 
 	struct tc_u_hnode *ht = (struct tc_u_hnode*)tp->root;
-	u8 *ptr = skb_network_header(skb);
+	unsigned int off = skb_network_header(skb) - skb->data;
 	struct tc_u_knode *n;
 	int sdepth = 0;
 	int off2 = 0;
@@ -134,8 +135,14 @@ next_knode:
 #endif
 
 		for (i = n->sel.nkeys; i>0; i--, key++) {
-
-			if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->val)&key->mask) {
+			unsigned int toff;
+			__be32 *data, _data;
+
+			toff = off + key->off + (off2 & key->offmask);
+			data = skb_header_pointer(skb, toff, 4, &_data);
+			if (!data)
+				goto out;
+			if ((get_unaligned(data) ^ key->val) & key->mask) {
 				n = n->next;
 				goto next_knode;
 			}
@@ -174,29 +181,46 @@ check_terminal:
 		if (sdepth >= TC_U32_MAXDEPTH)
 			goto deadloop;
 		stack[sdepth].knode = n;
-		stack[sdepth].ptr = ptr;
+		stack[sdepth].off = off;
 		sdepth++;
 
 		ht = n->ht_down;
 		sel = 0;
-		if (ht->divisor)
-			sel = ht->divisor&u32_hash_fold(*(__be32*)(ptr+n->sel.hoff), &n->sel,n->fshift);
-
+		if (ht->divisor) {
+			__be32 *data, _data;
+
+			data = skb_header_pointer(skb, off + n->sel.hoff, 4,
+						  &_data);
+			if (!data)
+				goto out;
+			sel = ht->divisor & u32_hash_fold(get_unaligned(data),
+							  &n->sel, n->fshift);
+		}
 		if (!(n->sel.flags&(TC_U32_VAROFFSET|TC_U32_OFFSET|TC_U32_EAT)))
 			goto next_ht;
 
 		if (n->sel.flags&(TC_U32_OFFSET|TC_U32_VAROFFSET)) {
 			off2 = n->sel.off + 3;
-			if (n->sel.flags&TC_U32_VAROFFSET)
-				off2 += ntohs(n->sel.offmask & *(__be16*)(ptr+n->sel.offoff)) >>n->sel.offshift;
+			if (n->sel.flags & TC_U32_VAROFFSET) {
+				__be16 *data, _data;
+
+				data = skb_header_pointer(skb,
+							  off + n->sel.offoff,
+							  2, &_data);
+				if (!data)
+					goto out;
+				off2 += ntohs(n->sel.offmask &
+					      get_unaligned(data)) >>
+					n->sel.offshift;
+			}
 			off2 &= ~3;
 		}
 		if (n->sel.flags&TC_U32_EAT) {
-			ptr += off2;
+			off += off2;
 			off2 = 0;
 		}
 
-		if (ptr < skb_tail_pointer(skb))
+		if (off < skb->len)
 			goto next_ht;
 	}
 
@@ -204,9 +228,10 @@ check_terminal:
 	if (sdepth--) {
 		n = stack[sdepth].knode;
 		ht = n->ht_up;
-		ptr = stack[sdepth].ptr;
+		off = stack[sdepth].off;
 		goto check_terminal;
 	}
+out:
 	return -1;
 
 deadloop:

^ permalink raw reply related

* Re: [PATCH] netdev:bfin_mac: reclaim and free tx skb as soon as possible after transfer
From: David Miller @ 2010-06-02 14:02 UTC (permalink / raw)
  To: sonic.adi; +Cc: netdev, uclinux-dist-devel
In-Reply-To: <1275383974.1010.5.camel@eight.analog.com>

From: sonic zhang <sonic.adi@gmail.com>
Date: Tue, 1 Jun 2010 17:19:34 +0800

> @@ -1485,6 +1506,10 @@ static int __devinit bfin_mac_probe(struct platform_device *pdev)
>  	ndev->netdev_ops = &bfin_mac_netdev_ops;
>  	ndev->ethtool_ops = &bfin_mac_ethtool_ops;
>  
> +	init_timer(&lp->tx_reclaim_timer);
> +	lp->tx_reclaim_timer.data = (unsigned long)(lp);

Putting parenthesis around "lp" is excessive, please don't do this.

> +	lp->tx_reclaim_timer.function = (void *)tx_reclaim_skb;
> +
>  	spin_lock_init(&lp->lock);

Do not cast the function pointer to "void *" like this, it's ugly and entirely
not necessary.

Instead have a wrapper function that uses the correct types as specified
for timer functions, that calls the existing function.

^ permalink raw reply

* Re: [PATCH] ipconfig: send host-name in DHCP requests
From: David Miller @ 2010-06-02 14:05 UTC (permalink / raw)
  To: fengguang.wu; +Cc: netdev, linux-kernel, andi
In-Reply-To: <20100531031953.GA10590@localhost>

From: Wu Fengguang <fengguang.wu@intel.com>
Date: Mon, 31 May 2010 11:19:53 +0800

> Normally dhclient can be configured to send the "host-name" option
> in DHCP requests to update the client's DNS record. However for an
> NFSROOT system, dhclient shall never be called (which may change the
> IP addr and therefore lose your root NFS mount connection).
> 
> So enable updating the DNS record with kernel parameter
> 
> 	ip=::::$HOST_NAME::dhcp
> 
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 2/2] mac8390: raise error logging priority
From: David Miller @ 2010-06-02 14:06 UTC (permalink / raw)
  To: fthain; +Cc: geert, joe, p_gortmaker, netdev, linux-kernel, linux-m68k
In-Reply-To: <alpine.OSX.2.00.1006011739160.299@localhost>

From: Finn Thain <fthain@telegraphics.com.au>
Date: Tue, 1 Jun 2010 22:18:56 +1000 (EST)

> Log error conditions using KERN_ERR priority.
> 
> Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

Applied, thank you.

^ permalink raw reply

* Re: [PATCHv3] Refactor update of IPv6 flowi destination address for srcrt (RH) option
From: David Miller @ 2010-06-02 14:08 UTC (permalink / raw)
  To: arno; +Cc: joe, yoshfuji, netdev
In-Reply-To: <87sk55q44a.fsf_-_@small.ssi.corp>

From: arno@natisbad.org (Arnaud Ebalard)
Date: Wed, 02 Jun 2010 09:35:01 +0200

> Here is a v3 based on the comments I received from Joe. The helper is
> now in exthdrs.c (no more inline) and has a proper name.
> 
> Applies on net-2.6. Tested on 2.6.34.

Looks good, applied to net-next-2.6, thanks.

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: replace hooks in __netif_receive_skb V5
From: David Miller @ 2010-06-02 14:11 UTC (permalink / raw)
  To: jpirko; +Cc: netdev, kaber, eric.dumazet, shemminger
In-Reply-To: <20100602075207.GD2603@psychotron.redhat.com>

From: Jiri Pirko <jpirko@redhat.com>
Date: Wed, 2 Jun 2010 09:52:08 +0200

> What this patch does is it removes two receive frame hooks (for bridge and for
> macvlan) from __netif_receive_skb. These are replaced them with a single
> hook for both. It only supports one hook per device because it makes no
> sense to do bridging and macvlan on the same device.
> 
> Then a network driver (of virtual netdev like macvlan or bridge) can register
> an rx_handler for needed net device.
> 
> Signed-off-by: Jiri Pirko <jpirko@redhat.com>
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Ok, this looks good to me.  Applied, thanks everyone.

^ permalink raw reply

* Re: [PATCH v2] cls_u32: use skb_header_pointer() to dereference data safely
From: David Miller @ 2010-06-02 14:15 UTC (permalink / raw)
  To: xiaosuo; +Cc: hadi, netdev
In-Reply-To: <1275487236-14452-1-git-send-email-xiaosuo@gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Wed,  2 Jun 2010 22:00:36 +0800

> use skb_header_pointer() to dereference data safely
> 
> the original skb->data dereference isn't safe, as there isn't any skb->len or
> skb_is_nonlinear() check. skb_header_pointer() is used instead in this patch.
> And when the skb isn't long enough, we terminate the function u32_classify()
> immediately with -1. Unaligned access is also fixed.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

Please don't do so many things at once Changli.

Everything is fine except the get_unaligned() addition.  It's
intentionally not there, and by adding the get_unaligned() you're
going to kill performance as this expands to 4 byte loads per u32 key
check on RISC systems.

If it's not aligned, that's a path or configuration that will need to
be fixed up such that the accesses are aligned.

Please respin this patch with the get_unaligned() part removed.

Thanks.

^ permalink raw reply

* Re: [PATCH] TCP: tcp_hybla: Fix integer overflow in slow start increment
From: David Miller @ 2010-06-02 14:16 UTC (permalink / raw)
  To: root; +Cc: netdev, kuznet, linux-kernel
In-Reply-To: <1275480124-24383-1-git-send-email-root@danielinux.net>

From: Daniele Lacamera <root@danielinux.net>
Date: Wed,  2 Jun 2010 14:02:04 +0200

> From: root <root@kitchen.(none)>

Please do not include incomplete authorship tags in your patch postings
like this, I used the more complete one from your email headers.

> For large values of rtt, 2^rho operation may overflow u32. Clamp down the increment to 2^16.
> Signed-off-by: Daniele Lacamera <root@danielinux.net>

Applied, thanks.

^ permalink raw reply

* Re: [Regression] [2.6.35-rc1] ssb_is_sprom_available
From: Maciej Rutecki @ 2010-06-02 14:20 UTC (permalink / raw)
  To: John W. Linville
  Cc: Matthew Wilcox, linux-kernel@vger.kernel.org, linux-pci,
	linux-usb, Rafael J. Wysocki, netdev, Michael Buesch
In-Reply-To: <20100601133402.GB26701@tuxdriver.com>

On wtorek, 1 czerwca 2010 o 15:34:02 John W. Linville wrote:
> On Mon, May 31, 2010 at 02:53:00PM -0600, Matthew Wilcox wrote:
> > On Mon, May 31, 2010 at 09:55:20PM +0200, Maciej Rutecki wrote:
> > > Last known good: 2.6.34
> > > Failing kernel: 2.6.35-rc1
> > >
> > > subsystem: PCI, USB(?)
> > >
> > > Kernel dies during booting on message "ssb_is_sprom_available", see
> > > picture:
> > > http://www.unixy.pl/maciek/download/kernel/2.6.35-rc1/gumis/DSC_0011.JP
> > >G
> >
> > Um, looks like it's something to do with the Sonics Silicon Backplane,
> > not PCI, nor USB.
> 
> Fix is on its way...
> 
> commit da1fdb02d9200ff28b6f3a380d21930335fe5429
> Author: Christoph Fritz <chf.fritz@googlemail.com>
> Date:   Fri May 28 10:45:59 2010 +0200
> 
>     ssb: fix NULL ptr deref when pcihost_wrapper is used
> 
>     Ethernet driver b44 does register ssb by it's pcihost_wrapper
>     and doesn't set ssb_chipcommon. A check on this value
>     introduced with commit d53cdbb94a52a920d5420ed64d986c3523a56743
>     and ea2db495f92ad2cf3301623e60cb95b4062bc484 triggers:
> 
>     BUG: unable to handle kernel NULL pointer dereference at 00000010
>     IP: [<c1266c36>] ssb_is_sprom_available+0x16/0x30
> 
>     Signed-off-by: Christoph Fritz <chf.fritz@googlemail.com>
>     Signed-off-by: John W. Linville <linville@tuxdriver.com>
> 

Yes this commit solves problem. Thanks

Tested-by: Maciej Rutecki <maciej.rutecki@gmail.com>

-- 
Maciej Rutecki
http://www.maciek.unixy.pl

^ permalink raw reply

* [PATCH v3] cls_u32: use skb_header_pointer() to dereference data safely
From: Changli Gao @ 2010-06-02 14:25 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

use skb_header_pointer() to dereference data safely

the original skb->data dereference isn't safe, as there isn't any skb->len or
skb_is_nonlinear() check. skb_header_pointer() is used instead in this patch.
And when the skb isn't long enough, we terminate the function u32_classify()
immediately with -1.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/cls_u32.c |   49 ++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 36 insertions(+), 13 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 9627542..4f52214 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -98,11 +98,11 @@ static int u32_classify(struct sk_buff *skb, struct tcf_proto *tp, struct tcf_re
 {
 	struct {
 		struct tc_u_knode *knode;
-		u8		  *ptr;
+		unsigned int	  off;
 	} stack[TC_U32_MAXDEPTH];
 
 	struct tc_u_hnode *ht = (struct tc_u_hnode*)tp->root;
-	u8 *ptr = skb_network_header(skb);
+	unsigned int off = skb_network_offset(skb);
 	struct tc_u_knode *n;
 	int sdepth = 0;
 	int off2 = 0;
@@ -134,8 +134,14 @@ next_knode:
 #endif
 
 		for (i = n->sel.nkeys; i>0; i--, key++) {
-
-			if ((*(__be32*)(ptr+key->off+(off2&key->offmask))^key->val)&key->mask) {
+			unsigned int toff;
+			__be32 *data, _data;
+
+			toff = off + key->off + (off2 & key->offmask);
+			data = skb_header_pointer(skb, toff, 4, &_data);
+			if (!data)
+				goto out;
+			if ((*data ^ key->val) & key->mask) {
 				n = n->next;
 				goto next_knode;
 			}
@@ -174,29 +180,45 @@ check_terminal:
 		if (sdepth >= TC_U32_MAXDEPTH)
 			goto deadloop;
 		stack[sdepth].knode = n;
-		stack[sdepth].ptr = ptr;
+		stack[sdepth].off = off;
 		sdepth++;
 
 		ht = n->ht_down;
 		sel = 0;
-		if (ht->divisor)
-			sel = ht->divisor&u32_hash_fold(*(__be32*)(ptr+n->sel.hoff), &n->sel,n->fshift);
-
+		if (ht->divisor) {
+			__be32 *data, _data;
+
+			data = skb_header_pointer(skb, off + n->sel.hoff, 4,
+						  &_data);
+			if (!data)
+				goto out;
+			sel = ht->divisor & u32_hash_fold(*data, &n->sel,
+							  n->fshift);
+		}
 		if (!(n->sel.flags&(TC_U32_VAROFFSET|TC_U32_OFFSET|TC_U32_EAT)))
 			goto next_ht;
 
 		if (n->sel.flags&(TC_U32_OFFSET|TC_U32_VAROFFSET)) {
 			off2 = n->sel.off + 3;
-			if (n->sel.flags&TC_U32_VAROFFSET)
-				off2 += ntohs(n->sel.offmask & *(__be16*)(ptr+n->sel.offoff)) >>n->sel.offshift;
+			if (n->sel.flags & TC_U32_VAROFFSET) {
+				__be16 *data, _data;
+
+				data = skb_header_pointer(skb,
+							  off + n->sel.offoff,
+							  2, &_data);
+				if (!data)
+					goto out;
+				off2 += ntohs(n->sel.offmask & *data) >>
+					n->sel.offshift;
+			}
 			off2 &= ~3;
 		}
 		if (n->sel.flags&TC_U32_EAT) {
-			ptr += off2;
+			off += off2;
 			off2 = 0;
 		}
 
-		if (ptr < skb_tail_pointer(skb))
+		if (off < skb->len)
 			goto next_ht;
 	}
 
@@ -204,9 +226,10 @@ check_terminal:
 	if (sdepth--) {
 		n = stack[sdepth].knode;
 		ht = n->ht_up;
-		ptr = stack[sdepth].ptr;
+		off = stack[sdepth].off;
 		goto check_terminal;
 	}
+out:
 	return -1;
 
 deadloop:

^ permalink raw reply related

* Re: [PATCH v3] cls_u32: use skb_header_pointer() to dereference data safely
From: David Miller @ 2010-06-02 14:33 UTC (permalink / raw)
  To: xiaosuo; +Cc: hadi, netdev
In-Reply-To: <1275488718-19588-1-git-send-email-xiaosuo@gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Wed,  2 Jun 2010 22:25:18 +0800

> use skb_header_pointer() to dereference data safely
> 
> the original skb->data dereference isn't safe, as there isn't any skb->len or
> skb_is_nonlinear() check. skb_header_pointer() is used instead in this patch.
> And when the skb isn't long enough, we terminate the function u32_classify()
> immediately with -1.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

Looks good, applied.

^ permalink raw reply

* Re: [PATCH] net: add additional lock to qdisc to increase throughput
From: Eric Dumazet @ 2010-06-02 14:52 UTC (permalink / raw)
  To: David Miller; +Cc: alexander.h.duyck, netdev
In-Reply-To: <20100602.051019.180420492.davem@davemloft.net>

Le mercredi 02 juin 2010 à 05:10 -0700, David Miller a écrit :
> From: "Duyck, Alexander H" <alexander.h.duyck@intel.com>
> Date: Fri, 21 May 2010 13:04:20 -0700
> 
> > Eric Dumazet wrote:
> >> Tests with following script gave a boost from ~50.000 pps to ~600.000
> >> pps on a dual quad core machine (E5450 @3.00GHz), tg3 driver.
> >> (A single netperf flow can reach ~800.000 pps on this platform)
> >> 
> >> for j in `seq 0 3`; do
> >>   for i in `seq 0 7`; do
> >>     netperf -H 192.168.0.1 -t UDP_STREAM -l 60 -N -T $i -- -m 6 &
> >>   done
> >> done
> > 
> > Running the same script with your patch my results went from 200Kpps to 1.2Mpps on a dual Xeon 5570.
> > 
> > Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>
> 
> Applied, thanks guys.

Thanks David, I realize you did all the necessary changes after
qdisc_run_begin/qdisc_is_running/ integration ;)



^ permalink raw reply

* [PATCH] act_pedit: access skb->data safely
From: Changli Gao @ 2010-06-02 14:55 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

access skb->data safely

we should use skb_header_pointer() and skb_store_bits() to access skb->data to
handle small or non-linear skbs.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/act_pedit.c |   24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index fdbd0b7..50e3d94 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -125,7 +125,7 @@ static int tcf_pedit(struct sk_buff *skb, struct tc_action *a,
 {
 	struct tcf_pedit *p = a->priv;
 	int i, munged = 0;
-	u8 *pptr;
+	unsigned int off;
 
 	if (!(skb->tc_verd & TC_OK2MUNGE)) {
 		/* should we set skb->cloned? */
@@ -134,7 +134,7 @@ static int tcf_pedit(struct sk_buff *skb, struct tc_action *a,
 		}
 	}
 
-	pptr = skb_network_header(skb);
+	off = skb_network_offset(skb);
 
 	spin_lock(&p->tcf_lock);
 
@@ -144,17 +144,17 @@ static int tcf_pedit(struct sk_buff *skb, struct tc_action *a,
 		struct tc_pedit_key *tkey = p->tcfp_keys;
 
 		for (i = p->tcfp_nkeys; i > 0; i--, tkey++) {
-			u32 *ptr;
+			u32 *ptr, _data;
 			int offset = tkey->off;
 
 			if (tkey->offmask) {
-				if (skb->len > tkey->at) {
-					 char *j = pptr + tkey->at;
-					 offset += ((*j & tkey->offmask) >>
-						   tkey->shift);
-				} else {
+				char *d, _d;
+
+				d = skb_header_pointer(skb, off + tkey->at, 1,
+						       &_d);
+				if (!d)
 					goto bad;
-				}
+				offset += (*d & tkey->offmask) >> tkey->shift;
 			}
 
 			if (offset % 4) {
@@ -169,9 +169,13 @@ static int tcf_pedit(struct sk_buff *skb, struct tc_action *a,
 				goto bad;
 			}
 
-			ptr = (u32 *)(pptr+offset);
+			ptr = skb_header_pointer(skb, off + offset, 4, &_data);
+			if (!ptr)
+				goto bad;
 			/* just do it, baby */
 			*ptr = ((*ptr & tkey->mask) ^ tkey->val);
+			if (ptr == &_data)
+				skb_store_bits(skb, off + offset, ptr, 4);
 			munged++;
 		}
 

^ permalink raw reply related

* Re: [Patch] fix packet loss and massive ping spikes with PPP multi-link
From: Ben McKeegan @ 2010-06-02 14:55 UTC (permalink / raw)
  To: Paul Mackerras
  Cc: netdev, linux-ppp, Alan Cox, Alexander E. Patrakov,
	Richard Hartmann, linux-kernel, gabriele.paoloni
In-Reply-To: <4C03E1CD.4000809@netservers.co.uk>

Ben McKeegan wrote:
> Paul Mackerras wrote:
> 
>>> I needed to do something similar a while back and I took a very
>>> different approach, which I think is more flexible.   Rather than
>>> implement a new round-robin scheduler I simply introduced a target
>>> minimum fragment size into the fragment size calculation, as a per
>>> bundle parameter that can be configured via a new ioctl.  This
>>> modifies the algorithm so that it tries to limit the number of
>>> fragments such that each fragment is at least the minimum size.  If
>>> the minimum size is greater than the packet size it will not be
>>> fragmented all but will instead just get sent down the next
>>> available channel.
>>>
>>> A pppd plugin generates the ioctl call allowing this to be tweaked
>>> per connection.  It is more flexible in that you can still have the
>>> larger packets fragmented if you wish.
>>
>> I like this a lot better than the other proposed patch.  It adds less
>> code because it uses the fact that ppp_mp_explode() already has a
>> round-robin capability using the ppp->nxchan field, plus it provides a
>> way to control it per bundle via pppd.
>>
>> If you fix up the indentation issues (2-space indent in some of the
>> added code -- if you're using emacs, set c-basic-offset to 8), I'll
>> ack it and hopefully DaveM will pick it up.
> 
> Okay, I'll fix it up when I'm back at work Tuesday and submit (today is 
> a UK public holiday) and also dig out and fix up the userspace code to 
> go with it.

Still working on this, updating the patch wasn't as trivial as I thought 
as it clashed with Gabriele Paoloni's ppp_mp_explode redesign.  However, 
while looking at this code I believe I have found a bug which might have 
been contributing to the poor performance the OP was experiencing.   For 
the case where channel speeds are unknown and there are more than 2 
channels it would miscalculate the fragment sizes so they are not 
balanced on the channels.

Patch for the bug to follow immediately.

Regards,
Ben.

^ permalink raw reply

* Re: [PATCH 00/12] sfc changes for 2.6.36
From: Ben Hutchings @ 2010-06-02 15:04 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <20100602.065711.191161830.davem@davemloft.net>

On Wed, 2010-06-02 at 06:57 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Wed, 02 Jun 2010 14:52:26 +0100
> 
> > Did you apply patch 12 "sfc: Get port number from CS_PORT_NUM, not PCI
> > function number" to net-2.6?
> 
> When you mix bug fixes and cleanups, I'm going to apply it all to
> net-next-2.6 You didn't even specify an intended destination in your
> subject lines, so you leave it entirely open for interpretation and
> my choice.
> 
> So, if you want a bug fix added to net-2.6, tossing it into a series
> which is not wholly appropriate for net-next-2.6 is not that way to
> accomplish that.

Well, I included a comment in the message and left it to your judgement
as to whether it was more appropriate for net-next-2.6 or net-2.6.

> The way to do it is:
> 
> 1) Submit the bug fix, explicitly state "[PATCH net-2.6 xxx] "
>    in your subject line.
> 
> 2) Prepare your net-next-2.6 changes on top of that, and do one
>    of two things:
>    a) Explicitly state in your patch series in the "[PATCH net-next-2.6 0/xxx] "
>       posting "this depends upon the bug fixes posted in the series
>       XXX posted earlier.
>    b) Wait for the bug fix to reach net-next-2.6 when I do a merge,
>       then post your series.

There are no dependencies in this case.

> Otherwise you make life way to miserable and complicated for me,
> the one who has to review and integrate all of your work.

Sorry.

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.


^ permalink raw reply

* [PATCH] ppp_generic: fix multilink fragment sizes
From: Ben McKeegan @ 2010-06-02 15:04 UTC (permalink / raw)
  To: davem; +Cc: ben, netdev, linux-kernel, gabriele.paoloni, alan, linux-ppp,
	paulus
In-Reply-To: <4C0670E2.1090708@netservers.co.uk>

Fix bug in multilink fragment size calculation introduced by
commit 9c705260feea6ae329bc6b6d5f6d2ef0227eda0a
"ppp: ppp_mp_explode() redesign"

Signed-off-by: Ben McKeegan <ben@netservers.co.uk>
---
 drivers/net/ppp_generic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index 0db3894..8b36bfe 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -1416,7 +1416,8 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
 		flen = len;
 		if (nfree > 0) {
 			if (pch->speed == 0) {
-				flen = totlen/nfree;
+				if (nfree > 1)
+					flen = DIV_ROUND_UP(len, nfree);
 				if (nbigger > 0) {
 					flen++;
 					nbigger--;
-- 
1.5.6.5

^ permalink raw reply related

* Re: [PATCH net-next-2.6] net: replace hooks in __netif_receive_skb V5
From: Stephen Hemminger @ 2010-06-02 15:07 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: netdev, davem, kaber, eric.dumazet
In-Reply-To: <20100602075207.GD2603@psychotron.redhat.com>

On Wed, 2 Jun 2010 09:52:08 +0200
Jiri Pirko <jpirko@redhat.com> wrote:

> +
> +	err = netdev_rx_handler_register(dev, macvlan_handle_frame);
> +	if (err) {
> +		rcu_assign_pointer(dev->macvlan_port, NULL);
> +		kfree(port);
> +	}
> +
> +	return err;
>  }

Rcu assign is not necessary here for because the hook didn't
get registered so there is no way for other CPU to see it.

-- 

^ permalink raw reply

* Re: [PATCH] phylib: Add support for the LXT973 phy.
From: Richard Cochran @ 2010-06-02 15:08 UTC (permalink / raw)
  To: David Miller; +Cc: afleming, netdev
In-Reply-To: <20100602.065017.139108801.davem@davemloft.net>

On Wed, Jun 02, 2010 at 06:50:17AM -0700, David Miller wrote:
> phy_device->priv "could one day get used for a different purpose"?
> What in the world are you smoking Andy?

I think he meant that the 'priv' pointer may one day be used to point
to some dynamically allocated data structure.

> It's clearly a private state pointer for the PHY driver to use,
> full stop.  There is absolutely no ambiguity of what this value
> is and what it is used for and who owns it.  The comments in the
> layout of struct phy_device state this clearly as well.
...
>
> Richard, please respin your patch so that you're using the ->priv
> field like in your original patch.

Well, is it okay to just use the first patch?

The fix needs just one bit of data, and I thought that (ab)using the
'priv' pointer would be okay, if a bit hacky. If, over time, the
driver needs more private data, it should be clear enought that the
existing bit "fiber selected" will also appear in the data structure.

Otherwise, the driver would have to allocate a structure with one
field, just to remember one bit.

Richard

^ permalink raw reply

* Re: [PATCH] phylib: Add support for the LXT973 phy.
From: David Miller @ 2010-06-02 15:15 UTC (permalink / raw)
  To: richardcochran; +Cc: afleming, netdev
In-Reply-To: <20100602150851.GA22591@riccoc20.at.omicron.at>

From: Richard Cochran <richardcochran@gmail.com>
Date: Wed, 2 Jun 2010 17:08:51 +0200

> Well, is it okay to just use the first patch?

Wasn't there another change you were asked to make that got included
in the v2 patch?

^ permalink raw reply

* Re: [PATCH net-next-2.6] net: replace hooks in __netif_receive_skb V5
From: Eric Dumazet @ 2010-06-02 15:15 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Jiri Pirko, netdev, davem, kaber, Paul E. McKenney
In-Reply-To: <20100602080730.53abea6d@nehalam>

Le mercredi 02 juin 2010 à 08:07 -0700, Stephen Hemminger a écrit :
> On Wed, 2 Jun 2010 09:52:08 +0200
> Jiri Pirko <jpirko@redhat.com> wrote:
> 
> > +
> > +	err = netdev_rx_handler_register(dev, macvlan_handle_frame);
> > +	if (err) {
> > +		rcu_assign_pointer(dev->macvlan_port, NULL);
> > +		kfree(port);
> > +	}
> > +
> > +	return err;
> >  }
> 
> Rcu assign is not necessary here for because the hook didn't
> get registered so there is no way for other CPU to see it.
> 

Thats a valid point, but we should use it, and not care of this litle
detail. Compiler generates same code anyway, since NULL value is tested
by rcu_assign_pointer().

If we dont use rcu_assign_pointer() ourself, Paul or Arnd will put it
one day or another :)

http://lkml.org/lkml/2010/6/1/290




^ permalink raw reply

* Re: [PATCH v2] netfilter: Xtables: idletimer target implementation
From: Jan Engelhardt @ 2010-06-02 15:16 UTC (permalink / raw)
  To: Luciano Coelho; +Cc: netdev, netfilter-devel, kaber, Timo Teras
In-Reply-To: <1275486062-23753-1-git-send-email-luciano.coelho@nokia.com>


On Wednesday 2010-06-02 15:41, Luciano Coelho wrote:

>+static int __init idletimer_tg_init(void)
>+{
>+	int ret;
>+
>+	idletimer_tg_kobj = kobject_create_and_add("idletimer",
>+						   &THIS_MODULE->mkobj.kobj);

Isn't this going to oops when you compile this module as =y?


^ permalink raw reply

* cls_u32: check unaligned data access
From: Changli Gao @ 2010-06-02 15:15 UTC (permalink / raw)
  To: Jamal Hadi Salim; +Cc: David S. Miller, netdev, Changli Gao

check unaligned data access

before accessing data, check if the corresponding address is aligned, and if
not, return -1.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
----
 net/sched/cls_u32.c |    9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 4f52214..309d275 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -102,7 +102,8 @@ static int u32_classify(struct sk_buff *skb, struct tcf_proto *tp, struct tcf_re
 	} stack[TC_U32_MAXDEPTH];
 
 	struct tc_u_hnode *ht = (struct tc_u_hnode*)tp->root;
-	unsigned int off = skb_network_offset(skb);
+	unsigned int noff = skb_network_offset(skb);
+	unsigned int off = noff;
 	struct tc_u_knode *n;
 	int sdepth = 0;
 	int off2 = 0;
@@ -138,6 +139,8 @@ next_knode:
 			__be32 *data, _data;
 
 			toff = off + key->off + (off2 & key->offmask);
+			if ((toff - noff) % 4)
+				goto out;
 			data = skb_header_pointer(skb, toff, 4, &_data);
 			if (!data)
 				goto out;
@@ -188,6 +191,8 @@ check_terminal:
 		if (ht->divisor) {
 			__be32 *data, _data;
 
+			if ((off + n->sel.hoff - noff) % 4)
+				goto out;
 			data = skb_header_pointer(skb, off + n->sel.hoff, 4,
 						  &_data);
 			if (!data)
@@ -203,6 +208,8 @@ check_terminal:
 			if (n->sel.flags & TC_U32_VAROFFSET) {
 				__be16 *data, _data;
 
+				if ((off + n->sel.offoff - noff) % 2)
+					goto out;
 				data = skb_header_pointer(skb,
 							  off + n->sel.offoff,
 							  2, &_data);

^ permalink raw reply related

* RE: [PATCH] ppp_generic: fix multilink fragment sizes
From: Paoloni, Gabriele @ 2010-06-02 15:17 UTC (permalink / raw)
  To: Ben McKeegan, davem@davemloft.net
  Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	alan@lxorguk.ukuu.org.uk, linux-ppp@vger.kernel.org,
	paulus@samba.org
In-Reply-To: <1275491094-9526-1-git-send-email-ben@netservers.co.uk>

The proposed patch looks wrong to me.

nbigger is already doing the job; I didn't use DIV_ROUND_UP because in general we don't have always to roundup, otherwise we would exceed the total bandwidth.

Regards

Gabriele Paoloni

-----Original Message-----
From: Ben McKeegan [mailto:ben@netservers.co.uk] 
Sent: 02 June 2010 16:05
To: davem@davemloft.net
Cc: ben@netservers.co.uk; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Paoloni, Gabriele; alan@lxorguk.ukuu.org.uk; linux-ppp@vger.kernel.org; paulus@samba.org
Subject: [PATCH] ppp_generic: fix multilink fragment sizes

Fix bug in multilink fragment size calculation introduced by
commit 9c705260feea6ae329bc6b6d5f6d2ef0227eda0a
"ppp: ppp_mp_explode() redesign"

Signed-off-by: Ben McKeegan <ben@netservers.co.uk>
---
 drivers/net/ppp_generic.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index 0db3894..8b36bfe 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -1416,7 +1416,8 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
 		flen = len;
 		if (nfree > 0) {
 			if (pch->speed == 0) {
-				flen = totlen/nfree;
+				if (nfree > 1)
+					flen = DIV_ROUND_UP(len, nfree);
 				if (nbigger > 0) {
 					flen++;
 					nbigger--;
-- 
1.5.6.5

--------------------------------------------------------------
Intel Shannon Limited
Registered in Ireland
Registered Office: Collinstown Industrial Park, Leixlip, County Kildare
Registered Number: 308263
Business address: Dromore House, East Park, Shannon, Co. Clare

This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.



^ permalink raw reply related


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