From: Ajit Khaparde <ajit.khaparde@broadcom.com>
To: dpdk-dev <dev@dpdk.org>
Cc: Ferruh Yigit <ferruh.yigit@intel.com>,
Lance Richardson <lance.richardson@broadcom.com>
Subject: Re: [dpdk-dev] [PATCH v2] net/bnxt: add support for flow mark action
Date: Fri, 20 Dec 2019 18:51:04 -0800 [thread overview]
Message-ID: <CACZ4nhtAKy5FAsSgGv4avojWrtr1jnK2C5tuZ26Wb-EY=7082g@mail.gmail.com> (raw)
In-Reply-To: <20191221023849.56131-1-ajit.khaparde@broadcom.com>
On Fri, Dec 20, 2019 at 6:39 PM Ajit Khaparde <ajit.khaparde@broadcom.com>
wrote:
> Add support for RTE_FLOW_ACTION_TYPE_MARK.
> Use the flow_id provided by FW during flow creation to lookup the
> mark id provided by the application.
>
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Lance Richardson <lance.richardson@broadcom.com>
>
Patch applied to dpdk-next-net-brcm
> ---
> drivers/net/bnxt/bnxt.h | 5 +++
> drivers/net/bnxt/bnxt_ethdev.c | 16 ++++++++-
> drivers/net/bnxt/bnxt_filter.h | 3 ++
> drivers/net/bnxt/bnxt_flow.c | 66 ++++++++++++++++++++++++++++------
> drivers/net/bnxt/bnxt_hwrm.c | 2 ++
> drivers/net/bnxt/bnxt_rxr.c | 41 ++++++++++++++++++++-
> drivers/net/bnxt/bnxt_rxr.h | 11 ++++++
> 7 files changed, 132 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/bnxt/bnxt.h b/drivers/net/bnxt/bnxt.h
> index ab0b8dde1..21ca059b8 100644
> --- a/drivers/net/bnxt/bnxt.h
> +++ b/drivers/net/bnxt/bnxt.h
> @@ -515,6 +515,7 @@ struct bnxt {
> #define BNXT_FLAG_INIT_DONE BIT(21)
> #define BNXT_FLAG_FW_CAP_ONE_STEP_TX_TS BIT(22)
> #define BNXT_FLAG_ADV_FLOW_MGMT BIT(23)
> +#define BNXT_FLAG_RX_VECTOR_PKT_MODE BIT(24)
> #define BNXT_PF(bp) (!((bp)->flags & BNXT_FLAG_VF))
> #define BNXT_VF(bp) ((bp)->flags & BNXT_FLAG_VF)
> #define BNXT_NPAR(bp) ((bp)->port_partition_type)
> @@ -654,6 +655,10 @@ struct bnxt {
>
> /* Struct to hold adapter error recovery related info */
> struct bnxt_error_recovery_info *recovery_info;
> +#define BNXT_MARK_TABLE_SZ (sizeof(uint32_t) * 64 * 1024)
> +/* TCAM and EM should be 16-bit only. Other modes not supported. */
> +#define BNXT_FLOW_ID_MASK 0x0000ffff
> + uint32_t *mark_table;
> };
>
> int bnxt_mtu_set_op(struct rte_eth_dev *eth_dev, uint16_t new_mtu);
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c
> index 88df82b86..7b5df9ac1 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -458,6 +458,10 @@ static int bnxt_init_chip(struct bnxt *bp)
> }
> bnxt_print_link_info(bp->eth_dev);
>
> + bp->mark_table = rte_zmalloc("bnxt_mark_table",
> BNXT_MARK_TABLE_SZ, 0);
> + if (!bp->mark_table)
> + PMD_DRV_LOG(ERR, "Allocation of mark table failed\n");
> +
> return 0;
>
> err_free:
> @@ -740,8 +744,10 @@ static int bnxt_scattered_rx(struct rte_eth_dev
> *eth_dev)
> }
>
> static eth_rx_burst_t
> -bnxt_receive_function(__rte_unused struct rte_eth_dev *eth_dev)
> +bnxt_receive_function(struct rte_eth_dev *eth_dev)
> {
> + struct bnxt *bp = eth_dev->data->dev_private;
> +
> #ifdef RTE_ARCH_X86
> #ifndef RTE_LIBRTE_IEEE1588
> /*
> @@ -762,6 +768,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev
> *eth_dev)
> DEV_RX_OFFLOAD_VLAN_FILTER))) {
> PMD_DRV_LOG(INFO, "Using vector mode receive for port
> %d\n",
> eth_dev->data->port_id);
> + bp->flags |= BNXT_FLAG_RX_VECTOR_PKT_MODE;
> return bnxt_recv_pkts_vec;
> }
> PMD_DRV_LOG(INFO, "Vector mode receive disabled for port %d\n",
> @@ -773,6 +780,7 @@ bnxt_receive_function(__rte_unused struct rte_eth_dev
> *eth_dev)
> eth_dev->data->dev_conf.rxmode.offloads);
> #endif
> #endif
> + bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
> return bnxt_recv_pkts;
> }
>
> @@ -956,6 +964,8 @@ static void bnxt_dev_stop_op(struct rte_eth_dev
> *eth_dev)
> bnxt_int_handler(eth_dev);
> bnxt_shutdown_nic(bp);
> bnxt_hwrm_if_change(bp, 0);
> + memset(bp->mark_table, 0, BNXT_MARK_TABLE_SZ);
> + bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
> bp->dev_stopped = 1;
> bp->rx_cosq_cnt = 0;
> }
> @@ -976,6 +986,9 @@ static void bnxt_dev_close_op(struct rte_eth_dev
> *eth_dev)
> bp->grp_info = NULL;
> }
>
> + rte_free(bp->mark_table);
> + bp->mark_table = NULL;
> +
> bnxt_dev_uninit(eth_dev);
> }
>
> @@ -4775,6 +4788,7 @@ bnxt_dev_init(struct rte_eth_dev *eth_dev)
> bp = eth_dev->data->dev_private;
>
> bp->dev_stopped = 1;
> + bp->flags &= ~BNXT_FLAG_RX_VECTOR_PKT_MODE;
>
> if (bnxt_vf_pciid(pci_dev->id.device_id))
> bp->flags |= BNXT_FLAG_VF;
> diff --git a/drivers/net/bnxt/bnxt_filter.h
> b/drivers/net/bnxt/bnxt_filter.h
> index fc40f112b..8f8a4c13b 100644
> --- a/drivers/net/bnxt/bnxt_filter.h
> +++ b/drivers/net/bnxt/bnxt_filter.h
> @@ -23,9 +23,11 @@ struct bnxt;
> #define BNXT_FLOW_L2_INNER_DST_VALID_FLAG BIT(4)
> #define BNXT_FLOW_L2_DROP_FLAG BIT(5)
> #define BNXT_FLOW_PARSE_INNER_FLAG BIT(6)
> +#define BNXT_FLOW_MARK_FLAG BIT(7)
>
> struct bnxt_filter_info {
> STAILQ_ENTRY(bnxt_filter_info) next;
> + uint32_t flow_id;
> uint64_t fw_l2_filter_id;
> struct bnxt_filter_info *matching_l2_fltr_ptr;
> uint64_t fw_em_filter_id;
> @@ -81,6 +83,7 @@ struct bnxt_filter_info {
> * to remember which vnic it was created on
> */
> struct bnxt_vnic_info *vnic;
> + uint32_t mark;
> };
>
> struct bnxt_filter_info *bnxt_alloc_filter(struct bnxt *bp);
> diff --git a/drivers/net/bnxt/bnxt_flow.c b/drivers/net/bnxt/bnxt_flow.c
> index 2bd492ea3..707aedcec 100644
> --- a/drivers/net/bnxt/bnxt_flow.c
> +++ b/drivers/net/bnxt/bnxt_flow.c
> @@ -994,6 +994,7 @@ bnxt_update_filter_flags_en(struct bnxt_filter_info
> *filter,
> }
> filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
> filter->l2_ref_cnt = filter1->l2_ref_cnt;
> + filter->flow_id = filter1->flow_id;
> PMD_DRV_LOG(DEBUG,
> "l2_filter: %p fw_l2_filter_id %" PRIx64 " l2_ref_cnt
> %u\n",
> filter1, filter->fw_l2_filter_id, filter->l2_ref_cnt);
> @@ -1036,6 +1037,8 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
> filter->flags = HWRM_CFA_EM_FLOW_ALLOC_INPUT_FLAGS_PATH_RX;
>
> use_ntuple = bnxt_filter_type_check(pattern, error);
> +
> +start:
> switch (act->type) {
> case RTE_FLOW_ACTION_TYPE_QUEUE:
> /* Allow this flow. Redirect to a VNIC. */
> @@ -1191,6 +1194,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
> }
>
> filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
> + filter->flow_id = filter1->flow_id;
> filter->flags =
> HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_FLAGS_METER;
> break;
> case RTE_FLOW_ACTION_TYPE_VF:
> @@ -1259,6 +1263,7 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev *dev,
> }
>
> filter->fw_l2_filter_id = filter1->fw_l2_filter_id;
> + filter->flow_id = filter1->flow_id;
> break;
> case RTE_FLOW_ACTION_TYPE_RSS:
> rss = (const struct rte_flow_action_rss *)act->conf;
> @@ -1422,6 +1427,34 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev
> *dev,
> PMD_DRV_LOG(DEBUG, "L2 filter created\n");
> bnxt_update_filter_flags_en(filter, filter1, use_ntuple);
> break;
> + case RTE_FLOW_ACTION_TYPE_MARK:
> + if (bp->flags & BNXT_FLAG_RX_VECTOR_PKT_MODE) {
> + PMD_DRV_LOG(DEBUG,
> + "Disable vector processing for
> mark\n");
> + rte_flow_error_set(error,
> + ENOTSUP,
> + RTE_FLOW_ERROR_TYPE_ACTION,
> + act,
> + "Disable vector processing for
> mark");
> + rc = -rte_errno;
> + goto ret;
> + }
> +
> + if (bp->mark_table == NULL) {
> + rte_flow_error_set(error,
> + ENOMEM,
> + RTE_FLOW_ERROR_TYPE_ACTION,
> + act,
> + "Mark table not allocated.");
> + rc = -rte_errno;
> + goto ret;
> + }
> +
> + filter->valid_flags |= BNXT_FLOW_MARK_FLAG;
> + filter->mark = ((const struct rte_flow_action_mark *)
> + act->conf)->id;
> + PMD_DRV_LOG(DEBUG, "Mark the flow %d\n", filter->mark);
> + break;
> default:
> rte_flow_error_set(error,
> EINVAL,
> @@ -1434,20 +1467,17 @@ bnxt_validate_and_parse_flow(struct rte_eth_dev
> *dev,
>
> done:
> act = bnxt_flow_non_void_action(++act);
> - if (act->type != RTE_FLOW_ACTION_TYPE_END) {
> - rte_flow_error_set(error,
> - EINVAL,
> - RTE_FLOW_ERROR_TYPE_ACTION,
> - act,
> - "Invalid action.");
> - rc = -rte_errno;
> - goto ret;
> - }
> + while (act->type != RTE_FLOW_ACTION_TYPE_END)
> + goto start;
>
> return rc;
> ret:
>
> - //TODO: Cleanup according to ACTION TYPE.
> + if (filter1) {
> + bnxt_hwrm_clear_l2_filter(bp, filter1);
> + bnxt_free_filter(bp, filter1);
> + }
> +
> if (rte_errno) {
> if (vnic && STAILQ_EMPTY(&vnic->filter))
> vnic->rx_queue_cnt = 0;
> @@ -1759,6 +1789,17 @@ bnxt_flow_create(struct rte_eth_dev *dev,
> STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
> PMD_DRV_LOG(DEBUG, "Successfully created flow.\n");
> STAILQ_INSERT_TAIL(&vnic->flow_list, flow, next);
> + if (filter->valid_flags & BNXT_FLOW_MARK_FLAG) {
> + PMD_DRV_LOG(DEBUG,
> + "Mark action: mark id 0x%x, flow id
> 0x%x\n",
> + filter->mark, filter->flow_id);
> +
> + /* TCAM and EM should be 16-bit only.
> + * Other modes not supported.
> + */
> + bp->mark_table[filter->flow_id &
> BNXT_FLOW_ID_MASK] =
> + filter->mark;
> + }
> bnxt_release_flow_lock(bp);
> return flow;
> }
> @@ -1850,6 +1891,11 @@ _bnxt_flow_destroy(struct bnxt *bp,
> if (ret == 0)
> PMD_DRV_LOG(ERR, "Could not find matching flow\n");
>
> + if (filter->valid_flags & BNXT_FLOW_MARK_FLAG) {
> + bp->mark_table[filter->flow_id & BNXT_FLOW_ID_MASK] = 0;
> + filter->flow_id = 0;
> + }
> +
> if (filter->filter_type == HWRM_CFA_EM_FILTER)
> ret = bnxt_hwrm_clear_em_filter(bp, filter);
> if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
> diff --git a/drivers/net/bnxt/bnxt_hwrm.c b/drivers/net/bnxt/bnxt_hwrm.c
> index 8178213e5..50272dcf7 100644
> --- a/drivers/net/bnxt/bnxt_hwrm.c
> +++ b/drivers/net/bnxt/bnxt_hwrm.c
> @@ -485,6 +485,7 @@ int bnxt_hwrm_set_l2_filter(struct bnxt *bp,
> HWRM_CHECK_RESULT();
>
> filter->fw_l2_filter_id = rte_le_to_cpu_64(resp->l2_filter_id);
> + filter->flow_id = rte_le_to_cpu_32(resp->flow_id);
> HWRM_UNLOCK();
>
> return rc;
> @@ -4321,6 +4322,7 @@ int bnxt_hwrm_set_ntuple_filter(struct bnxt *bp,
> HWRM_CHECK_RESULT();
>
> filter->fw_ntuple_filter_id =
> rte_le_to_cpu_64(resp->ntuple_filter_id);
> + filter->flow_id = rte_le_to_cpu_32(resp->flow_id);
> HWRM_UNLOCK();
>
> return rc;
> diff --git a/drivers/net/bnxt/bnxt_rxr.c b/drivers/net/bnxt/bnxt_rxr.c
> index 3b713c242..1ae0c3c38 100644
> --- a/drivers/net/bnxt/bnxt_rxr.c
> +++ b/drivers/net/bnxt/bnxt_rxr.c
> @@ -489,7 +489,8 @@ static int bnxt_rx_pkt(struct rte_mbuf **rx_pkt,
> mbuf->hash.rss = rxcmp->rss_hash;
> mbuf->ol_flags |= PKT_RX_RSS_HASH;
> } else {
> - mbuf->hash.fdir.id = rxcmp1->cfa_code;
> + mbuf->hash.fdir.id = bnxt_get_cfa_code_or_mark_id(rxq->bp,
> + rxcmp1);
> mbuf->ol_flags |= PKT_RX_FDIR | PKT_RX_FDIR_ID;
> }
> #ifdef RTE_LIBRTE_IEEE1588
> @@ -895,3 +896,41 @@ int bnxt_init_one_rx_ring(struct bnxt_rx_queue *rxq)
>
> return 0;
> }
> +
> +uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp,
> + struct rx_pkt_cmpl_hi *rxcmp1)
> +{
> + uint32_t cfa_code = 0;
> + uint8_t meta_fmt = 0;
> + uint16_t flags2 = 0;
> + uint32_t meta = 0;
> +
> + cfa_code = rte_le_to_cpu_16(rxcmp1->cfa_code);
> + if (!cfa_code)
> + return 0;
> +
> + if (cfa_code && !bp->mark_table[cfa_code])
> + return cfa_code;
> +
> + flags2 = rte_le_to_cpu_16(rxcmp1->flags2);
> + meta = rte_le_to_cpu_32(rxcmp1->metadata);
> + if (meta) {
> + meta >>= BNXT_RX_META_CFA_CODE_SHIFT;
> +
> + /*
> + * The flags field holds extra bits of info from [6:4]
> + * which indicate if the flow is in TCAM or EM or EEM
> + */
> + meta_fmt = (flags2 & BNXT_CFA_META_FMT_MASK) >>
> + BNXT_CFA_META_FMT_SHFT;
> +
> + /*
> + * meta_fmt == 4 => 'b100 => 'b10x => EM.
> + * meta_fmt == 5 => 'b101 => 'b10x => EM + VLAN
> + * meta_fmt == 6 => 'b110 => 'b11x => EEM
> + * meta_fmt == 7 => 'b111 => 'b11x => EEM + VLAN.
> + */
> + meta_fmt >>= BNXT_CFA_META_FMT_EM_EEM_SHFT;
> + }
> + return bp->mark_table[cfa_code];
> +}
> diff --git a/drivers/net/bnxt/bnxt_rxr.h b/drivers/net/bnxt/bnxt_rxr.h
> index 76bf88d70..bf860020c 100644
> --- a/drivers/net/bnxt/bnxt_rxr.h
> +++ b/drivers/net/bnxt/bnxt_rxr.h
> @@ -226,4 +226,15 @@ uint16_t bnxt_recv_pkts_vec(void *rx_queue, struct
> rte_mbuf **rx_pkts,
> int bnxt_rxq_vec_setup(struct bnxt_rx_queue *rxq);
> #endif
>
> +uint32_t bnxt_get_cfa_code_or_mark_id(struct bnxt *bp,
> + struct rx_pkt_cmpl_hi *rxcmp1);
> +#define BNXT_RX_META_CFA_CODE_SHIFT 19
> +#define BNXT_CFA_CODE_META_SHIFT 16
> +#define BNXT_RX_META_CFA_CODE_INT_ACT_REC_BIT 0x8000000
> +#define BNXT_RX_META_CFA_CODE_EEM_BIT 0x4000000
> +#define BNXT_CFA_META_FMT_MASK 0x70
> +#define BNXT_CFA_META_FMT_SHFT 4
> +#define BNXT_CFA_META_FMT_EM_EEM_SHFT 1
> +#define BNXT_CFA_META_FMT_EEM 3
> +
> #endif
> --
> 2.21.0 (Apple Git-122.2)
>
>
prev parent reply other threads:[~2019-12-21 2:51 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-13 22:42 [dpdk-dev] [PATCH] " Ajit Khaparde
2019-12-16 14:13 ` Lance Richardson
2019-12-21 2:38 ` [dpdk-dev] [PATCH v2] " Ajit Khaparde
2019-12-21 2:51 ` Ajit Khaparde [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CACZ4nhtAKy5FAsSgGv4avojWrtr1jnK2C5tuZ26Wb-EY=7082g@mail.gmail.com' \
--to=ajit.khaparde@broadcom.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=lance.richardson@broadcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).