
On 06/27/2018 08:01 AM, Joe Hershberger wrote:
On Wed, Jun 27, 2018 at 12:55 AM, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 06/27/2018 06:58 AM, Joe Hershberger wrote:
Hi Heinrich,
On Tue, Jun 26, 2018 at 8:15 PM, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 06/26/2018 11:19 PM, Joe Hershberger wrote:
With systemd stable interface names, eth0 will almost never exist. Instead of using that name in the sandbox.dts, use index 2 (the first interface after "lo"). Enumerate the interfaces on the system to choose a valid interace name.
Signed-off-by: Joe Hershberger joe.hershberger@ni.com
Hello Joe,
thanks for addressing this issue.
On my laptop the raw interfaces are currently called: 1 - lo - loopback 2 - enp0s25 - wired ethernet 3 - wlp12s0 - WLAN
But this list is not static:
It is allowable to define multiple loopback interfaces. Additional bridges, tap and tun interfaces may be defined. A computer may also have multiple wired and wireless interfaces. And of cause I may choose to plug in an additional ethernet adapter while my computer is running.
The interface index does not tell which interface is actually connected to the network. My laptop uses wpl12s0 (index = 3) for sending out this mail. So, please, do not replace one invalid assumption (eth0) by another (index = 2).
While index=2 is not an ideal assumption, I don't think invalid is a fair description. It is far better than eth0, which on modern systems
Using a laptop over WLAN is not special. So many users will be using an interface with index=3.
So your assumption that all computers are created equal (using index=2) is invalid.
is very unlikely. The driver is probed based on DTS entries. This is simply for manual testing, so if the common situation doesn't apply to a person, they can easily modify the DTS to specify the name of their interface, or they can specify a different index. I don't think the fact that it doesn't ideally fit every possible situation warrants changing how these are probed to somehow be dynamically created based on the OS.
I doubt there is a guarantee that index 1 is a loopback interface on POSIX systems. If you need a special treatment of loopback devices you could analyze the properties of each interface to detect which one is a loopback interface.
The only special treatment for local interfaces is that they don't work with raw sockets. You can see that in the code. At the same time, the local interface is pretty crippled for testing the network stack anyway (no ICMP support) so if there is more than one (why would you?) then that one not named "lo" will not work.
There is no need for using the device tree for the specification of interfaces. Instead a raw ethernet device can be set up for each interface. Let the user choose at runtime which device he wants to use for testing.
You say there is no need, but that's the way it works in the sandbox. There is no "OS" subsystem that is there to enumerate new interfaces dynamically, the way something like PCI on a real board would. I don't think we would want it completely automatic anyway, because things like the test.dts have only fake sandbox eth interfaces, not any raw, since we want the unit test to run consistently regardless of the arrangement of network interfaces on the test system it runs on.
NAK until the Sandbox works in the same way both on my laptop (WLAN, index=3) and on my workstation (wired, index=2) without manually patching source code.
So it sounds like you'd be fine if there were simply 2 entries in the DTS... one that lists index=2 and another that lists index=3.
No, you didn't get the point. First there is no guarantee that the loopback is index 1. And there is no guarantee that index 2 or 3 represent the connection to the internet.
Would you accept a patch that will only work if the power connector is for 110V/60 Hz and volume is measured in imperial gallons?
Best regards
Heinrich
Best regards
Heinrich
Cheers, -Joe
Best regards
Heinrich
arch/sandbox/cpu/eth-raw-os.c | 28 ++++++++++++++++++++++++++++ arch/sandbox/dts/sandbox.dts | 2 +- arch/sandbox/include/asm/eth-raw-os.h | 4 +++- drivers/net/sandbox-raw.c | 8 +++++++- 4 files changed, 39 insertions(+), 3 deletions(-)
diff --git a/arch/sandbox/cpu/eth-raw-os.c b/arch/sandbox/cpu/eth-raw-os.c index 61f23ed210..11b5271f31 100644 --- a/arch/sandbox/cpu/eth-raw-os.c +++ b/arch/sandbox/cpu/eth-raw-os.c @@ -150,6 +150,34 @@ int sandbox_eth_raw_os_start(const char *ifname, unsigned char *ethmac, return _raw_packet_start(ifname, ethmac, priv); }
+int sandbox_eth_raw_os_start_idx(unsigned int ifindex, unsigned char *ethmac,
struct eth_sandbox_raw_priv *priv)
+{
int ret;
struct if_nameindex *ni, *i;
const char *ifname = NULL;
ni = if_nameindex();
if (!ni)
return -EINVAL;
for (i = ni; !(i->if_index == 0 && !i->if_name); i++) {
if (ifindex == i->if_index) {
ifname = i->if_name;
printf("(%s)\n", ifname);
break;
}
}
/* Don't bother supporting 'lo' by index. Just use the name in DTS */
if (ifname)
ret = _raw_packet_start(ifname, ethmac, priv);
else
ret = -EINVAL;
if_freenameindex(ni);
return ret;
+}
- int sandbox_eth_raw_os_send(void *packet, int length, struct eth_sandbox_raw_priv *priv) {
diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts index 0ea2452742..e7f6c194d0 100644 --- a/arch/sandbox/dts/sandbox.dts +++ b/arch/sandbox/dts/sandbox.dts @@ -56,7 +56,7 @@ eth@80000000 { compatible = "sandbox,eth-raw"; reg = <0x80000000 0x1000>;
host-raw-interface = "eth0";
host-raw-interface-idx = <2>; }; eth@90000000 {
diff --git a/arch/sandbox/include/asm/eth-raw-os.h b/arch/sandbox/include/asm/eth-raw-os.h index f986d3142d..70b29b4508 100644 --- a/arch/sandbox/include/asm/eth-raw-os.h +++ b/arch/sandbox/include/asm/eth-raw-os.h @@ -29,7 +29,9 @@ struct eth_sandbox_raw_priv { };
int sandbox_eth_raw_os_start(const char *ifname, unsigned char *ethmac,
struct eth_sandbox_raw_priv *priv);
struct eth_sandbox_raw_priv *priv);
+int sandbox_eth_raw_os_start_idx(unsigned int ifindex, unsigned char *ethmac,
int sandbox_eth_raw_os_send(void *packet, int length, struct eth_sandbox_raw_priv *priv); int sandbox_eth_raw_os_recv(void *packet, int *length,struct eth_sandbox_raw_priv *priv);
diff --git a/drivers/net/sandbox-raw.c b/drivers/net/sandbox-raw.c index f835a6a7f3..3bde8d84ce 100644 --- a/drivers/net/sandbox-raw.c +++ b/drivers/net/sandbox-raw.c @@ -22,6 +22,7 @@ static int sb_eth_raw_start(struct udevice *dev) struct eth_sandbox_raw_priv *priv = dev_get_priv(dev); struct eth_pdata *pdata = dev_get_platdata(dev); const char *interface;
u32 index; debug("eth_sandbox_raw: Start\n");
@@ -37,7 +38,12 @@ static int sb_eth_raw_start(struct udevice *dev) priv); }
return -EINVAL;
if (dev_read_u32(dev, "host-raw-interface-idx", &index) < 0)
return -EINVAL;
printf("eth_sandbox_raw: Using interface index %d from DT ", index);
return sandbox_eth_raw_os_start_idx(index, pdata->enetaddr, priv);
}
static int sb_eth_raw_send(struct udevice *dev, void *packet, int length)
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot