On Thu, 2021-09-02 at 05:48 +0000, Milton Miller II wrote:
> On August 30, 2021, Ivan Mikhaylov" <i.mikhaylov@xxxxxxxxx> wrote:
> > This patch adds OEM Intel GMA command and response handler for it.
> >
> > /* Get Link Status */
> > struct ncsi_rsp_gls_pkt {
> > struct ncsi_rsp_pkt_hdr rsp; /* Response header */
> > diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
> > index d48374894817..6447a09932f5 100644
> > --- a/net/ncsi/ncsi-rsp.c
> > +++ b/net/ncsi/ncsi-rsp.c
> > @@ -699,9 +699,51 @@ static int ncsi_rsp_handler_oem_bcm(struct
> > ncsi_request *nr)
> > return 0;
> > }
> >
> > +/* Response handler for Intel command Get Mac Address */
> > +static int ncsi_rsp_handler_oem_intel_gma(struct ncsi_request *nr)
> > +{
> > + struct ncsi_dev_priv *ndp = nr->ndp;
> > + struct net_device *ndev = ndp->ndev.dev;
> > + const struct net_device_ops *ops = ndev->netdev_ops;
> > + struct ncsi_rsp_oem_pkt *rsp;
> > + struct sockaddr saddr;
> > + int ret = 0;
> > +
> > + /* Get the response header */
> > + rsp = (struct ncsi_rsp_oem_pkt *)skb_network_header(nr->rsp);
> > +
> > + saddr.sa_family = ndev->type;
> > + ndev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
> > + memcpy(saddr.sa_data, &rsp->data[INTEL_MAC_ADDR_OFFSET], ETH_ALEN);
> > + /* Increase mac address by 1 for BMC's address */
> > + eth_addr_inc((u8 *)saddr.sa_data);
> > + if (!is_valid_ether_addr((const u8 *)saddr.sa_data))
> > + return -ENXIO;
>
> The Intel GMA retireves the MAC address of the host, and the datasheet
> anticipates the BMC will "share" the MAC by stealing specific TCP and
> UDP port traffic destined to the host.
>
> This "add one" allocation of the MAC is therefore a policy, and one that
> is beyond the data sheet.
>
> While this +1 policy may work for some OEM boards, there are other boards
> where the MAC address assigned to the BMC does not follow this pattern,
> but instead the MAC is stored in some platform dependent location obtained
> in a platform specific manner.
>
> I suggest this BMC = ether_addr_inc(GMA) be opt in via a device tree
> property.
>
> as it appears it would be generic to more than one vendor.
>
> Unfortunately, we missed this when we added the broadcom and mellanox
> handlers.
>
>
>
Milton,
maybe something like "mac_addr_inc" or "ncsi,mac_addr_inc"? Also those 3(intel,
mellanox, broadcom) functions even with handlers similar to each other, they
could be unified on idea, difference in addresses, payload lengths, ids only as
I see. Joel proposed in the past about dts option for Intel OEM keep_phy option,
maybe that's the right time to reorganize all OEM related parts to fit in one
direction with dts options for ethernet interface without Kconfig options?
Thanks.
