[U-Boot] [PATCH 5/5] net: sandbox-raw: Allow interface to be specified by index
Heinrich Schuchardt
xypron.glpk at gmx.de
Wed Jun 27 05:55:09 UTC 2018
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 at 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 at 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.
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 at 80000000 {
>>> compatible = "sandbox,eth-raw";
>>> reg = <0x80000000 0x1000>;
>>> - host-raw-interface = "eth0";
>>> + host-raw-interface-idx = <2>;
>>> };
>>>
>>> eth at 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,
>>> + struct eth_sandbox_raw_priv *priv);
>>> 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,
>>> 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 at lists.denx.de
>> https://lists.denx.de/listinfo/u-boot
>
More information about the U-Boot
mailing list