Re: [U-Boot] [PATCH 08/10] test: eth: Add a test for ARP requests
Hi Joe,
On Wed, Jul 25, 2018 at 5:43 AM Joe Hershberger 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
> ---
>
> 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
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(&a
Re: [U-Boot] [PATCH 08/10] test: eth: Add a test for ARP requests
On 24 July 2018 at 15:40, Joe Hershberger 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 > --- > > arch/sandbox/include/asm/eth.h | 9 + > drivers/net/sandbox.c | 41 + > test/dm/eth.c | 81 > ++ > 3 files changed, 131 insertions(+) Nice! Reviewed-by: Simon Glass ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] [PATCH 08/10] test: eth: Add a test for ARP requests
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
---
arch/sandbox/include/asm/eth.h | 9 +
drivers/net/sandbox.c | 41 +
test/dm/eth.c | 81 ++
3 files changed, 131 insertions(+)
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
+ */
+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;
+
+ 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 a
