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=-2.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_PASS,USER_AGENT_NEOMUTT 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 2E474C43381 for ; Tue, 26 Mar 2019 04:27:29 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id F266720811 for ; Tue, 26 Mar 2019 04:27:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="Nj7A/Yzt" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726279AbfCZE1K (ORCPT ); Tue, 26 Mar 2019 00:27:10 -0400 Received: from mail-pf1-f193.google.com ([209.85.210.193]:46555 "EHLO mail-pf1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725776AbfCZE1K (ORCPT ); Tue, 26 Mar 2019 00:27:10 -0400 Received: by mail-pf1-f193.google.com with SMTP id 9so7454948pfj.13 for ; Mon, 25 Mar 2019 21:27:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=aMpxKAE/xKq7iVSB6EGRXG96C5N90YnMdAN8L/CTcDY=; b=Nj7A/YztPZXASXKnKLW0XVo0JpNhp5SiIr5glOnbEYZ5HwuP9LHFA40S29LJpGZiMA annuJhPdcMyhjjdz91CDAixiYllvWBJfhOxw3hszfecOaCu56yndTPmS7YOTjvGqMx8J OHSnabwuTbhIC85pmim2kgl92Z1E4yVYHHx6KumTpUqdUztHA/+mFiVtfFqS801fxAy+ PtBVuhmajLqU5aJueWw/s/QNdfqu+lrIL46tQBNfworWi8LXoRqvH67gV6n27u7y2lzu YnS5Y3CBOr6bfek/KqGzh0Gtm64VwezlOLdhri3q0RzIoE6/wec99jf1kLU6ZEJPzs1K grlQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=aMpxKAE/xKq7iVSB6EGRXG96C5N90YnMdAN8L/CTcDY=; b=GXZhG+VVywypIVtslt051WIXkgf1D8Gtakwxb4Uw2NW6krX9++WYNgYm7u9LcLbgZp v/6X1IR6tQ+i3e3UPvOzMunOl9gMY2k4JPyNggEJKEDJc99GTlsoxIisgFtfhHDRK2W8 1UmLDyKmK5T4OL2suwkl9zRbNf8vVAr8IeHa9w1hB3PuEkK9va3n/GoZ3tLUUNm0Z/ks pHTV+Dx4gJ868LWQMMeFdSESMUs/GQP9PhMn9ZS8XsQrmBH4zaczpChjER7CXLlZuKoQ xOsk/BksabBDBw7rtHkUlDXlwj1bcMSpE0zZVzT/IDC4PFIlE9s9qb7dKbfRCNparjJu 97iQ== X-Gm-Message-State: APjAAAXwyJJwqa1KuNiWRZ5Y2bGFGGcSS9VJ+v0UUmQ0T9rrO+WRUs1s flNxwLu9XAW0VwQzGGPDzBVx7nXO X-Google-Smtp-Source: APXvYqy2+nphzl0ZGUhuwSox/NkDeo6yYpBdnA2HmHZo1Gi1QiKs1HB1qoGFW05qpRYgxVOmhmja4w== X-Received: by 2002:a62:6383:: with SMTP id x125mr26862779pfb.239.1553574429799; Mon, 25 Mar 2019 21:27:09 -0700 (PDT) Received: from ast-mbp ([2620:10d:c090:180::b29f]) by smtp.gmail.com with ESMTPSA id 125sm26209341pfw.139.2019.03.25.21.27.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 25 Mar 2019 21:27:09 -0700 (PDT) Date: Mon, 25 Mar 2019 21:27:06 -0700 From: Alexei Starovoitov To: Eric Dumazet Cc: brakmo , netdev , Martin Lau , Alexei Starovoitov , Daniel Borkmann , Kernel Team Subject: Re: [PATCH bpf-next 0/7] bpf: Propagate cn to TCP Message-ID: <20190326042704.7szakyos3ofemowl@ast-mbp> References: <20190323080542.173569-1-brakmo@fb.com> <704cb63c-13cd-f0ed-d546-18e3596bb63d@gmail.com> <20190323154124.gorqpaqex7ihfs6d@ast-mbp> <0841fe0d-7fcd-bb59-3694-af9969cec5af@gmail.com> <20190324161911.h5eotv2j7f5avcpm@ast-mbp> <5aec97f1-545a-f898-fdd9-c5821d5c6e39@gmail.com> <27e91d11-b454-924d-58ab-a68a0aade906@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <27e91d11-b454-924d-58ab-a68a0aade906@gmail.com> User-Agent: NeoMutt/20180223 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Mon, Mar 25, 2019 at 01:48:27AM -0700, Eric Dumazet wrote: > > > On 03/25/2019 01:33 AM, Eric Dumazet wrote: > > > > > > On 03/24/2019 09:19 AM, Alexei Starovoitov wrote: > > >> Cover letter also explains why bpf_skb_ecn_set_ce is not enough. > >> Please realize that existing qdiscs already doing this. > >> The patchset allows bpf-cgroup to do the same. > > > > Not the same thing I am afraid. > > To be clear Alexei : > > Existing qdisc set CE mark on a packet, exactly like a router would do. > Simple and universal. > This can be stacked, and done far away from the sender. > > We do not _call_ back local TCP to propagate cn. How do you classify NET_XMIT_CN ? It's exactly local call back to indicate CN into tcp from layers below tcp. tc-bpf prog returning 'drop' code means drop+cn whereas cgroup-bpf prog returning 'drop' means drop only. This patch set is fixing this discrepancy. > We simply rely on the fact that incoming ACK will carry the needed information, > and TCP stack already handles the case just fine. > > Larry cover letter does not really explain why we need to handle a corner case > (local drops) with such intrusive changes. Only after so many rounds of back and forth I think I'm starting to understand your 'intrusive change' comment. I think you're referring to: - return ip_finish_output2(net, sk, skb); + ret = ip_finish_output2(net, sk, skb); + return ret ? : ret_bpf; right? And your concern that this change slows down ip stack? Please spell out your concerns in more verbose way to avoid this guessing game. I've looked at assembler and indeed this change pessimizes tailcall optimization. What kind of benchmark do you want to see ? As an alternative we can do it under static_key that cgroup-bpf is under. It will be larger number of lines changed, but tailcall optimization can be preserved. > TCP Small Queues already should make sure local drops are non existent. tsq don't help. Here is the comment from patch 5: "When a packet is dropped when calling queue_xmit in __tcp_transmit_skb and packets_out is 0, it is beneficial to set a small probe timer. Otherwise, the throughput for the flow can suffer because it may need to depend on the probe timer to start sending again. " In other words when we're clamping aggregated cgroup bandwidth with this facility we may need to drop the only in flight packet for this flow and the flow restarts after default 200ms probe timer. In such case it's well under tsq limit. Thinking about tsq... I think it would be very interesting to add bpf hook there as well and have programmable and dynamic tsq limit, but that is orthogonal to this patch set. We will explore this idea separately.