Skip to content

Commit

Permalink
net/bnxt: allow flow creation when RSS is enabled
Browse files Browse the repository at this point in the history 
Currently flow creation is allowed with queue action only
when RSS is disabled. Remove this restriction. Flows can be
created when RSS is enabled.

Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Rahul Gupta <rahul.gupta@broadcom.com>
Reviewed-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
  • Loading branch information
@ajitkhaparde
ajitkhaparde authored and Ferruh Yigit committed Oct 8, 2019
1 parent 36024b2 commit d24610f
Show file tree
Hide file tree
Showing 4 changed files with 148 additions and 33 deletions.
1 change: 1 addition & 0 deletions drivers/net/bnxt/bnxt.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ struct bnxt {
int bnxt_link_update_op(struct rte_eth_dev *eth_dev, int wait_to_complete);
int bnxt_rcv_msg_from_vf(struct bnxt *bp, uint16_t vf_id, void *msg);
int is_bnxt_in_error(struct bnxt *bp);
uint16_t bnxt_rss_ctxts(const struct bnxt *bp);

int bnxt_map_fw_health_status_regs(struct bnxt *bp);
uint32_t bnxt_read_fw_status_reg(struct bnxt *bp, uint32_t index);
Expand Down
2 changes: 1 addition & 1 deletion drivers/net/bnxt/bnxt_ethdev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ int is_bnxt_in_error(struct bnxt *bp)
* High level utility functions
*/

static uint16_t bnxt_rss_ctxts(const struct bnxt *bp)
uint16_t bnxt_rss_ctxts(const struct bnxt *bp)
{
if (!BNXT_CHIP_THOR(bp))
return 1;
Expand Down
176 changes: 144 additions & 32 deletions drivers/net/bnxt/bnxt_flow.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "bnxt.h"
#include "bnxt_filter.h"
#include "bnxt_hwrm.h"
#include "bnxt_ring.h"
#include "bnxt_rxq.h"
#include "bnxt_vnic.h"
#include "bnxt_util.h"
#include "hsi_struct_def_dpdk.h"
Expand Down Expand Up @@ -151,23 +153,13 @@ bnxt_validate_and_parse_flow_type(struct bnxt *bp,
int use_ntuple;
uint32_t en = 0;
uint32_t en_ethertype;
int dflt_vnic, rc = 0;
int dflt_vnic;

use_ntuple = bnxt_filter_type_check(pattern, error);
PMD_DRV_LOG(DEBUG, "Use NTUPLE %d\n", use_ntuple);
if (use_ntuple < 0)
return use_ntuple;

if (use_ntuple && (bp->eth_dev->data->dev_conf.rxmode.mq_mode &
ETH_MQ_RX_RSS)) {
PMD_DRV_LOG(ERR, "Cannot create ntuple flow on RSS queues\n");
rte_flow_error_set(error, EINVAL,
RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL,
"Cannot create flow on RSS queues");
rc = -rte_errno;
return rc;
}

filter->filter_type = use_ntuple ?
HWRM_CFA_NTUPLE_FILTER : HWRM_CFA_EM_FILTER;
en_ethertype = use_ntuple ?
Expand Down Expand Up @@ -715,17 +707,6 @@ bnxt_flow_parse_attr(const struct rte_flow_attr *attr,
"No support for priority.");
return -rte_errno;
}

/* Not supported */
if (attr->group) {
rte_flow_error_set(error,
EINVAL,
RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
attr,
"No support for group.");
return -rte_errno;
}

return 0;
}

Expand Down Expand Up @@ -764,6 +745,50 @@ bnxt_get_l2_filter(struct bnxt *bp, struct bnxt_filter_info *nf,
return filter1;
}

static int bnxt_vnic_prep(struct bnxt *bp, struct bnxt_vnic_info *vnic)
{
struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
uint64_t rx_offloads = dev_conf->rxmode.offloads;
int rc;

rc = bnxt_vnic_grp_alloc(bp, vnic);
if (rc)
goto ret;

rc = bnxt_hwrm_vnic_alloc(bp, vnic);
if (rc) {
PMD_DRV_LOG(ERR, "HWRM vnic alloc failure rc: %x\n", rc);
goto ret;
}
bp->nr_vnics++;

/* RSS context is required only when there is more than one RSS ring */
if (vnic->rx_queue_cnt > 1) {
rc = bnxt_hwrm_vnic_ctx_alloc(bp, vnic, 0 /* ctx_idx 0 */);
if (rc) {
PMD_DRV_LOG(ERR,
"HWRM vnic ctx alloc failure: %x\n", rc);
goto ret;
}
} else {
PMD_DRV_LOG(DEBUG, "No RSS context required\n");
}

if (rx_offloads & DEV_RX_OFFLOAD_VLAN_STRIP)
vnic->vlan_strip = true;
else
vnic->vlan_strip = false;

rc = bnxt_hwrm_vnic_cfg(bp, vnic);
if (rc)
goto ret;

bnxt_hwrm_vnic_plcmode_cfg(bp, vnic);

ret:
return rc;
}

static int
bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
const struct rte_flow_item pattern[],
Expand All @@ -775,12 +800,14 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
const struct rte_flow_action *act =
bnxt_flow_non_void_action(actions);
struct bnxt *bp = dev->data->dev_private;
struct rte_eth_conf *dev_conf = &bp->eth_dev->data->dev_conf;
const struct rte_flow_action_queue *act_q;
const struct rte_flow_action_vf *act_vf;
struct bnxt_vnic_info *vnic, *vnic0;
struct bnxt_filter_info *filter1;
struct bnxt_rx_queue *rxq = NULL;
int dflt_vnic, vnic_id;
uint32_t vf = 0;
int dflt_vnic;
int rc;

rc =
Expand All @@ -800,7 +827,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
case RTE_FLOW_ACTION_TYPE_QUEUE:
/* Allow this flow. Redirect to a VNIC. */
act_q = (const struct rte_flow_action_queue *)act->conf;
if (act_q->index >= bp->rx_nr_rings) {
if (!act_q->index || act_q->index >= bp->rx_nr_rings) {

This comment has been minimized.

Sign in to view

Copy link
@legend050709

legend050709 May 19, 2022

May I ask a question? Why Does act_q->index equals 0 return error? I want to create a rte_flow for the rx_queue 0, how to achieve this?

This comment has been minimized.

Sign in to view

Copy link
@ajitkhaparde

ajitkhaparde via email May 19, 2022

Author Contributor

This comment has been minimized.

Sign in to view

Copy link
@legend050709

legend050709 May 19, 2022

Under normal circumstances，Which kind of traffic flow is assigned to Rx queue 0 ?

"That is because we treat queue index 0 as a default queue and keep it for that role. It is a pure PMD design." Is this design for the purpose of Flow Bifurcation?

This comment has been minimized.

Sign in to view

Copy link
@ajitkhaparde

ajitkhaparde via email May 19, 2022

Author Contributor

This comment has been minimized.

Sign in to view

Copy link
@legend050709

legend050709 May 20, 2022

Thank for your answer. But there may be a problem with this design. If there are multiple rx-queues (eg: 4 rx queues, the index range of receive queues is 0-3.)，RSS can act on rx-queues 0 to 3, and fdir can only act on rx-queues 1 to 3.
For example, for a load balancing device in fullnat mode, the traffic in the ingress direction(clinet-->LB-->server) matches RSS, and the traffic in the egress direction(server--->LB-->client) matches FDIR. When the rx-queue is bound to the CPU, and then bound to the specified thread, In order to prevent the connection from missing, it is necessary to ensure that the ingress direction(client-->LB) of the same flow and the egress direction(server-->LB) are both handed over to the same receive queue.
In the ingress direction（client-->LB-->server）, some traffic may sent to the rx-queue 0 through RSS, and the response packets of server(egress direction) cannot reach the rx queue 0, because there is no fdir matching the receive queue 0, which may lead to the miss of conn.
Do you have an idea to solve this kind of problem？

Thanks
Legend050709

This comment has been minimized.

Sign in to view

Copy link
@ajitkhaparde

ajitkhaparde via email May 20, 2022

Author Contributor

This comment has been minimized.

Sign in to view

Copy link
@legend050709

legend050709 May 20, 2022

Using the Device BCM957508-P2100G（Single card dual port, each port 100G）in Dpvs(a layer-4 stateful load balancing software developed based on DPDK).

This comment has been minimized.

Sign in to view

Copy link
@ajitkhaparde

ajitkhaparde via email May 20, 2022

Author Contributor

This comment has been minimized.

Sign in to view

Copy link
@legend050709

legend050709 May 20, 2022 •
edited

I am Using the DPDK 20.11. And I see the judgment(! Act_q->index) still exists in the bnxt_validate_and_parse_flow function of DPDK21.11. Maybe I can have a try on the rough patch. Thanks for your help.

This comment has been minimized.

Sign in to view

Copy link
@ajitkhaparde

ajitkhaparde via email Oct 11, 2022

Author Contributor
rte_flow_error_set(error,
EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION,
Expand All @@ -811,18 +838,77 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
}
PMD_DRV_LOG(DEBUG, "Queue index %d\n", act_q->index);

vnic0 = &bp->vnic_info[0];
vnic = &bp->vnic_info[act_q->index];
vnic_id = attr->group;
if (!vnic_id) {
PMD_DRV_LOG(DEBUG, "Group id is 0\n");
vnic_id = act_q->index;
}

vnic = &bp->vnic_info[vnic_id];
if (vnic == NULL) {
rte_flow_error_set(error,
EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION,
act,
"No matching VNIC for queue ID.");
"No matching VNIC found.");
rc = -rte_errno;
goto ret;
}
if (vnic->rx_queue_cnt) {
if (vnic->start_grp_id != act_q->index) {
PMD_DRV_LOG(ERR,
"VNIC already in use\n");
rte_flow_error_set(error,
EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION,
act,
"VNIC already in use");
rc = -rte_errno;
goto ret;
}
goto use_vnic;
}

rxq = bp->rx_queues[act_q->index];

if (!(dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS) && rxq)
goto use_vnic;

if (!rxq ||
bp->vnic_info[0].fw_grp_ids[act_q->index] !=
INVALID_HW_RING_ID ||
!rxq->rx_deferred_start) {
PMD_DRV_LOG(ERR,
"Queue invalid or used with other VNIC\n");
rte_flow_error_set(error,
EINVAL,
RTE_FLOW_ERROR_TYPE_ACTION,
act,
"Queue invalid queue or in use");
rc = -rte_errno;
goto ret;
}

use_vnic:
rxq->vnic = vnic;
vnic->rx_queue_cnt++;
vnic->start_grp_id = act_q->index;
vnic->end_grp_id = act_q->index;
vnic->func_default = 0; //This is not a default VNIC.

PMD_DRV_LOG(DEBUG, "VNIC found\n");

rc = bnxt_vnic_prep(bp, vnic);
if (rc)
goto ret;

PMD_DRV_LOG(DEBUG,
"vnic[%d] = %p vnic->fw_grp_ids = %p\n",
act_q->index, vnic, vnic->fw_grp_ids);

vnic->ff_pool_idx = vnic_id;
PMD_DRV_LOG(DEBUG,
"Setting vnic ff_idx %d\n", vnic->ff_pool_idx);
filter->dst_id = vnic->fw_vnic_id;
filter1 = bnxt_get_l2_filter(bp, filter, vnic);
if (filter1 == NULL) {
Expand Down Expand Up @@ -989,9 +1075,12 @@ bnxt_match_filter(struct bnxt *bp, struct bnxt_filter_info *nf)
struct rte_flow *flow;
int i;

for (i = bp->nr_vnics - 1; i >= 0; i--) {
for (i = bp->max_vnics; i >= 0; i--) {
struct bnxt_vnic_info *vnic = &bp->vnic_info[i];

if (vnic->fw_vnic_id == INVALID_VNIC_ID)
continue;

STAILQ_FOREACH(flow, &vnic->flow_list, next) {
mf = flow->filter;

Expand Down Expand Up @@ -1063,8 +1152,8 @@ bnxt_flow_create(struct rte_eth_dev *dev,
struct rte_flow_error *error)
{
struct bnxt *bp = dev->data->dev_private;
struct bnxt_filter_info *filter;
struct bnxt_vnic_info *vnic = NULL;
struct bnxt_filter_info *filter;
bool update_flow = false;
struct rte_flow *flow;
unsigned int i;
Expand Down Expand Up @@ -1168,12 +1257,35 @@ bnxt_flow_create(struct rte_eth_dev *dev,
ret = bnxt_hwrm_set_ntuple_filter(bp, filter->dst_id, filter);
}

for (i = 0; i < bp->nr_vnics; i++) {
for (i = 0; i < bp->max_vnics; i++) {
vnic = &bp->vnic_info[i];
if (filter->dst_id == vnic->fw_vnic_id)
if (vnic->fw_vnic_id != INVALID_VNIC_ID &&
filter->dst_id == vnic->fw_vnic_id) {
PMD_DRV_LOG(ERR, "Found matching VNIC Id %d\n",
vnic->ff_pool_idx);
break;
}
}
done:
if (!ret) {
flow->filter = filter;
flow->vnic = vnic;
/* VNIC is set only in case of queue or RSS action */
if (vnic) {
/*
* RxQ0 is not used for flow filters.
*/

if (update_flow) {
ret = -EXDEV;
goto free_flow;
}
STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
}
PMD_DRV_LOG(ERR, "Successfully created flow.\n");
STAILQ_INSERT_TAIL(&vnic->flow_list, flow, next);
return flow;
}
if (!ret) {
flow->filter = filter;
flow->vnic = vnic;
Expand All @@ -1196,7 +1308,7 @@ bnxt_flow_create(struct rte_eth_dev *dev,
rte_flow_error_set(error, ret,
RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
"Flow with pattern exists, updating destination queue");
else
else if (!rte_errno)
rte_flow_error_set(error, -ret,
RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
"Failed to create flow.");
Expand Down
2 changes: 2 additions & 0 deletions drivers/net/bnxt/bnxt_vnic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
#include <sys/queue.h>
#include <stdbool.h>

#define INVALID_VNIC_ID ((uint16_t)-1)

struct bnxt_vnic_info {
STAILQ_ENTRY(bnxt_vnic_info) next;
uint8_t ff_pool_idx;
Expand Down

0 comments on commit d24610f

Please sign in to comment.