Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
From:   Shinas Rasheed <srasheed@marvell.com>
To:     Simon Horman <horms@kernel.org>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Haseeb Gani <hgani@marvell.com>,
        Vimlesh Kumar <vimleshk@marvell.com>,
        "egallen@redhat.com" <egallen@redhat.com>,
        "mschmidt@redhat.com" <mschmidt@redhat.com>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "wizhao@redhat.com" <wizhao@redhat.com>,
        "kheib@redhat.com" <kheib@redhat.com>,
        "konguyen@redhat.com" <konguyen@redhat.com>,
        Veerasenareddy Burru <vburru@marvell.com>,
        Sathesh B Edara <sedara@marvell.com>,
        Eric Dumazet <edumazet@google.com>
Subject: RE: [EXT] Re: [PATCH net-next v2 1/4] octeon_ep: add PF-VF mailbox
 communication
Thread-Topic: [EXT] Re: [PATCH net-next v2 1/4] octeon_ep: add PF-VF mailbox
 communication
Thread-Index: AQHaKnfPJ2OdgRUHoUGVXSDDjIkBfLCiZZYAgAE8tIA=
Date:   Mon, 11 Dec 2023 06:32:09 +0000
Message-ID: <PH0PR18MB47349A78068BB819B1030BB2C78FA@PH0PR18MB4734.namprd18.prod.outlook.com>
References: <20231209081450.2613561-1-srasheed@marvell.com>
 <20231209081450.2613561-2-srasheed@marvell.com>
 <20231210113732.GF5817@kernel.org>
In-Reply-To: <20231210113732.GF5817@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?KHIoHd6WN9ijw1D0nfk4eMm//nQ+H/0TbaXpP1L1RJF2X9dP50ukJIjuLOw7?=
 =?us-ascii?Q?pV3p/bway0AQPGaYkPc6YCcxHndoapjb+yQmqRqL/ZWIF/O3V3yAfhOmmASf?=
 =?us-ascii?Q?wziIWFKHkEii6f0vh6tFBcmJdFyr5jGjdbaH1pCqqBRIzJtxMB5ObQkuyf2c?=
 =?us-ascii?Q?0nR1eBGNvE2r6lSoLEJRmPC+nzCrH7zNc/w+vDuhecrVO73GWR+yXwP2nZ1c?=
 =?us-ascii?Q?lKRI6dGC8o7EzhyrbW9kmw0KCq0rcmyTXmg7jLZeE7rhz1gsIf9d5SJz3oPm?=
 =?us-ascii?Q?ka+Sd98x237/AHrCa9nsERaMBs+dqtfYJKCsXl9DHsYnYNs8pHd5I7mkbs/l?=
 =?us-ascii?Q?MzCUVlj+NhCl3xzxKDPyk5VN1X02U71e0RycWasJnjRomq4byCQ8O6pskhyM?=
 =?us-ascii?Q?W1QDZ+dNCwjSnEiPNcrCaQSpZUNN1HNva7aWBl9fBmnMZMHOqgR1wmamShUF?=
 =?us-ascii?Q?56T4yrICRxYZIgYG3H44LgYExjhxZSgff0lrogrhggjEuJJlIMYG/0BIxcQt?=
 =?us-ascii?Q?DpRkeeWbfOTnMztidy+oB0NPaKn9NPuIf17uwXplR29zIwpHB9xBDGFnsL20?=
 =?us-ascii?Q?MXTqKKpIOkF16JN+wacVo3ct9VyyPg0eulhB+NHcIzbcTsi7ZsezrLNkw2AX?=
 =?us-ascii?Q?mutSjXmkHBlFTvGjzSP+pJrLW8Tb5SXNVlCnRtvq13HP8Dm6qY9RxIN/ALlr?=
 =?us-ascii?Q?uovH9VvhwqnvgMnbiSC8ZQUlXA9c+4NdAJ6b7W/TFx1LDQLGWpoGOp7dhRQP?=
 =?us-ascii?Q?yLgfbkHJ4zcqHp9okDvdsM5t/I+Pvsvd5rvnDsFeSrG1xQlJgR1EhHj7cFOJ?=
 =?us-ascii?Q?431BMrjCRSzO8R/mRQpyY1yp/I07f/UDZ7DYVSlS3CEZM6ijHj7WT6jYqP1E?=
 =?us-ascii?Q?pNRn5SV4Gd18JIVZKrwKVzbYBvbOL4/9nuGzDXhEaCl6BB7WGkMV13O7+J5s?=
 =?us-ascii?Q?DRPJB+t4AovlfzIOl64Wn9hxyBfaa4GQ+OYrqI/fi0Y0fx1IxIBZ51YC7sy7?=
 =?us-ascii?Q?f81zGSAZRDm9NFVUMZabbYD2NxJzDJUk1+KhNZBAAZWo71257EWeUHGTYjmU?=
 =?us-ascii?Q?+iWlAYZsNUdD6IA1ANdJ7QEgqYlmf/dvNaPY3RkRBI8AuT67G/uQLk/9ticH?=
 =?us-ascii?Q?qvrhYwMJfX/Ro6fjKJmHjYa2YS2I46Ng9wL4i3UqSaBlnM39xtQ4zRlAhz7c?=
 =?us-ascii?Q?NLzjt/oiXiWZMFGT4RoLvio8/bCIBRbi6ZS57gVHAO0tiuEoBbWtmPff0Hyb?=
 =?us-ascii?Q?1kaNe0sK27JTx+Qk2WtLBTEVO3i4cBf80n/c8/1bUyf2ZXOin8L/Ok6dd4rV?=
 =?us-ascii?Q?pSUSNRHT2dQLgdDhsXo5Hk0R9YlSMJhRcevZOvBEXw/981GEY0NET7ikOJrV?=
 =?us-ascii?Q?84aqutOU+b6ROp3gFFlX5IBNMfz0o2KaLEIzrueOF2HnVZyigdMkf4X11wNY?=
 =?us-ascii?Q?QX66zYImXhJc/2VZzNS56l/sghA8b/xJ9ynL9ZkF72vZJ0j0wph3HVy/NaWb?=
 =?us-ascii?Q?rbzeQWL6Z24XYF5V+rYlJBseI4PKMQDLSQ+oB8yulEhebN8fgBuZ6fQzW5Qe?=
 =?us-ascii?Q?cKM32iEf2oac29wgrihJKIRIjJWpz6wjeB8d6MmJ?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: PH0PR18MB4734.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 79008fec-40d8-4740-5608-08dbfa12e712
X-MS-Exchange-CrossTenant-originalarrivaltime: 11 Dec 2023 06:32:09.5773
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: idXKHA/IGkSISrIG05QgeMu9nzIixZoI4BeSCSZTAPVE4AFn1/RCJShvxpwgWZdnZpIwrtjEymYJxxyxakjG5g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR18MB4337
X-Proofpoint-GUID: rs7JGPzu9s30K_Zpn14mbmzE4o5JT5tl
X-Proofpoint-ORIG-GUID: rs7JGPzu9s30K_Zpn14mbmzE4o5JT5tl
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.997,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-12-09_02,2023-12-07_01,2023-05-22_02
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,
	SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on groat.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 (groat.vger.email [0.0.0.0]); Sun, 10 Dec 2023 22:32:43 -0800 (PST)

Hi Simon,

> > +	err =3D octep_setup_pfvf_mbox(octep_dev);
> > +	if (err) {
> > +		dev_err(&pdev->dev, " pfvf mailbox setup failed\n");
> > +		octep_ctrl_net_uninit(octep_dev);
> > +		return err;
>=20
> Hi Shinas,
>=20
> This seems inconsistent with other error handling in this function, I
> suspect it is leaking resources. And even if it does not, it's likely to
> lead to such problems as this function is updated in future.  Please
> consider consistently handling error unwinding using goto labels in this
> function.
>=20
> I think that means either making sure that octep_delete_pfvf_mbox() can
> handle the case whereby octep_setup_pfvf_mbox() fails. Or including) the
> steps taken by octep_device_cleanup() in the unwind ladder provided by
> goto
> labels (which could be a separate patch, and I suspect to be the best
> approach). But I could well be wrong.

You are correct. I'll update this in the next version. Thanks for reviewing=
!

Shinas