From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2D350C43381 for ; Wed, 27 Feb 2019 15:53:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id DAF432184A for ; Wed, 27 Feb 2019 15:53:29 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="mg7VER6Y" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728751AbfB0Px2 (ORCPT ); Wed, 27 Feb 2019 10:53:28 -0500 Received: from mail-pf1-f196.google.com ([209.85.210.196]:42160 "EHLO mail-pf1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726122AbfB0Px2 (ORCPT ); Wed, 27 Feb 2019 10:53:28 -0500 Received: by mail-pf1-f196.google.com with SMTP id n74so8198797pfi.9 for ; Wed, 27 Feb 2019 07:53:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=RH4JNEdq7tXRtTj8HJ0td4E+hpstXcbWTwPQrWntiVA=; b=mg7VER6YRbRro0R1Fs38N/4BCM1yV/KDJ8EpsIfwQ+VIP1JY61+fW1K9XO9vN7nqzN 6vHPjeLtYtyr8MUC2MS69QcKyRHuui94TiFmCNUSbHd11dXtHZ5SRfD/tmrbN675v0eC Ai+N+UyK0QgjxBLE7ztwwpoiWMH8Kkic4hUe4OeZ2kyo27/vweioH0aoUyT1uLuEXR6l SWKZZaSDveT7HpKmidhyjXCKJa0jQTn8AQ1hOzqX0FBqL64s8/LVghxhxHTvJBr+Kv+d Xdvr2ddQv1gW0H7LQPfDgQz9jgzIdUUdYpj5ddYjrWU0uVe4uOTNvsf5WyMKgTZMZtEg 6ZtA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=RH4JNEdq7tXRtTj8HJ0td4E+hpstXcbWTwPQrWntiVA=; b=DuUO+bAX3+bZeNOdz8kQURYadTD8PClTi5IScDI+dbuLKiylSCweyZzsoP3NaTmE0D yOqB4xxiXbbQdLEwVRZy5eVl3IsiwYA1FgUpRl+yLxj0EmDd/53KIWhXU/sWRHd1oGBh SgtAhHVxgGwG4qHEBEsc4RzLWP7MLF8Y2LrxZ8y4AJid7H1RRX+HaNPIkcdGSlbmilvI pxQX59r8XvUuoJqsTtOtPvfXUj50o/A+wF8ph1m8XKPvxZLqgkwNF5OC0oj/Pp+Er/7z O+oZ173jP7xuhKJISvOwZeudqhFBkK0heUsUEiS3Cmjs/KYCLk5pH05DiREs2N1M0SHt 0JnQ== X-Gm-Message-State: AHQUAuZFXfsAFTfDxpDFsYD2x6GdJMw9TdcKinezI8Z5Bvc8HlwaDh2x Hob2kqGLwcfe8d9UTem0fO0= X-Google-Smtp-Source: AHgI3Ibgjl7KEjt/XU8Zg4fr47NjybMDB5/HUxnD71oHEG0NDGrAJxRzM7VEcWSlrR3OgKVeO8FXLQ== X-Received: by 2002:a63:ea52:: with SMTP id l18mr3586541pgk.317.1551282807369; Wed, 27 Feb 2019 07:53:27 -0800 (PST) Received: from [192.168.88.86] (179.107.197.35.bc.googleusercontent.com. [35.197.107.179]) by smtp.gmail.com with ESMTPSA id l73sm51228937pfb.113.2019.02.27.07.53.24 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 27 Feb 2019 07:53:26 -0800 (PST) Subject: Re: [PATCH] net: netem: fix skb length BUG_ON in __skb_to_sgvec To: Sheng Lan , Eric Dumazet , Stephen Hemminger Cc: davem@davemloft.net, netdev@vger.kernel.org, netem@lists.linux-foundation.org, xuhanbing@huawei.com, zhengshaoyu@huawei.com, jiqin.ji@huawei.com, liuzhiqiang26@huawei.com, yuehaibing@huawei.com References: <20190225080147.30128f73@shemminger-XPS-13-9360> <05fa74b9-6e3b-7bd3-fa1c-c02e37a521f8@huawei.com> <375ee6d3-ef86-c653-5d8d-df48cea8b3ba@gmail.com> From: Eric Dumazet Message-ID: <651ade58-af15-3980-562f-dd3d461f2fcb@gmail.com> Date: Wed, 27 Feb 2019 07:53:23 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 02/27/2019 03:26 AM, Sheng Lan wrote: > > I traced again and found that the skb was not sent, master skb was still in write queue, > because the function tcp_transmit_skb() returns 1 (NET_XMIT_DROP), thus it can be retransmit. > I found the error value NET_XMIT_DROP returns from netem_enqueue(), when the length of qdisc queue > is greater than queue limit value. > > In netem_enqueue() the skb is cloned before returning the NET_XMIT_DROP error value, > thus the master skb is still in write queue and be cloned in netem_enqueue(). This may cause the master > skb be retransmit and fragmented again while it is cloned. > > I think there are potential risks that tso_fragment() will get a cloned skb if skb is cloned by lower layer. > I try to fix it by moving returning error value statment to the front of the skb_clone() in netem_enqueue(), and it works. > And netem_enqueue() constructs corrupt packets statment returns NET_XMIT_DROP too. To fix this completely should I move the > constructing corrupt statment to the front of the skb_clone() ? > Please correct me if I am wrong, and I need your advice. > Hi Choices are either : 1) netem sends a proper return value if a packet has been queued. 2) TCP (and probably other protocols) no longer trust lower stack and always move the master skb in rtx queue, even if the transmit of the (first) clone failed. I prefer 1), since netem is not used in the fast path, generally... diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c index 75046ec7214449c631c38eaab5e4a51644cfa0e5..f6ea2d44dffe328a2fd1a468e209aac0bfaccd49 100644 --- a/net/sched/sch_netem.c +++ b/net/sched/sch_netem.c @@ -447,6 +447,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, int nb = 0; int count = 1; int rc = NET_XMIT_SUCCESS; + int rc_drop = NET_XMIT_DROP; /* Do not fool qdisc_drop_all() */ skb->prev = NULL; @@ -486,6 +487,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, q->duplicate = 0; rootq->enqueue(skb2, rootq, to_free); q->duplicate = dupsave; + rc_drop = NET_XMIT_SUCCESS; } /* @@ -498,7 +500,7 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, if (skb_is_gso(skb)) { segs = netem_segment(skb, sch, to_free); if (!segs) - return NET_XMIT_DROP; + return rc_drop; } else { segs = skb; } @@ -521,9 +523,10 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, 1<<(prandom_u32() % 8); } - if (unlikely(sch->q.qlen >= sch->limit)) - return qdisc_drop_all(skb, sch, to_free); - + if (unlikely(sch->q.qlen >= sch->limit)) { + qdisc_drop_all(skb, sch, to_free); + return rc_drop; + } qdisc_qstats_backlog_inc(sch, skb); cb = netem_skb_cb(skb);