From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-it0-f43.google.com (mail-it0-f43.google.com [209.85.214.43]) by dpdk.org (Postfix) with ESMTP id A89FF4C91 for ; Tue, 11 Sep 2018 05:31:39 +0200 (CEST) Received: by mail-it0-f43.google.com with SMTP id x79-v6so7763066ita.1 for ; Mon, 10 Sep 2018 20:31:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:sender:from:date:message-id :subject:to:cc:content-transfer-encoding; bh=STMYuMEhCm5wQ5yr0DSkhp7cI4qnBCzxeBScG432vMI=; b=UuEhPfZjeUZxgeo+MdCjZyiEZQ0VOcfbQ8W8c8oSzMTK4FxUjTzMGUyYGzC/PEpd+O LaXDDX6K0pRhEO62Jk/ZW/7APJcGS32yfJpoudm/9xQUS+PgBd9Uo//67hJZpbbyxpdG N2NIIR8towKFSazcWM7r1nwmTfP/F2tOO6X95rEaPHBITieZX/vVxb/iQPJtPKzC38H/ uMIzitAkS8g4kfPJ/NaPeltuGShDv7DHy778RvVnlIDpbD/3jM16OleJ9waKqOFDoiQZ FrAVLXvag+8BK0QJFynt67qB9M2T2jKYUNWyns/qz8Ayqm5DNJodVN8ewjKBD5InsDl+ kxZQ== 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:sender:from :date:message-id:subject:to:cc:content-transfer-encoding; bh=STMYuMEhCm5wQ5yr0DSkhp7cI4qnBCzxeBScG432vMI=; b=f/w2LqciGKumMe3jYsikPFETLtIxeYyNIWN3KDtxace4YUpGivnsHzRa+fmQXd5T3/ bIdtZTQvnPsJ4I5nMNAtT9VTAfSA6IYN/JZPHMRAUuY0xPbmERyeNIcsvoQIlPviYlJM CkNeAIUVyaX7AN3PiGOwnyNZBNzE3llxYLttW9LMq4Fh8g6gp0wdN1IiyiaP5xM9lcZg /nrfNVGNzuffqFUvUTm+XO25Rbiz0W/c8ciL5jK60rM5g9dMKwq22VdboQ1jSstm2n7L nbHdnJZPeJgOHr/Ms/mbw6fSnV5tKAAWg3SOIfV84n4EQcyDdvsDsmrJ5Xq69C7Gi6R8 lWXg== X-Gm-Message-State: APzg51CYPzwa6MwGt+EY73ds47vL0UWMCwM1xL4QU3FoNjO7hozaQb4P GRJPfkm4KVWWr4nnvror5EoUjdDBJj5t4nBwuds= X-Google-Smtp-Source: ANB0VdbQTjG7HVTsgPW5xxTmoEhMuP749gq88t29iSslIi5jf+dhcQYX4ME9MgigQaRPg1SVPxudC4R3dWmhTPim2Ow= X-Received: by 2002:a24:cfd7:: with SMTP id y206-v6mr20202888itf.112.1536636698733; Mon, 10 Sep 2018 20:31:38 -0700 (PDT) MIME-Version: 1.0 References: <1533128278-4685-1-git-send-email-radu.nicolau@intel.com> <2eac631f-1402-67b5-04de-1ce161cfcf92@intel.com> <017918fc-70dc-e6d3-6e9f-35bf9bd73fc3@intel.com> In-Reply-To: Sender: chasmosaurus@gmail.com X-Google-Sender-Delegation: chasmosaurus@gmail.com From: Chas Williams <3chas3@gmail.com> Date: Mon, 10 Sep 2018 23:31:27 -0400 X-Google-Sender-Auth: od7umFpl35KrZF4yIYzpEbVwvhw Message-ID: To: Matan Azrad Cc: Declan Doherty , Radu Nicolau , dev@dpdk.org, Chas Williams Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [PATCH] net/bonding: propagate promiscous mode in mode 4 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: , X-List-Received-Date: Tue, 11 Sep 2018 03:31:41 -0000 On Mon, Aug 6, 2018 at 3:35 PM Matan Azrad wrote: > > > Hi Chas > > From: Chas Williams > >On Mon, Aug 6, 2018 at 1:46 PM Matan Azrad w= rote: > >Hi Chas > > > >From: Chas Williams > >>On Fri, Aug 3, 2018 at 1:47 AM Matan Azrad wrote: > >>Hi Chas > >> > >> From: Chas Williams [mailto:mailto:mailto:mailto:3chas3@gmail.com] On = Thu, Aug 2, 2018 at 1:33 > >>> PM Matan Azrad wrote: > >>> > > >>> > > I suggest to do it like next, > >>> > > To add one more parameter for LACP which means how to configure t= he > >>> > LACP MC group - lacp_mc_grp_conf: > >>> > > 1. rte_flow. > >>> > > 2. flow director. > >>> > > 3. add_mac. > >>> > > 3. set_mc_add_list > >>> > > 4. allmulti > >>> > > 5. promiscuous > >>> > > Maybe more... or less :) > >>> > > > >>> > > By this way the user decides how to do it, if it's fail for a sla= ve, > >>> > > the salve > >>> > should be rejected. > >>> > > Conflict with another configuration(for example calling to > >>> > > promiscuous > >>> > disable while running LACP lacp_mc_grp_conf=3D5) should raise an er= ror. > >>> > > > >>> > > What do you think? > >>> > > > >>> > > >>> > Supporting an LACP mc group specific configuration does make sense, > >>> > but I wonder if this could just be handled by default during slave = add. > >>> > > >>> > > >>> > 1 and 2 are essentially the same hardware filtering offload mode, a= nd > >>> > the other modes are irrelevant if this is enabled, it should not be > >>> > possible to add the slave if the bond is configured for this mode, = or > >>> > possible to change the bond into this mode if an existing slave > >>> > doesn't support it. > >>> > >>> > > >>> > 3 should be the default expected behavior, but > >>> > rte_eth_bond_slave_add() should fail if the slave being added doesn= 't > >>> > support either adding the MAC to the slave or adding the LACP MC ad= dress. > >>> > > >>> > Then the user could try either rte_eth_allmulticast_enable() on the > >>> > bond port and then try to add the slave again, which should fail if > >>> > existing slave didn't support allmulticast or the add slave would f= ail > >>> > again if the slave didn't support allmulticast and finally just ca= ll > >>> > rte_eth_promiscuous_enable() on the bond and then try to re-add the > >>> > that slave. > >>> > > >>> > but maybe having a explicit configuration parameter would be better= . > >>> > >>> I don't sure you understand exactly what I=E2=80=99m suggesting here,= again: > >>> I suggest to add a new parameter to the LACP mode called > >>> lacp_mc_grp_conf(or something else). > >>> So, when the user configures LACP (mode 4) it must to configure the > >>> lacp_mc_grp_conf parameter to one of the options I suggested. > >>> This parameter is not per slave means the bond PMD will use the selec= ted > >>> option to configure the LACP MC group for all the slave ports. > >>> > >>> If one of the slaves doesn't support the selected option it should be= rejected. > >>> Conflicts should rais an error. > >>> > >>> I agree here. Yes, if a slave can't manage to subscribe to the multi= cast group, > >>> an error should be raised. The only way for this to happen is that y= ou don't > >>> have promisc support which is the ultimate fallback. > >> > >>> The advantages are: > >>> The user knows which option is better to synchronize with his applica= tion. > >>> The user knows better than the bond PMD what is the slaves capabiliti= es. > >>> All the slaves are configured by the same way - consistent traffic. > >>> > >>> > >>> It would be ideal if all the slaves would have the same features and > >>> capabilities. There wasn't enforced before, so this would be a new r= estriction > >>> that would be less flexible than what we currently have. That doesn'= t seem like > >>> an improvement. > >> > >>> The bonding user probably doesn't care which mode is used. > >>> The bonding user just wants bonding to work. He doesn't care about t= he details. If I am writing > >>> an application with this proposed API, I need to make a list of adapt= ers and > >>> what they support (and keep this up to date as DPDK evolves). Ugh. > >> > >>The applications commonly know what are the nics capabilities they work= with. > >> > >>I know at least an one big application which really suffering because t= he bond > >>configures promiscuous in mode 4 without the application asking (it's c= onsidered there as a bug in dpdk). > >>I think that providing another option will be better. > >> > >>I think providing another option will be better as well. However we di= sagree on the option. > >>If the PMD has no other way to subscribe the multicast group, it has to= use promiscuous mode. > > > >>Yes, it is true but there are a lot of other and better options, promis= cuous is greedy! Should be the last alternative to use. > > > >Unfortunately, it's the only option implemented. > > Yes, I know, I suggest to change it or at least not to make it worst. > > >>Providing a list of options only makes life complicated for the develop= er and doesn't really > >>make any difference in the end results. > > > >>A big different, for example: > >>Let's say the bonding groups 2 devices that support rte_flow. > >>The user don't want neither promiscuous nor all multicast, he just want= to get it's mac traffic + LACP MC group traffic,(a realistic use case) > >> if he has an option to tell to the bond PMD, please use rte_flow to c= onfigure the specific LACP MC group it will be great. > >>Think how much work these applications should do in the current behavio= r. > > > >The bond PMD should already know how to do that itself. > > The bond can do it with a lot of complexity, but again the user must know= what the bond chose to be synchronized. > So, I think it's better that the user will define it because it is a traf= fic configuration (the same as promiscuous configuration - the user configu= res it) > > Again, you are forcing more work on the user to ask them to select bet= ween the methods. > > We can create a default option as now(promiscuous). > > >> For instance, if the least common denominator between the two PMDs is= promiscuous mode, > >> you are going to be forced to run both in promiscuous mode > >>instead of selecting the best mode for each PMD. > > > >>In this case promiscuous is better, > >>Using a different configuration is worst and against the bonding PMD pr= inciple to get a consistent traffic from the slaves. > >>So, if one uses allmulti and one uses promiscuous the application may g= et an inconsistent traffic > >>and it may trigger a lot of problems and complications for some applica= tions. > > > >Those applications should already have those problems. > > I can make the counter > >argument that there are potentially applications relying on the broken b= ehavior. > > You right. So adding allmulticast will require changes in these applicati= ons. > > >We need to ignore those issues and fix this the "right" way. The "right= " way IMHO > >is the pass the least amount of traffic possible in each case. > > Not in cost of an inconsistency, but looks like we are not agree here. > I have recently run into this issue again with a device that doesn't support promiscuous, but does let me subscribe to the appropriate multicast groups. At this point, I am leaning toward adding another API call to the bonding API so that the user can provide a callback to setup whatever they want on the slaves. The default setup routine would be enable promiscuous. Comments?