From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <3chas3@gmail.com> Received: from mail-io0-f169.google.com (mail-io0-f169.google.com [209.85.223.169]) by dpdk.org (Postfix) with ESMTP id 074FF1B1FB for ; Mon, 6 Aug 2018 21:01:16 +0200 (CEST) Received: by mail-io0-f169.google.com with SMTP id k16-v6so11890180iom.12 for ; Mon, 06 Aug 2018 12:01:16 -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=SDkJpizb0O7thHwQMJ3i52uKHpUn1MhYgVynwSMYC2s=; b=oD0aoodHxkOBr8UtdavdjKYbry+JgLKxe3ik8lzHIxCuYzDWSwGE6Ii+7SBB9DnAis HF1WxbZBmyoqsap9BzLrQxNzqzY/z0pLc98Mcny8Q0Yl3bZHdpI1DBQ5dK+vmp4HNMd4 HhttVpRJuPtyQmkwBQwt2CxRxFr/CUlXf3TexCReaF9mwhLm7TsaRRle553qNRNK1pRK 4+Y60p1SYoj1ilOgbxzUDKyhT2fh0HN6ZiVP4CHnJ+IHOzo2bNm6lf15FsaJ4IFMpu1H WFyfUD/N8ODJ7vsZA9WNl8hYijmk94JzSt+jCpCsqcJMQ5Trw7CGYkaVU1WeVYDVSvHy Gshw== 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=SDkJpizb0O7thHwQMJ3i52uKHpUn1MhYgVynwSMYC2s=; b=Y8c90Y5HQESfxF0Z3BUN3QTeDb0swR1bbAqkcvz2Ek0T+U8htSJkxADfUZwXKK7Foa olUPwdMSkmqrutlHLG7dvNZzxJoImqSU4UoXTpxzUj1AuDjRYN98xkJwDI2arlM2caDY b3WowBwvXlv9WwE25/SIhH/GR1hs5f8eJweEINQZBEMJkRK6H9TMbK/aDTvMyjbnO0b5 xlvZLZp+zkfq6Dhqy6aU7rAUxPRqoGOLBD44S+jqRBlK9ay3LEVGyryE7Yuvjaj+X2yk JE8l00gSM7pCneYRWfxWrm/P7zMb67gLVgxEX1v1+V6y+TGROmTPuspdLBcicVlutcD3 SQbQ== X-Gm-Message-State: AOUpUlH4jf4xO9DDa73WfjPZwK0ZnoNkRPH6I6DW2Z2BNWo4kZNH3D/Z t0ny+LzJatMHNc3/qMdg5ZEf08JmHWtsZatfp0o= X-Google-Smtp-Source: AA+uWPzxiMQuUl/4QzYjTBWnnAHviw+BdYr5CNcikghEyQpCqndtEKXcqOP/MADDYuQNYEaijsU9QJSCBUAmtdYr6Ow= X-Received: by 2002:a6b:1e52:: with SMTP id e79-v6mr16455254ioe.110.1533582076355; Mon, 06 Aug 2018 12:01:16 -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: From: Chas Williams <3chas3@gmail.com> Date: Mon, 6 Aug 2018 15:01:05 -0400 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 X-Content-Filtered-By: Mailman/MimeDel 2.1.15 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: Mon, 06 Aug 2018 19:01:17 -0000 On Mon, Aug 6, 2018 at 1:46 PM Matan Azrad wrote: > Hi Chas > > From: Chas Williams > >On Fri, Aug 3, 2018 at 1:47 AM Matan Azrad > wrote: > >Hi Chas > > > > From: Chas Williams [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 th= e > >> > 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 slav= e, > >> > > 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 err= or. > >> > > > >> > > 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, an= d > >> > 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, o= r > >> > 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 > address. > >> > > >> > 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 fa= il > >> > again if the slave didn't support allmulticast and finally just cal= l > >> > 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 select= ed > >> 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 > multicast group, > >> an error should be raised. The only way for this to happen is that yo= u > don't > >> have promisc support which is the ultimate fallback. > > > >> The advantages are: > >> The user knows which option is better to synchronize with his > application. > >> The user knows better than the bond PMD what is the slaves capabilitie= s. > >> 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 > restriction > >> 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 th= e > details. If I am writing > >> an application with this proposed API, I need to make a list of > adapters 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 th= e > bond > >configures promiscuous in mode 4 without the application asking (it's > considered 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 > disagree 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, > promiscuous is greedy! Should be the last alternative to use. > Unfortunately, it's the only option implemented. > > >Providing a list of options only makes life complicated for the develope= r > 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 t= o > 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 > configure the specific LACP MC group it will be great. > Think how much work these applications should do in the current behavior. > The bond PMD should already know how to do that itself. Again, you are forcing more work on the user to ask them to select between the methods. > > > 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 > principle to get a consistent traffic from the slaves. > So, if one uses allmulti and one uses promiscuous the application may get > an inconsistent traffic > and it may trigger a lot of problems and complications for some > applications. > > Those applications should already have those problems. I can make the counter argument that there are potentially applications relying on the broken behavior. 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. > >DPDK already has a promiscuous flag for the PMDs: > > > > RTE_FUNC_PTR_OR_RET(*dev->dev_ops->promiscuous_enable); > > (*dev->dev_ops->promiscuous_enable)(dev); > > dev->data->promiscuous =3D 1; > > > >So the bonding PMD already should be able to tell if it can safely > propagate the enable/disable > >for promiscuous mode. However, for 802.3ad, that is always going to be = a > no until we add > >some other way to subscribe to the multicast group. > > > > > >So, providing to applications a list of options will ease the applicatio= n > life and may be big improvement > >while not hurting the current behavior. > > > >Matan > > >