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::442; helo=mail-pf1-x442.google.com; envelope-from=ming.huang@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-pf1-x442.google.com (mail-pf1-x442.google.com [IPv6:2607:f8b0:4864:20::442]) (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 DDC7F210EFAE2 for ; Mon, 13 Aug 2018 19:38:29 -0700 (PDT) Received: by mail-pf1-x442.google.com with SMTP id u24-v6so8541834pfn.13 for ; Mon, 13 Aug 2018 19:38:29 -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=4egXFXnP4Gy2Uz+wlWZfyOLFNDSgzFvMbZG3EEiV7dk=; b=GB3pD7UwT+DZxtT3X3HYE4EuuQKnJ64sqVDOH/reVw7qTms2yxgv4EdikVr6zdW2rd w/o6GiD97N90C0HHs1/IQPNsMaJ5BHscV3ZH7qzBd6V7wsynzLexl9Aurvp6IhFYD6tO Nt7VGNnU+e8miLfeE9uCk6m9Y+5dMbWD99MXo= 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=4egXFXnP4Gy2Uz+wlWZfyOLFNDSgzFvMbZG3EEiV7dk=; b=VGm3XD7oYw6XfRxW3jyGHHvEH5jl7ixl4C2uUC2ChqwItY7emKNKay6wALelIc2yoV iyvIQ04/sWlFeSnRlFQxuPCgdeyygsWLtkFPFgWObvHB9eI0truasQHucMUQy67deSLp pV7DGilUUO/JC6FTghj/qzO8amUfG/AcD5Nd8nTzLNKSQ/uMJBo9+uQteh0fqdF+eitn IR6C2qzK1XuyaCorRSR9YR2ibTMECP25y26wyEYvTlayEBcKUwlv9AlZWQ/QQ+IGyaka qNOIQQdslBtLiW7Z+SvdlOqTIxNxHncCjsqrEILOIf5AC/lVFUYzVqY7kOdT2wInQjZo lO+w== X-Gm-Message-State: AOUpUlF9pzZ4OVFmecGuqFC10GXpwFZxGVD0vyK7MzYTXu3c5HiQlAqd G5aZ6weNprlpcUjAyD4a5dcQQg== X-Google-Smtp-Source: AA+uWPzTbFCq4ONnsh8pOk84JgptksIH65skjoQH9Fez4ISLsJQkae+y/V2NAFiZwVAN+M+0GpSK7A== X-Received: by 2002:a62:b917:: with SMTP id z23-v6mr21440357pfe.131.1534214308886; Mon, 13 Aug 2018 19:38:28 -0700 (PDT) Received: from [10.196.1.150] ([64.64.108.142]) by smtp.gmail.com with ESMTPSA id d10-v6sm17864650pgo.2.2018.08.13.19.38.19 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 13 Aug 2018 19:38:28 -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: <780640eb-44fb-1c44-2cdf-7ea4d38119f8@linaro.org> Date: Tue, 14 Aug 2018 10:38:14 +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: Tue, 14 Aug 2018 02:38:30 -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. > >>>> +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 found some modules which invoke OemGetMac() don't judge the Status of OemGetMac, so it may cause some issue now if changing to EFI_NOT_FOUND. How about change it while we handle the fluke situation after 18.08 ? > > / > Leif >