Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:2 as permitted sender) client-ip=2620:137:e000::3:2;
Message-ID: <ba7040d4-337f-56a1-0f87-cc6dd70228ec@intel.com>
Date:   Mon, 25 Sep 2023 10:11:22 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH net-next] iavf: Avoid a memory allocation in
 iavf_print_link_message()
To:     Christophe JAILLET <christophe.jaillet@wanadoo.fr>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>
CC:     <linux-kernel@vger.kernel.org>, <kernel-janitors@vger.kernel.org>,
        <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>
References: <966968bda15a7128a381b589329184dfea3e0548.1695471387.git.christophe.jaillet@wanadoo.fr>
Content-Language: en-US
From:   Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <966968bda15a7128a381b589329184dfea3e0548.1695471387.git.christophe.jaillet@wanadoo.fr>
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?bWM0OW5nTHpNWk1WdDN6UXZFS1IxMEE1bDhIVXo3bXRzZHBURjVZcUpJZm93?=
 =?utf-8?B?QUlYOFdKWitjVzBuUS92dzFoMS8yWG9Wek5lQ0wwNmU2MVlwV3NiREppRlBK?=
 =?utf-8?B?VUtRTlhHQWQ3K2NuZUNXbjcrcGlUU0JUcy82aDBZVlVUcmFFc21kSC9pRVNT?=
 =?utf-8?B?VDlNemtWUWlWNHg4UVJIVDNRaGJaZjFyZm1JNWhkTWxLdGx4WlI5elYvMWhJ?=
 =?utf-8?B?SDh1OGpTRzg5V1BOTFhNYm1FYWYyRDlHbzlRT0ZJVmZBUjRDbWtycEVVZ2Vq?=
 =?utf-8?B?OXN4OWxRelIrT2t5ZUh3UmhYR3V0c3Y3eSs3N0s4bFlGZkRrT01vM0UyS3R4?=
 =?utf-8?B?MklYWnkwQU56WkJOeDk4ekFDZmdpL0l2c3EvWG1tUE1GaFRVcWhXR2tTUkdT?=
 =?utf-8?B?aHJpcmpybFgxdmVKM3FQTGVCWFBxWGVUckhBOC9hTUNKdlh3TExHRURHYWI3?=
 =?utf-8?B?OTh6NzJQemsyM0xIVllvcmJ0Ky9yTXBIa2ROd0pMeU9MSDBpRXA5SW8vbG96?=
 =?utf-8?B?S1ZyY1lGbXJVdy9jT29UMnpXUW0vZk4rU2V1V0NlNXRwdEJGcDBjbEZPT013?=
 =?utf-8?B?aC9BL09VWUROTmovbC9WVjB4RVl4eHUvMENSaHNDb3RqSGtQOVdGdVFvUUJu?=
 =?utf-8?B?UTJocldodGxkZXFNZlVkNVBPdElIZ1NWd0gxY1hReUJOeFJzVG4wRkhMb0cy?=
 =?utf-8?B?a2tvVGJoVC9Fem9ZbXFFc0d6MEZXTENSYUt2RkhTOXpxdHB4ZmdFOFBuMnlx?=
 =?utf-8?B?ejk4ZW5mYmlkcFhkdWFXanh2Ukw2TUEyQ084RStKNW5tQmxsMXVnTlM0V3hn?=
 =?utf-8?B?UUluRzdqVTZCYmJOeVJaNTFmbzNNT002RjBVZDBrMjhEczEwQk1uOFBqVkxP?=
 =?utf-8?B?QUd2eHBheE50dEQ3bXY3T2VPdXJKTzhhWnhtdHRCaVRkNkZrRHpZUFMyNkRr?=
 =?utf-8?B?TTVldWp0cW9UT1N6NjA4cU52N1dMc2NUQnhLYnhIS1pZZUtGTW0zK3hWUE9Q?=
 =?utf-8?B?Zlk0YTB4ekFndzNxVjNFZDhLV1Bxb2tiTHZaOWt1dlVoMGJIV3JrMDhlVWFT?=
 =?utf-8?B?UTg4TzAySWFSdHozbndCM2lDMWNsVnJTNkZ3QXRWQk5xTHhuN3NOOGM2S2I3?=
 =?utf-8?B?SkkzaVNuOTNNOUQ0NUFJcThrSVgrY01zbVRDMXY1blRON0RaMU1nZFM0S3Ny?=
 =?utf-8?B?ejRKNkd4YmpwZkZiTU44MmxjOGJJQUNoMkk3d1UvRlNQNFVOd20xQzZxdTA3?=
 =?utf-8?B?bVJEQU1YYmFNVTZmVzllNGNYdjE5bUZVTEJBZXpISE9ic1RQOTNRZ3pFTlhp?=
 =?utf-8?B?OUpNT1IyM2ZBc1Z4anYzT25OMGVENWRMNjAvdU5mcUx0ZHg0aExkY0k0UnEr?=
 =?utf-8?B?V29XeWtiYmNHSXUzeHRtQW5SMG9aY3d2WDFUV05DemQ1YjI5azhxUEtaNGRE?=
 =?utf-8?B?UmpkanFGMUxwcWdrUHMrTU5mclI5UGdmWkJFeHdqNEVCamxodlJhRnpqOUwz?=
 =?utf-8?B?NGRVVHZBV2JOazM0OUNqdlA3TTZNSEpzSmN1emNEN3drb0tRR2w0NmIvS1FO?=
 =?utf-8?B?cytESVlXWHZ2QTZoY0szc2xhbjdxQ3dCN1pDZHlCTkdocHN4Qmhua1I5Mjhy?=
 =?utf-8?B?RXVwMEFvcHFrM0xYMy9IY1l3aWFnYjZ4L0FQR0dIbEE5dFFwdWZNTUVBVldW?=
 =?utf-8?B?aGxTWXhKalBtWi9vZ0h5ZnFDRHN0Nkthc2FQckV5MW9wT0kwZS9Rc250c0xq?=
 =?utf-8?B?eHZFc2ZCdGdvTkQ2c1BVZHJ3RjBNOVUrVWxlNmNZU1RPQ3VmOGg0VmczNmJs?=
 =?utf-8?B?eFN1L3oyL0pvY0U4Ty9nTno4dFlrNUk4cWovR1NreFNCMG9xSjZPYUwzNGMy?=
 =?utf-8?B?RVJkK3R2UkltSktJNFQ2YnpUYkpCelQrV0RkSFNBS3NFMWJ5d0R2UEQ2MVRN?=
 =?utf-8?B?VzVpaWI0WUlaY0R3d2NUMEU5aFBMb3RVdEdpVlZ3bnhtYVc3bzkrZWdsRUdr?=
 =?utf-8?B?cytaa0VIVVIrb3I4Y1lISm0rVVQzZEN2QW5xTjJ1blZBYUpCME54RmNRRWVJ?=
 =?utf-8?B?N0VXYUdCaG1ubHRFNko0dHFsQnFieVhCV0FPcWM5bnNPd0tLcWVWeWVUQmtv?=
 =?utf-8?B?WFhZais4WFRNTkxrb3VhcDNPUE1heUVjL09Fd2YwT0MvckdIY2ZickhDWWJw?=
 =?utf-8?B?YlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: daa8c91c-7bf7-4762-05f0-08dbbd9f0903
X-MS-Exchange-CrossTenant-AuthSource: BYAPR11MB3672.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Sep 2023 08:11:35.4396
 (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: 0xnNDWDSSACQsXAd3+cBCOw2UH0aDl0wb0o5jUEJC9QXEmlQvgo7xhQGg8urH+PgLnQNLffU/+t45arIdGveur14xZWXtg9QcFuT7siDbJA=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB7639
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.3 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,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 agentk.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 (agentk.vger.email [0.0.0.0]); Mon, 25 Sep 2023 01:14:06 -0700 (PDT)

On 9/23/23 14:17, Christophe JAILLET wrote:
> IAVF_MAX_SPEED_STRLEN is only 13 and 'speed' is allocated and freed within
> iavf_print_link_message().
> 
> 'speed' is only used with some snprintf() and netdev_info() calls.
> 
> So there is no real use to kzalloc()/free() it. Use the stack instead.
> This saves a memory allocation.
> 
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
> ---
>   drivers/net/ethernet/intel/iavf/iavf_virtchnl.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> index 8ce6389b5815..980dc69d7fbe 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_virtchnl.c
> @@ -1389,18 +1389,14 @@ void iavf_disable_vlan_insertion_v2(struct iavf_adapter *adapter, u16 tpid)
>   static void iavf_print_link_message(struct iavf_adapter *adapter)
>   {
>   	struct net_device *netdev = adapter->netdev;
> +	char speed[IAVF_MAX_SPEED_STRLEN];
>   	int link_speed_mbps;
> -	char *speed;
>   
>   	if (!adapter->link_up) {
>   		netdev_info(netdev, "NIC Link is Down\n");
>   		return;
>   	}
>   
> -	speed = kzalloc(IAVF_MAX_SPEED_STRLEN, GFP_KERNEL);
> -	if (!speed)
> -		return;
> -
>   	if (ADV_LINK_SUPPORT(adapter)) {
>   		link_speed_mbps = adapter->link_speed_mbps;
>   		goto print_link_msg;
> @@ -1452,7 +1448,6 @@ static void iavf_print_link_message(struct iavf_adapter *adapter)
>   	}
>   
>   	netdev_info(netdev, "NIC Link is Up Speed is %s Full Duplex\n", speed);
> -	kfree(speed);
>   }
>   
>   /**

Looks fine, thanks!
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

I know that Jesse is fixing snprintf() calls currently,
but I bet it's not conflicting.