Re: [PATCH net-next] net/ncsi: allow to customize BMC MAC Address offset

[Tao Ren]

On 8/8/19 2:16 PM, Andrew Lunn wrote:
> On Thu, Aug 08, 2019 at 07:02:54PM +0000, Tao Ren wrote:
>> Hi Andrew,
>>
>> On 8/8/19 6:32 AM, Andrew Lunn wrote:
>>>> Let me prepare patch v2 using device tree. I'm not sure if standard
>>>> "mac-address" fits this situation because all we need is an offset
>>>> (integer) and BMC MAC is calculated by adding the offset to NIC's
>>>> MAC address. Anyways, let me work out v2 patch we can discuss more
>>>> then.
>>>
>>> Hi Tao
>>>
>>> I don't know BMC terminology. By NICs MAC address, you are referring
>>> to the hosts MAC address? The MAC address the big CPU is using for its
>>> interface?  Where does this NIC get its MAC address from? If the BMCs
>>> bootloader has access to it, it can set the mac-address property in
>>> the device tree.
>>
>> Sorry for the confusion and let me clarify more:
>>
> 
>> The NIC here refers to the Network controller which provide network
>> connectivity for both BMC (via NC-SI) and Host (for example, via
>> PCIe).
>>
> 
>> On Facebook Yamp BMC, BMC sends NCSI_OEM_GET_MAC command (as an
>> ethernet packet) to the Network Controller while bringing up eth0,
>> and the (Broadcom) Network Controller replies with the Base MAC
>> Address reserved for the platform. As for Yamp, Base-MAC and
>> Base-MAC+1 are used by Host (big CPU) and Base-MAC+2 are assigned to
>> BMC. In my opinion, Base MAC and MAC address assignments are
>> controlled by Network Controller, which is transparent to both BMC
>> and Host.
> 
> Hi Tao
> 
> I've not done any work in the BMC field, so thanks for explaining
> this.
> 
> In a typical embedded system, each network interface is assigned a MAC
> address by the vendor. But here, things are different. The BMC SoC
> network interface has not been assigned a MAC address, it needs to ask
> the network controller for its MAC address, and then do some magical
> transformation on the answer to derive a MAC address for
> itself. Correct?

Yes. It's correct.

> It seems like a better design would of been, the BMC sends a
> NCSI_OEM_GET_BMC_MAC and the answer it gets back is the MAC address
> the BMC should use. No magic involved. But i guess it is too late to
> do that now.

Some NCSI Network Controllers support such OEM command (Get Provisioned BMC MAC Address), but unfortunately it's not supported on Yamp.

>> I'm not sure if I understand your suggestion correctly: do you mean
>> we should move the logic (GET_MAC from Network Controller, adding
>> offset and configuring BMC MAC) from kernel to boot loader?
> 
> In general, the kernel is generic. It probably boots on any ARM system
> which is has the needed modules for. The bootloader is often much more
> specific. It might not be fully platform specific, but it will be at
> least specific to the general family of BMC SoCs. If you consider the
> combination of the BMC bootloader and the device tree blob, you have
> something specific to the platform. This magical transformation of
> adding 2 seems to be very platform specific. So having this magic in
> the bootloader+DT seems like the best place to put it.

I understand your concern now. Thank you for the explanation.

> However, how you pass the resulting MAC address to the kernel should
> be as generic as possible. The DT "mac-address" property is very
> generic, many MAC drivers understand it. Using it also allows for
> vendors which actually assign a MAC address to the BMC to pass it to
> the BMC, avoiding all this NCSI_OEM_GET_MAC handshake. Having an API
> which just passing '2' is not generic at all.

After giving it more thought, I'm thinking about adding ncsi dt node with following structure (mac/ncsi similar to mac/mdio/phy):

&mac0 {
    /* MAC properties... */

    use-ncsi;
    ncsi {
        /* ncsi level properties if any */

        package@0 {
            /* package level properties if any */

            channel@0 {
                /* channel level properties if any */

                bmc-mac-offset = <2>;
            };

            channel@1 {
                /* channel #1 properties */
            };
        };

        /* package #1 properties start here.. */
    };
};

The reasons behind this are:

1) mac driver doesn't need to parse "mac-offset" stuff: these ncsi-network-controller specific settings should be parsed in ncsi stack.

2) get_bmc_mac_address command is a channel specific command, and technically people can configure different offset/formula for different channels.

Any concerns or suggestions?


Thanks,

Tao