Hi Nelio,
On 12/12/2017 08:08 PM, Nelio Laranjeiro wrote:
Hi Anoob,
On Tue, Dec 12, 2017 at 07:34:31PM +0530, Anoob Joseph wrote:
Hi Nelio,
On 12/12/2017 07:14 PM, Nelio Laranjeiro wrote:
Hi Anoob,
On Tue, Dec 12, 2017 at 06:13:08PM +0530, Anoob Joseph wrote:
Hi Nelio,
On 12/11/2017 07:34 PM, Nelio Laranjeiro wrote:
Mellanox INNOVA NIC needs to have final target queue actions to perform
inline crypto.
Signed-off-by: Nelio Laranjeiro <nelio.laranje...@6wind.com>
---
Changes in v3:
* removed PASSTHRU test for ingress.
* removed check on configured queues for the queue action.
Changes in v2:
* Test the rule by PASSTHRU/RSS/QUEUE and apply the first one validated.
---
examples/ipsec-secgw/ipsec.c | 57
++++++++++++++++++++++++++++++++++++++++++--
examples/ipsec-secgw/ipsec.h | 2 +-
2 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/examples/ipsec-secgw/ipsec.c b/examples/ipsec-secgw/ipsec.c
index 17bd7620d..1b8b251c8 100644
--- a/examples/ipsec-secgw/ipsec.c
+++ b/examples/ipsec-secgw/ipsec.c
@@ -142,6 +142,7 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct ipsec_sa
*sa)
rte_eth_dev_get_sec_ctx(
sa->portid);
const struct rte_security_capability *sec_cap;
+ int ret = 0;
sa->sec_session = rte_security_session_create(ctx,
&sess_conf, ipsec_ctx->session_pool);
@@ -201,15 +202,67 @@ create_session(struct ipsec_ctx *ipsec_ctx, struct
ipsec_sa *sa)
sa->action[0].type = RTE_FLOW_ACTION_TYPE_SECURITY;
sa->action[0].conf = sa->sec_session;
- sa->action[1].type = RTE_FLOW_ACTION_TYPE_END;
-
sa->attr.egress = (sa->direction ==
RTE_SECURITY_IPSEC_SA_DIR_EGRESS);
sa->attr.ingress = (sa->direction ==
RTE_SECURITY_IPSEC_SA_DIR_INGRESS);
+ if (sa->attr.ingress) {
+ uint8_t rss_key[40];
+ struct rte_eth_rss_conf rss_conf = {
+ .rss_key = rss_key,
+ .rss_key_len = 40,
+ };
+ struct rte_eth_dev *eth_dev;
+ union {
+ struct rte_flow_action_rss rss;
+ struct {
+ const struct rte_eth_rss_conf *rss_conf;
+ uint16_t num;
+ uint16_t queue[RTE_MAX_QUEUES_PER_PORT];
+ } local;
+ } action_rss;
+ unsigned int i;
+ unsigned int j;
+
+ sa->action[2].type = RTE_FLOW_ACTION_TYPE_END;
+ /* Try RSS. */
+ sa->action[1].type = RTE_FLOW_ACTION_TYPE_RSS;
+ sa->action[1].conf = &action_rss;
+ eth_dev = ctx->device;
+ rte_eth_dev_rss_hash_conf_get(sa->portid,
+ &rss_conf);
+ for (i = 0, j = 0;
+ i < eth_dev->data->nb_rx_queues; ++i)
+ if (eth_dev->data->rx_queues[i])
+ action_rss.local.queue[j++] = i;
+ action_rss.local.num = j;
+ action_rss.local.rss_conf = &rss_conf;
+ ret = rte_flow_validate(sa->portid, &sa->attr,
+ sa->pattern, sa->action,
+ &err);
+ if (!ret)
+ goto flow_create;
+ /* Try Queue. */
+ sa->action[1].type = RTE_FLOW_ACTION_TYPE_QUEUE;
+ sa->action[1].conf =
+ &(struct rte_flow_action_queue){
+ .index = 0,
+ };
+ ret = rte_flow_validate(sa->portid, &sa->attr,
+ sa->pattern, sa->action,
+ &err);
+ if (ret)
+ goto flow_create_failure;
+ } else {
+ sa->action[1].type =
+ RTE_FLOW_ACTION_TYPE_PASSTHRU;
+ sa->action[2].type = RTE_FLOW_ACTION_TYPE_END;
We would need flow validate here also. And, for egress, the application will
be able to set metadata (set_pkt_metadata API) per packet. So flow may not
be required for such cases. But if the flow create fails, the session create
would also fail. It might be better if we check whether the PMD would need
metadata (part of the sec_cap->ol_flags).
Seems what you are describing is outside of this scope which is only
related to correctly implement the generic flow API with terminal
actions.
Since SECURITY+PASSTHRU won't be terminal, this code segment might be
misleading.
Well, I don't mind adding an extra verification even if the create
should fail if the validate fails, as there is no other option it
is just like adding another if statement considering the validate()
cannot guarantee the flow will be created(), other errors like ENOMEM
are still possible in the creation stage.
Good point. I was thinking of a scenario when flow for egress itself
would be optional.
I'll suggest to add it in another patch.
Anyway, the flow validate is useful in the ingress to select the best
behavior RSS/Queue, if the flow validate may fail, the flow create
should also fail for the same reasons.
If the driver doesn't need metadata and the flow create fails, then
the create session should fail. Any thoughts?
How the create_session() can fail without having all the informations
(pattern, metadata, ...) the application wants to offload?
Is flow mandatory for the egress traffic? My understanding is, it's not.
"set_pkt_metadata" API gives application the ability to do the lookup and
pass the info along with the packet. In such cases, flow creation is not
necessary.
Some NIC need to apply a flow rule for Egress and they don't need
metadata for the packet.
Understood. In that case, what I proposed could be a separate patch. The
ingress path is proper with this patch, but we can keep egress open for
improvements.
I do agree that this is outside the scope of this patch, but I was just
curious about the behavior since you touched the topic.
+ }
+flow_create:
sa->flow = rte_flow_create(sa->portid,
&sa->attr, sa->pattern, sa->action, &err);
if (sa->flow == NULL) {
+flow_create_failure:
RTE_LOG(ERR, IPSEC,
"Failed to create ipsec flow msg: %s\n",
err.message);
diff --git a/examples/ipsec-secgw/ipsec.h b/examples/ipsec-secgw/ipsec.h
index 775b316ff..3c367d392 100644
--- a/examples/ipsec-secgw/ipsec.h
+++ b/examples/ipsec-secgw/ipsec.h
@@ -133,7 +133,7 @@ struct ipsec_sa {
uint32_t ol_flags;
#define MAX_RTE_FLOW_PATTERN (4)
-#define MAX_RTE_FLOW_ACTIONS (2)
+#define MAX_RTE_FLOW_ACTIONS (3)
struct rte_flow_item pattern[MAX_RTE_FLOW_PATTERN];
struct rte_flow_action action[MAX_RTE_FLOW_ACTIONS];
struct rte_flow_attr attr;
Thanks,
Regards,
