Hi, > -----Original Message----- > From: Anoob Joseph [mailto:anoob.jos...@caviumnetworks.com] > Sent: Tuesday, February 27, 2018 11:32 AM > To: Nicolau, Radu <radu.nico...@intel.com>; Akhil Goyal > <akhil.go...@nxp.com>; Doherty, Declan <declan.dohe...@intel.com> > Cc: Jerin Jacob <jerin.ja...@caviumnetworks.com>; Narayana Prasad > <narayanaprasad.athr...@caviumnetworks.com>; Nelio Laranjeiro > <nelio.laranje...@6wind.com>; dev@dpdk.org > Subject: Re: [PATCH 1/5] lib/ethdev: support for inline IPsec events > > Hi Radu, > > Please see inline. > > Thanks, > > Anoob > > > On 27/02/18 15:49, Nicolau, Radu wrote: > >> -----Original Message----- > >> From: Anoob Joseph [mailto:anoob.jos...@caviumnetworks.com] > >> Sent: Tuesday, February 27, 2018 6:57 AM > >> To: Nicolau, Radu <radu.nico...@intel.com>; Akhil Goyal > >> <akhil.go...@nxp.com>; Doherty, Declan <declan.dohe...@intel.com> > >> Cc: Jerin Jacob <jerin.ja...@caviumnetworks.com>; Narayana Prasad > >> <narayanaprasad.athr...@caviumnetworks.com>; Nelio Laranjeiro > >> <nelio.laranje...@6wind.com>; dev@dpdk.org > >> Subject: Re: [PATCH 1/5] lib/ethdev: support for inline IPsec events > >> > >> Hi Radu, > >> > >> Please see inline. > >> > >> Thanks, > >> Anoob > >> > >> On 26/02/18 15:05, Nicolau, Radu wrote: > >>>> -----Original Message----- > >>>> From: Anoob Joseph [mailto:anoob.jos...@caviumnetworks.com] > >>>> Sent: Wednesday, February 21, 2018 5:37 AM > >>>> To: Akhil Goyal <akhil.go...@nxp.com>; Doherty, Declan > >>>> <declan.dohe...@intel.com>; Nicolau, Radu <radu.nico...@intel.com> > >>>> Cc: Anoob Joseph <anoob.jos...@caviumnetworks.com>; Jerin Jacob > >>>> <jerin.ja...@caviumnetworks.com>; Narayana Prasad > >>>> <narayanaprasad.athr...@caviumnetworks.com>; Nelio Laranjeiro > >>>> <nelio.laranje...@6wind.com>; dev@dpdk.org > >>>> Subject: [PATCH 1/5] lib/ethdev: support for inline IPsec events > >>>> > >>>> Adding support for IPsec events in rte_eth_event framework. In > >>>> inline IPsec offload, the per packet protocol defined variables, > >>>> like ESN, would be managed by PMD. In such cases, PMD would need > >>>> IPsec events to notify application about various conditions like, ESN > overflow. > >>>> > >>>> Signed-off-by: Anoob Joseph <anoob.jos...@caviumnetworks.com> > >>>> --- > >>>> lib/librte_ether/rte_ethdev.h | 22 ++++++++++++++++++++++ > >>>> 1 file changed, 22 insertions(+) > >>>> > >>>> diff --git a/lib/librte_ether/rte_ethdev.h > >>>> b/lib/librte_ether/rte_ethdev.h index 0361533..4e4e18d 100644 > >>>> --- a/lib/librte_ether/rte_ethdev.h > >>>> +++ b/lib/librte_ether/rte_ethdev.h > >>>> @@ -2438,6 +2438,27 @@ int > >>>> rte_eth_tx_done_cleanup(uint16_t port_id, uint16_t queue_id, > >>>> uint32_t free_cnt); > >>>> > >>>> /** > >>>> + * Subtypes for IPsec offload events raised by eth device. > >>>> + */ > >>>> +enum rte_eth_event_ipsec_subtype { > >>>> + RTE_ETH_EVENT_IPSEC_ESN_OVERFLOW, > >>>> + /** Sequence number overflow in security > >>>> offload */ > >>>> + RTE_ETH_EVENT_IPSEC_MAX > >>>> + /** Max value of this enum */ > >>>> +}; > >>> I would add some more events to the list (to make it look less like > >>> a very > >> specific case implementation): crypto/auth failed and > >> undefined/unspecified being the most obvious. > >>> Apart from this, the patchset looks fine. > >> Understood your point. But crypto/auth failed would be per packet, right? > >> How are we handling such error cases presently? Just want to make > >> sure we are not adding two error reporting mechanisms. > > The only reason for my suggestion was to keep the API as flexible and > generic as possible. > I agree to your suggestion. > > For the inline crypto on ixgbe we only flag the mbuf with the security error > flag, but no extra info is added. I guess we can have a ipsec crypto error > event with a list of failed mbufs or similar. In any case, it's just a > suggestion. > Do you think having a crypto error with failed mbufs would be useful? If yes, > I > can add that. While considering other SA specific events, there could be two > other such events that we may need to consider. > 1) Byte expiry of SA [1] > 2) Time expiry of SA [1] > You can add the flags even if we don't provide support for them in the sample app.
> Shall I add these events? Or do we need to make that a separate patch? > Considering that it would need an entry in conf for actually of any use. > > [1] https://tools.ietf.org/html/rfc4301#page-37 > > > >>>> + > >>>> +/** > >>>> + * Descriptor for IPsec event. Used by eth dev to send extra > >>>> +information of the > >>>> + * event. > >>>> + */ > >>>> +struct rte_eth_event_ipsec_desc { > >>>> + enum rte_eth_event_ipsec_subtype stype; > >>>> + /** Type of IPsec event */ > >>>> + uint64_t md; > >>>> + /** Event specific metadata */ > >>>> +}; > >>>> + > >>>> +/** > >>>> * The eth device event type for interrupt, and maybe others in > >>>> the > >> future. > >>>> */ > >>>> enum rte_eth_event_type { > >>>> @@ -2448,6 +2469,7 @@ enum rte_eth_event_type { > >>>> RTE_ETH_EVENT_INTR_RESET, > >>>> /**< reset interrupt event, sent to VF on PF > >>>> reset */ > >>>> RTE_ETH_EVENT_VF_MBOX, /**< message from the VF received by > >> PF */ > >>>> + RTE_ETH_EVENT_IPSEC, /**< IPsec offload related event */ > >>>> RTE_ETH_EVENT_MACSEC, /**< MACsec offload related event */ > >>>> RTE_ETH_EVENT_INTR_RMV, /**< device removal event */ > >>>> RTE_ETH_EVENT_NEW, /**< port is probed */ > >>>> -- > >>>> 2.7.4