Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <00c8d676-0389-47aa-b50a-66a9aa737ff8@intel.com>
Date:   Mon, 23 Oct 2023 17:29:48 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net 2/2] liquidio: Simplify octeon_download_firmware()
Content-Language: en-US
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        <dchickles@marvell.com>, <sburla@marvell.com>,
        <fmanlunas@marvell.com>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <veerasenareddy.burru@cavium.com>
CC:     <felix.manlunas@cavium.com>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <kernel-janitors@vger.kernel.org>
References: <cover.1698007858.git.christophe.jaillet@wanadoo.fr>
 <0278c7dfbc23f78a2d85060369132782f8466090.1698007858.git.christophe.jaillet@wanadoo.fr>
From:   Jacob Keller <jacob.e.keller@intel.com>
In-Reply-To: <0278c7dfbc23f78a2d85060369132782f8466090.1698007858.git.christophe.jaillet@wanadoo.fr>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RDJBWGNyQlBvV2Y3dmpLNGVwUkRlNUd4V29kZTUxUzZyTTVGMkI2ZktMOVU1?=
 =?utf-8?B?RFB1QzBYNzNNT1FvUjZjY3doeVVIVHFZVjZDL2FwcitsbTdEbUJZUW1RQ00r?=
 =?utf-8?B?RkpncXdsVVJSeWtGN3B5TmNjNVUwemN0eXlYRXVTc3IyWktQem5KS3ErTzhZ?=
 =?utf-8?B?M2xGemwzVU1UclhmTUtlTXo3VDlVcU5pOFR5VkNtZnkxMG44alVhaCtXbnRq?=
 =?utf-8?B?Qm1EWm9URi8yU3pqTFpDUHlRa2tEWmJzWXZGNGpRaDYxcmpHc1VRUEhNYXBn?=
 =?utf-8?B?WTBaQTRrcXFseTNEcU44N05nNGFiWHFFbXluWWxGMXNYZnFnbVFBZTZoOVlL?=
 =?utf-8?B?OXoyQnZHeG1Fek5KbDlYWVJMeEZvOXU2bEhsVkFPenZKY1owbFFLbnl0bkt1?=
 =?utf-8?B?Wm9YQ1EwaWNvcXBLOWo0bFljelZIYmVsdGVXSWhaZnFGY0VwQ01mMGRQQXgw?=
 =?utf-8?B?SlJkTWcra3VEM00yTHlvS3lnQ3Ywb1RBUVZ6d3lZMSthS0FrZVZFdjVnQVZ1?=
 =?utf-8?B?NmhCanNYUW5ucVJLTWRmQ2J6SmpWRmxtN2VPNUJkdmJkSTFVSEhJaFhXbG5H?=
 =?utf-8?B?d3lYazNNR2dIQ3E3bDNhWGNyKzJySHRPWlE1N21BR3NsOTd6dnMrVXVqaXE2?=
 =?utf-8?B?RXJHc0xVTTg3OUZWR0gyUlFDOTBTSXFkMWRUVU1CVGx1a3RrdXNRbHpBZGhF?=
 =?utf-8?B?NnNkdi9vZkwzU25zNkZXZGY1bkZEU1R0U3F1Wm1aaTJxb3RESS9FaGQ3T2U3?=
 =?utf-8?B?bEJ0Tmh5RE1qT3E0ZCtHUkU1QXYyRGxqUU5wbXRnQWVaZ1RlUnVHQzQrOElj?=
 =?utf-8?B?NnkyOTZIdDVPdS9TbEZkcS9yOUpwQlBjb1N2L0pSNzFNLzJRU2Zyd1dSbGE1?=
 =?utf-8?B?N0swUnVxMHJGT29KVkR4Vk54V0JBUmdianVyNDZBNVUyZHNCb3J4UGtzUWdJ?=
 =?utf-8?B?dEVtWTI4UzRvSFhGMkNRWk1qOENQcmJuREs4S3dER3dnN1ZJMEV1T3NUdTFQ?=
 =?utf-8?B?TS90ZFdWakpuYkpsdWJjVWJ4KzF6M05NL2N1RE52SkhHUlFCWll2Nm5HYXlR?=
 =?utf-8?B?R1R0RmV1c0lvbXRJa0ppZkZFMUtZZW1Ucm9rZFVDSWoyaXdwNWRLbDJySFpR?=
 =?utf-8?B?TkZyZm0xWDBNd1hJQno3c3dTc25mVll4alVLY3RpWjRXaGdFSnIyQ1N2K21J?=
 =?utf-8?B?WXoxVWRoZmxnWmE2UUxPN0hqem5rTzZMZWxHeVZQbXBwOCtPUzBoUGltaXQx?=
 =?utf-8?B?aCtiNGNxV2JuMjQyZFRqY3AwUTYrVjI3bS9Gay81c3c0cDNUVWFkN3RXRXBR?=
 =?utf-8?B?NG5kUjB4M0tTdlNTa3BlU1VrQmNKRE5zVXpFcDBOMU94c2c4WmFXdER4SUkr?=
 =?utf-8?B?Z2dvVmFML2g4ZHB0bGhKK2xETW10RldrdElDUVJ0RHlpZ1k1Y3ZObUVycUI5?=
 =?utf-8?B?MjJXdU9MSW9XT3ZIRmJxVU9sSTdoa2Q2dldlcmJkWlA2WHpHMFhXeURaS3ow?=
 =?utf-8?B?Vy84Mk5tMUpMbUZOdlZocTJrN2xzc1lqRXZZSnN3SzdtWHFLMFNmeTFQTXVz?=
 =?utf-8?B?bHFpREp6RVo2Z3pLYVpKcmh0T0hwM054L2h3dGhKT2lyWmJ1SWMrSHg5YTJC?=
 =?utf-8?B?ZnhFNDIyM2VWbHFwMG8rbk9EdHZIclVQMGEzOG1iS3ZEaUNDVjdCSEpVeWpM?=
 =?utf-8?B?YjUwTnJKM1pEdXBwbVo3NjlZYTY3OU52QXdhZzZPczVxdG9SWEhXQXFxazdR?=
 =?utf-8?B?V21TMTIzK1FrMGpYNTF4OVByWlRkN1U0cGpIMmo1cTh6aFB5Z29PVC90YXZs?=
 =?utf-8?B?TjQ5SGppN1hGM1AxaHNXdGpEV216THhueDJ1ckdwMkMrMHBjMk5iM1FnN1Nk?=
 =?utf-8?B?ZWhGQVFFTVNlRTBOY3dJK2VhMnp5TVhxWXFmZlVwV0E4RW9jSElTRlFOLzUx?=
 =?utf-8?B?dDVoVmJodmlxdG5RODlzaEdsNnVpZnl5b0ZHSVJ5ZzlVN0dkN01VWHBONWE2?=
 =?utf-8?B?bGVRb1dlSGppeFB2WEVGV1RyaHowd3lrQjl1WXhaVEJlZ2cvQS9UN2VNREpm?=
 =?utf-8?B?dEFJaXR1OTcwaHNvYS9jemw5Vzd4SlpZY3NTbWpQRDF2TE5kN1JPOXVNb3lC?=
 =?utf-8?B?c1BwaXJiUmhKcEpMaFFndG1maWlTRlAwRTdSN2ora20xU2hTUVdHaXRRdlVI?=
 =?utf-8?B?VHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0d8c214f-0f2e-47a4-c794-08dbd4285e26
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Oct 2023 00:30:05.0215
 (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: Ew3uQpAPvegoxpguVbxSGV8SEPdVv+BsYPohObt68PkD4aufc6QI7FYCWrAoXpw4P7p1VS3XyCBkCpK9N3wfgHEJ+3GQbOdbjs7TuAq0vEI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6527
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on howler.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Mon, 23 Oct 2023 17:30:49 -0700 (PDT)


On 10/22/2023 1:59 PM, Christophe JAILLET wrote:
> In order to remove the usage of strncat(), write directly at the rigth
> place in the 'h->bootcmd' array and check if the output is truncated.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
> The goal is to potentially remove the strncat() function from the kernel.
> Their are only few users and most of them use it wrongly.
> 
> This patch is compile tested only.
> ---
>  .../net/ethernet/cavium/liquidio/octeon_console.c   | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_console.c b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
> index bd6baf2872a5..f1f0d7a0309a 100644
> --- a/drivers/net/ethernet/cavium/liquidio/octeon_console.c
> +++ b/drivers/net/ethernet/cavium/liquidio/octeon_console.c
> @@ -802,19 +802,17 @@ static int octeon_console_read(struct octeon_device *oct, u32 console_num,
>  }
>  
>  #define FBUF_SIZE	(4 * 1024 * 1024)
> -#define MAX_BOOTTIME_SIZE    80
>  
>  int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
>  			     size_t size)
>  {
>  	struct octeon_firmware_file_header *h;
> -	char boottime[MAX_BOOTTIME_SIZE];
>  	struct timespec64 ts;
>  	u32 crc32_result;
> +	u32 i, rem, used;
>  	u64 load_addr;
>  	u32 image_len;
>  	int ret = 0;
> -	u32 i, rem;
>  
>  	if (size < sizeof(struct octeon_firmware_file_header)) {
>  		dev_err(&oct->pci_dev->dev, "Firmware file too small (%d < %d).\n",
> @@ -896,16 +894,15 @@ int octeon_download_firmware(struct octeon_device *oct, const u8 *data,
>  	 * Octeon always uses UTC time. so timezone information is not sent.
>  	 */
>  	ktime_get_real_ts64(&ts);
> -	ret = snprintf(boottime, MAX_BOOTTIME_SIZE,
> +
> +	used = strnlen(h->bootcmd, sizeof(h->bootcmd));
> +	ret = snprintf(h->bootcmd + used, sizeof(h->bootcmd) - used,
>  		       " time_sec=%lld time_nsec=%ld",
>  		       (s64)ts.tv_sec, ts.tv_nsec);

Maybe I am missing something here, but why not combine the boottime with
the original write of bootcmd? I guess this is being updated
periodically? The overall solution used here feels like the wrong way to
do it... I guess the bootcmd is setup else where but I couldn't find the
relevant code for that.

What you did seems ok to me, but it still feels like there is a simpler
way to achieve the desired result.

Thanks,
Jake