[U-Boot] [PATCH 02/10] net: sandbox: Refactor sandbox send function

[Simon Glass]

Hi Joe,

On 24 July 2018 at 15:40, Joe Hershberger <joe.hershberger at ni.com> wrote:
> Make the behavior of the send function reusable.
>
> Signed-off-by: Joe Hershberger <joe.hershberger at ni.com>
> ---
>
>  arch/sandbox/include/asm/eth.h |  20 +++++
>  drivers/net/sandbox.c          | 176 ++++++++++++++++++++++++-----------------
>  2 files changed, 124 insertions(+), 72 deletions(-)
>
> diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h
> index bfcd11b593..00062616a4 100644
> --- a/arch/sandbox/include/asm/eth.h
> +++ b/arch/sandbox/include/asm/eth.h
> @@ -13,4 +13,24 @@ void sandbox_eth_disable_response(int index, bool disable);
>
>  void sandbox_eth_skip_timeout(void);
>
> +/*
> + * sandbox_eth_arp_req_to_reply()
> + *
> + * Check for an arp request to be sent. If so, inject a reply

comments for args?

> + *
> + * returns 1 if injected, 0 if not
> + */
> +int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet,
> +                                unsigned int len);
> +
> +/*
> + * sandbox_eth_ping_req_to_reply()
> + *
> + * Check for a ping request to be sent. If so, inject a reply
> + *
> + * returns 1 if injected, 0 if not
> + */
> +int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet,
> +                                 unsigned int len);
> +
>  #endif /* __ETH_H */
> diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
> index 60fe065ee5..5746af11a6 100644
> --- a/drivers/net/sandbox.c
> +++ b/drivers/net/sandbox.c
> @@ -63,6 +63,108 @@ void sandbox_eth_skip_timeout(void)
>         skip_timeout = true;
>  }
>
> +/*
> + * sandbox_eth_arp_req_to_reply()
> + *
> + * Check for an arp request to be sent. If so, inject a reply
> + *
> + * returns 1 if injected, 0 if not
> + */
> +int sandbox_eth_arp_req_to_reply(struct udevice *dev, void *packet,
> +                                unsigned int len)
> +{
> +       struct eth_sandbox_priv *priv = dev_get_priv(dev);
> +       struct ethernet_hdr *eth = packet;
> +       struct arp_hdr *arp;
> +       struct ethernet_hdr *eth_recv;
> +       struct arp_hdr *arp_recv;
> +
> +       if (ntohs(eth->et_protlen) != PROT_ARP)
> +               return 0;

It seems odd to put this check here? Why not check it before calling
this function?

> +
> +       arp = packet + ETHER_HDR_SIZE;
> +
> +       if (ntohs(arp->ar_op) != ARPOP_REQUEST)
> +               return 0;
> +
> +       /* store this as the assumed IP of the fake host */
> +       priv->fake_host_ipaddr = net_read_ip(&arp->ar_tpa);
> +
> +       /* Formulate a fake response */
> +       eth_recv = (void *)priv->recv_packet_buffer;
> +       memcpy(eth_recv->et_dest, eth->et_src, ARP_HLEN);
> +       memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN);
> +       eth_recv->et_protlen = htons(PROT_ARP);
> +
> +       arp_recv = (void *)eth_recv + ETHER_HDR_SIZE;
> +       arp_recv->ar_hrd = htons(ARP_ETHER);
> +       arp_recv->ar_pro = htons(PROT_IP);
> +       arp_recv->ar_hln = ARP_HLEN;
> +       arp_recv->ar_pln = ARP_PLEN;
> +       arp_recv->ar_op = htons(ARPOP_REPLY);
> +       memcpy(&arp_recv->ar_sha, priv->fake_host_hwaddr, ARP_HLEN);
> +       net_write_ip(&arp_recv->ar_spa, priv->fake_host_ipaddr);
> +       memcpy(&arp_recv->ar_tha, &arp->ar_sha, ARP_HLEN);
> +       net_copy_ip(&arp_recv->ar_tpa, &arp->ar_spa);
> +
> +       priv->recv_packet_length = ETHER_HDR_SIZE + ARP_HDR_SIZE;
> +
> +       return 1;
> +}
> +

[..]

> +       sandbox_eth_arp_req_to_reply(dev, packet, length);
> +       sandbox_eth_ping_req_to_reply(dev, packet, length);

Here functions are called unconditionally, but presumably only at most
one function will do anything.

>
>         return 0;
>  }
> --
> 2.11.0
>


Regards,
Simon