mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [RFC] net: picoping: try to make it asynchronious: fail
@ 2014-05-30  9:14 Antony Pavlov
  2014-05-30  9:36 ` Holger Schurig
  2014-06-02  8:38 ` Sascha Hauer
  0 siblings, 2 replies; 5+ messages in thread
From: Antony Pavlov @ 2014-05-30  9:14 UTC (permalink / raw)
  To: barebox; +Cc: Daniele Lacamera

Picotcp tends to work in an asynchronious way.

E.g. for ping we have to use "pico_icmp4_ping()"
(to start ping task) and the special icmp4 handler
(callback function) "cb_ping()".

But the ping command in barebox works in very simple
way:

  * user types 'ping <IP>' in the barebox shell command line;
  * the ping command takes all control until it gets the positive
    or a negative result;
  * the control is returned to the barebox shell.

To use asynchronious-oriented picotcp functions in syncronious-oriented
barebox workflow we have to work out necessary picotcp usage template.

This patch propose simplest template. But this template has two major
disadvantages:

  * user can't completely cancel ping using ctrl-c,
    the ping request send task continue to work in the background:

    barebox@barebox sandbox:/ picoping 10.0.0.2
    48 bytes from 10.0.0.2: icmp_req=1 ttl=64 time=0 ms
    48 bytes from 10.0.0.2: icmp_req=2 ttl=64 time=0 ms
    48 bytes from 10.0.0.2: icmp_req=3 ttl=64 time=0 ms
    <ctrl-c> pressed
    barebox@barebox sandbox:/ picoping 10.0.0.2
    48 bytes from 10.0.0.2: icmp_req=1 ttl=64 time=1 ms
    48 bytes from 10.0.0.2: icmp_req=5 ttl=64 time=0 ms
    48 bytes from 10.0.0.2: icmp_req=2 ttl=64 time=0 ms
    48 bytes from 10.0.0.2: icmp_req=6 ttl=64 time=0 ms
    48 bytes from 10.0.0.2: icmp_req=3 ttl=64 time=0 ms
    48 bytes from 10.0.0.2: icmp_req=7 ttl=64 time=0 ms
    48 bytes from 10.0.0.2: icmp_req=4 ttl=64 time=0 ms
    48 bytes from 10.0.0.2: icmp_req=8 ttl=64 time=0 ms

  * user has to press <ctrl-c> after the last ping response is received
    to return to barebox shell.

Has anybody any ideas how to improve the situation?
---
 net/test_picotcp.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/net/test_picotcp.c b/net/test_picotcp.c
index 5f7138e..035ef09 100644
--- a/net/test_picotcp.c
+++ b/net/test_picotcp.c
@@ -98,9 +98,15 @@ BAREBOX_CMD_START(test_picotcp)
 BAREBOX_CMD_END
 
 #include <pico_icmp4.h>
+#include <poller.h>
 
 #define NUM_PING 10
 
+#define PING_STATE_WORK		0
+#define PING_STATE_DONE		1
+
+static int ping_state;
+
 /* callback function for receiving ping reply */
 void cb_ping(struct pico_icmp4_stats *s)
 {
@@ -108,6 +114,10 @@ void cb_ping(struct pico_icmp4_stats *s)
 	int time_sec = 0;
 	int time_msec = 0;
 
+	if (ping_state == PING_STATE_DONE) {
+		return;
+	}
+
 	/* convert ip address from icmp4_stats structure to string */
 	pico_ipv4_to_string(host, s->dst.addr);
 
@@ -132,8 +142,19 @@ static int do_picoping(int argc, char *argv[])
 		return 1;
 	}
 
+	ping_state = PING_STATE_WORK;
+
 	pico_icmp4_ping(argv[1], NUM_PING, 1000, 5000, 48, cb_ping);
 
+	while (ping_state != PING_STATE_DONE) {
+		if (ctrlc()) {
+			ping_state = PING_STATE_DONE;
+			break;
+		}
+		get_time_ns();
+		poller_call();
+	}
+
 	return 0;
 }
 
-- 
1.9.2


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] net: picoping: try to make it asynchronious: fail
  2014-05-30  9:14 [RFC] net: picoping: try to make it asynchronious: fail Antony Pavlov
@ 2014-05-30  9:36 ` Holger Schurig
  2014-06-02  8:38 ` Sascha Hauer
  1 sibling, 0 replies; 5+ messages in thread
From: Holger Schurig @ 2014-05-30  9:36 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox, Daniele Lacamera

> Has anybody any ideas how to improve the situation?

AFAIK barebox doesn't have the concept of aborting something. e.g. you
cannot do "msleep 20000" and then press Ctrl-C. The <INTERRUPT> you
see when you press Ctrl-C is from hush, and it is internal to hush.c
(see struct in_str, member interrupt).

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] net: picoping: try to make it asynchronious: fail
  2014-05-30  9:14 [RFC] net: picoping: try to make it asynchronious: fail Antony Pavlov
  2014-05-30  9:36 ` Holger Schurig
@ 2014-06-02  8:38 ` Sascha Hauer
  2014-06-02  9:43   ` Daniele Lacamera
  1 sibling, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2014-06-02  8:38 UTC (permalink / raw)
  To: Antony Pavlov; +Cc: barebox, Daniele Lacamera

On Fri, May 30, 2014 at 01:14:08PM +0400, Antony Pavlov wrote:
> Picotcp tends to work in an asynchronious way.
> 
> E.g. for ping we have to use "pico_icmp4_ping()"
> (to start ping task) and the special icmp4 handler
> (callback function) "cb_ping()".
> 
> But the ping command in barebox works in very simple
> way:
> 
>   * user types 'ping <IP>' in the barebox shell command line;
>   * the ping command takes all control until it gets the positive
>     or a negative result;
>   * the control is returned to the barebox shell.
> 
> To use asynchronious-oriented picotcp functions in syncronious-oriented
> barebox workflow we have to work out necessary picotcp usage template.
> 
> This patch propose simplest template. But this template has two major
> disadvantages:
> 
>   * user can't completely cancel ping using ctrl-c,
>     the ping request send task continue to work in the background:
> 
>     barebox@barebox sandbox:/ picoping 10.0.0.2
>     48 bytes from 10.0.0.2: icmp_req=1 ttl=64 time=0 ms
>     48 bytes from 10.0.0.2: icmp_req=2 ttl=64 time=0 ms
>     48 bytes from 10.0.0.2: icmp_req=3 ttl=64 time=0 ms
>     <ctrl-c> pressed
>     barebox@barebox sandbox:/ picoping 10.0.0.2
>     48 bytes from 10.0.0.2: icmp_req=1 ttl=64 time=1 ms
>     48 bytes from 10.0.0.2: icmp_req=5 ttl=64 time=0 ms
>     48 bytes from 10.0.0.2: icmp_req=2 ttl=64 time=0 ms
>     48 bytes from 10.0.0.2: icmp_req=6 ttl=64 time=0 ms
>     48 bytes from 10.0.0.2: icmp_req=3 ttl=64 time=0 ms
>     48 bytes from 10.0.0.2: icmp_req=7 ttl=64 time=0 ms
>     48 bytes from 10.0.0.2: icmp_req=4 ttl=64 time=0 ms
>     48 bytes from 10.0.0.2: icmp_req=8 ttl=64 time=0 ms
> 
>   * user has to press <ctrl-c> after the last ping response is received
>     to return to barebox shell.
> 
> Has anybody any ideas how to improve the situation?

This must be solved in the pico_icmp4.c code. The counterpart of
pico_tree_insert (pico_tree_delete?) must be called as an response to
ctrlc().

How I see it pico_icmp4_ping should return the cookie so that a ping
abort function can be implemented:

void pico_icmp4_ping_abort(struct pico_icmp4_ping_cookie *cookie)
{
	pico_tree_delete(cookie);

	release_whatever_resources();
}

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] net: picoping: try to make it asynchronious: fail
  2014-06-02  8:38 ` Sascha Hauer
@ 2014-06-02  9:43   ` Daniele Lacamera
  2014-06-02 12:36     ` Daniele Lacamera
  0 siblings, 1 reply; 5+ messages in thread
From: Daniele Lacamera @ 2014-06-02  9:43 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

On Mon, Jun 2, 2014 at 10:38 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Fri, May 30, 2014 at 01:14:08PM +0400, Antony Pavlov wrote:
> This must be solved in the pico_icmp4.c code. The counterpart of
> pico_tree_insert (pico_tree_delete?) must be called as an response to
> ctrlc().
>
> How I see it pico_icmp4_ping should return the cookie so that a ping
> abort function can be implemented:
>
> void pico_icmp4_ping_abort(struct pico_icmp4_ping_cookie *cookie)
> {
>         pico_tree_delete(cookie);
>
>         release_whatever_resources();
> }
>

Sascha,

I agree on this point. We are already working to provide abort calls
for both ping operations and DHCP transactions.
I will keep you posted.

Thanks

-- 
Daniele Lacamera

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [RFC] net: picoping: try to make it asynchronious: fail
  2014-06-02  9:43   ` Daniele Lacamera
@ 2014-06-02 12:36     ` Daniele Lacamera
  0 siblings, 0 replies; 5+ messages in thread
From: Daniele Lacamera @ 2014-06-02 12:36 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: barebox

> On Mon, Jun 2, 2014 at 10:38 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:

>> How I see it pico_icmp4_ping should return the cookie so that a ping
>> abort function can be implemented:
>>
>> void pico_icmp4_ping_abort(struct pico_icmp4_ping_cookie *cookie)

Hello Sascha, et al.

PicoTCP now supports abort operations for ping (both IPv4 and IPv6).
Our documentation has been updated accordingly.

I wrote a quick test on top of Antony's proposal, and it seems to work
as expected. Now the ping can be interrupted using ctrl+c, and the
operation is terminated.

See my version of the example over here (requires the newest
modules/pico_icmpv4.c and .h from picotcp:0c5b5e3a2d) :

http://pastebin.com/bG3tRZzm

I am going to do the same with the dhcp client shortly.

Regards,

-- 
Daniele

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-06-02 12:36 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-30  9:14 [RFC] net: picoping: try to make it asynchronious: fail Antony Pavlov
2014-05-30  9:36 ` Holger Schurig
2014-06-02  8:38 ` Sascha Hauer
2014-06-02  9:43   ` Daniele Lacamera
2014-06-02 12:36     ` Daniele Lacamera

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox