Netdev List
 help / color / mirror / Atom feed
* Re: Adding Support for SG,GSO,GRO
From: Ben Hutchings @ 2010-12-09 18:58 UTC (permalink / raw)
  To: Michał Mirosław
  Cc: Govindarajan, Sriramakrishnan, netdev@vger.kernel.org
In-Reply-To: <AANLkTi=8zWMYWG0DU147B08nN7xcvwaB9Bpq9fE8_fBe@mail.gmail.com>

On Thu, 2010-12-09 at 19:47 +0100, Michał Mirosław wrote:
> 2010/12/9 Ben Hutchings <bhutchings@solarflare.com>:
> > On Thu, 2010-12-09 at 16:03 +0530, Govindarajan, Sriramakrishnan wrote:
> >> Hi
> >> We have a NAPI compliant driver(net/drivers/davinci_emac.c), that does
> >> well at 10/100Mbps loads. Now the same controller/driver is used for
> >> 1000Mbps
> >> mode as well, where the CPU gets saturated easily
> >>
> >> Internally the module supports scatter gather DMA(which is currently not
> >> exercised) but there is no HW checksum support.
> >>
> >> To specifically implement GRO, GSO support would it be sufficient to add
> >> SG support to the driver? Are there other means of increasing the throughput
> >> and decreasing the CPU loading?
> [...]
> > On the TX side, NETIF_F_SG means that the stack may include data in the
> > skb by reference to arbitrary pages *even if their contents are still
> > being changed* (think sendfile()), which means it depends on hardware
> > checksum generation.
> 
> Isn't that condition too broad? If the data could change after packet
> is submitted to the driver then results would be unpredictable and
> allow sending wrong data with correct (because hw-calculated)
> checksum.

This is not done for a regular send(), only for functions such as
sendfile() which are specified to read the data asynchronously.

> Right now NETIF_F_SG is removed from dev->features by
> netdev_fix_features() if no checksum offloads are enabled.
> 
> Just an idea: would driver with NETIF_F_SG|NETIF_F_HW_CSUM using
> skb_checksum_help() in xmit path work? This would allow to use DMA
> scatter-gather without hardware checksumming (and avoid copying the
> packet's data before sending).

No, you cannot calculate a checksum for the fragments without also
copying them to ensure the data doesn't change afterward and invalidate
the checksum.  You could in theory make a copy into multiple fragments,
but there's no point in doing that unless the frame size is larger than
a page.

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] bridge: Fix return value of br_multicast_add_group()
From: Tobias Klauser @ 2010-12-09 18:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David S. Miller, bridge, netdev
In-Reply-To: <20101209082924.6d797871@nehalam>

On 2010-12-09 at 17:29:24 +0100, Stephen Hemminger <shemminger@linux-foundation.org> wrote:
> On Thu,  9 Dec 2010 15:02:36 +0100
> Tobias Klauser <tklauser@distanz.ch> wrote:
> 
> > If br_multicast_new_group returns NULL, we would return 0 (no error) to
> > the caller, which is not what we want. Instead we should return -ENOMEM
> > in this case.
> > 
> > Also replace IS_ERR(x) || !x by IS_ERR_OR_NULL(x)
> > 
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> >  net/bridge/br_multicast.c |    5 ++++-
> >  1 files changed, 4 insertions(+), 1 deletions(-)
> > 
> > diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
> > index 326e599..d4e1e81 100644
> > --- a/net/bridge/br_multicast.c
> > +++ b/net/bridge/br_multicast.c
> > @@ -713,8 +713,11 @@ static int br_multicast_add_group(struct net_bridge *br,
> >  
> >  	mp = br_multicast_new_group(br, port, group);
> >  	err = PTR_ERR(mp);
> > -	if (unlikely(IS_ERR(mp) || !mp))
> > +	if (IS_ERR_OR_NULL(mp)) {
> > +		if (!mp)
> > +			err = -ENOMEM;
> >  		goto err;
> > +	}
> >  
> >  	if (!port) {
> >  		hlist_add_head(&mp->mglist, &br->mglist);
> 
> I would rather fix br_multicast_new_group so it never returns
> NULL. Instead return PTR_ERR(-ENOMEM) on out of memory.

Ok, I'll change that and send an updated patch.

Thanks a lot
Tobias

^ permalink raw reply

* Re: [PATCH] Fix 2.6.34-rc1 regression in  disable_ipv6 support
From: Eric W. Biederman @ 2010-12-09 19:09 UTC (permalink / raw)
  To: Brian Haley
  Cc: David Miller, netdev, Mahesh Kelkar, Lorenzo Colitti,
	YOSHIFUJI Hideaki, Stephen Hemminger, stable
In-Reply-To: <4D00F58A.2050307@hp.com>

Brian Haley <brian.haley@hp.com> writes:

> On 12/08/2010 11:16 PM, Eric W. Biederman wrote:
>> Finding the real bug is beyond me right now, but fixing the regression
>> in disable_ipv6 is simple.  We can just delete ::1 when we bring down
>> the loopback interface, and it will be restored automatically when we
>> bring the loopback interface back up.
>
> Hi Eric,
>
> This would work as well, same check, different way.

But that looks like less of an obvious magic exception.  The only
address it is safe to ignore on is ::1, because we always restore it
when we bring the loopback interface up.

Long term we really do want to keep the loopback address.  But
that actually requires finding and fixing what is broken in
ipv6.

So let's please keep this a line that we can easily remove.  It isn't
like interfaces coming up and down are a fast path where every cycle
counts.  We just need to be reasonably efficient.

Eric

^ permalink raw reply

* Re: [PATCH] Fix 2.6.34-rc1 regression in  disable_ipv6 support
From: Stephen Hemminger @ 2010-12-09 19:16 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Brian Haley, David Miller, netdev, Mahesh Kelkar, Lorenzo Colitti,
	YOSHIFUJI Hideaki, stable
In-Reply-To: <m139q6ahld.fsf@fess.ebiederm.org>

On Thu, 09 Dec 2010 11:09:34 -0800
ebiederm@xmission.com (Eric W. Biederman) wrote:

> Brian Haley <brian.haley@hp.com> writes:
> 
> > On 12/08/2010 11:16 PM, Eric W. Biederman wrote:
> >> Finding the real bug is beyond me right now, but fixing the regression
> >> in disable_ipv6 is simple.  We can just delete ::1 when we bring down
> >> the loopback interface, and it will be restored automatically when we
> >> bring the loopback interface back up.
> >
> > Hi Eric,
> >
> > This would work as well, same check, different way.
> 
> But that looks like less of an obvious magic exception.  The only
> address it is safe to ignore on is ::1, because we always restore it
> when we bring the loopback interface up.
> 
> Long term we really do want to keep the loopback address.  But
> that actually requires finding and fixing what is broken in
> ipv6.
> 
> So let's please keep this a line that we can easily remove.  It isn't
> like interfaces coming up and down are a fast path where every cycle
> counts.  We just need to be reasonably efficient.

No but since removing address propagates up to user space daemons
like Quagga please analyze and fix the problem, don't just look
for band aid.

^ permalink raw reply

* Re: [PATCH] Fix 2.6.34-rc1 regression in  disable_ipv6 support
From: Eric W. Biederman @ 2010-12-09 19:22 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Brian Haley, David Miller, netdev, Mahesh Kelkar, Lorenzo Colitti,
	YOSHIFUJI Hideaki, stable
In-Reply-To: <20101209082703.6e5519e7@nehalam>

Stephen Hemminger <shemminger@vyatta.com> writes:

> On Thu, 09 Dec 2010 10:28:10 -0500
> Brian Haley <brian.haley@hp.com> wrote:
>
>> On 12/08/2010 11:16 PM, Eric W. Biederman wrote:

>> 		/* If just doing link down, and address is permanent
>> 		   and not link-local, then retain it. */
>> 		if (!how &&
>> 		    (ifa->flags&IFA_F_PERMANENT) &&
>> 		    !(ipv6_addr_type(&ifa->addr) & IPV6_ADDR_LINKLOCAL)) {
>> 			list_move_tail(&ifa->if_list, &keep_list);
>> 
>> 			/* If not doing DAD on this address, just keep it. */
>> 			if ((dev->flags&(IFF_NOARP|IFF_LOOPBACK)) ||
>> 			    idev->cnf.accept_dad <= 0 ||
>> 			    (ifa->flags & IFA_F_NODAD))
>
> I think the problem is on coming back up, not on the down step.

Oh it is.  All addresses that you keep break if you down the loopback
interface, no matter which interface those addresses are on.

Stephen the cause of the regression in 2.6.34-rc1 that you introduced
that breaks the disable_ipv6 functionality in practice is removing
the loopback address from the loopback interface.  So I sent
a partial revert.

It is safe to do a partial revert because the loopback address is always
reprogrammed when we bring the interface back up.  But that
reprogramming only works if it doesn't error out with -EEXIST.

So by all means properly fix the ancient bug that breaks usage of all
local ipv6 addresses when the loopback interface is brought down,
and we can remove the regression fix.

However complaining about a partial revert to fix a regression you
introduced because it fixes a problem deep within the ipv6 networking
stack that the smallest modicum of testing would have revealed on your
part before you broke things seems inappropriate.

Please let's get the disable_ipv6 functionality working again (where 
in practice we don't care about preserving addresses).  Then let's
take our time and tack and fix whatever this is properly.

Eric


^ permalink raw reply

* Re: tc: show format ABI changed
From: Jarek Poplawski @ 2010-12-09 19:25 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: David Miller, Eric Dumazet, netdev
In-Reply-To: <20101208145136.1ca3ece4@nehalam>

On Wed, Dec 08, 2010 at 02:51:36PM -0800, Stephen Hemminger wrote:
> Although well intentioned, the following patch should have not
> been applied since it changes the kernel ABI. It broke some scripts
> parsing the output format of tc commands.

I doubt you can blame this patch for changing any ABI. If there is
such a thing documented you would probably point us there. Otherwise,
it should be seen in the code, and tc has it all conditional:

        if (tbs[TCA_STATS_RATE_EST]) {
		...
                fprintf(fp, "\n%srate %s %upps ",
                        prefix, sprint_rate(re.bps, b1), re.pps);
        }
 
        if (tbs[TCA_STATS_QUEUE]) {
		...
                if (!tbs[TCA_STATS_RATE_EST])
                        fprintf(fp, "\n%s", prefix);
                fprintf(fp, "backlog %s %up requeues %u ",
                        sprint_size(q.backlog, b1), q.qlen, q.requeues);
        }

which suggests scripts should use similar logic.

Anyway, these are tc's printouts and it should handle source data
errors too. Since it's accepted it can't be wrong ;-)

Jarek P.

> 
> Before HTB would report bogus zero values, now it reports
> nothing and that changes the output format.  Like the empty
> fields in /proc, I argue we can't play fast and loose with
> netlink responses.
> 
> Not a big deal to fix the script in this case, in this case
> so don't revert it.
> 
> commit d250a5f90e53f5e150618186230795352d154c88
> Author: Eric Dumazet <eric.dumazet@gmail.com>
> Date:   Fri Oct 2 10:32:18 2009 +0000
> 
>     pkt_sched: gen_estimator: Dont report fake rate estimators
>     
>     Jarek Poplawski a écrit :
>     >
>     >
>     > Hmm... So you made me to do some "real" work here, and guess what?:
>     > there is one serious checkpatch warning! ;-) Plus, this new parameter
>     > should be added to the function description. Otherwise:
>     > Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
>     >
>     > Thanks,
>     > Jarek P.
>     >
>     > PS: I guess full "Don't" would show we really mean it...
>     
>     Okay :) Here is the last round, before the night !
>     
>     Thanks again
>     
>     [RFC] pkt_sched: gen_estimator: Don't report fake rate estimators
>     
>     We currently send TCA_STATS_RATE_EST elements to netlink users, even if no estimator
>     is running.
>     
>     # tc -s -d qdisc
>     qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>      Sent 112833764978 bytes 1495081739 pkt (dropped 0, overlimits 0 requeues 0)
>      rate 0bit 0pps backlog 0b 0p requeues 0
>     
>     User has no way to tell if the "rate 0bit 0pps" is a real estimation, or a fake
>     one (because no estimator is active)
>     
>     After this patch, tc command output is :
>     $ tc -s -d qdisc
>     qdisc pfifo_fast 0: dev eth0 root bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>      Sent 561075 bytes 1196 pkt (dropped 0, overlimits 0 requeues 0)
>      backlog 0b 0p requeues 0
>     
>     We add a parameter to gnet_stats_copy_rate_est() function so that
>     it can use gen_estimator_active(bstats, r), as suggested by Jarek.
>     
>     This parameter can be NULL if check is not necessary, (htb for
>     example has a mandatory rate estimator)
>     
>     Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>     Signed-off-by: Jarek Poplawski <jarkao2@gmail.com>
>     Signed-off-by: David S. Miller <davem@davemloft.net>

^ permalink raw reply

* Re: [PATCH] Fix 2.6.34-rc1 regression in  disable_ipv6 support
From: Eric W. Biederman @ 2010-12-09 19:31 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Brian Haley, David Miller, netdev, Mahesh Kelkar, Lorenzo Colitti,
	YOSHIFUJI Hideaki, stable
In-Reply-To: <20101209111611.1d2e6e2b@nehalam>

Stephen Hemminger <shemminger@vyatta.com> writes:

> On Thu, 09 Dec 2010 11:09:34 -0800
> ebiederm@xmission.com (Eric W. Biederman) wrote:
> So let's please keep this a line that we can easily remove.  It isn't
> like interfaces coming up and down are a fast path where every cycle
> counts.  We just need to be reasonably efficient.

> No but since removing address propagates up to user space daemons
> like Quagga please analyze and fix the problem, don't just look
> for band aid.

You fix the problem.  You introduced the regression, and you didn't test
that keeping addresses actually worked.  This is not the first patch
that has been applied to fix regressions in this area.

I introduced a targeted revert of your broken change that only preserves
the one address people are least likely to change.

I know people are not downing the ipv6 loopback interface in practice
and bringing it back up because in practice on running systems because
that breaks ipv6 networking.  So quagga should not see this issue
in practice.

Right now misplaced perfectionism is being a huge enemy of creating
a kernel that actually works.

Eric


^ permalink raw reply

* Re: Adding Support for SG,GSO,GRO
From: David Miller @ 2010-12-09 19:38 UTC (permalink / raw)
  To: mirqus; +Cc: bhutchings, srk, netdev
In-Reply-To: <AANLkTi=8zWMYWG0DU147B08nN7xcvwaB9Bpq9fE8_fBe@mail.gmail.com>

From: Michał Mirosław <mirqus@gmail.com>
Date: Thu, 9 Dec 2010 19:47:57 +0100

> Isn't that condition too broad? If the data could change after packet
> is submitted to the driver then results would be unpredictable and
> allow sending wrong data with correct (because hw-calculated)
> checksum.

They are intentionally like that, without question.

Otherwise we'd need to interlock with all application mapped,
filesystem, and other page writes while sending any page over the
network.

We absolutely do not want to have to freeze every page we try to send
via sendfile() or similar, the cost is just too high.

If the application or networked filesystem needs such synchronization,
it provides it for itself.

For example, SAMBA only uses sendfile() when the file has an op-lock
held on it.

The checksum requirement for using SG is not going away, so continuing
to discuss along the lines of removing that requirement is not a good
use of your time I don't think.

^ permalink raw reply

* [PATCH net-next-26] cxgb4vf: Ingress Queue Entry Size needs to be 64 bytes
From: Casey Leedom @ 2010-12-09 19:38 UTC (permalink / raw)
  To: netdev; +Cc: davem, Casey Leedom

Was using L1_CACHE_BYTES for the Ingress Queue Entry Size but it really
needs to be 64 bytes in order to support the largest message sizes.

Signed-off-by: Casey Leedom <leedom@chelsio.com>
---
 drivers/net/cxgb4vf/cxgb4vf_main.c |   15 +++++++++++----
 1 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/net/cxgb4vf/cxgb4vf_main.c b/drivers/net/cxgb4vf/cxgb4vf_main.c
index d887a76..6bf464a 100644
--- a/drivers/net/cxgb4vf/cxgb4vf_main.c
+++ b/drivers/net/cxgb4vf/cxgb4vf_main.c
@@ -2269,6 +2269,7 @@ static void __devinit cfg_queues(struct adapter *adapter)
 {
 	struct sge *s = &adapter->sge;
 	int q10g, n10g, qidx, pidx, qs;
+	size_t iqe_size;
 
 	/*
 	 * We should not be called till we know how many Queue Sets we can
@@ -2313,6 +2314,13 @@ static void __devinit cfg_queues(struct adapter *adapter)
 	s->ethqsets = qidx;
 
 	/*
+	 * The Ingress Queue Entry Size for our various Response Queues needs
+	 * to be big enough to accommodate the largest message we can receive
+	 * from the chip/firmware; which is 64 bytes ...
+	 */
+	iqe_size = 64;
+
+	/*
 	 * Set up default Queue Set parameters ...  Start off with the
 	 * shortest interrupt holdoff timer.
 	 */
@@ -2320,7 +2328,7 @@ static void __devinit cfg_queues(struct adapter *adapter)
 		struct sge_eth_rxq *rxq = &s->ethrxq[qs];
 		struct sge_eth_txq *txq = &s->ethtxq[qs];
 
-		init_rspq(&rxq->rspq, 0, 0, 1024, L1_CACHE_BYTES);
+		init_rspq(&rxq->rspq, 0, 0, 1024, iqe_size);
 		rxq->fl.size = 72;
 		txq->q.size = 1024;
 	}
@@ -2329,8 +2337,7 @@ static void __devinit cfg_queues(struct adapter *adapter)
 	 * The firmware event queue is used for link state changes and
 	 * notifications of TX DMA completions.
 	 */
-	init_rspq(&s->fw_evtq, SGE_TIMER_RSTRT_CNTR, 0, 512,
-		  L1_CACHE_BYTES);
+	init_rspq(&s->fw_evtq, SGE_TIMER_RSTRT_CNTR, 0, 512, iqe_size);
 
 	/*
 	 * The forwarded interrupt queue is used when we're in MSI interrupt
@@ -2346,7 +2353,7 @@ static void __devinit cfg_queues(struct adapter *adapter)
 	 * any time ...
 	 */
 	init_rspq(&s->intrq, SGE_TIMER_RSTRT_CNTR, 0, MSIX_ENTRIES + 1,
-		  L1_CACHE_BYTES);
+		  iqe_size);
 }
 
 /*
-- 
1.7.0.4


^ permalink raw reply related

* Re: 2.6.37-rc5: NULL pointer oops in selinux_socket_unix_stream_connect
From: Paul Moore @ 2010-12-09 19:42 UTC (permalink / raw)
  To: Jeremy Fitzhardinge
  Cc: James Morris, Stephen Smalley, NetDev, Linux Kernel Mailing List
In-Reply-To: <4CFFF3F3.90100@goop.org>

On Wed, 2010-12-08 at 13:09 -0800, Jeremy Fitzhardinge wrote:
> I just got this oops in a freshly booted 2.6.37-rc5 Xen domain, while
> sitting idle at the login prompt:
> 
> BUG: unable to handle kernel NULL pointer dereference at 0000000000000210
> IP: [<ffffffff811d55d4>] selinux_socket_unix_stream_connect+0x29/0xa0
> PGD 1c99d067 PUD 1cb03067 PMD 0 
> Oops: 0000 [#1] SMP 
> last sysfs file: /sys/devices/system/cpu/sched_mc_power_savings
> CPU 0 
> Modules linked in: sunrpc dm_mirror dm_region_hash dm_log [last unloaded: scsi_wait_scan]
> 
> Pid: 2297, comm: at-spi-registry Not tainted 2.6.37-rc5+ #293 /
> RIP: e030:[<ffffffff811d55d4>]  [<ffffffff811d55d4>] selinux_socket_unix_stream_connect+0x29/0xa0
> RSP: e02b:ffff880006e7dd68  EFLAGS: 00010292
> RAX: ffff88001d1ed8c0 RBX: ffff88001d06d9a0 RCX: 0000000000000022
> RDX: ffff88001d1ed580 RSI: 0000000000000000 RDI: ffff88001b7d6ac0
> RBP: ffff880006e7de18 R08: 00000000ffff0201 R09: ffff88001e78c968
> R10: 000000001f47e9c2 R11: ffff88001fbf4400 R12: ffff88001d1ed8c0
> R13: ffff88001d1ed580 R14: ffff88001ca00cc0 R15: 0000000000000000
> FS:  00007fa643031920(0000) GS:ffff88001ff85000(0000) knlGS:0000000000000000
> CS:  e033 DS: 0000 ES: 0000 CR0: 000000008005003b
> CR2: 0000000000000210 CR3: 000000001d78a000 CR4: 0000000000002660
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> Process at-spi-registry (pid: 2297, threadinfo ffff880006e7c000, task ffff88001cdd1140)
> Stack:
>  ffff88001d4c0bc0 000000004cffecc5 ffff880006e7ddc8 ffffffff81028dc5
>  ffff8800ffffffff 0001628b2ec3fe22 ffff880006e7dde8 ffff88001d1edb80
>  0000000000000001 0000936a4da34099 0000000000000000 00000000000000fa
> Call Trace:
>  [<ffffffff81028dc5>] ? pvclock_clocksource_read+0x48/0xb1
>  [<ffffffff810074ab>] ? xen_clocksource_read+0x20/0x22
>  [<ffffffff81008fd9>] ? xen_spin_lock+0xc6/0xd9
>  [<ffffffff811d1d1e>] security_unix_stream_connect+0x16/0x18
>  [<ffffffff81484366>] unix_stream_connect+0x215/0x3ff
>  [<ffffffff813f351d>] sys_connect+0x7a/0xa0
>  [<ffffffff8108cd9d>] ? audit_syscall_entry+0x1c2/0x1ee
>  [<ffffffff8100bb42>] system_call_fastpath+0x16/0x1b
> Code: c9 c3 55 48 89 e5 41 55 41 54 53 48 81 ec 98 00 00 00 0f 1f 44 00 00 b9 22 00 00 00 48 8b 47 20 48 8b 76 20 48 8b 98 10 02 00 00 <4c> 8b a6 10 02 00 00 31 c0 4c 8b aa 10 02 00 00 4c 8d 85 50 ff 
> RIP  [<ffffffff811d55d4>] selinux_socket_unix_stream_connect+0x29/0xa0
>  RSP <ffff880006e7dd68>
> CR2: 0000000000000210
> ---[ end trace 50030b578c1ee27e ]---
> 
> This corresponds to:
> 
> (gdb) list *0xffffffff811d55d4
> 0xffffffff811d55d4 is in selinux_socket_unix_stream_connect (/home/jeremy/git/upstream/security/selinux/hooks.c:3929).
> 3924	static int selinux_socket_unix_stream_connect(struct socket *sock,
> 3925						      struct socket *other,
> 3926						      struct sock *newsk)
> 3927	{
> 3928		struct sk_security_struct *sksec_sock = sock->sk->sk_security;
> 3929		struct sk_security_struct *sksec_other = other->sk->sk_security;
> 3930		struct sk_security_struct *sksec_new = newsk->sk_security;
> 3931		struct common_audit_data ad;
> 3932		int err;
> 3933	
> 
> 
> The system is a somewhat out of date Fedora 13 with
> selinux-policy-3.7.19-73.fc13.noarch and
> selinux-policy-targeted-3.7.19-73.fc13.noarch installed.
> 
> I'm not sure what at-spi-registry is or what it is trying to do here.
> The crash seems non-deterministic; I rebooted the domain without any issues.
> 
> Thanks,
>     J

Thanks for the report.

Unfortunately I don't have any great ideas off the top of my head but it
has been a couple of months since I've played with that code; I'll take
a look and see if anything jumps out at me.

For what it's worth, a quick Google makes me think that at-spi-registry
is part of Gnome's assistive technology functionality.  That said, I
have no idea what it does exactly, but evidently it does it over a UNIX
domain socket ...

If you're ever able to recreate the problem or if you can think of
anything else that might be useful please let me know.

Thanks.

-- 
paul moore
linux @ hp

^ permalink raw reply

* [PATCH] MAINTAINERS: remove me from tulip
From: Kyle McMartin @ 2010-12-09 19:50 UTC (permalink / raw)
  To: netdev; +Cc: davem, grundler

It was a nice idea, but -ENOTIME and -ENOHW. I never got around to doing
a lot of the clean up that I intended to.

Signed-off-by: Kyle McMartin <kyle@mcmartin.ca>
---
diff --git a/MAINTAINERS b/MAINTAINERS
index 1a1c27b..a54a738 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5932,7 +5932,6 @@ F:	include/linux/tty.h
 
 TULIP NETWORK DRIVERS
 M:	Grant Grundler <grundler@parisc-linux.org>
-M:	Kyle McMartin <kyle@mcmartin.ca>
 L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/tulip/

^ permalink raw reply related

* Re: [PATCH] netfilter: don't need to initialize instance_table
From: David Miller @ 2010-12-09 19:56 UTC (permalink / raw)
  To: xiaosuo; +Cc: kaber, netfilter-devel, netdev
In-Reply-To: <1291889421-10794-1-git-send-email-xiaosuo@gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Thu,  9 Dec 2010 18:10:21 +0800

> Since instance_table is a static array, and has been zeroed already, we
> don't need to take CPU cycles to initialize it.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>

Adding a dependency on the implementation of HLISTs is not a good
idea, I would not apply this patch.

^ permalink raw reply

* Re: [RFC PATCH v1] iproute2: add IFLA_TC support to 'ip link'
From: John Fastabend @ 2010-12-09 19:58 UTC (permalink / raw)
  To: hadi@cyberus.ca
  Cc: shemminger@vyatta.com, netdev@vger.kernel.org,
	tgraf@infradead.org, eric.dumazet@gmail.com, davem@davemloft.net
In-Reply-To: <1291374412.10126.17.camel@mojatatu>

On 12/3/2010 3:06 AM, jamal wrote:
> On Thu, 2010-12-02 at 11:51 -0800, John Fastabend wrote:
>> On 12/2/2010 2:40 AM, jamal wrote:
> 
> 
>> I viewed the HW QOS as L2 link attributes more than a queuing discipline per se.
>> Plus 'ip link' is already used to set things outside of ip. 
>> For example 'txqueuelen' and 'vf x'.
> 
> the vf one maybe borderline-ok txquelen is probably inherited from
> ifconfig (and not sure a single queue a scheduler qualifies)
> 
> 
>> However thinking about this a bit more qdisc support seems cleaner. 
>> For one we can configure QOS policies per class with Qdisc_class_ops. 
>> And then also aggregate statistics with dump_stats. I would avoid the 
>> "hardware-kinda-8021q-sched" name though to account for schedulers that 
>> may not be 802.1Q compliant maybe 'mclass-sched' for multi-class scheduler. 
> 
> Typically the scheduler would be a very familiar one implemented
> per-spec by many vendors and will have a name acceptable by all.
> So pick an appropriate noun so the user expectation matches it.
> 

I think what we really want is a container to create groups of tx queues which can then be managed and given a scheduler. One reason for this is the 802.1Q spec allows for different schedulers to be running on different traffic classes including vendor specific schedulers. So having a root "hardware-kinda-8021q-sched" doesn't seem flexible enough to handle adding/removing schedulers per traffic class.

With a container qdisc statistics roll up nicely as expected and the default scheduler can be the usual mq qdisc.

A first take at this coming shortly. Any thoughts?

>> I'll look into this. Thanks for the suggestion!
> 
>>
>> On egress the skb priority is mapped to a class which is associated with a
>> range of queues (qoffset:qoffset + qcount). 
>> In the 802.1Q case this queue range is mapped to the 802.1Qp 
>> traffic class in hardware. So the hardware traffic class is mapped 1-1 
>> with the software class. Additionally in software the VLAN egress mapping
>> is used to map the skb priority to the 802.1Q priority. Here I expect user
>> policies to configure this to get a consistent mapping. On ingress the 
>> skb priority is set using the 802.1Q ingress mapping. This case is 
>> something a userspace policy could configure if egress/ingress mappings
>> should be symmetric.
>>
> 
> Sounds sensible. 
> 
>> In the simpler case of hardware rate limiting (not 802.1Q) this is not
>> really a concern at all. With this mechanism we can identify traffic 
>> and push it to the correct queues that are grouped into a rate limited class.
> 
> Ok, so you can do rate control as well?
> 

Yes, but per tx_ring. So software needs to then balance the rings into an aggregated rate limiter. Using the container scheme I imagine a root mclass qdisc with multiple "sch_rate_limiter" qdiscs. This qdisc could manage the individual rate limiters per queue and get something like a rate limiter per groups of tx queues.

>> If there are egress/ingress mappings then those will apply skb priority tags 
>> on egress and the correct skb priority on ingress.
> 
> Curious how you would do this in a rate controlled environment. EX: on
> egress, do you use whatever skb prio you get to map to a specific rate
> queue in h/ware? Note: skb prio has a strict priority scheduling
> semantics so a 1-1 mapping doesnt sound reasonable..

Yes this is how I would expect this to work. The prio mapping is configurable so I think this could be worked around by policy in tc. iproute2 would need to pick a reasonable default mapping.

Warning thinking out loud here but maybe we could also add a qdisc op to pick the underlying tx queue basically a qdisc ops for dev_pick_tx(). This ops could be part of the root qdisc and called in dev_queue_xmit(). I would need to think about this some more to see if it is sane but bottom line is the tx queue needs to be learned before __dev_xmit_skb(). The default mechanism in this patch set being the skb prio.

> 
>> Currently everything works reasonably well with this scheme and the mq qdisc.
>>  The mq qdisc uses pfifo and the driver then pauses the queues as needed. 
>> Using the enhanced transmission selection algorithm (ETS - 802.1Qaz pre-standard)
>>  in hardware we see variations from expected bandwidth around +-5% with TCP/UDP. 
>> Instrumenting HW rate limiters gives similar variations. I tested this is with 
>> ixgbe and the 82599 device.
>>
>> Bit long winded but hopefully that answers your question.
> 
> I am curious about the rate based scheme - and i hope you are looking at
> a different qdisc for that?

Yes a different qdisc.

Thanks,
John

> 
> cheers,
> jamal
> 

^ permalink raw reply

* Re: [Bugme-new] [Bug 24472] New: Kernel panic - not syncing: Fatal Exception
From: Paweł Staszewski @ 2010-12-09 19:59 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Andrew Morton, netdev, Paul Mackerras, bugzilla-daemon,
	bugme-daemon, pstaszewski, Andrej Ota, Eric Dumazet
In-Reply-To: <4D011813.1090200@gmail.com>

W dniu 2010-12-09 18:55, Jarek Poplawski pisze:
> Paweł Staszewski wrote:
>> W dniu 2010-12-08 23:01, Jarek Poplawski pisze:
>>> Paweł Staszewski wrote:
>>>> W dniu 2010-12-08 21:22, Andrew Morton pisze:
>>>>> (switched to email.  Please respond via emailed reply-to-all, not
>>>>> via the
>>>>> bugzilla web interface).
>>>>>
>>>>> On Wed, 8 Dec 2010 20:14:45 GMT
>>>>> bugzilla-daemon@bugzilla.kernel.org wrote:
>>>>>
>>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=24472
>>>>>>
>>>>>>               Summary: Kernel panic - not syncing: Fatal Exception
>>>>>>               Product: Drivers
>>>>>>               Version: 2.5
>>>>>>        Kernel Version: 2.6.36.1
>>> Hi,
>>> Could you try to revert this patch?:
>>> http://git.kernel.org/?p=linux/kernel/git/stable/linux-2.6.36.y.git;a=commitdiff;h=55c95e738da85373965cb03b4f975d0fd559865b
>>>
>>>
>> After reverting this patch all is working
>> 200 connects-disconnects and no kernel panic
>>
>> I will make more session and test more.
> OK. I CC Andrej and Eric, who diagnosed it in this thread:
> http://lkml.org/lkml/2010/12/3/116
> [unable to handle kernel NULL pointer dereference in skb_dequeue]
>
> This should be also interesting:
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=db7bf6d97c6956b7eb0f22131cb5c37bd41f33c0
>
> Thanks for testing,
> Jarek P.
>
>
After 10 hours of testing all is working.

I can't reproduce kernel panic now with houndreds of pppoe sessions that 
connects-disconnects.


Thanks
Paweł

^ permalink raw reply

* [RFC PATCH 1/4] net: implement mechanism for HW based QOS
From: John Fastabend @ 2010-12-09 19:59 UTC (permalink / raw)
  To: davem; +Cc: netdev, hadi, shemminger, tgraf, eric.dumazet, john.r.fastabend

This patch provides a mechanism for lower layer devices to
steer traffic using skb->priority to tx queues. This allows
for hardware based QOS schemes to use the default qdisc without
incurring the penalties related to global state and the qdisc
lock. While reliably receiving skbs on the correct tx ring
to avoid head of line blocking resulting from shuffling in
the LLD. Finally, all the goodness from txq caching and xps/rps
can still be leveraged.

Many drivers and hardware exist with the ability to implement
QOS schemes in the hardware but currently these drivers tend
to rely on firmware to reroute specific traffic, a driver
specific select_queue or the queue_mapping action in the
qdisc.

By using select_queue for this drivers need to be updated for
each and every traffic type and we lose the goodness of much
of the upstream work. Firmware solutions are inherently
inflexible. And finally if admins are expected to build a
qdisc and filter rules to steer traffic this requires knowledge
of how the hardware is currently configured. The number of tx
queues and the queue offsets may change depending on resources.
Also this approach incurs all the overhead of a qdisc with filters.

With the mechanism in this patch users can set skb priority using
expected methods ie setsockopt() or the stack can set the priority
directly. Then the skb will be steered to the correct tx queues
aligned with hardware QOS traffic classes. In the normal case with
a single traffic class and all queues in this class everything
works as is until the LLD enables multiple tcs.

To steer the skb we mask out the lower 4 bits of the priority
and allow the hardware to configure upto 15 distinct classes
of traffic. This is expected to be sufficient for most applications
at any rate it is more then the 8021Q spec designates and is
equal to the number of prio bands currently implemented in
the default qdisc.

This in conjunction with a userspace application such as
lldpad can be used to implement 8021Q transmission selection
algorithms one of these algorithms being the extended transmission
selection algorithm currently being used for DCB.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 include/linux/netdevice.h |   65 +++++++++++++++++++++++++++++++++++++++++++++
 net/core/dev.c            |   39 ++++++++++++++++++++++++++-
 2 files changed, 103 insertions(+), 1 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index a9ac5dc..c0d4fb1 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -646,6 +646,12 @@ struct xps_dev_maps {
     (nr_cpu_ids * sizeof(struct xps_map *)))
 #endif /* CONFIG_XPS */
 
+/* HW offloaded queuing disciplines txq count and offset maps */
+struct netdev_tc_txq {
+	u16 count;
+	u16 offset;
+};
+
 /*
  * This structure defines the management hooks for network devices.
  * The following hooks can be defined; unless noted otherwise, they are
@@ -1146,6 +1152,10 @@ struct net_device {
 	/* Data Center Bridging netlink ops */
 	const struct dcbnl_rtnl_ops *dcbnl_ops;
 #endif
+	u8 max_tc;
+	u8 num_tc;
+	struct netdev_tc_txq *_tc_to_txq;
+	u8 prio_tc_map[16];
 
 #if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
 	/* max exchange id for FCoE LRO by ddp */
@@ -1162,6 +1172,58 @@ struct net_device {
 #define	NETDEV_ALIGN		32
 
 static inline
+int netdev_get_prio_tc_map(const struct net_device *dev, u32 prio)
+{
+	return dev->prio_tc_map[prio & 15];
+}
+
+static inline
+int netdev_set_prio_tc_map(struct net_device *dev, u8 prio, u8 tc)
+{
+	if (tc >= dev->num_tc)
+		return -EINVAL;
+
+	dev->prio_tc_map[prio & 15] = tc & 15;
+	return 0;
+}
+
+static inline
+int netdev_set_tc_queue(struct net_device *dev, u8 tc, u16 count, u16 offset)
+{
+	struct netdev_tc_txq *tcp;
+
+	if (tc >= dev->num_tc)
+		return -EINVAL;
+
+	tcp = &dev->_tc_to_txq[tc];
+	tcp->count = count;
+	tcp->offset = offset;
+	return 0;
+}
+
+static inline
+struct netdev_tc_txq *netdev_get_tc_queue(const struct net_device *dev, u8 tc)
+{
+	return &dev->_tc_to_txq[tc];
+}
+
+static inline
+int netdev_set_num_tc(struct net_device *dev, u8 num_tc)
+{
+	if (num_tc > dev->max_tc)
+		return -EINVAL;
+
+	dev->num_tc = num_tc;
+	return 0;
+}
+
+static inline
+u8 netdev_get_num_tc(const struct net_device *dev)
+{
+	return dev->num_tc;
+}
+
+static inline
 struct netdev_queue *netdev_get_tx_queue(const struct net_device *dev,
 					 unsigned int index)
 {
@@ -1386,6 +1448,9 @@ static inline void unregister_netdevice(struct net_device *dev)
 	unregister_netdevice_queue(dev, NULL);
 }
 
+extern int		netdev_alloc_max_tc(struct net_device *dev, u8 tc);
+extern void		netdev_free_tc(struct net_device *dev);
+
 extern int 		netdev_refcnt_read(const struct net_device *dev);
 extern void		free_netdev(struct net_device *dev);
 extern void		synchronize_net(void);
diff --git a/net/core/dev.c b/net/core/dev.c
index 55ff66f..cc00e66 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -2118,6 +2118,8 @@ static u32 hashrnd __read_mostly;
 u16 skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb)
 {
 	u32 hash;
+	u16 qoffset = 0;
+	u16 qcount = dev->real_num_tx_queues;
 
 	if (skb_rx_queue_recorded(skb)) {
 		hash = skb_get_rx_queue(skb);
@@ -2126,13 +2128,20 @@ u16 skb_tx_hash(const struct net_device *dev, const struct sk_buff *skb)
 		return hash;
 	}
 
+	if (dev->num_tc) {
+		u8 tc = netdev_get_prio_tc_map(dev, skb->priority);
+		struct netdev_tc_txq *tcp = netdev_get_tc_queue(dev, tc);
+		qoffset = tcp->offset;
+		qcount = tcp->count;
+	}
+
 	if (skb->sk && skb->sk->sk_hash)
 		hash = skb->sk->sk_hash;
 	else
 		hash = (__force u16) skb->protocol ^ skb->rxhash;
 	hash = jhash_1word(hash, hashrnd);
 
-	return (u16) (((u64) hash * dev->real_num_tx_queues) >> 32);
+	return (u16) ((((u64) hash * qcount)) >> 32) + qoffset;
 }
 EXPORT_SYMBOL(skb_tx_hash);
 
@@ -5091,6 +5100,33 @@ void netif_stacked_transfer_operstate(const struct net_device *rootdev,
 }
 EXPORT_SYMBOL(netif_stacked_transfer_operstate);
 
+int netdev_alloc_max_tc(struct net_device *dev, u8 tcs)
+{
+	struct netdev_tc_txq *tcp;
+
+	if (tcs > 16)
+		return -EINVAL;
+
+	tcp = kcalloc(tcs, sizeof(*tcp), GFP_KERNEL);
+	if (!tcp)
+		return -ENOMEM;
+
+	dev->_tc_to_txq = tcp;
+	dev->max_tc = tcs;
+	return 0;
+}
+EXPORT_SYMBOL(netdev_alloc_max_tc);
+
+void netdev_free_tc(struct net_device *dev)
+{
+	dev->max_tc = 0;
+	dev->num_tc = 0;
+	memset(dev->prio_tc_map, 0, sizeof(dev->prio_tc_map));
+	kfree(dev->_tc_to_txq);
+	dev->_tc_to_txq = NULL;
+}
+EXPORT_SYMBOL(netdev_free_tc);
+
 #ifdef CONFIG_RPS
 static int netif_alloc_rx_queues(struct net_device *dev)
 {
@@ -5699,6 +5735,7 @@ void free_netdev(struct net_device *dev)
 #ifdef CONFIG_RPS
 	kfree(dev->_rx);
 #endif
+	netdev_free_tc(dev);
 
 	kfree(rcu_dereference_raw(dev->ingress_queue));
 


^ permalink raw reply related

* [RFC PATCH 2/4] net/sched: Allow multiple mq qdisc to be used as non-root
From: John Fastabend @ 2010-12-09 20:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, hadi, shemminger, tgraf, eric.dumazet, john.r.fastabend
In-Reply-To: <20101209195956.3554.49511.stgit@jf-dev1-dcblab>

This patch modifies the mq qdisc to allow multiple mq qdiscs
to be used. Allowing TX queues to be grouped for management.

This allows a root container qdisc to create multiple traffic
classes and use the mq qdisc as a default queueing discipline. It
is expected other queueing disciplines can then be grafted to the
container as needed.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 net/sched/sch_mq.c |   73 +++++++++++++++++++++++++++++++++++++++++-----------
 1 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index ecc302f..deac04c 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -19,17 +19,42 @@
 
 struct mq_sched {
 	struct Qdisc		**qdiscs;
+	u8 num_tc;
 };
 
+static void mq_queues(struct net_device *dev, struct Qdisc *sch,
+		      unsigned int *count, unsigned int *offset)
+{
+	struct mq_sched *priv = qdisc_priv(sch);
+	if (priv->num_tc) {
+		struct netdev_tc_txq *tc;
+		int queue = TC_H_MIN(sch->parent) - 1;
+
+		tc = netdev_get_tc_queue(dev, queue);
+		if (count)
+			*count = tc->count;
+		if (offset)
+			*offset = tc->offset;
+	} else {
+		if (count)
+			*count = dev->num_tx_queues;
+		if (offset)
+			*offset = 0;
+	}
+}
+
 static void mq_destroy(struct Qdisc *sch)
 {
 	struct net_device *dev = qdisc_dev(sch);
 	struct mq_sched *priv = qdisc_priv(sch);
-	unsigned int ntx;
+	unsigned int ntx, count;
 
 	if (!priv->qdiscs)
 		return;
-	for (ntx = 0; ntx < dev->num_tx_queues && priv->qdiscs[ntx]; ntx++)
+
+	mq_queues(dev, sch, &count, NULL);
+
+	for (ntx = 0; ntx < count && priv->qdiscs[ntx]; ntx++)
 		qdisc_destroy(priv->qdiscs[ntx]);
 	kfree(priv->qdiscs);
 }
@@ -41,21 +66,26 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt)
 	struct netdev_queue *dev_queue;
 	struct Qdisc *qdisc;
 	unsigned int ntx;
+	unsigned int count, offset;
 
-	if (sch->parent != TC_H_ROOT)
+	if (sch->parent != TC_H_ROOT && !dev->num_tc)
 		return -EOPNOTSUPP;
 
 	if (!netif_is_multiqueue(dev))
 		return -EOPNOTSUPP;
 
+	/* Record num tc's in priv so we can tear down cleanly */
+	priv->num_tc = dev->num_tc;
+	mq_queues(dev, sch, &count, &offset);
+
 	/* pre-allocate qdiscs, attachment can't fail */
-	priv->qdiscs = kcalloc(dev->num_tx_queues, sizeof(priv->qdiscs[0]),
+	priv->qdiscs = kcalloc(count, sizeof(priv->qdiscs[0]),
 			       GFP_KERNEL);
 	if (priv->qdiscs == NULL)
 		return -ENOMEM;
 
-	for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
-		dev_queue = netdev_get_tx_queue(dev, ntx);
+	for (ntx = 0; ntx < count; ntx++) {
+		dev_queue = netdev_get_tx_queue(dev, ntx + offset);
 		qdisc = qdisc_create_dflt(dev_queue, &pfifo_fast_ops,
 					  TC_H_MAKE(TC_H_MAJ(sch->handle),
 						    TC_H_MIN(ntx + 1)));
@@ -65,7 +95,8 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt)
 		priv->qdiscs[ntx] = qdisc;
 	}
 
-	sch->flags |= TCQ_F_MQROOT;
+	if (!priv->num_tc)
+		sch->flags |= TCQ_F_MQROOT;
 	return 0;
 
 err:
@@ -78,9 +109,11 @@ static void mq_attach(struct Qdisc *sch)
 	struct net_device *dev = qdisc_dev(sch);
 	struct mq_sched *priv = qdisc_priv(sch);
 	struct Qdisc *qdisc;
-	unsigned int ntx;
+	unsigned int ntx, count;
 
-	for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
+	mq_queues(dev, sch, &count, NULL);
+
+	for (ntx = 0; ntx < count; ntx++) {
 		qdisc = priv->qdiscs[ntx];
 		qdisc = dev_graft_qdisc(qdisc->dev_queue, qdisc);
 		if (qdisc)
@@ -94,14 +127,17 @@ static int mq_dump(struct Qdisc *sch, struct sk_buff *skb)
 {
 	struct net_device *dev = qdisc_dev(sch);
 	struct Qdisc *qdisc;
-	unsigned int ntx;
+	unsigned int ntx, count, offset;
+
+	mq_queues(dev, sch, &count, &offset);
 
 	sch->q.qlen = 0;
 	memset(&sch->bstats, 0, sizeof(sch->bstats));
 	memset(&sch->qstats, 0, sizeof(sch->qstats));
 
-	for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
-		qdisc = netdev_get_tx_queue(dev, ntx)->qdisc_sleeping;
+	for (ntx = 0; ntx < count; ntx++) {
+		int txq = ntx + offset;
+		qdisc = netdev_get_tx_queue(dev, txq)->qdisc_sleeping;
 		spin_lock_bh(qdisc_lock(qdisc));
 		sch->q.qlen		+= qdisc->q.qlen;
 		sch->bstats.bytes	+= qdisc->bstats.bytes;
@@ -120,10 +156,13 @@ static struct netdev_queue *mq_queue_get(struct Qdisc *sch, unsigned long cl)
 {
 	struct net_device *dev = qdisc_dev(sch);
 	unsigned long ntx = cl - 1;
+	unsigned int count, offset;
+
+	mq_queues(dev, sch, &count, &offset);
 
-	if (ntx >= dev->num_tx_queues)
+	if (ntx >= count)
 		return NULL;
-	return netdev_get_tx_queue(dev, ntx);
+	return netdev_get_tx_queue(dev, offset + ntx);
 }
 
 static struct netdev_queue *mq_select_queue(struct Qdisc *sch,
@@ -203,13 +242,15 @@ static int mq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
 static void mq_walk(struct Qdisc *sch, struct qdisc_walker *arg)
 {
 	struct net_device *dev = qdisc_dev(sch);
-	unsigned int ntx;
+	unsigned int ntx, count;
+
+	mq_queues(dev, sch, &count, NULL);
 
 	if (arg->stop)
 		return;
 
 	arg->count = arg->skip;
-	for (ntx = arg->skip; ntx < dev->num_tx_queues; ntx++) {
+	for (ntx = arg->skip; ntx < count; ntx++) {
 		if (arg->fn(sch, ntx + 1, arg) < 0) {
 			arg->stop = 1;
 			break;


^ permalink raw reply related

* [RFC PATCH 3/4] net/sched: implement a root container qdisc sch_mclass
From: John Fastabend @ 2010-12-09 20:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, hadi, shemminger, tgraf, eric.dumazet, john.r.fastabend
In-Reply-To: <20101209195956.3554.49511.stgit@jf-dev1-dcblab>

This implements a mclass 'multi-class' queueing discipline that by
default creates multiple mq qdisc's one for each traffic class. Each
mq qdisc then owns a range of queues per the netdev_tc_txq mappings.

Using the mclass qdisc the number of tcs currently in use along
with the range of queues alloted to each class can be configured. By
default skbs are mapped to traffic classes using the skb priority.
This mapping is configurable.

To support HW QOS schemes on inflexible HW that require fixed
mappings between queues and classes a net device ops ndo_setup_tc
is used. The HW setup may be overridden.

Finally, qdiscs graft'd onto the mclass qdisc must be mq like in that
they must map queueing disciplines onto the netdev_queue.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 include/linux/netdevice.h |    3 
 include/linux/pkt_sched.h |    9 +
 include/net/sch_generic.h |    1 
 net/sched/Makefile        |    2 
 net/sched/sch_api.c       |    1 
 net/sched/sch_generic.c   |    8 +
 net/sched/sch_mclass.c    |  332 +++++++++++++++++++++++++++++++++++++++++++++
 7 files changed, 354 insertions(+), 2 deletions(-)
 create mode 100644 net/sched/sch_mclass.c

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c0d4fb1..ac265bb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -762,6 +762,8 @@ struct netdev_tc_txq {
  * int (*ndo_set_vf_port)(struct net_device *dev, int vf,
  *			  struct nlattr *port[]);
  * int (*ndo_get_vf_port)(struct net_device *dev, int vf, struct sk_buff *skb);
+ *
+ * int (*ndo_setup_tc)(struct net_device *dev, int tc);
  */
 #define HAVE_NET_DEVICE_OPS
 struct net_device_ops {
@@ -820,6 +822,7 @@ struct net_device_ops {
 						   struct nlattr *port[]);
 	int			(*ndo_get_vf_port)(struct net_device *dev,
 						   int vf, struct sk_buff *skb);
+	int			(*ndo_setup_tc)(struct net_device *dev, u8 tc);
 #if defined(CONFIG_FCOE) || defined(CONFIG_FCOE_MODULE)
 	int			(*ndo_fcoe_enable)(struct net_device *dev);
 	int			(*ndo_fcoe_disable)(struct net_device *dev);
diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h
index 2cfa4bc..0134ed4 100644
--- a/include/linux/pkt_sched.h
+++ b/include/linux/pkt_sched.h
@@ -481,4 +481,13 @@ struct tc_drr_stats {
 	__u32	deficit;
 };
 
+/* MCLASS */
+struct tc_mclass_qopt {
+	__u8	num_tc;
+	__u8	prio_tc_map[16];
+	__u8	hw;
+	__u16	count[16];
+	__u16	offset[16];
+};
+
 #endif
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index ea1f8a8..2bbcd09 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -276,6 +276,7 @@ extern struct Qdisc noop_qdisc;
 extern struct Qdisc_ops noop_qdisc_ops;
 extern struct Qdisc_ops pfifo_fast_ops;
 extern struct Qdisc_ops mq_qdisc_ops;
+extern struct Qdisc_ops mclass_qdisc_ops;
 
 struct Qdisc_class_common {
 	u32			classid;
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 960f5db..76dcf5b 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -2,7 +2,7 @@
 # Makefile for the Linux Traffic Control Unit.
 #
 
-obj-y	:= sch_generic.o sch_mq.o
+obj-y	:= sch_generic.o sch_mq.o sch_mclass.o
 
 obj-$(CONFIG_NET_SCHED)		+= sch_api.o sch_blackhole.o
 obj-$(CONFIG_NET_CLS)		+= cls_api.o
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c
index b22ca2d..24f40e0 100644
--- a/net/sched/sch_api.c
+++ b/net/sched/sch_api.c
@@ -1770,6 +1770,7 @@ static int __init pktsched_init(void)
 	register_qdisc(&bfifo_qdisc_ops);
 	register_qdisc(&pfifo_head_drop_qdisc_ops);
 	register_qdisc(&mq_qdisc_ops);
+	register_qdisc(&mclass_qdisc_ops);
 
 	rtnl_register(PF_UNSPEC, RTM_NEWQDISC, tc_modify_qdisc, NULL);
 	rtnl_register(PF_UNSPEC, RTM_DELQDISC, tc_get_qdisc, NULL);
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 0918834..73ed9b7 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -709,7 +709,13 @@ static void attach_default_qdiscs(struct net_device *dev)
 		dev->qdisc = txq->qdisc_sleeping;
 		atomic_inc(&dev->qdisc->refcnt);
 	} else {
-		qdisc = qdisc_create_dflt(txq, &mq_qdisc_ops, TC_H_ROOT);
+		if (dev->num_tc)
+			qdisc = qdisc_create_dflt(txq, &mclass_qdisc_ops,
+						  TC_H_ROOT);
+		else
+			qdisc = qdisc_create_dflt(txq, &mq_qdisc_ops,
+						  TC_H_ROOT);
+
 		if (qdisc) {
 			qdisc->ops->attach(qdisc);
 			dev->qdisc = qdisc;
diff --git a/net/sched/sch_mclass.c b/net/sched/sch_mclass.c
new file mode 100644
index 0000000..1ff156c
--- /dev/null
+++ b/net/sched/sch_mclass.c
@@ -0,0 +1,332 @@
+/*
+ * net/sched/sch_mclass.c
+ *
+ * Copyright (c) 2010 John Fastabend <john.r.fastabend@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * version 2 as published by the Free Software Foundation.
+ */
+
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+#include <net/sch_generic.h>
+
+struct mclass_sched {
+	struct Qdisc		**qdiscs;
+	int hw_owned;
+};
+
+static void mclass_destroy(struct Qdisc *sch)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct mclass_sched *priv = qdisc_priv(sch);
+	unsigned int ntc;
+
+	if (!priv->qdiscs)
+		return;
+
+	for (ntc = 0; ntc < dev->num_tc && priv->qdiscs[ntc]; ntc++)
+		qdisc_destroy(priv->qdiscs[ntc]);
+
+	if (priv->hw_owned && dev->netdev_ops->ndo_setup_tc)
+		dev->netdev_ops->ndo_setup_tc(dev, 0);
+	else
+		netdev_set_num_tc(dev, 0);
+
+	kfree(priv->qdiscs);
+}
+
+static int mclass_parse_opt(struct net_device *dev, struct tc_mclass_qopt *qopt)
+{
+	int i, j;
+
+	/* Verify TC offset and count are sane */
+	for (i = 0; i < qopt->num_tc; i++) {
+		int last = qopt->offset[i] + qopt->count[i];
+		if (last > dev->num_tx_queues)
+			return -EINVAL;
+		for (j = i + 1; j < qopt->num_tc; j++) {
+			if (last > qopt->offset[j])
+				return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int mclass_init(struct Qdisc *sch, struct nlattr *opt)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct mclass_sched *priv = qdisc_priv(sch);
+	struct netdev_queue *dev_queue;
+	struct Qdisc *qdisc;
+	int i, err = -EOPNOTSUPP;
+	struct tc_mclass_qopt *qopt = NULL;
+
+	if (sch->parent != TC_H_ROOT)
+		return -EOPNOTSUPP;
+
+	if (!netif_is_multiqueue(dev))
+		return -EOPNOTSUPP;
+
+	if (nla_len(opt) < sizeof(*qopt))
+		return -EINVAL;
+	qopt = nla_data(opt);
+
+	/* Workaround inflexible hardware where drivers may want to align
+	 * TX queues and traffic class support to provide HW offloaded
+	 * QOS.
+	 */
+	if (qopt->hw && dev->netdev_ops->ndo_setup_tc) {
+		priv->hw_owned = 1;
+		if (dev->netdev_ops->ndo_setup_tc(dev, qopt->num_tc))
+			return -EINVAL;
+	} else {
+		if (mclass_parse_opt(dev, qopt))
+			return -EINVAL;
+
+		if (!dev->max_tc && netdev_alloc_max_tc(dev, 16))
+			return -ENOMEM;
+
+		if (netdev_set_num_tc(dev, qopt->num_tc))
+			return -ENOMEM;
+
+		for (i = 0; i < qopt->num_tc; i++)
+			netdev_set_tc_queue(dev, i,
+					    qopt->count[i], qopt->offset[i]);
+	}
+
+	for (i = 0; i < 16; i++) {
+		if (netdev_set_prio_tc_map(dev, i, qopt->prio_tc_map[i])) {
+			err = -EINVAL;
+			goto tc_err;
+		}
+	}
+
+	/* pre-allocate qdiscs, attachment can't fail */
+	priv->qdiscs = kcalloc(qopt->num_tc,
+			       sizeof(priv->qdiscs[0]), GFP_KERNEL);
+	if (priv->qdiscs == NULL) {
+		err = -ENOMEM;
+		goto tc_err;
+	}
+
+	for (i = 0; i < dev->num_tc; i++) {
+		dev_queue = netdev_get_tx_queue(dev, 0);
+		qdisc = qdisc_create_dflt(dev_queue, &mq_qdisc_ops,
+					  TC_H_MAKE(TC_H_MAJ(sch->handle),
+						    TC_H_MIN(i + 1)));
+		if (qdisc == NULL) {
+			err = -ENOMEM;
+			goto err;
+		}
+		qdisc->flags |= TCQ_F_CAN_BYPASS;
+		priv->qdiscs[i] = qdisc;
+	}
+
+	sch->flags |= TCQ_F_MQROOT;
+	return 0;
+
+err:
+	mclass_destroy(sch);
+tc_err:
+	if (priv->hw_owned)
+		dev->netdev_ops->ndo_setup_tc(dev, 0);
+	else
+		netdev_set_num_tc(dev, 0);
+	return err;
+}
+
+static void mclass_attach(struct Qdisc *sch)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct mclass_sched *priv = qdisc_priv(sch);
+	struct Qdisc *qdisc;
+	unsigned int ntc;
+
+	/* Attach underlying qdisc */
+	for (ntc = 0; ntc < dev->num_tc; ntc++) {
+		qdisc = priv->qdiscs[ntc];
+		if (qdisc->ops && qdisc->ops->attach)
+			qdisc->ops->attach(qdisc);
+	}
+}
+
+static int mclass_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
+		    struct Qdisc **old)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct mclass_sched *priv = qdisc_priv(sch);
+	unsigned long ntc = cl - 1;
+
+	if (ntc >= dev->num_tc)
+		return -EINVAL;
+
+	if (dev->flags & IFF_UP)
+		dev_deactivate(dev);
+
+	*old = priv->qdiscs[ntc];
+	if (new == NULL)
+		new = &noop_qdisc;
+	priv->qdiscs[ntc] = new;
+	qdisc_reset(*old);
+
+	if (dev->flags & IFF_UP)
+		dev_activate(dev);
+
+	return 0;
+}
+
+static int mclass_dump(struct Qdisc *sch, struct sk_buff *skb)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct mclass_sched *priv = qdisc_priv(sch);
+	unsigned char *b = skb_tail_pointer(skb);
+	struct tc_mclass_qopt opt;
+	struct Qdisc *qdisc;
+	unsigned int ntc;
+
+	sch->q.qlen = 0;
+	memset(&sch->bstats, 0, sizeof(sch->bstats));
+	memset(&sch->qstats, 0, sizeof(sch->qstats));
+
+	for (ntc = 0; ntc < dev->num_tc; ntc++) {
+		qdisc = priv->qdiscs[ntc];
+		spin_lock_bh(qdisc_lock(qdisc));
+		sch->q.qlen		+= qdisc->q.qlen;
+		sch->bstats.bytes	+= qdisc->bstats.bytes;
+		sch->bstats.packets	+= qdisc->bstats.packets;
+		sch->qstats.qlen	+= qdisc->qstats.qlen;
+		sch->qstats.backlog	+= qdisc->qstats.backlog;
+		sch->qstats.drops	+= qdisc->qstats.drops;
+		sch->qstats.requeues	+= qdisc->qstats.requeues;
+		sch->qstats.overlimits	+= qdisc->qstats.overlimits;
+		spin_unlock_bh(qdisc_lock(qdisc));
+	}
+
+	opt.num_tc = dev->num_tc;
+	memcpy(opt.prio_tc_map, dev->prio_tc_map, 16);
+	opt.hw = priv->hw_owned;
+
+	for (ntc = 0; ntc < dev->num_tc; ntc++) {
+		struct netdev_tc_txq *tcp = &dev->_tc_to_txq[ntc];
+		opt.count[ntc] = tcp->count;
+		opt.offset[ntc] = tcp->offset;
+	}
+
+	NLA_PUT(skb, TCA_OPTIONS, sizeof(opt), &opt);
+
+	return skb->len;
+nla_put_failure:
+	nlmsg_trim(skb, b);
+	return -1;
+}
+
+static struct Qdisc *mclass_leaf(struct Qdisc *sch, unsigned long cl)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	struct mclass_sched *priv = qdisc_priv(sch);
+	unsigned long ntc = cl - 1;
+
+	if (ntc >= dev->num_tc)
+		return NULL;
+	return priv->qdiscs[ntc];
+}
+
+static unsigned long mclass_get(struct Qdisc *sch, u32 classid)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	unsigned int ntc = TC_H_MIN(classid);
+
+	if (ntc >= dev->num_tc)
+		return 0;
+	return ntc;
+}
+
+static void mclass_put(struct Qdisc *sch, unsigned long cl)
+{
+}
+
+static int mclass_dump_class(struct Qdisc *sch, unsigned long cl,
+			 struct sk_buff *skb, struct tcmsg *tcm)
+{
+	struct Qdisc *class;
+	struct net_device *dev = qdisc_dev(sch);
+	struct mclass_sched *priv = qdisc_priv(sch);
+	unsigned long ntc = cl - 1;
+
+	if (ntc >= dev->num_tc)
+		return -EINVAL;
+
+	class = priv->qdiscs[ntc];
+
+	tcm->tcm_parent = TC_H_ROOT;
+	tcm->tcm_handle |= TC_H_MIN(cl);
+	tcm->tcm_info = class->handle;
+	return 0;
+}
+
+static int mclass_dump_class_stats(struct Qdisc *sch, unsigned long cl,
+			       struct gnet_dump *d)
+{
+	struct Qdisc *class;
+	struct net_device *dev = qdisc_dev(sch);
+	struct mclass_sched *priv = qdisc_priv(sch);
+	unsigned long ntc = cl - 1;
+
+	if (ntc >= dev->num_tc)
+		return -EINVAL;
+
+	class = priv->qdiscs[ntc];
+	class->qstats.qlen = class->q.qlen;
+	if (gnet_stats_copy_basic(d, &class->bstats) < 0 ||
+	    gnet_stats_copy_queue(d, &class->qstats) < 0)
+		return -1;
+	return 0;
+}
+
+static void mclass_walk(struct Qdisc *sch, struct qdisc_walker *arg)
+{
+	struct net_device *dev = qdisc_dev(sch);
+	unsigned long ntc;
+
+	if (arg->stop)
+		return;
+
+	arg->count = arg->skip;
+	for (ntc = arg->skip; ntc < dev->num_tc; ntc++) {
+		if (arg->fn(sch, ntc + 1, arg) < 0) {
+			arg->stop = 1;
+			break;
+		}
+		arg->count++;
+	}
+}
+
+static const struct Qdisc_class_ops mclass_class_ops = {
+	.graft		= mclass_graft,
+	.leaf		= mclass_leaf,
+	.get		= mclass_get,
+	.put		= mclass_put,
+	.walk		= mclass_walk,
+	.dump		= mclass_dump_class,
+	.dump_stats	= mclass_dump_class_stats,
+};
+
+struct Qdisc_ops mclass_qdisc_ops __read_mostly = {
+	.cl_ops		= &mclass_class_ops,
+	.id		= "mclass",
+	.priv_size	= sizeof(struct mclass_sched),
+	.init		= mclass_init,
+	.destroy	= mclass_destroy,
+	.attach		= mclass_attach,
+	.dump		= mclass_dump,
+	.owner		= THIS_MODULE,
+};


^ permalink raw reply related

* [RFC PATCH 4/4] ixgbe: add multiple txqs per tc
From: John Fastabend @ 2010-12-09 20:00 UTC (permalink / raw)
  To: davem; +Cc: netdev, hadi, shemminger, tgraf, eric.dumazet, john.r.fastabend
In-Reply-To: <20101209195956.3554.49511.stgit@jf-dev1-dcblab>

This illustrates the usage model for hardware QOS offloading.

Currently, DCB only enables a single queue per tc. Due to
complications with how to map tc filter rules to traffic classes
when multiple queues are enabled. And previously there was no
mechanism to map flows to multiple queues by priority.

Using the QOS offloading API we allocate multiple queues per
tc and configure the stack to hash across these queues. The
hardware then offloads the DCB extended transmission selection
algorithm. Sockets can set the priority using the SO_PRIORITY
socket option and expect ETS to work.

Signed-off-by: John Fastabend <john.r.fastabend@intel.com>
---

 drivers/net/ixgbe/ixgbe.h        |    2 
 drivers/net/ixgbe/ixgbe_dcb_nl.c |    4 -
 drivers/net/ixgbe/ixgbe_main.c   |  256 ++++++++++++++++++++++----------------
 3 files changed, 149 insertions(+), 113 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe.h b/drivers/net/ixgbe/ixgbe.h
index 3ae30b8..860b1fa 100644
--- a/drivers/net/ixgbe/ixgbe.h
+++ b/drivers/net/ixgbe/ixgbe.h
@@ -243,7 +243,7 @@ enum ixgbe_ring_f_enum {
 	RING_F_ARRAY_SIZE      /* must be last in enum set */
 };
 
-#define IXGBE_MAX_DCB_INDICES   8
+#define IXGBE_MAX_DCB_INDICES  64
 #define IXGBE_MAX_RSS_INDICES  16
 #define IXGBE_MAX_VMDQ_INDICES 64
 #define IXGBE_MAX_FDIR_INDICES 64
diff --git a/drivers/net/ixgbe/ixgbe_dcb_nl.c b/drivers/net/ixgbe/ixgbe_dcb_nl.c
index bf566e8..d49b8ce 100644
--- a/drivers/net/ixgbe/ixgbe_dcb_nl.c
+++ b/drivers/net/ixgbe/ixgbe_dcb_nl.c
@@ -354,12 +354,12 @@ static u8 ixgbe_dcbnl_set_all(struct net_device *netdev)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(netdev);
 	int ret;
+	int tc = max(netdev->num_tc, MAX_TRAFFIC_CLASS);
 
 	if (!adapter->dcb_set_bitmap)
 		return DCB_NO_HW_CHG;
 
-	ret = ixgbe_copy_dcb_cfg(&adapter->temp_dcb_cfg, &adapter->dcb_cfg,
-				 adapter->ring_feature[RING_F_DCB].indices);
+	ret = ixgbe_copy_dcb_cfg(&adapter->temp_dcb_cfg, &adapter->dcb_cfg, tc);
 
 	if (ret)
 		return DCB_NO_HW_CHG;
diff --git a/drivers/net/ixgbe/ixgbe_main.c b/drivers/net/ixgbe/ixgbe_main.c
index a12e86f..46b700d 100644
--- a/drivers/net/ixgbe/ixgbe_main.c
+++ b/drivers/net/ixgbe/ixgbe_main.c
@@ -638,6 +638,43 @@ void ixgbe_unmap_and_free_tx_resource(struct ixgbe_ring *tx_ring,
 	/* tx_buffer_info must be completely set up in the transmit path */
 }
 
+#define IXGBE_MAX_Q_PER_TC	(IXGBE_MAX_DCB_INDICES / MAX_TRAFFIC_CLASS)
+
+/* ixgbe setup routine for many traffic classes hardware only supports
+ * 4 or 8 traffic classes.
+ *
+ * JF: Todo, software should be able to map arbitrary TCs to 4 or 8 HW
+ * tcs. For illustration purposes require 4 or 8 tcs for now.
+ */
+int ixgbe_setup_tc(struct net_device *dev, u8 tcs)
+{
+	struct ixgbe_adapter *adapter = netdev_priv(dev);
+	int i, err = 0;
+	unsigned int q, offset = 0;
+
+	if (!tcs) {
+		err = netdev_set_num_tc(dev, tcs);
+	} else if (tcs != 4 || tcs != 8) {
+		if (!dev->max_tc && netdev_alloc_max_tc(dev, tcs))
+			return -ENOMEM;
+
+		if (netdev_set_num_tc(dev, tcs))
+			return -EINVAL;
+
+		/* Partition TX queues evenly amongst traffic classes */
+		for (i = 0; i < tcs; i++) {
+			q = min((int)num_online_cpus(), IXGBE_MAX_Q_PER_TC);
+			netdev_set_prio_tc_map(adapter->netdev, i, i);
+			netdev_set_tc_queue(adapter->netdev, i, q, offset);
+			offset += q;
+		}
+	} else {
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
 /**
  * ixgbe_dcb_txq_to_tc - convert a reg index to a traffic class
  * @adapter: driver private struct
@@ -651,7 +688,7 @@ void ixgbe_unmap_and_free_tx_resource(struct ixgbe_ring *tx_ring,
 u8 ixgbe_dcb_txq_to_tc(struct ixgbe_adapter *adapter, u8 reg_idx)
 {
 	int tc = -1;
-	int dcb_i = adapter->ring_feature[RING_F_DCB].indices;
+	u8 num_tcs = netdev_get_num_tc(adapter->netdev);
 
 	/* if DCB is not enabled the queues have no TC */
 	if (!(adapter->flags & IXGBE_FLAG_DCB_ENABLED))
@@ -666,13 +703,13 @@ u8 ixgbe_dcb_txq_to_tc(struct ixgbe_adapter *adapter, u8 reg_idx)
 		tc = reg_idx >> 2;
 		break;
 	default:
-		if (dcb_i != 4 && dcb_i != 8)
+		if (num_tcs != 4 && num_tcs != 8)
 			break;
 
 		/* if VMDq is enabled the lowest order bits determine TC */
 		if (adapter->flags & (IXGBE_FLAG_SRIOV_ENABLED |
 				      IXGBE_FLAG_VMDQ_ENABLED)) {
-			tc = reg_idx & (dcb_i - 1);
+			tc = reg_idx & (num_tcs - 1);
 			break;
 		}
 
@@ -685,9 +722,9 @@ u8 ixgbe_dcb_txq_to_tc(struct ixgbe_adapter *adapter, u8 reg_idx)
 		 * will only ever be 8 or 4 and that reg_idx will never
 		 * be greater then 128. The code without the power of 2
 		 * optimizations would be:
-		 * (((reg_idx % 32) + 32) * dcb_i) >> (9 - reg_idx / 32)
+		 * (((reg_idx % 32) + 32) * num_tcs) >> (9 - reg_idx / 32)
 		 */
-		tc = ((reg_idx & 0X1F) + 0x20) * dcb_i;
+		tc = ((reg_idx & 0X1F) + 0x20) * num_tcs;
 		tc >>= 9 - (reg_idx >> 5);
 	}
 
@@ -4205,10 +4242,17 @@ static inline bool ixgbe_set_dcb_queues(struct ixgbe_adapter *adapter)
 {
 	bool ret = false;
 	struct ixgbe_ring_feature *f = &adapter->ring_feature[RING_F_DCB];
+	int i, q;
 
 	if (!(adapter->flags & IXGBE_FLAG_DCB_ENABLED))
 		return ret;
 
+	f->indices = 0;
+	for (i = 0; i < MAX_TRAFFIC_CLASS; i++) {
+		q = min((int)num_online_cpus(), MAX_TRAFFIC_CLASS);
+		f->indices += q;
+	}
+
 	f->mask = 0x7 << 3;
 	adapter->num_rx_queues = f->indices;
 	adapter->num_tx_queues = f->indices;
@@ -4295,12 +4339,7 @@ static inline bool ixgbe_set_fcoe_queues(struct ixgbe_adapter *adapter)
 	if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
 		adapter->num_rx_queues = 1;
 		adapter->num_tx_queues = 1;
-#ifdef CONFIG_IXGBE_DCB
-		if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-			e_info(probe, "FCoE enabled with DCB\n");
-			ixgbe_set_dcb_queues(adapter);
-		}
-#endif
+
 		if (adapter->flags & IXGBE_FLAG_RSS_ENABLED) {
 			e_info(probe, "FCoE enabled with RSS\n");
 			if ((adapter->flags & IXGBE_FLAG_FDIR_HASH_CAPABLE) ||
@@ -4356,16 +4395,15 @@ static int ixgbe_set_num_queues(struct ixgbe_adapter *adapter)
 	if (ixgbe_set_sriov_queues(adapter))
 		goto done;
 
-#ifdef IXGBE_FCOE
-	if (ixgbe_set_fcoe_queues(adapter))
-		goto done;
-
-#endif /* IXGBE_FCOE */
 #ifdef CONFIG_IXGBE_DCB
 	if (ixgbe_set_dcb_queues(adapter))
 		goto done;
-
 #endif
+
+#ifdef IXGBE_FCOE
+	if (ixgbe_set_fcoe_queues(adapter))
+		goto done;
+#endif /* IXGBE_FCOE */
 	if (ixgbe_set_fdir_queues(adapter))
 		goto done;
 
@@ -4457,6 +4495,63 @@ static inline bool ixgbe_cache_ring_rss(struct ixgbe_adapter *adapter)
 }
 
 #ifdef CONFIG_IXGBE_DCB
+
+ /* ixgbe_get_first_reg_idx - Return first register index associated
+ *  with this traffic class
+ */
+void ixgbe_get_first_reg_idx(struct ixgbe_adapter *adapter, u8 tc,
+				     unsigned int *tx, unsigned int *rx)
+{
+	struct net_device *dev = adapter->netdev;
+	struct ixgbe_hw *hw = &adapter->hw;
+	u8 num_tcs = netdev_get_num_tc(dev);
+
+	*tx = 0;
+	*rx = 0;
+
+	switch (hw->mac.type) {
+	case ixgbe_mac_82598EB:
+		*tx = tc << 3;
+		*rx = tc << 2;
+		break;
+	case ixgbe_mac_82599EB:
+	case ixgbe_mac_X540:
+		if (num_tcs == 8) {
+			if (tc < 3) {
+				*tx = tc << 5;
+				*rx = tc << 4;
+			} else if (tc <  5) {
+				*tx = ((tc + 2) << 4);
+				*rx = tc << 4;
+			} else if (tc < num_tcs) {
+				*tx = ((tc + 8) << 3);
+				*rx = tc << 4;
+			}
+		} else if (num_tcs == 4) {
+				*rx =  tc << 5;
+				switch (tc) {
+				case 0:
+					*tx =  0;
+					break;
+				case 1:
+					*tx = 64;
+					break;
+				case 2:
+					*tx = 96;
+					break;
+				case 3:
+					*tx = 112;
+					break;
+				default:
+					break;
+				}
+		}
+		break;
+	default:
+		break;
+	}
+}
+
 /**
  * ixgbe_cache_ring_dcb - Descriptor ring to register mapping for DCB
  * @adapter: board private structure to initialize
@@ -4466,72 +4561,26 @@ static inline bool ixgbe_cache_ring_rss(struct ixgbe_adapter *adapter)
  **/
 static inline bool ixgbe_cache_ring_dcb(struct ixgbe_adapter *adapter)
 {
-	int i;
-	bool ret = false;
-	int dcb_i = adapter->ring_feature[RING_F_DCB].indices;
+	struct net_device *dev = adapter->netdev;
+	int i, j, k;
+	u8 num_tcs = netdev_get_num_tc(dev);
+	unsigned int tx_s, rx_s;
 
 	if (!(adapter->flags & IXGBE_FLAG_DCB_ENABLED))
 		return false;
 
 	/* the number of queues is assumed to be symmetric */
-	switch (adapter->hw.mac.type) {
-	case ixgbe_mac_82598EB:
-		for (i = 0; i < dcb_i; i++) {
-			adapter->rx_ring[i]->reg_idx = i << 3;
-			adapter->tx_ring[i]->reg_idx = i << 2;
+	for (i = 0, k = 0; i < num_tcs; i++) {
+		struct netdev_tc_txq *tcp = netdev_get_tc_queue(dev, i);
+		u16 qcount = tcp->count;
+		ixgbe_get_first_reg_idx(adapter, i, &tx_s, &rx_s);
+		for (j = 0; j < qcount; j++, k++) {
+			adapter->tx_ring[k]->reg_idx = tx_s + j;
+			adapter->rx_ring[k]->reg_idx = rx_s + j;
 		}
-		ret = true;
-		break;
-	case ixgbe_mac_82599EB:
-	case ixgbe_mac_X540:
-		if (dcb_i == 8) {
-			/*
-			 * Tx TC0 starts at: descriptor queue 0
-			 * Tx TC1 starts at: descriptor queue 32
-			 * Tx TC2 starts at: descriptor queue 64
-			 * Tx TC3 starts at: descriptor queue 80
-			 * Tx TC4 starts at: descriptor queue 96
-			 * Tx TC5 starts at: descriptor queue 104
-			 * Tx TC6 starts at: descriptor queue 112
-			 * Tx TC7 starts at: descriptor queue 120
-			 *
-			 * Rx TC0-TC7 are offset by 16 queues each
-			 */
-			for (i = 0; i < 3; i++) {
-				adapter->tx_ring[i]->reg_idx = i << 5;
-				adapter->rx_ring[i]->reg_idx = i << 4;
-			}
-			for ( ; i < 5; i++) {
-				adapter->tx_ring[i]->reg_idx = ((i + 2) << 4);
-				adapter->rx_ring[i]->reg_idx = i << 4;
-			}
-			for ( ; i < dcb_i; i++) {
-				adapter->tx_ring[i]->reg_idx = ((i + 8) << 3);
-				adapter->rx_ring[i]->reg_idx = i << 4;
-			}
-			ret = true;
-		} else if (dcb_i == 4) {
-			/*
-			 * Tx TC0 starts at: descriptor queue 0
-			 * Tx TC1 starts at: descriptor queue 64
-			 * Tx TC2 starts at: descriptor queue 96
-			 * Tx TC3 starts at: descriptor queue 112
-			 *
-			 * Rx TC0-TC3 are offset by 32 queues each
-			 */
-			adapter->tx_ring[0]->reg_idx = 0;
-			adapter->tx_ring[1]->reg_idx = 64;
-			adapter->tx_ring[2]->reg_idx = 96;
-			adapter->tx_ring[3]->reg_idx = 112;
-			for (i = 0 ; i < dcb_i; i++)
-				adapter->rx_ring[i]->reg_idx = i << 5;
-			ret = true;
-		}
-		break;
-	default:
-		break;
 	}
-	return ret;
+
+	return true;
 }
 #endif
 
@@ -4659,17 +4708,15 @@ static void ixgbe_cache_ring_register(struct ixgbe_adapter *adapter)
 
 	if (ixgbe_cache_ring_sriov(adapter))
 		return;
-
+#ifdef CONFIG_IXGBE_DCB
+	if (ixgbe_cache_ring_dcb(adapter))
+		return;
+#endif /* IXGBE_DCB */
 #ifdef IXGBE_FCOE
 	if (ixgbe_cache_ring_fcoe(adapter))
 		return;
 
 #endif /* IXGBE_FCOE */
-#ifdef CONFIG_IXGBE_DCB
-	if (ixgbe_cache_ring_dcb(adapter))
-		return;
-
-#endif
 	if (ixgbe_cache_ring_fdir(adapter))
 		return;
 
@@ -5133,7 +5180,7 @@ static int __devinit ixgbe_sw_init(struct ixgbe_adapter *adapter)
 	adapter->dcb_cfg.round_robin_enable = false;
 	adapter->dcb_set_bitmap = 0x00;
 	ixgbe_copy_dcb_cfg(&adapter->dcb_cfg, &adapter->temp_dcb_cfg,
-			   adapter->ring_feature[RING_F_DCB].indices);
+			   MAX_TRAFFIC_CLASS);
 
 #endif
 
@@ -5986,7 +6033,7 @@ static void ixgbe_watchdog_task(struct work_struct *work)
 		if (link_up) {
 #ifdef CONFIG_DCB
 			if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-				for (i = 0; i < MAX_TRAFFIC_CLASS; i++)
+				for (i = 0; i < netdev->max_tc; i++)
 					hw->mac.ops.fc_enable(hw, i);
 			} else {
 				hw->mac.ops.fc_enable(hw, 0);
@@ -6511,25 +6558,6 @@ static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb)
 {
 	struct ixgbe_adapter *adapter = netdev_priv(dev);
 	int txq = smp_processor_id();
-#ifdef IXGBE_FCOE
-	__be16 protocol;
-
-	protocol = vlan_get_protocol(skb);
-
-	if ((protocol == htons(ETH_P_FCOE)) ||
-	    (protocol == htons(ETH_P_FIP))) {
-		if (adapter->flags & IXGBE_FLAG_FCOE_ENABLED) {
-			txq &= (adapter->ring_feature[RING_F_FCOE].indices - 1);
-			txq += adapter->ring_feature[RING_F_FCOE].mask;
-			return txq;
-#ifdef CONFIG_IXGBE_DCB
-		} else if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-			txq = adapter->fcoe.up;
-			return txq;
-#endif
-		}
-	}
-#endif
 
 	if (adapter->flags & IXGBE_FLAG_FDIR_HASH_CAPABLE) {
 		while (unlikely(txq >= dev->real_num_tx_queues))
@@ -6537,14 +6565,20 @@ static u16 ixgbe_select_queue(struct net_device *dev, struct sk_buff *skb)
 		return txq;
 	}
 
-	if (adapter->flags & IXGBE_FLAG_DCB_ENABLED) {
-		if (skb->priority == TC_PRIO_CONTROL)
-			txq = adapter->ring_feature[RING_F_DCB].indices-1;
-		else
-			txq = (skb->vlan_tci & IXGBE_TX_FLAGS_VLAN_PRIO_MASK)
-			       >> 13;
+#ifdef IXGBE_FCOE
+	/*
+	 * If DCB is not enabled to assign FCoE a priority mapping
+	 * we need to steer the skb to FCoE enabled tx rings.
+	 */
+	if ((adapter->flags & IXGBE_FLAG_FCOE_ENABLED) &&
+	    !(adapter->flags & IXGBE_FLAG_DCB_ENABLED) &&
+	    ((skb->protocol == htons(ETH_P_FCOE)) ||
+	     (skb->protocol == htons(ETH_P_FIP)))) {
+		txq &= (adapter->ring_feature[RING_F_FCOE].indices - 1);
+		txq += adapter->ring_feature[RING_F_FCOE].mask;
 		return txq;
 	}
+#endif
 
 	return skb_tx_hash(dev, skb);
 }
@@ -6867,6 +6901,7 @@ static const struct net_device_ops ixgbe_netdev_ops = {
 	.ndo_set_vf_tx_rate	= ixgbe_ndo_set_vf_bw,
 	.ndo_get_vf_config	= ixgbe_ndo_get_vf_config,
 	.ndo_get_stats64	= ixgbe_get_stats64,
+	.ndo_setup_tc		= ixgbe_setup_tc,
 #ifdef CONFIG_NET_POLL_CONTROLLER
 	.ndo_poll_controller	= ixgbe_netpoll,
 #endif
@@ -7007,8 +7042,9 @@ static int __devinit ixgbe_probe(struct pci_dev *pdev,
 	else
 		indices = min_t(unsigned int, indices, IXGBE_MAX_FDIR_INDICES);
 
+#if defined(CONFIG_IXGBE_DCB)
 	indices = max_t(unsigned int, indices, IXGBE_MAX_DCB_INDICES);
-#ifdef IXGBE_FCOE
+#elif defined(IXGBE_FCOE)
 	indices += min_t(unsigned int, num_possible_cpus(),
 			 IXGBE_MAX_FCOE_INDICES);
 #endif


^ permalink raw reply related

* [trivial PATCH 00/15] remove duplicate unlikely from IS_ERR
From: Joe Perches @ 2010-12-09 20:03 UTC (permalink / raw)
  To: netdev, uclinux-dist-devel, rtc-linux, linux-s390, osd-dev,
	linux-arm-msm, linux-
  Cc: Jiri Kosina, dri-devel, linux-kernel, linux-scsi, linux-wireless,
	devel
In-Reply-To: <1291906801-1389-2-git-send-email-tklauser@distanz.ch>

Tobias Klauser <tklauser@distanz.ch> sent a patch to remove
an unnecessary unlikely from drivers/misc/c2port/core.c,
https://lkml.org/lkml/2010/12/9/199

Here are the other instances treewide.

I think it'd be good if people would, when noticing defects in a
specific subsystem, look for and correct the same defect treewide.

IS_ERR already has an unlikely test so remove unnecessary
unlikelys from the call sites.

from: include/linux/err.h
#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
[...]
static inline long __must_check IS_ERR(const void *ptr)
{
	return IS_ERR_VALUE((unsigned long)ptr);
}

Sending directly to maintainers for now, will resend in a month
or so only to trivial if not picked up.
 
Joe Perches (15):
  drm: Remove duplicate unlikely from IS_ERR
  stmmac: Remove duplicate unlikely from IS_ERR
  rtc: Remove duplicate unlikely from IS_ERR
  s390: Remove duplicate unlikely from IS_ERR
  osd: Remove duplicate unlikely from IS_ERR
  serial: Remove duplicate unlikely from IS_ERR
  brcm80211: Remove duplicate unlikely from IS_ERR
  gadget: Remove duplicate unlikely from IS_ERR
  exofs: Remove duplicate unlikely from IS_ERR
  ext2: Remove duplicate unlikely from IS_ERR
  ext3: Remove duplicate unlikely from IS_ERR
  ext4: Remove duplicate unlikely from IS_ERR
  nfs: Remove duplicate unlikely from IS_ERR
  mm: Remove duplicate unlikely from IS_ERR
  ipv6: Remove duplicate unlikely from IS_ERR

 drivers/gpu/drm/ttm/ttm_tt.c                     |    4 ++--
 drivers/net/stmmac/stmmac_main.c                 |    2 +-
 drivers/rtc/rtc-bfin.c                           |    2 +-
 drivers/s390/scsi/zfcp_fsf.c                     |    4 ++--
 drivers/scsi/osd/osd_initiator.c                 |    2 +-
 drivers/serial/msm_serial.c                      |    2 +-
 drivers/staging/brcm80211/brcmfmac/wl_cfg80211.c |    2 +-
 drivers/usb/gadget/f_fs.c                        |    4 ++--
 fs/exofs/super.c                                 |    2 +-
 fs/ext2/namei.c                                  |    2 +-
 fs/ext3/namei.c                                  |    2 +-
 fs/ext4/namei.c                                  |    2 +-
 fs/nfs/mount_clnt.c                              |    2 +-
 mm/vmalloc.c                                     |    2 +-
 net/ipv6/af_inet6.c                              |    2 +-
 15 files changed, 18 insertions(+), 18 deletions(-)

-- 
1.7.3.3.464.gf80b6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* [PATCH 02/15] stmmac: Remove duplicate unlikely from IS_ERR
From: Joe Perches @ 2010-12-09 20:03 UTC (permalink / raw)
  To: Giuseppe Cavallaro; +Cc: Jiri Kosina, netdev, linux-kernel
In-Reply-To: <cover.1291923888.git.joe@perches.com>

IS_ERR already uses unlikely, remove unlikely from the call sites.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/stmmac/stmmac_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/stmmac/stmmac_main.c b/drivers/net/stmmac/stmmac_main.c
index c0dc785..20f803d 100644
--- a/drivers/net/stmmac/stmmac_main.c
+++ b/drivers/net/stmmac/stmmac_main.c
@@ -949,7 +949,7 @@ static int stmmac_sw_tso(struct stmmac_priv *priv, struct sk_buff *skb)
 	       skb, skb->len);
 
 	segs = skb_gso_segment(skb, priv->dev->features & ~NETIF_F_TSO);
-	if (unlikely(IS_ERR(segs)))
+	if (IS_ERR(segs))
 		goto sw_tso_end;
 
 	do {
-- 
1.7.3.3.464.gf80b6

^ permalink raw reply related

* [PATCH 15/15] ipv6: Remove duplicate unlikely from IS_ERR
From: Joe Perches @ 2010-12-09 20:04 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, Pekka Savola (ipv6),
	James Morris, Hideaki YOSHIFUJI <yosh
  Cc: Jiri Kosina, netdev, linux-kernel
In-Reply-To: <cover.1291923888.git.joe@perches.com>

IS_ERR already uses unlikely, remove unlikely from the call sites.

Signed-off-by: Joe Perches <joe@perches.com>
---
 net/ipv6/af_inet6.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index 54e8e42..059a3de 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -810,7 +810,7 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb, int features)
 	}
 	rcu_read_unlock();
 
-	if (unlikely(IS_ERR(segs)))
+	if (IS_ERR(segs))
 		goto out;
 
 	for (skb = segs; skb; skb = skb->next) {
-- 
1.7.3.3.464.gf80b6

^ permalink raw reply related

* Re: [PATCH] netfilter: don't need to initialize instance_table
From: David Miller @ 2010-12-09 20:05 UTC (permalink / raw)
  To: xiaosuo; +Cc: jengelh, kaber, netfilter-devel, netdev
In-Reply-To: <AANLkTin6u9ZkSNAoxR7az5yBW-WM0RebPcCrOMOYgFrP@mail.gmail.com>

From: Changli Gao <xiaosuo@gmail.com>
Date: Thu, 9 Dec 2010 22:27:24 +0800

> On Thu, Dec 9, 2010 at 10:03 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>>
>> This is here for correctness and documentation. If the hlist
>> implementation changed, one would have a hard time figuring out the
>> callsites which then need to add the initialization back.
>>
> 
> I am quite sure there are other users rely on this assumption. So who
> wants to change the macro INIT_HLIST_HEAD() has to review all the
> code. It is a huge task, but can't be avoided.

Wrong.  Adding debugging facilities to the hlist head would just
require changing this macro.

That is, unless we apply patches like your's, which we won't.

^ permalink raw reply

* Re: [stable] [PATCH 2.6.36] vlan: Avoid hwaccel vlan packets when vid not used
From: Greg KH @ 2010-12-09 20:13 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David Miller, lkml20101129, netdev, jesse, linux-kernel, greearb,
	stable
In-Reply-To: <1291857933.2795.7.camel@edumazet-laptop>

On Thu, Dec 09, 2010 at 02:25:33AM +0100, Eric Dumazet wrote:
> Le mercredi 08 décembre 2010 à 15:16 -0800, Greg KH a écrit :
> > On Wed, Dec 08, 2010 at 08:47:31AM -0800, David Miller wrote:
> > > From: Eric Dumazet <eric.dumazet@gmail.com>
> > > Date: Wed, 01 Dec 2010 11:55:14 +0100
> > > 
> > > 
> > > Greg/-stable, please integrate this patch from Eric into 2.6.36 if you
> > > haven't already done so.
> > 
> > I've updated the version in the .36-stable queue with this one, thanks.
> > 
> 
> Thanks.
> 
> By the way, all credits given to Jesse, I only made a change to his
> patch, so I left him as the author.
> 
> Ah I see I forgot the "From: Jesse Gross <jesse@nicira.com>" header when
> I sent it, my bad, sorry !

No problem, I have that in the patch header already :)

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH] Fix 2.6.34-rc1 regression in disable_ipv6 support
From: David Miller @ 2010-12-09 20:20 UTC (permalink / raw)
  To: shemminger
  Cc: ebiederm, brian.haley, netdev, maheshkelkar, lorenzo, yoshfuji,
	stable
In-Reply-To: <20101209111611.1d2e6e2b@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Thu, 9 Dec 2010 11:16:11 -0800

> No but since removing address propagates up to user space daemons
> like Quagga please analyze and fix the problem, don't just look
> for band aid.

Stephen, we lived with the previous behavior for 12+ years.

You broke stuff that did work before your change.

Putting the onus on Eric to fix it exactly how you want it to
be fixed is therefore not appropriate.

You seem to be putting exactly zero effort into trying to reproduce
the problem yourself and fixing a bug you introduced.  And hey we
have a standard way to deal with a regression when the guilty party
is uncooperative, revert.

There are therefore three choices:

1) Revert.  And this is the one I'm favoring because of how you are
   handling this issue.  The responsibility to resolve this regression
   is your's not Eric's.

   Frankly, Eric is being incredibly nice by working on trying to fix
   a bug which you introduced.

2) Accept Eric's proposed fix.

3) Figure out the real bug yourself and fix the problem the way you
   find acceptable in a reasonable, short, amount of time.

Loopback has always been special, especially on ipv6.  When we don't
have a device to point something at, we point it at loopback.

Also destination cache entries which still have references when they
get zapped get pointed at loopback.

^ permalink raw reply

* Re: [PATCH] Fix 2.6.34-rc1 regression in disable_ipv6 support
From: David Miller @ 2010-12-09 20:20 UTC (permalink / raw)
  To: ebiederm
  Cc: shemminger, brian.haley, netdev, maheshkelkar, lorenzo, yoshfuji,
	stable
In-Reply-To: <m1y67y3fqf.fsf@fess.ebiederm.org>

From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 09 Dec 2010 11:31:36 -0800

> You fix the problem.  You introduced the regression, and you didn't test
> that keeping addresses actually worked.  This is not the first patch
> that has been applied to fix regressions in this area.

Exactly, I fully agree with Eric.

^ 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