[U-Boot] [PATCH RFC] sandbox: Add tap based networking

[Mike Frysinger]

On Tuesday 29 November 2011 13:21:38 Matthias Weisser wrote:
> Am 29.11.2011 16:24, schrieb Mike Frysinger:
> > On Tuesday 29 November 2011 09:09:03 Matthias Weisser wrote:
> >> As sandbox is build using the native compiler, which is in my case
> >> x86_64, ulong is 64 bit in size. This caused non-working IP stuff as
> >> IPaddr_t is typedefed to ulong. I changed this typedef to u32 which
> >> helped but is quite invasive to all network related stuff. This is the
> >> main reason why this patched is marked as RFC.
> > 
> > could you elaborate on "non-working IP stuff" ?
> 
> Nothing worked. e.g. ARP wasn't working due to the memcpy in function
> NetReadIP in net.h which uses sizeof(IPaddr_t) to copy the IP address
> out of the network packet. sizeof(IPaddr_t) yields 8 on x86_64. After
> changing the typedef to a type having 32 bits everything worked. So this
> is one of the zillion problems fixed which we will see if u-boot is
> ported to real 64 bit hardware :-)

ok, so IPaddr_t is holding the exact contents of the IP address and gets 
read/written directly to the network packets.  and being an IPv4 address, it 
needs to be 32bits exactly.

please split that one line change out into a dedicated patch.

> >> +static int tap_set_hwaddr(struct eth_device *dev)
> >> +{
> >> +	/* Nothing to be done here */
> >> +	return 0;
> >> +}
> > 
> > isn't there an ioctl that lets you control this ?
> 
> Sure. But if I read the the docs correct it is an privileged operation
> and I don't think we wan't to run u-boot as super user all the time. How
> is the situation handled on real hardware when the MAC is programmed to
> an EEPROM on the NIC. Can the MAC be read from the NIC and set to
> u-boot? This would be the best solution as linux should take care about
> MAC address assignment.

the tap_initialize() func should read the current MAC address assigned to the 
tap device and write that to dev->enetaddr

then when tap_set_hwaddr() gets called, if the MAC is different, it will 
attempt to set the MAC to what the user requested.  if they don't have 
permission, then the code can yell at them.  but if they do, this should work 
imo.  this gets us the best of all worlds i think.

> >> +	if (!edev) {
> >> +		puts("tap: not enough malloc memory for eth_device\n");
> >> +		res = -ENOMEM;
> >> +		goto err1;
> >> +	}
> >> +
> >> +	if (!tap) {
> >> +		puts("tap: not enough malloc memory for tap_priv\n");
> >> +		res = -ENOMEM;
> >> +		goto err2;
> >> +	}
> > 
> > drop the puts(), and return 0, not -ENOMEM
> 
> I don't understand why, but done. We are in a PC environment where the
> puts shouldn't hurt in code size and may help in debugging.

while true, i'm thinking in terms of standard u-boot network driver style here 
rather than "let's be as helpful as possible to the user".  i'd still prefer 
we drop the puts(), but i understand what you mean ...

also, please keep replies on list :)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20111129/c1c4dfae/attachment.pgp>