From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id C940DA0562; Tue, 31 Mar 2020 12:23:38 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A4216322C; Tue, 31 Mar 2020 12:23:38 +0200 (CEST) Received: from mail-pg1-f196.google.com (mail-pg1-f196.google.com [209.85.215.196]) by dpdk.org (Postfix) with ESMTP id E0C0D2C15 for ; Tue, 31 Mar 2020 12:23:37 +0200 (CEST) Received: by mail-pg1-f196.google.com with SMTP id g32so4105341pgb.6 for ; Tue, 31 Mar 2020 03:23:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=semihalf-com.20150623.gappssmtp.com; s=20150623; h=subject:to:references:from:message-id:date:user-agent:mime-version :in-reply-to:content-language:content-transfer-encoding; bh=s/2iZlDQ45jf3hcYQIMKFvf+RJInGcg0ezwlf71Tl50=; b=BXyZZ6o9ojtFencdpPtK1ATyCBfJgunv4mFhAwZcmJh6cPQcNqMiiFG0ohPs3V4sat tpL1kMuaLV/9RHHwMpFqoXuXarMUCmRiYMiaWd5rf4doRFMA3nMGZUcS7/sN5rch20sr EojN/DzgQQ/Rm9+uVULyHkYHblU41PzrAihPWsqEHUZYW3F0a3hO9mRZVe18zVdFf5SG 0cQdc0jzVbd+FRLxjexAOcTMe32gYdH0qTxH5rp3GEPgC0LWXI2I4QKIMuQ4qHdNeZod 8m0H5VsBfEklhgdqtTyAdNMjQVLAgQuriOG6xhQRl3fMiUepJdMF/SY9HRIk2xmWYR1A 0p8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=s/2iZlDQ45jf3hcYQIMKFvf+RJInGcg0ezwlf71Tl50=; b=tUdzNVKDb7lhwQzsydn5LKnH5T7H7l7KoxJnk05GkEGPOwiYdHLBXwuNgEFwU3iJ9D r4QCvx01s9WLj0C730XjY/TDJ6kd6GXWkEa+Mtw0j0XsXjN2nMQde8KamFEd/aCQkeGo VUcODLThv7/62sdHUnT5+lZNnz9dVxFsv5DAaXppzRmV0EF0DiITNBEsoneO6i28RL0X mJPOPkc7DXORyGBSCe75KiEEMxVScxZH4ElPmgiFEzhVCFg+8Cf9qQ7yzlpsuW5RnDEu O/cdkAhVkcyoHatOspioUHpGhFRSkjxeQCRvuiGBNTHfzKMsKkE3ycvghfFtLCMlfgTN wh6A== X-Gm-Message-State: ANhLgQ0bcF8SwciyC2hcc8sFr/j9NgRR6QzfYndyqv1BtZAkFvEJGZxo wv4VLktJaeQ9VZD8X7YdUxMcTWq83fAIdA== X-Google-Smtp-Source: ADFU+vvYDkjwdsR9OvPMxxriEj6x0OCjJPXLoVN6oXSWj7pg7pSRxovtd+s47BCwDG3xVkXJJde8iA== X-Received: by 2002:a63:f13:: with SMTP id e19mr16504796pgl.135.1585650216647; Tue, 31 Mar 2020 03:23:36 -0700 (PDT) Received: from [10.95.131.153] ([199.233.58.37]) by smtp.gmail.com with ESMTPSA id q67sm652175pjq.29.2020.03.31.03.23.35 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 31 Mar 2020 03:23:36 -0700 (PDT) To: dev@dpdk.org References: <20200324123446.2397-2-pbhagavatula@marvell.com> <20200326064216.5676-1-pbhagavatula@marvell.com> From: Andrzej Ostruszka Message-ID: Date: Tue, 31 Mar 2020 12:23:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Firefox/68.0 Thunderbird/68.4.1 MIME-Version: 1.0 In-Reply-To: <20200326064216.5676-1-pbhagavatula@marvell.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [dpdk-dev v2] [PATCH] examples/l2fwd-event: add option to configure port pairs X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 3/26/20 7:42 AM, pbhagavatula@marvell.com wrote: > From: Pavan Nikhilesh > > Current l2fwd-event application statically configures adjacent ports as > destination ports for forwarding the traffic. > > Add a config option to pass the forwarding port pair mapping which allows > the user to configure forwarding port mapping. > > If no config argument is specified, destination port map is not > changed and traffic gets forwarded with existing mapping. > > To align port/queue configuration of each lcore with destination port > map, port/queue configuration of each lcore gets modified when config > option is specificed. > > Ex: ./l2fwd-event -c 0xff -- -p 0x3f -q 2 --config="(0,3)(1,4)(2,5)" > > With above config option, traffic received from portid = 0 gets forwarded > to port = 3 and vice versa, similarly traffic gets forwarded on other port > pairs (1,4) and (2,5). > > Signed-off-by: Pavan Nikhilesh > --- > v2 Changes: > - Fix minor formatting error. > - Change uint8_t to bool. [...] > @@ -99,6 +103,69 @@ l2fwd_event_parse_eventq_sched(const char *optarg, > rsrc->sched_type = RTE_SCHED_TYPE_PARALLEL; > } > > +static int > +l2fwd_parse_port_pair_config(const char *q_arg, struct l2fwd_resources *rsrc) > +{ > + enum fieldnames { > + FLD_PORT1 = 0, > + FLD_PORT2, > + _NUM_FLD > + }; > + const char *p, *p0 = q_arg; > + uint16_t int_fld[_NUM_FLD]; > + char *str_fld[_NUM_FLD]; > + uint16_t port_pair = 0; > + unsigned int size; > + char s[256]; > + char *end; > + int i; > + > + while ((p = strchr(p0, '(')) != NULL) { > + ++p; > + p0 = strchr(p, ')'); > + if (p0 == NULL) > + return -1; > + > + size = p0 - p; > + if (size >= sizeof(s)) > + return -1; > + > + snprintf(s, sizeof(s), "%.*s", size, p); This is a bit peculiar form of memcpy - you want no more than sizeof(s) copied but that you checked above so here simple memcpy should be enough. > + if (rte_strsplit(s, sizeof(s), str_fld, > + _NUM_FLD, ',') != _NUM_FLD) > + return -1; > + > + for (i = 0; i < _NUM_FLD; i++) { > + errno = 0; > + int_fld[i] = strtoul(str_fld[i], &end, 0); > + if (errno != 0 || end == str_fld[i] || int_fld[i] > 255) Replace 255 with check on ">= RTE_MAX_ETHPORTS". > + return -1; > + } > + > + if (port_pair >= RTE_MAX_ETHPORTS / 2) { > + printf("exceeded max number of port pair params: Current %d Max = %d\n", > + port_pair, RTE_MAX_ETHPORTS / 2); > + return -1; > + } > + > + if ((rsrc->dst_ports[int_fld[FLD_PORT1]] != UINT32_MAX) || > + (rsrc->dst_ports[int_fld[FLD_PORT2]] != UINT32_MAX)) { > + printf("Duplicate port pair (%d,%d) config\n", > + int_fld[FLD_PORT1], int_fld[FLD_PORT2]); > + return -1; > + } > + > + rsrc->dst_ports[int_fld[FLD_PORT1]] = int_fld[FLD_PORT2]; > + rsrc->dst_ports[int_fld[FLD_PORT2]] = int_fld[FLD_PORT1]; > + > + port_pair++; > + } > + > + rsrc->port_pairs = true; > + > + return 0; > +} > + [...] > @@ -209,6 +293,51 @@ l2fwd_event_parse_args(int argc, char **argv, > return ret; > } > > +/* > + * Check port pair config with enabled port mask, > + * and for valid port pair combinations. > + */ > +static int > +check_port_pair_config(struct l2fwd_resources *rsrc) > +{ > + uint32_t port_pair_mask = 0; > + uint32_t portid; > + uint16_t index; > + > + for (index = 0; index < rte_eth_dev_count_avail(); index++) { > + if ((rsrc->enabled_port_mask & (1 << index)) == 0) > + continue; > + > + portid = rsrc->dst_ports[index]; > + if (portid == UINT32_MAX) { > + printf("port %u is enabled in but no valid port pair\n", > + index); > + return -1; > + } > + > + if (!rte_eth_dev_is_valid_port(index)) { > + printf("port %u is not valid\n", index); > + return -1; > + } > + > + if (!rte_eth_dev_is_valid_port(portid)) { > + printf("port %u is not valid\n", portid); > + return -1; > + } > + > + if (port_pair_mask & (1 << portid) && > + rsrc->dst_ports[portid] != index) { > + printf("port %u is used in other port pairs\n", portid); > + return -1; > + } > + > + port_pair_mask |= (1 << portid); > + port_pair_mask |= (1 << index); > + } In the above loop you are doing checks twice. Suppose you have pair (2,3) and you go by index from 0 (like you do) and reach point i=2. Then you check i=2 and p=3, then on next iteration you do the same checks (this time i=3,p=2). I guess simple fix would be to skip loop iteration both on not enabled (like you do) and on check if the port was already checked (test bit in port_pair_mask). With regards Andrzej Ostruszka