Received-SPF: pass (google.com: domain of linux-kernel+bounces-104615-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <b4cf355e-8310-422c-8ff8-9e96d7efc9e5@cornelisnetworks.com>
Date: Fri, 15 Mar 2024 12:12:15 -0400
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2] IB/hfi1: allocate dummy net_device dynamically
Content-Language: en-US
To: Breno Leitao <leitao@debian.org>, Jason Gunthorpe <jgg@ziepe.ca>,
 Leon Romanovsky <leon@kernel.org>
Cc: kuba@kernel.org, keescook@chromium.org,
 "open list:HFI1 DRIVER" <linux-rdma@vger.kernel.org>,
 open list <linux-kernel@vger.kernel.org>
References: <20240313103311.2926567-1-leitao@debian.org>
From: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
In-Reply-To: <20240313103311.2926567-1-leitao@debian.org>
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?aGZlaTJ5bm9FWlN5QmFkNDdXYTZlUVg2NVMycEJ1OTRlWWU1TzVzTS82QzlE?=
 =?utf-8?B?QlEvNDFMRDdTcWVMVlVjeTlLdG9odDRiSE14TE43bUJKaXp3L0xjbHdSWUlv?=
 =?utf-8?B?emUrWEh6T2RzaWlxVDRuemI0S0JXdlpKQmpVdTdvTjJpaUR4K3k1alRPRWRP?=
 =?utf-8?B?Tlg5NDV3NlUvb2w0TzFHeXVlZHJDb1RWV1pzd2xPeDJaQkNtVkxNMGs0U2Vm?=
 =?utf-8?B?M3JqOS9HNjd5N08rTWh2ckh2dnErY04wNk1BdllTanJWSkI4Rm5CZC9WRDlh?=
 =?utf-8?B?YjloZHNVbVRxVVBTOW1kZUtCQ0h5WE5ZRzlPdUpKRTFYYjJ0SC84U1pZTmNx?=
 =?utf-8?B?L0RpVnRSWUJXOWp4Y1U0L1drU1JqSkdWNVdQSnF5Q3Q5Y1FBM2M5a3BiaVJB?=
 =?utf-8?B?TVJnb3I3N3BxSFVSaUFOYmZsckVSU3lIWUI4VzJNeHhYS3BXMHNmdXZKa3JB?=
 =?utf-8?B?SXlaOU1CRnFWTXkyY0xIUTNVREpZOFdCVmJvQ2IzREoyQkh0MU9NYzZlbk00?=
 =?utf-8?B?UVNwZHZxbTZEOWFVNDBreitTTU9RVEo4NUZYSXQ3ck5NWFIvelpYK2pEeXBF?=
 =?utf-8?B?Z0FFRU5tbVUyaDZVbnRwdXJQcWhiVENyK3VQNFpDdm5CeTgvTHNVbnBqSC93?=
 =?utf-8?B?STFPb1lsb0ExdXRBN0szMW94bzRCNkU3c081cU5BQit0TFZIWkc3SU1XdWdG?=
 =?utf-8?B?dDA3NFZBWXlKZjNQVTVhY0h1ZmlhVE5xbzdteXlBdE9EN2xwUFljYXhXRTFk?=
 =?utf-8?B?TjVzMEFWOWlRVnIyUlNmZ2ZsY2sxeGFsbmV6Ti8vVFE1RUxIL0JCUVFvY0FW?=
 =?utf-8?B?cktkWFVHb1NwenQ4OVM3TGQ3OTJOZW1hN09oZlhDNERJclBwSDV5aUFoQmlt?=
 =?utf-8?B?eklTaFh3RXJpbnNhM0xxZXJSd2VsUTRQSHVOVFBNTE94ZHVRMTRzYmZBK2FP?=
 =?utf-8?B?MHRkS3BDdmlGcmtVZkhHQTkyQ2cwbDc3VXlVQW80bEc1YVNub0oxUStYZGVN?=
 =?utf-8?B?RFMvbHVZNC8rYWNPUDBOYy8wU3pkcVhDRmJiblprdzIwV0FqQmpmZEh3VnR2?=
 =?utf-8?B?NitVUHVGUHpJeWhzKzhjSjBZbEsvODVOelB0RXJmR1VXMjVpUVpGakd0aTRB?=
 =?utf-8?B?Wk5rakZMQmdUUkNZRElHanNwSHp2NENNcjF0bE5zRUkxaHZJRzFvdEcwTDN5?=
 =?utf-8?B?K0V2REw2YVFYcDZqL0RXMlo1OGlYYVV6bUJRTkZmaDVHbnByeHh2TlVqdW11?=
 =?utf-8?B?QS9qU2owR25vc0YxV3l4b2l5UEJmdGlvaUFCeUtzNnZVRlpVQ0tTbkZFTjE0?=
 =?utf-8?B?cFhMcmNuTllkUmQ0cWw3WFpjbklWc2xFN2NYdk9DZndXUVIvSHRnckczRjFr?=
 =?utf-8?B?OUNZYkR0dHBmbVZSaUVkYmkxbVZlUlhMMkxrcHpiVXBaYzZTckliRm9adVlG?=
 =?utf-8?B?Y1JyREh0K0lzRzc4S2VzMzdRNlhSNmUvemFzOVZsemk5QjUxK3dkN2EvMHl4?=
 =?utf-8?B?SGlWNE9FeHU5NGdBTk0rSTdVTkZqc3RCaEl3anpVUW51TUVXdTgybjFXbTll?=
 =?utf-8?B?OUhUcit4c0NuelNDVjIvR3VQWHV6bitUbktaZjRRZDBmZnhHOFZCc1JsdjJ1?=
 =?utf-8?B?QWowL0NvMG16a0hLbWJ2M0F6anBxU0o5UGVoczNxUjhacGt0SVZNb0JXU3Nj?=
 =?utf-8?B?cmdkU2dHRU5OZjZiWHNTdzFwY0lqa1BWd09keDViT3FWemRrNjBLNktNUjNI?=
 =?utf-8?B?QkhobXZDNWxFY04vaXJIMTBtQ3UyNVoyY3VHMlBBc2VxN04vcU1rTmVrdGhh?=
 =?utf-8?B?cm1vNXBzOWtXZ0hjSkdsS2l3enlsTlBjVS9JdG82NXo0VjUxalBxUHYxbzRq?=
 =?utf-8?B?WXpJREpCVU5na1FlZDN3b0tuSTBIR3VCMjJLT2N6cDU0ZjQybDRBK05zd1pZ?=
 =?utf-8?B?cjNST1JleVpPM1VFME5iYk9RUUZXTXRtWVN1MDlwYlN0Ty9KUDFOcDhJSkZH?=
 =?utf-8?B?VmZpUU9MQkl2bTYyZkRIMGJlbDBsUUlrRVBJODRjcTU3LzlOU1kyeXFrMDZV?=
 =?utf-8?B?MzVWcW1aQzNBd2V0Z2hYRG1VVlcyNVl2b1hxOW5NNEdPU0VFTnlibS9qUkZO?=
 =?utf-8?B?VkJaS2NlK3grNnlYVGlsTXRUUDFWWUlVcWdFVENuZjVpS3RsZzNzQ0VlVHMw?=
 =?utf-8?Q?L8kb4sdItGPCGuhtWFGCRY0=3D?=
X-OriginatorOrg: cornelisnetworks.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 63a06105-ca50-42ba-0329-08dc450aaf0a
X-MS-Exchange-CrossTenant-AuthSource: BL0PR01MB4817.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Mar 2024 16:12:17.0449
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4dbdb7da-74ee-4b45-8747-ef5ce5ebe68a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2Hc0TYWIcoBpuU14JaJl4dQFd7rvC5iMsqvkm/AvgMBbqPxqcYIuGmL01PRf3AIcItr8/bdzoQx3MmdXU3sUpLSPZCueUHkBWypD+fostAqubCWEXblphOBGEVvkmiiO
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CYYPR01MB8565

On 3/13/24 6:33 AM, Breno Leitao wrote:
> struct net_device shouldn't be embedded into any structure, instead,
> the owner should use the priv space to embed their state into net_device.
> 
> Embedding net_device into structures prohibits the usage of flexible
> arrays in the net_device structure. For more details, see the discussion
> at [1].
> 
> Un-embed the net_device from struct hfi1_netdev_rx by converting it
> into a pointer. Then use the leverage alloc_netdev() to allocate the
> net_device object at hfi1_alloc_rx().
> 
> [1] https://lore.kernel.org/all/20240229225910.79e224cf@kernel.org/
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> ----
> PS: this diff needs d160c66cda0ac8614 ("net: Do not return value from
> init_dummy_netdev()") in order to apply and build cleanly.
> ---
> Changelog:
> 
> v2:
> 	* Free struct hfi1_netdev_rx allocation if alloc_netdev() fails
> 	* Pass zero as the private size for alloc_netdev().
> 	* Remove wrong reference for iwl in the comments
> ---

Very lightly tested, but interface came up and I could send traffic. Code seems
OK too.

I'd prefer to at least remove the first sentence of the commit message. It makes
it sound like hfi1 was doing something incorrectly when it was using the
interface as it was designed. Instead it should read more like this is an
enhancement.


Regardless, we can call this one acked.


Acked-by: Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>