diff mbox

bisected regression: 3c59x corrupts packets in 3.17-rc5

Message ID 20140917102701.GA5699@hmsreliant.think-freely.org
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Neil Horman Sept. 17, 2014, 10:27 a.m. UTC 
On Tue, Sep 16, 2014 at 04:29:23PM -0400, David Miller wrote:
> From: Neil Horman <nhorman@tuxdriver.com>
> Date: Tue, 16 Sep 2014 06:17:04 -0400
> 
> > I'm guessing the above change has uncovered another bug,
> 
> Neil, read your patch carefully, I think it added the bug.
> 
> The ->page_offset of the frag gets applied two times.
> 
> skb_dma_map_frag() already takes frag->page_offset into consideration,
> you then pass it in as the 'offset' argument and it then gets added to
> itself to compute th final offset.
> 
Shit, you're right, sorry about that.  Its odd, I'm running it here, and its not 
causing problems, but thats obviously wrong.  Meelis, please add the above fix
to your test and confirm that it sovles the problem.  If you could keep the
previous patch in place too that would be great, as we should probably add the
dma error checking anyway.


[PATCH] 3c59x: Fix bad offset spec in skb_frag_dma_map

Recently aded the use of skb_frag_dma_map to 3c59x, but didn't realize it
automatically included the frag_offset internally, as well as provided an option
to specify an extra offset in the parameter list.  We need to specify an offset
of 0 in the parameter list to avoid skb corruption that results in lost
connections.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
---
 drivers/net/ethernet/3com/3c59x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Meelis Roos Sept. 17, 2014, 12:43 p.m. UTC | #1 
> Shit, you're right, sorry about that.  Its odd, I'm running it here, and its not 
> causing problems, but thats obviously wrong.  Meelis, please add the above fix
> to your test and confirm that it sovles the problem.  If you could keep the
> previous patch in place too that would be great, as we should probably add the
> dma error checking anyway.
> 
> 
> [PATCH] 3c59x: Fix bad offset spec in skb_frag_dma_map

Tested 2 variants: only this patch (backported to old state) and both 
patches together.

Both work fine.
Neil Horman Sept. 17, 2014, 12:56 p.m. UTC | #2 
On Wed, Sep 17, 2014 at 03:43:54PM +0300, mroos@linux.ee wrote:
> > Shit, you're right, sorry about that.  Its odd, I'm running it here, and its not 
> > causing problems, but thats obviously wrong.  Meelis, please add the above fix
> > to your test and confirm that it sovles the problem.  If you could keep the
> > previous patch in place too that would be great, as we should probably add the
> > dma error checking anyway.
> > 
> > 
> > [PATCH] 3c59x: Fix bad offset spec in skb_frag_dma_map
> 
> Tested 2 variants: only this patch (backported to old state) and both 
> patches together.
> 
> Both work fine.
> 
Thank you, Meelis.  Dave, I'll propose these patches officially in just a bit

sorry for the 
> -- 
> Meelis Roos (mroos@linux.ee)
> 
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
index 5621dab..0f59d68 100644
--- a/drivers/net/ethernet/3com/3c59x.c
+++ b/drivers/net/ethernet/3com/3c59x.c
@@ -2186,7 +2186,7 @@  boomerang_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
 
 			dma_addr = skb_frag_dma_map(&VORTEX_PCI(vp)->dev, frag,
-						    frag->page_offset,
+						    0,
 						    frag->size,
 						    DMA_TO_DEVICE);
 			if (dma_mapping_error(&VORTEX_PCI(vp)->dev, dma_addr)) {