From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Lendacky Subject: Re: [PATCH net-next v1 1/9] amd-xgbe: Disable VLAN filtering when in promiscuous mode Date: Wed, 17 Feb 2016 10:35:31 -0600 Message-ID: <56C4A153.1020403@amd.com> References: <20160212143224.23973.91104.stgit@tlendack-t1.amdoffice.net> <20160216.201934.2034325737936595643.davem@davemloft.net> <56C49B3E.9030202@amd.com> <20160217.113106.1360496601754038100.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Cc: To: David Miller Return-path: Received: from mail-by2on0092.outbound.protection.outlook.com ([207.46.100.92]:20331 "EHLO na01-by2-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1422903AbcBQQfo (ORCPT ); Wed, 17 Feb 2016 11:35:44 -0500 In-Reply-To: <20160217.113106.1360496601754038100.davem@davemloft.net> Sender: netdev-owner@vger.kernel.org List-ID: On 02/17/2016 10:31 AM, David Miller wrote: > From: Tom Lendacky > Date: Wed, 17 Feb 2016 10:09:34 -0600 > >> On 02/16/2016 07:19 PM, David Miller wrote: >>> From: Tom Lendacky >>> Date: Fri, 12 Feb 2016 08:32:24 -0600 >>> >>>> +#ifndef CRCPOLY_LE >>>> +#define CRCPOLY_LE 0xedb88320 >>>> +#endif >>>> +static u32 xgbe_vid_crc32_le(__le16 vid_le) >>> >>> Please do not implement yet another copy of a CRC implementation. >>> >>> Make use of the various versions the kernel provides already instead, >>> add Kconfig dependencies as needed. >> >> I can't use the existing kernel implementations because there aren't >> any that support the 12-bit length of the VLAN VID (lengths are all >> at the byte level). I created this function (back when I added VLAN >> filtering support in 2014 - it's only being moved in this patch) >> because of this 12-bit length. >> >> I do use the kernel functions in other areas, I just can't use them >> here. > > This crummy ifdef looks terrible though, is it even needed? I probably added it in case the #defines in the crc32defs.h were ever made public. > > Please remove it. Will do. I'll send out a v2 with the ifdef removed. Thanks, Tom >