From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::441; helo=mail-pf1-x441.google.com; envelope-from=ming.huang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf1-x441.google.com (mail-pf1-x441.google.com [IPv6:2607:f8b0:4864:20::441]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 589FE210E4355 for ; Thu, 9 Aug 2018 07:41:28 -0700 (PDT) Received: by mail-pf1-x441.google.com with SMTP id u24-v6so2933376pfn.13 for ; Thu, 09 Aug 2018 07:41:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=BZGDGdcJhWwOVJ7TNi69aYaTIWbbW+RKIJVvSoEhK/4=; b=MSKC5i9e4CXoJNxzfsdBNkYNC8BL0zXwtWyWTTb66mj1l7CsEbRYEkX2+9wD3eOREh jTMH2zM3qN1Do0uEE6NkVWCpx4TiN7d0oytqZasPQKm6cZdEqx3nMmSitkcbhCbiLQal /N9XSBDVwcUMHV+0BBUW+I3nwmagnrtvyeCYM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=BZGDGdcJhWwOVJ7TNi69aYaTIWbbW+RKIJVvSoEhK/4=; b=HqsW1+is0z5/aT8lhnQLTfvtElRULB3A4d1K8u8tpVM5WEoEqaWNNUwSTVOydqPFqT YTKyupYQ7Lmir0XhDe4zD/ZVJgjfY9IaBuRuV77gWGXGuLd0NFiuyEkM5yS1aKUYpU1+ I3COBEmYZf+G/mefSkvJKRn4rnQT3d58JvetcbgD8RWM2d7dKvpPkMo2ILMNJdS815Ts kiWse4QbG1+fwSbb2wLgBCJ5fqe+WU2RK7rtIcLR3I66xUpiN6FfDY9ZkHqjuKBgUpcq HK5JMQ5SVC6pAudPMbfIUvguD8+6oXfQTygXIDU+pmFwE6YPleUow2py3Z8fcEPKeicB m46A== X-Gm-Message-State: AOUpUlGP7BqoyL4Z6oyCkZNIt70KfkP0cVJF1OR4O/eZPCaI6eCABElo CCmZy4bV4UcqapjAQpjLi3tkhg== X-Google-Smtp-Source: AA+uWPyZcIG1+n4Zqyltfm5g3oAECByOUjftQbHRtgKL1BZkx2y75PQaTzz8vAJd1wzgB366yCJtag== X-Received: by 2002:a62:c288:: with SMTP id w8-v6mr2701926pfk.92.1533825688530; Thu, 09 Aug 2018 07:41:28 -0700 (PDT) Received: from [10.199.0.182] ([64.64.108.224]) by smtp.gmail.com with ESMTPSA id k123-v6sm6929455pga.21.2018.08.09.07.41.18 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 09 Aug 2018 07:41:27 -0700 (PDT) To: Leif Lindholm Cc: linaro-uefi@lists.linaro.org, edk2-devel@lists.01.org, graeme.gregory@linaro.org, ard.biesheuvel@linaro.org, guoheyi@huawei.com, wanghuiqiang@huawei.com, huangming23@huawei.com, zhangjinsong2@huawei.com, huangdaode@hisilicon.com, john.garry@huawei.com, xinliang.liu@linaro.org, Heyi Guo References: <20180724070922.63362-1-ming.huang@linaro.org> <20180724070922.63362-23-ming.huang@linaro.org> <20180803143604.brqho43khtiwbs5i@bivouac.eciton.net> <9300682f-c86e-e9f2-4d25-4455c492a3ca@linaro.org> <20180809101936.q4cqhe6qncnghgtr@bivouac.eciton.net> From: Ming Message-ID: Date: Thu, 9 Aug 2018 22:41:13 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <20180809101936.q4cqhe6qncnghgtr@bivouac.eciton.net> Subject: Re: [PATCH edk2-platforms v1 22/38] Platform/Hisilicon/D06: Add OemNicLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 09 Aug 2018 14:41:29 -0000 Content-Type: text/plain; charset=gbk Content-Transfer-Encoding: 8bit ÔÚ 8/9/2018 6:19 PM, Leif Lindholm дµÀ: > On Thu, Aug 09, 2018 at 02:16:49PM +0800, Ming wrote: >>>> +UINT16 crc_tab[256] = { >>> >>> CrcTable. >> >> Modify it in v2. >> >>> Hmm. >>> This might be useful to add to a core library/driver/protocol. We >>> don't appear to have it yet. This is another note to the universe. >> >> Do you suggest to move the CRC16 function to MdePkg/Library/BaseLib/CheckSum.c ? >> I think it's not enouth time to do this before Linaro 18.08 maybe. > > Not needed for 18.08, but I would like to see the improvement made > afterwards. OK, I will do this after 18.08. > >>>> +EFI_STATUS >>>> +EFIAPI >>>> +OemGetMac ( >>>> + IN OUT EFI_MAC_ADDRESS *Mac, >>>> + IN UINTN Port >>>> + ) >>>> +{ >>>> + EFI_STATUS Status; >>>> + >>>> + if (NULL == Mac) { >>> >>> No jeopardy tests. Turn the comparison around to the logical order. >>> >>>> + DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Mac buffer is null!\n", >>>> + __FUNCTION__, __LINE__)); >>>> + return EFI_INVALID_PARAMETER; >>>> + } >>>> + >>>> + Status = OemGetMacE2prom (Port, Mac->Addr); >>>> + if ((EFI_ERROR (Status))) { >>> >>> Parantheses around EFI_ERROR are not needed. >>> >>>> + DEBUG ((DEBUG_ERROR, >>>> + "[%a]:[%dL] Cannot get MAC from EEPROM, Status: %r; using default MAC.\n", >>>> + __FUNCTION__, __LINE__, Status)); >>>> + >>>> + Mac->Addr[0] = 0x00; >>>> + Mac->Addr[1] = 0x18; >>>> + Mac->Addr[2] = 0x82; >>>> + Mac->Addr[3] = 0x2F; >>>> + Mac->Addr[4] = 0x02; >>>> + Mac->Addr[5] = Port; >>> >>> I'm not super happy about this. This would wreak havoc on any real >>> network. >>> Arguably, a server platform should just fail hard at this point. >>> I would certainly appreciate a Pcd making that an option. >>> >>> Otherwise, some sort of proper scheme would need to be implemented: >>> using the 'locally administered range' of MAC addresses, and ensuring >>> addresses are only allocated after checking for possible duplicates on >>> the network. >> >> Do you suggest we should return EFI_NOT_FOUND here? > > Yes please. > It would be good if we could have some (common) code to handle the > fluke situation where you end up without your own MAC address. > (So that the node can boot up and report that it is broken.) > But it needs to be done in a reliable way, and that's too big a task > for 18.08. I will think about this after 18.08. Thanks. > > / > Leif >