[PATCH 0/3] efi_selftest: Update SIMPLE_NETWORK_PROTOCOL selftest

[PATCH 0/3] efi_selftest: Update SIMPLE_NETWORK_PROTOCOL selftest

[Masami Hiramatsu]

Hello Heinrich,

Here is a series of patches to update the SIMPLE_NETWORK_PROTOCOL
according to the explanation in the previous thread [1].

[1] https://lists.denx.de/pipermail/u-boot/2021-September/460711.html

So basically this seires modifies the SNP testcase as I said
in the previous mail [1].

----
net->get_status();
if (!net->mode.MediaPresent) {
   error(no link up!)
   return;
}

submit_dhcp_discover()
for (;;) {
   wait_for_event(net)
   while (net->receive() != EFI_NOT_READY) {
      // check dhcp reply
   }
}
----

I removed EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT check because
that is just expectation what the received packet avaiability
is meaning that the EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT flag
bit is set. Of course U-Boot EFI SNP implementation does it,
but that is not ensured in the UEFI spec. The spec said that
the get_status() should update the MediaPresent flag (which
means the network link up or down). So I added the get_status()
test case before starting the network test so that it can
test the link status.

BTW, actually the mode->media_present is not supported yet.
Is there any way to get the network link status?

Thank you,

---

Masami Hiramatsu (3):
      efi_selftest: Use EFI_SIMPLE_NETWORK_PROTOCOL::GetStatus() for media check
      efi_selftest: Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT
      efi_selftest: Recieve the packets until the receive buffer is empty


 lib/efi_selftest/efi_selftest_snp.c |   90 +++++++++++++++++++----------------
 1 file changed, 49 insertions(+), 41 deletions(-)

--
Masami Hiramatsu <masami.hiramatsu@linaro.org>

[PATCH 1/3] efi_selftest: Use EFI_SIMPLE_NETWORK_PROTOCOL::GetStatus() for media check

[Masami Hiramatsu]

According to the UEF specification v2.9, the main purpose of the
EFI_SIMPLE_NETWORK_PROTOCOL::GetStatus() is for checking the link
status via EFI_SIMPLE_NETWORK_MODE::MediaPresent.
So this uses net->get_status() for checking the link status before
running network test.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 lib/efi_selftest/efi_selftest_snp.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c
index 79f0467803..cb0db7eea2 100644
--- a/lib/efi_selftest/efi_selftest_snp.c
+++ b/lib/efi_selftest/efi_selftest_snp.c
@@ -309,6 +309,18 @@ static int execute(void)
 		return EFI_ST_FAILURE;
 	}
 
+	/* Check media connected */
+	ret = net->get_status(net, NULL, NULL);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to get status");
+		return EFI_ST_FAILURE;
+	}
+	if (net->mode && net->mode->media_present_supported &&
+	    !net->mode->media_present) {
+		efi_st_error("Network media is not connected");
+		return EFI_ST_FAILURE;
+	}
+
 	/*
 	 * Send DHCP discover message
 	 */

[PATCH 2/3] efi_selftest: Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT

[Masami Hiramatsu]

Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet
receiving loop. This depends on the implementation and not
related to whether the packet can be received or not.

Whether the received packets are available or not is ensured
by wait_for_packet, and that is already done in the loop.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
---
 lib/efi_selftest/efi_selftest_snp.c |   11 -----------
 1 file changed, 11 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c
index cb0db7eea2..c5366c872c 100644
--- a/lib/efi_selftest/efi_selftest_snp.c
+++ b/lib/efi_selftest/efi_selftest_snp.c
@@ -340,8 +340,6 @@ 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.
 		 */
@@ -367,15 +365,6 @@ static int execute(void)
 		 * Receive packet
 		 */
 		buffer_size = sizeof(buffer);
-		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) {

Re: [PATCH 2/3] efi_selftest: Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT

[Heinrich Schuchardt]

On 9/16/21 10:53 AM, Masami Hiramatsu wrote:
> Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet
> receiving loop. This depends on the implementation and not
> related to whether the packet can be received or not.

Shouldn't EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT always be set if a
package needs to be processed?

Best regards

Heinrich

>
> Whether the received packets are available or not is ensured
> by wait_for_packet, and that is already done in the loop.
>
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> ---
>   lib/efi_selftest/efi_selftest_snp.c |   11 -----------
>   1 file changed, 11 deletions(-)
>
> diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c
> index cb0db7eea2..c5366c872c 100644
> --- a/lib/efi_selftest/efi_selftest_snp.c
> +++ b/lib/efi_selftest/efi_selftest_snp.c
> @@ -340,8 +340,6 @@ 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.
>   		 */
> @@ -367,15 +365,6 @@ static int execute(void)
>   		 * Receive packet
>   		 */
>   		buffer_size = sizeof(buffer);
> -		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) {
>

Re: [PATCH 2/3] efi_selftest: Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT

[Masami Hiramatsu]

Hi Heinrich,

2021年9月16日(木) 18:30 Heinrich Schuchardt <xypron.glpk@gmx.de>:
>
>
>
> On 9/16/21 10:53 AM, Masami Hiramatsu wrote:
> > Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT in packet
> > receiving loop. This depends on the implementation and not
> > related to whether the packet can be received or not.
>
> Shouldn't EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT always be set if a
> package needs to be processed?

No, that is not promised as far as I can read the specification.
It is just a bit flag of the device (Spec says "the interrupt status will be
read from the device."). Not ensure the received packet buffer status.

However, the WaitForPacket ensures the packet is received.
"Event used with EFI_BOOT_SERVICES.WaitForEvent() to wait
for a packet to be received."

So I think it is enough to check the WaitForPacket.

Thank you,


>
> Best regards
>
> Heinrich
>
> >
> > Whether the received packets are available or not is ensured
> > by wait_for_packet, and that is already done in the loop.
> >
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > ---
> >   lib/efi_selftest/efi_selftest_snp.c |   11 -----------
> >   1 file changed, 11 deletions(-)
> >
> > diff --git a/lib/efi_selftest/efi_selftest_snp.c b/lib/efi_selftest/efi_selftest_snp.c
> > index cb0db7eea2..c5366c872c 100644
> > --- a/lib/efi_selftest/efi_selftest_snp.c
> > +++ b/lib/efi_selftest/efi_selftest_snp.c
> > @@ -340,8 +340,6 @@ 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.
> >                */
> > @@ -367,15 +365,6 @@ static int execute(void)
> >                * Receive packet
> >                */
> >               buffer_size = sizeof(buffer);
> > -             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) {
> >



--
Masami Hiramatsu

[PATCH 3/3] efi_selftest: Recieve the packets until the receive buffer is empty

[Masami Hiramatsu]

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(-)

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.
 	 */

Re: [PATCH 3/3] efi_selftest: Recieve the packets until the receive buffer is empty

[Heinrich Schuchardt]

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.
>   	 */
>

Re: [PATCH 3/3] efi_selftest: Recieve the packets until the receive buffer is empty

[Masami Hiramatsu]

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

Re: [PATCH 0/3] efi_selftest: Update SIMPLE_NETWORK_PROTOCOL selftest

[Heinrich Schuchardt]

On 9/16/21 10:53 AM, Masami Hiramatsu wrote:
> Hello Heinrich,
>
> Here is a series of patches to update the SIMPLE_NETWORK_PROTOCOL
> according to the explanation in the previous thread [1].
>
> [1] https://lists.denx.de/pipermail/u-boot/2021-September/460711.html
>
> So basically this seires modifies the SNP testcase as I said
> in the previous mail [1].
>
> ----
> net->get_status();
> if (!net->mode.MediaPresent) {
>     error(no link up!)
>     return;
> }
>
> submit_dhcp_discover()
> for (;;) {
>     wait_for_event(net)
>     while (net->receive() != EFI_NOT_READY) {
>        // check dhcp reply
>     }
> }
> ----
>
> I removed EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT check because
> that is just expectation what the received packet avaiability
> is meaning that the EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT flag
> bit is set. Of course U-Boot EFI SNP implementation does it,
> but that is not ensured in the UEFI spec. The spec said that

SN_GetStatus() in edk2-platforms
Drivers/ASIX/Bus/Usb/UsbNetworking/Ax88179/SimpleNetwork.c always
returns *InterruptStatus = 0.

> the get_status() should update the MediaPresent flag (which
> means the network link up or down). So I added the get_status()
> test case before starting the network test so that it can
> test the link status.
>
> BTW, actually the mode->media_present is not supported yet.
> Is there any way to get the network link status?

The driver interface struct eth_ops has no method for determining media
presence. We have to assume that it is always present.

Best regards

Heinrich

>
> Thank you,
>
> ---
>
> Masami Hiramatsu (3):
>        efi_selftest: Use EFI_SIMPLE_NETWORK_PROTOCOL::GetStatus() for media check
>        efi_selftest: Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT
>        efi_selftest: Recieve the packets until the receive buffer is empty
>
>
>   lib/efi_selftest/efi_selftest_snp.c |   90 +++++++++++++++++++----------------
>   1 file changed, 49 insertions(+), 41 deletions(-)
>
> --
> Masami Hiramatsu <masami.hiramatsu@linaro.org>
>

Re: [PATCH 0/3] efi_selftest: Update SIMPLE_NETWORK_PROTOCOL selftest

[Masami Hiramatsu]

Hi Heinrich,

2021年9月17日(金) 12:54 Heinrich Schuchardt <xypron.glpk@gmx.de>:
>
> On 9/16/21 10:53 AM, Masami Hiramatsu wrote:
> > Hello Heinrich,
> >
> > Here is a series of patches to update the SIMPLE_NETWORK_PROTOCOL
> > according to the explanation in the previous thread [1].
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2021-September/460711.html
> >
> > So basically this seires modifies the SNP testcase as I said
> > in the previous mail [1].
> >
> > ----
> > net->get_status();
> > if (!net->mode.MediaPresent) {
> >     error(no link up!)
> >     return;
> > }
> >
> > submit_dhcp_discover()
> > for (;;) {
> >     wait_for_event(net)
> >     while (net->receive() != EFI_NOT_READY) {
> >        // check dhcp reply
> >     }
> > }
> > ----
> >
> > I removed EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT check because
> > that is just expectation what the received packet avaiability
> > is meaning that the EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT flag
> > bit is set. Of course U-Boot EFI SNP implementation does it,
> > but that is not ensured in the UEFI spec. The spec said that
>
> SN_GetStatus() in edk2-platforms
> Drivers/ASIX/Bus/Usb/UsbNetworking/Ax88179/SimpleNetwork.c always
> returns *InterruptStatus = 0.

Yes, it seems to depend on driver implementation.

>
> > the get_status() should update the MediaPresent flag (which
> > means the network link up or down). So I added the get_status()
> > test case before starting the network test so that it can
> > test the link status.
> >
> > BTW, actually the mode->media_present is not supported yet.
> > Is there any way to get the network link status?
>
> The driver interface struct eth_ops has no method for determining media
> presence. We have to assume that it is always present.

Hmm, OK. So we can not implement it...

Thank you,

>
> Best regards
>
> Heinrich
>
> >
> > Thank you,
> >
> > ---
> >
> > Masami Hiramatsu (3):
> >        efi_selftest: Use EFI_SIMPLE_NETWORK_PROTOCOL::GetStatus() for media check
> >        efi_selftest: Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT
> >        efi_selftest: Recieve the packets until the receive buffer is empty
> >
> >
> >   lib/efi_selftest/efi_selftest_snp.c |   90 +++++++++++++++++++----------------
> >   1 file changed, 49 insertions(+), 41 deletions(-)
> >
> > --
> > Masami Hiramatsu <masami.hiramatsu@linaro.org>
> >
>


-- 
Masami Hiramatsu

Re: [PATCH 0/3] efi_selftest: Update SIMPLE_NETWORK_PROTOCOL selftest

[Masami Hiramatsu]

Hi Heinrich,

What is the current status of this patch series?
Could you give me any comments?

Thank you,

2021年9月17日(金) 13:47 Masami Hiramatsu <masami.hiramatsu@linaro.org>:
>
> Hi Heinrich,
>
> 2021年9月17日(金) 12:54 Heinrich Schuchardt <xypron.glpk@gmx.de>:
> >
> > On 9/16/21 10:53 AM, Masami Hiramatsu wrote:
> > > Hello Heinrich,
> > >
> > > Here is a series of patches to update the SIMPLE_NETWORK_PROTOCOL
> > > according to the explanation in the previous thread [1].
> > >
> > > [1] https://lists.denx.de/pipermail/u-boot/2021-September/460711.html
> > >
> > > So basically this seires modifies the SNP testcase as I said
> > > in the previous mail [1].
> > >
> > > ----
> > > net->get_status();
> > > if (!net->mode.MediaPresent) {
> > >     error(no link up!)
> > >     return;
> > > }
> > >
> > > submit_dhcp_discover()
> > > for (;;) {
> > >     wait_for_event(net)
> > >     while (net->receive() != EFI_NOT_READY) {
> > >        // check dhcp reply
> > >     }
> > > }
> > > ----
> > >
> > > I removed EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT check because
> > > that is just expectation what the received packet avaiability
> > > is meaning that the EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT flag
> > > bit is set. Of course U-Boot EFI SNP implementation does it,
> > > but that is not ensured in the UEFI spec. The spec said that
> >
> > SN_GetStatus() in edk2-platforms
> > Drivers/ASIX/Bus/Usb/UsbNetworking/Ax88179/SimpleNetwork.c always
> > returns *InterruptStatus = 0.
>
> Yes, it seems to depend on driver implementation.
>
> >
> > > the get_status() should update the MediaPresent flag (which
> > > means the network link up or down). So I added the get_status()
> > > test case before starting the network test so that it can
> > > test the link status.
> > >
> > > BTW, actually the mode->media_present is not supported yet.
> > > Is there any way to get the network link status?
> >
> > The driver interface struct eth_ops has no method for determining media
> > presence. We have to assume that it is always present.
>
> Hmm, OK. So we can not implement it...
>
> Thank you,
>
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > Thank you,
> > >
> > > ---
> > >
> > > Masami Hiramatsu (3):
> > >        efi_selftest: Use EFI_SIMPLE_NETWORK_PROTOCOL::GetStatus() for media check
> > >        efi_selftest: Do not check EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT
> > >        efi_selftest: Recieve the packets until the receive buffer is empty
> > >
> > >
> > >   lib/efi_selftest/efi_selftest_snp.c |   90 +++++++++++++++++++----------------
> > >   1 file changed, 49 insertions(+), 41 deletions(-)
> > >
> > > --
> > > Masami Hiramatsu <masami.hiramatsu@linaro.org>
> > >
> >
>
>
> --
> Masami Hiramatsu



-- 
Masami Hiramatsu