[{"id":2548741,"web_url":"http://patchwork.ozlabs.org/comment/2548741/","msgid":"","list_archive_url":null,"date":"2020-10-08T08:06:14","subject":"Re: [PATCH v4 bpf-next 09/13] bpf: introduce multibuff support to\n bpf_prog_test_run_xdp()","submitter":{"id":79035,"url":"http://patchwork.ozlabs.org/api/people/79035/","name":"Shay Agroskin","email":"shayagr@amazon.com"},"content":"Lorenzo Bianconi writes:\n\n> Introduce the capability to allocate a xdp multi-buff in\n> bpf_prog_test_run_xdp routine. This is a preliminary patch to \n> introduce\n> the selftests for new xdp multi-buff ebpf helpers\n>\n> Signed-off-by: Lorenzo Bianconi \n> ---\n> net/bpf/test_run.c | 51 \n> ++++++++++++++++++++++++++++++++++++++--------\n> 1 file changed, 43 insertions(+), 8 deletions(-)\n>\n> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c\n> index bd291f5f539c..ec7286cd051b 100644\n> --- a/net/bpf/test_run.c\n> +++ b/net/bpf/test_run.c\n> @@ -617,44 +617,79 @@ int bpf_prog_test_run_xdp(struct bpf_prog \n> *prog, const union bpf_attr *kattr,\n> {\n> \tu32 tailroom = SKB_DATA_ALIGN(sizeof(struct \n> skb_shared_info));\n> \tu32 headroom = XDP_PACKET_HEADROOM;\n> -\tu32 size = kattr->test.data_size_in;\n> \tu32 repeat = kattr->test.repeat;\n> \tstruct netdev_rx_queue *rxqueue;\n> +\tstruct skb_shared_info *sinfo;\n> \tstruct xdp_buff xdp = {};\n> +\tu32 max_data_sz, size;\n> \tu32 retval, duration;\n> -\tu32 max_data_sz;\n> +\tint i, ret, data_len;\n> \tvoid *data;\n> -\tint ret;\n> \n> \tif (kattr->test.ctx_in || kattr->test.ctx_out)\n> \t\treturn -EINVAL;\n> \n> -\t/* XDP have extra tailroom as (most) drivers use full page \n> */\n> \tmax_data_sz = 4096 - headroom - tailroom;\n\nFor the sake of consistency, can this 4096 be changed to PAGE_SIZE \n?\nSame as in\n data_len = min_t(int, kattr->test.data_size_in - size, \n PAGE_SIZE);\n\nexpression below\n\n> +\tsize = min_t(u32, kattr->test.data_size_in, max_data_sz);\n> +\tdata_len = size;\n> \n> -\tdata = bpf_test_init(kattr, kattr->test.data_size_in,\n> -\t\t\t max_data_sz, headroom, tailroom);\n> +\tdata = bpf_test_init(kattr, size, max_data_sz, headroom, \n> tailroom);\n> \tif (IS_ERR(data))\n> \t\treturn PTR_ERR(data);\n> \n> \txdp.data_hard_start = data;\n> \txdp.data = data + headroom;\n> \txdp.data_meta = xdp.data;\n> -\txdp.data_end = xdp.data + size;\n> +\txdp.data_end = xdp.data + data_len;\n> \txdp.frame_sz = headroom + max_data_sz + tailroom;\n> \n> +\tsinfo = xdp_get_shared_info_from_buff(&xdp);\n> +\tif (unlikely(kattr->test.data_size_in > size)) {\n> +\t\tvoid __user *data_in = \n> u64_to_user_ptr(kattr->test.data_in);\n> +\n> +\t\twhile (size < kattr->test.data_size_in) {\n> +\t\t\tskb_frag_t *frag = \n> &sinfo->frags[sinfo->nr_frags];\n> +\t\t\tstruct page *page;\n> +\t\t\tint data_len;\n> +\n> +\t\t\tpage = alloc_page(GFP_KERNEL);\n> +\t\t\tif (!page) {\n> +\t\t\t\tret = -ENOMEM;\n> +\t\t\t\tgoto out;\n> +\t\t\t}\n> +\n> +\t\t\t__skb_frag_set_page(frag, page);\n> +\t\t\tdata_len = min_t(int, \n> kattr->test.data_size_in - size,\n> +\t\t\t\t\t PAGE_SIZE);\n> +\t\t\tskb_frag_size_set(frag, data_len);\n> +\t\t\tif (copy_from_user(page_address(page), \n> data_in + size,\n> +\t\t\t\t\t data_len)) {\n> +\t\t\t\tret = -EFAULT;\n> +\t\t\t\tgoto out;\n> +\t\t\t}\n> +\t\t\tsinfo->nr_frags++;\n> +\t\t\tsize += data_len;\n> +\t\t}\n> +\t\txdp.mb = 1;\n> +\t}\n> +\n> \trxqueue = \n> __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, \n> 0);\n> \txdp.rxq = &rxqueue->xdp_rxq;\n> \tbpf_prog_change_xdp(NULL, prog);\n> \tret = bpf_test_run(prog, &xdp, repeat, &retval, &duration, \n> true);\n> \tif (ret)\n> \t\tgoto out;\n> +\n> \tif (xdp.data != data + headroom || xdp.data_end != \n> xdp.data + size)\n> -\t\tsize = xdp.data_end - xdp.data;\n> +\t\tsize += xdp.data_end - xdp.data - data_len;\n\nCan we please drop the variable shadowing of data_len ? This is \nconfusing since the initial value of data_len is correct in the \n`size` calculation, while its value inside the while loop it not.\n\nThis seem to be syntactically correct, but I think it's better \npractice to avoid shadowing here.\n\n> +\n> \tret = bpf_test_finish(kattr, uattr, xdp.data, size, \n> retval, duration);\n> out:\n> \tbpf_prog_change_xdp(prog, NULL);\n> +\tfor (i = 0; i < sinfo->nr_frags; i++)\n> +\t\t__free_page(skb_frag_page(&sinfo->frags[i]));\n> \tkfree(data);\n> +\n> \treturn ret;\n> }","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming-netdev@ozlabs.org","Delivered-To":"patchwork-incoming-netdev@ozlabs.org","Authentication-Results":["ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=23.128.96.18; helo=vger.kernel.org;\n envelope-from=netdev-owner@vger.kernel.org; receiver=)","ozlabs.org;\n dmarc=pass (p=quarantine dis=none) header.from=amazon.com","ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=amazon.com header.i=@amazon.com header.a=rsa-sha256\n header.s=amazon201209 header.b=fYQ5l0P7;\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [23.128.96.18])\n\tby ozlabs.org (Postfix) with ESMTP id 4C6P1S2Jbwz9sRK\n\tfor ;\n Thu, 8 Oct 2020 19:06:52 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S1728409AbgJHIGv (ORCPT\n );\n Thu, 8 Oct 2020 04:06:51 -0400","from smtp-fw-9102.amazon.com ([207.171.184.29]:35656 \"EHLO\n smtp-fw-9102.amazon.com\" rhost-flags-OK-OK-OK-OK) by vger.kernel.org\n with ESMTP id S1728395AbgJHIGu (ORCPT\n ); Thu, 8 Oct 2020 04:06:50 -0400","from sea32-co-svc-lb4-vlan3.sea.corp.amazon.com (HELO\n email-inbound-relay-1d-98acfc19.us-east-1.amazon.com) ([10.47.23.38])\n by smtp-border-fw-out-9102.sea19.amazon.com with ESMTP;\n 08 Oct 2020 08:06:43 +0000","from EX13D28EUC001.ant.amazon.com\n (iad12-ws-svc-p26-lb9-vlan2.iad.amazon.com [10.40.163.34])\n by email-inbound-relay-1d-98acfc19.us-east-1.amazon.com (Postfix) with\n ESMTPS id 3A62DA1F3D;\n Thu, 8 Oct 2020 08:06:39 +0000 (UTC)","from u68c7b5b1d2d758.ant.amazon.com.amazon.com (10.43.162.231) by\n EX13D28EUC001.ant.amazon.com (10.43.164.4) with Microsoft SMTP Server (TLS)\n id 15.0.1497.2; Thu, 8 Oct 2020 08:06:33 +0000"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209;\n t=1602144409; x=1633680409;\n h=references:from:to:cc:subject:in-reply-to:date:\n message-id:mime-version;\n bh=bg3O7qEGH1fNCK+zQ+U6E2nWrjAtawJmaEIw/54CrhU=;\n b=fYQ5l0P700TOncHISyku0k7RlBKOfqFnqfhJ9Fz3nfKAdxFjyBg2fI45\n fHuhN6+SMLdNDjtGxVlrir1VpbeIQ0PTZ3Ff9/MyDieg6jjLaG44URgac\n BYMrWVMmq/vEdaSxcO/81Zz4dB/tl6DN14nCYFoiFcIiByhyPPmpI1dAS\n 4=;","X-IronPort-AV":"E=Sophos;i=\"5.77,350,1596499200\";\n d=\"scan'208\";a=\"82535724\"","References":"\n ","User-agent":"mu4e 1.4.12; emacs 27.1","From":"Shay Agroskin ","To":"Lorenzo Bianconi ","CC":", ,\n , , ,\n , ,\n , ,\n , ,\n ","Subject":"Re: [PATCH v4 bpf-next 09/13] bpf: introduce multibuff support to\n bpf_prog_test_run_xdp()","In-Reply-To":"\n ","Date":"Thu, 8 Oct 2020 11:06:14 +0300","Message-ID":"","MIME-Version":"1.0","Content-Type":"text/plain; format=flowed","X-Originating-IP":"[10.43.162.231]","X-ClientProxiedBy":"EX13D01UWB002.ant.amazon.com (10.43.161.136) To\n EX13D28EUC001.ant.amazon.com (10.43.164.4)","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}},{"id":2548882,"web_url":"http://patchwork.ozlabs.org/comment/2548882/","msgid":"<20201008124604.05db39e8@carbon>","list_archive_url":null,"date":"2020-10-08T10:46:04","subject":"Re: [PATCH v4 bpf-next 09/13] bpf: introduce multibuff support to\n bpf_prog_test_run_xdp()","submitter":{"id":13625,"url":"http://patchwork.ozlabs.org/api/people/13625/","name":"Jesper Dangaard Brouer","email":"brouer@redhat.com"},"content":"On Thu, 8 Oct 2020 11:06:14 +0300\nShay Agroskin wrote:\n\n> Lorenzo Bianconi writes:\n> \n> > Introduce the capability to allocate a xdp multi-buff in\n> > bpf_prog_test_run_xdp routine. This is a preliminary patch to \n> > introduce\n> > the selftests for new xdp multi-buff ebpf helpers\n> >\n> > Signed-off-by: Lorenzo Bianconi \n> > ---\n> > net/bpf/test_run.c | 51 ++++++++++++++++++++++++++++++++++++++--------\n> > 1 file changed, 43 insertions(+), 8 deletions(-)\n> >\n> > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c\n> > index bd291f5f539c..ec7286cd051b 100644\n> > --- a/net/bpf/test_run.c\n> > +++ b/net/bpf/test_run.c\n> > @@ -617,44 +617,79 @@ int bpf_prog_test_run_xdp(struct bpf_prog \n> > *prog, const union bpf_attr *kattr,\n> > {\n> > \tu32 tailroom = SKB_DATA_ALIGN(sizeof(struct \n> > skb_shared_info));\n> > \tu32 headroom = XDP_PACKET_HEADROOM;\n> > -\tu32 size = kattr->test.data_size_in;\n> > \tu32 repeat = kattr->test.repeat;\n> > \tstruct netdev_rx_queue *rxqueue;\n> > +\tstruct skb_shared_info *sinfo;\n> > \tstruct xdp_buff xdp = {};\n> > +\tu32 max_data_sz, size;\n> > \tu32 retval, duration;\n> > -\tu32 max_data_sz;\n> > +\tint i, ret, data_len;\n> > \tvoid *data;\n> > -\tint ret;\n> > \n> > \tif (kattr->test.ctx_in || kattr->test.ctx_out)\n> > \t\treturn -EINVAL;\n> > \n> > -\t/* XDP have extra tailroom as (most) drivers use full page \n> > */\n> > \tmax_data_sz = 4096 - headroom - tailroom; \n> \n> For the sake of consistency, can this 4096 be changed to PAGE_SIZE \n> ?\n\nThe size 4096 is explicitly use, because the selftest xdp_adjust_tail\nexpect this, else it will fail on ARCHs with 64K PAGE_SIZE. It also\nseems excessive to create 64K packets for testing XDP.\n\nSee: tools/testing/selftests/bpf/prog_tests/xdp_adjust_tail.c\n\n> Same as in\n> data_len = min_t(int, kattr->test.data_size_in - size, \n> PAGE_SIZE);\n> \n> expression below","headers":{"Return-Path":"","X-Original-To":"patchwork-incoming-netdev@ozlabs.org","Delivered-To":"patchwork-incoming-netdev@ozlabs.org","Authentication-Results":["ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=23.128.96.18; helo=vger.kernel.org;\n envelope-from=netdev-owner@vger.kernel.org; receiver=)","ozlabs.org;\n dmarc=pass (p=none dis=none) header.from=redhat.com","ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256\n header.s=mimecast20190719 header.b=ENsk83Ai;\n\tdkim-atps=neutral"],"Received":["from vger.kernel.org (vger.kernel.org [23.128.96.18])\n\tby ozlabs.org (Postfix) with ESMTP id 4C6SYf0Js7z9sSn\n\tfor ;\n Thu, 8 Oct 2020 21:46:30 +1100 (AEDT)","(majordomo@vger.kernel.org) by vger.kernel.org via listexpand\n id S1729562AbgJHKq3 (ORCPT\n );\n Thu, 8 Oct 2020 06:46:29 -0400","from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:54578 \"EHLO\n us-smtp-delivery-124.mimecast.com\" rhost-flags-OK-OK-OK-OK)\n by vger.kernel.org with ESMTP id S1729571AbgJHKqZ (ORCPT\n ); Thu, 8 Oct 2020 06:46:25 -0400","from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com\n [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id\n us-mta-333-qWcHv-RvPXCue3sAczM7NQ-1; Thu, 08 Oct 2020 06:46:19 -0400","from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com\n [10.5.11.16])\n (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n (No client certificate requested)\n by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 883851054F8A;\n Thu, 8 Oct 2020 10:46:16 +0000 (UTC)","from carbon (unknown [10.40.208.22])\n by smtp.corp.redhat.com (Postfix) with ESMTP id 23FE15C1DC;\n Thu, 8 Oct 2020 10:46:05 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n s=mimecast20190719; t=1602153983;\n h=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n to:to:cc:cc:mime-version:mime-version:content-type:content-type:\n content-transfer-encoding:content-transfer-encoding:\n in-reply-to:in-reply-to:references:references;\n bh=/1s2AOQ8c8V+Fh7lGuyVZZ09B+PUmg+jBpS2SsvH9XY=;\n b=ENsk83Aiq30lkS4hTdzAUrLLMY5nqX9wgX7v0QKHrUT98UKzvtFVTGM5MuNeoYHYMsB9WP\n XBNAqUvb5PFaxubMS00/vMBnjKtr2P/LZlwDRwvKvKg6XD4KyHTincmGwWtQjEHqKbl/Eb\n oLM0lrUSz6UeC5oyQbwslBPShjxYJVE=","X-MC-Unique":"qWcHv-RvPXCue3sAczM7NQ-1","Date":"Thu, 8 Oct 2020 12:46:04 +0200","From":"Jesper Dangaard Brouer ","To":"Shay Agroskin ","Cc":"Lorenzo Bianconi , ,\n , , ,\n , , ,\n , ,\n , ,\n brouer@redhat.com","Subject":"Re: [PATCH v4 bpf-next 09/13] bpf: introduce multibuff support to\n bpf_prog_test_run_xdp()","Message-ID":"<20201008124604.05db39e8@carbon>","In-Reply-To":"","References":"\n \n ","MIME-Version":"1.0","Content-Type":"text/plain; charset=US-ASCII","Content-Transfer-Encoding":"7bit","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","Precedence":"bulk","List-ID":"","X-Mailing-List":"netdev@vger.kernel.org"}}]