From patchwork Fri Feb 24 16:33:56 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ben McKeegan X-Patchwork-Id: 142950 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id D91CAB6ED0 for ; Sat, 25 Feb 2012 04:05:49 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932180Ab2BXRFs (ORCPT ); Fri, 24 Feb 2012 12:05:48 -0500 Received: from mail.netservers.co.uk ([80.248.178.71]:38105 "EHLO mail.netservers.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757813Ab2BXRFr (ORCPT ); Fri, 24 Feb 2012 12:05:47 -0500 X-Greylist: delayed 1872 seconds by postgrey-1.27 at vger.kernel.org; Fri, 24 Feb 2012 12:05:47 EST X-Envelope-From: ben@netservers.co.uk Received: from benxen ([192.168.3.213]) by mail.netservers.co.uk (8.14.4/8.14.4) with ESMTP id q1OGXug1009346; Fri, 24 Feb 2012 16:33:57 GMT Received: from ben (helo=localhost) by benxen with local-esmtp (Exim 3.36 #1 (Debian)) id 1S0y5k-0001fN-00; Fri, 24 Feb 2012 16:33:56 +0000 Date: Fri, 24 Feb 2012 16:33:56 +0000 (GMT) From: Ben McKeegan To: davem@davemloft.net cc: paulus@samba.org, netdev@vger.kernel.org, linux-ppp@vger.kernel.org Subject: [PATCH] ppp: fix 'ppp_mp_reconstruct bad seq' errors Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=0.8 required=5.0 tests=FSL_HELO_NON_FQDN_1, HELO_NO_DOMAIN,RDNS_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.netservers.co.uk Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org This patch fixes a (mostly cosmetic) bug introduced by the patch 'ppp: Use SKB queue abstraction interfaces in fragment processing' found here: http://www.spinics.net/lists/netdev/msg153312.html The above patch rewrote and moved the code responsible for cleaning up discarded fragments but the new code does not catch every case where this is necessary. This results in some discarded fragments remaining in the queue, and triggering a 'bad seq' error on the subsequent call to ppp_mp_reconstruct. Fragments are discarded whenever other fragments of the same frame have been lost. This can generate a lot of unwanted and misleading log messages. This patch also adds additional detail to the debug logging to make it clearer which fragments were lost and which other fragments were discarded as a result of losses. (Run pppd with 'kdebug 1' option to enable debug logging.) Signed-off-by: Ben McKeegan --- 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 -upr linux-3.2.7-orig/drivers/net/ppp/ppp_generic.c linux-3.2.7-fix-mp-reconstruct/drivers/net/ppp/ppp_generic.c --- linux-3.2.7-orig/drivers/net/ppp/ppp_generic.c 2012-02-21 23:27:47.000000000 +0000 +++ linux-3.2.7-fix-mp-reconstruct/drivers/net/ppp/ppp_generic.c 2012-02-23 18:05:29.000000000 +0000 @@ -2162,14 +2162,22 @@ ppp_mp_reconstruct(struct ppp *ppp) continue; } if (PPP_MP_CB(p)->sequence != seq) { + u32 oldseq; /* Fragment `seq' is missing. If it is after minseq, it might arrive later, so stop here. */ if (seq_after(seq, minseq)) break; /* Fragment `seq' is lost, keep going. */ lost = 1; + oldseq = seq; seq = seq_before(minseq, PPP_MP_CB(p)->sequence)? minseq + 1: PPP_MP_CB(p)->sequence; + + if (ppp->debug & 1) + netdev_printk(KERN_DEBUG, ppp->dev, + "lost frag %u..%u\n", + oldseq, seq-1); + goto again; } @@ -2214,6 +2222,10 @@ ppp_mp_reconstruct(struct ppp *ppp) struct sk_buff *tmp2; skb_queue_reverse_walk_from_safe(list, p, tmp2) { + if (ppp->debug & 1) + netdev_printk(KERN_DEBUG, ppp->dev, + "discarding frag %u\n", + PPP_MP_CB(p)->sequence); __skb_unlink(p, list); kfree_skb(p); } @@ -2229,6 +2241,17 @@ ppp_mp_reconstruct(struct ppp *ppp) /* If we have discarded any fragments, signal a receive error. */ if (PPP_MP_CB(head)->sequence != ppp->nextseq) { + skb_queue_walk_safe(list, p, tmp) { + if (p == head) + break; + if (ppp->debug & 1) + netdev_printk(KERN_DEBUG, ppp->dev, + "discarding frag %u\n", + PPP_MP_CB(p)->sequence); + __skb_unlink(p, list); + kfree_skb(p); + } + if (ppp->debug & 1) netdev_printk(KERN_DEBUG, ppp->dev, " missed pkts %u..%u\n",