Received-SPF: pass (google.com: domain of linux-kernel+bounces-195795-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Message-ID: <6f4874d9-4233-48cd-8ee3-0576068fbc4c@intel.com>
Date: Thu, 30 May 2024 11:38:21 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [net-next PATCH v3] octeontx2-pf: Add ucast filter count
 configurability via devlink.
To: Sai Krishna <saikrishnag@marvell.com>, <davem@davemloft.net>,
	<edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
	<netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	<sgoutham@marvell.com>, <gakula@marvell.com>, <hkelam@marvell.com>,
	<sbhatta@marvell.com>
References: <20240530101515.201635-1-saikrishnag@marvell.com>
Content-Language: en-US
From: Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <20240530101515.201635-1-saikrishnag@marvell.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?Ymo3YWJCbEdsSVRQWlNYQVpCc1l4TGsrYnU0YWUxdDdvNktaT3B0Q1hMd2Za?=
 =?utf-8?B?T2ZrOTRWdG0rbXFmNXQvYkFvOEhkU2ROMlJycitZaCtoYXpzRzEySkZmRVUv?=
 =?utf-8?B?K3Jya1hxWDVDL3NrSW5VVUpRVUVLbG4yWnY4dWNzMEg5R05WUkJGTkZDWCs3?=
 =?utf-8?B?TXR6dG1xbEgrSXZveGpUWExGRUdaUG9YVjRzRjRUQU1KK1VvOWpSV0RLQlcy?=
 =?utf-8?B?eUJyK0lYNVFPcnhhTHd2UGZKdzJVSWYzRUNjb2lMSXRuNWFCWEx0WlVpSWF4?=
 =?utf-8?B?bVNuS3VRK3BKT3JzbGxoMjVlUk9GajcxUjFBZ1dKM1lsN3NremdKQlJ2M2Ri?=
 =?utf-8?B?RHFUWVRNMmpYMVRHSFlXbmg5YXYzUk52VlBpZFh6ajhlcEkvYlR0T1ZpdTEz?=
 =?utf-8?B?eXRoelhEZkJ2L3JmeE9wRXd4L3h2cFB3cjVLSisxTldxVlltZ2t4UjdTWTZK?=
 =?utf-8?B?Q2kzUm12MzF0MTd2aHFvZ2VjcGFkdjkwMXZQaW1CcEZyVzBNOGg2QmZhNC9O?=
 =?utf-8?B?SFVuRFJMNVhORDhBenVjM3h2aWVXWDl4bjNGdXRQaXQvamlMMmlYeFFrZmt1?=
 =?utf-8?B?ZGhqVlRPcU4yMkMyY1NTZzVIWlYzWUJwVkNSWmp6QzM3SVlra2FCNHFKRFBo?=
 =?utf-8?B?OGVTYnRnM0djbEZSTUFDZGtFaEMvRlI3aDRvVzBnSm9yWWxucktXN1FnU0JM?=
 =?utf-8?B?K0xTT0xCcUZOVjB1VkZKZ0gvdS9TN1hacnlVc1duampXSnVlaTZ4VXp1blh5?=
 =?utf-8?B?TGlYZHAvRUJmRFh4UlFGTTUyV1hJL2NxejdUUWw3c1ZoRllBam5jMDJXWG10?=
 =?utf-8?B?MG1MSnp3ZVRQQkVCY1JtSVdjV0ZOU05tRW43YlNLZXk5TndiTURGdTJETlY3?=
 =?utf-8?B?T2FDdXczQ1NaM2c2c2Q4ZHFhMU1haTNCeGR4dDBUY1dUVDZ4VDN0a2U0N1hU?=
 =?utf-8?B?NE5NMWRORW81NktoampUWnhaL252aDMrWE81Tm55S2NDangxcWhocWZ3dXFk?=
 =?utf-8?B?dE4zenBhNHBHM2EzaFg1OXVtTU9IZVN1RjlyNldtSUpJQ0M4NjZrN1Bzbm1W?=
 =?utf-8?B?MmQ3cTJVQy8zNmlVbWN1OG1VWlQ4K2NzUWw1bFRQTjJHdWYrTndwNWJiTEpB?=
 =?utf-8?B?RS8xYWxZdlYzN3ZpWW1LRnh3RWI0alg0Q2h0SWFKZTExV21FN0hMNHBZZ0Jh?=
 =?utf-8?B?c2oyY1RuTWdESmpOa3ZFTWhqbjEzSUl6bkpFc1pQRXdZYTQ1VG1RU1lKRnNn?=
 =?utf-8?B?Yk1mZW1NSUVMZ1NSSWxWc0hSVHY3ZFRMY1c2MlJ5RXJncDNxTS84dGVJa0hm?=
 =?utf-8?B?TUhueU1aUVRCVk1aN2VQZlRjM3lhRnpEcDlEZ0FQMUpJaDViZ3QyVHNOUnVD?=
 =?utf-8?B?d040UXhONUZlZnVmRUFHbGovVzRlV3VReWYreHJwR3JFUzNTSDdBZWJBZEVw?=
 =?utf-8?B?QnJTVzh5bDNGWXBIb1R4RWN5bStzN2p3Vzd3MFNlNWl2czFQZWttQ2pOUE0r?=
 =?utf-8?B?SDluaUozREpiY0d3WERZZ0hTM2pHZGM2VmZPbGdDYUU1RXhZT3dpN3cvV2xD?=
 =?utf-8?B?M2t3RURXUGoyOGVHMVlHRmljTklBUzV3Mzl2SlY5S1cybDNiMzFmS3lScnBJ?=
 =?utf-8?B?dWpIdEQ4SUs5dXF0TU5XdjhIb3l2VTJnb2tnVUp3cVFES0tCN3VSVjQ3MUxC?=
 =?utf-8?B?cFVQTzhDR3ozNnA2aWsybDhxaTUzanVTalMxK2NVSVNnUGVKdU9IQ1VIT0FQ?=
 =?utf-8?B?a2grL3RXaDU1M3F5UmdFNUJOWXdxMzZtRDFPbTk3cDRqK01BZ0d6TEtlMXlh?=
 =?utf-8?B?OWJtN3l4OEhqZER4aXRycmZ3L3FwVEpuaVdDOER2MEVIaDlLYkdWVWJjZ0cr?=
 =?utf-8?B?TkdWREhnUkhTVWZiQ205QTlzWStmUTRWUHhKa0dQUlBzRlZwam0zVlJBblFR?=
 =?utf-8?B?VVN2eGZLSy94WkR2RzhXd1loZ1dsaDRxTTFNNXd2SEVxYWNFUUJVR25JNGtK?=
 =?utf-8?B?ZWVDSzQxeWl3ZWpZa21FMGF6VFI2N216VkRoY3VIOGxGYUdPcEhCZHBjdndV?=
 =?utf-8?B?RCtrVEpqRGlRYUl5ZkRnT1B3RE9pTTRIQlVERHh5TGFHSHRIT25GU0lla2Ez?=
 =?utf-8?B?aHJqVmxYMndNMHJpUkpHR2ZWdE83UjRGTytlRFV6aTg1akNSeHZ3eDF5YXAw?=
 =?utf-8?B?S2c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: fbe6cc67-55c3-45ce-de5e-08dc80d7af38
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 May 2024 18:38:22.6611
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: AvN/tq8K++quRLCCBBsxVfrG6n6t716ouH50CJN5XJxPkNVYWJZ/a0byCrJBA2ZA+9Jbl9b1mtVwPyMSOYWsAVKqVDmFlXkMVzggllb9Jho=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR11MB8455
X-OriginatorOrg: intel.com


On 5/30/2024 3:15 AM, Sai Krishna wrote:
> Added a devlink param to set/modify unicast filter count. Currently
> it's hardcoded with a macro.
> 
> commands:
> 
> To get the current unicast filter count
>  # devlink dev param show pci/0002:02:00.0 name unicast_filter_count
> 
> To change/set the unicast filter count
>  # devlink dev param  set  pci/0002:02:00.0  name unicast_filter_count
>  value 5 cmode runtime
> 

A bit of explanation about why this needs to be configurable would be
useful. What is the impact of lowering or raising this value? Presumably
you need to change the MCAM table size? Lowering this on one port might
enable raising it on another?

It seems reasonable to me, but it is helpful to provide such motivations
in the commit message.

> Signed-off-by: Sai Krishna <saikrishnag@marvell.com>
> ---
> v3:
>     - Addressed review comments given by Jakub Kicinski
>         1. Documented unicast_filter_count devlink param
>         2. Minor change to match upstream code base
> v2:
>     - Addressed review comments given by Simon Horman
> 	1. Updated the commit message with example commads
>         2. Modified/optimized conditions
> 
>  .../networking/devlink/octeontx2.rst          | 16 +++++
>  .../marvell/octeontx2/nic/otx2_common.h       |  7 +-
>  .../marvell/octeontx2/nic/otx2_devlink.c      | 64 +++++++++++++++++++
>  .../marvell/octeontx2/nic/otx2_flows.c        | 20 +++---
>  .../ethernet/marvell/octeontx2/nic/otx2_pf.c  |  2 +-
>  5 files changed, 95 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/networking/devlink/octeontx2.rst b/Documentation/networking/devlink/octeontx2.rst
> index 610de99b728a..5910289b4d4e 100644
> --- a/Documentation/networking/devlink/octeontx2.rst
> +++ b/Documentation/networking/devlink/octeontx2.rst
> @@ -40,3 +40,19 @@ The ``octeontx2 AF`` driver implements the following driver-specific parameters.
>       - runtime
>       - Use to set the quantum which hardware uses for scheduling among transmit queues.
>         Hardware uses weighted DWRR algorithm to schedule among all transmit queues.
> +
> +The ``octeontx2 PF`` driver implements the following driver-specific parameters.
> +
> +.. list-table:: Driver-specific parameters implemented
> +   :widths: 5 5 5 85
> +
> +   * - Name
> +     - Type
> +     - Mode
> +     - Description
> +   * - ``unicast_filter_count``
> +     - u8
> +     - runtime
> +     - Used to Set/modify unicast filter count, which helps in better utilization of
> +       resources against possible wastage(un-used) with current scheme of hardcoded
> +       unicast count.

The text here could be worded a little better. Once the patch is applied
then hard coding is no longer the "current scheme".

I might have worded this like:

"Set the maximum number of unicast filters that can be programmed for
the device. This can be used to achieve better device resource
utilization, avoiding over consumption of unused MCAM table entries."

Or something similar.