Hi Joe,
On Wed, Jul 25, 2018 at 5:43 AM Joe Hershberger <joe.hershber...@ni.com> wrote:
>
> This tests that ARP requests made to this target's IP address are
> responded-to by the target when it is doing other networking operations.
>
> This currently corrupts the ongoing operation of the device if it
> happens to be awaiting an ARP reply of its own to whatever serverip it
> is attempting to communicate with. In the test, add an expectation that
> the user operation (ping, in this case) will fail. A later patch will
> address this problem.
>
> Signed-off-by: Joe Hershberger <joe.hershber...@ni.com>
> ---
>
> arch/sandbox/include/asm/eth.h | 9 +++++
> drivers/net/sandbox.c | 41 +++++++++++++++++++++
> test/dm/eth.c | 81
> ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 131 insertions(+)
>
Reviewed-by: Bin Meng <bmeng...@gmail.com>
But please see comments below:
> diff --git a/arch/sandbox/include/asm/eth.h b/arch/sandbox/include/asm/eth.h
> index 6bb3f1bbfd..7cd5b551d9 100644
> --- a/arch/sandbox/include/asm/eth.h
> +++ b/arch/sandbox/include/asm/eth.h
> @@ -33,6 +33,15 @@ int sandbox_eth_arp_req_to_reply(struct udevice *dev, void
> *packet,
> int sandbox_eth_ping_req_to_reply(struct udevice *dev, void *packet,
> unsigned int len);
>
> +/*
> + * sandbox_eth_recv_arp_req()
> + *
> + * Inject an ARP request for this target
> + *
> + * returns 1 if injected, 0 if not
again the return codes, can we use -Exxx instead? I see the return
error is not checked anywhere, maybe we should just make it return
void?
> + */
> +int sandbox_eth_recv_arp_req(struct udevice *dev);
> +
> /**
> * A packet handler
> *
> diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
> index 29767ef291..2c2a2c6311 100644
> --- a/drivers/net/sandbox.c
> +++ b/drivers/net/sandbox.c
> @@ -161,6 +161,47 @@ int sandbox_eth_ping_req_to_reply(struct udevice *dev,
> void *packet,
> }
>
> /*
> + * sandbox_eth_recv_arp_req()
> + *
> + * Inject an ARP request for this target
> + *
> + * returns 1 if injected, 0 if not
> + */
> +int sandbox_eth_recv_arp_req(struct udevice *dev)
> +{
> + struct eth_sandbox_priv *priv = dev_get_priv(dev);
> + struct ethernet_hdr *eth_recv;
> + struct arp_hdr *arp_recv;
> +
> + /* Don't allow the buffer to overrun */
> + if (priv->recv_packets >= PKTBUFSRX)
> + return 0;
> +
> + /* Formulate a fake request */
> + eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets];
> + memcpy(eth_recv->et_dest, net_bcast_ethaddr, 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_REQUEST);
> + 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, net_null_ethaddr, ARP_HLEN);
> + net_write_ip(&arp_recv->ar_tpa, net_ip);
> +
> + priv->recv_packet_length[priv->recv_packets] =
> + ETHER_HDR_SIZE + ARP_HDR_SIZE;
> + ++priv->recv_packets;
> +
> + return 1;
> +}
> +
> +/*
> * sb_default_handler()
> *
> * perform typical responses to simple ping
> diff --git a/test/dm/eth.c b/test/dm/eth.c
> index 1a7684a887..9b5f53e819 100644
> --- a/test/dm/eth.c
> +++ b/test/dm/eth.c
> @@ -258,3 +258,84 @@ static int dm_test_net_retry(struct unit_test_state *uts)
> return retval;
> }
> DM_TEST(dm_test_net_retry, DM_TESTF_SCAN_FDT);
> +
> +static int sb_check_arp_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 unit_test_state *uts = priv->priv;
This is not used anywhere. What is this for?
> +
> + if (ntohs(eth->et_protlen) != PROT_ARP)
> + return 0;
> +
> + arp = packet + ETHER_HDR_SIZE;
> +
> + if (ntohs(arp->ar_op) != ARPOP_REPLY)
> + return 0;
> +
> + /* This test would be worthless if we are not waiting */
> + ut_assert(arp_is_waiting());
> +
> + /* Validate response */
> + ut_assert(memcmp(eth->et_src, net_ethaddr, ARP_HLEN) == 0);
> + ut_assert(memcmp(eth->et_dest, priv->fake_host_hwaddr, ARP_HLEN) ==
> 0);
> + ut_assert(eth->et_protlen == htons(PROT_ARP));
> +
> + ut_assert(arp->ar_hrd == htons(ARP_ETHER));
> + ut_assert(arp->ar_pro == htons(PROT_IP));
> + ut_assert(arp->ar_hln == ARP_HLEN);
> + ut_assert(arp->ar_pln == ARP_PLEN);
> + ut_assert(memcmp(&arp->ar_sha, net_ethaddr, ARP_HLEN) == 0);
> + ut_assert(net_read_ip(&arp->ar_spa).s_addr == net_ip.s_addr);
> + ut_assert(memcmp(&arp->ar_tha, priv->fake_host_hwaddr, ARP_HLEN) ==
> 0);
> + ut_assert(net_read_ip(&arp->ar_tpa).s_addr ==
> + string_to_ip("1.1.2.4").s_addr);
> +
> + return 0;
> +}
> +
> +static int sb_with_async_arp_handler(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 = packet + ETHER_HDR_SIZE;
> +
> + /*
> + * If we are about to generate a reply to ARP, first inject a request
> + * from another host
> + */
> + if (ntohs(eth->et_protlen) == PROT_ARP &&
> + ntohs(arp->ar_op) == ARPOP_REQUEST) {
> + /* Make sure sandbox_eth_recv_arp_req() knows who is asking */
> + priv->fake_host_ipaddr = string_to_ip("1.1.2.4");
> +
> + sandbox_eth_recv_arp_req(dev);
> + }
> +
> + sandbox_eth_arp_req_to_reply(dev, packet, len);
> + sandbox_eth_ping_req_to_reply(dev, packet, len);
> +
> + return sb_check_arp_reply(dev, packet, len);
> +}
> +
> +static int dm_test_eth_async_arp_reply(struct unit_test_state *uts)
> +{
> + net_ping_ip = string_to_ip("1.1.2.2");
> +
> + sandbox_eth_set_tx_handler(0, sb_with_async_arp_handler);
> + sandbox_eth_set_priv(0, uts);
Looks this is not needed since it was not used anywhere.
> + sandbox_eth_skip_timeout();
> +
> + env_set("ethact", "eth@10002000");
> + ut_assert(net_loop(PING) == -ETIMEDOUT);
> + ut_asserteq_str("eth@10002000", env_get("ethact"));
> +
> + sandbox_eth_set_tx_handler(0, NULL);
> +
> + return 0;
> +}
> +
> +DM_TEST(dm_test_eth_async_arp_reply, DM_TESTF_SCAN_FDT);
> --
Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
