adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.08k stars 1.2k forks source link

Wiznet Ethernet Featherwing Addresses Unstable in CPy #2274

Closed phrogger closed 3 years ago

phrogger commented 4 years ago

Although the Wiznet Ethernet Featherwing, is shipped with a unique Adafruit MAC address, there is no way to use it or enter it with Circuit Python. As a result, the Wiznet chip makes up a random MAC address every time it is initialized, and gets a different DHCP assigned IP address, hopping up and down the user's network address space.

The initialization of the wiznet.WIZNET5K() class should provide for a parameter of the desired MAC address, to be loaded into the Wiznet chip before the DHCP process is started. This exists in the Arduino implementation, but not Circuit Python.

There are multiple network side effects and potential problems from this kind of random MAC address hopping.

brentru commented 4 years ago

@phrogger Were you observing a similar error as this issue? https://github.com/adafruit/circuitpython/issues/2073

phrogger commented 4 years ago

@brentru I don't think they are the same problem. The issue I am reporting is that the Wiznet 5500 has no on-board long term memory for a MAC address, so if you don't give it one after boot, it makes up a random number for the MAC, and causes an IP address change every time you reboot. The Arduino API has a function to assign/force a MAC address. The CP API does not. As far as I know, it is stable as long as power is still applied, so should not cause this issue. I didn't go beyond discovering it was hopping IP address every power cycle, and I put it back in the desk drawer. --- graham (discord), aka phrogger(github)

nickzoic commented 4 years ago

Yeah, that's correct. The implementation as it is makes up a random MAC (in the appropriate range) but if there's an Adafruit-assigned MAC serial number or something we could change it to use that.

It'd also be easy enough to add an interface to set the MAC.

(check out https://github.com/adafruit/circuitpython/blob/d9c808d93886cbe071b4e1acd3730cf508dac3df/shared-module/wiznet/wiznet5k.c#L382 and https://github.com/adafruit/circuitpython/blob/d9c808d93886cbe071b4e1acd3730cf508dac3df/shared-module/network/__init__.c#L90 for details)

phrogger commented 4 years ago

@nickzoic Thanks for the pointer about RMII on ESP32. I have read about some people doing that. In my case, I just switched to BBB for my application, which seems to have a well behaved network interface.

nickzoic commented 4 years ago

On Thu, 28 Nov 2019, at 10:54, Graham wrote:

@nickzoic https://github.com/nickzoic Thanks for the pointer about RMII on ESP32. I have read about some people doing that. In my case, I just switched to BBB for my application, which seems to have a well behaved network interface.

No worries: it's a bit of a fringe application but maybe handy for someone. My feeling is if you've got wired ethernet, you've probably got wired power (at least PoE) and therefore you probably can get away with a 'heavier' CPU (beaglebone, rpi, etc) anyway.

ssube commented 4 years ago

While the MAC is still changing every power cycle, calling ifconfig appears to correctly set the IP address now (v4.1.2):

# set up networking
spi = busio.SPI(clock=board.SCK, MOSI=board.MOSI, MISO=board.MISO)

eth = wiznet.WIZNET5K(spi, board.D10, board.D11)
eth.ifconfig(('10.2.1.220', '255.255.255.0', '10.2.1.1', '10.2.1.1'))

I didn't need to explicitly disable DHCP, and the correct IP shows up in eth.ifconfig() calls after a few seconds.

nickzoic commented 4 years ago

Yeah, the MAC thing is interesting. The random MAC function works for me, but doesn't match a lot of other people's use cases by the looks, eg: people who want to have DHCP set a stable IP.

@tannewt et al what do you think of adding: a) a configuration method to get/set the MAC (eth.macaddr(b'\xaa\xbb\xcc\xdd\xee\xff'); print(eth.macaddr())) and/or b) stashing the MAC to NVRAM somewhere so it is persistent once chosen? (the NVRAM is on the SoC not the W5500 so we'd have to work out how that fits in on various platforms though ... are there other things which require persistence at the moment?)

dhalbert commented 4 years ago

@nickzoic I think we should provide some way to set the address. But I was thinking it should be an optional argument the constructor so doesn't change from the very start.

We have 256 bytes of internal config storage on atmel-samd; right now a small fraction of that is used to store USB clock calibration info. On nrf we have 32kB of flash used for bonding information. In both of those cases that storage is used by the underlying native modules, and is not directly exposed to the user. But the MAC address is more "public", the user could store in some file in the file system, in the program code itself, or in microcontroller.nvm as well. I'm not sure it needs to be in a special reserved area, though we could do that.

nickzoic commented 4 years ago

Hey Dan! Happy New Year!

I was thinking that for 99% of users it's something they never want to think about so long as it doesn't change, so it'd make sense to stash six bytes into "hidden" non-volatile storage (eg: not the parts exposed by the filesystem or microcontroller.nvm) ...

dhalbert commented 4 years ago

Suppose we use the burned in serial number (available as microcontroller.cpu.uid) to generate the MAC address. It's meant to be unique, so I think it will serve the purpose well. There could still be a way to set it in the constructor if the user so desired, but we don't have to add that now.

P.S. Happy New Year to you too!

nickzoic commented 4 years ago

Yeah, good point. I guess rather than picking an actual random MAC we could hash the uid down to 46 bits and the odds of collision are still really rather low. Elegant.

(see also: #462)

dhalbert commented 4 years ago

Re hashing: I don't have much experience with this, but I read some stackoverflow answer which pointed to one of the functions mentioned here: https://en.wikipedia.org/wiki/List_of_hash_functions#Non-cryptographic_hash_functions

nickzoic commented 4 years ago

On Wed, 22 Jan 2020, at 10:07, Dan Halbert wrote:

Re hashing: I don't have much experience with this, but I read some stackoverflow answer which pointed to one of the functions mentioned here: https://en.wikipedia.org/wiki/List_of_hash_functions#Non-cryptographic_hash_functions

Normally I'd just use N bits out of an SHA1 from axtls, which is pulled into MicroPython to support uhashlib ... but I'm not sure that we link that generally in CircuitPython.

I was looking at a very similar list just now :-). Since we don't really need any crypto properties (i'm just trying to avoid any structure we don't know about in the vendor uid) and since the input and output lengths are set those simple hashes might be a good idea.

I'll implement it that way for now and then we can have a think about it.

phrogger commented 4 years ago

Adafruit has purchased a block of MAC addresses, OUI = 98:76:B6 , which are the upper three bytes of the MAC address. (OUI = Organization Unique Identifier) (check out https://www.wireshark.org/tools/oui-lookup.html or Google "OUI lookup" ) If you follow the rules about how MAC addresses are supposed to work, you should retain these three upper bytes as fixed. That only gives you the lower three bytes, or 24 bits to hash into. You have some implied responsibility to not assign the same MAC address twice, because of the problems it will cause if they ever show up in the same network. 24 bits is still almost 17 million MAC addresses that belong to Adafruit.

--- Graham

==

On Tue, Jan 21, 2020 at 5:32 PM Nick Moore notifications@github.com wrote:

On Wed, 22 Jan 2020, at 10:07, Dan Halbert wrote:

Re hashing: I don't have much experience with this, but I read some stackoverflow answer which pointed to one of the functions mentioned here: https://en.wikipedia.org/wiki/List_of_hash_functions#Non-cryptographic_hash_functions

Normally I'd just use N bits out of an SHA1 from axtls, which is pulled into MicroPython to support uhashlib ... but I'm not sure that we link that generally in CircuitPython.

I was looking at a very similar list just now :-). Since we don't really need any crypto properties (i'm just trying to avoid any structure we don't know about in the vendor uid) and since the input and output lengths are set those simple hashes might be a good idea.

I'll implement it that way for now and then we can have a think about it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/adafruit/circuitpython/issues/2274?email_source=notifications&email_token=ABFRWBKERPLZM24XYE7ES43Q66AZZA5CNFSM4JKJ3QMKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJRVVHY#issuecomment-576936607, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFRWBLC6BGCTSWK24TKBZLQ66AZZANCNFSM4JKJ3QMA .

nickzoic commented 4 years ago

I'd argue that we really don't want to pick random/hashed UAA addresses (eg: within Adafruit's OUI) ... they're supposed to be administratively controlled.

Better to use a LAA address where we get 46 bits of address space to play with and a much lower probability of collision.