netdev - Re: [net-next PATCH 07/15] eth: fbnic: allocate a netdevice and napi vectors with queues

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <391f2b71-a811-4536-ac6b-9581fc9457ff@lunn.ch>
Date: Thu, 4 Apr 2024 00:26:04 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Alexander Duyck <alexander.duyck@...il.com>
Cc: netdev@...r.kernel.org, Alexander Duyck <alexanderduyck@...com>,
	kuba@...nel.org, davem@...emloft.net, pabeni@...hat.com
Subject: Re: [net-next PATCH 07/15] eth: fbnic: allocate a netdevice and napi
 vectors with queues

On Wed, Apr 03, 2024 at 03:15:25PM -0700, Alexander Duyck wrote:
> On Wed, Apr 3, 2024 at 1:58 PM Andrew Lunn <andrew@...n.ch> wrote:
> >
> > > +static int fbnic_dsn_to_mac_addr(u64 dsn, char *addr)
> > > +{
> > > +     addr[0] = (dsn >> 56) & 0xFF;
> > > +     addr[1] = (dsn >> 48) & 0xFF;
> > > +     addr[2] = (dsn >> 40) & 0xFF;
> > > +     addr[3] = (dsn >> 16) & 0xFF;
> > > +     addr[4] = (dsn >> 8) & 0xFF;
> > > +     addr[5] = dsn & 0xFF;
> >
> > u64_to_ether_addr() might work here.
> 
> Actually I think it is the opposite byte order. In addition we have to
> skip over bytes 3 and 4 in the center of this as those are just {
> 0xff, 0xff } assuming the DSN is properly formed.

O.K. That is why i used 'might'

> > > +/**
> > > + * fbnic_netdev_register - Initialize general software structures
> > > + * @netdev: Netdev containing structure to initialize and register
> > > + *
> > > + * Initialize the MAC address for the netdev and register it.
> > > + **/
> > > +int fbnic_netdev_register(struct net_device *netdev)
> > > +{
> > > +     struct fbnic_net *fbn = netdev_priv(netdev);
> > > +     struct fbnic_dev *fbd = fbn->fbd;
> > > +     u64 dsn = fbd->dsn;
> > > +     u8 addr[ETH_ALEN];
> > > +     int err;
> > > +
> > > +     err = fbnic_dsn_to_mac_addr(dsn, addr);
> > > +     if (!err) {
> > > +             ether_addr_copy(netdev->perm_addr, addr);
> > > +             eth_hw_addr_set(netdev, addr);
> > > +     } else {
> > > +             dev_err(fbd->dev, "MAC addr %pM invalid\n", addr);
> >
> > Rather than fail, it is more normal to allocate a random MAC address.
> 
> If the MAC address is invalid we are probably looking at an EEPROM
> corruption. If requested we could port over a module parameter we have
> that enables fallback as you are mentioning. However for us it is
> better to default to failing since the MAC address is used to identify
> the system within the datacenter and if it is randomly assigned it
> will make it hard to correctly provision the system anyway.

So maybe add a comment about that.

But i would also expect your DHCP server to be helping out here. If it
gets a request with a MAC it does not know, it could allocate an IP
address from a pool for devices which are FUBAR. You can then at least
ssh into it and try to figure out what has gone wrong?

    Andrew