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 70281A04B3; Mon, 10 Feb 2020 15:51:49 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 410A24C90; Mon, 10 Feb 2020 15:51:48 +0100 (CET) Received: from mail-io1-f67.google.com (mail-io1-f67.google.com [209.85.166.67]) by dpdk.org (Postfix) with ESMTP id A3B064C87 for ; Mon, 10 Feb 2020 15:51:46 +0100 (CET) Received: by mail-io1-f67.google.com with SMTP id z193so7868211iof.1 for ; Mon, 10 Feb 2020 06:51:46 -0800 (PST) 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:content-transfer-encoding; bh=ewMjRYsLNJsremWnQXKrartfId17Wv+8BEJYuGaOrvg=; b=ZF9I5Xlq80Uh93SmqenlxsTSah+UITxxIkk394fUwVinX4IL7smQCTUBNfNHht/OR1 lfUfhEg6fmodtbjLtd73m7Vhb2IjeFhXP3SDOtbjnhhVNWycbn3rWJ0gAO3ifDB+a5Oc GqX/rOeqDRdrLeiyKVcpJ9vFnUmu+Kr9EmsZY7gdHXDaTnpvaJ99VHG+ivacOR3LyKBD vGt1Z9cX8uKjG1JVKwx9O/Mrsv7S/fPqvQKOBpkiOyCpPXaxQsZbUBNQ/iyrRG7o1wpp ssoze3oxEPPfm1w3ZHCjDWQ5lLl9WXFBI7V7abKHErPM14YaY/260a2C9BGnL6BEdKL6 JcPQ== 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:content-transfer-encoding; bh=ewMjRYsLNJsremWnQXKrartfId17Wv+8BEJYuGaOrvg=; b=o5C/ho3Y7gJzqypjV+Sy5kb/tA3A5iofIExuCh0vFscJTfnhhMcv6b+dDafkM0JLzB dLUG4Ao95DXYxW7TbLccHjgegZhfA0QQqgkn+qGO3rGEEBPkJi1U1PlcvtTS+APUmkdd uKmo8Q2/TcnkHiXsstMeb6e7QzmY5Jp1rfbEa2vJYdolV4XhPrQm1wO5I0AmnreJ47ZY tmVXPBu5d+iWAqKXLLt+4jJIGMTWGfQ1LAJcMIJx0Ex1adJZl9RBlJV5NLgGitesrvO6 3cebpsM8MrBT0pZtKlIXdm3cZvYaX//8V67KadpOLstOLDxvql6kBDFusY0eQ8iZ86G2 LqrA== X-Gm-Message-State: APjAAAU+l+vW4+WKkvv7yxYnXEIno0TFBd7Z1fjfIBUa9l8tKtcGNsu9 R1lzmKP7NbINNrOZZDu+Y4j6f65Js20Vzj1N/vM= X-Google-Smtp-Source: APXvYqz9q+wtTvM33xj3jZv2V46+v/mSxLyaKySmp8LfyglvVVNC5zkaxA9pLQmkhxeNGx/N50NnZYc71vtY5vWCHPw= X-Received: by 2002:a02:c995:: with SMTP id b21mr10462643jap.112.1581346305322; Mon, 10 Feb 2020 06:51:45 -0800 (PST) MIME-Version: 1.0 References: <3503128.aV6nBDHxoP@xps> <4205291.LvFx2qVVIh@xps> <790778b3-8d6c-0ec2-79dd-7fb254f021cd@u256.net> In-Reply-To: <790778b3-8d6c-0ec2-79dd-7fb254f021cd@u256.net> From: Jerin Jacob Date: Mon, 10 Feb 2020 20:21:29 +0530 Message-ID: To: Gaetan Rivet Cc: Thomas Monjalon , Pavan Nikhilesh Bhagavatula , Vamsi Krishna Attunuru , David Marchand , dpdk-dev , Jerin Jacob Kollanukkaran , "Richardson, Bruce" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH v7] eal: add manual probing option 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 Tue, Feb 4, 2020 at 9:32 PM Gaetan Rivet wrote: > > On 04/02/2020 16:06, Thomas Monjalon wrote: > > 04/02/2020 13:43, Gaetan Rivet: > >> On 04/02/2020 12:07, Thomas Monjalon wrote: > >>> 04/02/2020 11:03, Gaetan Rivet: > >>>> On 03/02/2020 23:21, Thomas Monjalon wrote: > >>>>> 03/02/2020 06:16, Pavan Nikhilesh Bhagavatula: > >>>>>> @David Marchand @thomas@monjalon.net > >>>>>> > >>>>>> Ping? > >>>>>> > >>>>>> Are there any more changes required for this patch? It's been in q= ueue since last October. > >>>>> > >>>>> Sorry we have not decided whether it is a good idea or not. > >>>>> > >>>>> All changes related to probing are very sensitive, > >>>>> and we know a big refactoring would be better than stacking > >>>>> more and more options and corner cases. > >>>>> > >>>>> As we are busy with ABI stability stuff, we did not allocate > >>>>> enough time to properly think about this feature. > >>>>> Please accept our apologies, and let's consider it as > >>>>> a high priority for 20.05 cycle. > >>>>> > >>>> > >>>> Hello Thomas, > >>>> > >>>> This is unfortunate. I pushed Pavan to accept an alternative impleme= ntation of this functionality that was less obtrusive, to make the integrat= ion smoother. I took care to alleviate those risks from the common path. > >>>> > >>>> The big refactoring is needed yes, but considering the current path = I'm not seeing it happen in 20.05. If that means taking this patch as-is in= 20.05 for Marvell users, I'm not sure much is gained from waiting 3 months= , except minimal risk avoidance. > >>> > >>> > >>> Yes, life is full of bad decisions and consequences. > >> > >> > >> Ah, yes, but I stand by my initial opinion, the first implementation [= 1] was riskier and less useful. > >> > >>> > >>> I still think there is a risk in adding new user expectations, > >>> and maintaining some code to workaround unknown issues. > >>> > >>> The real question here is to know why this patch? > >>> Is it to workaround a broken driver? > >>> Or to workaround a broken design in EAL and bus drivers? > >> > >> Two birds - one stone here: OVS needed a way to disable automatic prob= ing cleanly (current workaround seen in multiple deployment is to add a dum= my whitelisted device, which will be ignored by the PCI bus --> it sets the= bus in whitelist mode but avoid probing anything), and as a bonus this opt= ion allows using devices that depends on other devices being probed already= (LAG, representors, failsafe, etc). > >> > >> I'm not sure having a dependent-probe by default is good, and that wou= ld be a big change. > >> > >> If we are doing the genesis of this patch, the initial motivation shou= ld be asked for more details from Marvell people and David for the OVS side= . > >> > >> [1]: First proposal: > >> http://mails.dpdk.org/archives/dev/2019-September/144166.html > >> My arguments: > >> http://mails.dpdk.org/archives/dev/2019-September/144564.html > > > > > > OK so there are two needs: > > > > 1/ Better control whitelist/blacklist mode. > > We already know that a rework is needed here. > > Unfortunately neither you or me had time to work on it, > > and others who were interested disappeared. > > > > 2/ Associate ports with equivalent properties in applications. > > This must be done in applications. > > Tweaking the probe order is a hack. > > > > > > An application that want to tightly control the port init order, currentl= y (by doing exactly like me here, hotpluging one by one the ports), would s= till need the bigger hack that consist in inserting a whitelist PCI devargs= with a dummy address, depending on a undocumented PCI bus feature consisti= ng in ignoring matching errors but keeping probing policy from failed devar= gs processing. > > Instead, with this patch this app can do > > rte_eal_manual_probe_set(1); > rte_eal_init(); > > to have the same behavior and be able to hotplug ports as it sees fit. > > You are worried about creating user expectations about this behavior (bei= ng forced to replicate in some way the functionality during the rewrite, as= I understand it?), but then you are currently forcing users to expect this= workaround to exist in the PCI bus, blocking devs from touching it as it w= ill thus break current app configurations. I've seen systemd unit file usin= g this -w dummy flag, as well as the programmatic equivalent. Which is bett= er, to have to rework it cutting short these configs, or to propose beforeh= and a deprecation path?. > > This rework won't happen in 20.05, nor in the medium term unless you deci= de to drive this change. This workaround serves three needs (PCI normalizat= ion, port congruence and port dependency) in a low-risk implementation. Thomas, What would be the resolution of this? What is your recommendation to fix the issue as you have the concern of this patch? Issue: 1) When l2fwd does the forwarding for simplicity and performance reason it just xor the port to find the destination port to forward. 2) If the adjacent ports are not symmetrical(example: one is 40G and other 10G) then forwarding will drop the packets. So, either a) We need to control the probing order b) Or Application need 1) To track the symmetrical ports and maintain the forwarding table OR 2) Have the command-line option to specify destination port like l3fwd. We can fix it in the application, but do we need to complicate l2fwd? I am fine with that, if that is consensus. Thoughts? If you think, there is a rework needed in eal then could you enumerate the items for the rework.