On 10/19/2017 1:29 PM, Ananyev, Konstantin wrote:
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Ananyev, Konstantin
Sent: Thursday, October 19, 2017 1:17 PM
To: Nicolau, Radu <[email protected]>; Akhil Goyal <[email protected]>;
[email protected]
Cc: Doherty, Declan <[email protected]>; De Lara Guarch, Pablo
<[email protected]>; [email protected];
[email protected]; [email protected]; [email protected];
[email protected]; [email protected];
Mcnamara, John <[email protected]>; [email protected];
[email protected]
Subject: Re: [dpdk-dev] [PATCH v4 10/12] net/ixgbe: enable inline ipsec
-----Original Message-----
From: Nicolau, Radu
Sent: Thursday, October 19, 2017 12:57 PM
To: Ananyev, Konstantin <[email protected]>; Akhil Goyal
<[email protected]>; [email protected]
Cc: Doherty, Declan <[email protected]>; De Lara Guarch, Pablo
<[email protected]>; [email protected];
[email protected]; [email protected]; [email protected];
[email protected]; [email protected];
Mcnamara, John <[email protected]>; [email protected];
[email protected]
Subject: RE: [PATCH v4 10/12] net/ixgbe: enable inline ipsec
-----Original Message-----
From: Ananyev, Konstantin
Sent: Thursday, October 19, 2017 12:04 PM
To: Nicolau, Radu <[email protected]>; Akhil Goyal
<[email protected]>; [email protected]
Cc: Doherty, Declan <[email protected]>; De Lara Guarch, Pablo
<[email protected]>; [email protected];
[email protected]; [email protected]; [email protected];
[email protected]; [email protected]; Mcnamara,
John <[email protected]>; [email protected];
[email protected]
Subject: RE: [PATCH v4 10/12] net/ixgbe: enable inline ipsec
<snip>
+
+static int
+ixgbe_crypto_update_mb(void *device __rte_unused,
+ struct rte_security_session *session,
+ struct rte_mbuf *m, void *params __rte_unused) {
Another sort of generic question - why not make security_set_pkt_metadata
function
to accept bulk of packets?
In that case o can minimize the cost of function calls, accessing session data,
etc.
Though I suppose that could wait till next patch series.
Konstantin
It is a good suggestion, but we need to discuss it further; for example
if it can accept a bulk of packets, will it need also a bulk of metadata
pointers, or just one for all the packets?
+ struct ixgbe_crypto_session *ic_session =
+ get_sec_session_private_data(session);
+ if (ic_session->op == IXGBE_OP_AUTHENTICATED_ENCRYPTION) {
+ struct ixgbe_crypto_tx_desc_md *mdata =
+ (struct ixgbe_crypto_tx_desc_md *)&m->udata64;
+ mdata->enc = 1;
+ mdata->sa_idx = ic_session->sa_index;
+ mdata->pad_len = *rte_pktmbuf_mtod_offset(m,
+ uint8_t *, rte_pktmbuf_pkt_len(m) - 18) + 18;
Could you explain what pad_len supposed to contain?
Also what is a magical constant '18'?
Could you create some macro if needed?
I added an explanation in the code, we read the payload padding size
that is stored at the len-18 bytes and add 18 bytes, 2 for ESP trailer
and 16 for ICV.
Ok, can we at least have a macros for all these constants?
Another question: you do use pkt_len() here - does it mean that multi-
segment packets are not supported by ixgbe-ipsec?
Konstantin
It does support multisegment, but the pad_len has to be set only for single
send, it will be ignored otherwise. I have updated the code to
set
it for single segment packets only.
Sorry, I didn't understand that.
If that function does support multiseg packets, then it has to go to the last
segment via m->next,
If it doesn't, then it should return an error I case of m->nb_seg != 1.
Right?
Also, our test app does not support multisegment packets.
Ok, I suppose that means, multi-seg case wasn't tested :)
