lwIP - 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: |
Simon Goldschmidt <goldsimon>![]() ![]() |
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:
|
David GIRAULT <dgirault> |
Fri 01 Feb 2019 04:03:56 PM UTC, comment #8:
I'll try to merge this by adding defines for the timing. |
Simon Goldschmidt <goldsimon>![]() ![]() |
Fri 01 Feb 2019 03:10:47 PM UTC, comment #7:
Additionally, since lwiperf only use RAW API, I've assumed it is only compatible with NO_SYS=1 platforms.
At least, it should be a define.
|
David GIRAULT <dgirault> |
Wed 30 Jan 2019 06:35:06 PM UTC, comment #6:
|
Simon Goldschmidt <goldsimon>![]() ![]() |
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?
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>![]() ![]() |
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:
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>![]() ![]() |
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:
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>![]() ![]() |
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
|
David GIRAULT <dgirault> |
Depends on the following items: None found
Items that depend on this one: None found
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 | Status | None | ![]() |
In Progress |
Assigned to | None | ![]() |
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 |
comment #10:
Yes. I just haven't found the time for the extra work, yet :-(