| Message ID | 163178242457.65790.16820317670072849950.stgit@localhost |
|---|---|
| State | Accepted |
| Commit | 28fc87ee3ae8730fc556757cff05480b5cf94381 |
| Headers | show |
| Series | efi_selftest: Update SIMPLE_NETWORK_PROTOCOL selftest | expand |
On 9/16/21 10:53 AM, Masami Hiramatsu wrote: > Repeatedly receive the packets until the receive buffer is empty. > If the buffer is empty, EFI_SIMPLE_NETWORK_PROTOCOL::Receive() > returns EFI_NOT_READY. We don't need to use the wait_for_event() > every time. Why do you want to make the change? Was anything not working with the prior version? Best regards Heinrich > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > --- > lib/efi_selftest/efi_selftest_snp.c | 67 +++++++++++++++++++---------------- > 1 file changed, 37 insertions(+), 30 deletions(-) > > diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c > index c5366c872c..818cbfcacd 100644 > --- a/lib/efi_selftest/efi_selftest_snp.c > +++ b/lib/efi_selftest/efi_selftest_snp.c > @@ -362,39 +362,46 @@ static int execute(void) > continue; > } > /* > - * Receive packet > + * Receive packets until buffer is empty > */ > - buffer_size = sizeof(buffer); > - 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; > + for (;;) { > + buffer_size = sizeof(buffer); > + ret = net->receive(net, NULL, &buffer_size, &buffer, > + &srcaddr, &destaddr, NULL); > + if (ret == EFI_NOT_READY) { > + /* The received buffer is empty. */ > + break; > + } > + > + if (ret != EFI_SUCCESS) { > + efi_st_error("Failed to receive packet"); > + return EFI_ST_FAILURE; > + } > + /* > + * Check the packet is meant for this system. > + * Unfortunately QEMU ignores the broadcast flag. > + * So we have to check for broadcasts too. > + */ > + if (memcmp(&destaddr, &net->mode->current_address, ARP_HLEN) && > + memcmp(&destaddr, BROADCAST_MAC, ARP_HLEN)) > + continue; > + /* > + * Check this is a DHCP reply > + */ > + if (buffer.p.eth_hdr.et_protlen != ntohs(PROT_IP) || > + buffer.p.ip_udp.ip_hl_v != 0x45 || > + buffer.p.ip_udp.ip_p != IPPROTO_UDP || > + buffer.p.ip_udp.udp_src != ntohs(67) || > + buffer.p.ip_udp.udp_dst != ntohs(68) || > + buffer.p.dhcp_hdr.op != BOOTREPLY) > + continue; > + /* > + * We successfully received a DHCP reply. > + */ > + goto received; > } > - /* > - * Check the packet is meant for this system. > - * Unfortunately QEMU ignores the broadcast flag. > - * So we have to check for broadcasts too. > - */ > - if (memcmp(&destaddr, &net->mode->current_address, ARP_HLEN) && > - memcmp(&destaddr, BROADCAST_MAC, ARP_HLEN)) > - continue; > - /* > - * Check this is a DHCP reply > - */ > - if (buffer.p.eth_hdr.et_protlen != ntohs(PROT_IP) || > - buffer.p.ip_udp.ip_hl_v != 0x45 || > - buffer.p.ip_udp.ip_p != IPPROTO_UDP || > - buffer.p.ip_udp.udp_src != ntohs(67) || > - buffer.p.ip_udp.udp_dst != ntohs(68) || > - buffer.p.dhcp_hdr.op != BOOTREPLY) > - continue; > - /* > - * We successfully received a DHCP reply. > - */ > - break; > } > - > +received: > /* > * Write a log message. > */ >
Hi Heinrich, 2021年9月16日(木) 18:29 Heinrich Schuchardt <xypron.glpk@gmx.de>: > > > > On 9/16/21 10:53 AM, Masami Hiramatsu wrote: > > Repeatedly receive the packets until the receive buffer is empty. > > If the buffer is empty, EFI_SIMPLE_NETWORK_PROTOCOL::Receive() > > returns EFI_NOT_READY. We don't need to use the wait_for_event() > > every time. > > Why do you want to make the change? > Was anything not working with the prior version? Actually, if [2/3] is merged, our problem will be solved. :-) So this is optional for me. This is a kind of optimization. If we run this on the busy network, the interface can receive more than 2 packets until the wait_for_event() returns. This will avoid calling wait_for_event() in such case, and also it can test the net->receive() returns EFI_NOT_READY correctly. Thank you, > > Best regards > > Heinrich > > > > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> > > --- > > lib/efi_selftest/efi_selftest_snp.c | 67 +++++++++++++++++++---------------- > > 1 file changed, 37 insertions(+), 30 deletions(-) > > > > diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c > > index c5366c872c..818cbfcacd 100644 > > --- a/lib/efi_selftest/efi_selftest_snp.c > > +++ b/lib/efi_selftest/efi_selftest_snp.c > > @@ -362,39 +362,46 @@ static int execute(void) > > continue; > > } > > /* > > - * Receive packet > > + * Receive packets until buffer is empty > > */ > > - buffer_size = sizeof(buffer); > > - 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; > > + for (;;) { > > + buffer_size = sizeof(buffer); > > + ret = net->receive(net, NULL, &buffer_size, &buffer, > > + &srcaddr, &destaddr, NULL); > > + if (ret == EFI_NOT_READY) { > > + /* The received buffer is empty. */ > > + break; > > + } > > + > > + if (ret != EFI_SUCCESS) { > > + efi_st_error("Failed to receive packet"); > > + return EFI_ST_FAILURE; > > + } > > + /* > > + * Check the packet is meant for this system. > > + * Unfortunately QEMU ignores the broadcast flag. > > + * So we have to check for broadcasts too. > > + */ > > + if (memcmp(&destaddr, &net->mode->current_address, ARP_HLEN) && > > + memcmp(&destaddr, BROADCAST_MAC, ARP_HLEN)) > > + continue; > > + /* > > + * Check this is a DHCP reply > > + */ > > + if (buffer.p.eth_hdr.et_protlen != ntohs(PROT_IP) || > > + buffer.p.ip_udp.ip_hl_v != 0x45 || > > + buffer.p.ip_udp.ip_p != IPPROTO_UDP || > > + buffer.p.ip_udp.udp_src != ntohs(67) || > > + buffer.p.ip_udp.udp_dst != ntohs(68) || > > + buffer.p.dhcp_hdr.op != BOOTREPLY) > > + continue; > > + /* > > + * We successfully received a DHCP reply. > > + */ > > + goto received; > > } > > - /* > > - * Check the packet is meant for this system. > > - * Unfortunately QEMU ignores the broadcast flag. > > - * So we have to check for broadcasts too. > > - */ > > - if (memcmp(&destaddr, &net->mode->current_address, ARP_HLEN) && > > - memcmp(&destaddr, BROADCAST_MAC, ARP_HLEN)) > > - continue; > > - /* > > - * Check this is a DHCP reply > > - */ > > - if (buffer.p.eth_hdr.et_protlen != ntohs(PROT_IP) || > > - buffer.p.ip_udp.ip_hl_v != 0x45 || > > - buffer.p.ip_udp.ip_p != IPPROTO_UDP || > > - buffer.p.ip_udp.udp_src != ntohs(67) || > > - buffer.p.ip_udp.udp_dst != ntohs(68) || > > - buffer.p.dhcp_hdr.op != BOOTREPLY) > > - continue; > > - /* > > - * We successfully received a DHCP reply. > > - */ > > - break; > > } > > - > > +received: > > /* > > * Write a log message. > > */ > > -- Masami Hiramatsu
diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c index c5366c872c..818cbfcacd 100644 --- a/lib/efi_selftest/efi_selftest_snp.c +++ b/lib/efi_selftest/efi_selftest_snp.c @@ -362,39 +362,46 @@ static int execute(void) continue; } /* - * Receive packet + * Receive packets until buffer is empty */ - buffer_size = sizeof(buffer); - 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; + for (;;) { + buffer_size = sizeof(buffer); + ret = net->receive(net, NULL, &buffer_size, &buffer, + &srcaddr, &destaddr, NULL); + if (ret == EFI_NOT_READY) { + /* The received buffer is empty. */ + break; + } + + if (ret != EFI_SUCCESS) { + efi_st_error("Failed to receive packet"); + return EFI_ST_FAILURE; + } + /* + * Check the packet is meant for this system. + * Unfortunately QEMU ignores the broadcast flag. + * So we have to check for broadcasts too. + */ + if (memcmp(&destaddr, &net->mode->current_address, ARP_HLEN) && + memcmp(&destaddr, BROADCAST_MAC, ARP_HLEN)) + continue; + /* + * Check this is a DHCP reply + */ + if (buffer.p.eth_hdr.et_protlen != ntohs(PROT_IP) || + buffer.p.ip_udp.ip_hl_v != 0x45 || + buffer.p.ip_udp.ip_p != IPPROTO_UDP || + buffer.p.ip_udp.udp_src != ntohs(67) || + buffer.p.ip_udp.udp_dst != ntohs(68) || + buffer.p.dhcp_hdr.op != BOOTREPLY) + continue; + /* + * We successfully received a DHCP reply. + */ + goto received; } - /* - * Check the packet is meant for this system. - * Unfortunately QEMU ignores the broadcast flag. - * So we have to check for broadcasts too. - */ - if (memcmp(&destaddr, &net->mode->current_address, ARP_HLEN) && - memcmp(&destaddr, BROADCAST_MAC, ARP_HLEN)) - continue; - /* - * Check this is a DHCP reply - */ - if (buffer.p.eth_hdr.et_protlen != ntohs(PROT_IP) || - buffer.p.ip_udp.ip_hl_v != 0x45 || - buffer.p.ip_udp.ip_p != IPPROTO_UDP || - buffer.p.ip_udp.udp_src != ntohs(67) || - buffer.p.ip_udp.udp_dst != ntohs(68) || - buffer.p.dhcp_hdr.op != BOOTREPLY) - continue; - /* - * We successfully received a DHCP reply. - */ - break; } - +received: /* * Write a log message. */
Repeatedly receive the packets until the receive buffer is empty. If the buffer is empty, EFI_SIMPLE_NETWORK_PROTOCOL::Receive() returns EFI_NOT_READY. We don't need to use the wait_for_event() every time. Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org> --- lib/efi_selftest/efi_selftest_snp.c | 67 +++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 30 deletions(-)