Hi Akhil, > > > >>>> Subject: Re: [dpdk-dev] [PATCH v2 1/2] examples/ipsec-secgw: fix 1st > >>>> pkt dropped for inline crypto > >>>> > >>>> Hi Bernard, > >>>> > >>>> On 3/7/2019 8:27 PM, Bernard Iremonger wrote: > >>>>> Inline crypto installs a flow rule in the NIC. This flow rule must > >>>>> be installed before the first inbound packet is received. > >>>>> > >>>>> The create_session() function installs the flow rule, > >>>>> create_session() has been refactored into create_inline_session() > >>>>> and create_lookaside_session(). The create_inline_session() function > >>>>> uses the socket_ctx data and is now called at initialisation in > >>>>> sa_add_rules(). > >>>> why do we need a separate function for session creation for inline > >>>> and lookaside cases? > >>>> Why can't we initialize the sessions on sa_init in both the cases? > >>> For the create_inline_session(struct socket_ctx *skt_ctx, struct > >>> ipsec_sa *sa) function, all of the required data is available in the in > >>> the > >> skt_ctx variable. > >>> The skt_ctx variable is already setup when sa_init() is called. > >>> > >>> For the create_lookaside_session(struct ipsec_ctx *ipsec_ctx, struct > >>> ipsec_sa *sa) function, the required data is available in the ipsec_ctx > >> variable. > >>> The ipsec_ctx variable is not setup when sa_init() is called. > >>> It is setup in the main_loop() function when the variable qconf is setup. > >>> The main_loop() function is called after the sa_init() function is called. > >>> > >>> I hope this answers your question > >> Whatever information that is required for session creation is available > >> before > >> we call the main loop() in both the cases. > >> My point is both the sessions(inline/lookaside) can be init at the same > >> position, we do not need to have a separate path for them. > >> If it is not possible in sa_init(), it may be somewhere else before the > >> actual > >> data path is started. > >> > >> The problem with inline processing is that, h/w need to know the SA before > >> the first packet is received. So we cannot init the session on receive of > >> first > >> packet. However there is no such limitation in case of lookaside, it can be > >> initialized anywhere. > >> > >> -Akhil > > This patch is intended to fix the bug in the inline processing, which is > > that the flow rule must be installed, before the first packet is > received while leaving the lookaside processing as it was originally. > > > > It is not intended to refactor the lookaside processing. > The fix provided should not make the code complex and difficult to > understand when we have the option available to fix that in a simpler > way. Application is already complex enough(4 action types) and by adding > separate code for session init at two different places will be difficult > to maintain in future and will reduce the code readability.
It can be changed to do initialization for both security and crypto sessions at sa_init() time, but it would cause much more changes in the code: the idea is for each SA go through all available crypto-devices, check their capabilities, and if they match, we'll invoke init_session() for that SA and device. That way at the end of sa_init() we'll have crypto-sessions that can be used on any attached crypto-dev. As a drawback - each session might occupy more memory. Though as I said, it would require more changes (and testing) in init code. So my thought to have this patch as it is and then add these changes as a separate patch series (phase 2). What do you think? Konstantin
