From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Ajit Khaparde <ajit.khaparde@broadcom.com>, dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 8/8] net/bnxt: add support for flow filter ops
Date: Tue, 29 Aug 2017 17:16:14 +0100 [thread overview]
Message-ID: <1fefcf30-465b-b8f5-818f-9f4ba50e9224@intel.com> (raw)
In-Reply-To: <20170824162956.62761-10-ajit.khaparde@broadcom.com>
On 8/24/2017 5:29 PM, Ajit Khaparde wrote:
> This patch adds support for flow validate/create/destroy/flush ops.
Can you please update feature file [1] to document flow API support and
for supported filters.
Also I believe this worth mentioning in release notes [2].
[1]
doc/guides/nics/features/bnxt.ini
[2]
doc/guides/rel_notes/release_17_11.rst
> Signed-off-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
<...>
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -610,7 +610,7 @@ static void bnxt_mac_addr_remove_op(struct rte_eth_dev *eth_dev,
> if (filter->mac_index == index) {
> STAILQ_REMOVE(&vnic->filter, filter,
> bnxt_filter_info, next);
> - bnxt_hwrm_clear_filter(bp, filter);
> + bnxt_hwrm_clear_l2_filter(bp, filter);
Is it possible to extract these function name changes into different patch?
<...>
> + default:
> + RTE_LOG(ERR, PMD, "Unknown Flow type");
> + use_ntuple |= 1;
> + //return -1;
Please avoid dead code. "break" ?
> + }
> + item++;
> + }
> + return use_ntuple;
> +}
> +
<...>
> + switch (item->type) {
> + case RTE_FLOW_ITEM_TYPE_ETH:
> + //filter_TYPE = EM
can be removed.
<...>
> + } /*
> + * else {
> + * RTE_LOG(ERR, PMD, "Handle this condition\n");
> + * }
> + */
Please remove unsued code.
<...>
> + rc = bnxt_flow_parse_attr(attr, error);
> + if (rc != 0)
> + goto ret;
> + //Since we support ingress attribute only - right now.
Please prefer /* */ comments.
<...>
> + filter->flags |=
> + HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_FLAGS_DROP;
> + //HWRM_CFA_L2_FILTER_CFG_INPUT_FLAGS_DROP;
> + //HWRM_CFA_L2_FILTER_ALLOC_INPUT_FLAGS_DROP;
> + //HWRM_CFA_NTUPLE_FILTER_ALLOC_INPUT_FLAGS_DROP
> + //HWRM_CFA_EM_FLOW_ALLOC_INPUT_FLAGS_DROP
> + //HWRM_CFA_FLOW_ALLOC_INPUT_ACTION_FLAGS_DROP
These can go.
<...>
> +free_flow:
> + RTE_LOG(ERR, PMD, "Failed to create flow.\n");
> + rte_flow_error_set(error, -ret,
> + RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
> + "Failed to create flow.");
> + rte_free(flow);
> + flow = NULL;
> + return flow;
return NULL; ?
> +static int
> +bnxt_flow_destroy(struct rte_eth_dev *dev,
> + struct rte_flow *flow,
> + struct rte_flow_error *error)
> +{
> + struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
> + struct bnxt_filter_info *filter = flow->filter;
> + int ret = 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)
> + ret = bnxt_hwrm_clear_ntuple_filter(bp, filter);
> +
> + if (!ret) {
> + TAILQ_REMOVE(&bp->flow_list, flow, node);
> + rte_free(flow);
> + } else {
> + rte_flow_error_set(error, -ret,
> + RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
> + "Failed to destroy flow.");
> + }
> +
> + return ret;
> +}
> +
> +static int
> +bnxt_flow_flush(struct rte_eth_dev *dev, struct rte_flow_error *error)
> +{
> + struct bnxt *bp = (struct bnxt *)dev->data->dev_private;
> + struct rte_flow *flow;
> + int ret = 0;
> + void *temp;
> +
> + TAILQ_FOREACH_SAFE(flow, &bp->flow_list, node, temp) {
> + struct bnxt_filter_info *filter = flow->filter;
> +
> + if (filter->filter_type == HWRM_CFA_EM_FILTER)
> + ret = bnxt_hwrm_clear_em_filter(bp, filter);
> + if (filter->filter_type == HWRM_CFA_NTUPLE_FILTER)
> + ret = bnxt_hwrm_clear_ntuple_filter(bp, filter);
> +
> + if (ret) {
> + rte_flow_error_set(error, -ret,
> + RTE_FLOW_ERROR_TYPE_HANDLE, NULL,
> + "Failed to flush flow in HW.");
> + return -rte_errno;
> + }
> +
> + TAILQ_REMOVE(&bp->flow_list, flow, node);
> + rte_free(flow);
This part looks like duplication of bnxt_flow_destroy()
> + }
> +
> + return ret;
> +}
<...>
> diff --git a/drivers/net/bnxt/bnxt_rxq.c b/drivers/net/bnxt/bnxt_rxq.c
> index 0793820..da53e99 100644
> --- a/drivers/net/bnxt/bnxt_rxq.c
> +++ b/drivers/net/bnxt/bnxt_rxq.c
> @@ -98,7 +98,7 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
> }
>
> /* Multi-queue mode */
> - if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_VMDQ_FLAG) {
> + if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_VMDQ_DCB_RSS) {
Is this change related?
> /* VMDq ONLY, VMDq+RSS, VMDq+DCB, VMDq+DCB+RSS */
> enum rte_eth_nb_pools pools;
>
> @@ -113,6 +113,9 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
> pools = conf->nb_queue_pools;
> break;
> }
> + case ETH_MQ_RX_RSS:
> + pools = 1; //bp->rx_cp_nr_rings;
> + break;
> default:
> RTE_LOG(ERR, PMD, "Unsupported mq_mod %d\n",
> dev_conf->rxmode.mq_mode);
> @@ -203,12 +206,42 @@ int bnxt_mq_rx_configure(struct bnxt *bp)
> }
> STAILQ_INSERT_TAIL(&vnic->filter, filter, next);
>
> - if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG)
> - vnic->hash_type =
> - HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV4 |
> - HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV6;
> -
> out:
> + if (dev_conf->rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG) {
> + struct rte_eth_rss_conf *rss = &dev_conf->rx_adv_conf.rss_conf;
> + uint16_t hash_type = 0;
> +
> + if (rss->rss_hf & ETH_RSS_IPV4)
> + hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV4;
> + if (rss->rss_hf & ETH_RSS_NONFRAG_IPV4_TCP)
> + hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_TCP_IPV4;
> + if (rss->rss_hf & ETH_RSS_NONFRAG_IPV4_UDP)
> + hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_UDP_IPV4;
> + if (rss->rss_hf & ETH_RSS_IPV6)
> + hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_IPV6;
> + if (rss->rss_hf & ETH_RSS_NONFRAG_IPV6_TCP)
> + hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_TCP_IPV6;
> + if (rss->rss_hf & ETH_RSS_NONFRAG_IPV6_UDP)
> + hash_type |= HWRM_VNIC_RSS_CFG_INPUT_HASH_TYPE_UDP_IPV6;
> +
> + for (i = 0; i < bp->nr_vnics; i++) {
> + STAILQ_FOREACH(vnic, &bp->ff_pool[i], next) {
> + vnic->hash_type |= hash_type;
> +
> + /*
> + * Use the supplied key if the key length is
> + * acceptable and the rss_key is not NULL
> + */
> + if (rss->rss_key &&
> + rss->rss_key_len <= HW_HASH_KEY_SIZE)
> + memcpy(vnic->rss_hash_key,
> + rss->rss_key, rss->rss_key_len);
> + }
> + }
> + RTE_LOG(ERR, PMD,
> + "VNIC rss hash key_len %d\n", rss->rss_key_len);
> + }
> +
Same for above, these looks like logically unrelated, can be separated
into another patch?
<...>
prev parent reply other threads:[~2017-08-29 16:16 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-24 16:29 [dpdk-dev] [PATCH 0/8] bnxt patchset Ajit Khaparde
2017-08-24 16:29 ` [dpdk-dev] [PATCH 1/8] net/bnxt: add support for xstats get by id Ajit Khaparde
2017-08-29 16:10 ` Ferruh Yigit
2017-08-24 16:29 ` [dpdk-dev] [PATCH 2/8] net/bnxt: add support for rx_queue_count Ajit Khaparde
2017-08-29 16:10 ` Ferruh Yigit
2017-08-24 16:29 ` [dpdk-dev] [PATCH 3/8] net/bnxt: add support for rx_descriptor_status Ajit Khaparde
2017-08-29 16:11 ` Ferruh Yigit
2017-08-24 16:29 ` [dpdk-dev] [PATCH 4/8] net/bnxt: add support for rx_descriptor_done Ajit Khaparde
2017-08-29 16:11 ` Ferruh Yigit
2017-08-24 16:29 ` [dpdk-dev] [PATCH 5/8] net/bnxt: add support for tx_descriptor_status Ajit Khaparde
2017-08-29 16:12 ` Ferruh Yigit
2017-08-24 16:29 ` [dpdk-dev] [PATCH 6/8] net/bnxt: add new HWRM structures Ajit Khaparde
2017-08-24 16:29 ` [dpdk-dev] [PATCH 7/8] net/bnxt: fix HWRM_*() macros and locking Ajit Khaparde
2017-08-29 16:12 ` Ferruh Yigit
2017-08-24 16:29 ` [dpdk-dev] [PATCH 8/8] net/bnxt: add support for flow filter ops Ajit Khaparde
2017-08-29 16:16 ` Ferruh Yigit [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=1fefcf30-465b-b8f5-818f-9f4ba50e9224@intel.com \
--to=ferruh.yigit@intel.com \
--cc=ajit.khaparde@broadcom.com \
--cc=dev@dpdk.org \
/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).