[PATCH] net: ping: reset stored IP once the command finishes

[PATCH] net: ping: reset stored IP once the command finishes

[Marek Szyprowski]

Reset stored ping IP address before leaving the netloop to ensure that
the subsequent calls to the netloop, especially for the other protocols,
won't be interrupted by the received ICMP_ECHO_REPLY packet.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 net/ping.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/ping.c b/net/ping.c
index 633c942..d912e3d 100644
--- a/net/ping.c
+++ b/net/ping.c
@@ -63,6 +63,7 @@ static int ping_send(void)
 static void ping_timeout_handler(void)
 {
 	eth_halt();
+	net_ping_ip.s_addr = 0;
 	net_set_state(NETLOOP_FAIL);	/* we did not get the reply */
 }
 
@@ -84,8 +85,10 @@ void ping_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len)
 	switch (icmph->type) {
 	case ICMP_ECHO_REPLY:
 		src_ip = net_read_ip((void *)&ip->ip_src);
-		if (src_ip.s_addr == net_ping_ip.s_addr)
+		if (src_ip.s_addr == net_ping_ip.s_addr) {
+			net_ping_ip.s_addr = 0;
 			net_set_state(NETLOOP_SUCCESS);
+		}
 		return;
 	case ICMP_ECHO_REQUEST:
 		eth_hdr_size = net_update_ether(et, et->et_src, PROT_IP);
-- 
1.9.1

[PATCH] net: ping: reset stored IP once the command finishes

[Tom Rini]

On Wed, Mar 25, 2020 at 02:42:00PM +0100, Marek Szyprowski wrote:

> Reset stored ping IP address before leaving the netloop to ensure that
> the subsequent calls to the netloop, especially for the other protocols,
> won't be interrupted by the received ICMP_ECHO_REPLY packet.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  net/ping.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ping.c b/net/ping.c
> index 633c942..d912e3d 100644
> --- a/net/ping.c
> +++ b/net/ping.c
> @@ -63,6 +63,7 @@ static int ping_send(void)
>  static void ping_timeout_handler(void)
>  {
>  	eth_halt();
> +	net_ping_ip.s_addr = 0;
>  	net_set_state(NETLOOP_FAIL);	/* we did not get the reply */
>  }
>  
> @@ -84,8 +85,10 @@ void ping_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len)
>  	switch (icmph->type) {
>  	case ICMP_ECHO_REPLY:
>  		src_ip = net_read_ip((void *)&ip->ip_src);
> -		if (src_ip.s_addr == net_ping_ip.s_addr)
> +		if (src_ip.s_addr == net_ping_ip.s_addr) {
> +			net_ping_ip.s_addr = 0;
>  			net_set_state(NETLOOP_SUCCESS);
> +		}
>  		return;
>  	case ICMP_ECHO_REQUEST:
>  		eth_hdr_size = net_update_ether(et, et->et_src, PROT_IP);

This breaks a number of tests in test/dm/eth.c.  Can you please adjust
the tests as well to not rely on the behavior you're fixing?  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200612/a548967d/attachment.sig>

[PATCH] net: ping: reset stored IP once the command finishes

[Marek Szyprowski]

Hi Tom,

On 12.06.2020 18:04, Tom Rini wrote:
> On Wed, Mar 25, 2020 at 02:42:00PM +0100, Marek Szyprowski wrote:
>> Reset stored ping IP address before leaving the netloop to ensure that
>> the subsequent calls to the netloop, especially for the other protocols,
>> won't be interrupted by the received ICMP_ECHO_REPLY packet.
>>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   net/ping.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/ping.c b/net/ping.c
>> index 633c942..d912e3d 100644
>> --- a/net/ping.c
>> +++ b/net/ping.c
>> @@ -63,6 +63,7 @@ static int ping_send(void)
>>   static void ping_timeout_handler(void)
>>   {
>>   	eth_halt();
>> +	net_ping_ip.s_addr = 0;
>>   	net_set_state(NETLOOP_FAIL);	/* we did not get the reply */
>>   }
>>   
>> @@ -84,8 +85,10 @@ void ping_receive(struct ethernet_hdr *et, struct ip_udp_hdr *ip, int len)
>>   	switch (icmph->type) {
>>   	case ICMP_ECHO_REPLY:
>>   		src_ip = net_read_ip((void *)&ip->ip_src);
>> -		if (src_ip.s_addr == net_ping_ip.s_addr)
>> +		if (src_ip.s_addr == net_ping_ip.s_addr) {
>> +			net_ping_ip.s_addr = 0;
>>   			net_set_state(NETLOOP_SUCCESS);
>> +		}
>>   		return;
>>   	case ICMP_ECHO_REQUEST:
>>   		eth_hdr_size = net_update_ether(et, et->et_src, PROT_IP);
> This breaks a number of tests in test/dm/eth.c.  Can you please adjust
> the tests as well to not rely on the behavior you're fixing?  Thanks!

I've checked and it turned out that it is not possible to fix the 
net_retry test. It clearly shows that the net_retry feature relies on 
the fact that the state machine of the net related commands are 
preserved between the calls.

I wonder how to fix this without breaking the netretry feature. The only 
solution I see is to clear net_ping_ip at the beginning of the 
net_loop() if the protocol is not equal to PING. Quite ugly...

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland