Looks good to me. Thanks for submitting this! Acked by: Jay Jayatheerthan <[email protected]>
-Jay > -----Original Message----- > From: Naga Harish K, S V <[email protected]> > Sent: Monday, October 17, 2022 6:07 PM > To: Jerin Jacob <[email protected]>; mattias.ronnblom > <[email protected]> > Cc: Jayatheerthan, Jay <[email protected]>; Carrillo, Erik G > <[email protected]>; Gujjar, Abhinandan S > <[email protected]>; Jerin Jacob <[email protected]>; > [email protected]; Van Haaren, Harry <[email protected]>; > [email protected] > Subject: RE: [PATCH 2/4] eventdev: have ethernet Rx adapter appropriately > report idle > > > > > -----Original Message----- > > From: Jerin Jacob <[email protected]> > > Sent: Friday, October 14, 2022 11:07 PM > > To: mattias.ronnblom <[email protected]> > > Cc: Naga Harish K, S V <[email protected]>; Jayatheerthan, Jay > > <[email protected]>; Carrillo, Erik G <[email protected]>; > > Gujjar, Abhinandan S <[email protected]>; Jerin Jacob > > <[email protected]>; [email protected]; Van Haaren, Harry > > <[email protected]>; [email protected] > > Subject: Re: [PATCH 2/4] eventdev: have ethernet Rx adapter appropriately > > report idle > > > > On Thu, Oct 13, 2022 at 3:23 PM Mattias Rönnblom > > <[email protected]> wrote: > > > > > > On 2022-10-13 03:32, Naga Harish K, S V wrote: > > > > > > > > > > > >> -----Original Message----- > > > >> From: Jayatheerthan, Jay <[email protected]> > > > >> Sent: Tuesday, October 11, 2022 12:40 PM > > > >> To: mattias.ronnblom <[email protected]>; Carrillo, > > > >> Erik G <[email protected]>; Gujjar, Abhinandan S > > > >> <[email protected]>; Jerin Jacob <[email protected]>; > > > >> Naga Harish K, S V <[email protected]> > > > >> Cc: [email protected]; Van Haaren, Harry <[email protected]>; > > > >> [email protected]; mattias.ronnblom > > > >> <[email protected]> > > > >> Subject: RE: [PATCH 2/4] eventdev: have ethernet Rx adapter > > > >> appropriately report idle > > nitpick: > the headline can be modified as "eventdev/eth_rx:" for Rx adapter patches, > to make it consistent with the currently following procedure. > > Similar change for other patches of other adapters. > > After these changes, my "Reviewed-by: " tag can be added for patches 2 and 3 > (Rx adapter and Tx adapter). > > -Harish > > > > >> > > > >> @Harish, Could you review the patch ? > > > >> > > > >> -Jay > > > >> > > > >>> > > > >>> done: > > > >>> - rx_adapter->stats.rx_intr_packets += nb_rx; > > > >>> + if (nb_rx > 0) { > > > > > > > > How are the performance numbers before and after this patch? > > > > Trying to understand the performance impact, as new condition is added > > to the service function Datapath. > > > > > > > I haven't tested the RX and TX adapters separately, but if you run > > > them on the same core, I get the following result: > > > > > > Without patches, with stats disabled: 16,0 Mpps Without patches, with > > > stats enabled: 16,1 Mpps With patches, with stats disabled: 16,1 Mpps > > > With patches, with stats enabled: 16,2 Mpps > > > > > > So these patches, with this particular hardware, compiler, and test > > > application, adding a tiny bit of additional logic actually make the > > > RX+TX adapter perform better. This is contrary to what you might > > > RX+expect, > > > and I'm sure YMMV. > > > > > > Enabling service core statistics (which boils down to a 2x rdtsc and > > > some cheap arithmetic in rte_service.c) actually make the RX+TX > > > adapter core perform better, both before and after this patchset. Also > > > contrary to what you might expect. > > > > > > The results are consistent across multiple runs. > > > > > > GCC 11.2.0 and AMD Zen 3 @ 3,7 GHz. Event device is DSW and I/O is the > > > ring Ethdev. > > > > @Naga Harish K, S V @Jayatheerthan, Jay @Gujjar, Abhinandan S @Erik > > Gabriel Carrillo Planning to take this series for rc2. If there are no other > > comments, I will merge the series then.
