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 4958AA055B; Fri, 3 Jun 2022 12:32:56 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0645A40694; Fri, 3 Jun 2022 12:32:56 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 3B4CD40691 for ; Fri, 3 Jun 2022 12:32:55 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 7FA81F2; Fri, 3 Jun 2022 13:32:54 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 7FA81F2 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1654252374; bh=NSF8Y9v8iMNDsLiTNbr8gXRL4I+KEJFV0qMb0KfTK2I=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=xGS6P9peQ9UbUfapP5+zct51tlVTLHweGXVXKbLVS3RFBv9jsZmxYLEyD1TIiyBvM vNaKFvn9bspxkfXbaXAkGzCIazUCRoSlj78uYzJ83nKztQ20xXn/pTwc1FvAGc0dUr Wel3zQyrCy3t50h1hi3eA1wWR74eF8RO9xPsw+zc= Message-ID: <75d4a26f-e224-7416-c454-53aece4f5853@oktetlabs.ru> Date: Fri, 3 Jun 2022 13:32:54 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 Subject: Re: [PATCH v1 5/5] examples/l3fwd: enable direct rearm mode Content-Language: en-US To: Honnappa Nagarahalli , Konstantin Ananyev , "dev@dpdk.org" Cc: "honnappanagarahalli@gmail.com" , Feifei Wang , Ruifeng Wang , nd References: <20220420081650.2043183-1-feifei.wang2@arm.com> <20220420081650.2043183-6-feifei.wang2@arm.com> <94fc60e2-09a5-adb3-7926-1b21e9ebe8ab@yandex.ru> From: Andrew Rybchenko Organization: OKTET Labs 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 On 5/31/22 20:14, Honnappa Nagarahalli wrote: > >> >> 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. > There is no drop in L3fwd performance due to this patch. It is a questionable statement. Sorry. The patch adds code on data path. So, I'm almost confident that it is possible to find a case when performance slightly drops. May be it is always minor and acceptable. >> >> 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. > This is not a guess. The code understands the incoming traffic and configures accordingly. So, it should be correct. Since it is a sample application, we do not expect the traffic to be complex. If it is complex, the performance will be the same as before or better. > >> >> BTW, what's wrong with using l2fwd to demonstrate that feature? >> Seems like a natural choice to me. > The performance of L3fwd application in DPDK has become a industry standard, hence we need to showcase the performance in L3fwd application. > >> >>>> 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, >