From: Finn Christensen <[email protected]> AFAIK, most (if not all) NICs (including Mellanox and Intel) do not support a pure MARK action. It's required to be used together with some other actions, like QUEUE.
To workaround it, retry with a queue action when first try failed. Moreover, some Intel's NIC (say XL710) needs the QUEUE action set before the MARK action. Co-authored-by: Yuanhan Liu <[email protected]> Signed-off-by: Finn Christensen <[email protected]> Signed-off-by: Yuanhan Liu <[email protected]> --- v3: - retry when 2 specific errors are met v2: - workarounded the queue action issue, which sets the queue index to 0 blindly. - added some comments regarding to the retry with queue action --- lib/netdev-dpdk.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 525536a..3e2b96b 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -3324,6 +3324,7 @@ static struct ovs_mutex ufid_lock = OVS_MUTEX_INITIALIZER; struct ufid_dpdk_flow_data { struct hmap_node node; ovs_u128 ufid; + int rxq; struct rte_flow *rte_flow; }; @@ -3369,7 +3370,8 @@ del_ufid_dpdk_flow_mapping(const ovs_u128 *ufid) /* Add ufid to dpdk_flow mapping */ static inline void -add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow) +add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow, + int rxq) { size_t hash = hash_bytes(ufid, sizeof(*ufid), 0); struct ufid_dpdk_flow_data *data = xzalloc(sizeof(*data)); @@ -3384,6 +3386,7 @@ add_ufid_dpdk_flow_mapping(const ovs_u128 *ufid, struct rte_flow *rte_flow) data->ufid = *ufid; data->rte_flow = rte_flow; + data->rxq = rxq; ovs_mutex_lock(&ufid_lock); hmap_insert(&ufid_dpdk_flow, &data->node, hash); @@ -3665,15 +3668,55 @@ netdev_dpdk_add_rte_flow_offload(struct netdev *netdev, add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); + bool tried = 0; + struct rte_flow_action_queue queue; +again: flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items, actions.actions, &error); - if (!flow) { + if (!flow && !tried && + (error.type == RTE_FLOW_ERROR_TYPE_ACTION || + error.type == RTE_FLOW_ERROR_TYPE_HANDLE)) { + /* + * Seems most (if not all) NICs do not support a pure MARK + * action. It's required to be used together with some other + * actions, such as QUEUE. + * + * Thus, if flow creation fails (more precisely, if above 2 + * errors are met), here we try again with QUEUE action. + * + * The reason why there are 2 error codes is DPDK drivers are + * not quite consistent on error report for this case. + */ + if (info->rxq < 0 || info->rxq >= netdev->n_rxq) { + VLOG_ERR("invalid queue index: %d\n", info->rxq); + ret = -1; + goto out; + } + queue.index = info->rxq; + + /* re-build the action */ + actions.cnt = 0; + /* + * NOTE: Intel PMD driver needs the QUEUE action set before + * the MARK action. + */ + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_QUEUE, &queue); + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_MARK, &mark); + add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); + + VLOG_INFO("failed to create rte flow, " + "try again with QUEUE action (queue index = %d)\n", + info->rxq); + tried = true; + + goto again; + } else if (!flow) { VLOG_ERR("rte flow creat error: %u : message : %s\n", error.type, error.message); ret = -1; goto out; } - add_ufid_dpdk_flow_mapping(ufid, flow); + add_ufid_dpdk_flow_mapping(ufid, flow, info->rxq); VLOG_INFO("installed flow %p by ufid "UUID_FMT"\n", flow, UUID_ARGS((struct uuid *)ufid)); @@ -3807,17 +3850,23 @@ netdev_dpdk_flow_put(struct netdev *netdev, struct match *match, const ovs_u128 *ufid, struct offload_info *info, struct dpif_flow_stats *stats OVS_UNUSED) { - struct rte_flow *rte_flow; + struct ufid_dpdk_flow_data *flow_data; int ret; /* * If an old rte_flow exists, it means it's a flow modification. * Here destroy the old rte flow first before adding a new one. */ - rte_flow = get_rte_flow_by_ufid(ufid); - if (rte_flow) { + flow_data = find_ufid_dpdk_flow_mapping(ufid); + if (flow_data && flow_data->rte_flow) { + /* + * there is no rxq given for flow modification. Instead, we + * retrieve it from the rxq firstly registered here (by flow + * add operation). + */ + info->rxq = flow_data->rxq; ret = netdev_dpdk_destroy_rte_flow(netdev_dpdk_cast(netdev), - ufid, rte_flow); + ufid, flow_data->rte_flow); if (ret < 0) { return ret; } -- 2.7.4 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
