On 27/10/17 13:11, Ilya Maximets wrote:
...
...
I feel that this patch makes the code more confusing. It's not clear when
I should just use pmd->ctx.now, or call pmd_thread_ctx_time_update() first.
Maybe just say we update it in pmd_thread_main() before every call to
dp_netdev_process_rxq_port(). If you need more accurate time, get it directly,
and obsolete the pmd_thread_ctx_time_update() call?
I understand your confusion, but make a syscall for each empty (no rx packets)
port may cause serious performance drop in case we're receiving intensive 
traffic
only on one port. So, I'm still insist on calling update inside 
*process_rxq_port()
only if we have some packets to process.

One more thing is that we also have main and revalidator threads which are able
to send packets, and they need at least nearly accurate time.

We have, actually, 2 functions which needs accurate time:
dp_netdev_input() and dp_netdev_pmd_flush_output_packets().

So, what if we'll keep only 5 calls to update functions when all patches 
applied?
Like this:

1. dp_netdev_configure_pmd() - initialization
2. dp_netdev_process_rxq_port() - time update before packet processing start
3. pmd_thread_main() - if (need_to_flush) - there was no time updates in this
                        processing cycle. Fix that before calling flush 
function.
4. dpif_netdev_run() - same, but for non-pmd thread.
5. dpif_netdev_execute() - because it's the one more packet source beside the
                            direct receiving from port.

Above schema will ensure that we have at least one time update per polling 
cycle.
And we will not need to have any other calls to pmd_thread_ctx_time_update().
It'll be safe for all other functions to just use pmd->ctx.now without thinking
when it was updated last time.
Hi Ilya,

I'll like this approach, it's more clean, and I'll ack it as part of your v5 submission.

//Eelco
Note:
As you can see above, update call will be removed from 
dp_netdev_pmd_try_optimize()
in this case. But it still will be in the first patch to be sure that time was
updated at least once before optimization. But I think, we can move it out of
the function to pmd_thread_main() just before calling 
dp_netdev_pmd_try_optimize().
The same about time update before XPS revalidation in dpif_netdev_run().

I'm going to implement above schema because, I think, it's really better than
currently implemented one. IMHO, it's simpler to understand and maintain.
Any thoughts?

Best regards, Ilya Maximets.

...
...
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to