Hi Joe,
On Wed, Jul 25, 2018 at 5:42 AM Joe Hershberger <joe.hershber...@ni.com> wrote:
>
> The target will respond to pings while doing other network handling.
> Make sure that the response happens and is correct.
>
> 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 | 51 ++++++++++++++++++++++++++
> test/dm/eth.c | 81
> ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 141 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 7cd5b551d9..66f6b4078b 100644
> --- a/arch/sandbox/include/asm/eth.h
> +++ b/arch/sandbox/include/asm/eth.h
> @@ -42,6 +42,15 @@ int sandbox_eth_ping_req_to_reply(struct udevice *dev,
> void *packet,
> */
> int sandbox_eth_recv_arp_req(struct udevice *dev);
>
> +/*
> + * sandbox_eth_recv_ping_req()
> + *
> + * Inject a ping request for this target
> + *
> + * returns 1 if injected, 0 if not
again the return code issue
> + */
> +int sandbox_eth_recv_ping_req(struct udevice *dev);
> +
> /**
> * A packet handler
> *
> diff --git a/drivers/net/sandbox.c b/drivers/net/sandbox.c
> index 2c2a2c6311..039c1e3222 100644
> --- a/drivers/net/sandbox.c
> +++ b/drivers/net/sandbox.c
> @@ -202,6 +202,57 @@ int sandbox_eth_recv_arp_req(struct udevice *dev)
> }
>
> /*
> + * sandbox_eth_recv_ping_req()
> + *
> + * Inject a ping request for this target
> + *
> + * returns 1 if injected, 0 if not
> + */
> +int sandbox_eth_recv_ping_req(struct udevice *dev)
> +{
> + struct eth_sandbox_priv *priv = dev_get_priv(dev);
> + struct ethernet_hdr *eth_recv;
> + struct ip_udp_hdr *ipr;
> + struct icmp_hdr *icmpr;
> +
> + /* Don't allow the buffer to overrun */
> + if (priv->recv_packets >= PKTBUFSRX)
> + return 0;
> +
> + /* Formulate a fake ping */
> + eth_recv = (void *)priv->recv_packet_buffer[priv->recv_packets];
> +
> + memcpy(eth_recv->et_dest, net_ethaddr, ARP_HLEN);
> + memcpy(eth_recv->et_src, priv->fake_host_hwaddr, ARP_HLEN);
> + eth_recv->et_protlen = htons(PROT_IP);
> +
> + ipr = (void *)eth_recv + ETHER_HDR_SIZE;
> + ipr->ip_hl_v = 0x45;
> + ipr->ip_len = htons(IP_ICMP_HDR_SIZE);
> + ipr->ip_off = htons(IP_FLAGS_DFRAG);
> + ipr->ip_p = IPPROTO_ICMP;
> + ipr->ip_sum = 0;
> + net_write_ip(&ipr->ip_src, priv->fake_host_ipaddr);
> + net_write_ip(&ipr->ip_dst, net_ip);
> + ipr->ip_sum = compute_ip_checksum(ipr, IP_HDR_SIZE);
> +
> + icmpr = (struct icmp_hdr *)&ipr->udp_src;
> +
> + icmpr->type = ICMP_ECHO_REQUEST;
> + icmpr->code = 0;
> + icmpr->checksum = 0;
> + icmpr->un.echo.id = 0;
> + icmpr->un.echo.sequence = htons(1);
> + icmpr->checksum = compute_ip_checksum(icmpr, ICMP_HDR_SIZE);
> +
> + priv->recv_packet_length[priv->recv_packets] =
> + ETHER_HDR_SIZE + IP_ICMP_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 9b5f53e819..77c602beaf 100644
> --- a/test/dm/eth.c
> +++ b/test/dm/eth.c
> @@ -339,3 +339,84 @@ static int dm_test_eth_async_arp_reply(struct
> unit_test_state *uts)
> }
>
> DM_TEST(dm_test_eth_async_arp_reply, DM_TESTF_SCAN_FDT);
> +
> +static int sb_check_ping_reply(struct udevice *dev, void *packet,
> + unsigned int len)
> +{
> + struct eth_sandbox_priv *priv = dev_get_priv(dev);
> + struct ethernet_hdr *eth = packet;
> + struct ip_udp_hdr *ip;
> + struct icmp_hdr *icmp;
> + struct unit_test_state *uts = priv->priv;
This is not used anywhere.
> +
> + if (ntohs(eth->et_protlen) != PROT_IP)
> + return 0;
> +
> + ip = packet + ETHER_HDR_SIZE;
> +
> + if (ip->ip_p != IPPROTO_ICMP)
> + return 0;
> +
> + icmp = (struct icmp_hdr *)&ip->udp_src;
> +
> + if (icmp->type != ICMP_ECHO_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_IP));
> +
> + ut_assert(net_read_ip(&ip->ip_src).s_addr == net_ip.s_addr);
> + ut_assert(net_read_ip(&ip->ip_dst).s_addr ==
> + string_to_ip("1.1.2.4").s_addr);
> +
> + return 0;
> +}
> +
> +static int sb_with_async_ping_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_ping_req(dev);
> + }
> +
> + sandbox_eth_arp_req_to_reply(dev, packet, len);
> + sandbox_eth_ping_req_to_reply(dev, packet, len);
> +
> + return sb_check_ping_reply(dev, packet, len);
> +}
> +
> +static int dm_test_eth_async_ping_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_ping_handler);
> + sandbox_eth_set_priv(0, uts);
ditto
> + 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_ping_reply, DM_TESTF_SCAN_FDT);
> --
Regards,
Bin
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
