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 3842DA0524; Sat, 4 Jul 2020 15:36:39 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 7C8A81DB7A; Sat, 4 Jul 2020 15:36:38 +0200 (CEST) Received: from mail-io1-f66.google.com (mail-io1-f66.google.com [209.85.166.66]) by dpdk.org (Postfix) with ESMTP id E243D1DB5B for ; Sat, 4 Jul 2020 15:36:37 +0200 (CEST) Received: by mail-io1-f66.google.com with SMTP id o5so34900526iow.8 for ; Sat, 04 Jul 2020 06:36:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=PcFd6IUUdBB+IhY4iHrqFf/+Pz3lquKphmyOvMOUyag=; b=V+aFiDmPYqU1zLZvvB+pqPbPc69zVgedSL9NoKkh+vYfwfx2mjc3hC5AnMAquPwzkY ASZxNdkzvNzx1pu0s5dQFHX30jjjRDOwdKqZLX8GuZgkjPKHU/8V+iOJdzwDZCz8lxj8 qMhdMDdsKccHYzRMs6/Wpkwru5BwfP1AOO8bJOlDKFJNxebPdXHvEMNxwIgM4MxrAdTS E0qXGGRYOOL5UvUV/EQBmm/4y8f9fFvONnvfCpkXi1jxRQr1K2MnySAXW9lfojFSmQTt 7ATaV2t8XIcwrkc7IX3cB+OmUT7FINefQkqikxjWvi+4OpSlikcqW5Khabf+HaLIPt0w aRzA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=PcFd6IUUdBB+IhY4iHrqFf/+Pz3lquKphmyOvMOUyag=; b=OeU3sZQhtOdVrCFcr7z7/aRDqpqSS2pdSEmfS3iCQoB5CKwrqfDKRcji2SQ06p53Sx S1H+UBzevanwuJRV1iLCBGLJMBxCsxpeouJkTuQvrkd4FGaXrpT2mFRWv5N/VY+BEblg VoN27KcCbimYd73BoZdIiky/xrN4gRib/Au5qrM1726R3PzNeSjXeZksI0dicUk2JJZo JeVzVgrXRnxWzYjBAyW1R3DXjRT1eHcap/e8R7RJiKIdq06UgyUi8JIouyp/yXhRQyd9 +GjbnoyKdGdvYf0P/g/XmWc5IiAq1g2ZyWdZVvg0Ev3+mCzWksYq1SBV72TeqnAma6w2 nPbg== X-Gm-Message-State: AOAM5303TW9qhXnUQxn3gDK7OJPQGf7qd9vyfeq1Wt1f9Dpu/sZZAR+B awR6p+ys2m/xZ/lTpiUll+AuDlE4IgpeKkdMEPU= X-Google-Smtp-Source: ABdhPJzwaAiqrZvdjGkEZKTwPUp6vAYtcd2s2ZkMxvR/3KO7Of4D6xLblyeXdPRQYWG7y4N/gDY0E6yp4dURmM+vyCs= X-Received: by 2002:a02:3501:: with SMTP id k1mr43380887jaa.133.1593869797050; Sat, 04 Jul 2020 06:36:37 -0700 (PDT) MIME-Version: 1.0 References: <20200427075944.1314-1-pbhagavatula@marvell.com> <4C9E0AB70F954A408CC4ADDBF0F8FA7D4D4BB8CB@BGSMSX101.gar.corp.intel.com> <10969041.qOdnLc68Dm@thomas> <20200525092937.GA891@bricha3-MOBL.ger.corp.intel.com> In-Reply-To: <20200525092937.GA891@bricha3-MOBL.ger.corp.intel.com> From: Jerin Jacob Date: Sat, 4 Jul 2020 19:06:20 +0530 Message-ID: To: Bruce Richardson Cc: Thomas Monjalon , "Varghese, Vipin" , Jerin Jacob Kollanukkaran , "Mcnamara, John" , "Kovacevic, Marko" , Ori Kam , "Nicolau, Radu" , Akhil Goyal , "Kantecki, Tomasz" , Sunil Kumar Kori , dpdk-dev , "Andrzej Ostruszka [C]" , Vamsi Krishna Attunuru , Pavan Nikhilesh Bhagavatula Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v4] examples/l2fwd: add cmdline option for forwarding port info 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 Mon, May 25, 2020 at 2:59 PM Bruce Richardson wrote: > > On Sun, May 24, 2020 at 06:13:22PM +0200, Thomas Monjalon wrote: > > Bruce, as maintainer of l2fwd example, any opinion about this change? > > > Assuming all previous discussion on it is resolved, I'm fine with this > patch, though I suspect it will only make 20.08 now. > > Acked-by: Bruce Richardson Ping for merge. > > > > > 11/05/2020 02:23, Pavan Nikhilesh Bhagavatula: > > > Hi Vipin, > > > > > > >Hi Pavan, > > > > > > > >snipped > > > >> > > > > >> >Should we check & warn the user if > > > >> >1. port speed mismatch > > > >> >2. on different NUMA > > > >> >3. port pairs are physical and vdev like tap, and KNI (performance). > > > >> > > > > >> > > > >> Sure, it can be a separate patch as it will be applicable for multiple > > > >examples. > > > >I believe this patch is for example `l2fwd`. But you would like to have to > > > >updated for all `example`. I am ok for this. > > > > > > > >snipped > > > >> > > > > >> >Should not the check_port_pair be after this? If the port is not > > > >> >enabled in port_mask will you skip that pair? or skip RX-TX from that > > > >port? > > > >> > > > >> We check every port pair against l2fwd_enabled_port_mask in > > > >> check_port_pair_config() > > > > > > > > > > > >> > > > >snipped > > > >> > > > > >> >As mentioned above there can ports in mask which might be > > > >disabled for > > > >> >port pair. Should not that be skipped rather than setting last port rx- > > > >> >tx loopback? > > > >> > > > >> There could be scenarios where user might want to test 2x10G and > > > >1x40G Why > > > >> force the user to explicitly mention 1x40G as port pair of itself in the > > > >portpair > > > >> config? > > > >I am not sure if I follow your thought, as your current port map only > > > >allows `1:1` mapping by `struct port_pair_params`. This can be to self > > > >like `(port0:port0),(port1:port1)` or `(port-0:port-1)`. > > > > > > > >1. But current `l2fwd_parse_port_pair_config` does not consider the > > > >same port mapping as we have hard check for `if (nb_port_pair_params > > > >>= RTE_MAX_ETHPORTS/2)`. > > > > > > > >2. `l2fwd_enabled_port_mask` is global variable of user port mask. This > > > >can contain both valid and invalid mask. Hence we check > > > >`l2fwd_enabled_port_mask & ~((1 << nb_ports) - 1)`. > > > > > > > >3. can these scenarios are true if we invoke `check_port_pair_config` > > > >before actual port_mask check. > > > > a. there are only 4 ports, hence possible mask is `0xf`. > > > > b. user passes port argument as `0xe` > > > > c. `check_port_pair_config` gets masks for `(1,3)` as input and > > > >populates `port_pair_config_mask`. > > > > d. As per the code, port 2 which is valid port and part of user port mask > > > >will have lastport (which is port 3)? May be I did understand the logic > > > >correct. Can you help me? > > > > > > Here user needs to explicitly mention (2,2) for port 2 to be setup else it > > > will be skipped. > > > If you see `check_port_pair_config` below we disable the ports that are not > > > Mentioned in portmap. > > > > > > " > > > check_port_pair_config(void) > > > { > > > > > > > > > port_pair_config_mask |= port_pair_mask; > > > } > > > > > > l2fwd_enabled_port_mask &= port_pair_config_mask; > > > > > > return 0; > > > } > > > " > > > > > > > > > > > > > >So my concerns are 1) there is no same port mapping, 2) my > > > >understanding on lastport logic is not clear and 3) as per the code there > > > >is 1:N but 1:1. > > > > > > > >Hence there should be sufficient warning to user if port are of wrong > > > >speed and NUMA. > > > > > > Unless the user disables stats using -T 0 option all the prints will be skipped. > > > > > > > > > > >Note: current speed can be fetched only if the port are started too (in > > > >Fortville). > > > > > > > >snipped > > > > > > > >