Netdev List
 help / color / mirror / Atom feed
* [PATCH] ip : fix truesize mismatch in ip fragmentation
From: Eric Dumazet @ 2010-09-21 14:16 UTC (permalink / raw)
  To: Nick Bowler; +Cc: linux-kernel, netdev, David S. Miller
In-Reply-To: <20100921140501.GA21572@elliptictech.com>

Le mardi 21 septembre 2010 à 10:05 -0400, Nick Bowler a écrit :

> This hunk introduces some whitespace damage.
> 
> Anyway, I tried this with ESP on both IPv4 and IPv6 and it appears to
> correct the issue.  Thanks!
> 

Indeed good catch.

Here is an updated patch, I added your Tested-by

Thanks for testing !

[PATCH] ip : fix truesize mismatch in ip fragmentation

We should not set frag->destructor to sock_wkfree() until we are sure we
dont hit slow path in ip_fragment(). Or we risk uncharging
frag->truesize twice, and in the end, having negative socket
sk_wmem_alloc counter, or even freeing socket sooner than expected.

Many thanks to Nick Bowler, who provided a very clean bug report and
test program.

While Nick bisection pointed to commit 2b85a34e911bf483 (net: No more
expensive sock_hold()/sock_put() on each tx), underlying bug is older.

Reported-and-bisected-by: Nick Bowler <nbowler@elliptictech.com>
Tested-by: Nick Bowler <nbowler@elliptictech.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv4/ip_output.c  |    8 ++++----
 net/ipv6/ip6_output.c |    8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 04b6989..126d9b3 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -490,7 +490,6 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 	if (skb_has_frags(skb)) {
 		struct sk_buff *frag;
 		int first_len = skb_pagelen(skb);
-		int truesizes = 0;
 
 		if (first_len - hlen > mtu ||
 		    ((first_len - hlen) & 7) ||
@@ -510,11 +509,13 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 				goto slow_path;
 
 			BUG_ON(frag->sk);
-			if (skb->sk) {
+		}
+		if (skb->sk) {
+			skb_walk_frags(skb, frag) {
 				frag->sk = skb->sk;
 				frag->destructor = sock_wfree;
+				skb->truesize -= frag->truesize;
 			}
-			truesizes += frag->truesize;
 		}
 
 		/* Everything is OK. Generate! */
@@ -524,7 +525,6 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 		frag = skb_shinfo(skb)->frag_list;
 		skb_frag_list_init(skb);
 		skb->data_len = first_len - skb_headlen(skb);
-		skb->truesize -= truesizes;
 		skb->len = first_len;
 		iph->tot_len = htons(first_len);
 		iph->frag_off = htons(IP_MF);
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index d40b330..633217d 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -639,7 +639,6 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 
 	if (skb_has_frags(skb)) {
 		int first_len = skb_pagelen(skb);
-		int truesizes = 0;
 
 		if (first_len - hlen > mtu ||
 		    ((first_len - hlen) & 7) ||
@@ -658,10 +657,12 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 				goto slow_path;
 
 			BUG_ON(frag->sk);
-			if (skb->sk) {
+		}
+		if (skb->sk) {
+			skb_walk_frags(skb, frag) {
 				frag->sk = skb->sk;
 				frag->destructor = sock_wfree;
-				truesizes += frag->truesize;
+				skb->truesize -= frag->truesize;
 			}
 		}
 
@@ -693,7 +694,6 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 
 		first_len = skb_pagelen(skb);
 		skb->data_len = first_len - skb_headlen(skb);
-		skb->truesize -= truesizes;
 		skb->len = first_len;
 		ipv6_hdr(skb)->payload_len = htons(first_len -
 						   sizeof(struct ipv6hdr));

^ permalink raw reply related

* Re: [PATCH] net/sched/sch_hfsc.c: initialize parent's cl_cfmin properly in init_vf()
From: Patrick McHardy @ 2010-09-21 15:00 UTC (permalink / raw)
  To: David Miller; +Cc: soltys, denys, netdev
In-Reply-To: <20100917.164127.09754402.davem@davemloft.net>

Am 18.09.2010 01:41, schrieb David Miller:
> From: Patrick McHardy <kaber@trash.net>
> Date: Wed, 15 Sep 2010 19:54:49 +0200
> 
>> Am 01.09.2010 23:30, schrieb David Miller:
>>> From: Michal Soltys <soltys@ziu.info>
>>> Date: Mon, 30 Aug 2010 23:34:10 +0200
>>>
>>>> This patch fixes init_vf() function, so on each new backlog period parent's
>>>> cl_cfmin is properly updated (including further propgation towards the root),
>>>> even if the activated leaf has no upperlimit curve defined.
>>>>
>>>> Signed-off-by: Michal Soltys <soltys@ziu.info>
>>>
>>> Applied, thanks.
>>
>> For the record, the patch seems fine to me. The root cause seems to be
>> an optimization I introduced (pre-git, even history.git unfortunately)
>> in vttree_get_minvt() that wasn't present in the original version:
>>
>> static struct hfsc_class *
>> vttree_get_minvt(struct hfsc_class *cl, u64 cur_time)
>> {
>>         /* if root-class's cfmin is bigger than cur_time nothing to do */
>>         if (cl->cl_cfmin > cur_time)
>>                 return NULL;
> 
> Actually, the HFSC scheduler was added with this test present.
> The function had a different name, actlist_get_minvt(), at the
> time.  See commit:
> 
> commit 6e22ce74ea0666a869ce82f418ce78b5be089fb4
> Author: Patrick McHardy <kaber@trash.net>
> Date:   Wed Jan 28 20:15:03 2004 -0800
> 
>     [NET_SCHED]: Add HFSC packet scheduler.
> 
> in the history-2.6 GIT tree.

Indeed, what I meant to say was that I made this optimization
before HFSC was added to BitKeeper, so there's no changelog
present for this particular optimization.

^ permalink raw reply

* Re: [RFC] skge: limit DMA to 32 bit
From: Stephen Hemminger @ 2010-09-21 15:02 UTC (permalink / raw)
  To: Stanislaw Gruszka; +Cc: netdev
In-Reply-To: <20100921071518.GA9081@redhat.com>

On Tue, 21 Sep 2010 09:15:19 +0200
Stanislaw Gruszka <sgruszka@redhat.com> wrote:

> We have old fedora bug report about not working skge device on some
> Gigabyte motherboard:
> https://bugzilla.redhat.com/show_bug.cgi?id=447489
> 
> In dmesg there are messages:
> 
> skge 0000:02:0b.0: PCI error cmd=0x7 status=0x22b0
> skge 0000:02:0b.0: unable to clear error (so ignoring them)
> 
> Status value 0x22b0 include PCI_STATUS_REC_MASTER_ABORT bit set, what
> mean that DMA transfer on PCI bus was aborted.
> 
> I provided to user modified driver with module parameter, that allow
> to experiment with different dma_mask's. He told that maximum working 
> dma_mask for him is 32 bits set.
> 
> Is 32bit DMA limit for every skge device acceptable patch?

No. The device does support 64 bit DMA, the problem is a crap implementation
of the device the motherboard. Therefore a PCI quirk or module parameter
is a better solution.

^ permalink raw reply

* Re: [PATCH v5 2/2] netfilter: save the hash of the tuple in the original direction for latter use
From: Patrick McHardy @ 2010-09-21 15:32 UTC (permalink / raw)
  To: Changli Gao
  Cc: David S. Miller, Eric Dumazet, Mathieu Desnoyers, akpm,
	netfilter-devel, netdev, linux-kernel
In-Reply-To: <AANLkTi=NXj9PjYA06+UFYms8q5K3Z4DsPMcpVHpj7Gmq@mail.gmail.com>

Am 21.09.2010 02:02, schrieb Changli Gao:
> On Tue, Sep 21, 2010 at 1:08 AM, Patrick McHardy <kaber@trash.net> wrote:
>>
>> Sure we can, dropping unconfirmed conntracks is a rare exception,
>> not a common case. Even under DoS we usually drop *unassured*
>> conntracks, which have already enterered the hash. If we're unable
>> to do that, we won't even allocate a new conntrack.
>>
> 
> Even so, saving the hash of the reply tuple isn't a good idea.
> 
> If NAT is turned on, the current code is:
> 
> mangle the reply tuple -> compute the hash of the reply tuple ->
> insert into the conntrack hash table.
> 
> the new code is
> 
> compute the hash of the reply tuple -> mangle the reply tuple ->
> recompute the hash of the reply tuple -> insert into the conntrack
> hash table.
> 
> As you see, the hash computing is done twice, and we use more CPU
> cycles than before.

You're right of course, we actually don't compute the reply hash
before inserting the conntrack into the hash table (except in a
few NAT cases, but we can look at those later).

^ permalink raw reply

* Re: [RFC][PATCH 1/4] IRQ: IRQ groups for multiqueue devices
From: Thomas Gleixner @ 2010-09-21 15:34 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Tom Herbert, netdev, linux-net-drivers, linux-kernel,
	Peter Zijlstra, Ingo Molnar
In-Reply-To: <1285071945.2307.21.camel@achroite.uk.solarflarecom.com>

On Tue, 21 Sep 2010, Ben Hutchings wrote:
> On Mon, 2010-09-20 at 23:27 +0200, Thomas Gleixner wrote:
> > > queue for which the response will be handled on the same or a nearby
> > > CPU.  IRQ groups hold a mapping of CPU to IRQ which will be updated
> > > based on the inverse of IRQ CPU-affinities plus CPU topology
> > > information.
> > 
> > Can you please explain, why you need that reverse mapping including
> > the below code ? What problem does this solve which can not be deduced
> > by the exisiting information/infrastructure ? And why is that reverse
> > mapping tied to interrupts and not something which we want to see in
> > some generic available (library) code ?
> 
> Are you thinking of a per-CPU distance map?  That seems like it would be
> useful to have.  I wonder about the storage requirements on larger
> systems.

  That might be a problem, but we should at least look into this.

  But even if a prebuilt map is too expensive storage wise, then the
  functions which build the lookup map might be useful for similar
  distance lookup scenarios as well.

> > > +static inline struct irq_group *alloc_irq_group(unsigned int size, gfp_t flags)
> > > +{
> > > +	static struct irq_group dummy;
> > 
> >   That will create one static instance per callsite. Is that on
> >   purpose? If yes, it needs a damned good comment.
> 
> Which uses no storage, or maybe 1 byte.  I'm open to suggestions for a
> better dummy implementation.
> 
> > > +	return &dummy;

  Why does it need to return a real struct instead of NULL ?
 
> > > +	}
> > 
> >   This is called from irq_set_affinity() with desc->lock held and
> >   interrupts disabled. You're not serious about that, are you ?
> > 
> >   Damned, there are two iterations over each online cpu and another
> >   one over the affinity mask. Did you ever extrapolate how long that
> >   runs on a really large machine ?
> 
> No, and I know this sucks.  So, do you want to suggest anything better?
> Should I look at building those CPU distance maps (trading space for
> time)?

  The point is that there is no reason why this code needs to run in a
  spinlocked irq disabled reason. I know why your implementation needs
  it, because it protects you against module unload etc. Come on, we
  have enough infrastructure to control the lifetime of objects in
  sane ways and not by hijacking a heavy lock and disabling
  interrupts.

  Btw, how does this code deal with concurrent affinity setting of
  multiple irqs in the group ? Random number generator ?

> > > + *	@flags:		Allocation flags e.g. %GFP_KERNEL

  Why do we need flags here? That code gets called in a device setup
  routine and not from random context.

> > > + */
> > > +struct irq_group *alloc_irq_group(unsigned int size, gfp_t flags)
> > > +{

> > > +
> > > +/**
> > > + *	free_irq_group - free IRQ group
> > > + *	@group:		IRQ group allocated with alloc_irq_group(), or %NULL
> > 
> >   How is this serialized or sanity checked against free_irq() ?
> 
> free_irq() should be called first, for all IRQs in the group.  The IRQs
> should then be really freed with pci_disable_msix() or similar.

  I explained that several times, that I do not care about what should
  be called in which order and what not. Driver coders get it wrong
  all the time, then it explodes in the genirq code and I get the crap
  to debug. No, thanks.

  Also if you call free_irq() first, then you still have a reference
  to that very irq descriptor in your group and a reference to the
  group in the irq descriptor. Brilliant, as the irq descriptor can be
  reused immediately after free_irq().

  Also it's not only problematic against free_irq, it's damned racy
  against a concurrent affinity setting as well. And that's not
  controlled by "should be called" at all.

> > > +/**
> > > + *	irq_group_add - add IRQ to a group
> > > + *	@group:		IRQ group allocated with alloc_irq_group()
> > > + *	@irq:		Interrupt to add to group
> > > + */
> > > +void irq_group_add(struct irq_group *group, unsigned int irq)
> > > +{
> > > +	struct irq_desc *desc = irq_to_desc(irq);
> > 
> >   Again, how is this serialized against anything else fiddling with
> >   irq_desc[irq]?
> 
> The driver should call this after allocating IRQs with pci_enable_msix()
> or similar and before setting the handlers with request_irq().  Is that
> sufficient?

  "should call" is _NEVER_ sufficient. fiddling with irq_desc w/o
  holding the lock is a nono, period.

> > > +	BUG_ON(desc->group);
> > > +	BUG_ON(group->used >= group->size);
> > > +
> > > +	desc->group = group;
> > > +	desc->group_index = group->used;
> > > +	group->irq[group->used++] = desc;

  Forgot to ask yesterday. Why do we need to store irq desc in the
  group ? For free_irq_group() I guess, but I explained already why
  this code is broken. And it's even more broken when an irq is moved
  to a different node and CONFIG_NUMA_IRQ_DESC=y.

Lemme summarize what I understand so far. For multiqueue devices with
several queues and interrupts you want to lookup the closest queue/irq
to the cpu on which you are currently running to avoid expensive cross
node memory access. Now you need to follow affinity changes for this
lookup table.

The basic idea of your code is ok. The lookup table itself is fine,
but the integration into the genirq code sucks. What needs to be done:

1) Remove the references to irq_desc in the group. They are not needed
   and will never work. The group does not need any information about
   the irq number and the irq descriptor.

2) Add proper refcounting to struct irq_group so it can be accessed
   outside of irq_desc->lock.

3) Move the update of the map outside of irq_desc->lock and the irq
   disabled region. Update the map in setup_affinity() as well. That
   code can be called from various atomic contexts, so it's probably
   the best thing to delegate the update to a workqueue or such unless
   you come up with a nice prebuilt lookup table.

4) Add comments so the code is understandable for mere mortals

5) Add proper install/free handling. Hint: affinity_hint

6) All modifications of irq_desc need a check whether irq_to_desc
   returned a valid pointer and must hold desc->lock

7) Deal with concurrent updates of multiple irqs in a group (at least
   a comment why the code does not need serialization)

8) Move the code to kernel/irq/irqgroup.c or some other sensible name
   and make it configurable so embedded folks don't have to carry it
   around as useless binary bloat.

Thanks,

	tglx

^ permalink raw reply

* Re: [PATCH v6] netfilter: save the hash of the tuple in the original direction for latter use
From: Patrick McHardy @ 2010-09-21 15:50 UTC (permalink / raw)
  To: Changli Gao
  Cc: David S. Miller, Eric Dumazet, Mathieu Desnoyers, akpm,
	netfilter-devel, netdev, linux-kernel
In-Reply-To: <1284995176-2994-1-git-send-email-xiaosuo@gmail.com>

Am 20.09.2010 17:06, schrieb Changli Gao:
> Since we don't change the tuple in the original direction, we can save it
> in ct->tuplehash[IP_CT_DIR_REPLY].hnode.pprev for __nf_conntrack_confirm()
> use.
> 
> __hash_conntrack() is split into two steps: hash_conntrack_raw() is used
> to get the raw hash, and __hash_bucket() is used to get the bucket id.
> 
> In SYN-flood case, early_drop() doesn't need to recompute the hash again.

Applied, thanks Changli.

^ permalink raw reply

* [PATCH v3] ip: fix truesize mismatch in ip fragmentation
From: Eric Dumazet @ 2010-09-21 15:58 UTC (permalink / raw)
  To: Nick Bowler
  Cc: linux-kernel, netdev, David S. Miller, Jarek Poplawski,
	Patrick McHardy
In-Reply-To: <1285078613.2617.503.camel@edumazet-laptop>

Le mardi 21 septembre 2010 à 16:16 +0200, Eric Dumazet a écrit :
> Le mardi 21 septembre 2010 à 10:05 -0400, Nick Bowler a écrit :
> 
> > This hunk introduces some whitespace damage.
> > 
> > Anyway, I tried this with ESP on both IPv4 and IPv6 and it appears to
> > correct the issue.  Thanks!
> > 
> 
> Indeed good catch.
> 
> Here is an updated patch, I added your Tested-by
> 
> Thanks for testing !
> 
> [PATCH] ip : fix truesize mismatch in ip fragmentation
> 
> We should not set frag->destructor to sock_wkfree() until we are sure we
> dont hit slow path in ip_fragment(). Or we risk uncharging
> frag->truesize twice, and in the end, having negative socket
> sk_wmem_alloc counter, or even freeing socket sooner than expected.
> 
> Many thanks to Nick Bowler, who provided a very clean bug report and
> test program.
> 
> While Nick bisection pointed to commit 2b85a34e911bf483 (net: No more
> expensive sock_hold()/sock_put() on each tx), underlying bug is older.
> 


Hmm, while looking at git history, I found commit from Patrick
b2722b1c3a893e (ip_fragment: also adjust skb->truesize for packets not
owned by a socket)
As my patch reverts it, we probably want a more polished patch.

(Also port Patrick work to ipv6)

So here is a V3

[PATCH v3] ip: fix truesize mismatch in ip fragmentation

Special care should be taken when slow path is hit in ip_fragment() :

When walking through frags, we transfert truesize ownership from skb to
frags. Then if we hit a slow_path condition, we must undo this or risk
uncharging frags->truesize twice, and in the end, having negative socket
sk_wmem_alloc counter, or even freeing socket sooner than expected.

Many thanks to Nick Bowler, who provided a very clean bug report and
test program.

Thanks to Jarek for reviewing my first patch and providing a V2

While Nick bisection pointed to commit 2b85a34e911 (net: No more
expensive sock_hold()/sock_put() on each tx), underlying bug is older
(2.6.12-rc5)

A side effect is to extend work done in commit b2722b1c3a893e
(ip_fragment: also adjust skb->truesize for packets not owned by a
socket) to ipv6 as well.

Reported-and-bisected-by: Nick Bowler <nbowler@elliptictech.com>
Tested-by: Nick Bowler <nbowler@elliptictech.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Jarek Poplawski <jarkao2@gmail.com>
CC: Patrick McHardy <kaber@trash.net>
---
 net/ipv4/ip_output.c  |   19 +++++++++++++------
 net/ipv6/ip6_output.c |   18 +++++++++++++-----
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 04b6989..a643f7a 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -488,9 +488,8 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 	 * we can switch to copy when see the first bad fragment.
 	 */
 	if (skb_has_frags(skb)) {
-		struct sk_buff *frag;
+		struct sk_buff *frag, *frag2;
 		int first_len = skb_pagelen(skb);
-		int truesizes = 0;
 
 		if (first_len - hlen > mtu ||
 		    ((first_len - hlen) & 7) ||
@@ -503,18 +502,18 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 			if (frag->len > mtu ||
 			    ((frag->len & 7) && frag->next) ||
 			    skb_headroom(frag) < hlen)
-			    goto slow_path;
+			    goto slow_path_undo;
 
 			/* Partially cloned skb? */
 			if (skb_shared(frag))
-				goto slow_path;
+				goto slow_path_undo;
 
 			BUG_ON(frag->sk);
 			if (skb->sk) {
 				frag->sk = skb->sk;
 				frag->destructor = sock_wfree;
 			}
-			truesizes += frag->truesize;
+			skb->truesize -= frag->truesize;
 		}
 
 		/* Everything is OK. Generate! */
@@ -524,7 +523,6 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 		frag = skb_shinfo(skb)->frag_list;
 		skb_frag_list_init(skb);
 		skb->data_len = first_len - skb_headlen(skb);
-		skb->truesize -= truesizes;
 		skb->len = first_len;
 		iph->tot_len = htons(first_len);
 		iph->frag_off = htons(IP_MF);
@@ -576,6 +574,15 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 		}
 		IP_INC_STATS(dev_net(dev), IPSTATS_MIB_FRAGFAILS);
 		return err;
+
+slow_path_undo:
+		skb_walk_frags(skb, frag2) {
+			if (frag2 == frag)
+				break;
+			frag2->sk = NULL;
+			frag2->destructor = NULL;
+			skb->truesize += frag2->truesize;
+		}
 	}
 
 slow_path:
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index d40b330..ca7ba44 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -639,7 +639,7 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 
 	if (skb_has_frags(skb)) {
 		int first_len = skb_pagelen(skb);
-		int truesizes = 0;
+		struct sk_buff *frag2;
 
 		if (first_len - hlen > mtu ||
 		    ((first_len - hlen) & 7) ||
@@ -651,18 +651,18 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 			if (frag->len > mtu ||
 			    ((frag->len & 7) && frag->next) ||
 			    skb_headroom(frag) < hlen)
-			    goto slow_path;
+			    goto slow_path_undo;
 
 			/* Partially cloned skb? */
 			if (skb_shared(frag))
-				goto slow_path;
+				goto slow_path_undo;
 
 			BUG_ON(frag->sk);
 			if (skb->sk) {
 				frag->sk = skb->sk;
 				frag->destructor = sock_wfree;
-				truesizes += frag->truesize;
 			}
+			skb->truesize -= frag->truesize;
 		}
 
 		err = 0;
@@ -693,7 +693,6 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 
 		first_len = skb_pagelen(skb);
 		skb->data_len = first_len - skb_headlen(skb);
-		skb->truesize -= truesizes;
 		skb->len = first_len;
 		ipv6_hdr(skb)->payload_len = htons(first_len -
 						   sizeof(struct ipv6hdr));
@@ -756,6 +755,15 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 			      IPSTATS_MIB_FRAGFAILS);
 		dst_release(&rt->dst);
 		return err;
+
+slow_path_undo:
+		skb_walk_frags(skb, frag2) {
+			if (frag2 == frag)
+				break;
+			frag2->sk = NULL;
+			frag2->destructor = NULL;
+			skb->truesize += frag2->truesize;
+		}
 	}
 
 slow_path:

^ permalink raw reply related

* [PATCH] cxgbi: bug fixes and code cleanup
From: kxie-ut6Up61K2wZBDgjK7y7TUQ @ 2010-09-21 16:09 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA, linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	open-iscsi-/JYPxA39Uh5TLH3MbocFFw
  Cc: rranjan-ut6Up61K2wZBDgjK7y7TUQ, kxie-ut6Up61K2wZBDgjK7y7TUQ,
	James.Bottomley-d9PhHud1JfjCXq6kfMZ53/egYHeGw8Jk,
	michaelc-hcNo3dDEHLuVc3sceRu5cw, davem-fT/PcQaiUtIeIZ0/mPfg9Q

[PATCH] cxgbi: bug fixes and code cleanup

From: Karen Xie <kxie-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>

The patch includes the following changes.
- removed un-used code
- renamed alloc_cpl() to alloc_wr().
- fixed connecting over VLAN.
- updated cxgb4i connection setting and pagepod programming.

Signed-off-by: Karen Xie <kxie-ut6Up61K2wZBDgjK7y7TUQ@public.gmane.org>
---

 drivers/scsi/cxgbi/cxgb3i/cxgb3i.c |   65 ++++++++++++++-----
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.c |  126 +++++++++++++++++++-----------------
 drivers/scsi/cxgbi/cxgb4i/cxgb4i.h |    5 +
 drivers/scsi/cxgbi/libcxgbi.c      |   94 +++++++++++++--------------
 drivers/scsi/cxgbi/libcxgbi.h      |   18 +----
 5 files changed, 172 insertions(+), 136 deletions(-)


diff --git a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
index a01c1e2..a129a17 100644
--- a/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
+++ b/drivers/scsi/cxgbi/cxgb3i/cxgb3i.c
@@ -320,7 +320,7 @@ static u32 send_rx_credits(struct cxgbi_sock *csk, u32 credits)
 		"csk 0x%p,%u,0x%lx,%u, credit %u, dack %u.\n",
 		csk, csk->state, csk->flags, csk->tid, credits, dack);
 
-	skb = alloc_cpl(sizeof(*req), 0, GFP_ATOMIC);
+	skb = alloc_wr(sizeof(*req), 0, GFP_ATOMIC);
 	if (!skb) {
 		pr_info("csk 0x%p, credit %u, OOM.\n", csk, credits);
 		return 0;
@@ -572,7 +572,7 @@ static void act_open_retry_timer(unsigned long data)
 
 	cxgbi_sock_get(csk);
 	spin_lock_bh(&csk->lock);
-	skb = alloc_cpl(sizeof(struct cpl_act_open_req), 0, GFP_ATOMIC);
+	skb = alloc_wr(sizeof(struct cpl_act_open_req), 0, GFP_ATOMIC);
 	if (!skb)
 		cxgbi_sock_fail_act_open(csk, -ENOMEM);
 	else {
@@ -589,9 +589,10 @@ static int do_act_open_rpl(struct t3cdev *tdev, struct sk_buff *skb, void *ctx)
 	struct cxgbi_sock *csk = ctx;
 	struct cpl_act_open_rpl *rpl = cplhdr(skb);
 
-	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
-		"csk 0x%p,%u,0x%lx,%u, status %u.\n",
-		csk, csk->state, csk->flags, csk->atid, rpl->status);
+	pr_info("csk 0x%p,%u,0x%lx,%u, status %u, %pI4:%u-%pI4:%u.\n",
+		csk, csk->state, csk->flags, csk->atid, rpl->status,
+		&csk->saddr.sin_addr.s_addr, ntohs(csk->saddr.sin_port),
+		&csk->daddr.sin_addr.s_addr, ntohs(csk->daddr.sin_port));
 
 	if (rpl->status != CPL_ERR_TCAM_FULL &&
 	    rpl->status != CPL_ERR_CONN_EXIST &&
@@ -662,8 +663,7 @@ static int abort_status_to_errno(struct cxgbi_sock *csk, int abort_reason,
 	switch (abort_reason) {
 	case CPL_ERR_BAD_SYN: /* fall through */
 	case CPL_ERR_CONN_RESET:
-		return csk->state > CTP_ESTABLISHED ?
-			-EPIPE : -ECONNRESET;
+		return csk->state > CTP_ESTABLISHED ? -EPIPE : -ECONNRESET;
 	case CPL_ERR_XMIT_TIMEDOUT:
 	case CPL_ERR_PERSIST_TIMEDOUT:
 	case CPL_ERR_FINWAIT2_TIMEDOUT:
@@ -881,16 +881,16 @@ static int do_wr_ack(struct t3cdev *cdev, struct sk_buff *skb, void *ctx)
  */
 static int alloc_cpls(struct cxgbi_sock *csk)
 {
-	csk->cpl_close = alloc_cpl(sizeof(struct cpl_close_con_req), 0,
+	csk->cpl_close = alloc_wr(sizeof(struct cpl_close_con_req), 0,
 					GFP_KERNEL);
 	if (!csk->cpl_close)
 		return -ENOMEM;
-	csk->cpl_abort_req = alloc_cpl(sizeof(struct cpl_abort_req), 0,
+	csk->cpl_abort_req = alloc_wr(sizeof(struct cpl_abort_req), 0,
 					GFP_KERNEL);
 	if (!csk->cpl_abort_req)
 		goto free_cpl_skbs;
 
-	csk->cpl_abort_rpl = alloc_cpl(sizeof(struct cpl_abort_rpl), 0,
+	csk->cpl_abort_rpl = alloc_wr(sizeof(struct cpl_abort_rpl), 0,
 					GFP_KERNEL);
 	if (!csk->cpl_abort_rpl)
 		goto free_cpl_skbs;
@@ -945,17 +945,44 @@ static void release_offload_resources(struct cxgbi_sock *csk)
 	csk->cdev = NULL;
 }
 
+static void update_address(struct cxgbi_hba *chba)
+{
+	if (chba->ipv4addr) {
+		if (chba->vdev &&
+		    chba->ipv4addr != cxgb3i_get_private_ipv4addr(chba->vdev)) {
+			cxgb3i_set_private_ipv4addr(chba->vdev, chba->ipv4addr);
+			cxgb3i_set_private_ipv4addr(chba->ndev, 0);
+			pr_info("%s set %pI4.\n",
+				chba->vdev->name, &chba->ipv4addr);
+		} else if (chba->ipv4addr !=
+				cxgb3i_get_private_ipv4addr(chba->ndev)) {
+			cxgb3i_set_private_ipv4addr(chba->ndev, chba->ipv4addr);
+			pr_info("%s set %pI4.\n",
+				chba->ndev->name, &chba->ipv4addr);
+		}
+	} else if (cxgb3i_get_private_ipv4addr(chba->ndev)) {
+		if (chba->vdev)
+			cxgb3i_set_private_ipv4addr(chba->vdev, 0);
+		cxgb3i_set_private_ipv4addr(chba->ndev, 0);
+	}
+}
+
 static int init_act_open(struct cxgbi_sock *csk)
 {
 	struct dst_entry *dst = csk->dst;
 	struct cxgbi_device *cdev = csk->cdev;
 	struct t3cdev *t3dev = (struct t3cdev *)cdev->lldev;
 	struct net_device *ndev = cdev->ports[csk->port_id];
+	struct cxgbi_hba *chba = cdev->hbas[csk->port_id];
 	struct sk_buff *skb = NULL;
 
 	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
 		"csk 0x%p,%u,0x%lx.\n", csk, csk->state, csk->flags);
 
+	update_address(chba);
+	if (chba->ipv4addr)
+		csk->saddr.sin_addr.s_addr = chba->ipv4addr;
+
 	csk->rss_qid = 0;
 	csk->l2t = t3_l2t_get(t3dev, dst->neighbour, ndev);
 	if (!csk->l2t) {
@@ -972,7 +999,7 @@ static int init_act_open(struct cxgbi_sock *csk)
 	cxgbi_sock_set_flag(csk, CTPF_HAS_ATID);
 	cxgbi_sock_get(csk);
 
-	skb = alloc_cpl(sizeof(struct cpl_act_open_req), 0, GFP_KERNEL);
+	skb = alloc_wr(sizeof(struct cpl_act_open_req), 0, GFP_KERNEL);
 	if (!skb)
 		goto rel_resource;
 	skb->sk = (struct sock *)csk;
@@ -984,6 +1011,12 @@ static int init_act_open(struct cxgbi_sock *csk)
 	cxgbi_sock_reset_wr_list(csk);
 	csk->err = 0;
 
+	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
+		"csk 0x%p,%u,0x%lx, %pI4:%u-%pI4:%u.\n",
+		csk, csk->state, csk->flags,
+		&csk->saddr.sin_addr.s_addr, ntohs(csk->saddr.sin_port),
+		&csk->daddr.sin_addr.s_addr, ntohs(csk->daddr.sin_port));
+
 	cxgbi_sock_set_state(csk, CTP_ACTIVE_OPEN);
 	send_act_open_req(csk, skb, csk->l2t);
 	return 0;
@@ -1141,11 +1174,11 @@ static int ddp_alloc_gl_skb(struct cxgbi_ddp_info *ddp, int idx,
 		"ddp 0x%p, idx %d, cnt %d.\n", ddp, idx, cnt);
 
 	for (i = 0; i < cnt; i++) {
-		struct sk_buff *skb = alloc_cpl(sizeof(struct ulp_mem_io) +
+		struct sk_buff *skb = alloc_wr(sizeof(struct ulp_mem_io) +
 						PPOD_SIZE, 0, gfp);
-		if (skb) {
+		if (skb)
 			ddp->gl_skb[idx + i] = skb;
-		} else {
+		else {
 			ddp_free_gl_skb(ddp, idx, i);
 			return -ENOMEM;
 		}
@@ -1156,7 +1189,7 @@ static int ddp_alloc_gl_skb(struct cxgbi_ddp_info *ddp, int idx,
 static int ddp_setup_conn_pgidx(struct cxgbi_sock *csk,
 				       unsigned int tid, int pg_idx, bool reply)
 {
-	struct sk_buff *skb = alloc_cpl(sizeof(struct cpl_set_tcb_field), 0,
+	struct sk_buff *skb = alloc_wr(sizeof(struct cpl_set_tcb_field), 0,
 					GFP_KERNEL);
 	struct cpl_set_tcb_field *req;
 	u64 val = pg_idx < DDP_PGIDX_MAX ? pg_idx : 0;
@@ -1193,7 +1226,7 @@ static int ddp_setup_conn_pgidx(struct cxgbi_sock *csk,
 static int ddp_setup_conn_digest(struct cxgbi_sock *csk, unsigned int tid,
 			     int hcrc, int dcrc, int reply)
 {
-	struct sk_buff *skb = alloc_cpl(sizeof(struct cpl_set_tcb_field), 0,
+	struct sk_buff *skb = alloc_wr(sizeof(struct cpl_set_tcb_field), 0,
 					GFP_KERNEL);
 	struct cpl_set_tcb_field *req;
 	u64 val = (hcrc ? 1 : 0) | (dcrc ? 2 : 0);
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
index b375a68..8b6ada8 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.c
@@ -34,8 +34,8 @@ static unsigned int dbg_level;
 
 #define	DRV_MODULE_NAME		"cxgb4i"
 #define DRV_MODULE_DESC		"Chelsio T4 iSCSI Driver"
-#define	DRV_MODULE_VERSION	"0.9.0"
-#define	DRV_MODULE_RELDATE	"May 2010"
+#define	DRV_MODULE_VERSION	"0.9.1"
+#define	DRV_MODULE_RELDATE	"Aug. 2010"
 
 static char version[] =
 	DRV_MODULE_DESC " " DRV_MODULE_NAME
@@ -332,7 +332,7 @@ static u32 send_rx_credits(struct cxgbi_sock *csk, u32 credits)
 		"csk 0x%p,%u,0x%lx,%u, credit %u.\n",
 		csk, csk->state, csk->flags, csk->tid, credits);
 
-	skb = alloc_cpl(sizeof(*req), 0, GFP_ATOMIC);
+	skb = alloc_wr(sizeof(*req), 0, GFP_ATOMIC);
 	if (!skb) {
 		pr_info("csk 0x%p, credit %u, OOM.\n", csk, credits);
 		return 0;
@@ -388,7 +388,7 @@ static inline void send_tx_flowc_wr(struct cxgbi_sock *csk)
 	int flowclen, i;
 
 	flowclen = 80;
-	skb = alloc_cpl(flowclen, 0, GFP_ATOMIC);
+	skb = alloc_wr(flowclen, 0, GFP_ATOMIC);
 	flowc = (struct fw_flowc_wr *)skb->head;
 	flowc->op_to_nparams =
 		htonl(FW_WR_OP(FW_FLOWC_WR) | FW_FLOWC_WR_NPARAMS(8));
@@ -396,7 +396,7 @@ static inline void send_tx_flowc_wr(struct cxgbi_sock *csk)
 		htonl(FW_WR_LEN16(DIV_ROUND_UP(72, 16)) |
 				FW_WR_FLOWID(csk->tid));
 	flowc->mnemval[0].mnemonic = FW_FLOWC_MNEM_PFNVFN;
-	flowc->mnemval[0].val = htonl(0);
+	flowc->mnemval[0].val = htonl(csk->cdev->pfvf);
 	flowc->mnemval[1].mnemonic = FW_FLOWC_MNEM_CH;
 	flowc->mnemval[1].val = htonl(csk->tx_chan);
 	flowc->mnemval[2].mnemonic = FW_FLOWC_MNEM_PORT;
@@ -568,6 +568,12 @@ static void do_act_establish(struct cxgbi_device *cdev, struct sk_buff *skb)
 		goto rel_skb;
 	}
 
+	if (csk->atid != atid) {
+		pr_err("bad conn atid %u, csk 0x%p,%u,0x%lx,tid %u, atid %u.\n",
+			atid, csk, csk->state, csk->flags, csk->tid, csk->atid);
+		goto rel_skb;
+	}
+
 	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
 		"csk 0x%p,%u,0x%lx, tid %u, atid %u, rseq %u.\n",
 		csk, csk->state, csk->flags, tid, atid, rcv_isn);
@@ -651,7 +657,7 @@ static void csk_act_open_retry_timer(unsigned long data)
 
 	cxgbi_sock_get(csk);
 	spin_lock_bh(&csk->lock);
-	skb = alloc_cpl(sizeof(struct cpl_act_open_req), 0, GFP_ATOMIC);
+	skb = alloc_wr(sizeof(struct cpl_act_open_req), 0, GFP_ATOMIC);
 	if (!skb)
 		cxgbi_sock_fail_act_open(csk, -ENOMEM);
 	else {
@@ -681,9 +687,10 @@ static void do_act_open_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
 		goto rel_skb;
 	}
 
-	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
-		"csk 0x%p,%u,0x%lx, status %u, atid %u, tid %u.\n",
-			csk, csk->state, csk->flags, status, atid, tid);
+	pr_info("%pI4:%u-%pI4:%u, atid %u,%u, status %u, csk 0x%p,%u,0x%lx.\n",
+		&csk->saddr.sin_addr.s_addr, ntohs(csk->saddr.sin_port),
+		&csk->daddr.sin_addr.s_addr, ntohs(csk->daddr.sin_port),
+		atid, tid, status, csk, csk->state, csk->flags);
 
 	if (status && status != CPL_ERR_TCAM_FULL &&
 	    status != CPL_ERR_CONN_EXIST &&
@@ -1073,18 +1080,18 @@ static void do_set_tcb_rpl(struct cxgbi_device *cdev, struct sk_buff *skb)
 
 static int alloc_cpls(struct cxgbi_sock *csk)
 {
-	csk->cpl_close = alloc_cpl(sizeof(struct cpl_close_con_req),
-					0, GFP_NOIO);
+	csk->cpl_close = alloc_wr(sizeof(struct cpl_close_con_req),
+					0, GFP_KERNEL);
 	if (!csk->cpl_close)
 		return -ENOMEM;
 
-	csk->cpl_abort_req = alloc_cpl(sizeof(struct cpl_abort_req),
-					0, GFP_NOIO);
+	csk->cpl_abort_req = alloc_wr(sizeof(struct cpl_abort_req),
+					0, GFP_KERNEL);
 	if (!csk->cpl_abort_req)
 		goto free_cpls;
 
-	csk->cpl_abort_rpl = alloc_cpl(sizeof(struct cpl_abort_rpl),
-					0, GFP_NOIO);
+	csk->cpl_abort_rpl = alloc_wr(sizeof(struct cpl_abort_rpl),
+					0, GFP_KERNEL);
 	if (!csk->cpl_abort_rpl)
 		goto free_cpls;
 	return 0;
@@ -1158,7 +1165,7 @@ static int init_act_open(struct cxgbi_sock *csk)
 	}
 	cxgbi_sock_get(csk);
 
-	skb = alloc_cpl(sizeof(struct cpl_act_open_req), 0, GFP_NOIO);
+	skb = alloc_wr(sizeof(struct cpl_act_open_req), 0, GFP_KERNEL);
 	if (!skb)
 		goto rel_resource;
 	skb->sk = (struct sock *)csk;
@@ -1234,41 +1241,41 @@ int cxgb4i_ofld_init(struct cxgbi_device *cdev)
 /*
  * functions to program the pagepod in h/w
  */
+#define ULPMEM_IDATA_MAX_NPPODS	4 /* 256/PPOD_SIZE */
 static inline void ulp_mem_io_set_hdr(struct ulp_mem_io *req,
-				unsigned int dlen, unsigned int pm_addr)
+				unsigned int wr_len, unsigned int dlen,
+				unsigned int pm_addr)
 {
-	struct ulptx_sgl *sgl;
-	unsigned int wr_len = roundup(sizeof(struct ulp_mem_io) +
-					sizeof(struct ulptx_sgl), 16);
+	struct ulptx_idata *idata = (struct ulptx_idata *)(req + 1);
 
 	INIT_ULPTX_WR(req, wr_len, 0, 0);
-	req->cmd = htonl(ULPTX_CMD(ULP_TX_MEM_WRITE));
+	req->cmd = htonl(ULPTX_CMD(ULP_TX_MEM_WRITE) | (1 << 23));
 	req->dlen = htonl(ULP_MEMIO_DATA_LEN(dlen >> 5));
 	req->lock_addr = htonl(ULP_MEMIO_ADDR(pm_addr >> 5));
 	req->len16 = htonl(DIV_ROUND_UP(wr_len - sizeof(req->wr), 16));
-	sgl = (struct ulptx_sgl *)(req + 1);
-	sgl->cmd_nsge = htonl(ULPTX_CMD(ULP_TX_SC_DSGL) | ULPTX_NSGE(1));
-	sgl->len0 = htonl(dlen);
+
+	idata->cmd_more = htonl(ULPTX_CMD(ULP_TX_SC_IMM));
+	idata->len = htonl(dlen);
 }
 
-static int ddp_ppod_write_sgl(struct cxgbi_device *cdev, unsigned int port_id,
+static int ddp_ppod_write_idata(struct cxgbi_device *cdev, unsigned int port_id,
 				struct cxgbi_pagepod_hdr *hdr, unsigned int idx,
 				unsigned int npods,
 				struct cxgbi_gather_list *gl,
 				unsigned int gl_pidx)
 {
 	struct cxgbi_ddp_info *ddp = cdev->ddp;
-	unsigned int dlen, pm_addr;
 	struct sk_buff *skb;
 	struct ulp_mem_io *req;
-	struct ulptx_sgl *sgl;
+	struct ulptx_idata *idata;
 	struct cxgbi_pagepod *ppod;
+	unsigned int pm_addr = idx * PPOD_SIZE + ddp->llimit;
+	unsigned int dlen = PPOD_SIZE * npods;
+	unsigned int wr_len = roundup(sizeof(struct ulp_mem_io) +
+				sizeof(struct ulptx_idata) + dlen, 16);
 	unsigned int i;
 
-	dlen = PPOD_SIZE * npods;
-	pm_addr = idx * PPOD_SIZE + ddp->llimit;
-
-	skb = alloc_cpl(sizeof(*req) + sizeof(*sgl), dlen, GFP_ATOMIC);
+	skb = alloc_wr(wr_len, 0, GFP_ATOMIC);
 	if (!skb) {
 		pr_err("cdev 0x%p, idx %u, npods %u, OOM.\n",
 			cdev, idx, npods);
@@ -1277,10 +1284,9 @@ static int ddp_ppod_write_sgl(struct cxgbi_device *cdev, unsigned int port_id,
 	req = (struct ulp_mem_io *)skb->head;
 	set_queue(skb, CPL_PRIORITY_CONTROL, NULL);
 
-	ulp_mem_io_set_hdr(req, dlen, pm_addr);
-	sgl = (struct ulptx_sgl *)(req + 1);
-	ppod = (struct cxgbi_pagepod *)(sgl + 1);
-	sgl->addr0 = cpu_to_be64(virt_to_phys(ppod));
+	ulp_mem_io_set_hdr(req, wr_len, dlen, pm_addr);
+	idata = (struct ulptx_idata *)(req + 1);
+	ppod = (struct cxgbi_pagepod *)(idata + 1);
 
 	for (i = 0; i < npods; i++, ppod++, gl_pidx += PPOD_PAGES_MAX) {
 		if (!hdr && !gl)
@@ -1302,9 +1308,9 @@ static int ddp_set_map(struct cxgbi_sock *csk, struct cxgbi_pagepod_hdr *hdr,
 
 	for (i = 0; i < npods; i += cnt, idx += cnt) {
 		cnt = npods - i;
-		if (cnt > ULPMEM_DSGL_MAX_NPPODS)
-			cnt = ULPMEM_DSGL_MAX_NPPODS;
-		err = ddp_ppod_write_sgl(csk->cdev, csk->port_id, hdr,
+		if (cnt > ULPMEM_IDATA_MAX_NPPODS)
+			cnt = ULPMEM_IDATA_MAX_NPPODS;
+		err = ddp_ppod_write_idata(csk->cdev, csk->port_id, hdr,
 					idx, cnt, gl, 4 * i);
 		if (err < 0)
 			break;
@@ -1320,9 +1326,9 @@ static void ddp_clear_map(struct cxgbi_hba *chba, unsigned int tag,
 
 	for (i = 0; i < npods; i += cnt, idx += cnt) {
 		cnt = npods - i;
-		if (cnt > ULPMEM_DSGL_MAX_NPPODS)
-			cnt = ULPMEM_DSGL_MAX_NPPODS;
-		err = ddp_ppod_write_sgl(chba->cdev, chba->port_id, NULL,
+		if (cnt > ULPMEM_IDATA_MAX_NPPODS)
+			cnt = ULPMEM_IDATA_MAX_NPPODS;
+		err = ddp_ppod_write_idata(chba->cdev, chba->port_id, NULL,
 					idx, cnt, NULL, 0);
 		if (err < 0)
 			break;
@@ -1334,26 +1340,22 @@ static int ddp_setup_conn_pgidx(struct cxgbi_sock *csk, unsigned int tid,
 {
 	struct sk_buff *skb;
 	struct cpl_set_tcb_field *req;
-	u64 val = pg_idx < DDP_PGIDX_MAX ? pg_idx : 0;
 
-	if (!pg_idx)
+	if (!pg_idx || pg_idx >= DDP_PGIDX_MAX)
 		return 0;
 
-	skb = alloc_cpl(sizeof(*req), 0, GFP_KERNEL);
+	skb = alloc_wr(sizeof(*req), 0, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
 
-	/*  set up ulp submode and page size */
-	val = (val & 0x03) << 2;
-	val |= TCB_ULP_TYPE(ULP2_MODE_ISCSI);
-
+	/*  set up ulp page size */
 	req = (struct cpl_set_tcb_field *)skb->head;
 	INIT_TP_WR(req, csk->tid);
 	OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_SET_TCB_FIELD, csk->tid));
 	req->reply_ctrl = htons(NO_REPLY(reply) | QUEUENO(csk->rss_qid));
-	req->word_cookie = htons(TCB_WORD(W_TCB_ULP_RAW));
-	req->mask = cpu_to_be64(TCB_ULP_TYPE(TCB_ULP_TYPE_MASK));
-	req->val = cpu_to_be64(val);
+	req->word_cookie = htons(0);
+	req->mask = cpu_to_be64(0x3 << 8);
+	req->val = cpu_to_be64(pg_idx << 8);
 	set_wr_txq(skb, CPL_PRIORITY_CONTROL, csk->port_id);
 
 	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
@@ -1368,25 +1370,25 @@ static int ddp_setup_conn_digest(struct cxgbi_sock *csk, unsigned int tid,
 {
 	struct sk_buff *skb;
 	struct cpl_set_tcb_field *req;
-	u64 val = (hcrc ? ULP_CRC_HEADER : 0) | (dcrc ? ULP_CRC_DATA : 0);
 
-	val = TCB_ULP_RAW(val);
-	val |= TCB_ULP_TYPE(ULP2_MODE_ISCSI);
+	if (!hcrc && !dcrc)
+		return 0;
 
-	skb = alloc_cpl(sizeof(*req), 0, GFP_KERNEL);
+	skb = alloc_wr(sizeof(*req), 0, GFP_KERNEL);
 	if (!skb)
 		return -ENOMEM;
 
 	csk->hcrc_len = (hcrc ? 4 : 0);
 	csk->dcrc_len = (dcrc ? 4 : 0);
-	/*  set up ulp submode and page size */
+	/*  set up ulp submode */
 	req = (struct cpl_set_tcb_field *)skb->head;
 	INIT_TP_WR(req, tid);
 	OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_SET_TCB_FIELD, tid));
 	req->reply_ctrl = htons(NO_REPLY(reply) | QUEUENO(csk->rss_qid));
-	req->word_cookie = htons(TCB_WORD(W_TCB_ULP_RAW));
-	req->mask = cpu_to_be64(TCB_ULP_RAW(TCB_ULP_RAW_MASK));
-	req->val = cpu_to_be64(val);
+	req->word_cookie = htons(0);
+	req->mask = cpu_to_be64(0x3 << 4);
+	req->val = cpu_to_be64(((hcrc ? ULP_CRC_HEADER : 0) |
+				(dcrc ? ULP_CRC_DATA : 0)) << 4);
 	set_wr_txq(skb, CPL_PRIORITY_CONTROL, csk->port_id);
 
 	log_debug(1 << CXGBI_DBG_TOE | 1 << CXGBI_DBG_SOCK,
@@ -1477,6 +1479,10 @@ static void *t4_uld_add(const struct cxgb4_lld_info *lldi)
 	cdev->skb_rx_extra = sizeof(struct cpl_iscsi_hdr);
 	cdev->itp = &cxgb4i_iscsi_transport;
 
+	cdev->pfvf = FW_VIID_PFN_GET(cxgb4_port_viid(lldi->ports[0])) << 8;
+	pr_info("cdev 0x%p,%s, pfvf %u.\n",
+		cdev, lldi->ports[0]->name, cdev->pfvf);
+
 	rc = cxgb4i_ddp_init(cdev);
 	if (rc) {
 		pr_info("t4 0x%p ddp init failed.\n", cdev);
@@ -1516,7 +1522,7 @@ static int t4_uld_rx_handler(void *handle, const __be64 *rsp,
 	if (pgl == NULL) {
 		unsigned int len = 64 - sizeof(struct rsp_ctrl) - 8;
 
-		skb = alloc_cpl(len, 0, GFP_ATOMIC);
+		skb = alloc_wr(len, 0, GFP_ATOMIC);
 		if (!skb)
 			goto nomem;
 		skb_copy_to_linear_data(skb, &rsp[1], len);
diff --git a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.h b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.h
index 342263b..1096026 100644
--- a/drivers/scsi/cxgbi/cxgb4i/cxgb4i.h
+++ b/drivers/scsi/cxgbi/cxgb4i/cxgb4i.h
@@ -23,6 +23,11 @@
 #define CXGB4I_TX_HEADER_LEN \
 	(sizeof(struct fw_ofld_tx_data_wr) + sizeof(struct sge_opaque_hdr))
 
+struct ulptx_idata {
+	__be32 cmd_more;
+	__be32 len;
+};
+
 struct cpl_rx_data_ddp {
 	union opcode_tid ot;
 	__be16 urg;
diff --git a/drivers/scsi/cxgbi/libcxgbi.c b/drivers/scsi/cxgbi/libcxgbi.c
index db9d08a..b29e77d 100644
--- a/drivers/scsi/cxgbi/libcxgbi.c
+++ b/drivers/scsi/cxgbi/libcxgbi.c
@@ -195,16 +195,22 @@ EXPORT_SYMBOL_GPL(cxgbi_device_find_by_lldev);
 static struct cxgbi_device *cxgbi_device_find_by_netdev(struct net_device *ndev,
 							int *port)
 {
+	struct net_device *vdev = NULL;
 	struct cxgbi_device *cdev, *tmp;
 	int i;
 
-	if (ndev->priv_flags & IFF_802_1Q_VLAN)
+	if (ndev->priv_flags & IFF_802_1Q_VLAN) {
+		vdev = ndev;
 		ndev = vlan_dev_real_dev(ndev);
+		log_debug(1 << CXGBI_DBG_DEV,
+			"vlan dev %s -> %s.\n", vdev->name, ndev->name);
+	}
 
 	mutex_lock(&cdev_mutex);
 	list_for_each_entry_safe(cdev, tmp, &cdev_list, list_head) {
 		for (i = 0; i < cdev->nports; i++) {
 			if (ndev == cdev->ports[i]) {
+				cdev->hbas[i]->vdev = vdev;
 				mutex_unlock(&cdev_mutex);
 				if (port)
 					*port = i;
@@ -218,24 +224,6 @@ static struct cxgbi_device *cxgbi_device_find_by_netdev(struct net_device *ndev,
 	return NULL;
 }
 
-struct cxgbi_hba *cxgbi_hba_find_by_netdev(struct net_device *dev,
-					struct cxgbi_device *cdev)
-{
-	int i;
-
-	if (dev->priv_flags & IFF_802_1Q_VLAN)
-		dev = vlan_dev_real_dev(dev);
-
-	for (i = 0; i < cdev->nports; i++) {
-		if (cdev->hbas[i]->ndev == dev)
-			return cdev->hbas[i];
-	}
-	log_debug(1 << CXGBI_DBG_DEV,
-		"ndev 0x%p, %s, cdev 0x%p, NO match found.\n",
-		dev, dev->name, cdev);
-	return NULL;
-}
-
 void cxgbi_hbas_remove(struct cxgbi_device *cdev)
 {
 	int i;
@@ -532,12 +520,6 @@ static struct cxgbi_sock *cxgbi_check_route(struct sockaddr *dst_addr)
 			dst->neighbour->dev->name, ndev->name, mtu);
 	}
 
-	if (ndev->priv_flags & IFF_802_1Q_VLAN) {
-		ndev = vlan_dev_real_dev(ndev);
-		pr_info("rt dev %s, vlan -> %s.\n",
-			dst->neighbour->dev->name, ndev->name);
-	}
-
 	cdev = cxgbi_device_find_by_netdev(ndev, &port);
 	if (!cdev) {
 		pr_info("dst %pI4, %s, NOT cxgbi device.\n",
@@ -561,10 +543,7 @@ static struct cxgbi_sock *cxgbi_check_route(struct sockaddr *dst_addr)
 	csk->dst = dst;
 	csk->daddr.sin_addr.s_addr = daddr->sin_addr.s_addr;
 	csk->daddr.sin_port = daddr->sin_port;
-	if (cdev->hbas[port]->ipv4addr)
-		csk->saddr.sin_addr.s_addr = cdev->hbas[port]->ipv4addr;
-	else
-		csk->saddr.sin_addr.s_addr = rt->rt_src;
+	csk->saddr.sin_addr.s_addr = rt->rt_src;
 
 	return csk;
 
@@ -593,11 +572,11 @@ static void cxgbi_inform_iscsi_conn_closing(struct cxgbi_sock *csk)
 		csk, csk->state, csk->flags, csk->user_data);
 
 	if (csk->state != CTP_ESTABLISHED) {
-		read_lock(&csk->callback_lock);
+		read_lock_bh(&csk->callback_lock);
 		if (csk->user_data)
 			iscsi_conn_failure(csk->user_data,
 					ISCSI_ERR_CONN_FAILED);
-		read_unlock(&csk->callback_lock);
+		read_unlock_bh(&csk->callback_lock);
 	}
 }
 
@@ -1712,12 +1691,10 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 			"csk 0x%p, conn 0x%p, id %d, suspend_rx %lu!\n",
 			csk, conn, conn ? conn->id : 0xFF,
 			conn ? conn->suspend_rx : 0xFF);
-		read_unlock(&csk->callback_lock);
 		return;
 	}
 
 	while (!err) {
-		read_lock(&csk->callback_lock);
 		skb = skb_peek(&csk->receive_queue);
 		if (!skb ||
 		    !(cxgbi_skcb_test_flag(skb, SKCBF_RX_STATUS))) {
@@ -1725,11 +1702,9 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 				log_debug(1 << CXGBI_DBG_PDU_RX,
 					"skb 0x%p, NOT ready 0x%lx.\n",
 					skb, cxgbi_skcb_flags(skb));
-			read_unlock(&csk->callback_lock);
 			break;
 		}
 		__skb_unlink(skb, &csk->receive_queue);
-		read_unlock(&csk->callback_lock);
 
 		read += cxgbi_skcb_rx_pdulen(skb);
 		log_debug(1 << CXGBI_DBG_PDU_RX,
@@ -1739,18 +1714,37 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 
 		if (cxgbi_skcb_test_flag(skb, SKCBF_RX_COALESCED)) {
 			err = skb_read_pdu_bhs(conn, skb);
-			if (err < 0)
+			if (err < 0) {
+				pr_err("coalesced bhs, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
 				break;
+			}
 			err = skb_read_pdu_data(conn, skb, skb,
 						err + cdev->skb_rx_extra);
+			if (err < 0) {
+				pr_err("coalesced data, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
+				break;
+			}
 		} else {
 			err = skb_read_pdu_bhs(conn, skb);
-			if (err < 0)
+			if (err < 0) {
+				pr_err("bhs, csk 0x%p, skb 0x%p,%u, "
+					"f 0x%lx, plen %u.\n",
+					csk, skb, skb->len,
+					cxgbi_skcb_flags(skb),
+					cxgbi_skcb_rx_pdulen(skb));
 				break;
+			}
 			if (cxgbi_skcb_test_flag(skb, SKCBF_RX_DATA)) {
 				struct sk_buff *dskb;
 
-				read_lock(&csk->callback_lock);
 				dskb = skb_peek(&csk->receive_queue);
 				if (!dskb) {
 					read_unlock(&csk->callback_lock);
@@ -1759,9 +1753,16 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 					break;
 				}
 				__skb_unlink(dskb, &csk->receive_queue);
-				read_unlock(&csk->callback_lock);
 
 				err = skb_read_pdu_data(conn, skb, dskb, 0);
+				if (err < 0)
+					pr_err("data, csk 0x%p, skb 0x%p,%u, "
+						"f 0x%lx, plen %u, dskb 0x%p,"
+						"%u.\n",
+						csk, skb, skb->len,
+						cxgbi_skcb_flags(dskb),
+						cxgbi_skcb_rx_pdulen(skb),
+						dskb, dskb->len);
 				__kfree_skb(dskb);
 			} else
 				err = skb_read_pdu_data(conn, skb, skb, 0);
@@ -1780,7 +1781,8 @@ void cxgbi_conn_pdu_ready(struct cxgbi_sock *csk)
 	}
 
 	if (err < 0) {
-		pr_info("csk 0x%p, 0x%p, rx failed %d.\n", csk, conn, err);
+		pr_info("csk 0x%p, 0x%p, rx failed %d, read %u.\n",
+			csk, conn, err, read);
 		iscsi_conn_failure(conn, ISCSI_ERR_CONN_FAILED);
 	}
 }
@@ -1861,7 +1863,7 @@ int cxgbi_conn_alloc_pdu(struct iscsi_task *task, u8 opcode)
 	struct cxgbi_device *cdev = cconn->chba->cdev;
 	struct iscsi_conn *conn = task->conn;
 	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = task->dd_data + sizeof(*tcp_task);
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct scsi_cmnd *sc = task->sc;
 	int headroom = SKB_TX_ISCSI_PDU_HEADER_MAX;
 
@@ -1916,8 +1918,7 @@ int cxgbi_conn_init_pdu(struct iscsi_task *task, unsigned int offset,
 			      unsigned int count)
 {
 	struct iscsi_conn *conn = task->conn;
-	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = tcp_task->dd_data;
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct sk_buff *skb = tdata->skb;
 	unsigned int datalen = count;
 	int i, padlen = iscsi_padding(count);
@@ -2019,8 +2020,7 @@ int cxgbi_conn_xmit_pdu(struct iscsi_task *task)
 {
 	struct iscsi_tcp_conn *tcp_conn = task->conn->dd_data;
 	struct cxgbi_conn *cconn = tcp_conn->dd_data;
-	struct iscsi_tcp_task *tcp_task = task->dd_data;
-	struct cxgbi_task_data *tdata = tcp_task->dd_data;
+	struct cxgbi_task_data *tdata = iscsi_task_cxgbi_data(task);
 	struct sk_buff *skb = tdata->skb;
 	unsigned int datalen;
 	int err;
@@ -2290,12 +2290,12 @@ int cxgbi_bind_conn(struct iscsi_cls_session *cls_session,
 	/*  calculate the tag idx bits needed for this conn based on cmds_max */
 	cconn->task_idx_bits = (__ilog2_u32(conn->session->cmds_max - 1)) + 1;
 
-	write_lock(&csk->callback_lock);
+	write_lock_bh(&csk->callback_lock);
 	csk->user_data = conn;
 	cconn->chba = cep->chba;
 	cconn->cep = cep;
 	cep->cconn = cconn;
-	write_unlock(&csk->callback_lock);
+	write_unlock_bh(&csk->callback_lock);
 
 	cxgbi_conn_max_xmit_dlength(conn);
 	cxgbi_conn_max_recv_dlength(conn);
diff --git a/drivers/scsi/cxgbi/libcxgbi.h b/drivers/scsi/cxgbi/libcxgbi.h
index 40551f3..c57d59d 100644
--- a/drivers/scsi/cxgbi/libcxgbi.h
+++ b/drivers/scsi/cxgbi/libcxgbi.h
@@ -162,16 +162,6 @@ struct cxgbi_ddp_info {
 #define PPOD_VALID(x)		((x) << PPOD_VALID_SHIFT)
 #define PPOD_VALID_FLAG		PPOD_VALID(1U)
 
-#define W_TCB_ULP_TYPE          0
-#define TCB_ULP_TYPE_SHIFT      0
-#define TCB_ULP_TYPE_MASK       0xfULL
-#define TCB_ULP_TYPE(x)         ((x) << TCB_ULP_TYPE_SHIFT)
-
-#define W_TCB_ULP_RAW           0
-#define TCB_ULP_RAW_SHIFT       4
-#define TCB_ULP_RAW_MASK        0xffULL
-#define TCB_ULP_RAW(x)          ((x) << TCB_ULP_RAW_SHIFT)
-
 /*
  * sge_opaque_hdr -
  * Opaque version of structure the SGE stores at skb->head of TX_DATA packets
@@ -410,16 +400,15 @@ static inline unsigned int cxgbi_sock_compute_wscale(unsigned int win)
 	return wscale;
 }
 
-static inline struct sk_buff *alloc_cpl(int cpl_len, int dlen, gfp_t gfp)
+static inline struct sk_buff *alloc_wr(int wrlen, int dlen, gfp_t gfp)
 {
-	int wrlen = roundup(cpl_len, 16);
 	struct sk_buff *skb = alloc_skb(wrlen + dlen, gfp);
 
 	if (skb) {
 		__skb_put(skb, wrlen);
 		memset(skb->head, 0, wrlen + dlen);
 	} else
-		pr_info("alloc cpl skb %u+%u, OOM.\n", cpl_len, dlen);
+		pr_info("alloc cpl wr skb %u+%u, OOM.\n", wrlen, dlen);
 	return skb;
 }
 
@@ -501,6 +490,7 @@ void cxgbi_sock_free_cpl_skbs(struct cxgbi_sock *);
 
 struct cxgbi_hba {
 	struct net_device *ndev;
+	struct net_device *vdev;	/* vlan dev */
 	struct Scsi_Host *shost;
 	struct cxgbi_device *cdev;
 	__be32 ipv4addr;
@@ -593,6 +583,8 @@ struct cxgbi_task_data {
 	unsigned int count;
 	unsigned int sgoffset;
 };
+#define iscsi_task_cxgbi_data(task) \
+	((task)->dd_data + sizeof(struct iscsi_tcp_task))
 
 static inline int cxgbi_is_ddp_tag(struct cxgbi_tag_format *tformat, u32 tag)
 {

-- 
You received this message because you are subscribed to the Google Groups "open-iscsi" group.
To post to this group, send email to open-iscsi-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
To unsubscribe from this group, send email to open-iscsi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit this group at http://groups.google.com/group/open-iscsi?hl=en.

^ permalink raw reply related

* kvm networking todo wiki
From: Michael S. Tsirkin @ 2010-09-21 16:11 UTC (permalink / raw)
  To: David Stevens, sri, Anthony Liguori, Rusty Russell,
	Krishna Kumar2, Sh
  Cc: kvm, qemu-devel, netdev

I've put up a wiki page with a kvm networking todo list,
mainly to avoid effort duplication, but also in the hope
to draw attention to what I think we should try addressing
in KVM:

http://www.linux-kvm.org/page/NetworkingTodo

This page could cover all networking related activity in KVM,
currently most info is related to virtio-net.

Note: if there's no developer listed for an item,
this just means I don't know of anyone actively working
on an issue at the moment, not that no one intends to.

I would appreciate it if others working on one of the items on this list
would add their names so we can communicate better.  If others like this
wiki page, please go ahead and add stuff you are working on if any.

It would be especially nice to add autotest projects:
there is just a short test matrix and a catch-all
'Cover test matrix with autotest', currently.

Currently there are some links to Red Hat bugzilla entries,
feel free to add links to other bugzillas.

Thanks!

-- 
MST

^ permalink raw reply

* Re: [PATCH v3] ip: fix truesize mismatch in ip fragmentation
From: Henrique de Moraes Holschuh @ 2010-09-21 16:26 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Nick Bowler, linux-kernel, netdev, David S. Miller,
	Jarek Poplawski, Patrick McHardy
In-Reply-To: <1285084705.2617.636.camel@edumazet-laptop>

Should this be a candidate for -stable?

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply

* Re: [PATCH v3] ip: fix truesize mismatch in ip fragmentation
From: Eric Dumazet @ 2010-09-21 16:31 UTC (permalink / raw)
  To: Henrique de Moraes Holschuh
  Cc: Nick Bowler, linux-kernel, netdev, David S. Miller,
	Jarek Poplawski, Patrick McHardy
In-Reply-To: <20100921162657.GA22183@khazad-dum.debian.net>

Le mardi 21 septembre 2010 à 13:26 -0300, Henrique de Moraes Holschuh a
écrit :
> Should this be a candidate for -stable?
> 

Yes, of course, but David wants to handle stable submissions himself.

I am not sure we want to bug stable team with dozens of mails while
polishing patches ?

Thanks



^ permalink raw reply

* RE: REGRESSION: e1000e fails to communicate in 2.6.36-rcx (82566DM)
From: Tantilov, Emil S @ 2010-09-21 16:36 UTC (permalink / raw)
  To: Priit Laes; +Cc: netdev@vger.kernel.org, e1000-devel@lists.sourceforge.net
In-Reply-To: <1285054018.1487.28.camel@chi>

Priit Laes wrote:
> Ühel kenal päeval, R, 2010-09-10 kell 11:21, kirjutas Tantilov, Emil
> S: 
>>>>> After trying out newer 2.6.35-rc3 kernel, the network interface
>>>>> fails to
>>> 
>>> Uhoh.. it was a typo ^^, real kernel version is 2.6.36-rc3+ :S
>>> 
>>>> Is this still an issue in the latest net-next pull?
>>> Yes, it is still not working (latest net-next pull was on 4th of
>>> Aug) 
>>> 
>>>> Also please provide the output of ethtool -e.
>>> 
>>> Attached output from 2.6.36-rc3
>> 
>> I think this is a known issue for this device - we have a patch
>> currently in test, and we should have it out soon.
> 
> Any news on the patch?
Unfortunatelly we ran into some issues with the patch and we are still looking into it.

> 
> 2.6.36-rc5 is already out and I really do not want to see it getting
> into major release :(

Same here. We are working on it and we'll post it ASAP.

> Cheers,
> Priit :)

Thanks,
Emil

^ permalink raw reply

* [PATCH net-next-2.6] net: constify some ppp/pptp structs
From: Eric Dumazet @ 2010-09-21 16:43 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Dmitry Kozlov

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 drivers/net/pppoe.c      |    2 +-
 drivers/net/pppox.c      |    4 ++--
 drivers/net/pptp.c       |    8 ++++----
 include/linux/if_pppox.h |    2 +-
 net/l2tp/l2tp_ppp.c      |    2 +-
 5 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/net/pppoe.c b/drivers/net/pppoe.c
index c07de35..d72fb05 100644
--- a/drivers/net/pppoe.c
+++ b/drivers/net/pppoe.c
@@ -1124,7 +1124,7 @@ static const struct proto_ops pppoe_ops = {
 	.ioctl		= pppox_ioctl,
 };
 
-static struct pppox_proto pppoe_proto = {
+static const struct pppox_proto pppoe_proto = {
 	.create	= pppoe_create,
 	.ioctl	= pppoe_ioctl,
 	.owner	= THIS_MODULE,
diff --git a/drivers/net/pppox.c b/drivers/net/pppox.c
index d4191ef..8c0d170 100644
--- a/drivers/net/pppox.c
+++ b/drivers/net/pppox.c
@@ -36,9 +36,9 @@
 
 #include <asm/uaccess.h>
 
-static struct pppox_proto *pppox_protos[PX_MAX_PROTO + 1];
+static const struct pppox_proto *pppox_protos[PX_MAX_PROTO + 1];
 
-int register_pppox_proto(int proto_num, struct pppox_proto *pp)
+int register_pppox_proto(int proto_num, const struct pppox_proto *pp)
 {
 	if (proto_num < 0 || proto_num > PX_MAX_PROTO)
 		return -EINVAL;
diff --git a/drivers/net/pptp.c b/drivers/net/pptp.c
index 761f0ec..ccbc913 100644
--- a/drivers/net/pptp.c
+++ b/drivers/net/pptp.c
@@ -53,7 +53,7 @@ static struct pppox_sock **callid_sock;
 static DEFINE_SPINLOCK(chan_lock);
 
 static struct proto pptp_sk_proto __read_mostly;
-static struct ppp_channel_ops pptp_chan_ops;
+static const struct ppp_channel_ops pptp_chan_ops;
 static const struct proto_ops pptp_ops;
 
 #define PPP_LCP_ECHOREQ 0x09
@@ -628,7 +628,7 @@ static int pptp_ppp_ioctl(struct ppp_channel *chan, unsigned int cmd,
 	return err;
 }
 
-static struct ppp_channel_ops pptp_chan_ops = {
+static const struct ppp_channel_ops pptp_chan_ops = {
 	.start_xmit = pptp_xmit,
 	.ioctl      = pptp_ppp_ioctl,
 };
@@ -659,12 +659,12 @@ static const struct proto_ops pptp_ops = {
 	.ioctl      = pppox_ioctl,
 };
 
-static struct pppox_proto pppox_pptp_proto = {
+static const struct pppox_proto pppox_pptp_proto = {
 	.create = pptp_create,
 	.owner  = THIS_MODULE,
 };
 
-static struct gre_protocol gre_pptp_protocol = {
+static const struct gre_protocol gre_pptp_protocol = {
 	.handler = pptp_rcv,
 };
 
diff --git a/include/linux/if_pppox.h b/include/linux/if_pppox.h
index 29bcd55..397921b 100644
--- a/include/linux/if_pppox.h
+++ b/include/linux/if_pppox.h
@@ -204,7 +204,7 @@ struct pppox_proto {
 	struct module	*owner;
 };
 
-extern int register_pppox_proto(int proto_num, struct pppox_proto *pp);
+extern int register_pppox_proto(int proto_num, const struct pppox_proto *pp);
 extern void unregister_pppox_proto(int proto_num);
 extern void pppox_unbind_sock(struct sock *sk);/* delete ppp-channel binding */
 extern int pppox_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg);
diff --git a/net/l2tp/l2tp_ppp.c b/net/l2tp/l2tp_ppp.c
index ff954b3..39a21d0 100644
--- a/net/l2tp/l2tp_ppp.c
+++ b/net/l2tp/l2tp_ppp.c
@@ -1768,7 +1768,7 @@ static const struct proto_ops pppol2tp_ops = {
 	.ioctl		= pppox_ioctl,
 };
 
-static struct pppox_proto pppol2tp_proto = {
+static const struct pppox_proto pppol2tp_proto = {
 	.create		= pppol2tp_create,
 	.ioctl		= pppol2tp_ioctl
 };



^ permalink raw reply related

* Re: [PATCH] mm: do not print backtraces on GFP_ATOMIC failures
From: Andrew Morton @ 2010-09-21 16:46 UTC (permalink / raw)
  To: Rik van Riel; +Cc: linux-mm, linux-kernel, KOSAKI Motohiro, netdev
In-Reply-To: <20100921121818.4745f038@annuminas.surriel.com>

On Tue, 21 Sep 2010 12:18:18 -0400 Rik van Riel <riel@redhat.com> wrote:

> Atomic allocations cannot fall back to the page eviction code
> and are expected to fail.  In fact, in some network intensive
> workloads, it is common to experience hundreds of GFP_ATOMIC
> allocation failures.
> 
> Printing out a backtrace for every one of those expected
> allocation failures accomplishes nothing good. At multi-gigabit
> network speeds with jumbo frames, a burst of allocation failure
> backtraces could even slow down the system.
> 
> We're better off not printing out backtraces on GFP_ATOMIC
> allocation failures.
> 
> Signed-off-by: Rik van Riel <riel@redhat.com>
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 975609c..5a0bddb 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -72,7 +72,7 @@ struct vm_area_struct;
>  /* This equals 0, but use constants in case they ever change */
>  #define GFP_NOWAIT	(GFP_ATOMIC & ~__GFP_HIGH)
>  /* GFP_ATOMIC means both !wait (__GFP_WAIT not set) and use emergency pool */
> -#define GFP_ATOMIC	(__GFP_HIGH)
> +#define GFP_ATOMIC	(__GFP_HIGH | __GFP_NOWARN)
>  #define GFP_NOIO	(__GFP_WAIT)
>  #define GFP_NOFS	(__GFP_WAIT | __GFP_IO)
>  #define GFP_KERNEL	(__GFP_WAIT | __GFP_IO | __GFP_FS)

A much finer-tuned implementation would be to add __GFP_NOWARN just to
the networking call sites.  I asked about this in June and it got
nixed:

http://www.spinics.net/lists/netdev/msg131965.html

--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH 1/5] ptp: Added a brand new class driver for ptp clocks.
From: Stephan Gatzka @ 2010-09-21 16:54 UTC (permalink / raw)
  To: netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <20100906063327.GA4549-7KxsofuKt4IfAd9E5cN8NEzG7cXyKsk/@public.gmane.org>


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

Hi Richard,

> 1. Character device
> 2. POSIX clock with dynamic ids
>
> Please, just take your pick ;^)

Just to reopen this nearly dead but very interesting thread: I'm happy 
with every solution that has no impact on the accuracy of the PTP clock.

If I can choose, I would prefer the POSIX clock with dynamic ids solution.

Kind regards and thanks for your effort,

Stephan Gatzka


[-- Attachment #1.2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5136 bytes --]

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

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

^ permalink raw reply

* [PATCH net-next-2.6] ipv6: addrconf.h cleanups
From: Eric Dumazet @ 2010-09-21 16:57 UTC (permalink / raw)
  To: David Miller; +Cc: netdev

- Use rcu_dereference_rtnl() in __in6_dev_get
- kerneldoc for __in6_dev_get() and in6_dev_get()
- Use inline functions instead of macros

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 include/net/addrconf.h |   63 ++++++++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 20 deletions(-)

diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 45375b4..39cf213 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -174,20 +174,32 @@ extern int ipv6_chk_acast_addr(struct net *net, struct net_device *dev,
 extern int register_inet6addr_notifier(struct notifier_block *nb);
 extern int unregister_inet6addr_notifier(struct notifier_block *nb);
 
-static inline struct inet6_dev *
-__in6_dev_get(struct net_device *dev)
+/**
+ * __in6_dev_get - get inet6_dev pointer from netdevice
+ * @dev: network device
+ *
+ * Caller must hold rcu_read_lock or RTNL, because this function
+ * does not take a reference on the inet6_dev.
+ */
+static inline struct inet6_dev *__in6_dev_get(const struct net_device *dev)
 {
-	return rcu_dereference_check(dev->ip6_ptr,
-				     rcu_read_lock_held() ||
-				     lockdep_rtnl_is_held());
+	return rcu_dereference_rtnl(dev->ip6_ptr);
 }
 
-static inline struct inet6_dev *
-in6_dev_get(struct net_device *dev)
+/**
+ * in6_dev_get - get inet6_dev pointer from netdevice
+ * @dev: network device
+ *
+ * This version can be used in any context, and takes a reference
+ * on the inet6_dev. Callers must use in6_dev_put() later to
+ * release this reference.
+ */
+static inline struct inet6_dev *in6_dev_get(const struct net_device *dev)
 {
-	struct inet6_dev *idev = NULL;
+	struct inet6_dev *idev;
+
 	rcu_read_lock();
-	idev = __in6_dev_get(dev);
+	idev = rcu_dereference(dev->ip6_ptr);
 	if (idev)
 		atomic_inc(&idev->refcnt);
 	rcu_read_unlock();
@@ -196,16 +208,21 @@ in6_dev_get(struct net_device *dev)
 
 extern void in6_dev_finish_destroy(struct inet6_dev *idev);
 
-static inline void
-in6_dev_put(struct inet6_dev *idev)
+static inline void in6_dev_put(struct inet6_dev *idev)
 {
 	if (atomic_dec_and_test(&idev->refcnt))
 		in6_dev_finish_destroy(idev);
 }
 
-#define __in6_dev_put(idev)  atomic_dec(&(idev)->refcnt)
-#define in6_dev_hold(idev)   atomic_inc(&(idev)->refcnt)
+static inline void __in6_dev_put(struct inet6_dev *idev)
+{
+	atomic_dec(&idev->refcnt);
+}
 
+static inline void in6_dev_hold(struct inet6_dev *idev)
+{
+	atomic_inc(&idev->refcnt);
+}
 
 extern void inet6_ifa_finish_destroy(struct inet6_ifaddr *ifp);
 
@@ -215,9 +232,15 @@ static inline void in6_ifa_put(struct inet6_ifaddr *ifp)
 		inet6_ifa_finish_destroy(ifp);
 }
 
-#define __in6_ifa_put(ifp)	atomic_dec(&(ifp)->refcnt)
-#define in6_ifa_hold(ifp)	atomic_inc(&(ifp)->refcnt)
+static inline void __in6_ifa_put(struct inet6_ifaddr *ifp)
+{
+	atomic_dec(&ifp->refcnt);
+}
 
+static inline void in6_ifa_hold(struct inet6_ifaddr *ifp)
+{
+	atomic_inc(&ifp->refcnt);
+}
 
 
 /*
@@ -240,23 +263,23 @@ static inline int ipv6_addr_is_multicast(const struct in6_addr *addr)
 
 static inline int ipv6_addr_is_ll_all_nodes(const struct in6_addr *addr)
 {
-	return (((addr->s6_addr32[0] ^ htonl(0xff020000)) |
+	return ((addr->s6_addr32[0] ^ htonl(0xff020000)) |
 		addr->s6_addr32[1] | addr->s6_addr32[2] |
-		(addr->s6_addr32[3] ^ htonl(0x00000001))) == 0);
+		(addr->s6_addr32[3] ^ htonl(0x00000001))) == 0;
 }
 
 static inline int ipv6_addr_is_ll_all_routers(const struct in6_addr *addr)
 {
-	return (((addr->s6_addr32[0] ^ htonl(0xff020000)) |
+	return ((addr->s6_addr32[0] ^ htonl(0xff020000)) |
 		addr->s6_addr32[1] | addr->s6_addr32[2] |
-		(addr->s6_addr32[3] ^ htonl(0x00000002))) == 0);
+		(addr->s6_addr32[3] ^ htonl(0x00000002))) == 0;
 }
 
 extern int __ipv6_isatap_ifid(u8 *eui, __be32 addr);
 
 static inline int ipv6_addr_is_isatap(const struct in6_addr *addr)
 {
-	return ((addr->s6_addr32[2] | htonl(0x02000000)) == htonl(0x02005EFE));
+	return (addr->s6_addr32[2] | htonl(0x02000000)) == htonl(0x02005EFE);
 }
 
 #ifdef CONFIG_PROC_FS



^ permalink raw reply related

* Re: [PATCH] mm: do not print backtraces on GFP_ATOMIC failures
From: Eric Dumazet @ 2010-09-21 17:00 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Rik van Riel, linux-mm, linux-kernel, KOSAKI Motohiro, netdev
In-Reply-To: <20100921094638.9910add0.akpm@linux-foundation.org>

Le mardi 21 septembre 2010 à 09:46 -0700, Andrew Morton a écrit :
> On Tue, 21 Sep 2010 12:18:18 -0400 Rik van Riel <riel@redhat.com> wrote:
> 
> > Atomic allocations cannot fall back to the page eviction code
> > and are expected to fail.  In fact, in some network intensive
> > workloads, it is common to experience hundreds of GFP_ATOMIC
> > allocation failures.
> > 
> > Printing out a backtrace for every one of those expected
> > allocation failures accomplishes nothing good. At multi-gigabit
> > network speeds with jumbo frames, a burst of allocation failure
> > backtraces could even slow down the system.
> > 
> > We're better off not printing out backtraces on GFP_ATOMIC
> > allocation failures.
> > 
> > Signed-off-by: Rik van Riel <riel@redhat.com>
> > 
> > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > index 975609c..5a0bddb 100644
> > --- a/include/linux/gfp.h
> > +++ b/include/linux/gfp.h
> > @@ -72,7 +72,7 @@ struct vm_area_struct;
> >  /* This equals 0, but use constants in case they ever change */
> >  #define GFP_NOWAIT	(GFP_ATOMIC & ~__GFP_HIGH)
> >  /* GFP_ATOMIC means both !wait (__GFP_WAIT not set) and use emergency pool */
> > -#define GFP_ATOMIC	(__GFP_HIGH)
> > +#define GFP_ATOMIC	(__GFP_HIGH | __GFP_NOWARN)
> >  #define GFP_NOIO	(__GFP_WAIT)
> >  #define GFP_NOFS	(__GFP_WAIT | __GFP_IO)
> >  #define GFP_KERNEL	(__GFP_WAIT | __GFP_IO | __GFP_FS)
> 
> A much finer-tuned implementation would be to add __GFP_NOWARN just to
> the networking call sites.  I asked about this in June and it got
> nixed:
> 
> http://www.spinics.net/lists/netdev/msg131965.html
> --

Yes, I remember this particular report was useful to find and correct a
bug.

I dont know what to say.

Being silent or verbose, it really depends on the context ?



--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply

* Re: [PATCH v3] ip: fix truesize mismatch in ip fragmentation
From: Jarek Poplawski @ 2010-09-21 17:50 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Nick Bowler, linux-kernel, netdev, David S. Miller,
	Patrick McHardy
In-Reply-To: <1285084705.2617.636.camel@edumazet-laptop>

On Tue, Sep 21, 2010 at 05:58:25PM +0200, Eric Dumazet wrote:
> Hmm, while looking at git history, I found commit from Patrick
> b2722b1c3a893e (ip_fragment: also adjust skb->truesize for packets not
> owned by a socket)
> As my patch reverts it, we probably want a more polished patch.
> 
> (Also port Patrick work to ipv6)
> 
> So here is a V3
> 
> [PATCH v3] ip: fix truesize mismatch in ip fragmentation
> 
> Special care should be taken when slow path is hit in ip_fragment() :
> 
> When walking through frags, we transfert truesize ownership from skb to
> frags. Then if we hit a slow_path condition, we must undo this or risk
> uncharging frags->truesize twice, and in the end, having negative socket
> sk_wmem_alloc counter, or even freeing socket sooner than expected.
> 
> Many thanks to Nick Bowler, who provided a very clean bug report and
> test program.
> 
> Thanks to Jarek for reviewing my first patch and providing a V2
> 
> While Nick bisection pointed to commit 2b85a34e911 (net: No more
> expensive sock_hold()/sock_put() on each tx), underlying bug is older
> (2.6.12-rc5)
> 
> A side effect is to extend work done in commit b2722b1c3a893e
> (ip_fragment: also adjust skb->truesize for packets not owned by a
> socket) to ipv6 as well.
> 
> Reported-and-bisected-by: Nick Bowler <nbowler@elliptictech.com>
> Tested-by: Nick Bowler <nbowler@elliptictech.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Jarek Poplawski <jarkao2@gmail.com>
> CC: Patrick McHardy <kaber@trash.net>
> ---
>  net/ipv4/ip_output.c  |   19 +++++++++++++------
>  net/ipv6/ip6_output.c |   18 +++++++++++++-----
>  2 files changed, 26 insertions(+), 11 deletions(-)
> 
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 04b6989..a643f7a 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -488,9 +488,8 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
>  	 * we can switch to copy when see the first bad fragment.
>  	 */
>  	if (skb_has_frags(skb)) {
> -		struct sk_buff *frag;
> +		struct sk_buff *frag, *frag2;
>  		int first_len = skb_pagelen(skb);
> -		int truesizes = 0;
>  
>  		if (first_len - hlen > mtu ||
>  		    ((first_len - hlen) & 7) ||
> @@ -503,18 +502,18 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
>  			if (frag->len > mtu ||
>  			    ((frag->len & 7) && frag->next) ||
>  			    skb_headroom(frag) < hlen)
> -			    goto slow_path;
> +			    goto slow_path_undo;

Looks better and better to me, except, checkpatch complains about the
(existing) indentation fault here (and later), but I guess you've seen
that?

Jarek P.

^ permalink raw reply

* Re: [PATCH v3] ip: fix truesize mismatch in ip fragmentation
From: Henrique de Moraes Holschuh @ 2010-09-21 18:09 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Nick Bowler, linux-kernel, netdev, David S. Miller,
	Jarek Poplawski, Patrick McHardy
In-Reply-To: <1285086679.2617.672.camel@edumazet-laptop>

On Tue, 21 Sep 2010, Eric Dumazet wrote:
> Le mardi 21 septembre 2010 à 13:26 -0300, Henrique de Moraes Holschuh a
> écrit :
> > Should this be a candidate for -stable?
> > 
> 
> Yes, of course, but David wants to handle stable submissions himself.
> 
> I am not sure we want to bug stable team with dozens of mails while
> polishing patches ?

We don't.  But one often marks commits that should go to -stable using a Cc:
pseudo-header, and also includes relevant information (e.g. to which stable
kernels it should be applied to) to the commit message.

Since there wasn't one, and I didn't readly find any post in this thread
that mentioned it should also go to stable, AND it looked at first glance
like something that should go to stable, I asked about it.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply

* pull request: wireless-2.6 2010-09-21
From: John W. Linville @ 2010-09-21 18:26 UTC (permalink / raw)
  To: davem; +Cc: linux-wireless, netdev, linux-kernel

Dave,

Here are two more small fixes intended for 2.6.36.  One is a one-liner
(kmalloc->kzalloc) that prevents a potential information leak.
The other is a small fix that skips firmware reloads (a workaround
for firmware problems) while a scan is in progress in order to avoid
the stack trace cited in the commit log.

Please let me know if there are problems!

Thanks,

John

---

The following changes since commit 3779298b81cd9a2531cec93e3beefc1acdb01382:

  Merge branch 'vhost-net' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost (2010-09-20 11:13:34 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master

Johannes Berg (1):
      wext: fix potential private ioctl memory content leak

Wey-Yi Guy (1):
      iwlwifi: do not perferm force reset while doing scan

 drivers/net/wireless/iwlwifi/iwl-core.c |    5 +++++
 net/wireless/wext-priv.c                |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
index 07dbc27..e23c406 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.c
+++ b/drivers/net/wireless/iwlwifi/iwl-core.c
@@ -2613,6 +2613,11 @@ int iwl_force_reset(struct iwl_priv *priv, int mode, bool external)
 	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
 		return -EINVAL;
 
+	if (test_bit(STATUS_SCANNING, &priv->status)) {
+		IWL_DEBUG_INFO(priv, "scan in progress.\n");
+		return -EINVAL;
+	}
+
 	if (mode >= IWL_MAX_FORCE_RESET) {
 		IWL_DEBUG_INFO(priv, "invalid reset request.\n");
 		return -EINVAL;
diff --git a/net/wireless/wext-priv.c b/net/wireless/wext-priv.c
index 3feb28e..674d426 100644
--- a/net/wireless/wext-priv.c
+++ b/net/wireless/wext-priv.c
@@ -152,7 +152,7 @@ static int ioctl_private_iw_point(struct iw_point *iwp, unsigned int cmd,
 	} else if (!iwp->pointer)
 		return -EFAULT;
 
-	extra = kmalloc(extra_size, GFP_KERNEL);
+	extra = kzalloc(extra_size, GFP_KERNEL);
 	if (!extra)
 		return -ENOMEM;
 
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply related

* Re: [PATCH v3] ip: fix truesize mismatch in ip fragmentation
From: Eric Dumazet @ 2010-09-21 18:47 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: Nick Bowler, linux-kernel, netdev, David S. Miller,
	Patrick McHardy
In-Reply-To: <20100921175014.GA2066@del.dom.local>


> Looks better and better to me, except, checkpatch complains about the
> (existing) indentation fault here (and later), but I guess you've seen
> that?
> 

Indeed, there is checkpatch somewhere ;)

Thanks !

[PATCH v4] ip: fix truesize mismatch in ip fragmentation

Special care should be taken when slow path is hit in ip_fragment() :

When walking through frags, we transfert truesize ownership from skb to
frags. Then if we hit a slow_path condition, we must undo this or risk
uncharging frags->truesize twice, and in the end, having negative socket
sk_wmem_alloc counter, or even freeing socket sooner than expected.

Many thanks to Nick Bowler, who provided a very clean bug report and
test program.

Thanks to Jarek for reviewing my first patch and providing a V2

While Nick bisection pointed to commit 2b85a34e911 (net: No more
expensive sock_hold()/sock_put() on each tx), underlying bug is older
(2.6.12-rc5)

A side effect is to extend work done in commit b2722b1c3a893e
(ip_fragment: also adjust skb->truesize for packets not owned by a
socket) to ipv6 as well.

Reported-and-bisected-by: Nick Bowler <nbowler@elliptictech.com>
Tested-by: Nick Bowler <nbowler@elliptictech.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
CC: Jarek Poplawski <jarkao2@gmail.com>
CC: Patrick McHardy <kaber@trash.net>
---
 net/ipv4/ip_output.c  |   19 +++++++++++++------
 net/ipv6/ip6_output.c |   18 +++++++++++++-----
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 04b6989..7649d77 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -488,9 +488,8 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 	 * we can switch to copy when see the first bad fragment.
 	 */
 	if (skb_has_frags(skb)) {
-		struct sk_buff *frag;
+		struct sk_buff *frag, *frag2;
 		int first_len = skb_pagelen(skb);
-		int truesizes = 0;
 
 		if (first_len - hlen > mtu ||
 		    ((first_len - hlen) & 7) ||
@@ -503,18 +502,18 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 			if (frag->len > mtu ||
 			    ((frag->len & 7) && frag->next) ||
 			    skb_headroom(frag) < hlen)
-			    goto slow_path;
+				goto slow_path_clean;
 
 			/* Partially cloned skb? */
 			if (skb_shared(frag))
-				goto slow_path;
+				goto slow_path_clean;
 
 			BUG_ON(frag->sk);
 			if (skb->sk) {
 				frag->sk = skb->sk;
 				frag->destructor = sock_wfree;
 			}
-			truesizes += frag->truesize;
+			skb->truesize -= frag->truesize;
 		}
 
 		/* Everything is OK. Generate! */
@@ -524,7 +523,6 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 		frag = skb_shinfo(skb)->frag_list;
 		skb_frag_list_init(skb);
 		skb->data_len = first_len - skb_headlen(skb);
-		skb->truesize -= truesizes;
 		skb->len = first_len;
 		iph->tot_len = htons(first_len);
 		iph->frag_off = htons(IP_MF);
@@ -576,6 +574,15 @@ int ip_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 		}
 		IP_INC_STATS(dev_net(dev), IPSTATS_MIB_FRAGFAILS);
 		return err;
+
+slow_path_clean:
+		skb_walk_frags(skb, frag2) {
+			if (frag2 == frag)
+				break;
+			frag2->sk = NULL;
+			frag2->destructor = NULL;
+			skb->truesize += frag2->truesize;
+		}
 	}
 
 slow_path:
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index d40b330..980912e 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -639,7 +639,7 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 
 	if (skb_has_frags(skb)) {
 		int first_len = skb_pagelen(skb);
-		int truesizes = 0;
+		struct sk_buff *frag2;
 
 		if (first_len - hlen > mtu ||
 		    ((first_len - hlen) & 7) ||
@@ -651,18 +651,18 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 			if (frag->len > mtu ||
 			    ((frag->len & 7) && frag->next) ||
 			    skb_headroom(frag) < hlen)
-			    goto slow_path;
+				goto slow_path_clean;
 
 			/* Partially cloned skb? */
 			if (skb_shared(frag))
-				goto slow_path;
+				goto slow_path_clean;
 
 			BUG_ON(frag->sk);
 			if (skb->sk) {
 				frag->sk = skb->sk;
 				frag->destructor = sock_wfree;
-				truesizes += frag->truesize;
 			}
+			skb->truesize -= frag->truesize;
 		}
 
 		err = 0;
@@ -693,7 +693,6 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 
 		first_len = skb_pagelen(skb);
 		skb->data_len = first_len - skb_headlen(skb);
-		skb->truesize -= truesizes;
 		skb->len = first_len;
 		ipv6_hdr(skb)->payload_len = htons(first_len -
 						   sizeof(struct ipv6hdr));
@@ -756,6 +755,15 @@ static int ip6_fragment(struct sk_buff *skb, int (*output)(struct sk_buff *))
 			      IPSTATS_MIB_FRAGFAILS);
 		dst_release(&rt->dst);
 		return err;
+
+slow_path_clean:
+		skb_walk_frags(skb, frag2) {
+			if (frag2 == frag)
+				break;
+			frag2->sk = NULL;
+			frag2->destructor = NULL;
+			skb->truesize += frag2->truesize;
+		}
 	}
 
 slow_path:

^ permalink raw reply related

* [PATCH v2] xmit_compl_seq: information to reclaim vmsplice buffers
From: Tom Herbert @ 2010-09-21 18:57 UTC (permalink / raw)
  To: netdev, davem; +Cc: sridharr

In this patch we propose to adds some socket API to retrieve the
 "transmit completion sequence number", essentially a byte counter
for the number of bytes that have been transmitted and will not be
retransmitted.  In the case of TCP, this should correspond to snd_una.

The purpose of this API is to provide information to userspace about
which buffers can be reclaimed when sending with vmsplice() on a
socket.

There are two methods for retrieving the completed sequence number:
through a simple getsockopt (implemented here for TCP), as well as
returning the value in the ancilary data of a recvmsg.

The expected flow would be something like:
   - Connect is created
   - Initial completion seq # is retrieved through the sockopt, and is
     stored in userspace "compl_seq" variable for the connection.
   - Whenever a send is done, compl_seq += # bytes sent.
   - When doing a vmsplice the completion sequence number is saved
     for each user space buffer, buffer_compl_seq = compl_seq.
   - When recvmsg returns with a completion sequence number in
     ancillary data, any buffers cover by that sequence number
     (where buffer_compl_seq < recvmsg_compl_seq) are reclaimed
     and can be written to again.
   - If no data is receieved on a connection (recvmsg does not
     return), a timeout can be used to call the getsockopt and
     reclaim buffers as a fallback.

Using recvmsg data in this manner is sort of a cheap way to get a
"callback" for when a vmspliced buffer is consumed.  It will work
well for a client where the response causes recvmsg to return.
On the server side it works well if there are a sufficient
number of requests coming on the connection (resorting to the
timeout if necessary as described above).

Signed-off-by: Tom Herbert <therbert@google.com>
---
diff --git a/arch/alpha/include/asm/socket.h b/arch/alpha/include/asm/socket.h
index 06edfef..3587082 100644
--- a/arch/alpha/include/asm/socket.h
+++ b/arch/alpha/include/asm/socket.h
@@ -69,6 +69,9 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
+
 /* O_NONBLOCK clashes with the bits used for socket types.  Therefore we
  * have to define SOCK_NONBLOCK to a different value here.
  */
diff --git a/arch/arm/include/asm/socket.h b/arch/arm/include/asm/socket.h
index 90ffd04..962c365 100644
--- a/arch/arm/include/asm/socket.h
+++ b/arch/arm/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/avr32/include/asm/socket.h b/arch/avr32/include/asm/socket.h
index c8d1fae..de59dfe 100644
--- a/arch/avr32/include/asm/socket.h
+++ b/arch/avr32/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* __ASM_AVR32_SOCKET_H */
diff --git a/arch/cris/include/asm/socket.h b/arch/cris/include/asm/socket.h
index 1a4a619..fe9a1ba 100644
--- a/arch/cris/include/asm/socket.h
+++ b/arch/cris/include/asm/socket.h
@@ -64,6 +64,8 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
 
 
diff --git a/arch/frv/include/asm/socket.h b/arch/frv/include/asm/socket.h
index a6b2688..769f1f5 100644
--- a/arch/frv/include/asm/socket.h
+++ b/arch/frv/include/asm/socket.h
@@ -62,5 +62,7 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
 
diff --git a/arch/h8300/include/asm/socket.h b/arch/h8300/include/asm/socket.h
index 04c0f45..5ef4551 100644
--- a/arch/h8300/include/asm/socket.h
+++ b/arch/h8300/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/ia64/include/asm/socket.h b/arch/ia64/include/asm/socket.h
index 51427ea..217e4d8 100644
--- a/arch/ia64/include/asm/socket.h
+++ b/arch/ia64/include/asm/socket.h
@@ -71,4 +71,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_IA64_SOCKET_H */
diff --git a/arch/m32r/include/asm/socket.h b/arch/m32r/include/asm/socket.h
index 469787c..e0830c6 100644
--- a/arch/m32r/include/asm/socket.h
+++ b/arch/m32r/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_M32R_SOCKET_H */
diff --git a/arch/m68k/include/asm/socket.h b/arch/m68k/include/asm/socket.h
index 9bf49c8..438cb8e 100644
--- a/arch/m68k/include/asm/socket.h
+++ b/arch/m68k/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/mips/include/asm/socket.h b/arch/mips/include/asm/socket.h
index 9de5190..cb927d7 100644
--- a/arch/mips/include/asm/socket.h
+++ b/arch/mips/include/asm/socket.h
@@ -82,6 +82,9 @@ To add: #define SO_REUSEPORT 0x0200	/* Allow local address and port reuse.  */
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
+
 #ifdef __KERNEL__
 
 /** sock_type - Socket types
diff --git a/arch/mn10300/include/asm/socket.h b/arch/mn10300/include/asm/socket.h
index 4e60c42..1c09487 100644
--- a/arch/mn10300/include/asm/socket.h
+++ b/arch/mn10300/include/asm/socket.h
@@ -62,4 +62,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/parisc/include/asm/socket.h b/arch/parisc/include/asm/socket.h
index 225b7d6..c1b0479 100644
--- a/arch/parisc/include/asm/socket.h
+++ b/arch/parisc/include/asm/socket.h
@@ -61,6 +61,9 @@
 
 #define SO_RXQ_OVFL             0x4021
 
+#define SO_XMIT_COMPL_SEQ	0x4022
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
+
 /* O_NONBLOCK clashes with the bits used for socket types.  Therefore we
  * have to define SOCK_NONBLOCK to a different value here.
  */
diff --git a/arch/powerpc/include/asm/socket.h b/arch/powerpc/include/asm/socket.h
index 866f760..9452547 100644
--- a/arch/powerpc/include/asm/socket.h
+++ b/arch/powerpc/include/asm/socket.h
@@ -69,4 +69,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif	/* _ASM_POWERPC_SOCKET_H */
diff --git a/arch/s390/include/asm/socket.h b/arch/s390/include/asm/socket.h
index fdff1e9..3e3c17b 100644
--- a/arch/s390/include/asm/socket.h
+++ b/arch/s390/include/asm/socket.h
@@ -70,4 +70,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* _ASM_SOCKET_H */
diff --git a/arch/sparc/include/asm/socket.h b/arch/sparc/include/asm/socket.h
index 9d3fefc..0675f54 100644
--- a/arch/sparc/include/asm/socket.h
+++ b/arch/sparc/include/asm/socket.h
@@ -58,6 +58,9 @@
 
 #define SO_RXQ_OVFL             0x0024
 
+#define SO_XMIT_COMPL_SEQ	0x0025
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
+
 /* Security levels - as per NRL IPv6 - don't actually do anything */
 #define SO_SECURITY_AUTHENTICATION		0x5001
 #define SO_SECURITY_ENCRYPTION_TRANSPORT	0x5002
diff --git a/arch/xtensa/include/asm/socket.h b/arch/xtensa/include/asm/socket.h
index cbdf2ff..3c5afbf 100644
--- a/arch/xtensa/include/asm/socket.h
+++ b/arch/xtensa/include/asm/socket.h
@@ -73,4 +73,6 @@
 
 #define SO_RXQ_OVFL             40
 
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif	/* _XTENSA_SOCKET_H */
diff --git a/include/asm-generic/socket.h b/include/asm-generic/socket.h
index 9a6115e..6dc1ed8 100644
--- a/include/asm-generic/socket.h
+++ b/include/asm-generic/socket.h
@@ -64,4 +64,7 @@
 #define SO_DOMAIN		39
 
 #define SO_RXQ_OVFL             40
+
+#define SO_XMIT_COMPL_SEQ	41
+#define SCM_XMIT_COMPL_SEQ	SO_XMIT_COMPL_SEQ
 #endif /* __ASM_GENERIC_SOCKET_H */
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index e64f4c6..f044aff 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -106,6 +106,7 @@ enum {
 #define TCP_THIN_LINEAR_TIMEOUTS 16      /* Use linear timeouts for thin streams*/
 #define TCP_THIN_DUPACK         17      /* Fast retrans. after 1 dupack */
 #define TCP_USER_TIMEOUT	18	/* How long for loss retry before timeout */
+#define TCP_XMIT_COMPL_SEQ	19	/* Return current snd_una */
 
 /* for TCP_INFO socket option */
 #define TCPI_OPT_TIMESTAMPS	1
diff --git a/include/net/sock.h b/include/net/sock.h
index 8ae97c4..e820e2b 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -543,6 +543,7 @@ enum sock_flags {
 	SOCK_TIMESTAMPING_SYS_HARDWARE, /* %SOF_TIMESTAMPING_SYS_HARDWARE */
 	SOCK_FASYNC, /* fasync() active */
 	SOCK_RXQ_OVFL,
+	SOCK_XMIT_COMPL_SEQ, /* SO_XMIT_COMPL_SEQ setting */
 };
 
 static inline void sock_copy_flags(struct sock *nsk, struct sock *osk)
diff --git a/net/core/sock.c b/net/core/sock.c
index f3a06c4..7a10215 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -740,6 +740,12 @@ set_rcvbuf:
 		else
 			sock_reset_flag(sk, SOCK_RXQ_OVFL);
 		break;
+	case SO_XMIT_COMPL_SEQ:
+		if (valbool)
+			sock_set_flag(sk, SOCK_XMIT_COMPL_SEQ);
+		else
+			sock_reset_flag(sk, SOCK_XMIT_COMPL_SEQ);
+		break;
 	default:
 		ret = -ENOPROTOOPT;
 		break;
@@ -961,6 +967,10 @@ int sock_getsockopt(struct socket *sock, int level, int optname,
 		v.val = !!sock_flag(sk, SOCK_RXQ_OVFL);
 		break;
 
+	case SO_XMIT_COMPL_SEQ:
+		v.val = !!sock_flag(sk, SOCK_XMIT_COMPL_SEQ);
+		break;
+
 	default:
 		return -ENOPROTOOPT;
 	}
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 3e8a4db..5e30381 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -1387,6 +1387,21 @@ int tcp_read_sock(struct sock *sk, read_descriptor_t *desc,
 EXPORT_SYMBOL(tcp_read_sock);
 
 /*
+ * Copy the first unacked seq into the receive msg control part.
+ */
+static inline void tcp_sock_xmit_compl_seq(struct msghdr *msg,
+					   struct sock *sk)
+{
+	if (sock_flag(sk, SOCK_XMIT_COMPL_SEQ)) {
+		struct tcp_sock *tp = tcp_sk(sk);
+		if (msg->msg_controllen >= sizeof(tp->snd_una)) {
+			put_cmsg(msg, SOL_SOCKET, SCM_XMIT_COMPL_SEQ,
+			    sizeof(tp->snd_una), &tp->snd_una);
+		}
+	}
+}
+
+/*
  *	This routine copies from a sock struct into the user buffer.
  *
  *	Technical note: in 2.3 we work on _locked_ socket, so that
@@ -1763,6 +1778,8 @@ skip_copy:
 	 * on connected socket. I was just happy when found this 8) --ANK
 	 */
 
+	tcp_sock_xmit_compl_seq(msg, sk);
+
 	/* Clean up data we have read: This will do ACK frames. */
 	tcp_cleanup_rbuf(sk, copied);
 
@@ -2617,6 +2634,9 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
 	case TCP_USER_TIMEOUT:
 		val = jiffies_to_msecs(icsk->icsk_user_timeout);
 		break;
+	case TCP_XMIT_COMPL_SEQ:
+		val = tp->snd_una;
+		break;
 	default:
 		return -ENOPROTOOPT;
 	}

^ permalink raw reply related

* Re: [RFC][PATCH 1/4] IRQ: IRQ groups for multiqueue devices
From: Thomas Gleixner @ 2010-09-21 19:04 UTC (permalink / raw)
  To: Ben Hutchings
  Cc: Tom Herbert, netdev, linux-net-drivers, linux-kernel,
	Peter Zijlstra, Ingo Molnar
In-Reply-To: <alpine.LFD.2.00.1009211455140.2416@localhost6.localdomain6>

On Tue, 21 Sep 2010, Thomas Gleixner wrote:
> On Tue, 21 Sep 2010, Ben Hutchings wrote:
> > On Mon, 2010-09-20 at 23:27 +0200, Thomas Gleixner wrote:
> Lemme summarize what I understand so far. For multiqueue devices with
> several queues and interrupts you want to lookup the closest queue/irq
> to the cpu on which you are currently running to avoid expensive cross
> node memory access. Now you need to follow affinity changes for this
> lookup table.
> 
> The basic idea of your code is ok. The lookup table itself is fine,
> but the integration into the genirq code sucks. What needs to be done:
> 
> 1) Remove the references to irq_desc in the group. They are not needed
>    and will never work. The group does not need any information about
>    the irq number and the irq descriptor.
> 
> 2) Add proper refcounting to struct irq_group so it can be accessed
>    outside of irq_desc->lock.
> 
> 3) Move the update of the map outside of irq_desc->lock and the irq
>    disabled region. Update the map in setup_affinity() as well. That
>    code can be called from various atomic contexts, so it's probably
>    the best thing to delegate the update to a workqueue or such unless
>    you come up with a nice prebuilt lookup table.
> 
> 4) Add comments so the code is understandable for mere mortals
> 
> 5) Add proper install/free handling. Hint: affinity_hint
> 
> 6) All modifications of irq_desc need a check whether irq_to_desc
>    returned a valid pointer and must hold desc->lock
> 
> 7) Deal with concurrent updates of multiple irqs in a group (at least
>    a comment why the code does not need serialization)
> 
> 8) Move the code to kernel/irq/irqgroup.c or some other sensible name
>    and make it configurable so embedded folks don't have to carry it
>    around as useless binary bloat.

Talked to Peter about it and we came to the conclusion, that we should
just provide a callback infrastructure in the irq code which does not
care about the action behind it. That's going to solve #1,#2,#3,#5,#6
and parts of #8

That queue/index map code should move to lib/ or some other
appropriate place so it can be shared with storage or whatever is
going to grow multiqueue. comments #4, #7, #8 (s@kernel/irq@lib/@)
above still apply :)

The modification to the genirq code would be based on registering

struct irq_affinity_callback {
	unsigned int irq;
	struct kref kref;
	struct work work;
	void (*callback)(struct irq_affinity_callback *, const cpumask_t *mask);
	void (*release)(struct kref *ref);
};

for an interrupt via 

int irq_set_affinity_callback(unsigned int irq,
    			      struct irq_affinity_callback *cb);

That function can be called with cb=NULL to remove the callback. if
cb!=NULL, irq, kref and work are initialized.

The genirq code schedules work when affinity changes and the callback
struct pointer is non NULL. The callback is then called from the
worker thread with a copy of the irq affinity mask.

So on any affinity change, we do with desc->lock held:

   if (desc->affinity_callback) {
      	kref_get(&desc->affinity_callback->kref);
	schedule_work(&desc->affinity_callback->work);
   }

The worker function in the genirq code does:
{
    struct irq_affinity_callback *cb;
    struct irq_desc *desc;
    cpu_mask_t cpumask;

    cb = container_of(work, struct irq_affinity_callback, work);

    desc = irq_to_desc(cb->irq);
    if (!desc)
       goto out;

    if (!alloc_cpumask_var(&cpumask, GFP_KERNEL)
       goto out;

    raw_spin_lock_irqsave(&desc->lock, flags);
    cpumask_copy(cpumask, desc->affinity);
    raw_spin_unlock_irqrestore(&desc->lock, flags);

    cb->callback(cb, cpumask);

    free_cpumask_var(cpumask);
out:
    kref_put(&cb->kref, cb->release);
}

That allows you to do all kind of magic in thread context, updating
the queue map, reallocating queue memory when the node affinity
changes (I know that you want to), go wild.

Thoughts ?

	 tglx

^ permalink raw reply

* Re: [PATCH v3] ip: fix truesize mismatch in ip fragmentation
From: Jarek Poplawski @ 2010-09-21 19:21 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Nick Bowler, linux-kernel, netdev, David S. Miller,
	Patrick McHardy
In-Reply-To: <1285094865.2452.2.camel@edumazet-laptop>

On Tue, Sep 21, 2010 at 08:47:45PM +0200, Eric Dumazet wrote:
> 
> > Looks better and better to me, except, checkpatch complains about the
> > (existing) indentation fault here (and later), but I guess you've seen
> > that?
> > 
> 
> Indeed, there is checkpatch somewhere ;)
> 
> Thanks !
> 
> [PATCH v4] ip: fix truesize mismatch in ip fragmentation
> 
> Special care should be taken when slow path is hit in ip_fragment() :
> 
> When walking through frags, we transfert truesize ownership from skb to
> frags. Then if we hit a slow_path condition, we must undo this or risk
> uncharging frags->truesize twice, and in the end, having negative socket
> sk_wmem_alloc counter, or even freeing socket sooner than expected.
> 
> Many thanks to Nick Bowler, who provided a very clean bug report and
> test program.
> 
> Thanks to Jarek for reviewing my first patch and providing a V2
> 
> While Nick bisection pointed to commit 2b85a34e911 (net: No more
> expensive sock_hold()/sock_put() on each tx), underlying bug is older
> (2.6.12-rc5)
> 
> A side effect is to extend work done in commit b2722b1c3a893e
> (ip_fragment: also adjust skb->truesize for packets not owned by a
> socket) to ipv6 as well.
> 
> Reported-and-bisected-by: Nick Bowler <nbowler@elliptictech.com>
> Tested-by: Nick Bowler <nbowler@elliptictech.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> CC: Jarek Poplawski <jarkao2@gmail.com>
> CC: Patrick McHardy <kaber@trash.net>

Looks perfect to me.

Jarek P.

^ permalink raw reply

* Re: [PATCH v3] ip: fix truesize mismatch in ip fragmentation
From: David Miller @ 2010-09-21 19:24 UTC (permalink / raw)
  To: hmh; +Cc: eric.dumazet, nbowler, linux-kernel, netdev, jarkao2, kaber
In-Reply-To: <20100921180940.GA8754@khazad-dum.debian.net>

From: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Date: Tue, 21 Sep 2010 15:09:40 -0300

> On Tue, 21 Sep 2010, Eric Dumazet wrote:
>> Le mardi 21 septembre 2010 à 13:26 -0300, Henrique de Moraes Holschuh a
>> écrit :
>> > Should this be a candidate for -stable?
>> > 
>> 
>> Yes, of course, but David wants to handle stable submissions himself.
>> 
>> I am not sure we want to bug stable team with dozens of mails while
>> polishing patches ?
> 
> We don't.  But one often marks commits that should go to -stable using a Cc:
> pseudo-header, and also includes relevant information (e.g. to which stable
> kernels it should be applied to) to the commit message.
> 
> Since there wasn't one, and I didn't readly find any post in this thread
> that mentioned it should also go to stable, AND it looked at first glance
> like something that should go to stable, I asked about it.

Sorry, that is not how we typically handle things in the networking.

I queue up all appropriate -stable patches automatically.

I do this mainly because:

1) It isn't the submitter who gets to decide all by himself that
   something is -stable material, that's partly my job too.

   So if the submitter puts the CC: stable thing in the commit
   message, that takes me out of the decision making process.

2) I do not want -stable submissions to go in just because a patch
   made it into Linus's tree.

   I want fixes to sit and cook in Linus's tree for a while before
   they go to -stable unless it's an _incredibly_ obvious fix.
   This allows any bugs in the fix to be shaken out first.

   Again, the CC: stable tag subverts that.

I really think the "CC: stable" tag is only appropriate for a very
limited scope of bug fixes.  The incredibly obvious ones that need
almost no testing and time exposure in Linus's tree.

All of the rest should be carefully queued up for -stable and
submitted there after a week or two of the patch sitting in Linus's
tree.

^ 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