From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757850AbcFAKzJ (ORCPT ); Wed, 1 Jun 2016 06:55:09 -0400 Received: from mail-bl2on0136.outbound.protection.outlook.com ([65.55.169.136]:25649 "EHLO na01-bl2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750716AbcFAKzG convert rfc822-to-8bit (ORCPT ); Wed, 1 Jun 2016 06:55:06 -0400 From: Yuval Mintz To: Arnd Bergmann , David Miller CC: Manish Chopra , Sudarsana Kalluru , netdev , linux-kernel , Ariel Elior Subject: RE: [PATCH] qed: fix qed_fill_link() error handling Thread-Topic: [PATCH] qed: fix qed_fill_link() error handling Thread-Index: AQHRuopr8dM2Bm5F7kKDXFvV0ks9JJ/RqZeQgAHl6wCAABSOAIAAzT7w Date: Wed, 1 Jun 2016 10:55:02 +0000 Message-ID: References: <1464623197-2084229-1-git-send-email-arnd@arndb.de> <20160531.142046.2026659803711147043.davem@davemloft.net> <5288285.fZuaAytaxX@wuerfel> In-Reply-To: <5288285.fZuaAytaxX@wuerfel> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: arndb.de; dkim=none (message not signed) header.d=none;arndb.de; dmarc=none action=none header.from=qlogic.com; x-originating-ip: [31.168.140.228] x-ms-office365-filtering-correlation-id: 357cffc3-4885-4318-f701-08d38a0b2f2d x-microsoft-exchange-diagnostics: 1;SN1PR11MB0831;5:Ncb+zf/nU4eyrpTV5ZeZJrDoBzmiF1uswYIiJga8yf5BAQ75ErES+NAi2fx1YIVbJKglYZLkyb2UcoacxF0rv1s/6IyGLOs7nX4kUNcGd3cjVB556ZNJaXK6yCEZ9yOt+uU3txAFZsHln4OW0gYwhw==;24:Y7Umk+BVKLNQkJ0/no6T7CBLky1JuQPRud40oLBzRkjMCuwqtiL5rByQR3T+MlH1pHx5vVRIccxqY5FBbvVRxA//gAh/lu3iaWGwRlmh7Xg=;7:UcgqFoxZvUYrAmL9QDxTIKY97hPxhYtp9Idf/GU0FeIy1+5Ik/ltHQdixBwhDrRLWFMFOJD1zIQHwpxG5wh5F5fS7KIQFDcHkWh/+zvNJwiGoHFOtYMhG68AVmDrEarcG65cPLu2Q5mJt4vQK6Vsl9mTCz765Wt6hXSbIoi0FG0r6QNhwu+IEjhXnh1mZ453;20:ymmsCHekUb+4vAAZPgDwPWzU44kjwQRbWjS7VV2igscS2nS7CxDxkGZ+u47gCg9Eo3lHEALcyMvq4BzU2z45YJfnEnrml+ZSvn3x8tOxNTrC79U6L8E8urtk0k/jCWzcnnEDjj54MNTSix84mujUcwsA+tOM1al14ucni1oC0Vk= x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:SN1PR11MB0831; x-ld-processed: 0d68a1f9-1490-4d0e-8767-a87dab3ef2ba,ExtAddr x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001);SRVR:SN1PR11MB0831;BCL:0;PCL:0;RULEID:;SRVR:SN1PR11MB0831; x-forefront-prvs: 096029FF66 x-forefront-antispam-report: SFV:NSPM;SFS:(10019020)(6009001)(51444003)(52314003)(8676002)(81166006)(92566002)(74316001)(76176999)(50986999)(2906002)(54356999)(4326007)(2900100001)(4001430100002)(2950100001)(8936002)(87936001)(77096005)(93886004)(76576001)(3280700002)(106116001)(107886002)(5004730100002)(11100500001)(5008740100001)(33656002)(10400500002)(5002640100001)(9686002)(86362001)(3660700001)(102836003)(3846002)(189998001)(6116002)(122556002)(5001770100001)(586003)(5003600100002)(99286002)(66066001);DIR:OUT;SFP:1102;SCL:1;SRVR:SN1PR11MB0831;H:CO2PR11MB0088.namprd11.prod.outlook.com;FPR:;SPF:None;MLV:sfv;LANG:en; spamdiagnosticoutput: 1:23 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 8BIT MIME-Version: 1.0 X-OriginatorOrg: qlogic.com X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jun 2016 10:55:02.4561 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 0d68a1f9-1490-4d0e-8767-a87dab3ef2ba X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN1PR11MB0831 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > > I think both solutions are equally valid/elegant. > > > > Arnd? > > I think we can just remove the IS_ENABLED() check there and define the > IS_PF() macro conditionally to become 'true' if CONFIG_QED_SRIOV is not set, > like some other drivers do > > diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c > b/drivers/net/ethernet/qlogic/qed/qed_main.c > index 287f61c20c19..756176525cf9 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_main.c > +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c > @@ -1110,7 +1110,7 @@ static int qed_get_link_data(struct qed_hwfn *hwfn, > { > void *p; > > - if (IS_ENABLED(CONFIG_QED_SRIOV) && !IS_PF(hwfn->cdev)) { > + if (!IS_PF(hwfn->cdev)) { > qed_vf_get_link_params(hwfn, params); > qed_vf_get_link_state(hwfn, link); > qed_vf_get_link_caps(hwfn, link_caps); diff --git > a/drivers/net/ethernet/qlogic/qed/qed_sriov.h > b/drivers/net/ethernet/qlogic/qed/qed_sriov.h > index c8667c65e685..c90b2b6ad969 100644 > --- a/drivers/net/ethernet/qlogic/qed/qed_sriov.h > +++ b/drivers/net/ethernet/qlogic/qed/qed_sriov.h > @@ -12,11 +12,13 @@ > #include "qed_vf.h" > #define QED_VF_ARRAY_LENGTH (3) > > +#ifdef CONFIG_QED_SRIOV > #define IS_VF(cdev) ((cdev)->b_is_vf) > #define IS_PF(cdev) (!((cdev)->b_is_vf)) > -#ifdef CONFIG_QED_SRIOV > #define IS_PF_SRIOV(p_hwfn) (!!((p_hwfn)->cdev->p_iov_info)) > #else > +#define IS_VF(cdev) (0) > +#define IS_PF(cdev) (1) > #define IS_PF_SRIOV(p_hwfn) (0) > #endif > #define IS_PF_SRIOV_ALLOC(p_hwfn) (!!((p_hwfn)->pf_iov_info)) > > I don't see why that isn't already the case actually. If this is ok, I'll send an > updated patch. > > For the PF case, we still need to fix the qed_mcp_get_link_params() failure case, > so the rest of my patch is needed anyway, regardless of how we address the > warning. > > Arnd I think that would be unsafe with current qede - qede currently publishes its VFs' PCI device-id as part its MODULE_DEVICE_TABLE, even if CONFIG_QED_SRIOV isn't enabled [might be the wrong thing to do, but that how it goes]. Without changing this, if for some reason we'd have an assigned VF to a VM whose kernel isn't compiled with CONFIG_QED_SRIOV [which is an odd config], that VM is likely to miserably crash.