<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> From: Mattias Rönnblom <hofors@lysator.liu.se></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> Sent: Wednesday, October 2, 2024 8:43 PM</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> To: Marchand, David <david.marchand@redhat.com></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> Cc: Mattias Rönnblom <mattias.ronnblom@ericsson.com>; dev@dpdk.org <dev@dpdk.org>; Van Haaren, Harry <harry.van.haaren@intel.com>; Stefan Sundkvist <stefan.sundkvist@ericsson.com></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> Subject: Re: [PATCH v2] service: extend service function call statistics</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> </div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> On 2024-10-02 21:08, David Marchand wrote:</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> > On Wed, Oct 2, 2024 at 8:57 PM Mattias Rönnblom <hofors@lysator.liu.se> wrote:</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >>> Coverity flagged this patch with issue #445158.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >>> rte_delay_ms() is now unreachable.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >>></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >>> I suppose this delay is not that important for the unit test and we</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >>> can remove it, but as I am not sure I'll let you have a look and send</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >>> a fix.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >>></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >> It works without it I think, but I would keep it, and add it to the</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >> "case 0" branch.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> >> Let me know if you want a v2.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> ></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> > Unfortunately, coverity is run only for merged stuff.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> > So the report we got is for merged patch in the main repo.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> ></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> > Please send a fix.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> ></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> Done.</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
></div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> I decided I could just as well reintroduce the old behavior. You could</div>
<div style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> refactor away all this sleeping business I think, but that is for</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
> another day.</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Thanks for sending the patch Mattias - Coverity flagged me too, was intending on</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
having a look with colleague Sean here today - instead we'll review your patch shortly.</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
The delays were introduced so thread-spawns in the CI have some time to take affect,</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
and to (intentionally) introduce some jitter so we cover all cases of one-thread-before-the-other</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
checking poll counts and correct behaviour; this is useful as service-cores is inherently racy in</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
terms of things running in parallel, hence the service_dummy() function has the delay.</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
All this to say, lets leave the rte_delay_ms() in.</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof" style="font-family: Calibri, sans-serif; font-size: 11pt; color: rgb(0, 0, 0);">
Expect a response (hopefully Tested/Acked-by) soon! Regards -Harry</div>
</body>
</html>