[U-Boot] [RFC PATCH v4 23/23] sandbox: eth: Add support for using the 'lo' interface

Simon Glass sjg at chromium.org
Mon Mar 2 23:16:21 CET 2015


Hi Joe,

On 2 March 2015 at 00:18, Joe Hershberger <joe.hershberger at gmail.com> wrote:
>
>
> On Sun, Mar 1, 2015 at 12:07 PM, Simon Glass <sjg at chromium.org> wrote:
>>
>> Hi Joe,
>>
>> On 24 February 2015 at 17:02, Joe Hershberger <joe.hershberger at ni.com>
>> wrote:
>> > The 'lo' interface on Linux doesn't support thinks like ARP or
>> > link-layer access like we use to talk to a normal network interface.
>> > A higher-level network API must be used to access localhost.
>> >
>> > As written, this interface is limited to not supporting ICMP since the
>> > API doesn't allow the socket to be opened for all IP traffic and be able
>> > to receive at the same time. UDP is far more useful to test with, so it
>> > was selected over ICMP. Ping won't work, but things like TFTP should
>> > work.
>> >
>> > Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
>> >
>> > ---
>> >
>> > Changes in v4:
>> > -Added support for the 'lo' network interface
>> >
>> > Changes in v3: None
>> > Changes in v2: None
>> >
>> >  arch/sandbox/cpu/eth-raw-os.c         | 152
>> > +++++++++++++++++++++++++++-------
>> >  arch/sandbox/dts/sandbox.dts          |  10 +++
>> >  arch/sandbox/include/asm/eth-raw-os.h |  10 ++-
>> >  drivers/net/sandbox-raw.c             |  62 +++++++++++++-
>> >  4 files changed, 203 insertions(+), 31 deletions(-)
>> >
>> > diff --git a/arch/sandbox/cpu/eth-raw-os.c
>> > b/arch/sandbox/cpu/eth-raw-os.c
>> > index 9218f94..acb150a 100644
>> > --- a/arch/sandbox/cpu/eth-raw-os.c
>> > +++ b/arch/sandbox/cpu/eth-raw-os.c
>> > @@ -11,6 +11,8 @@
>> >  #include <errno.h>
>> >  #include <net/if.h>
>> >  #include <netinet/in.h>
>> > +#include <netinet/ip.h>
>> > +#include <netinet/udp.h>
>> >  #include <stdio.h>
>> >  #include <stdlib.h>
>> >  #include <string.h>
>> > @@ -19,51 +21,139 @@
>> >  #include <sys/socket.h>
>> >  #include <unistd.h>
>> >
>> > +#include <arpa/inet.h>
>> >  #include <linux/if_ether.h>
>> >  #include <linux/if_packet.h>
>> >
>> >  int sandbox_eth_raw_os_init(const char *ifname, unsigned char *ethmac,
>> >                             struct eth_sandbox_raw_priv *priv)
>> >  {
>> > -       struct sockaddr_ll *device;
>> > -       struct packet_mreq mr;
>> > -
>> > -       /* Prepare device struct */
>> > -       priv->device = malloc(sizeof(struct sockaddr_ll));
>> > -       device = priv->device;
>> > -       memset(device, 0, sizeof(struct sockaddr_ll));
>> > -       device->sll_ifindex = if_nametoindex(ifname);
>> > -       device->sll_family = AF_PACKET;
>> > -       memcpy(device->sll_addr, ethmac, 6);
>> > -       device->sll_halen = htons(6);
>> > -
>> > -       /* Open socket */
>> > -       priv->sd = socket(PF_PACKET, SOCK_RAW, htons(ETH_P_ALL));
>> > -       if (priv->sd < 0) {
>> > -               printf("Failed to open socket: %d %s\n", errno,
>> > -                      strerror(errno));
>> > -               return -errno;
>> > +       if (priv->local) {
>>
>> Can you put these two blocks of code (if and else) in separate
>> functions and call them from here? This function is too long.
>
> OK
>
>> > +               struct sockaddr_in *device;
>> > +               int ret;
>> > +               struct timeval tv;
>> > +               int one = 1;
>> > +
>> > +               /* Prepare device struct */
>> > +               priv->device = malloc(sizeof(struct sockaddr_in));
>>
>> return -ENOMEM if NULL.
>>
>> > +               device = priv->device;
>> > +               memset(device, 0, sizeof(struct sockaddr_in));
>> > +               device->sin_family = AF_INET;
>> > +               ret = inet_pton(AF_INET, "127.0.0.1",
>>
>> Is this INADDR_LOOPBACK? Maybe you can just assign it here?
>>
>> > +                         (struct in_addr *)&device->sin_addr.s_addr);
>> > +               if (ret < 0) {
>> > +                       printf("Failed to convert address: %d %s\n",
>> > errno,
>> > +                              strerror(errno));
>> > +                       return -errno;
>> > +               }
>> > +
>> > +               /**
>> > +                * Open socket
>> > +                *  Since we specify UDP here, any incoming ICMP packets
>> > will
>> > +                *  not be received, so things like ping will not work
>> > on this
>> > +                *  localhost interface.
>> > +                */
>> > +               priv->sd = socket(AF_INET, SOCK_RAW, IPPROTO_UDP);
>> > +               if (priv->sd < 0) {
>> > +                       printf("Failed to open socket: %d %s\n", errno,
>> > +                              strerror(errno));
>> > +                       return -errno;
>> > +               }
>> > +
>> > +               /* Allow the receive to timeout after a millisecond */
>> > +               tv.tv_sec = 0;
>> > +               tv.tv_usec = 1000;
>> > +               ret = setsockopt(priv->sd, SOL_SOCKET, SO_RCVTIMEO,
>> > (char *)&tv,
>> > +                          sizeof(struct timeval));
>> > +               if (ret < 0) {
>> > +                       printf("Failed to set opt: %d %s\n", errno,
>> > +                              strerror(errno));
>> > +                       return -errno;
>> > +               }
>> > +
>> > +               /* Include the UDP/IP headers on send and receive */
>> > +               ret = setsockopt(priv->sd, IPPROTO_IP, IP_HDRINCL, &one,
>> > +                                sizeof(one));
>> > +               if (ret < 0) {
>> > +                       printf("Failed to set opt: %d %s\n", errno,
>> > +                              strerror(errno));
>> > +                       return -errno;
>> > +               }
>> > +               priv->local_bind_sd = -1;
>> > +               priv->local_bind_udp_port = 0;
>> > +       } else {
>> > +               struct sockaddr_ll *device;
>> > +               struct packet_mreq mr;
>> > +
>> > +               /* Prepare device struct */
>> > +               priv->device = malloc(sizeof(struct sockaddr_ll));
>> > +               device = priv->device;
>> > +               memset(device, 0, sizeof(struct sockaddr_ll));
>> > +               device->sll_ifindex = if_nametoindex(ifname);
>> > +               device->sll_family = AF_PACKET;
>> > +               memcpy(device->sll_addr, ethmac, 6);
>> > +               device->sll_halen = htons(6);
>> > +
>> > +               /* Open socket */
>> > +               priv->sd = socket(PF_PACKET, SOCK_RAW,
>> > htons(ETH_P_ALL));
>> > +               if (priv->sd < 0) {
>> > +                       printf("Failed to open socket: %d %s\n", errno,
>> > +                              strerror(errno));
>> > +                       return -errno;
>> > +               }
>> > +               /* Bind to the specified interface */
>> > +               setsockopt(priv->sd, SOL_SOCKET, SO_BINDTODEVICE,
>> > ifname,
>> > +                          strlen(ifname) + 1);
>> > +
>> > +               /* Enable promiscuous mode to receive responses meant
>> > for us */
>> > +               mr.mr_ifindex = device->sll_ifindex;
>> > +               mr.mr_type = PACKET_MR_PROMISC;
>> > +               setsockopt(priv->sd, SOL_PACKET, PACKET_ADD_MEMBERSHIP,
>> > +                          &mr, sizeof(mr));
>> >         }
>> > -       /* Bind to the specified interface */
>> > -       setsockopt(priv->sd, SOL_SOCKET, SO_BINDTODEVICE, ifname,
>> > -                  strlen(ifname) + 1);
>> > -
>> > -       /* Enable promiscuous mode to receive responses meant for us */
>> > -       mr.mr_ifindex = device->sll_ifindex;
>> > -       mr.mr_type = PACKET_MR_PROMISC;
>> > -       setsockopt(priv->sd, SOL_PACKET, PACKET_ADD_MEMBERSHIP,
>> > -                  &mr, sizeof(mr));
>> > +
>> >         return 0;
>> >  }
>> >
>> >  int sandbox_eth_raw_os_send(void *packet, int length,
>> > -                           const struct eth_sandbox_raw_priv *priv)
>> > +                           struct eth_sandbox_raw_priv *priv)
>> >  {
>> >         int retval;
>> > +       struct udphdr *udph = packet + sizeof(struct iphdr);
>> >
>> >         if (!priv->sd || !priv->device)
>> >                 return -EINVAL;
>> >
>> > +       if (priv->local && (priv->local_bind_sd == -1 ||
>> > +                           priv->local_bind_udp_port != udph->source))
>> > {
>>
>> What does this block of code do? Comment?
>
> OK, I'll add a comment. This block of code came about when testing tftp on
> the localhost interface. When using the RAW AF_INET API, the network stack
> is still in play responding to incoming traffic based on open "ports". Since
> it is raw (at the IP layer, no Ethernet) the network stack tells the TFTP
> server that the port it responded to is closed. This causes the TFTP
> transfer to be aborted. This block of code inspects the outgoing packet as
> formulated by the u-boot network stack to determine the source port (that
> the TFTP server will send packets back to) and opens a typical UDP socket on
> that port, thus preventing the network stack from sending that ICMP message
> claiming that the port has no bound socket.

Sounds like a good comment :-)

>
>> > +               struct iphdr *iph = packet;
>> > +               struct sockaddr_in addr;
>> > +
>> > +               if (priv->local_bind_sd != -1)
>> > +                       close(priv->local_bind_sd);
>> > +
>> > +               /* A normal UDP socket is required to bind */
>> > +               priv->local_bind_sd = socket(AF_INET, SOCK_DGRAM, 0);
>> > +               if (priv->local_bind_sd < 0) {
>> > +                       printf("Failed to open bind sd: %d %s\n", errno,
>> > +                              strerror(errno));
>> > +                       return -errno;
>> > +               }
>> > +               priv->local_bind_udp_port = udph->source;
>> > +
>> > +               /**
>> > +                * Bind the UDP port that we intend to use as our source
>> > port
>> > +                * so that the kernel will not send ICMP port
>> > unreachable
>>
>> Do you mean return the 'ICMP port unreachable' error?
>
> I guess it is an error. I just thought of it as guidance to the sender.
>
>> > +                */
>> > +               addr.sin_family = AF_INET;
>> > +               addr.sin_port = udph->source;
>> > +               addr.sin_addr.s_addr = iph->saddr;
>> > +               retval = bind(priv->local_bind_sd, &addr, sizeof(addr));
>> > +               if (retval < 0)
>> > +                       printf("Failed to bind: %d %s\n", errno,
>> > +                              strerror(errno));
>> > +       }
>> > +
>> >         retval = sendto(priv->sd, packet, length, 0,
>> >                         (struct sockaddr *)priv->device,
>> >                         sizeof(struct sockaddr_ll));
>> > @@ -99,4 +189,10 @@ void sandbox_eth_raw_os_halt(struct
>> > eth_sandbox_raw_priv *priv)
>> >         priv->device = NULL;
>> >         close(priv->sd);
>> >         priv->sd = -1;
>> > +       if (priv->local) {
>> > +               if (priv->local_bind_sd != -1)
>> > +                       close(priv->local_bind_sd);
>> > +               priv->local_bind_sd = -1;
>> > +               priv->local_bind_udp_port = 0;
>> > +       }
>> >  }
>> > diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
>> > index b6762f4..9851bda 100644
>> > --- a/arch/sandbox/dts/sandbox.dts
>> > +++ b/arch/sandbox/dts/sandbox.dts
>> > @@ -4,6 +4,10 @@
>> >         #address-cells = <1>;
>> >         #size-cells = <0>;
>> >
>> > +       aliases {
>> > +               eth5 = "/eth at 90000000";
>> > +       };
>> > +
>> >         chosen {
>> >                 stdout-path = "/serial";
>> >         };
>> > @@ -192,4 +196,10 @@
>> >                 reg = <0x80000000 0x1000>;
>> >                 host-raw-interface = "eth0";
>> >         };
>> > +
>> > +       eth at 90000000 {
>> > +               compatible = "sandbox,eth-raw";
>> > +               reg = <0x90000000 0x1000>;
>> > +               host-raw-interface = "lo";
>> > +       };
>> >  };
>> > diff --git a/arch/sandbox/include/asm/eth-raw-os.h
>> > b/arch/sandbox/include/asm/eth-raw-os.h
>> > index d92b72c..44e7050 100644
>> > --- a/arch/sandbox/include/asm/eth-raw-os.h
>> > +++ b/arch/sandbox/include/asm/eth-raw-os.h
>> > @@ -15,16 +15,24 @@
>> >   *
>> >   * sd: socket descriptor - the open socket during a session
>> >   * device: struct sockaddr_ll - the host interface packets move to/from
>> > + * local: 1 or 0 to select a local interface or not
>>
>> Should expand this a bit. The local interface is the loopback
>> device....why do you want one over the other?
>
> Not sure what you're getting at here. You want local if you want to talk to
> a local server (TFTP is what I tested with). You want non-local if you want
> to talk over a real Ethernet interface to a different machine than the one
> running sandbox.

That's what I was wondering. Just a few notes in the comment about how
to choose the parameter value.

>
>> > + * local_bindsd: socket descriptor to prevent the kernel from sending
>> > + *              a message to the server claiming the port is
>> > + *              unreachable
>> > + * local_bind_udp_port: The UDP port number that we bound to
>> >   */
>> >  struct eth_sandbox_raw_priv {
>> >         int sd;
>> >         void *device;
>> > +       int local;
>> > +       int local_bind_sd;
>> > +       unsigned short local_bind_udp_port;
>> >  };
>> >
>> >  int sandbox_eth_raw_os_init(const char *ifname, unsigned char *ethmac,
>> >                             struct eth_sandbox_raw_priv *priv);
>> >  int sandbox_eth_raw_os_send(void *packet, int length,
>> > -                           const struct eth_sandbox_raw_priv *priv);
>> > +                           struct eth_sandbox_raw_priv *priv);
>> >  int sandbox_eth_raw_os_recv(void *packet, int *length,
>> >                             const struct eth_sandbox_raw_priv *priv);
>> >  void sandbox_eth_raw_os_halt(struct eth_sandbox_raw_priv *priv);
>> > diff --git a/drivers/net/sandbox-raw.c b/drivers/net/sandbox-raw.c
>> > index 01b33a9..4b0f836 100644
>> > --- a/drivers/net/sandbox-raw.c
>> > +++ b/drivers/net/sandbox-raw.c
>> > @@ -15,6 +15,8 @@
>> >
>> >  DECLARE_GLOBAL_DATA_PTR;
>> >
>> > +static int reply_arp;
>> > +static IPaddr_t arp_ip;
>> >
>> >  static int sb_eth_raw_start(struct udevice *dev)
>> >  {
>> > @@ -28,6 +30,12 @@ static int sb_eth_raw_start(struct udevice *dev)
>> >         interface = fdt_getprop(gd->fdt_blob, dev->of_offset,
>> >                                             "host-raw-interface", NULL);
>> >
>> > +       if (strcmp(interface, "lo") == 0) {
>> > +               priv->local = 1;
>> > +               setenv("ipaddr", "127.0.0.1");
>> > +               setenv("serverip", "127.0.0.1");
>> > +       }
>> > +
>> >         retval = sandbox_eth_raw_os_init(interface, pdata->enetaddr,
>> > priv);
>> >
>> >         return retval;
>> > @@ -39,19 +47,69 @@ static int sb_eth_raw_send(struct udevice *dev, void
>> > *packet, int length)
>> >
>> >         debug("eth_sandbox_raw: Send packet %d\n", length);
>> >
>> > +       if (priv->local) {
>> > +               struct ethernet_hdr *eth = packet;
>> > +
>> > +               if (ntohs(eth->et_protlen) == PROT_ARP) {
>> > +                       struct arp_hdr *arp = packet + ETHER_HDR_SIZE;
>> > +
>> > +                       /**
>> > +                        * localhost works on a higher level API in
>> > Linux than
>>
>> nit: higher-level
>>
>> > +                        * ARP packets, so fake it
>>
>> fake what?
>
> Fake an ARP response. The u-boot network stack is sending an ARP request (to
> find the MAC address to address the actual packet to) and requires an ARP
> response to continue. Since this is the localhost interface, there is no
> Etherent level traffic at all, so there is no way to send an ARP request or
> to get a response. For this reason we fake the response to make the u-boot
> network stack happy.

OK, please add this to the comment too.

>
>
>> > +                        */
>> > +                       arp_ip = NetReadIP(&arp->ar_tpa);
>> > +                       reply_arp = 1;
>> > +                       return 0;
>> > +               }
>> > +               packet += ETHER_HDR_SIZE;
>> > +               length -= ETHER_HDR_SIZE;
>> > +       }
>> >         return sandbox_eth_raw_os_send(packet, length, priv);
>> >  }
>> >
>> >  static int sb_eth_raw_recv(struct udevice *dev)
>> >  {
>> > +       struct eth_pdata *pdata = dev_get_platdata(dev);
>> >         struct eth_sandbox_raw_priv *priv = dev_get_priv(dev);
>> > -       int retval;
>> > +       int retval = 0;
>> >         uchar buffer[PKTSIZE];
>> >         int length;
>> >
>> > -       retval = sandbox_eth_raw_os_recv(buffer, &length, priv);
>> > +       if (reply_arp) {
>> > +               struct arp_hdr *arp = (void *)buffer + ETHER_HDR_SIZE;
>> > +
>> > +               /* Formulate a fake ARP */
>> > +               arp->ar_hrd = htons(ARP_ETHER);
>> > +               arp->ar_pro = htons(PROT_IP);
>> > +               arp->ar_hln = ARP_HLEN;
>> > +               arp->ar_pln = ARP_PLEN;
>> > +               arp->ar_op = htons(ARPOP_REPLY);
>> > +               /* Any non-zero MAC address will work */
>> > +               memset(&arp->ar_sha, 0x01, ARP_HLEN);
>> > +               /* Use whatever IP we were looking for (always
>> > 127.0.0.1?) */
>> > +               NetWriteIP(&arp->ar_spa, arp_ip);
>> > +               memcpy(&arp->ar_tha, pdata->enetaddr, ARP_HLEN);
>> > +               NetWriteIP(&arp->ar_tpa, NetOurIP);
>> > +               length = ARP_HDR_SIZE;
>> > +       } else {
>> > +               /* If local, the Ethernet header won't be included; skip
>> > it */
>> > +               uchar *pktptr = priv->local ? buffer + ETHER_HDR_SIZE :
>> > buffer;
>> > +
>> > +               retval = sandbox_eth_raw_os_recv(pktptr, &length, priv);
>> > +       }
>> >
>> >         if (!retval && length) {
>> > +               if (priv->local) {
>> > +                       struct ethernet_hdr *eth = (void *)buffer;
>> > +
>> > +                       /* Fill in enough of the missing Ethernet header
>> > */
>> > +                       memcpy(eth->et_dest, pdata->enetaddr, ARP_HLEN);
>> > +                       memset(eth->et_src, 0x01, ARP_HLEN);
>> > +                       eth->et_protlen = htons(reply_arp ? PROT_ARP :
>> > PROT_IP);
>> > +                       reply_arp = 0;
>> > +                       length += ETHER_HDR_SIZE;
>> > +               }
>> > +
>> >                 debug("eth_sandbox_raw: received packet %d\n",
>> >                       length);
>> >                 NetReceive(buffer, length);
>> > --
>> > 1.7.11.5

Regards,
Simon


More information about the U-Boot mailing list