[U-Boot] [PATCH 0/5] efi_loader: fix simple network protocol

This patch series fixes several bugs in the simple network protocol:
* Correctly set and reset the interrupt status. * Support filling the header in the Transmit() service. * Correct the checking and setting of the network state. * Implement the MCastIPtoMAC() service. * Adjust the simple network protocol unit test.
Heinrich Schuchardt (5): efi_loader: interrupts in simple network protocol efi_selftest: check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT efi_loader: EFI_SIMPLE_NETWORK.Transmit() fill header efi_loader: fix status management in network stack efi_loader: implement MCastIPtoMAC
include/efi_api.h | 2 + lib/efi_loader/efi_net.c | 184 +++++++++++++++++++++++----- lib/efi_selftest/efi_selftest_snp.c | 64 ++++++++-- 3 files changed, 212 insertions(+), 38 deletions(-)
-- 2.20.1

GetStatus() must clear the interrupt status. Transmit() should set the TX interrupt. Receive() should clear the RX interrupt. Initialize() and Start() should clear the interrupt status.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- include/efi_api.h | 2 ++ lib/efi_loader/efi_net.c | 40 +++++++++++++++++++++++++--------------- 2 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/include/efi_api.h b/include/efi_api.h index 43778197af..7f7b67fa00 100644 --- a/include/efi_api.h +++ b/include/efi_api.h @@ -1281,6 +1281,8 @@ struct efi_simple_network { struct efi_mac_address *dest_addr, u16 *protocol); struct efi_event *wait_for_packet; struct efi_simple_network_mode *mode; + /* private fields */ + u32 int_status; };
#define EFI_PXE_BASE_CODE_PROTOCOL_GUID \ diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 825e064f9a..bf6d5ab0b3 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -66,10 +66,13 @@ static efi_status_t EFIAPI efi_net_start(struct efi_simple_network *this) goto out; }
- if (this->mode->state != EFI_NETWORK_STOPPED) + if (this->mode->state != EFI_NETWORK_STOPPED) { ret = EFI_ALREADY_STARTED; - else + } else { + this->int_status = 0; + wait_for_packet->is_signaled = false; this->mode->state = EFI_NETWORK_STARTED; + } out: return EFI_EXIT(ret); } @@ -144,6 +147,8 @@ static efi_status_t EFIAPI efi_net_initialize(struct efi_simple_network *this, r = EFI_DEVICE_ERROR; goto out; } else { + this->int_status = 0; + wait_for_packet->is_signaled = false; this->mode->state = EFI_NETWORK_INITIALIZED; } out: @@ -192,6 +197,8 @@ static efi_status_t EFIAPI efi_net_shutdown(struct efi_simple_network *this) }
eth_halt(); + this->int_status = 0; + wait_for_packet->is_signaled = false; this->mode->state = EFI_NETWORK_STOPPED;
out: @@ -350,10 +357,8 @@ static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this, }
if (int_status) { - /* We send packets synchronously, so nothing is outstanding */ - *int_status = EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; - if (new_rx_packet) - *int_status |= EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; + *int_status = this->int_status; + this->int_status = 0; } if (txbuf) *txbuf = new_tx_packet; @@ -429,7 +434,7 @@ static efi_status_t EFIAPI efi_net_transmit net_send_packet(transmit_buffer, buffer_size);
new_tx_packet = buffer; - + this->int_status |= EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT; out: return EFI_EXIT(ret); } @@ -487,12 +492,6 @@ static efi_status_t EFIAPI efi_net_receive ret = EFI_NOT_READY; goto out; } - /* Check that we at least received an Ethernet header */ - if (net_rx_packet_len < sizeof(struct ethernet_hdr)) { - new_rx_packet = false; - ret = EFI_NOT_READY; - goto out; - } /* Fill export parameters */ eth_hdr = (struct ethernet_hdr *)net_rx_packet; protlen = ntohs(eth_hdr->et_protlen); @@ -517,7 +516,8 @@ static efi_status_t EFIAPI efi_net_receive /* Copy packet */ memcpy(buffer, net_rx_packet, net_rx_packet_len); *buffer_size = net_rx_packet_len; - new_rx_packet = false; + new_rx_packet = 0; + this->int_status &= ~EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; out: return EFI_EXIT(ret); } @@ -548,7 +548,6 @@ void efi_net_set_dhcp_ack(void *pkt, int len) static void efi_net_push(void *pkt, int len) { new_rx_packet = true; - wait_for_packet->is_signaled = true; }
/** @@ -577,6 +576,17 @@ static void EFIAPI efi_network_timer_notify(struct efi_event *event, push_packet = efi_net_push; eth_rx(); push_packet = NULL; + if (new_rx_packet) { + /* Check that we at least received an Ethernet header */ + if (net_rx_packet_len >= + sizeof(struct ethernet_hdr)) { + this->int_status |= + EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT; + wait_for_packet->is_signaled = true; + } else { + new_rx_packet = 0; + } + } } out: EFI_EXIT(EFI_SUCCESS); -- 2.20.1

Check that when the WaitForPacket event occurs EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT is set.
Check the return value of Receive().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_selftest/efi_selftest_snp.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c index 4c26619001..807b8657b9 100644 --- a/lib/efi_selftest/efi_selftest_snp.c +++ b/lib/efi_selftest/efi_selftest_snp.c @@ -268,6 +268,7 @@ static int execute(void) struct efi_mac_address destaddr; size_t buffer_size; u8 *addr; + /* * The timeout is to occur after 10 s. */ @@ -298,6 +299,8 @@ static int execute(void) events[0] = timer; events[1] = net->wait_for_packet; for (;;) { + u32 int_status; + /* * Wait for packet to be received or timer event. */ @@ -323,8 +326,17 @@ static int execute(void) * Receive packet */ buffer_size = sizeof(buffer); - net->receive(net, NULL, &buffer_size, &buffer, - &srcaddr, &destaddr, NULL); + ret = net->get_status(net, &int_status, NULL); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to get status"); + return EFI_ST_FAILURE; + } + if (!(int_status & EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT)) { + efi_st_error("RX interrupt not set"); + return EFI_ST_FAILURE; + } + ret = net->receive(net, NULL, &buffer_size, &buffer, + &srcaddr, &destaddr, NULL); if (ret != EFI_SUCCESS) { efi_st_error("Failed to receive packet"); return EFI_ST_FAILURE; -- 2.20.1

Fill the media header in EFI_SIMPLE_NETWORK.Transmit(). Check that the buffer size is large enough for the header.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_net.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index bf6d5ab0b3..646540ac08 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -409,15 +409,33 @@ static efi_status_t EFIAPI efi_net_transmit goto out; }
- if (header_size) { - /* - * TODO: We would need to create the header - * if header_size != 0 - */ - ret = EFI_UNSUPPORTED; + /* At least the IP header has to fit into the buffer */ + if (buffer_size < this->mode->media_header_size) { + ret = EFI_BUFFER_TOO_SMALL; goto out; }
+ /* + * TODO: + * Support VLANs. Use net_set_ether() for copying the header. Use a + * U_BOOT_ENV_CALLBACK to update the media header size. + */ + if (header_size) { + struct ethernet_hdr *header = buffer; + + if (!dest_addr || !protocol || + header_size != this->mode->media_header_size) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + if (!src_addr) + src_addr = &this->mode->current_address; + + memcpy(header->et_dest, dest_addr, ARP_HLEN); + memcpy(header->et_src, src_addr, ARP_HLEN); + header->et_protlen = htons(*protocol); + } + switch (this->mode->state) { case EFI_NETWORK_STOPPED: ret = EFI_NOT_STARTED; @@ -764,6 +782,7 @@ efi_status_t efi_net_register(void) netobj->net_mode.state = EFI_NETWORK_STARTED; memcpy(netobj->net_mode.current_address.mac_addr, eth_get_ethaddr(), 6); netobj->net_mode.hwaddr_size = ARP_HLEN; + netobj->net_mode.media_header_size = ETHER_HDR_SIZE; netobj->net_mode.max_packet_size = PKTSIZE; netobj->net_mode.if_type = ARP_ETHER;
-- 2.20.1

The network should start in status EfiSimpleNetworkStopped.
Add and correct status checks in the simple network protocol.
Correct the unit test: * Shutdown() and Stop() during setup if needed * invoke Shutdown() before Stop() when tearing down
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_net.c | 69 ++++++++++++++++++++++++++--- lib/efi_selftest/efi_selftest_snp.c | 48 +++++++++++++++++--- 2 files changed, 104 insertions(+), 13 deletions(-)
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 646540ac08..970b5ab38e 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -1,8 +1,18 @@ // SPDX-License-Identifier: GPL-2.0+ /* - * EFI application network access support + * Simple network protocol + * PXE base code protocol * - * Copyright (c) 2016 Alexander Graf + * Copyright (c) 2016 Alexander Graf + * + * The simple network protocol has the following statuses and services + * to move between them: + * + * Start(): EfiSimpleNetworkStopped -> EfiSimpleNetworkStarted + * Initialize(): EfiSimpleNetworkStarted -> EfiSimpleNetworkInitialized + * Shutdown(): EfiSimpleNetworkInitialized -> EfiSimpleNetworkStarted + * Stop(): EfiSimpleNetworkStarted -> EfiSimpleNetworkStopped + * Reset(): EfiSimpleNetworkInitialized -> EfiSimpleNetworkInitialized */
#include <common.h> @@ -99,10 +109,13 @@ static efi_status_t EFIAPI efi_net_stop(struct efi_simple_network *this) goto out; }
- if (this->mode->state == EFI_NETWORK_STOPPED) + if (this->mode->state == EFI_NETWORK_STOPPED) { ret = EFI_NOT_STARTED; - else + } else { + /* Disable hardware and put it into the reset state */ + eth_halt(); this->mode->state = EFI_NETWORK_STOPPED; + } out: return EFI_EXIT(ret); } @@ -133,6 +146,15 @@ static efi_status_t EFIAPI efi_net_initialize(struct efi_simple_network *this, goto out; }
+ switch (this->mode->state) { + case EFI_NETWORK_INITIALIZED: + case EFI_NETWORK_STARTED: + break; + default: + r = EFI_NOT_STARTED; + goto out; + } + /* Setup packet buffers */ net_init(); /* Disable hardware and put it into the reset state */ @@ -169,9 +191,31 @@ out: static efi_status_t EFIAPI efi_net_reset(struct efi_simple_network *this, int extended_verification) { + efi_status_t ret; + EFI_ENTRY("%p, %x", this, extended_verification);
- return EFI_EXIT(EFI_CALL(efi_net_initialize(this, 0, 0))); + /* Check parameters */ + if (!this) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + switch (this->mode->state) { + case EFI_NETWORK_INITIALIZED: + break; + case EFI_NETWORK_STOPPED: + ret = EFI_NOT_STARTED; + goto out; + default: + ret = EFI_DEVICE_ERROR; + goto out; + } + + this->mode->state = EFI_NETWORK_STARTED; + ret = EFI_CALL(efi_net_initialize(this, 0, 0)); +out: + return EFI_EXIT(ret); }
/* @@ -196,10 +240,21 @@ static efi_status_t EFIAPI efi_net_shutdown(struct efi_simple_network *this) goto out; }
+ switch (this->mode->state) { + case EFI_NETWORK_INITIALIZED: + break; + case EFI_NETWORK_STOPPED: + ret = EFI_NOT_STARTED; + goto out; + default: + ret = EFI_DEVICE_ERROR; + goto out; + } + eth_halt(); this->int_status = 0; wait_for_packet->is_signaled = false; - this->mode->state = EFI_NETWORK_STOPPED; + this->mode->state = EFI_NETWORK_STARTED;
out: return EFI_EXIT(ret); @@ -779,7 +834,7 @@ efi_status_t efi_net_register(void) netobj->net.transmit = efi_net_transmit; netobj->net.receive = efi_net_receive; netobj->net.mode = &netobj->net_mode; - netobj->net_mode.state = EFI_NETWORK_STARTED; + netobj->net_mode.state = EFI_NETWORK_STOPPED; memcpy(netobj->net_mode.current_address.mac_addr, eth_get_ethaddr(), 6); netobj->net_mode.hwaddr_size = ARP_HLEN; netobj->net_mode.media_header_size = ETHER_HDR_SIZE; diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c index 807b8657b9..9797ecaf42 100644 --- a/lib/efi_selftest/efi_selftest_snp.c +++ b/lib/efi_selftest/efi_selftest_snp.c @@ -228,6 +228,26 @@ static int setup(const efi_handle_t handle, efi_st_error("WaitForPacket event missing\n"); return EFI_ST_FAILURE; } + if (net->mode->state == EFI_NETWORK_INITIALIZED) { + /* + * Shut down network adapter. + */ + ret = net->shutdown(net); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to shut down network adapter\n"); + return EFI_ST_FAILURE; + } + } + if (net->mode->state == EFI_NETWORK_STARTED) { + /* + * Stop network adapter. + */ + ret = net->stop(net); + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to stop network adapter\n"); + return EFI_ST_FAILURE; + } + } /* * Start network adapter. */ @@ -236,6 +256,10 @@ static int setup(const efi_handle_t handle, efi_st_error("Failed to start network adapter\n"); return EFI_ST_FAILURE; } + if (net->mode->state != EFI_NETWORK_STARTED) { + efi_st_error("Failed to start network adapter\n"); + return EFI_ST_FAILURE; + } /* * Initialize network adapter. */ @@ -244,6 +268,10 @@ static int setup(const efi_handle_t handle, efi_st_error("Failed to initialize network adapter\n"); return EFI_ST_FAILURE; } + if (net->mode->state != EFI_NETWORK_INITIALIZED) { + efi_st_error("Failed to initialize network adapter\n"); + return EFI_ST_FAILURE; + } return EFI_ST_SUCCESS; }
@@ -412,21 +440,29 @@ static int teardown(void) } if (net) { /* - * Stop network adapter. + * Shut down network adapter. */ - ret = net->stop(net); + ret = net->shutdown(net); if (ret != EFI_SUCCESS) { - efi_st_error("Failed to stop network adapter\n"); + efi_st_error("Failed to shut down network adapter\n"); exit_status = EFI_ST_FAILURE; } + if (net->mode->state != EFI_NETWORK_STARTED) { + efi_st_error("Failed to shutdown network adapter\n"); + return EFI_ST_FAILURE; + } /* - * Shut down network adapter. + * Stop network adapter. */ - ret = net->shutdown(net); + ret = net->stop(net); if (ret != EFI_SUCCESS) { - efi_st_error("Failed to shut down network adapter\n"); + efi_st_error("Failed to stop network adapter\n"); exit_status = EFI_ST_FAILURE; } + if (net->mode->state != EFI_NETWORK_STOPPED) { + efi_st_error("Failed to stop network adapter\n"); + return EFI_ST_FAILURE; + } }
return exit_status; -- 2.20.1

Implement the MCastIPtoMAC service of the simple network protocol. It converts an multicast IPv4 (or IPv6) address to a multicast Ethernet address.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/efi_net.c | 44 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c index 970b5ab38e..2c20cc3c28 100644 --- a/lib/efi_loader/efi_net.c +++ b/lib/efi_loader/efi_net.c @@ -332,7 +332,7 @@ static efi_status_t EFIAPI efi_net_statistics(struct efi_simple_network *this, /* * efi_net_mcastiptomac() - translate multicast IP address to MAC address * - * This function implements the Statistics service of the + * This function implements the MCastIPtoMAC service of the * EFI_SIMPLE_NETWORK_PROTOCOL. See the Unified Extensible Firmware Interface * (UEFI) specification for details. * @@ -347,9 +347,49 @@ static efi_status_t EFIAPI efi_net_mcastiptomac(struct efi_simple_network *this, struct efi_ip_address *ip, struct efi_mac_address *mac) { + efi_status_t ret = EFI_SUCCESS; + EFI_ENTRY("%p, %x, %p, %p", this, ipv6, ip, mac);
- return EFI_EXIT(EFI_INVALID_PARAMETER); + if (!this || !ip || !mac) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + if (ipv6) { + ret = EFI_UNSUPPORTED; + goto out; + } + + /* Multi-cast addresses are in the range 224.0.0.0 - 239.255.255.255 */ + if ((ip->ip_addr[0] & 0xf0) != 0xe0) { + ret = EFI_INVALID_PARAMETER; + goto out; + }; + + switch (this->mode->state) { + case EFI_NETWORK_INITIALIZED: + case EFI_NETWORK_STARTED: + break; + default: + ret = EFI_NOT_STARTED; + goto out; + } + + memset(mac, 0, sizeof(struct efi_mac_address)); + + /* + * Copy lower 23 bits of IPv4 multi-cast address + * RFC 1112, RFC 7042 2.1.1. + */ + mac->mac_addr[0] = 0x01; + mac->mac_addr[1] = 0x00; + mac->mac_addr[2] = 0x5E; + mac->mac_addr[3] = ip->ip_addr[1] & 0x7F; + mac->mac_addr[4] = ip->ip_addr[2]; + mac->mac_addr[5] = ip->ip_addr[3]; +out: + return EFI_EXIT(ret); }
/** -- 2.20.1
participants (1)
-
Heinrich Schuchardt