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 215C8A0C49; Tue, 20 Jul 2021 10:59:34 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0F2AA40E03; Tue, 20 Jul 2021 10:59:34 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id 1B2724068B; Tue, 20 Jul 2021 10:59:33 +0200 (CEST) Received: from [192.168.100.116] (unknown [37.139.99.76]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id CBE267F4DA; Tue, 20 Jul 2021 11:59:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru CBE267F4DA DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1626771572; bh=7rqN9lghzYEcjdTCcQnGgkXAoZ7sNWlq46KZdYcuJr8=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=toR3Cj67yhAiIm/ZklJtES5v6zO+IexSQCXv21eH1ON4jYoBLrRRDJGgYPkUYHKip aijKhxyJWRLXKlHXuondTcCArD0j7toNiaqtBu3+YXAE8jGTb6M45elYQpS1wkReSj 44QY37DxE3Jrnmht0VUSg1cPXqagw7IfmoOizzo0= To: "Xueming(Steven) Li" , Ajit Khaparde , Somnath Kotur , John Daley , Hyong Youb Kim , Beilei Xing , Qiming Yang , Qi Zhang , Haiyue Wang , Matan Azrad , Shahaf Shuler , Slava Ovsiienko , NBU-Contact-Thomas Monjalon , Ferruh Yigit Cc: "dev@dpdk.org" , Viacheslav Galaktionov , "stable@dpdk.org" References: <20210712161747.958019-1-andrew.rybchenko@oktetlabs.ru> <498b2d48-4c04-24f5-12a7-b1da3a55fc51@oktetlabs.ru> <0164acb5-9644-4058-ee4c-8eeead751f58@oktetlabs.ru> From: Andrew Rybchenko Message-ID: Date: Tue, 20 Jul 2021 11:59:31 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] ethdev: fix representor port ID search by name 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 Sender: "dev" On 7/19/21 3:50 PM, Xueming(Steven) Li wrote: > > >> -----Original Message----- >> From: Andrew Rybchenko >> Sent: Monday, July 19, 2021 8:36 PM >> To: Xueming(Steven) Li ; Ajit Khaparde ; Somnath Kotur >> ; John Daley ; Hyong Youb Kim ; Beilei Xing >> ; Qiming Yang ; Qi Zhang ; Haiyue Wang >> ; Matan Azrad ; Shahaf Shuler ; Slava Ovsiienko >> ; NBU-Contact-Thomas Monjalon ; Ferruh Yigit >> Cc: dev@dpdk.org; Viacheslav Galaktionov ; stable@dpdk.org >> Subject: Re: [PATCH] ethdev: fix representor port ID search by name >> >> On 7/19/21 2:54 PM, Xueming(Steven) Li wrote: >>> >>> >>>> -----Original Message----- >>>> From: Andrew Rybchenko >>>> Sent: Monday, July 19, 2021 4:46 PM >>>> To: Xueming(Steven) Li ; Ajit Khaparde >>>> ; Somnath Kotur >>>> ; John Daley ; Hyong >>>> Youb Kim ; Beilei Xing ; >>>> Qiming Yang ; Qi Zhang ; >>>> Haiyue Wang ; Matan Azrad ; >>>> Shahaf Shuler ; Slava Ovsiienko >>>> ; NBU-Contact-Thomas Monjalon >>>> ; Ferruh Yigit >>>> Cc: dev@dpdk.org; Viacheslav Galaktionov >>>> ; stable@dpdk.org >>>> Subject: Re: [PATCH] ethdev: fix representor port ID search by name >>>> >>>> On 7/19/21 9:58 AM, Xueming(Steven) Li wrote: >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: Andrew Rybchenko >>>>>> Sent: Tuesday, July 13, 2021 12:18 AM >>>>>> To: Ajit Khaparde ; Somnath Kotur >>>>>> ; John Daley ; >>>>>> Hyong Youb Kim ; Beilei Xing >>>>>> ; Qiming Yang ; Qi >>>>>> Zhang ; Haiyue Wang ; >>>>>> Matan Azrad ; Shahaf Shuler ; >>>>>> Slava Ovsiienko ; NBU-Contact-Thomas >>>>>> Monjalon ; Ferruh Yigit >>>>>> ; >>>>>> Xueming(Steven) Li >>>>>> Cc: dev@dpdk.org; Viacheslav Galaktionov >>>>>> ; stable@dpdk.org >>>>>> Subject: [PATCH] ethdev: fix representor port ID search by name >>>>>> >>>>>> From: Viacheslav Galaktionov >>>>>> >>>>>> Fix representor port ID search by name if the representor itself >>>>>> does not provide representors info. Getting a list of representors >>>>>> from a representor does not make sense. Instead, a parent device >>>> should be used. >>>>>> >>>>>> To this end, extend the rte_eth_dev_data structure to include the port ID of the parent device for representors. >>>>>> >>>>>> Fixes: df7547a6a2cc ("ethdev: add helper function to get >>>>>> representor >>>>>> ID") >>>>>> Cc: stable@dpdk.org >>>>>> >>>>>> Signed-off-by: Viacheslav Galaktionov >>>>>> >>>>>> Signed-off-by: Andrew Rybchenko >>>>>> --- >>>>>> The new field is added into the hole in rte_eth_dev_data structure. >>>>>> The patch does not change ABI, but extra care is required since ABI >>>>>> check is disabled for the structure because of the libabigail bug >>>> [1]. >>>>>> >>>>>> Potentially it is bad for out-of-tree drivers which implement >>>>>> representors but do not fill in a new parert_port_id field in rte_eth_dev_data structure. Do we care? >>>>>> >>>>>> May be the patch should add lines to release notes, but I'd like to get initial feedback first. >>>>>> >>>>>> mlx5 changes should be reviwed by maintainers very carefully, since we are not sure if we patch it correctly. >>>>>> >>>>>> [1] https://sourceware.org/bugzilla/show_bug.cgi?id=28060 >>>> >>>> [snip] >>>> >>>>>> --- a/lib/ethdev/ethdev_driver.h >>>>>> +++ b/lib/ethdev/ethdev_driver.h >>>>>> @@ -1248,8 +1248,8 @@ struct rte_eth_devargs { >>>>>> * For backward compatibility, if no representor info, direct >>>>>> * map legacy VF (no controller and pf). >>>>>> * >>>>>> - * @param ethdev >>>>>> - * Handle of ethdev port. >>>>>> + * @param parent_port_id >>>>>> + * Port ID of the backing device. >>>>>> * @param type >>>>>> * Representor type. >>>>>> * @param controller >>>>>> @@ -1266,7 +1266,7 @@ struct rte_eth_devargs { >>>>>> */ >>>>>> __rte_internal >>>>>> int >>>>>> -rte_eth_representor_id_get(const struct rte_eth_dev *ethdev, >>>>>> +rte_eth_representor_id_get(uint16_t parent_port_id, >>>>> >>>>> It make more sense to get representor info from parent port. >>>>> Representor is a member of switch domain, PMD owns the information >>>>> of the representor owner port and info of representors. This change >>>>> looks better, but not sure whether it valuable to introduce a new >>>> member to the EAL data structure. >>>> >>>> IMHO, it is simply incorrect to return representors info on a >>>> representor itself. Representor info is an information which representors may be populated using the device. >>>> >>>> If above statement is correct, we need a way to get parent device by >>>> representor to do name to representor ID mapping. I see two options to do it: >>>> A. Dedicated field in rte_eth_dev_data as the patch does. >>>> B. Dedicated ethdev op (since representor knows parent port ID anyway). >>>> We have chosen (A) because of simplicity. >>> >>> Just recalled that representor port could be probed w/o owner PF, is a force for parent port? >> >> I thought that it is impossible and parent port is absolutely required for a representor. Could you provide an example and explain how >> will it work? > > In case of bonding, PF0 and PF1 become one PF port `bond0`, PCI address is PF0. > -a ,representor=pf[0-1]vf[0-99] // this is the syntax we proposed. Is it net/bonding or vendor-specific bonding in HW? If I remember correctly in the case of net/bonding we have ethdev ports for bonded devices. > > To be backward compatible, also support the following 2 devargs: > -a ,representor=[0-99] // probe bond0 and representor on pf0 > -a ,representor=[0-99] // probe representors on pf1. > If devargs start with PF1 devargs, no owner PF1 created as it disabled in bonding. Can't create bond0(PF0) automatically here as > device is located by PCI address(PF1) from devargs. So, I guess the problem is vendor-specific bonding in HW. Anyway legacy backward compatible representor spec should not require representors info since it worked before without it. So, it does not sound like a reason to have representors info on a representor itself.