From patchwork Fri Mar 25 18:33:20 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Samuel Thibault X-Patchwork-Id: 8672771 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id E080F9F326 for ; Fri, 25 Mar 2016 18:43:19 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1CE5020340 for ; Fri, 25 Mar 2016 18:43:19 +0000 (UTC) Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F1C88202F0 for ; Fri, 25 Mar 2016 18:43:17 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ajWey-0001G4-Vv; Fri, 25 Mar 2016 18:40:36 +0000 Received: from mail6.bemta6.messagelabs.com ([85.158.143.247]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ajWey-0001Fv-2A for xen-devel@lists.xen.org; Fri, 25 Mar 2016 18:40:36 +0000 Received: from [85.158.143.35] by server-2.bemta-6.messagelabs.com id 85/77-09532-32685F65; Fri, 25 Mar 2016 18:40:35 +0000 X-Env-Sender: samuel.thibault@ens-lyon.org X-Msg-Ref: server-5.tower-21.messagelabs.com!1458931233!5934484!1 X-Originating-IP: [192.134.164.104] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 8.11; banners=-,-,- X-VirusChecked: Checked Received: (qmail 41081 invoked from network); 25 Mar 2016 18:40:33 -0000 Received: from mail3-relais-sop.national.inria.fr (HELO mail3-relais-sop.national.inria.fr) (192.134.164.104) by server-5.tower-21.messagelabs.com with RC4-SHA encrypted SMTP; 25 Mar 2016 18:40:33 -0000 X-IronPort-AV: E=Sophos;i="5.24,392,1454972400"; d="scan'208";a="170871103" Received: from 37-161-50-49.coucou-networks.fr (HELO var.youpi.perso.aquilenet.fr) ([37.161.50.49]) by mail3-relais-sop.national.inria.fr with ESMTP/TLS/AES128-GCM-SHA256; 25 Mar 2016 19:40:02 +0100 Received: from samy by var.youpi.perso.aquilenet.fr with local (Exim 4.86_2) (envelope-from ) id 1ajWXw-00076U-MV; Fri, 25 Mar 2016 19:33:20 +0100 Date: Fri, 25 Mar 2016 19:33:20 +0100 From: Samuel Thibault To: Wei Liu Message-ID: <20160325183320.GB2792@var> Mail-Followup-To: Samuel Thibault , Wei Liu , Sarah Newman , xen-devel@lists.xen.org, minios-devel@lists.xen.org References: <1458768411-6024-1-git-send-email-srn@prgmr.com> <20160325130907.GC20441@citrix.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20160325130907.GC20441@citrix.com> User-Agent: Mutt/1.5.21+34 (58baf7c9f32f) (2010-12-30) Cc: xen-devel@lists.xen.org, minios-devel@lists.xen.org, Sarah Newman Subject: Re: [Xen-devel] [PATCH] Mini-OS: netfront: fix off-by-one error introduced in 7c8f3483 X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Wei Liu, on Fri 25 Mar 2016 13:09:07 +0000, wrote: > CC Samuel Thanks. > On Wed, Mar 23, 2016 at 02:26:51PM -0700, Sarah Newman wrote: > > 7c8f3483 introduced a break within a loop in netfront.c such that > > cons and nr_consumed were no longer always being incremented. The > > offset at cons will be processed multiple times with the break in > > place. > > > > Remove the break and re-add "some !=0" in the loop for HAVE_LIBC. Mmm, right. That ifdef makes things even more difficult to understand then. That however makes me think: how about the attached patch, which actually simplifies the rest. Thanks! Samuel netfront: fix off-by-one error introduced in 7c8f3483 7c8f3483 introduced a break within a loop in netfront.c such that cons and nr_consumed were no longer always being incremented. The offset at cons will be processed multiple times with the break in place. Remove the break and re-add "some !=0" in the loop for HAVE_LIBC, rename it into dobreak to mitigate confusion. Signed-off-by: Sarah Newman Signed-off-by: Samuel Thibault diff --git a/netfront.c b/netfront.c index 0eca5b5..b8fac62 100644 --- a/netfront.c +++ b/netfront.c @@ -97,19 +97,15 @@ void network_rx(struct netfront_dev *dev) { RING_IDX rp,cons,req_prod; int nr_consumed, more, i, notify; -#ifdef HAVE_LIBC - int some; -#endif + int dobreak; nr_consumed = 0; moretodo: rp = dev->rx.sring->rsp_prod; rmb(); /* Ensure we see queued responses up to 'rp'. */ -#ifdef HAVE_LIBC - some = 0; -#endif - for (cons = dev->rx.rsp_cons; cons != rp; nr_consumed++, cons++) + dobreak = 0; + for (cons = dev->rx.rsp_cons; cons != rp && !dobreak; nr_consumed++, cons++) { struct net_buffer* buf; unsigned char* page; @@ -134,8 +130,8 @@ moretodo: len = dev->len; memcpy(dev->data, page+rx->offset, len); dev->rlen = len; - some = 1; - break; + /* No need to receive the rest for now */ + dobreak = 1; } else #endif dev->netif_rx(page+rx->offset,rx->status); @@ -144,11 +140,7 @@ moretodo: dev->rx.rsp_cons=cons; RING_FINAL_CHECK_FOR_RESPONSES(&dev->rx,more); -#ifdef HAVE_LIBC - if(more && !some) goto moretodo; -#else - if(more) goto moretodo; -#endif + if(more && !dobreak) goto moretodo; req_prod = dev->rx.req_prod_pvt;