Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 19 May 2023 13:54:38 +0200
From:   Simon Horman <simon.horman@corigine.com>
To:     Jiapeng Chong <jiapeng.chong@linux.alibaba.com>
Cc:     jesse.brandeburg@intel.com, anthony.l.nguyen@intel.com,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, intel-wired-lan@lists.osuosl.org,
        netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        Abaci Robot <abaci@linux.alibaba.com>
Subject: Re: [PATCH] iavf: Remove useless else if
Message-ID: <ZGdjfn1anf/DJt1G@corigine.com>
References: <20230519083848.58828-1-jiapeng.chong@linux.alibaba.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230519083848.58828-1-jiapeng.chong@linux.alibaba.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?/S5fWu/iwnlwVsTFmx4GEPXOebGsB012diTMOOMj/clZXx4aIHFc8Ind1cbg?=
 =?us-ascii?Q?BA6eNWVTBV//PnPnmUBloOfRH+nymkjtuaLrwmE9iFk2Bx/2BWcImhqgc17W?=
 =?us-ascii?Q?fVVjWacSujlf4p2G+xk++anN1tfFdqv+ydUGla5C61NvLyd6NnMRwgZORggd?=
 =?us-ascii?Q?NmqWUOwADpUeA7rUi5j08RvCCWNBytDK9uqeQoRET0N3r99fhGM9y1BN/C70?=
 =?us-ascii?Q?4DEsuRFebcxDfd6x0PQVy7pBXGr4vdzhsm3lTlHx3buOl4C6dysyT67Piahd?=
 =?us-ascii?Q?yPDdzOq7X9FBx+zPyqo1lIeBNcJQSdZ0+sIF3KpE79DCuRv5b/8gBsU3Ce1P?=
 =?us-ascii?Q?4xLWFocsKtXOrRJAN+FX+A1Z2tSHGE9dga08I+9idGqdvZR++7obJ8a6rdHM?=
 =?us-ascii?Q?MEZoReeilVTtlJHWi9m4YmroLvFP3Zr9x92NGtnSfAvFxr6EXGu4NEDs7Kai?=
 =?us-ascii?Q?C4dS/LCv5gC6ak1L/M+URzPiRYQjCKtBSENslo710t1qF5BoCthXPruF2sIb?=
 =?us-ascii?Q?PKhEgyBnVNxF2uszYgsrZxJQFN+oFPJNKGBHppOjp1SL5CqE0/e7PFAYFc1B?=
 =?us-ascii?Q?3lkSkEJKQMCzBR5qUor97zbPMXR8mq6mxbDO5Tmr26mpocs4vmjfsiAYWWGo?=
 =?us-ascii?Q?cnFAypAEYP3H8GTyXru4MouwU9/eRp/0id2XtMhJ2jzC6Sduvi5JsLvPm3I3?=
 =?us-ascii?Q?MhnLtiN0biwb3voGcIuQr6eukwCG/ByN7YowpLpg/+3upNLfKL5pWz5ew4bD?=
 =?us-ascii?Q?B2c493V4aclx4RrefXaS+8WbeeZA5yflNMi6sUj1wp8fdGb9CwpHJU8XJoFI?=
 =?us-ascii?Q?oUJYTS+xenZm4ILhNO6T4JcY3stPCiN9FkK8YHYUHBOL2Q/gDtfQhJH4+9f9?=
 =?us-ascii?Q?ySVUaQURu+4hwUMp+qj8HLMW1VnAYpjbdWdLTCxyNj1Atb8PD8pxU/2fkXpv?=
 =?us-ascii?Q?w7eIEdTqO1D8Fj207Q1vSkwMK2/a39FifDJtGmVOGt7LZ9dPELAsKnWRWME9?=
 =?us-ascii?Q?fIkVh07vXVvOdzrvGRAfCQa80+ZMbyIG0/bjN/01tgoKBTYBPmb5qRg3e1AK?=
 =?us-ascii?Q?cQ/lwXzssMl8n2GBjthOI4R8RjoJjWRBQQqF/pKFy7HlCqFyFB6KvpzIkN2o?=
 =?us-ascii?Q?yFnrsFsmtw9/8JCq5cuxqoTb4Sjt2ago474KwLX6j+Bbjo54/YVhgbwwbh01?=
 =?us-ascii?Q?ah8DXj+4XRsyKfy8iniz8nJG0lQ6tTm57sxj7xQ+buhdqxw+zFWG4XETs38E?=
 =?us-ascii?Q?1CEq2Ls7FUkJYezHpMt1M1JxkUAVBZ5Pwgrbxbu7vkl1APESIZUiQF4C6Bec?=
 =?us-ascii?Q?w93TkYBJccja154aXmt8wQY3J94sgQ06GdwoGCY3GulpCGzGVY258lqwPyjj?=
 =?us-ascii?Q?sw2U9krA23VYIsoBX4D9vLZg5Xm/lDQyxQNcFzwvDRQifZxmTLgWXCnCGe2O?=
 =?us-ascii?Q?nnxJHruCE5xNTPu4dp6HppjH2l4eg5Sn5M4rwwojqEPMpW+Ije5j0jMKyIQ/?=
 =?us-ascii?Q?yGYMgF7tpwsemp3qspLVc/MzPo4E2J+BfpWx1qj3x8Hz9VCu18DhkGu8EzCH?=
 =?us-ascii?Q?h5Un8kRgGtahgnv+LYs7d0W8hFFCKxdjR7UC7JJhxXm22Lgnj9rFS5Rl6Lbu?=
 =?us-ascii?Q?hxUSzPPh8T74rXR3IvZ07nVHwRbRUWA1SaT5/L+Hsaab0yGO+ZT9v59urRGZ?=
 =?us-ascii?Q?8YQeGA=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 7bfbee25-2b2a-4b08-09e6-08db585fd5c1
X-MS-Exchange-CrossTenant-AuthSource: PH0PR13MB4842.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 May 2023 11:54:43.5559
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bOkr+t4A6Dg+tdZ1YN8XjzKCLGXrnH5YR1YtqxD2FYlDzkGsFUsiMbH5IoICzGbfLxy0P7IRnhqGbqv+t8ZYtr8REl4rq//TT/rCGSQBUBs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR13MB5285
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=ham autolearn_force=no
        version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Fri, May 19, 2023 at 04:38:48PM +0800, Jiapeng Chong wrote:
> The assignment of the else and if branches is the same, so the if else
> here is redundant, so we remove it.
> 
> ./drivers/net/ethernet/intel/iavf/iavf_main.c:2203:6-8: WARNING: possible condition with no effect (if == else).
> 
> Reported-by: Abaci Robot <abaci@linux.alibaba.com>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=5255
> Signed-off-by: Jiapeng Chong <jiapeng.chong@linux.alibaba.com>

nit: This should be targeted at net-next.

     Subject: [PATCH net-next] ...

> ---
>  drivers/net/ethernet/intel/iavf/iavf_main.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c
> index 2de4baff4c20..cf8538cebfe0 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c
> @@ -2200,8 +2200,6 @@ iavf_set_vlan_offload_features(struct iavf_adapter *adapter,
>  		vlan_ethertype = ETH_P_8021Q;
>  	else if (prev_features & (NETIF_F_HW_VLAN_STAG_RX | NETIF_F_HW_VLAN_STAG_TX))
>  		vlan_ethertype = ETH_P_8021AD;
> -	else if (prev_features & (NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_TX))
> -		vlan_ethertype = ETH_P_8021Q;

I agree that the assignment is the same.
But the current arrangement does very obviously handle
NETIF_F_HW_VLAN_CTAG_TX, whereas in the new arrangement
it's unclear if it is handled or forgotten.

Perhaps a comment is warranted here?
Something like this?

>  	else

		/* This covers both the default and the
		 * (NETIF_F_HW_VLAN_CTAG_RX | NETIF_F_HW_VLAN_CTAG_TX case.
		 */

>  		vlan_ethertype = ETH_P_8021Q;
>  
> -- 
> 2.20.1.7.g153144c
> 
>