On 01/08/2018 05:31 PM, Stokes, Ian wrote: >> >> Hi Ian, >> >>> -----Original Message----- >>> From: Stokes, Ian [mailto:[email protected]] >>> Sent: Monday, 08 January, 2018 17:09 >>> To: Jan Scheurich <[email protected]>; Kevin Traynor >>> <[email protected]>; Ilya Maximets <[email protected]> >>> Cc: [email protected] >>> Subject: RE: [ovs-dev] [RFC PATCH 0/8] dpif-netdev: Refactor cycle >>> count and rebased patches >>> >>>> Hi Ian, >>>> >>>> That's why I brought up my original question before Christmas, but >>>> apparently too late ☹. >>> >>> Apologies, Christmas was a bit hectic with the last push for output >> batching and finishing for the holidays so I missed following up on it. >> >> No worries, I understand. Actually Ilya responded and agreed to my >> proposal for deciding on an order. But we didn't have time to discuss the >> best order. He had v9 out before Christmas based on dpdk_merge (later >> merged to master) and I took that for rebase. >> >>>> >>>> Myself, was hoping that we could get all three changes: PMD >>>> performance metrics, time-based tx batching and Kevin's enhancement >>>> for the pmd-rxq- show command into 2.9. >>>> >>> >>> I think this is the ideal situation, but does require sign off from >>> Ilya and Kevin as it will be their features it will impact on, I guess >> they can answer and give an idea on bandwidth to cooperate on something >> like this. >>> >>>> PMD performance metrics are for sure around long enough to warrant >> that. >>>> And they are highly wanted too. >>> >>> To my mind the detailed PMD logs patchset has a lot of content to work >>> through and there are reviews in progress as well as re-work requests >>> from previous reviews of the v4. I was waiting to see these completed >> before focusing on it myself as I haven't had the bandwidth to date to >> take it on. >> >> All pending review comments have been incorporated in v5 posted on Jan 4th >> (http://patchwork.ozlabs.org/cover/855572/). That version is rebased to >> master after merging the non-tima-based output batching. > > Apologies, Jan, I've spotted the v5 now (it was v4 assigned to myself in > patchwork so that was what I had been planning to look at). > >> >> I am just now preparing a v6 that adds the missing documentation for the >> new commands to the ovs-vswitchd man page. > > Excellent, is there an ETA on the v6? I think Billy is already reviewing the > v5 but I'll confirm, the switch to v6 should be painless if it's mainly > documentation. > >> >>> >>>> >>>> Since all three are heavily affecting same parts of the code the >>>> order of merging matters a lot. If we want to make this happen in >>>> reasonable time we need to avoid constant manual re-basing for all of >> us. >>>> >>>> That’s why I have taken the initiative to serialize them into one >>>> particular order for merging. That was a painful exercise and I am >>>> not looking forward to doing it again in a different order. >>> >>> Agreed and thanks for taking the initiative. I do think this is >> important if all three are to make it in.
+1. thanks for this Jan. >>> >>>> >>>> So I would greatly appreciate if we could agree on the proposed >>>> order and discuss how we review/test the resulting overall >>>> contribution with the ambition to get all parts into 2.9. >>> >>> I'm open to others input here, based on the points you've raised I >>> would suggest the following (only a suggestion, please feel free to >>> counter): >>> >>> 1: Output Time batching (it's v9 and is well understood at this point, >>> I would think would be little re-work needed if the cases suit the PMD >> balancing already in place). >>> 2: Detailed PMD logs (from what I understand there is a review in >>> press from Billy, and re-work requests from Aaron, we could roll these >> changes into the next rebase on top of the time output batching). >> >> As stated above, v5 is already out since Thursday and used as my #1. > > Ok. > >> >>> 3: Kevin's PMD patches: probably the smallest of the set and lowest risk >> IMO as you seem familiar already with these Jan? >> >> The patch was IMHU unnecessarily big and is now, after my simplifications, >> rather small. > > Ok, that lowers the risk a little bit more, sounds good to me but will wait > for an ACK from Kevin on this before signing off myself after > reviewing/testing. > I like the new scheme of not collecting the idle cycles per rxq, especially with the changes that will be there now for tx-batching/pmd metrics. There was some data structure simplification in the original patches which got dropped, so I'd like to take a look at that area and see if there's something that I can add. Possibly a couple of minor issues/changes too, but I need to review more/test. >> >>> >>> Does this seem acceptable? >> >> Actually, as I have already prepared the patches in the other order >> 1. Detailed PMD metrics (Jan) >> 2. Time-based output batching (Ilya) >> 3. Refactor cycle counting (Jan) >> 4. PMD load in pmd-rxq-show (Kevin/Jan) I would prefer not to swap 1 and >> 2. I am afraid it will imply a few extra hours to sort out the conflicts >> again. Time none of us has and which would be better spent on >> reviewing/testing. > > Ok let's avoid the overhead re-work and go with the approach you've suggested > above. > Yes, but it may actually be possible to take 4 out of the dependency chain, I'll check that - leave it there for now. >> >> The end result should anyway be the same. This is what we need to focus >> on. We are just talking about different intermediate steps on the way that >> will not live for more than a few hours (or days). > > Sure. > >> >>> It's probably a good topic for the community meeting Wednesday but I'd >>> like to try and get agreement before then if we are to coordinate to get >> these upstreamed. >> >> I hope we can agree on a common approach earlier than the meeting. If mail >> I too slow, I can call for a short dedicated Skype call to discuss and >> agree. > > Ok, I think you're approach looks good to me, I'll start reviewing the time > based output batching based on your RFC tomorrow, I'll coordinate with Billy > as regards the review and testing of the v5 detailed logs with the > understanding these are pre requisite. I see Kevin is already testing and > reviewing the PMD changes. > > Unless there are any objections (I don’t think there are) then let's go with > this. > +1 thanks, Kevin. > Thanks again for bringing this up. > > Ian >> >>> >>> Ian >>>> >>>> Thanks, Jan >>>> >>>>> -----Original Message----- >>>>> From: [email protected] >>>>> [mailto:[email protected]] On Behalf Of Stokes, Ian >>>>> Sent: Monday, 08 January, 2018 16:04 >>>>> To: Jan Scheurich <[email protected]>; Kevin Traynor >>>>> <[email protected]>; Ilya Maximets <[email protected]> >>>>> Cc: [email protected] >>>>> Subject: Re: [ovs-dev] [RFC PATCH 0/8] dpif-netdev: Refactor cycle >>>>> count and rebased patches >>>>> >>>>>> Hi guys, >>>>>> >>>>>> It would be great to get your feedback on the proposal for >>>>>> combining (and >>>>>> simplifying) our patches. >>>>>> >>>>>> Do you agree with a) the cycle counting refactoring and b) the >>>>>> simplification of rxq pmd load calculation? >>>>>> >>>>>> For actual merge I would suggest to re-order the changes and >>>>>> take the cycle counting refactoring before the time-based output >>>>>> batching. But that is perhaps just a matter of taste affecting >>>>>> some intermediate commits and not so important for the end- >> result... >>>>>> >>>>>> How should we proceed with including these to the dpdk_merge >> branch? >>>>>> - One patch series at a time in the order now laid out in my RFC >>>>>> series >>>>>> - One large series comprising all 4 contributions? >>>>>> >>>>> >>>>> Hi Jan, >>>>> >>>>> From my side I was hoping to review/test Ilya's v9 time based >>>>> output batching first with a view to upstreaming that feature for >>>>> the 2.9 >>>> release. >>>>> >>>>> My worry was that by attaching it to other feature changes it may >>>>> not >>>> get up streamed. >>>>> >>>>> I've only had a cursory look at the Kevin's percentage pad patches. >>>>> >>>>> I was thinking of working to the following on dpdk_merge >>>>> >>>>> 1: Review/Upstream Ilya's time based output. >>>>> https://patchwork.ozlabs.org/project/openvswitch/list/?series=1986 >>>>> 5 >>>>> 2: Review/Upstream Kevin's >>>>> https://patchwork.ozlabs.org/user/todo/openvswitch/?series=18301 >>>>> >>>>> Just a question, in your mail below I see 'Detailed PMD >>>>> performance metrics and supervision' is included in [1], Billy >>>>> O'Mahony is currently reviewing the latest revision of this from >>>>> what I'm aware of, >>>> this a pre-requisite for you before upstreaming Ilya and Kevin's >> patches? >>>>> >>>>> I'd like to hear from Ilya on Kevin on this also? If they are >>>>> happy to combine the work and review/validate as a group then it may >> be possible. >>>>> >>>>> Thanks >>>>> Ian >>>>> >>>>>> Regards, Jan >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: [email protected] [mailto:[email protected]] On >>>>>>> Behalf Of Jan Scheurich >>>>>>> Sent: Thursday, 04 January, 2018 21:03 >>>>>>> To: [email protected] >>>>>>> Cc: Jan Scheurich <[email protected]> >>>>>>> Subject: [RFC PATCH 0/8] dpif-netdev: Refactor cycle count and >>>>>>> rebased patches >>>>>>> >>>>>>> This RFC patch series contains three contributions: >>>>>>> >>>>>>> 1. A rebase of Ilya's series "[PATCH v9,0/5] Output packet >>>>>>> batching >>>>>> (Time-based)." >>>>>>> (http://patchwork.ozlabs.org/cover/852003/) on top of "[PATCH >>>>>>> v5,0/3] >>>>>> dpif-netdev: >>>>>>> Detailed PMD performance metrics and supervision" >>>>>> (http://patchwork.ozlabs.org/cover/855572/). >>>>>>> >>>>>>> 2. Refactoring and simplification of the PMD cycle counting id >>>>>>> dpif- >>>>>> netdev.c. >>>>>>> >>>>>>> 3. A rebase and simplification of Kevin's patches >>>>>>> (http://patchwork.ozlabs.org/patch/847972/) >>>>>>> to display PMD usage per queue in the ovs-appctl pmd-rxq-show >>>> command. >>>>>>> >>>>>>> The patches pass check_patch and "make check" and I have done >>>>>>> some basic tests of the simplified rxq cycle counting and the >>>>>>> PMD usage >>>>>> reporting in pmd-rxq-show. >>>>>>> >>>>>>> This is my proposal how to combine the three existing patch >>>>>>> series together with the simplified cycle counting the into >>>>>>> branch dpdk_merge >>>>>> for release in OSV 2.9. >>>>>>> >>>>>>> Before merging the last two patches should probably be combined. >>>>>>> >>>>>>> >>>>>>> Ilya Maximets (5): >>>>>>> dpif-netdev: Use microsecond granularity. >>>>>>> dpif-netdev: Count cycles on per-rxq basis. >>>>>>> dpif-netdev: Time based output batching. >>>>>>> docs: Describe output packet batching in DPDK guide. >>>>>>> NEWS: Mark output packet batching support. >>>>>>> >>>>>>> Jan Scheurich (2): >>>>>>> dpif-netdev: Refactor cycle counting >>>>>>> dpif-netdev: Add percentage of pmd/core used by each rxq. >>>>>>> >>>>>>> Kevin Traynor (1): >>>>>>> dpif-netdev: Reset the rxq current cycle counter on reload. >>>>>>> >>>>>>> Documentation/howto/dpdk.rst | 12 ++ >>>>>>> Documentation/intro/install/dpdk.rst | 58 ++++++ >>>>>>> NEWS | 3 + >>>>>>> lib/dpif-netdev.c | 350 >> ++++++++++++++++++++++-- >>>> ---- >>>>>> ------- >>>>>>> tests/pmd.at | 51 +++-- >>>>>>> vswitchd/vswitch.xml | 16 ++ >>>>>>> 6 files changed, 349 insertions(+), 141 deletions(-) >>>>>>> >>>>>>> -- >>>>>>> 1.9.1 >>>>> >>>>> _______________________________________________ >>>>> dev mailing list >>>>> [email protected] >>>>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
