Hi Stephen, Please see inline.
Thanks, Anoob > -----Original Message----- > From: Stephen Hemminger <[email protected]> > Sent: Thursday, May 25, 2023 12:02 AM > To: Anoob Joseph <[email protected]> > Cc: Thomas Monjalon <[email protected]>; Akhil Goyal > <[email protected]>; Jerin Jacob Kollanukkaran <[email protected]>; > Konstantin Ananyev <[email protected]>; Bernard > Iremonger <[email protected]>; Volodymyr Fialko > <[email protected]>; Hemant Agrawal <[email protected]>; > Mattias Rönnblom <[email protected]>; Kiran Kumar > Kokkilagadda <[email protected]>; [email protected]; Olivier Matz > <[email protected]> > Subject: [EXT] Re: [PATCH v3 21/22] pdcp: add thread safe processing > > External Email > > ---------------------------------------------------------------------- > On Wed, 24 May 2023 21:31:15 +0530 > Anoob Joseph <[email protected]> wrote: > > > From: Volodymyr Fialko <[email protected]> > > > > PDCP state has to be guarded for: > > > > - Uplink pre_process: > > - tx_next atomic increment > > > > - Downlink pre_process: > > - rx_deliv - read > > > > - Downlink post_process: > > - rx_deliv, rx_reorder, rx_next - read/write > > - bitmask/reorder buffer - read/write > > > > When application requires thread safe processing, the state variables > > need to be updated atomically. Add config option to select this option > > per entity. > > > > Signed-off-by: Anoob Joseph <[email protected]> > > Signed-off-by: Volodymyr Fialko <[email protected]> > > NAK > Conditional locking is a bad design pattern. > It leads to lots of problems, and makes it almost impossible for analysis > tools. [Anoob] With PDCP (& most other protocols), we have to update the states atomically. Application designers would have a choice of either use single thread or do multi-thread processing. If the library is designed for multi-thread and if application uses only single thread, then there would be unnecessary overheads from library. If library sticks to single-thread and if application needs more threads for scaling, then again it would become a library issue. Is your issue with providing such an option or is it about how it is implemented? IPsec also has a similar challenge and similar per SA configuration is provided in lib IPsec as well.
