From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2EC2C41BB3; Thu, 2 Feb 2023 22:10:51 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B93A840EDC; Thu, 2 Feb 2023 22:10:50 +0100 (CET) Received: from wout2-smtp.messagingengine.com (wout2-smtp.messagingengine.com [64.147.123.25]) by mails.dpdk.org (Postfix) with ESMTP id 1807840693 for ; Thu, 2 Feb 2023 22:10:49 +0100 (CET) Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id 2F605320091B; Thu, 2 Feb 2023 16:10:46 -0500 (EST) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Thu, 02 Feb 2023 16:10:47 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:cc:content-transfer-encoding:content-type:date:date:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to; s=fm3; t=1675372245; x= 1675458645; bh=WFPAgeyb7HRgZ7BkQgiuICzFv2YjkiRbIa3AWBQ40UQ=; b=h BnXz6tScMBCO6SmXzD0zOXUi4gIzUztPcN2z9s1gCkJkwZ7TLF1LESgQBJlILqDZ AeDD02waUDt8X6oBXV3QgbNjHuJYQuwinQ7RvSiKWokD5fYpkJY21wRR0/nUxFe8 XrdOkLcHAjExEURtEtF/X9SNeSAXQGCEALkKUtmSEk9p61zGQMeX0ke37LFh3Zkx F7WS7xVfuhiDfC0lOFhK6bQyxj2ggL3/vTYMw9AdpN5L4aXpeFvekA7SHDIuEQ7F Fcaz3fSOUIhVUMc8QbSaD1dDCSVOuXPWnpamgSEYMBRVAsRZZuzWUFiss6GJTzB3 M0q7Rm0J5ufQCQXni9Aag== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:date:date:feedback-id:feedback-id:from:from :in-reply-to:in-reply-to:message-id:mime-version:references :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t=1675372245; x= 1675458645; bh=WFPAgeyb7HRgZ7BkQgiuICzFv2YjkiRbIa3AWBQ40UQ=; b=I pSWdoCoeDllInVvDlJ9Au8iTIFXS2+vrTgrtnS16h3F3Zluz5t2+iE35wzE1Rl4r QOGQeotM4EsYdS4fQ2ZQfKrcTqXUqPov9VarCrRnBfdtYMWJ4eVVIsmTCXbLfgud FvAor1gS7yI5tTgGcZFcXV2xYdN6a/5fJxT6fE2lgoaRP0pCO61U9kRpVzzyIqJs FdLA/LABOzNGnLAScOALa9GUWZIGrBJL2j9q4yCYMT7wfNuA1fmJCsKigOzrIJQk j9q+M4BF+62jiYlXVjUFzi9EgA1pFubLw6m01XXYXrKh4XECoblqifJTsBk2h8y0 oKyBSZn0sc26S65v+FE/g== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrudefkedgudegvdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpefhvfevufffkfgjfhgggfgtsehtufertddttddvnecuhfhrohhmpefvhhho mhgrshcuofhonhhjrghlohhnuceothhhohhmrghssehmohhnjhgrlhhonhdrnhgvtheqne cuggftrfgrthhtvghrnheptdejieeifeehtdffgfdvleetueeffeehueejgfeuteeftddt ieekgfekudehtdfgnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilh hfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: Feedback-ID: i47234305:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 2 Feb 2023 16:10:43 -0500 (EST) From: Thomas Monjalon To: Huisong Li , Ferruh Yigit Cc: dev@dpdk.org, andrew.rybchenko@oktetlabs.ru, liudongdong3@huawei.com, huangdaode@huawei.com, fengchengwen@huawei.com, jerinj@marvell.com, bruce.richardson@intel.com, maxime.coquelin@redhat.com, david.marchand@redhat.com, Morten =?ISO-8859-1?Q?Br=F8rup?= Subject: Re: [PATCH V8] ethdev: fix one address occupies two entries in MAC addrs Date: Thu, 02 Feb 2023 22:10:41 +0100 Message-ID: <6754145.G0QQBjFxQf@thomas> In-Reply-To: <9b816855-2b5e-4296-d954-aa23cbd97a4c@amd.com> References: <20221020093102.20679-1-lihuisong@huawei.com> <20230202123625.14975-1-lihuisong@huawei.com> <9b816855-2b5e-4296-d954-aa23cbd97a4c@amd.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org 02/02/2023 19:09, Ferruh Yigit: > On 2/2/2023 12:36 PM, Huisong Li wrote: > > The dev->data->mac_addrs[0] will be changed to a new MAC address when > > applications modify the default MAC address by .mac_addr_set(). However, > > if the new default one has been added as a non-default MAC address by > > .mac_addr_add(), the .mac_addr_set() doesn't remove it from the mac_addrs > > list. As a result, one MAC address occupies two entries in the list. Like: > > add(MAC1) > > add(MAC2) > > add(MAC3) > > add(MAC4) > > set_default(MAC3) > > default=MAC3, the rest of the list=MAC1, MAC2, MAC3, MAC4 > > Note: MAC3 occupies two entries. > > > > In addition, some PMDs, such as i40e, ice, hns3 and so on, do remove the > > old default MAC when set default MAC. If user continues to do > > set_default(MAC5), and the mac_addrs list is default=MAC5, filters=(MAC1, > > MAC2, MAC3, MAC4). At this moment, user can still see MAC3 from the list, > > but packets with MAC3 aren't actually received by the PMD. > > > > So need to ensure that the new default address is removed from the rest of > > the list if the address was already in the list. > > > > Same comment from past seems already valid, I am not looking to the set > for a while, sorry if this is already discussed and decided, > if not, I am referring to the side effect that setting MAC addresses > cause to remove MAC addresses, think following case: > > add(MAC1) -> MAC1 > add(MAC2) -> MAC1, MAC2 > add(MAC3) -> MAC1, MAC2, MAC3 > add(MAC4) -> MAC1, MAC2, MAC3, MAC4 > set(MAC3) -> MAC3, MAC2, MAC4 > set(MAC4) -> MAC4, MAC2 > set(MAC2) -> MAC2 > > I am not exactly clear what is the intention with set(), That's the problem, nobody is clear with the current behavior. The doc says "Set the default MAC address." and nothing else. > if there is > single MAC I guess intention is to replace it with new one, but if there > are multiple MACs and one of them are already in the list intention may > be just to change the default MAC. The assumption in this patch is that "Set" means "Replace", not "Swap". So this patch takes the approach 1/ Replace and keep Unique. > If above assumption is correct, what about following: > > set(MAC) { > if only_default_mac_exist > replace_default_mac > > if MAC exists in list > swap MAC and list[0] > else > replace_default_mac > } This approach 2/ is a mix of Swap and Replace. The old default MAC destiny depends on whether we have added the new MAC as "secondary" before setting as new default. > This swap prevents removing MAC side affect, does it make sense? Another approach would be 3/ to do an "Always Swap" even if the new MAC didn't exist before, you keep the old default MAC as a secondary MAC. And the current approach 0/ is to Replace default MAC address without touching the secondary addresses at all. So we have 4 choices. We could vote, roll a dice, or find a strong argument?