From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id B4125A055A; Fri, 27 May 2022 13:28:07 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4831D40E78; Fri, 27 May 2022 13:28:07 +0200 (CEST) Received: from forward500p.mail.yandex.net (forward500p.mail.yandex.net [77.88.28.110]) by mails.dpdk.org (Postfix) with ESMTP id 7756F40E5A for ; Fri, 27 May 2022 13:28:06 +0200 (CEST) Received: from myt6-7a675ea05ad1.qloud-c.yandex.net (myt6-7a675ea05ad1.qloud-c.yandex.net [IPv6:2a02:6b8:c12:5224:0:640:7a67:5ea0]) by forward500p.mail.yandex.net (Yandex) with ESMTP id C6990F014AD; Fri, 27 May 2022 14:28:05 +0300 (MSK) Received: from myt6-efff10c3476a.qloud-c.yandex.net (myt6-efff10c3476a.qloud-c.yandex.net [2a02:6b8:c12:13a3:0:640:efff:10c3]) by myt6-7a675ea05ad1.qloud-c.yandex.net (mxback/Yandex) with ESMTP id oaZhopvJ47-S5fu5lNs; Fri, 27 May 2022 14:28:05 +0300 X-Yandex-Fwd: 2 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yandex.ru; s=mail; t=1653650885; bh=5q7LVkBNcydVxjRBBciGbIU22vmQlz4F+/dD3kWYFhY=; h=In-Reply-To:From:Subject:Cc:References:Date:Message-ID:To; b=pNXjul45t61WNU5JsQeIX0dY6dTyw2clrT8jsFUWf4WjB4A6mefZCUUowucLVY4KJ JZwUHMjpEOgp7ySkTXy+Nc0U23pJwLcVLghXFbs/o8gWdtMMsS2HwOqtA7eiLoEzbs LQ03QyIEfVOgQBrvWRtaKf+vD7FWvlIwY9K9Z5UI= Authentication-Results: myt6-7a675ea05ad1.qloud-c.yandex.net; dkim=pass header.i=@yandex.ru Received: by myt6-efff10c3476a.qloud-c.yandex.net (smtp/Yandex) with ESMTPSA id U9Ue8jgl5U-S4GSX2LM; Fri, 27 May 2022 14:28:04 +0300 (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (Client certificate not present) Message-ID: <94fc60e2-09a5-adb3-7926-1b21e9ebe8ab@yandex.ru> Date: Fri, 27 May 2022 12:28:02 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.1 Subject: Re: [PATCH v1 5/5] examples/l3fwd: enable direct rearm mode Content-Language: en-US To: Honnappa Nagarahalli , dev@dpdk.org Cc: honnappanagarahalli@gmail.com, feifei.wang2@arm.com, ruifeng.wang@arm.com, nd@arm.com References: <20220420081650.2043183-1-feifei.wang2@arm.com> <20220420081650.2043183-6-feifei.wang2@arm.com> From: Konstantin Ananyev In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 25/05/2022 01:24, Honnappa Nagarahalli пишет: > From: Konstantin Ananyev > > 20/04/2022 09:16, Feifei Wang пишет: >>> Enable direct rearm mode. The mapping is decided in the data plane based >>> on the first packet received. >>> >>> Suggested-by: Honnappa Nagarahalli >>> Signed-off-by: Feifei Wang >>> Reviewed-by: Ruifeng Wang >>> Reviewed-by: Honnappa Nagarahalli >>> --- >>>   examples/l3fwd/l3fwd_lpm.c | 16 +++++++++++++++- >>>   1 file changed, 15 insertions(+), 1 deletion(-) >>> >>> diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c >>> index bec22c44cd..38ffdf4636 100644 >>> --- a/examples/l3fwd/l3fwd_lpm.c >>> +++ b/examples/l3fwd/l3fwd_lpm.c >>> @@ -147,7 +147,7 @@ lpm_main_loop(__rte_unused void *dummy) >>>       unsigned lcore_id; >>>       uint64_t prev_tsc, diff_tsc, cur_tsc; >>>       int i, nb_rx; >>> -    uint16_t portid; >>> +    uint16_t portid, tx_portid; >>>       uint8_t queueid; >>>       struct lcore_conf *qconf; >>>       const uint64_t drain_tsc = (rte_get_tsc_hz() + US_PER_S - 1) / >>> @@ -158,6 +158,8 @@ lpm_main_loop(__rte_unused void *dummy) >>>       const uint16_t n_rx_q = qconf->n_rx_queue; >>>       const uint16_t n_tx_p = qconf->n_tx_port; >>> +    int direct_rearm_map[n_rx_q]; >>> + >>>       if (n_rx_q == 0) { >>>           RTE_LOG(INFO, L3FWD, "lcore %u has nothing to do\n", >>> lcore_id); >>>           return 0; >>> @@ -169,6 +171,7 @@ lpm_main_loop(__rte_unused void *dummy) >>>           portid = qconf->rx_queue_list[i].port_id; >>>           queueid = qconf->rx_queue_list[i].queue_id; >>> +        direct_rearm_map[i] = 0; >>>           RTE_LOG(INFO, L3FWD, >>>               " -- lcoreid=%u portid=%u rxqueueid=%hhu\n", >>>               lcore_id, portid, queueid); >>> @@ -209,6 +212,17 @@ lpm_main_loop(__rte_unused void *dummy) >>>               if (nb_rx == 0) >>>                   continue; >>> +            /* Determine the direct rearm mapping based on the first >>> +             * packet received on the rx queue >>> +             */ >>> +            if (direct_rearm_map[i] == 0) { >>> +                tx_portid = lpm_get_dst_port(qconf, pkts_burst[0], >>> +                            portid); >>> +                rte_eth_direct_rxrearm_map(portid, queueid, >>> +                                tx_portid, queueid); >>> +                direct_rearm_map[i] = 1; >>> +            } >>> + > >> That just doesn't look right to me: why to make decision based on the >> first packet? > The TX queue depends on the incoming packet. So, this method covers more > scenarios > than doing it in the control plane where the outgoing queue is not known. > > >> What would happen if second and all other packets have to be routed >> to different ports? > This is an example application and it should be fine to make this > assumption. > More over, it does not cause any problems if packets change in between. > When > the packets change back, the feature works again. > >> In fact, this direct-rearm mode seems suitable only for hard-coded >> one to one mapped forwarding (examples/l2fwd, testpmd). >> For l3fwd it can be used safely only when we have one port in use. > Can you elaborate more on the safety issue when more than one port is used? > >> Also I think it should be selected at init-time and >> it shouldn't be on by default. >> To summarize, my opinion: >> special cmd-line parameter to enable it. > Can you please elaborate why a command line parameter is required? Other > similar > features like RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE are enabled without a > command > line parameter. IMO, this is how it should ber. Essentially we are > trying to measure > how different PMDs perform, the ones that have implemented performance > improvement > features would show better performance (i.e. the PMDs implementing the > features > should not be penalized by asking for additional user input). From my perspective, main purpose of l3fwd application is to demonstrate DPDK ability to do packet routing based on input packet contents. Making guesses about packet contents is a change in expected behavior. For some cases it might improve performance, for many others - will most likely cause performance drop. I think that performance drop as default behavior (running the same parameters as before) should not be allowed. Plus you did not provided ability to switch off that behavior, if undesired. About comparison with RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE default enablement - I don't think it is correct. Within l3fwd app we can safely guarantee that all RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE pre-requirements are met: in each TX queue all mbufs will belong to the same mempool and their refcnt will always equal to one. Here you are making guesses about contents of input packets, without any guarantee that you guess will always be valid. BTW, what's wrong with using l2fwd to demonstrate that feature? Seems like a natural choice to me. >> allowable only when we run l3fwd over one port. > > >>>   #if defined RTE_ARCH_X86 || defined __ARM_NEON \ >>>                || defined RTE_ARCH_PPC_64 >>>               l3fwd_lpm_send_packets(nb_rx, pkts_burst,