Message-ID: <07f5a4cb-2a55-c774-6b23-d3fa6605cb43@nvidia.com>
Date:   Tue, 31 Jan 2023 13:42:53 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.6.1
Subject: Re: [PATCH 1/6] vdpa: fix improper error message when adding vdpa dev
To:     Si-Wei Liu <si-wei.liu@oracle.com>, mst@redhat.com,
        jasowang@redhat.com, parav@nvidia.com
Cc:     virtualization@lists.linux-foundation.org,
        linux-kernel@vger.kernel.org
References: <1675110643-28143-1-git-send-email-si-wei.liu@oracle.com>
 <1675110643-28143-2-git-send-email-si-wei.liu@oracle.com>
Content-Language: en-US
From:   Eli Cohen <elic@nvidia.com>
In-Reply-To: <1675110643-28143-2-git-send-email-si-wei.liu@oracle.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Sm4veWRCdXFLRTZvOG9SL3RJckxYTXNQVjNzV2RsVXhTL2JQYkJvRURSQXZp?=
 =?utf-8?B?d1E2SWNMTHp2L1pSSHdBc1g0R0RMc1ZVdFdXaVlTM2g1dmJGMmFZT0o2N2lt?=
 =?utf-8?B?NktwSDdXMmpSTWcwYnQ4QzdHS2R6NmNWRzczZkZ0VEJJZEF4T0ViZ2dwTGti?=
 =?utf-8?B?cFFlcks2a0IyenZBWDZ0QUpHWjVkUWtWMklrRGFpTE1JZFdlekJwOXpmVWIw?=
 =?utf-8?B?VWhheENKSTladVhqZlo3SWVwekkrdnR0dVFIWkFnMlJCbFNDMHdyVDFpazJv?=
 =?utf-8?B?L0VyQ3luZXZjOW9pYmdESC8ydGxNc0N3SFk5NTRJTEFWT01OdmZTU2FrVk1a?=
 =?utf-8?B?aDdvY2lDaHkxNG54WTlxMUVjNVBKWHoralc2dEtqOWpySkhWbXRVdWMxTFpi?=
 =?utf-8?B?K2tMY0UwaHRNZU1GK0FYYTlWRjFkS3hYd0I1YWVKNmExZ2h5VXF3cmVvT2Iy?=
 =?utf-8?B?S3hvVkV5V2pRUDNoWjI1SWNtN3RNeVg4Q1B3Y01ReTNIZ2laVTdmZXBPTG1k?=
 =?utf-8?B?TGlyNFhPYWhrRjhiQzBlYXg4Q09tNGhZTmNFN1REMnc5WHNzaS9IajRmaHcy?=
 =?utf-8?B?M1FLL2RFMTd1NTNoU3FrYU1MKzlPeGtmOHI0c3RIdFRBdjY0d29GVnVNbkJr?=
 =?utf-8?B?alRPMWtoNUhMRmRhakJyMVRWeHRnZ3cwQk5OQ1V1cnF3SDliazJQZ0pLb3Zw?=
 =?utf-8?B?NU1rSjBvWDdoNEFBZTdFdGRFS2FJRXQ1bCtzV2ZSQTdCMEwzRjhYamNPVndo?=
 =?utf-8?B?NnczNVNHZlZ1dmd5dkZXRTFCejBxcWpiTzROVTBjN25YQ1pjTTQ0TmRnYlh3?=
 =?utf-8?B?UGl5bzhKWUp0UVdqUXFvR3NkVldxTVdpTWhkdHgwdUVtYXBkbG9sNU9TYUJv?=
 =?utf-8?B?eXhaaWs5NHFlTy9FenZCUFdQTEZaZGM3VlNJaHdxb0VGOUpxVkRTMUdjVVQz?=
 =?utf-8?B?ckszcW53VTlVam1MdmF0VUNjdmR4cmVqNXF6MmFxc1pveGp6eVdOU09RNE9h?=
 =?utf-8?B?OC9DVzFzemJnTFBLcDJoVHZ4THFkbmtubDhMWnBySlpjMlNFbVBoSG1ienVV?=
 =?utf-8?B?bTNFanFyM0R5amdxTG9KQ2xwTGM4U3phdXNLSW1KQmdDejdNaGxGUS9SMVBa?=
 =?utf-8?B?K1JDRGlpV0xBN09lemN4TmpRTXFKOXFycVdwMFo1NnUwMnh4WEZCUndHci9F?=
 =?utf-8?B?L3J0enZRYzVTdENNU1M5UkNXOUN4RE5OOUFhUVYwWU04VkczaTZUWXlFOHpi?=
 =?utf-8?B?MDlDUkNLSTR1eXVuUTQ2OEdKcjh1UGM3eTQwYkZ3a1ZsMmE3N09JY3c5K25H?=
 =?utf-8?B?RHBGVVJCc0p4Y3BOVk5rQ0UxUzBLK2l5YjVZYUVSbnB0cEF3SjRjRG1wQXk5?=
 =?utf-8?B?eVhRT3JGRmRlTnVMQzFTVG5PWnZoS3JISmI2djl4eHZGbnI2TkVCUFVIMDdq?=
 =?utf-8?B?U1FCY0VQN3ZMYlRiWnR4ZXd5bzZTMDEzN1NiUi9vamV0ZmxGQldKMnRJQ0Jl?=
 =?utf-8?B?VjBrNkIxL1d4MmYydFZYODR3RnJmNGhJV0xjR0pnVkxqN2FZemlRc1hSYzgx?=
 =?utf-8?B?KzJsdVNsZUdZclB2ODBqY0FFaEJ1ZHdxdyt0elJ3NTk5RFhvMG9jUUNTTTds?=
 =?utf-8?B?Ly9lNkwxbWVQMVhZQ3JYeUZMY2MrMC9vd1RlZ05LM3JTVlc2Z2dVajdUOS9W?=
 =?utf-8?B?bk9GZjVFMkE3Qm1kZThwVU5HRzFIdHo5S0JQaXZ1TkpCdXRsUzIvQm8wRm5i?=
 =?utf-8?B?ZnFnQ0RkTTlDNlV1RFdyU2k2dS84dTFLSmVpKzAvRXd0U1orY050NDNwNTNq?=
 =?utf-8?B?cEhWU1BXeUZmcWllamcwb1pORGUwT1B6QXlKN3FoQTliZlkyMGsvRkthd1Vk?=
 =?utf-8?B?VXdNM3JiZlY4L04yYjd6ekw1M1dOSkFBLzA4WFdDWmhTOFdQY1o3UkU4eGc0?=
 =?utf-8?B?a1ZldHlGdTE4V000QXRFRW9FbmV0SnVBZ2dpNzFzQ09GRTdjMzhkSWNLaHJ3?=
 =?utf-8?B?Q3RyM004VHBGeFprT2YzNXp0Q0pKWTZvYjJoNVUyeUltTFJWK2RWeFFvMGhF?=
 =?utf-8?B?Rno1aFBHZENIa0NQdStwaUlVTk1XM3UyNDhqU2RhSktEQmxOZTRVMUh0T3pk?=
 =?utf-8?Q?Aqaprv67jduU27+r+FI6UaG5K?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2fad7ad6-9d99-45fd-f498-08db03804d58
X-MS-Exchange-CrossTenant-AuthSource: DM8PR12MB5400.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2023 11:42:59.7669
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: fGnXk7hcg497Hbc7kDtT4IHYI8zx/8BpuFlKCnTUjkZUCeQSgRWx1gSWPL9i/R+0
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR12MB7922
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


On 30/01/2023 22:30, Si-Wei Liu wrote:
> In below example, before the fix, mtu attribute is supported
> by the parent mgmtdev, but the error message showing "All
> provided are not supported" is just misleading.
>
> $ vdpa mgmtdev show
> vdpasim_net:
>    supported_classes net
>    max_supported_vqs 3
>    dev_features MTU MAC CTRL_VQ CTRL_MAC_ADDR ANY_LAYOUT VERSION_1 ACCESS_PLATFORM
>
> $ vdpa dev add mgmtdev vdpasim_net name vdpasim0 mtu 5000 max_vqp 2
> Error: vdpa: All provided attributes are not supported.
> kernel answers: Operation not supported
>
> After fix, the relevant error message will be like:
>
> $ vdpa dev add mgmtdev vdpasim_net name vdpasim0 mtu 5000 max_vqp 2
> Error: vdpa: Some provided attributes are not supported.
> kernel answers: Operation not supported
>
> $ vdpa dev add mgmtdev vdpasim_net name vdpasim0 max_vqp 2
> Error: vdpa: All provided attributes are not supported.
> kernel answers: Operation not supported
>
> Signed-off-by: Si-Wei Liu <si-wei.liu@oracle.com>
> Acked-by: Jason Wang <jasowang@redhat.com>
> ---
>   drivers/vdpa/vdpa.c | 9 ++++++++-
>   1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/vdpa/vdpa.c b/drivers/vdpa/vdpa.c
> index 8ef7aa1..5e57935 100644
> --- a/drivers/vdpa/vdpa.c
> +++ b/drivers/vdpa/vdpa.c
> @@ -622,13 +622,20 @@ static int vdpa_nl_cmd_dev_add_set_doit(struct sk_buff *skb, struct genl_info *i
>   		err = PTR_ERR(mdev);
>   		goto err;
>   	}
> -	if ((config.mask & mdev->config_attr_mask) != config.mask) {
> +	if (config.mask && (config.mask & mdev->config_attr_mask) == 0) {
If config.mask is zero than the condition is false even without the 
change so I don't see why is this change needed.
>   		NL_SET_ERR_MSG_MOD(info->extack,
>   				   "All provided attributes are not supported");
>   		err = -EOPNOTSUPP;
>   		goto err;
>   	}
>   
> +	if ((config.mask & mdev->config_attr_mask) != config.mask) {
> +		NL_SET_ERR_MSG_MOD(info->extack,
> +				   "Some provided attributes are not supported");
Changing the message is needed but maybe list the attributes that were 
specified but are not supported?
> +		err = -EOPNOTSUPP;
> +		goto err;
> +	}
> +
>   	err = mdev->ops->dev_add(mdev, name, &config);
>   err:
>   	up_write(&vdpa_dev_lock);