patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9751, UDP client/serveur support in...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9751: UDP client/serveur support in lwiperf

Submitter:  David GIRAULT <dgirault>
Submitted:  Tue 29 Jan 2019 03:53:15 PM UTC
   
 
Category:  apps Priority:  5 - Normal
Status:  In Progress Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Open
Planned Release:  None

Jump to the original submission

Wed 17 Jul 2019 07:33:30 PM UTC, comment #11: 


comment #10:

> Hello,
>
> Is this patch is going to be merged?


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

Simon Goldschmidt <goldsimon>
Group administrator
Thu 21 Mar 2019 12:30:17 PM UTC, comment #10: 

Hello,

Is this patch is going to be merged?

Reda Maher <redamaher>
Tue 05 Feb 2019 08:09:36 AM UTC, comment #9: 


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

David GIRAULT <dgirault>
Fri 01 Feb 2019 04:03:56 PM UTC, comment #8: 


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

Simon Goldschmidt <goldsimon>
Group administrator
Fri 01 Feb 2019 03:10:47 PM UTC, comment #7: 


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

David GIRAULT <dgirault>
Wed 30 Jan 2019 06:35:06 PM UTC, comment #6: 


> 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?

Simon Goldschmidt <goldsimon>
Group administrator
Wed 30 Jan 2019 06:33:07 PM UTC, comment #5: 

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

Simon Goldschmidt <goldsimon>
Group administrator
Wed 30 Jan 2019 05:44:24 PM UTC, comment #4: 

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.

David GIRAULT <dgirault>
Wed 30 Jan 2019 04:55:28 PM UTC, comment #3: 

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?

Simon Goldschmidt <goldsimon>
Group administrator
Wed 30 Jan 2019 03:20:46 PM UTC, comment #2: 

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.

David GIRAULT <dgirault>
Wed 30 Jan 2019 02:57:35 PM UTC, 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.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 29 Jan 2019 03:53:15 PM UTC, original submission:  

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

David GIRAULT <dgirault>

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by spoboril
  • -email is unavailable- added by redamaher (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by dgirault (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-04-04 spoboril Carbon-Copy- Added spoboril
    2019-01-30 goldsimon StatusNone In Progress
        Assigned toNone goldsimon
    2019-01-29 dgirault Attached File- Added 0001-lwiperf-added-support-for-UDP-client-server-mode.patch, #46123
        Attached File- Added 0002-lwiperf-small-fixes-on-TCP-client-server-mode.patch, #46124

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code