| Message ID | 20180724214012.22798-10-joe.hershberger@ni.com |
|---|---|
| State | Superseded |
| Delegated to: | Joe Hershberger |
| Headers | show |
| Series | net: Fix packet corruption issue when handling asynch replies | expand |
On 24 July 2018 at 15:40, Joe Hershberger <joe.hershberger@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.hershberger@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: Simon Glass <sjg@chromium.org>
Hi Joe, On Wed, Jul 25, 2018 at 5:42 AM Joe Hershberger <joe.hershberger@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.hershberger@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.cn@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
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 + */ +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; + + 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); + 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);
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.hershberger@ni.com> --- arch/sandbox/include/asm/eth.h | 9 +++++ drivers/net/sandbox.c | 51 ++++++++++++++++++++++++++ test/dm/eth.c | 81 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+)