[patch #9751] UDP client/serveur support in lwiperf

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
URL:
  <https://savannah.nongnu.org/patch/?9751>

                 Summary: UDP client/serveur support in lwiperf
                 Project: lwIP - A Lightweight TCP/IP stack
            Submitted by: dgirault
            Submitted on: mar. 29 janv. 2019 15:53:15 UTC
                Category: apps
                Priority: 5 - Normal
                  Status: None
                 Privacy: Public
             Assigned to: None
        Originator Email:
             Open/Closed: Open
         Discussion Lock: Any
         Planned Release: None

    _______________________________________________________

Details:

Since we require this for WiFi certification of our product, I've added UDP
modes in lwiperf.c

- Client/server mode are supported
- Setting IP TOS is supported
- dual/tradeoff mode are supported
- setting UDP bandwidth in Kb/s (pps isn't supported)
- setting amount for TCP client is added




    _______________________________________________________

File Attachments:


-------------------------------------------------------
Date: mar. 29 janv. 2019 15:53:15 UTC  Name:
0001-lwiperf-added-support-for-UDP-client-server-mode.patch  Size: 31kio   By:
dgirault

<http://savannah.nongnu.org/patch/download.php?file_id=46123>
-------------------------------------------------------
Date: mar. 29 janv. 2019 15:53:15 UTC  Name:
0002-lwiperf-small-fixes-on-TCP-client-server-mode.patch  Size: 9kio   By:
dgirault

<http://savannah.nongnu.org/patch/download.php?file_id=46124>

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Update of patch #9751 (project lwip):

                  Status:                    None => In Progress            
             Assigned to:                    None => goldsimon              

    _______________________________________________________

Follow-up Comment #1:

Nice work, but it doesn't compile because of:
- using some stdint.h types
- using unknown struct timespec and unix-like 'clock_gettime'
- using unknown keyword 'inline'
- mixing declarations and code

Also, coding style is not lwIP.

I can fix most of the errors if it's OK for you, but I cannot fix your using
struct timeval and clock_gettime. In lwIP, the only time available is
sys_now() returning u32_t.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #2, patch #9751 (project lwip):

Oh my bad, I don't try it on another platform.

We have added implementation of `clock_gettime` in our platform to follow
Linux API, and I miss the `sys_now` lwIP API, but not sure the millisecond
granularity is enough.

conn->delay_target is in microsecond (as in original iperf program).

For the coding style, I almost follow existing coding style of lwiperf.c (and
I read again the contrib.txt where it is explained), so I'm a little
surprised.

Anyway, it's OK for me. I'll take care of your change in my next rebase.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #3, patch #9751 (project lwip):

Coding style things are easily fixed before pushing, I don't reject patches
because of that.

But we really can't make all platforms implement clock_gettime().

I haven't looked at the details, why do you think milliseconds aren't enough?
Can't we just convert from microseconds to milliseconds?

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #4, patch #9751 (project lwip):

According to the delay_target calculation, since buf_len is constant 1470 (or
1450 with IPv6), the calculated delay is:
- 11760µs for 1Mb/s (1000000)
- 1176µs  for 10Mb/s (10000000)
- 373µs   for 30Mb/s (30000000)

Using a timing limited to one microsecond granularity will limit the maximum
UDP bw to 11.76Mb/s.

WiFi certification require UDP bandwidth you can't achieve sending one frame
each millisecond (sorry don't remember the required bw for WiFi N test plan).

You can send more frame for each millisecond but this will impact the
calculation of jitter at the receiving endpoint.

May be we can have a new `u64_t sys_now_us()` to allow fine grained timing in
lwIP ? Defaulting to `sys_now()*1000`.

This let user providing whatever implementation they want. If not provided
maximum UDP bandwidth will be limited.


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #5, patch #9751 (project lwip):

Ok, so now I took the time to review your implementation and I see why you
need the µs timing. However, seeing how you did that I can say this
implementation is not portable. It only works in main loop mode (NO_SYS=1).

For threaded mode, you can't just call 'lwiperf_poll_udp_client' in a loop as
you might have other threads that need to run *AND* have to obey threading
requirements. So you would need to use a timer that calls your code at the
right time, much like 'sys_timemout', but on µs base. I'm not sure how that
would work.

Also, why do you substract '5' from 'conn->delay_target' in the sending loop?
Is that supposed to be the delay from that code line until the packet is on
the wire? Wouldn't that need to be a define that can be adjusted to the
hardware?

> May be we can have a new `u64_t sys_now_us()` to allow fine
> grained timing in lwIP ? Defaulting to `sys_now()*1000`.

That might be an idea, but it doesn't really work as sys_now() overflows after
~49 days. Differences between to u32_t still work, but if you multiplied this
by 1000 and compared 2 u64_t, you'd get a huge negative diff after ~49
days...

To get this pushed, maybe we can start with wrapping the timing in defines and
providing both your 'clock_gettime' version as well as a 32-bit
millisecond-based version. We could then see where it goes...

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #6, patch #9751 (project lwip):

> WiFi certification require UDP bandwidth you can't achieve
> sending one frame each millisecond (sorry don't remember the
> required bw for WiFi N test plan).

Thinking about that again: if you need a 373µs delay to achieve 30Mb/s, would
it work to wait 1ms and send 3 packets? Or do they really need to be evenly
distributed?

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #7, patch #9751 (project lwip):

> It only works in main loop mode (NO_SYS=1).

Yes, I forgot this point when I developed this. Indeed, a timer will have been
much more portable. But since `sys_timeout` is based on miliseconds, I did not
think about it.

Additionally, since lwiperf only use RAW API, I've assumed it is only
compatible with NO_SYS=1 platforms.

> Also, why do you substract '5' from 'conn->delay_target' in the sending
loop? Is that supposed to be the delay from that code line until the packet is
on the wire? Wouldn't that need to be a define that can be adjusted to the
hardware?

Yes, this is a little trick to take care of processing time to send one UDP
frame. It should be dynamically calculated. This value is for our target to
achieve same transmit rate as requested. Original IPerf2 code use a dynamic
adjust in loop (see Client.cpp).

At least, it should be a define.

> To get this pushed, maybe we can start with wrapping the timing in defines
and providing both your 'clock_gettime' version as well as a 32-bit
millisecond-based version. We could then see where it goes...

Yes, I think it's the best way.

> Thinking about that again: if you need a 373µs delay to achieve 30Mb/s,
would it work to wait 1ms and send 3 packets? Or do they really need to be
evenly distributed?

This can be a valid workaround for the BW limits, but this will result in a
very bad jitter result in the receiver. If not evenly distributed, jitter will
be very high (according to my understand of the jitter calculation).


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #8, patch #9751 (project lwip):

> Additionally, since lwiperf only use RAW API, I've assumed it is only
compatible with NO_SYS=1 platforms.

No, RAW API can be used for NO_SYS=0, too.

> Yes, this is a little trick to take care of processing time to send one UDP
frame.

Hmm, if you evenly distribute the time where you call 'udp_send' (or
whatever), why do you need the diff to the wire than? What's the negative
result when leaving this diff out?

> If not evenly distributed, jitter will be very high (according to my
understand of the jitter calculation).

Oh, ok, I hadn't thought about jitter. Maybe it's still a valid solution if
sub-ms timing is not available...

I'll try to merge this by adding defines for the timing.

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #9, patch #9751 (project lwip):

> Hmm, if you evenly distribute the time where you call 'udp_send' (or
whatever), why do you need the diff to the wire than? What's the negative
result when leaving this diff out?

This only impact the effective BW the device can send. If you don't take care
of processing time, you'll never reach BW you specify. Optimal delay is
calculated inter-frame minus processing time.


> I'll try to merge this by adding defines for the timing.

Thank you. Sorry for the extra work.


    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message posté via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #10, patch #9751 (project lwip):

Hello,

Is this patch is going to be merged?

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel
Reply | Threaded
Open this post in threaded view
|

[patch #9751] UDP client/serveur support in lwiperf

David GIRAULT-2
Follow-up Comment #11, patch #9751 (project lwip):


[comment #10 comment #10:]
> Hello,
>
> Is this patch is going to be merged?

Yes. I just haven't found the time for the extra work, yet :-(

    _______________________________________________________

Reply to this item at:

  <https://savannah.nongnu.org/patch/?9751>

_______________________________________________
  Message sent via Savannah
  https://savannah.nongnu.org/


_______________________________________________
lwip-devel mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/lwip-devel