> -----Original Message-----
> From: David Marchand <david.march...@redhat.com>
> Sent: Tuesday, November 12, 2024 2:21 PM
> To: Jerin Jacob <jer...@marvell.com>
> Cc: Huichao Cai <chcch...@163.com>; Kiran Kumar Kokkilagadda
> <kirankum...@marvell.com>; Nithin Kumar Dabilpuram
> <ndabilpu...@marvell.com>; yanzhirun_...@163.com; dev@dpdk.org;
> Thomas Monjalon <tho...@monjalon.net>; Robin Jarry <rja...@redhat.com>
> Subject: Re: Re:RE: [EXTERNAL] [PATCH] graph: optimize graph search when
> scheduling nodes
>
> On Mon, Nov 11, 2024 at 6: 39 AM Jerin Jacob <jerinj@ marvell. com> wrote: >
> > > > > -----Original Message----- > > From: David Marchand
> <david. marchand@ redhat. com> > > Sent: Friday, November 8, 2024 7: 08
>
> On Mon, Nov 11, 2024 at 6:39 AM Jerin Jacob <jer...@marvell.com> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: David Marchand <david.march...@redhat.com>
> > > Sent: Friday, November 8, 2024 7:08 PM
> > > To: Jerin Jacob <jer...@marvell.com>
> > > Cc: Huichao Cai <chcch...@163.com>; Kiran Kumar Kokkilagadda
> > > <kirankum...@marvell.com>; Nithin Kumar Dabilpuram
> > > <ndabilpu...@marvell.com>; yanzhirun_...@163.com; dev@dpdk.org;
> > > Thomas Monjalon <tho...@monjalon.net>; Robin Jarry
> > > <rja...@redhat.com>
> > > Subject: Re: Re:RE: [EXTERNAL] [PATCH] graph: optimize graph search
> > > when scheduling nodes
> > >
> > > Hello Jerin, On Fri, Nov 8, 2024 at 1: 22 PM Jerin Jacob <jerinj@
> > > marvell. com>
> > > wrote: > > > Is n't breaking the ABI? > > > > So can't we modify the
> > > ABI, or is there any special operation required to modify > > Hello
> > > Jerin,
> >
> > Hello David,
> >
> > >
> > > On Fri, Nov 8, 2024 at 1:22 PM Jerin Jacob <jer...@marvell.com> wrote:
> > > > > > Is n't breaking the ABI?
> > > > >
> > > > > So can't we modify the ABI, or is there any special operation
> > > > > required to modify the ABI?
> > > >
> > > > Only LTS release (xx.11) can change the ABI after sending deprecation
> notice.
> > > > Looking at the pahole output, one option will be making dispatch
> > > > and new semi fastpath Additions like xstat_off can be min cache
> > > > aligned to make room for future expansion and to make sure have
> > > > better
> > > performance.
> > >
> > > Adding holes may be a short term solution, but in my opinion, the
> > > slow path part should be entirely hidden and we only expose the fp part.
> >
> > The new cache line alignment items are proposed are fastpath items only.
>
> I had only noticed the second comment:
>
> + alignas(RTE_CACHE_LINE_MIN_SIZE)
> rte_graph_off_t xstat_off; /**< Offset to xstat counters. */
> /* Fast path area */
> ^^^^^^^^^^^^
>
> And I assumed the part in the struct before was slow path.
> (it may be worth enhancing these comments, with a single limit of slow/fast
> path areas)
Yes. Xstat_off was new addition as a fastpath item in this release and there
was no space
in original Fastpath area. And, Yes, the comment needs to be updated.
>
>
> >
> > > Reminder, those holes must be in a "known state" as we release
> > > v24.11 so that the presence of future additions can be safely detected.
>
> If the rte_node objects are allocated by the graph library and zero'd, then we
> are good.
> It seems to be the case in graph_nodes_populate(), and the rte_node objects
> are embedded in the rte_graph object.
>
> Is there another location in the graph library where a rte_node object is
> allocated?
No
>
> If not, and an application can not create a rte_node object, your proposal
> looks
> good to me.
OK. @Huichao Cai Please send two patches (a) new proposal and (b) your
improvement as series.
Update ABI Changes section in doc/guides/rel_notes/release_24_11.rst
>
>
> --
> David Marchand
