RE: [PATCH] net/cpfl: reset devargs during the first probe
> -Original Message-
> From: Xing, Beilei
> Sent: Thursday, October 12, 2023 12:47 AM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Xing, Beilei
> Subject: [PATCH] net/cpfl: reset devargs during the first probe
>
> From: Beilei Xing
> Reset devargs during the first probe. Otherwise, probe again will
> be affected.
>
> Fixes: a607312291b3 ("net/cpfl: support probe again")
>
> Signed-off-by: Beilei Xing
> ---
> drivers/net/cpfl/cpfl_ethdev.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index 762fbddfe6..890a027a1d 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -1611,11 +1611,12 @@ cpfl_parse_devargs(struct rte_pci_device *pci_dev,
> struct cpfl_adapter_ext *adap
> struct rte_kvargs *kvlist;
> int ret;
>
> - cpfl_args->req_vport_nb = 0;
> -
> if (devargs == NULL)
> return 0;
>
> + if (first)
> + memset(cpfl_args, 0, sizeof(struct cpfl_devargs));
> +
adapter is allocated by rte_zmalloc. It should be zero already.
If I understand correctly, memset to 0 should be happened when first probe is
done or before probe again but not at the beginning when first probe.
RE: [PATCH v2] net/cpfl: update CP channel API
> -Original Message- > From: Xing, Beilei > Sent: Thursday, October 19, 2023 6:58 PM > To: Wu, Jingjing ; Zhang, Yuying > > Cc: dev@dpdk.org; Xing, Beilei > Subject: [PATCH v2] net/cpfl: update CP channel API > > From: Beilei Xing > > Update the cpchnl2 function type according to the definition in > MEV 1.0 release. > > Signed-off-by: Beilei Xing Acked-by: Jingjing Wu
RE: [PATCH v4 07/15] common/idpf: add irq map/unmap
> @@ -247,8 +247,21 @@ idpf_vport_init(struct idpf_vport *vport,
> goto err_rss_lut;
> }
>
> + /* recv_vectors is used for VIRTCHNL2_OP_ALLOC_VECTORS response,
> + * reserve maximum size for it now, may need optimization in future.
> + */
> + vport->recv_vectors = rte_zmalloc("recv_vectors",
> IDPF_DFLT_MBX_BUF_SIZE, 0);
> + if (vport->recv_vectors == NULL) {
> + DRV_LOG(ERR, "Failed to allocate ecv_vectors");
ecv-> recv?
> + ret = -ENOMEM;
> + goto err_recv_vec;
> + }
> +
> return 0;
>
> +err_recv_vec:
> + rte_free(vport->rss_lut);
> + vport->rss_lut = NULL;
> err_rss_lut:
> vport->dev_data = NULL;
> rte_free(vport->rss_key);
> @@ -261,6 +274,8 @@ idpf_vport_init(struct idpf_vport *vport,
> int
> idpf_vport_deinit(struct idpf_vport *vport)
> {
> + rte_free(vport->recv_vectors);
> + vport->recv_vectors = NULL;
> rte_free(vport->rss_lut);
> vport->rss_lut = NULL;
>
> @@ -298,4 +313,88 @@ idpf_config_rss(struct idpf_vport *vport)
>
> return ret;
> }
> +
> +int
> +idpf_config_irq_map(struct idpf_vport *vport, uint16_t nb_rx_queues)
> +{
> + struct idpf_adapter *adapter = vport->adapter;
> + struct virtchnl2_queue_vector *qv_map;
> + struct idpf_hw *hw = &adapter->hw;
> + uint32_t dynctl_val, itrn_val;
> + uint32_t dynctl_reg_start;
> + uint32_t itrn_reg_start;
> + uint16_t i;
> +
> + qv_map = rte_zmalloc("qv_map",
> + nb_rx_queues *
> + sizeof(struct virtchnl2_queue_vector), 0);
> + if (qv_map == NULL) {
> + DRV_LOG(ERR, "Failed to allocate %d queue-vector map",
> + nb_rx_queues);
> + goto qv_map_alloc_err;
Use error code -ENOMEM instead of using -1?
> + }
> +
> + /* Rx interrupt disabled, Map interrupt only for writeback */
> +
> + /* The capability flags adapter->caps.other_caps should be
> + * compared with bit VIRTCHNL2_CAP_WB_ON_ITR here. The if
> + * condition should be updated when the FW can return the
> + * correct flag bits.
> + */
> + dynctl_reg_start =
> + vport->recv_vectors->vchunks.vchunks->dynctl_reg_start;
> + itrn_reg_start =
> + vport->recv_vectors->vchunks.vchunks->itrn_reg_start;
> + dynctl_val = IDPF_READ_REG(hw, dynctl_reg_start);
> + DRV_LOG(DEBUG, "Value of dynctl_reg_start is 0x%x", dynctl_val);
> + itrn_val = IDPF_READ_REG(hw, itrn_reg_start);
> + DRV_LOG(DEBUG, "Value of itrn_reg_start is 0x%x", itrn_val);
> + /* Force write-backs by setting WB_ON_ITR bit in DYN_CTL
> + * register. WB_ON_ITR and INTENA are mutually exclusive
> + * bits. Setting WB_ON_ITR bits means TX and RX Descs
> + * are written back based on ITR expiration irrespective
> + * of INTENA setting.
> + */
> + /* TBD: need to tune INTERVAL value for better performance. */
> + itrn_val = (itrn_val == 0) ? IDPF_DFLT_INTERVAL : itrn_val;
> + dynctl_val = VIRTCHNL2_ITR_IDX_0 <<
> + PF_GLINT_DYN_CTL_ITR_INDX_S |
> + PF_GLINT_DYN_CTL_WB_ON_ITR_M |
> + itrn_val << PF_GLINT_DYN_CTL_INTERVAL_S;
> + IDPF_WRITE_REG(hw, dynctl_reg_start, dynctl_val);
> +
> + for (i = 0; i < nb_rx_queues; i++) {
> + /* map all queues to the same vector */
> + qv_map[i].queue_id = vport->chunks_info.rx_start_qid + i;
> + qv_map[i].vector_id =
> + vport->recv_vectors->vchunks.vchunks->start_vector_id;
> + }
> + vport->qv_map = qv_map;
> +
> + if (idpf_vc_config_irq_map_unmap(vport, nb_rx_queues, true) != 0) {
> + DRV_LOG(ERR, "config interrupt mapping failed");
> + goto config_irq_map_err;
> + }
> +
> + return 0;
> +
> +config_irq_map_err:
> + rte_free(vport->qv_map);
> + vport->qv_map = NULL;
> +
> +qv_map_alloc_err:
> + return -1;
> +}
> +
RE: [PATCH v4 09/15] common/idpf: add vport info initialization
> +int
> +idpf_create_vport_info_init(struct idpf_vport *vport,
> + struct virtchnl2_create_vport *vport_info)
> +{
> + struct idpf_adapter *adapter = vport->adapter;
> +
> + vport_info->vport_type = rte_cpu_to_le_16(VIRTCHNL2_VPORT_TYPE_DEFAULT);
> + if (adapter->txq_model == 0) {
> + vport_info->txq_model =
> + rte_cpu_to_le_16(VIRTCHNL2_QUEUE_MODEL_SPLIT);
Byte order is consider for txq_model, how about other fields?
> + vport_info->num_tx_q = IDPF_DEFAULT_TXQ_NUM;
> + vport_info->num_tx_complq =
> + IDPF_DEFAULT_TXQ_NUM * IDPF_TX_COMPLQ_PER_GRP;
> + } else {
> + vport_info->txq_model =
> + rte_cpu_to_le_16(VIRTCHNL2_QUEUE_MODEL_SINGLE);
> + vport_info->num_tx_q = IDPF_DEFAULT_TXQ_NUM;
> + vport_info->num_tx_complq = 0;
> + }
> + if (adapter->rxq_model == 0) {
> + vport_info->rxq_model =
> + rte_cpu_to_le_16(VIRTCHNL2_QUEUE_MODEL_SPLIT);
> + vport_info->num_rx_q = IDPF_DEFAULT_RXQ_NUM;
> + vport_info->num_rx_bufq =
> + IDPF_DEFAULT_RXQ_NUM * IDPF_RX_BUFQ_PER_GRP;
> + } else {
> + vport_info->rxq_model =
> + rte_cpu_to_le_16(VIRTCHNL2_QUEUE_MODEL_SINGLE);
> + vport_info->num_rx_q = IDPF_DEFAULT_RXQ_NUM;
> + vport_info->num_rx_bufq = 0;
> + }
> +
> + return 0;
> +}
> +
RE: [PATCH v3 1/6] common/idpf: add hw statistics
> @@ -327,6 +407,11 @@ idpf_dev_start(struct rte_eth_dev *dev)
> goto err_vport;
> }
>
> + if (idpf_dev_stats_reset(dev)) {
> + PMD_DRV_LOG(ERR, "Failed to reset stats");
> + goto err_vport;
If stats reset fails, will block the start process and roll back? I think print
ERR may be enough.
> + }
> +
> vport->stopped = 0;
>
> return 0;
> @@ -606,6 +691,8 @@ static const struct eth_dev_ops idpf_eth_dev_ops = {
> .tx_queue_release = idpf_dev_tx_queue_release,
> .mtu_set= idpf_dev_mtu_set,
> .dev_supported_ptypes_get = idpf_dev_supported_ptypes_get,
> + .stats_get = idpf_dev_stats_get,
> + .stats_reset= idpf_dev_stats_reset,
> };
>
> static uint16_t
> --
> 2.25.1
RE: [PATCH v3 2/6] common/idpf: add RSS set/get ops
> +static int idpf_config_rss_hf(struct idpf_vport *vport, uint64_t rss_hf)
> +{
> + uint64_t hena = 0, valid_rss_hf = 0;
According to the coding style, only the last variable on a line should be
initialized.
> + int ret = 0;
> + uint16_t i;
> +
> + /**
> + * RTE_ETH_RSS_IPV4 and RTE_ETH_RSS_IPV6 can be considered as 2
> + * generalizations of all other IPv4 and IPv6 RSS types.
> + */
> + if (rss_hf & RTE_ETH_RSS_IPV4)
> + rss_hf |= idpf_ipv4_rss;
> +
> + if (rss_hf & RTE_ETH_RSS_IPV6)
> + rss_hf |= idpf_ipv6_rss;
> +
> + for (i = 0; i < RTE_DIM(idpf_map_hena_rss); i++) {
> + uint64_t bit = BIT_ULL(i);
> +
> + if (idpf_map_hena_rss[i] & rss_hf) {
> + valid_rss_hf |= idpf_map_hena_rss[i];
> + hena |= bit;
> + }
> + }
> +
> + vport->rss_hf = hena;
> +
> + ret = idpf_vc_set_rss_hash(vport);
> + if (ret != 0) {
> + PMD_DRV_LOG(WARNING,
> + "fail to set RSS offload types, ret: %d", ret);
> + return ret;
> + }
> +
> + if (valid_rss_hf & idpf_ipv4_rss)
> + valid_rss_hf |= rss_hf & RTE_ETH_RSS_IPV4;
> +
> + if (valid_rss_hf & idpf_ipv6_rss)
> + valid_rss_hf |= rss_hf & RTE_ETH_RSS_IPV6;
> +
> + if (rss_hf & ~valid_rss_hf)
> + PMD_DRV_LOG(WARNING, "Unsupported rss_hf 0x%" PRIx64,
> + rss_hf & ~valid_rss_hf);
It makes me a bit confused, valid_rss_hf is would be the sub of rss_hf
according above assignment. Would it be possible to go here?
And if it is possible, why not set valid_rss_hf before calling vc command?
> + vport->last_general_rss_hf = valid_rss_hf;
> +
> + return ret;
> +}
> +
> static int
> idpf_init_rss(struct idpf_vport *vport)
> {
> @@ -256,6 +357,204 @@ idpf_init_rss(struct idpf_vport *vport)
> return ret;
> }
>
> +static int
> +idpf_rss_reta_update(struct rte_eth_dev *dev,
> + struct rte_eth_rss_reta_entry64 *reta_conf,
> + uint16_t reta_size)
> +{
> + struct idpf_vport *vport = dev->data->dev_private;
> + struct idpf_adapter *adapter = vport->adapter;
> + uint16_t idx, shift;
> + uint32_t *lut;
> + int ret = 0;
> + uint16_t i;
> +
> + if (adapter->caps.rss_caps == 0 || dev->data->nb_rx_queues == 0) {
> + PMD_DRV_LOG(DEBUG, "RSS is not supported");
> + return -ENOTSUP;
> + }
> +
> + if (reta_size != vport->rss_lut_size) {
> + PMD_DRV_LOG(ERR, "The size of hash lookup table configured "
> + "(%d) doesn't match the number of hardware can
> "
> + "support (%d)",
> + reta_size, vport->rss_lut_size);
> + return -EINVAL;
> + }
> +
> + /* It MUST use the current LUT size to get the RSS lookup table,
> + * otherwise if will fail with -100 error code.
> + */
> + lut = rte_zmalloc(NULL, reta_size * sizeof(uint32_t), 0);
> + if (!lut) {
> + PMD_DRV_LOG(ERR, "No memory can be allocated");
> + return -ENOMEM;
> + }
> + /* store the old lut table temporarily */
> + rte_memcpy(lut, vport->rss_lut, reta_size * sizeof(uint32_t));
Stored the vport->rss_lut to lut? But you overwrite the lut below?
> +
> + for (i = 0; i < reta_size; i++) {
> + idx = i / RTE_ETH_RETA_GROUP_SIZE;
> + shift = i % RTE_ETH_RETA_GROUP_SIZE;
> + if (reta_conf[idx].mask & (1ULL << shift))
> + lut[i] = reta_conf[idx].reta[shift];
> + }
> +
> + rte_memcpy(vport->rss_lut, lut, reta_size * sizeof(uint32_t));
> + /* send virtchnl ops to configure RSS */
> + ret = idpf_vc_set_rss_lut(vport);
> + if (ret) {
> + PMD_INIT_LOG(ERR, "Failed to configure RSS lut");
> + goto out;
> + }
> +out:
> + rte_free(lut);
> +
> + return ret;
> +}
RE: [PATCH v3 3/6] common/idpf: support single q scatter RX datapath
>
> +uint16_t
> +idpf_singleq_recv_scatter_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> +uint16_t nb_pkts)
> +{
> + struct idpf_rx_queue *rxq = rx_queue;
> + volatile union virtchnl2_rx_desc *rx_ring = rxq->rx_ring;
> + volatile union virtchnl2_rx_desc *rxdp;
> + union virtchnl2_rx_desc rxd;
> + struct idpf_adapter *ad;
> + struct rte_mbuf *first_seg = rxq->pkt_first_seg;
> + struct rte_mbuf *last_seg = rxq->pkt_last_seg;
> + struct rte_mbuf *rxm;
> + struct rte_mbuf *nmb;
> + struct rte_eth_dev *dev;
> + const uint32_t *ptype_tbl = rxq->adapter->ptype_tbl;
> + uint16_t nb_hold = 0, nb_rx = 0;
According to the coding style, only the last variable on a line should be
initialized.
> + uint16_t rx_id = rxq->rx_tail;
> + uint16_t rx_packet_len;
> + uint16_t rx_status0;
> + uint64_t pkt_flags;
> + uint64_t dma_addr;
> + uint64_t ts_ns;
> +
RE: [PATCH v3 5/6] common/idpf: add alarm to support handle vchnl message
> @@ -83,12 +84,49 @@ static int
> idpf_dev_link_update(struct rte_eth_dev *dev,
>__rte_unused int wait_to_complete)
> {
> + struct idpf_vport *vport = dev->data->dev_private;
> struct rte_eth_link new_link;
>
> memset(&new_link, 0, sizeof(new_link));
>
> - new_link.link_speed = RTE_ETH_SPEED_NUM_NONE;
> + switch (vport->link_speed) {
> + case 10:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_10M;
> + break;
> + case 100:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_100M;
> + break;
> + case 1000:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_1G;
> + break;
> + case 1:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_10G;
> + break;
> + case 2:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_20G;
> + break;
> + case 25000:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_25G;
> + break;
> + case 4:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_40G;
> + break;
> + case 5:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_50G;
> + break;
> + case 10:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_100G;
> + break;
> + case 20:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_200G;
> + break;
> + default:
> + new_link.link_speed = RTE_ETH_SPEED_NUM_NONE;
> + }
> +
> new_link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
> + new_link.link_status = vport->link_up ? RTE_ETH_LINK_UP :
> + RTE_ETH_LINK_DOWN;
> new_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> RTE_ETH_LINK_SPEED_FIXED);
Better to use RTE_ETH_LINK_[AUTONEG/FIXED] instead.
>
> @@ -927,6 +965,127 @@ idpf_parse_devargs(struct rte_pci_device *pci_dev,
> struct
> idpf_adapter_ext *adap
> return ret;
> }
>
> +static struct idpf_vport *
> +idpf_find_vport(struct idpf_adapter_ext *adapter, uint32_t vport_id)
> +{
> + struct idpf_vport *vport = NULL;
> + int i;
> +
> + for (i = 0; i < adapter->cur_vport_nb; i++) {
> + vport = adapter->vports[i];
> + if (vport->vport_id != vport_id)
> + continue;
> + else
> + return vport;
> + }
> +
> + return vport;
> +}
> +
> +static void
> +idpf_handle_event_msg(struct idpf_vport *vport, uint8_t *msg, uint16_t
> msglen)
> +{
> + struct virtchnl2_event *vc_event = (struct virtchnl2_event *)msg;
> + struct rte_eth_dev *dev = (struct rte_eth_dev *)vport->dev;
> +
> + if (msglen < sizeof(struct virtchnl2_event)) {
> + PMD_DRV_LOG(ERR, "Error event");
> + return;
> + }
> +
> + switch (vc_event->event) {
> + case VIRTCHNL2_EVENT_LINK_CHANGE:
> + PMD_DRV_LOG(DEBUG, "VIRTCHNL2_EVENT_LINK_CHANGE");
> + vport->link_up = vc_event->link_status;
Any conversion between bool and uint8?
RE: [PATCH v3 5/6] common/idpf: add alarm to support handle vchnl message
> > > +
> > > new_link.link_duplex = RTE_ETH_LINK_FULL_DUPLEX;
> > > + new_link.link_status = vport->link_up ? RTE_ETH_LINK_UP :
> > > + RTE_ETH_LINK_DOWN;
> > > new_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> > > RTE_ETH_LINK_SPEED_FIXED);
> > Better to use RTE_ETH_LINK_[AUTONEG/FIXED] instead.
> >
> [Liu, Mingxia] According to the comment description of struct rte_eth_conf,
> RTE_ETH_LINK_SPEED_FIXED is better.
> struct rte_eth_conf {
> uint32_t link_speeds; /**< bitmap of RTE_ETH_LINK_SPEED_XXX of speeds to be
> used. RTE_ETH_LINK_SPEED_FIXED disables link
> autonegotiation, and a unique speed shall be
> set. Otherwise, the bitmap defines the set of
> speeds to be advertised. If the special value
> RTE_ETH_LINK_SPEED_AUTONEG (0) is used, all
> speeds
> supported are advertised. */
>
I am talking about link_autoneg but not link_speeds
RE: [PATCH v4 02/10] net/cpfl: introduce interface structure
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, September 8, 2023 7:17 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei
> ; Zhang, Qi Z
> Subject: [PATCH v4 02/10] net/cpfl: introduce interface structure
>
> From: Beilei Xing
>
> Introduce cplf interface structure to distinguish vport and port
> representor.
>
> Signed-off-by: Qi Zhang
> Signed-off-by: Beilei Xing
> ---
> drivers/net/cpfl/cpfl_ethdev.c | 3 +++
> drivers/net/cpfl/cpfl_ethdev.h | 16
> 2 files changed, 19 insertions(+)
>
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index 46b3a52e49..92fe92c00f 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -1803,6 +1803,9 @@ cpfl_dev_vport_init(struct rte_eth_dev *dev, void
> *init_params)
> goto err;
> }
>
> + cpfl_vport->itf.type = CPFL_ITF_TYPE_VPORT;
> + cpfl_vport->itf.adapter = adapter;
> + cpfl_vport->itf.data = dev->data;
> adapter->vports[param->idx] = cpfl_vport;
> adapter->cur_vports |= RTE_BIT32(param->devarg_id);
> adapter->cur_vport_nb++;
> diff --git a/drivers/net/cpfl/cpfl_ethdev.h b/drivers/net/cpfl/cpfl_ethdev.h
> index b637bf2e45..53e45035e8 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.h
> +++ b/drivers/net/cpfl/cpfl_ethdev.h
> @@ -86,7 +86,19 @@ struct p2p_queue_chunks_info {
> uint32_t rx_buf_qtail_spacing;
> };
>
> +enum cpfl_itf_type {
> + CPFL_ITF_TYPE_VPORT,
> + CPFL_ITF_TYPE_REPRESENTOR
Defined but not used in this patch, how about move CPFL_ITF_TYPE_REPRESENTOR to
the patch that uses it?
> +};
> +
> +struct cpfl_itf {
> + enum cpfl_itf_type type;
> + struct cpfl_adapter_ext *adapter;
> + void *data;
> +};
> +
> struct cpfl_vport {
> + struct cpfl_itf itf;
> struct idpf_vport base;
> struct p2p_queue_chunks_info *p2p_q_chunks_info;
>
> @@ -124,5 +136,9 @@ TAILQ_HEAD(cpfl_adapter_list, cpfl_adapter_ext);
> RTE_DEV_TO_PCI((eth_dev)->device)
> #define CPFL_ADAPTER_TO_EXT(p) \
> container_of((p), struct cpfl_adapter_ext, base)
> +#define CPFL_DEV_TO_VPORT(dev) \
> + ((struct cpfl_vport *)((dev)->data->dev_private))
> +#define CPFL_DEV_TO_ITF(dev) \
> + ((struct cpfl_itf *)((dev)->data->dev_private))
>
> #endif /* _CPFL_ETHDEV_H_ */
> --
> 2.34.1
RE: [PATCH v4 03/10] net/cpfl: refine handle virtual channel message
> -static struct idpf_vport *
> +static struct cpfl_vport *
> cpfl_find_vport(struct cpfl_adapter_ext *adapter, uint32_t vport_id)
> {
> - struct idpf_vport *vport = NULL;
> + struct cpfl_vport *vport = NULL;
> int i;
>
> for (i = 0; i < adapter->cur_vport_nb; i++) {
> - vport = &adapter->vports[i]->base;
> - if (vport->vport_id != vport_id)
> + vport = adapter->vports[i];
> + if (vport->base.vport_id != vport_id)
Check if vport is NULL to ensure the structure access?
> continue;
> else
> return vport;
> }
>
> - return vport;
> + return NULL;
> }
RE: [PATCH v4 08/10] net/cpfl: support vport list/info get
> -Original Message- > From: Xing, Beilei > Sent: Friday, September 8, 2023 7:17 PM > To: Wu, Jingjing > Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei > > Subject: [PATCH v4 08/10] net/cpfl: support vport list/info get > > From: Beilei Xing > > Support cp channel ops CPCHNL2_OP_CPF_GET_VPORT_LIST and > CPCHNL2_OP_CPF_GET_VPORT_INFO. > > Signed-off-by: Beilei Xing Can we merge this patch to previous cpchnl handle one or move ahead before representor is introduced?
RE: [PATCH v4 09/10] net/cpfl: create port representor
> + /* warning if no match vport detected */ > + if (!matched) > + PMD_INIT_LOG(WARNING, "No matched vport for > representor %s " > + "creation will be deferred when > vport is detected", > + name); > + If vport info is responded successfully, what is the case that matched is false? And I did not find the defer process. > + rte_spinlock_unlock(&adapter->vport_map_lock); > + } > + > +err: > + rte_spinlock_unlock(&adapter->repr_lock); > + rte_free(vlist_resp); > + return ret; > +}
RE: [PATCH v4 10/10] net/cpfl: support link update for representor
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, September 8, 2023 7:17 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei
>
> Subject: [PATCH v4 10/10] net/cpfl: support link update for representor
>
> From: Beilei Xing
>
> Add link update ops for representor.
>
> Signed-off-by: Jingjing Wu
> Signed-off-by: Beilei Xing
> ---
> drivers/net/cpfl/cpfl_ethdev.h | 1 +
> drivers/net/cpfl/cpfl_representor.c | 21 +
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/net/cpfl/cpfl_ethdev.h b/drivers/net/cpfl/cpfl_ethdev.h
> index 4937d2c6e3..0dd9d4e7f9 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.h
> +++ b/drivers/net/cpfl/cpfl_ethdev.h
> @@ -162,6 +162,7 @@ struct cpfl_repr {
> struct cpfl_repr_id repr_id;
> struct rte_ether_addr mac_addr;
> struct cpfl_vport_info *vport_info;
> + bool func_up; /* If the represented function is up */
> };
>
> struct cpfl_adapter_ext {
> diff --git a/drivers/net/cpfl/cpfl_representor.c
> b/drivers/net/cpfl/cpfl_representor.c
> index 0cd92b1351..3c0fa957de 100644
> --- a/drivers/net/cpfl/cpfl_representor.c
> +++ b/drivers/net/cpfl/cpfl_representor.c
> @@ -308,6 +308,23 @@ cpfl_repr_tx_queue_setup(__rte_unused struct
> rte_eth_dev *dev,
> return 0;
> }
>
> +static int
> +cpfl_repr_link_update(struct rte_eth_dev *ethdev,
> + __rte_unused int wait_to_complete)
> +{
> + struct cpfl_repr *repr = CPFL_DEV_TO_REPR(ethdev);
> + struct rte_eth_link *dev_link = ðdev->data->dev_link;
> +
> + if (!(ethdev->data->dev_flags & RTE_ETH_DEV_REPRESENTOR)) {
> + PMD_INIT_LOG(ERR, "This ethdev is not representor.");
> + return -EINVAL;
> + }
> + dev_link->link_status = repr->func_up ?
> + RTE_ETH_LINK_UP : RTE_ETH_LINK_DOWN;
> +
> + return 0;
> +}
> +
> static const struct eth_dev_ops cpfl_repr_dev_ops = {
> .dev_start = cpfl_repr_dev_start,
> .dev_stop = cpfl_repr_dev_stop,
> @@ -317,6 +334,8 @@ static const struct eth_dev_ops cpfl_repr_dev_ops = {
>
> .rx_queue_setup = cpfl_repr_rx_queue_setup,
> .tx_queue_setup = cpfl_repr_tx_queue_setup,
> +
> + .link_update= cpfl_repr_link_update,
> };
>
> static int
> @@ -331,6 +350,8 @@ cpfl_repr_init(struct rte_eth_dev *eth_dev, void
> *init_param)
> repr->itf.type = CPFL_ITF_TYPE_REPRESENTOR;
> repr->itf.adapter = adapter;
> repr->itf.data = eth_dev->data;
> + if (repr->vport_info->vport_info.vport_status ==
> CPCHNL2_VPORT_STATUS_ENABLED)
> + repr->func_up = true;
>
Now event process? Think about the vsi status changes?
> eth_dev->dev_ops = &cpfl_repr_dev_ops;
>
> --
> 2.34.1
RE: [PATCH v3 1/9] net/cpfl: parse flow parser file in devargs
> -Original Message- > From: Qiao, Wenjing > Sent: Wednesday, September 6, 2023 5:34 PM > To: Zhang, Yuying ; dev@dpdk.org; Zhang, Qi Z > ; Wu, Jingjing ; Xing, Beilei > > Cc: Liu, Mingxia ; Qiao, Wenjing > > Subject: [PATCH v3 1/9] net/cpfl: parse flow parser file in devargs > > Add devargs "flow_parser" for rte_flow json parser. > > Signed-off-by: Wenjing Qiao > --- > doc/guides/nics/cpfl.rst | 32 > drivers/net/cpfl/cpfl_ethdev.c | 38 > +- > drivers/net/cpfl/cpfl_ethdev.h | 3 +++ > drivers/net/cpfl/meson.build | 6 ++ > 4 files changed, 78 insertions(+), 1 deletion(-) > > diff --git a/doc/guides/nics/cpfl.rst b/doc/guides/nics/cpfl.rst > index c20334230b..7032dd1a1a 100644 > --- a/doc/guides/nics/cpfl.rst > +++ b/doc/guides/nics/cpfl.rst > @@ -128,12 +128,24 @@ Runtime Configuration > > -a BDF,representor=vf[0-3],representor=c1pf1 > > +- ``flow_parser`` (default ``not enabled``) > + > + The PMD supports using a JSON file to parse rte_flow tokens into low level > hardware > + resources defined in a DDP package file. > + > + The user can specify the path of json file, for example:: > + > +-a ca:00.0,flow_parser="refpkg.json" > + > + Then the PMD will load json file for device ``ca:00.0``. > + The parameter is optional. > > Driver compilation and testing > -- > > Refer to the document :doc:`build_and_test` for details. > > +Rte flow need to install json-c library. > > Features > > @@ -164,3 +176,23 @@ Hairpin queue > E2100 Series can loopback packets from RX port to TX port. > This feature is called port-to-port or hairpin. > Currently, the PMD only supports single port hairpin. > + > +Rte_flow > +~ > + > +Rte_flow uses a json file to direct CPF PMD to parse rte_flow tokens into > +low level hardware resources defined in a DDP package file. > + > +#. install json-c library:: > + > + .. code-block:: console > + > + git clone https://github.com/json-c/json-c.git > + cd json-c > + git checkout 777dd06be83ef7fac71c2218b565557cd068a714 > + Json-c is the dependency, we can install by package management tool, such as apt, can you add that refer? If we need to install from source code, version number might be better that commit id.
RE: [PATCH v3 2/9] net/cpfl: add flow json parser
> +static int
> +cpfl_json_object_to_int(json_object *object, const char *name, int *value)
> +{
> + json_object *subobject;
> +
> + if (!object) {
> + PMD_DRV_LOG(ERR, "object doesn't exist.");
> + return -EINVAL;
> + }
> + subobject = json_object_object_get(object, name);
> + if (!subobject) {
> + PMD_DRV_LOG(ERR, "%s doesn't exist.", name);
> + return -EINVAL;
> + }
> + *value = json_object_get_int(subobject);
> +
> + return 0;
> +}
> +
> +static int
> +cpfl_json_object_to_uint16(json_object *object, const char *name, uint16_t
> *value)
> +{
Looks no need to define a new function as there is no difference with
cpfl_json_object_to_int func beside the type of return value.
[...]
> +
> +static int
> +cpfl_flow_js_pattern_key_proto_field(json_object *cjson_field,
> + struct cpfl_flow_js_pr_key_proto *js_field)
> +{
> + int len, i;
> +
> + if (!cjson_field)
> + return 0;
> + len = json_object_array_length(cjson_field);
> + js_field->fields_size = len;
> + if (len == 0)
Move if check above, before set js_field->fields_size?
> + return 0;
> + js_field->fields =
> + rte_malloc(NULL, sizeof(struct cpfl_flow_js_pr_key_proto_field) *
> len, 0);
> + if (!js_field->fields) {
> + PMD_DRV_LOG(ERR, "Failed to alloc memory.");
> + return -ENOMEM;
> + }
> + for (i = 0; i < len; i++) {
> + json_object *object;
> + const char *name, *mask;
> +
> + object = json_object_array_get_idx(cjson_field, i);
> + name = cpfl_json_object_to_string(object, "name");
> + if (!name) {
> + PMD_DRV_LOG(ERR, "Can not parse string 'name'.");
> + goto err;
> + }
> + if (strlen(name) > CPFL_FLOW_JSON_STR_SIZE_MAX) {
> + PMD_DRV_LOG(ERR, "The 'name' is too long.");
> + goto err;
> + }
> + memcpy(js_field->fields[i].name, name, strlen(name));
Is js_field->fields[i].name zeroed? If not, using strlen() cannot guarantee
string copy correct.
> + if (js_field->type == RTE_FLOW_ITEM_TYPE_ETH ||
> + js_field->type == RTE_FLOW_ITEM_TYPE_IPV4) {
> + mask = cpfl_json_object_to_string(object, "mask");
> + if (!mask) {
> + PMD_DRV_LOG(ERR, "Can not parse string
> 'mask'.");
> + goto err;
> + }
> + memcpy(js_field->fields[i].mask, mask, strlen(mask));
The same as above.
> + } else {
> + uint32_t mask_32b;
> + int ret;
> +
> + ret = cpfl_json_object_to_uint32(object, "mask",
> &mask_32b);
> + if (ret < 0) {
> + PMD_DRV_LOG(ERR, "Can not parse uint32
> 'mask'.");
> + goto err;
> + }
> + js_field->fields[i].mask_32b = mask_32b;
> + }
> + }
> +
> + return 0;
> +
> +err:
> + rte_free(js_field->fields);
> + return -EINVAL;
> +}
> +
> +static int
> +cpfl_flow_js_pattern_key_proto(json_object *cjson_pr_key_proto, struct
> cpfl_flow_js_pr *js_pr)
> +{
> + int len, i, ret;
> +
> + len = json_object_array_length(cjson_pr_key_proto);
> + js_pr->key.proto_size = len;
> + js_pr->key.protocols = rte_malloc(NULL, sizeof(struct
> cpfl_flow_js_pr_key_proto) * len, 0);
> + if (!js_pr->key.protocols) {
> + PMD_DRV_LOG(ERR, "Failed to alloc memory.");
> + return -ENOMEM;
> + }
> +
> + for (i = 0; i < len; i++) {
> + json_object *object, *cjson_pr_key_proto_fields;
> + const char *type;
> + enum rte_flow_item_type item_type;
> +
> + object = json_object_array_get_idx(cjson_pr_key_proto, i);
> + /* pr->key->proto->type */
> + type = cpfl_json_object_to_string(object, "type");
> + if (!type) {
> + PMD_DRV_LOG(ERR, "Can not parse string 'type'.");
> + goto err;
> + }
> + item_type = cpfl_get_item_type_by_str(type);
> + if (item_type == RTE_FLOW_ITEM_TYPE_VOID)
> + goto err;
> + js_pr->key.protocols[i].type = item_type;
> + /* pr->key->proto->fields */
> + cjson_pr_key_proto_fields = json_object_object_get(object,
> "fields");
> + ret =
> cpfl_flow_js_pattern_key_proto_field(cjson_pr_key_proto_fields,
> +&js_pr-
> >key.protocols[i]);
> + if (ret < 0)
> + goto err;
> + }
> +
> + return 0;
> +
> +err:
> + rte_free(js_pr->key.protocols);
> + retu
RE: [PATCH v3 4/9] net/cpfl: setup ctrl path
> -Original Message- > From: Qiao, Wenjing > Sent: Wednesday, September 6, 2023 5:34 PM > To: Zhang, Yuying ; dev@dpdk.org; Zhang, Qi Z > ; Wu, Jingjing ; Xing, Beilei > > Cc: Liu, Mingxia ; Qiao, Wenjing > > Subject: [PATCH v3 4/9] net/cpfl: setup ctrl path > > Setup the control vport and control queue for flow offloading. In general, "[PATCH 3/9] net/cpfl: add FXP low level implementation" also contains ctrl queue setup functions. May need better organize the patch set.
RE: [PATCH v6 00/10] net/cpfl: support port representor
> -Original Message- > From: Xing, Beilei > Sent: Wednesday, September 13, 2023 1:30 AM > To: Wu, Jingjing > Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei > > Subject: [PATCH v6 00/10] net/cpfl: support port representor > > From: Beilei Xing Acked-by: Jingjing Wu
RE: [PATCH] raw/ntb: add support for 5th and 6th Gen Intel Xeon
> -Original Message- > From: Guo, Junfeng > Sent: Friday, September 15, 2023 3:08 PM > To: Wu, Jingjing > Cc: dev@dpdk.org; Guo, Junfeng > Subject: [PATCH] raw/ntb: add support for 5th and 6th Gen Intel Xeon > > Add support for 5th and 6th Gen Intel Xeon Scalable processors. Note > that NTB devices within the 3rd, 4th and 5th Gen Intel Xeon Scalable > processors share the same device id, and compliant to PCIe 4.0 spec. > And the NTB devices within 6th Gen Intel Xeon compliant to PCIe 5.0. > > Signed-off-by: Junfeng Guo > --- > doc/guides/rawdevs/ntb.rst | 8 +- > drivers/raw/ntb/ntb.c | 10 +- > drivers/raw/ntb/ntb.h | 6 +- > drivers/raw/ntb/ntb_hw_intel.c | 190 +++-- > drivers/raw/ntb/ntb_hw_intel.h | 96 ++--- > usertools/dpdk-devbind.py | 8 +- > 6 files changed, 159 insertions(+), 159 deletions(-) > > diff --git a/doc/guides/rawdevs/ntb.rst b/doc/guides/rawdevs/ntb.rst > index f8befc6594..e663b7a088 100644 > --- a/doc/guides/rawdevs/ntb.rst > +++ b/doc/guides/rawdevs/ntb.rst > @@ -153,6 +153,8 @@ Limitation > > This PMD is only supported on Intel Xeon Platforms: > > -- 4th Generation Intel® Xeon® Scalable Processors. > -- 3rd Generation Intel® Xeon® Scalable Processors. > -- 2nd Generation Intel® Xeon® Scalable Processors. > +- 6th Generation Intel® Xeon® Scalable Processors. (NTB GEN5 device id: > 0x0DB4) > +- 5th Generation Intel® Xeon® Scalable Processors. (NTB GEN4 device id: > 0x347E) > +- 4th Generation Intel® Xeon® Scalable Processors. (NTB GEN4 device id: > 0x347E) > +- 3rd Generation Intel® Xeon® Scalable Processors. (NTB GEN4 device id: > 0x347E) > +- 2nd Generation Intel® Xeon® Scalable Processors. (NTB GEN3 device id: > 0x201C) Add release note update too as new platform is added?
RE: [PATCH v2] raw/ntb: add PPD status check for SPR
> -Original Message- > From: Guo, Junfeng > Sent: Thursday, June 30, 2022 4:56 PM > To: Wu, Jingjing > Cc: dev@dpdk.org; Guo, Junfeng > Subject: [PATCH v2] raw/ntb: add PPD status check for SPR > > Add PPD (PCIe Port Definition) status check for SPR (Sapphire Rapids). > > Note that NTB on SPR has the same device id with that on ICX, while > the field offsets of PPD Control Register are different. Here, we use > the PCI device revision id to distinguish the HW platform (ICX/SPR) > and check the Port Config Status and Port Definition accordingly. > > +---+++ > | Fields | Bit Range (on ICX) | Bit Range (on SPR) | > +---+++ > | Port Configuration Status | 12 | 14 | > | Port Definition | 9:8| 10:8 | > +---+++ > > v2: > fix revision id value check logic. > > Signed-off-by: Junfeng Guo Acked-by: Jingjing Wu
RE: [PATCH v3] raw/ntb: clear all valid DB bits when DB init
> -Original Message-
> From: Guo, Junfeng
> Sent: Wednesday, February 9, 2022 12:47 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; sta...@dpdk.org; Guo, Junfeng
> Subject: [PATCH v3] raw/ntb: clear all valid DB bits when DB init
>
> Before registering the doorbell interrupt handler callback function,
> all the valid doorbell bits within the NTB private data struct should
> be cleared to avoid the confusion of the handshake timing sequence
> diagram when setting up the NTB connection in back-to-back mode.
>
> Fixes: 62012a76811e ("raw/ntb: add handshake process")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Junfeng Guo
> ---
> drivers/raw/ntb/ntb.c | 2 ++
> 1 file changed, 2 insertions(+)
>
Better to add changes compared to previous version, which would help reviewers.
> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c
> index 46ac02e5ab..cc611dfbb9 100644
> --- a/drivers/raw/ntb/ntb.c
> +++ b/drivers/raw/ntb/ntb.c
> @@ -1398,6 +1398,8 @@ ntb_init_hw(struct rte_rawdev *dev, struct
> rte_pci_device
> *pci_dev)
>
> /* Init doorbell. */
> hw->db_valid_mask = RTE_LEN2MASK(hw->db_cnt, uint64_t);
> + /* Clear all valid doorbell bits before registering intr handler */
> + (*hw->ntb_ops->db_clear)(dev, hw->db_valid_mask);
Check if hw->ntb_ops->db_clear is NULL before call it.
>
> intr_handle = pci_dev->intr_handle;
> /* Register callback func to eal lib */
> --
> 2.25.1
RE: [PATCH] raw/ntb: add check for DB intr handler registering
> -Original Message-
> From: Guo, Junfeng
> Sent: Thursday, February 10, 2022 2:29 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; sta...@dpdk.org; Guo, Junfeng
> Subject: [PATCH] raw/ntb: add check for DB intr handler registering
>
> The callback registering of doorbell interrupt handler should be
> finished before enabling the interrupt event fd. Thus add the return
> value check for this callback registering.
>
> Fixes: 62012a76811e ("raw/ntb: add handshake process")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Junfeng Guo
> ---
> drivers/raw/ntb/ntb.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/raw/ntb/ntb.c b/drivers/raw/ntb/ntb.c
> index cc611dfbb9..0801e6d1ae 100644
> --- a/drivers/raw/ntb/ntb.c
> +++ b/drivers/raw/ntb/ntb.c
> @@ -1403,8 +1403,12 @@ ntb_init_hw(struct rte_rawdev *dev, struct
> rte_pci_device
> *pci_dev)
>
> intr_handle = pci_dev->intr_handle;
> /* Register callback func to eal lib */
> - rte_intr_callback_register(intr_handle,
> -ntb_dev_intr_handler, dev);
> + ret = rte_intr_callback_register(intr_handle,
> + ntb_dev_intr_handler, dev);
> + if (ret) {
> + NTB_LOG(ERR, "Unable to register doorbell intr handler.");
> + return ret;
> + }
When will this register failure happen? Have you checked what is the root cause?
>
> ret = rte_intr_efd_enable(intr_handle, hw->db_cnt);
> if (ret)
Need roll back, such as rte_intr_callback_unregister is required when fail or
driver remove?
> --
> 2.25.1
RE: [PATCH v4] raw/ntb: clear all valid DB bits when DB init
> -Original Message-
> From: Guo, Junfeng
> Sent: Thursday, February 10, 2022 3:07 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; sta...@dpdk.org; Guo, Junfeng
> Subject: [PATCH v4] raw/ntb: clear all valid DB bits when DB init
>
> Before registering the doorbell interrupt handler callback function,
> all the valid doorbell bits within the NTB private data struct should
> be cleared to avoid the confusion of the handshake timing sequence
> diagram when setting up the NTB connection in back-to-back mode.
>
> Fixes: 62012a76811e ("raw/ntb: add handshake process")
> Cc: sta...@dpdk.org
>
> v2: fix typo
> v3: fix coding style issue
> v4: add ops check before calling it
>
> Signed-off-by: Junfeng Guo
Acked-by: Jingjing Wu
Re: [dpdk-dev] [PATCH v8 6/7] net/iavf: add watchdog for VFLR
> -Original Message- > From: Nicolau, Radu > Sent: Friday, October 15, 2021 6:15 PM > To: Wu, Jingjing ; Xing, Beilei > Cc: dev@dpdk.org; Doherty, Declan ; Sinha, Abhijit > ; Zhang, Qi Z ; Richardson, > Bruce > ; Ananyev, Konstantin > ; > Nicolau, Radu > Subject: [PATCH v8 6/7] net/iavf: add watchdog for VFLR > > Add watchdog to iAVF PMD which support monitoring the VFLR register. If > the device is not already in reset then if a VF reset in progress is > detected then notfiy user through callback and set into reset state. > If the device is already in reset then poll for completion of reset. > > The watchdog is disabled by default, to enable it set > IAVF_DEV_WATCHDOG_PERIOD to a non zero value (microseconds) > > Signed-off-by: Declan Doherty > Signed-off-by: Radu Nicolau Acked-by: Jingjing Wu
Re: [dpdk-dev] [PATCH v2 2/4] net/iavf: add iAVF IPsec inline crypto support
In general, the patch is too big to review. Patch split would help a lot!
[...]
> +static const struct rte_cryptodev_symmetric_capability *
> +get_capability(struct iavf_security_ctx *iavf_sctx,
> + uint32_t algo, uint32_t type)
> +{
> + const struct rte_cryptodev_capabilities *capability;
> + int i = 0;
> +
> + capability = &iavf_sctx->crypto_capabilities[i];
> +
> + while (capability->op != RTE_CRYPTO_OP_TYPE_UNDEFINED) {
> + if (capability->op == RTE_CRYPTO_OP_TYPE_SYMMETRIC &&
> + capability->sym.xform_type == type &&
> + capability->sym.cipher.algo == algo)
> + return &capability->sym;
> + /** try next capability */
> + capability = &iavf_crypto_capabilities[i++];
Better to check i to avoid out of boundary.
[...]
> +
> +static int
> +valid_length(uint32_t len, uint32_t min, uint32_t max, uint32_t increment)
> +{
> + if (len < min || len > max)
> + return 0;
> +
> + if (increment == 0)
> + return 1;
> +
> + if ((len - min) % increment)
> + return 0;
> +
> + return 1;
> +}
Would it be better to use true/false instead of 1/0? And the same to following
valid functions.
[...]
> +static int
> +iavf_ipsec_crypto_session_validate_conf(struct iavf_security_ctx *iavf_sctx,
> + struct rte_security_session_conf *conf)
> +{
> + /** validate security action/protocol selection */
> + if (conf->action_type != RTE_SECURITY_ACTION_TYPE_INLINE_CRYPTO ||
> + conf->protocol != RTE_SECURITY_PROTOCOL_IPSEC) {
> + PMD_DRV_LOG(ERR, "Unsupported action / protocol specified");
> + return -EINVAL;
> + }
> +
> + /** validate IPsec protocol selection */
> + if (conf->ipsec.proto != RTE_SECURITY_IPSEC_SA_PROTO_ESP) {
> + PMD_DRV_LOG(ERR, "Unsupported IPsec protocol specified");
> + return -EINVAL;
> + }
> +
> + /** validate selected options */
> + if (conf->ipsec.options.copy_dscp ||
> + conf->ipsec.options.copy_flabel ||
> + conf->ipsec.options.copy_df ||
> + conf->ipsec.options.dec_ttl ||
> + conf->ipsec.options.ecn ||
> + conf->ipsec.options.stats) {
> + PMD_DRV_LOG(ERR, "Unsupported IPsec option specified");
> + return -EINVAL;
> + }
> +
> + /**
> + * Validate crypto xforms parameters.
> + *
> + * AEAD transforms can be used for either inbound/outbound IPsec SAs,
> + * for non-AEAD crypto transforms we explicitly only support CIPHER/AUTH
> + * for outbound and AUTH/CIPHER chained transforms for inbound IPsec.
> + */
> + if (conf->crypto_xform->type == RTE_CRYPTO_SYM_XFORM_AEAD) {
> + if (!valid_aead_xform(iavf_sctx, &conf->crypto_xform->aead)) {
> + PMD_DRV_LOG(ERR, "Unsupported IPsec option specified");
> + return -EINVAL;
> + }
Invalid parameter, but not unsupported option, right? Same to below.
[...]
> +static void
> +sa_add_set_aead_params(struct virtchnl_ipsec_crypto_cfg_item *cfg,
> + struct rte_crypto_aead_xform *aead, uint32_t salt)
> +{
> + cfg->crypto_type = VIRTCHNL_AEAD;
> +
> + switch (aead->algo) {
> + case RTE_CRYPTO_AEAD_AES_CCM:
> + cfg->algo_type = VIRTCHNL_AES_CCM; break;
> + case RTE_CRYPTO_AEAD_AES_GCM:
> + cfg->algo_type = VIRTCHNL_AES_GCM; break;
> + case RTE_CRYPTO_AEAD_CHACHA20_POLY1305:
> + cfg->algo_type = VIRTCHNL_CHACHA20_POLY1305; break;
> + default:
> + RTE_ASSERT("we should be here");
Assert just because invalid config? Similar comments to other valid functions.
> + }
> +
> + cfg->key_len = aead->key.length;
> + cfg->iv_len = aead->iv.length;
> + cfg->digest_len = aead->digest_length;
> + cfg->salt = salt;
> +
> + RTE_ASSERT(sizeof(cfg->key_data) < cfg->key_len);
> +
Not only data, but length, better to valid before setting? The same to other
kind params setting.
[...]
> +static inline void
> +iavf_build_data_desc_cmd_offset_fields(volatile uint64_t *qw1,
> + struct rte_mbuf *m)
> +{
> + uint64_t command = 0;
> + uint64_t offset = 0;
> + uint64_t l2tag1 = 0;
> +
> + *qw1 = IAVF_TX_DESC_DTYPE_DATA;
> +
> + command = (uint64_t)IAVF_TX_DESC_CMD_ICRC;
> +
> + /* Descriptor based VLAN insertion */
> + if (m->ol_flags & PKT_TX_VLAN_PKT) {
> + command |= (uint64_t)IAVF_TX_DESC_CMD_IL2TAG1;
> + l2tag1 |= m->vlan_tci;
> + }
> +
> /* Set MACLEN */
> - *td_offset |= (tx_offload.l2_len >> 1) <<
> - IAVF_TX_DESC_LENGTH_MACLEN_SHIFT;
> -
> - /* Enable L3 checksum offloads */
> - if (ol_flags & PKT_TX_IP_CKSUM) {
> - *td_cmd |= IAVF_TX_DESC_CMD_IIPT_IPV4_CSUM;
> - *td_offset |= (tx_offload.l3_len >> 2) <<
> -
Re: [dpdk-dev] Not able to start IAVF PMD with dpdk 20.11.3
Could you have a try to switch from uio_pci_generic to vfio_pci? From: Dey, Souvik Sent: Thursday, September 23, 2021 11:29 PM To: dev@dpdk.org; Xing, Beilei ; Wu, Jingjing Subject: Not able to start IAVF PMD with dpdk 20.11.3 Hi All, Trying to test E810 Sr-IOV based nic card with 20.11.3 dpdk. I am running the host kernel driver and only the VF is attached to the VM where I am running dpdk. But when I attaching the VF to the testpmd it is throwing the below error. Is there are way to move forward here ? [root@connexip linuxadmin]# ./dpdk-testpmd -c 0xf -n 4 -a 08:00.0 -- -i --rxq=16 --txq=16 EAL: Detected 4 lcore(s) EAL: Detected 1 NUMA nodes EAL: Detected static linkage of DPDK EAL: Multi-process socket /var/run/dpdk/rte/mp_socket EAL: Selected IOVA mode 'PA' EAL: No available hugepages reported in hugepages-1048576kB EAL: Probing VFIO support... EAL: Probe PCI driver: net_iavf (8086:1889) device: :08:00.0 (socket 0) EAL: Error reading from file descriptor 27: Input/output error EAL: No legacy callbacks, legacy socket not created Interactive-mode selected testpmd: create a new mbuf pool : n=171456, size=2176, socket=0 testpmd: preferred mempool ops selected: ring_mp_mc Warning! port-topology=paired and odd forward ports number, the last port will pair with itself. Configuring Port 0 (socket 0) EAL: Error reading from file descriptor 27: Input/output error iavf_execute_vf_cmd(): No response for cmd 28 iavf_disable_vlan_strip(): Failed to execute command of OP_DISABLE_VLAN_STRIPPING iavf_execute_vf_cmd(): No response for cmd 24 iavf_configure_rss_lut(): Failed to execute command of OP_CONFIG_RSS_LUT iavf_dev_configure(): configure rss failed Port0 dev_configure = -1 Fail to configure port 0 EAL: Error - exiting with code: 1 Cause: Start ports failed [root@connexip linuxadmin]# I do see the matching list for driver but is this required even if I use kernel driver on the host and only IAVF PMD on the guest ? DPDK Kernel Driver OS Default DDP COMMS DDP Wireless DDP Firmware 20.11 1.3.2 1.3.20 1.3.24 N/A 2.3 21.02 1.4.11 1.3.24 1.3.28 1.3.4 2.4 Host details: Red Hat Enterprise Linux release 8.2 (Ootpa) [root@localhost ~]# modinfo ice filename: /lib/modules/4.18.0-193.19.1.el8_2.x86_64/kernel/drivers/net/ethernet/intel/ice/ice.ko.xz firmware: intel/ice/ddp/ice.pkg (ice-1.3.4.0.pkg) version:0.8.1-k license:GPL v2 description:Intel(R) Ethernet Connection E800 Series Linux Driver author: Intel Corporation, linux.n...@intel.com<mailto:linux.n...@intel.com> rhelversion:8.2 Using vfio-pci to connect the VF to the VM. Guest Details: OS : debian buster Dpdk : 20.11.3 Interface bound to uio_pci_generic [root@connexip linuxadmin]# /opt/sonus/bin/np/swe/dpdk-devbind.py --status Network devices using DPDK-compatible driver :01:00.0 'Virtio network device 1041' drv=uio_pci_generic unused=virtio_pci :08:00.0 'Ethernet Adaptive Virtual Function 1889' drv=uio_pci_generic unused=i40evf :09:00.0 'Ethernet Adaptive Virtual Function 1889' drv=uio_pci_generic unused=i40evf Network devices using kernel driver === :02:00.0 'Virtio network device 1041' if=ha0 drv=virtio-pci unused=virtio_pci,uio_pci_generic *Active* Notice: This e-mail together with any attachments may contain information of Ribbon Communications Inc. and its Affiliates that is confidential and/or proprietary for the sole use of the intended recipient. Any review, disclosure, reliance or distribution by others or forwarding without express permission is strictly prohibited. If you are not the intended recipient, please notify the sender immediately and then delete all copies, including any attachments.
Re: [dpdk-dev] [PATCH] net/iavf: remove interrupt handler
> > -Original Message-
> > From: Zhang, RobinX
> > Sent: Friday, July 23, 2021 3:47 PM
> > To: dev@dpdk.org
> > Cc: Wu, Jingjing ; Xing, Beilei
> > ; Zhang, Qi Z ; Guo,
> > Junfeng ; Yang, SteveX
> ;
> > Zhang, RobinX
> > Subject: [PATCH] net/iavf: remove interrupt handler
>
> As you are not going to remove interrupt handler for all the cases, the title
> is
> misleading Better replace it with "enable interrupt polling"
>
> >
> > For VF hosted by Intel 700 series NICs, internal rx interrupt and
> > adminq interrupt share the same source, that cause a lot cpu cycles be
> > wasted on interrupt handler on rx path.
> >
> > The patch disable pci interrupt and remove the interrupt handler,
> > replace it with a low frequency(50ms) interrupt polling daemon which
> > is implemtented by registering an alarm callback periodly.
> >
> > The virtual channel capability bit VIRTCHNL_VF_OFFLOAD_WB_ON_ITR can
> > be used to negotiate if iavf PMD needs to enable background alarm or
> > not, so ideally this change will not impact the case hosted by Intel 800
> > series
> NICS.
> >
> > Suggested-by: Jingjing Wu
> > Signed-off-by: Qi Zhang
>
As it is a kind of problem solving, can it be sent to dpdk-stable too?
> No need to add me as the author for this patch but, you can add a reference
> to the original i40e commit to explain you implement the same logic.
>
> > Signed-off-by: Robin Zhang
> > ---
> > drivers/net/iavf/iavf.h| 3 +++
> > drivers/net/iavf/iavf_ethdev.c | 37
> > ++
> > drivers/net/iavf/iavf_vchnl.c | 11 --
> > 3 files changed, 22 insertions(+), 29 deletions(-)
> >
> > diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h index
> > b3bd078111..771f3b79d7 100644
> > --- a/drivers/net/iavf/iavf.h
> > +++ b/drivers/net/iavf/iavf.h
> > @@ -69,6 +69,8 @@
> > #define IAVF_QUEUE_ITR_INTERVAL_DEFAULT 32 /* 32 us */
> > #define IAVF_QUEUE_ITR_INTERVAL_MAX 8160 /* 8160 us */
> >
> > +#define IAVF_ALARM_INTERVAL 5 /* us */
> > +
> > /* The overhead from MTU to max frame size.
> > * Considering QinQ packet, the VLAN tag needs to be counted twice.
> > */
> > @@ -372,6 +374,7 @@ int iavf_config_irq_map_lv(struct iavf_adapter
> > *adapter, uint16_t num, void iavf_add_del_all_mac_addr(struct
> > iavf_adapter *adapter, bool add); int iavf_dev_link_update(struct
> > rte_eth_dev *dev, __rte_unused int wait_to_complete);
> > +void iavf_dev_alarm_handler(void *param);
> > int iavf_query_stats(struct iavf_adapter *adapter,
> > struct virtchnl_eth_stats **pstats); int
> > iavf_config_promisc(struct iavf_adapter *adapter, bool enable_unicast,
> > diff --git a/drivers/net/iavf/iavf_ethdev.c
> > b/drivers/net/iavf/iavf_ethdev.c index
> > 41382c6d66..bbe5b3ddb1 100644
> > --- a/drivers/net/iavf/iavf_ethdev.c
> > +++ b/drivers/net/iavf/iavf_ethdev.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -692,9 +693,9 @@ static int iavf_config_rx_queues_irqs(struct
> > rte_eth_dev *dev,
> > */
> > vf->msix_base = IAVF_MISC_VEC_ID;
> >
> > -/* set ITR to max */
> > +/* set ITR to default */
> > interval = iavf_calc_itr_interval(
> > -IAVF_QUEUE_ITR_INTERVAL_MAX);
> > +IAVF_QUEUE_ITR_INTERVAL_DEFAULT);
> > IAVF_WRITE_REG(hw, IAVF_VFINT_DYN_CTL01,
> > IAVF_VFINT_DYN_CTL01_INTENA_MASK |
> > (IAVF_ITR_INDEX_DEFAULT <<
> > @@ -853,9 +854,8 @@ iavf_dev_start(struct rte_eth_dev *dev)
> > PMD_DRV_LOG(ERR, "configure irq failed"); goto err_queue; }
> > -/* re-enable intr again, because efd assign may change */
> > +/* only enable interrupt in rx interrupt mode */
> > if (dev->data->dev_conf.intr_conf.rxq != 0) {
> > -rte_intr_disable(intr_handle); rte_intr_enable(intr_handle); }
> >
> > @@ -889,6 +889,9 @@ iavf_dev_stop(struct rte_eth_dev *dev)
> >
> > PMD_INIT_FUNC_TRACE();
> >
> > +if (dev->data->dev_conf.intr_conf.rxq != 0)
> > +rte_intr_disable(intr_handle);
> > +
> > if (adapter->stopped == 1)
> > return 0;
> >
> > @@ -1669,8 +1672,6 @@ iavf_dev_rx_queue_intr_enable(struct
> rte_eth_dev
> > *dev, uint16_t queue_id)
> >
> > IAVF_WRITE_FLUSH(hw);
> >
> > -rte_intr_ack(&pci_dev->intr_handle);
> > -
> > return 0;
> > }
RE: [PATCH v6 0/6] add idpf pmd enhancement features
> -Original Message- > From: Liu, Mingxia > Sent: Tuesday, February 7, 2023 6:17 PM > To: dev@dpdk.org; Zhang, Qi Z ; Wu, Jingjing > ; Xing, Beilei > Cc: Liu, Mingxia > Subject: [PATCH v6 0/6] add idpf pmd enhancement features > > This patchset add several enhancement features of idpf pmd. > Including the following: > - add hw statistics, support stats/xstats ops > - add rss configure/show ops > - add event handle: link status > - add scattered data path for single queue > > This patchset is based on the refactor idpf PMD code: > http://patches.dpdk.org/project/dpdk/patch/20230207084549.2225214-2- > wenjun1...@intel.com/ > > v2 changes: > - Fix rss lut config issue. > v3 changes: > - rebase to the new baseline. > v4 changes: > - rebase to the new baseline. > - optimize some code > - give "not supported" tips when user want to config rss hash type > - if stats reset fails at initialization time, don't rollback, just >print ERROR info. > v5 changes: > - fix some spelling error > v6 changes: > - add cover-letter > Reviewed-by: Jingjing Wu
RE: [PATCH] common/idpf: fix Rx queue configuration
> -Original Message-
> From: Xing, Beilei
> Sent: Thursday, February 23, 2023 11:17 AM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Xing, Beilei ; sta...@dpdk.org
> Subject: [PATCH] common/idpf: fix Rx queue configuration
>
> From: Beilei Xing
>
> IDPF PMD enables 2 buffer queues by default. According to the data
> sheet, if there is a second buffer queue, the second buffer queue
> is valid only if bugq2_ena is set.
>
> Fixes: c2494d783d31 ("net/idpf: support queue start")
> Fixes: 8b95ced47a13 ("common/idpf: add Rx/Tx queue structs")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Beilei Xing
Acked-by: Jingjing Wu
RE: [PATCH v3 02/10] net/cpfl: support hairpin queue capbility get
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, May 19, 2023 3:31 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei
> ; Wang, Xiao W
> Subject: [PATCH v3 02/10] net/cpfl: support hairpin queue capbility get
>
> From: Beilei Xing
>
> This patch adds hairpin_cap_get ops support.
>
> Signed-off-by: Xiao Wang
> Signed-off-by: Mingxia Liu
> Signed-off-by: Beilei Xing
> ---
> drivers/net/cpfl/cpfl_ethdev.c | 13 +
> drivers/net/cpfl/cpfl_rxtx.h | 4
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index e587155db6..b6fd0b05d0 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -154,6 +154,18 @@ cpfl_dev_link_update(struct rte_eth_dev *dev,
> return rte_eth_linkstatus_set(dev, &new_link);
> }
>
> +static int
> +cpfl_hairpin_cap_get(__rte_unused struct rte_eth_dev *dev,
> + struct rte_eth_hairpin_cap *cap)
> +{
> + cap->max_nb_queues = CPFL_MAX_P2P_NB_QUEUES;
> + cap->max_rx_2_tx = CPFL_MAX_HAIRPINQ_RX_2_TX;
> + cap->max_tx_2_rx = CPFL_MAX_HAIRPINQ_TX_2_RX;
> + cap->max_nb_desc = CPFL_MAX_HAIRPINQ_NB_DESC;
> +
Is that better to check if p2p queue group is added successfully and then
return success?
RE: [PATCH v3 04/10] net/cpfl: add haipin queue group during vport init
> static int
> cpfl_dev_vport_init(struct rte_eth_dev *dev, void *init_params)
> {
> @@ -1306,6 +1414,8 @@ cpfl_dev_vport_init(struct rte_eth_dev *dev, void
> *init_params)
> struct cpfl_adapter_ext *adapter = param->adapter;
> /* for sending create vport virtchnl msg prepare */
> struct virtchnl2_create_vport create_vport_info;
> + struct virtchnl2_add_queue_groups p2p_queue_grps_info;
> + uint8_t p2p_q_vc_out_info[IDPF_DFLT_MBX_BUF_SIZE] = {0};
> int ret = 0;
>
> dev->dev_ops = &cpfl_eth_dev_ops;
> @@ -1340,8 +1450,28 @@ cpfl_dev_vport_init(struct rte_eth_dev *dev, void
> *init_params)
> rte_ether_addr_copy((struct rte_ether_addr *)vport->default_mac_addr,
> &dev->data->mac_addrs[0]);
>
> + if (!adapter->base.is_rx_singleq && !adapter->base.is_tx_singleq) {
> + memset(&p2p_queue_grps_info, 0, sizeof(p2p_queue_grps_info));
> + ret = cpfl_p2p_q_grps_add(vport, &p2p_queue_grps_info,
> p2p_q_vc_out_info);
> + if (ret != 0) {
> + PMD_INIT_LOG(ERR, "Failed to add p2p queue group.");
> + goto err_q_grps_add;
> + }
> + ret = cpfl_p2p_queue_info_init(cpfl_vport,
> +(struct virtchnl2_add_queue_groups
> *)p2p_q_vc_out_info);
> + if (ret != 0) {
> + PMD_INIT_LOG(ERR, "Failed to init p2p queue info.");
> + goto err_p2p_qinfo_init;
If it is failed to add p2p queue group, the device init will quit?
I think it should be better to continue initialization just without p2p
capability.
> + }
> + }
> +
> return 0;
>
RE: [PATCH v3 05/10] net/cpfl: support hairpin queue setup and release
>
> +static int
> +cpfl_rx_hairpin_bufq_setup(struct rte_eth_dev *dev, struct idpf_rx_queue
> *bufq,
> +uint16_t logic_qid, uint16_t nb_desc)
> +{
> + struct cpfl_vport *cpfl_vport =
> + (struct cpfl_vport *)dev->data->dev_private;
> + struct idpf_vport *vport = &cpfl_vport->base;
> + struct idpf_adapter *adapter = vport->adapter;
> + struct rte_mempool *mp;
> + char pool_name[RTE_MEMPOOL_NAMESIZE];
> +
> + mp = cpfl_vport->p2p_mp;
> + if (!mp) {
> + snprintf(pool_name, RTE_MEMPOOL_NAMESIZE, "p2p_mb_pool_%u",
> + dev->data->port_id);
> + mp = rte_pktmbuf_pool_create(pool_name, CPFL_P2P_NB_MBUF,
> CPFL_P2P_CACHE_SIZE,
> + 0, CPFL_P2P_MBUF_SIZE, dev->device-
> >numa_node);
> + if (!mp) {
> + PMD_INIT_LOG(ERR, "Failed to allocate mbuf pool for
> p2p");
> + return -ENOMEM;
> + }
> + cpfl_vport->p2p_mp = mp;
> + }
> +
> + bufq->mp = mp;
> + bufq->nb_rx_desc = nb_desc;
> + bufq->queue_id = cpfl_hw_qid_get(cpfl_vport-
> >p2p_q_chunks_info.rx_buf_start_qid, logic_qid);
> + bufq->port_id = dev->data->port_id;
> + bufq->adapter = adapter;
> + bufq->rx_buf_len = CPFL_P2P_MBUF_SIZE - RTE_PKTMBUF_HEADROOM;
> +
> + bufq->sw_ring = rte_zmalloc("sw ring",
> + sizeof(struct rte_mbuf *) * nb_desc,
> + RTE_CACHE_LINE_SIZE);
Is sw_ring required in p2p case? It has been never used right?
Please also check the sw_ring in tx queue.
> + if (!bufq->sw_ring) {
> + PMD_INIT_LOG(ERR, "Failed to allocate memory for SW ring");
> + return -ENOMEM;
> + }
> +
> + bufq->q_set = true;
> + bufq->ops = &def_rxq_ops;
> +
> + return 0;
> +}
> +
> +int
> +cpfl_rx_hairpin_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
> + uint16_t nb_desc,
> + const struct rte_eth_hairpin_conf *conf)
> +{
> + struct cpfl_vport *cpfl_vport = (struct cpfl_vport
> *)dev->data->dev_private;
> + struct idpf_vport *vport = &cpfl_vport->base;
> + struct idpf_adapter *adapter_base = vport->adapter;
> + uint16_t logic_qid = cpfl_vport->nb_p2p_rxq;
> + struct cpfl_rxq_hairpin_info *hairpin_info;
> + struct cpfl_rx_queue *cpfl_rxq;
> + struct idpf_rx_queue *bufq1 = NULL;
> + struct idpf_rx_queue *rxq;
> + uint16_t peer_port, peer_q;
> + uint16_t qid;
> + int ret;
> +
> + if (vport->rxq_model == VIRTCHNL2_QUEUE_MODEL_SINGLE) {
> + PMD_INIT_LOG(ERR, "Only spilt queue model supports hairpin
> queue.");
> + return -EINVAL;
> + }
> +
> + if (conf->peer_count != 1) {
> + PMD_INIT_LOG(ERR, "Can't support Rx hairpin queue peer count
> %d",
> conf->peer_count);
> + return -EINVAL;
> + }
> +
> + peer_port = conf->peers[0].port;
> + peer_q = conf->peers[0].queue;
> +
> + if (nb_desc % CPFL_ALIGN_RING_DESC != 0 ||
> + nb_desc > CPFL_MAX_RING_DESC ||
> + nb_desc < CPFL_MIN_RING_DESC) {
> + PMD_INIT_LOG(ERR, "Number (%u) of receive descriptors is
> invalid",
> nb_desc);
> + return -EINVAL;
> + }
> +
> + /* Free memory if needed */
> + if (dev->data->rx_queues[queue_idx]) {
> + cpfl_rx_queue_release(dev->data->rx_queues[queue_idx]);
> + dev->data->rx_queues[queue_idx] = NULL;
> + }
> +
> + /* Setup Rx description queue */
> + cpfl_rxq = rte_zmalloc_socket("cpfl hairpin rxq",
> + sizeof(struct cpfl_rx_queue),
> + RTE_CACHE_LINE_SIZE,
> + SOCKET_ID_ANY);
> + if (!cpfl_rxq) {
> + PMD_INIT_LOG(ERR, "Failed to allocate memory for rx queue data
> structure");
> + return -ENOMEM;
> + }
> +
> + rxq = &cpfl_rxq->base;
> + hairpin_info = &cpfl_rxq->hairpin_info;
> + rxq->nb_rx_desc = nb_desc * 2;
> + rxq->queue_id =
> cpfl_hw_qid_get(cpfl_vport->p2p_q_chunks_info.rx_start_qid,
> logic_qid);
> + rxq->port_id = dev->data->port_id;
> + rxq->adapter = adapter_base;
> + rxq->rx_buf_len = CPFL_P2P_MBUF_SIZE - RTE_PKTMBUF_HEADROOM;
> + hairpin_info->hairpin_q = true;
> + hairpin_info->peer_txp = peer_port;
> + hairpin_info->peer_txq_id = peer_q;
> +
> + if (conf->manual_bind != 0)
> + cpfl_vport->p2p_manual_bind = true;
> + else
> + cpfl_vport->p2p_manual_bind = false;
> +
> + /* setup 1 Rx buffer queue for the 1st hairpin rxq */
> + if (logic_qid == 0) {
> + bufq1 = rte_zmalloc_socket("hairpin rx bufq1",
> +sizeof(struct idpf_rx_queue),
> +RTE_CACHE_LINE_SIZE,
> +
RE: [PATCH v3 07/10] net/cpfl: support hairpin queue start/stop
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, May 19, 2023 3:31 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei
> ; Wang, Xiao W
> Subject: [PATCH v3 07/10] net/cpfl: support hairpin queue start/stop
>
> From: Beilei Xing
>
> This patch supports Rx/Tx hairpin queue start/stop.
>
> Signed-off-by: Xiao Wang
> Signed-off-by: Mingxia Liu
> Signed-off-by: Beilei Xing
> ---
> drivers/common/idpf/idpf_common_virtchnl.c | 2 +-
> drivers/common/idpf/idpf_common_virtchnl.h | 3 +
> drivers/common/idpf/version.map| 1 +
> drivers/net/cpfl/cpfl_ethdev.c | 41 ++
> drivers/net/cpfl/cpfl_rxtx.c | 153 ++---
> drivers/net/cpfl/cpfl_rxtx.h | 14 ++
> 6 files changed, 195 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/common/idpf/idpf_common_virtchnl.c
> b/drivers/common/idpf/idpf_common_virtchnl.c
> index 211b44a88e..6455f640da 100644
> --- a/drivers/common/idpf/idpf_common_virtchnl.c
> +++ b/drivers/common/idpf/idpf_common_virtchnl.c
> @@ -733,7 +733,7 @@ idpf_vc_vectors_dealloc(struct idpf_vport *vport)
> return err;
> }
>
> -static int
> +int
> idpf_vc_ena_dis_one_queue(struct idpf_vport *vport, uint16_t qid,
> uint32_t type, bool on)
> {
> diff --git a/drivers/common/idpf/idpf_common_virtchnl.h
> b/drivers/common/idpf/idpf_common_virtchnl.h
> index db83761a5e..9ff5c38c26 100644
> --- a/drivers/common/idpf/idpf_common_virtchnl.h
> +++ b/drivers/common/idpf/idpf_common_virtchnl.h
> @@ -71,6 +71,9 @@ __rte_internal
> int idpf_vc_txq_config_by_info(struct idpf_vport *vport, struct
> virtchnl2_txq_info
> *txq_info,
> uint16_t num_qs);
> __rte_internal
> +int idpf_vc_ena_dis_one_queue(struct idpf_vport *vport, uint16_t qid,
> + uint32_t type, bool on);
> +__rte_internal
> int idpf_vc_queue_grps_del(struct idpf_vport *vport,
> uint16_t num_q_grps,
> struct virtchnl2_queue_group_id *qg_ids);
> diff --git a/drivers/common/idpf/version.map b/drivers/common/idpf/version.map
> index 17e77884ce..25624732b0 100644
> --- a/drivers/common/idpf/version.map
> +++ b/drivers/common/idpf/version.map
> @@ -40,6 +40,7 @@ INTERNAL {
> idpf_vc_cmd_execute;
> idpf_vc_ctlq_post_rx_buffs;
> idpf_vc_ctlq_recv;
> + idpf_vc_ena_dis_one_queue;
> idpf_vc_irq_map_unmap_config;
> idpf_vc_one_msg_read;
> idpf_vc_ptype_info_query;
This change is in common, better to split this patch to two.
RE: [PATCH v3 08/10] net/cpfl: enable write back based on ITR expire
> idpf_vport_irq_unmap_config(struct idpf_vport *vport, uint16_t nb_rx_queues)
> {
> diff --git a/drivers/common/idpf/idpf_common_device.h
> b/drivers/common/idpf/idpf_common_device.h
> index 112367dae8..f767ea7cec 100644
> --- a/drivers/common/idpf/idpf_common_device.h
> +++ b/drivers/common/idpf/idpf_common_device.h
> @@ -200,5 +200,9 @@ int idpf_vport_info_init(struct idpf_vport *vport,
>struct virtchnl2_create_vport *vport_info);
> __rte_internal
> void idpf_vport_stats_update(struct virtchnl2_vport_stats *oes, struct
> virtchnl2_vport_stats *nes);
> +__rte_internal
> +int idpf_vport_irq_map_config_by_qids(struct idpf_vport *vport,
> + uint32_t *qids,
> + uint16_t nb_rx_queues);
>
> #endif /* _IDPF_COMMON_DEVICE_H_ */
> diff --git a/drivers/common/idpf/version.map b/drivers/common/idpf/version.map
> index 25624732b0..0729f6b912 100644
> --- a/drivers/common/idpf/version.map
> +++ b/drivers/common/idpf/version.map
> @@ -69,6 +69,7 @@ INTERNAL {
> idpf_vport_info_init;
> idpf_vport_init;
> idpf_vport_irq_map_config;
> + idpf_vport_irq_map_config_by_qids;
> idpf_vport_irq_unmap_config;
> idpf_vport_rss_config;
> idpf_vport_stats_update;
The same, split common change with net one?
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index c2ab0690fc..3b480178c0 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -730,11 +730,22 @@ cpfl_dev_configure(struct rte_eth_dev *dev)
> static int
> cpfl_config_rx_queues_irqs(struct rte_eth_dev *dev)
> {
> + uint32_t qids[CPFL_MAX_P2P_NB_QUEUES + IDPF_DEFAULT_RXQ_NUM] = {0};
> struct cpfl_vport *cpfl_vport = dev->data->dev_private;
> struct idpf_vport *vport = &cpfl_vport->base;
> uint16_t nb_rx_queues = dev->data->nb_rx_queues;
> + struct cpfl_rx_queue *cpfl_rxq;
> + int i;
>
> - return idpf_vport_irq_map_config(vport, nb_rx_queues);
> + for (i = 0; i < nb_rx_queues; i++) {
> + cpfl_rxq = dev->data->rx_queues[i];
> + if (cpfl_rxq->hairpin_info.hairpin_q)
> + qids[i] = cpfl_hw_qid_get(cpfl_vport-
> >p2p_q_chunks_info.rx_start_qid,
> + (i -
> cpfl_vport->nb_data_rxq));
> + else
> + qids[i] =
> cpfl_hw_qid_get(vport->chunks_info.rx_start_qid, i);
Looks like cpfl_hw_qid_get and is used cross files, how about defined it as
inline or Macro in header file?
> + }
> + return idpf_vport_irq_map_config_by_qids(vport, qids, nb_rx_queues);
> }
>
> /* Update hairpin_info for dev's tx hairpin queue */
> --
> 2.26.2
RE: [PATCH v3 09/10] net/cpfl: support peer ports get
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, May 19, 2023 3:31 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei
> ; Wang, Xiao W
> Subject: [PATCH v3 09/10] net/cpfl: support peer ports get
>
> From: Beilei Xing
>
> This patch supports get hairpin peer ports.
>
> Signed-off-by: Xiao Wang
> Signed-off-by: Beilei Xing
> ---
> drivers/net/cpfl/cpfl_ethdev.c | 34 ++
> 1 file changed, 34 insertions(+)
>
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index 3b480178c0..59c7e75d2a 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -1069,6 +1069,39 @@ cpfl_dev_close(struct rte_eth_dev *dev)
> return 0;
> }
>
> +static int
> +cpfl_hairpin_get_peer_ports(struct rte_eth_dev *dev, uint16_t *peer_ports,
> + __rte_unused size_t len, uint32_t tx)
> +{
Param len is used to identify the size of the peer ports array.
You should use it, and check if peer_ports is null. Otherwise will cause
invalid access.
* array length
> + struct cpfl_vport *cpfl_vport =
> + (struct cpfl_vport *)dev->data->dev_private;
> + struct idpf_tx_queue *txq;
> + struct idpf_rx_queue *rxq;
> + struct cpfl_tx_queue *cpfl_txq;
> + struct cpfl_rx_queue *cpfl_rxq;
> + int i, j;
> +
> + if (tx > 0) {
> + for (i = cpfl_vport->nb_data_txq, j = 0; i <
> dev->data->nb_tx_queues; i++,
> j++) {
> + txq = dev->data->tx_queues[i];
> + if (txq == NULL)
> + return -EINVAL;
> + cpfl_txq = (struct cpfl_tx_queue *)txq;
> + peer_ports[j] = cpfl_txq->hairpin_info.peer_rxp;
> + }
> + } else if (tx == 0) {
> + for (i = cpfl_vport->nb_data_rxq, j = 0; i <
> dev->data->nb_rx_queues; i++,
> j++) {
> + rxq = dev->data->rx_queues[i];
> + if (rxq == NULL)
> + return -EINVAL;
> + cpfl_rxq = (struct cpfl_rx_queue *)rxq;
> + peer_ports[j] = cpfl_rxq->hairpin_info.peer_txp;
> + }
> + }
> +
> + return j;
> +}
> +
> static const struct eth_dev_ops cpfl_eth_dev_ops = {
> .dev_configure = cpfl_dev_configure,
> .dev_close = cpfl_dev_close,
> @@ -1098,6 +1131,7 @@ static const struct eth_dev_ops cpfl_eth_dev_ops = {
> .hairpin_cap_get= cpfl_hairpin_cap_get,
> .rx_hairpin_queue_setup = cpfl_rx_hairpin_queue_setup,
> .tx_hairpin_queue_setup = cpfl_tx_hairpin_queue_setup,
> + .hairpin_get_peer_ports = cpfl_hairpin_get_peer_ports,
> };
>
> static int
> --
> 2.26.2
RE: [PATCH v2] net/i40e: fix FDIR Rxq receives broadcast packets
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, July 21, 2023 1:35 PM
> To: Wu, Jingjing ; Zhang, Yuying
>
> Cc: dev@dpdk.org; Xing, Beilei ; sta...@dpdk.org
> Subject: [PATCH v2] net/i40e: fix FDIR Rxq receives broadcast packets
>
> From: Beilei Xing
>
> FDIR Rxq is excepted to only receive FDIR programming status, won't
> receive broadcast packets.
>
> Fixes: a778a1fa2e4e ("i40e: set up and initialize flow director")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Beilei Xing
Acked-by: Jingjing Wu
RE: [PATCH] graph: fix head move when graph walk in mcore dispatch
> > /* skip the src nodes which not bind with current worker */ > > if ((int32_t)head < 0 && node->dispatch.lcore_id != graph- > > >dispatch.lcore_id) > > continue; > > - > > + head++; > If current src node not bind with current core, It will go into infinite loop. > This line would have no chance to run. Seems reasonable, it might be OK to change "head<0" to "head <1" the condition check?
RE: [PATCH] graph: fix head move when graph walk in mcore dispatch
> -Original Message- > From: Yan, Zhirun > Sent: Wednesday, March 20, 2024 4:43 PM > To: Wu, Jingjing ; dev@dpdk.org > Cc: jer...@marvell.com; pbhagavat...@marvell.com; sta...@dpdk.org > Subject: RE: [PATCH] graph: fix head move when graph walk in mcore dispatch > > > > > -Original Message- > > From: Wu, Jingjing > > Sent: Wednesday, March 20, 2024 2:25 PM > > To: Yan, Zhirun ; dev@dpdk.org > > Cc: jer...@marvell.com; pbhagavat...@marvell.com; sta...@dpdk.org > > Subject: RE: [PATCH] graph: fix head move when graph walk in mcore > dispatch > > > > > > > > /* skip the src nodes which not bind with current > > > > worker */ > > > > if ((int32_t)head < 0 && node->dispatch.lcore_id != > > > > graph- > > > > >dispatch.lcore_id) > > > > continue; > > > > - > > > > + head++; > > > If current src node not bind with current core, It will go into infinite > > > loop. > > > This line would have no chance to run. > > > > Seems reasonable, it might be OK to change "head<0" to "head <1" the > condition > > check? > > No. "head<0" means it is src node. > All src node would put before head = 0. "Head<1" is confused. > You could find the details of graph reel under rte_graph_walk_rtc() in > lib/graph/rte_graph_model_rtc.h > > I guess if there are some src node missed, it may be caused by wrong config, > for example, the missed src node not pin to a lcore. > Use rte_graph_model_mcore_dispatch_node_lcore_affinity_set() to pin the > src node first. I don't think it is confusing because head++ happens before head < 0 check. Yes, it happens when lcore affinity is not set. For example, we have two source nodes, both of them have no lcore affinity setting. By current code, the second node will also be executed which is not as expected. Thanks Jingjing
RE: [PATCH v4 1/2] common/idpf: move PF specific functions from common init
> -Original Message-
> From: Xing, Beilei
> Sent: Thursday, April 6, 2023 3:43 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Xing, Beilei
> Subject: [PATCH v4 1/2] common/idpf: move PF specific functions from common
> init
>
> From: Beilei Xing
>
> Move PF reset and PF mailbox initialization functions from
> idpf_adapter_init function to _adapter_ext_init function,
> since they're different between PF and VF support.
>
> Signed-off-by: Beilei Xing
> ---
> drivers/common/idpf/idpf_common_device.c | 72 +---
> drivers/common/idpf/idpf_common_device.h | 11
> drivers/common/idpf/version.map | 5 ++
> drivers/net/cpfl/cpfl_ethdev.c | 51 +
> drivers/net/idpf/idpf_ethdev.c | 51 +
> 5 files changed, 131 insertions(+), 59 deletions(-)
>
> diff --git a/drivers/common/idpf/idpf_common_device.c
> b/drivers/common/idpf/idpf_common_device.c
> index c5e7bbf66c..3b58bdd41e 100644
> --- a/drivers/common/idpf/idpf_common_device.c
> +++ b/drivers/common/idpf/idpf_common_device.c
> @@ -6,8 +6,8 @@
> #include
> #include
>
> -static void
> -idpf_reset_pf(struct idpf_hw *hw)
> +void
> +idpf_hw_pf_reset(struct idpf_hw *hw)
> {
> uint32_t reg;
>
> @@ -15,9 +15,8 @@ idpf_reset_pf(struct idpf_hw *hw)
> IDPF_WRITE_REG(hw, PFGEN_CTRL, (reg | PFGEN_CTRL_PFSWR));
> }
>
> -#define IDPF_RESET_WAIT_CNT 100
> -static int
> -idpf_check_pf_reset_done(struct idpf_hw *hw)
> +int
> +idpf_hw_pf_reset_check(struct idpf_hw *hw)
> {
> uint32_t reg;
> int i;
> @@ -33,48 +32,13 @@ idpf_check_pf_reset_done(struct idpf_hw *hw)
> return -EBUSY;
> }
>
> -#define CTLQ_NUM 2
> -static int
> -idpf_init_mbx(struct idpf_hw *hw)
> +int
> +idpf_hw_mbx_init(struct idpf_hw *hw, struct idpf_ctlq_create_info *ctlq_info)
> {
> - struct idpf_ctlq_create_info ctlq_info[CTLQ_NUM] = {
> - {
> - .type = IDPF_CTLQ_TYPE_MAILBOX_TX,
> - .id = IDPF_CTLQ_ID,
> - .len = IDPF_CTLQ_LEN,
> - .buf_size = IDPF_DFLT_MBX_BUF_SIZE,
> - .reg = {
> - .head = PF_FW_ATQH,
> - .tail = PF_FW_ATQT,
> - .len = PF_FW_ATQLEN,
> - .bah = PF_FW_ATQBAH,
> - .bal = PF_FW_ATQBAL,
> - .len_mask = PF_FW_ATQLEN_ATQLEN_M,
> - .len_ena_mask = PF_FW_ATQLEN_ATQENABLE_M,
> - .head_mask = PF_FW_ATQH_ATQH_M,
> - }
> - },
> - {
> - .type = IDPF_CTLQ_TYPE_MAILBOX_RX,
> - .id = IDPF_CTLQ_ID,
> - .len = IDPF_CTLQ_LEN,
> - .buf_size = IDPF_DFLT_MBX_BUF_SIZE,
> - .reg = {
> - .head = PF_FW_ARQH,
> - .tail = PF_FW_ARQT,
> - .len = PF_FW_ARQLEN,
> - .bah = PF_FW_ARQBAH,
> - .bal = PF_FW_ARQBAL,
> - .len_mask = PF_FW_ARQLEN_ARQLEN_M,
> - .len_ena_mask = PF_FW_ARQLEN_ARQENABLE_M,
> - .head_mask = PF_FW_ARQH_ARQH_M,
> - }
> - }
> - };
> struct idpf_ctlq_info *ctlq;
> int ret;
>
> - ret = idpf_ctlq_init(hw, CTLQ_NUM, ctlq_info);
> + ret = idpf_ctlq_init(hw, IDPF_CTLQ_NUM, ctlq_info);
> if (ret != 0)
> return ret;
>
> @@ -96,6 +60,12 @@ idpf_init_mbx(struct idpf_hw *hw)
> return ret;
> }
You can check the device id if idpf_hw, and then decide how to init ctlq and
then this function can also be consumed by VF driver. No need to move them out
from common. The same as other functions.
>
RE: [PATCH] net/idpf: refine devargs parse functions
> -Original Message-
> From: Liu, Mingxia
> Sent: Friday, April 21, 2023 3:15 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing ; Xing, Beilei
> ; Liu, Mingxia
>
> Subject: [PATCH] net/idpf: refine devargs parse functions
>
> This patch refines devargs parsing functions and use valid
> variable max_vport_nb to replace IDPF_MAX_VPORT_NUM.
>
> Signed-off-by: Mingxia Liu
> ---
> drivers/net/idpf/idpf_ethdev.c | 61 +-
> 1 file changed, 30 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/idpf/idpf_ethdev.c b/drivers/net/idpf/idpf_ethdev.c
> index e02ec2ec5a..a8dd5a0a80 100644
> --- a/drivers/net/idpf/idpf_ethdev.c
> +++ b/drivers/net/idpf/idpf_ethdev.c
> @@ -857,12 +857,6 @@ insert_value(struct idpf_devargs *devargs, uint16_t id)
> return 0;
> }
>
> - if (devargs->req_vport_nb >= RTE_DIM(devargs->req_vports)) {
> - PMD_INIT_LOG(ERR, "Total vport number can't be > %d",
> - IDPF_MAX_VPORT_NUM);
> - return -EINVAL;
> - }
> -
> devargs->req_vports[devargs->req_vport_nb] = id;
> devargs->req_vport_nb++;
>
> @@ -879,12 +873,10 @@ parse_range(const char *value, struct idpf_devargs
> *devargs)
>
> result = sscanf(value, "%hu%n-%hu%n", &lo, &n, &hi, &n);
> if (result == 1) {
> - if (lo >= IDPF_MAX_VPORT_NUM)
> - return NULL;
> if (insert_value(devargs, lo) != 0)
> return NULL;
> } else if (result == 2) {
> - if (lo > hi || hi >= IDPF_MAX_VPORT_NUM)
> + if (lo > hi)
> return NULL;
> for (i = lo; i <= hi; i++) {
> if (insert_value(devargs, i) != 0)
> @@ -969,40 +961,46 @@ idpf_parse_devargs(struct rte_pci_device *pci_dev,
> struct
> idpf_adapter_ext *adap
> return -EINVAL;
> }
>
> + ret = rte_kvargs_process(kvlist, IDPF_VPORT, &parse_vport,
> + idpf_args);
> + if (ret != 0)
> + goto fail;
> +
> + ret = rte_kvargs_process(kvlist, IDPF_TX_SINGLE_Q, &parse_bool,
> + &adapter->base.is_tx_singleq);
> + if (ret != 0)
> + goto fail;
> +
> + ret = rte_kvargs_process(kvlist, IDPF_RX_SINGLE_Q, &parse_bool,
> + &adapter->base.is_rx_singleq);
> + if (ret != 0)
> + goto fail;
> +
> /* check parsed devargs */
> if (adapter->cur_vport_nb + idpf_args->req_vport_nb >
> - IDPF_MAX_VPORT_NUM) {
> + adapter->max_vport_nb) {
> PMD_INIT_LOG(ERR, "Total vport number can't be > %d",
> - IDPF_MAX_VPORT_NUM);
> + adapter->max_vport_nb);
> ret = -EINVAL;
> - goto bail;
> + goto fail;
> }
>
> for (i = 0; i < idpf_args->req_vport_nb; i++) {
> + if (idpf_args->req_vports[i] > adapter->max_vport_nb - 1) {
> + PMD_INIT_LOG(ERR, "Invalid vport id %d, it should be 0
> ~ %d",
> + idpf_args->req_vports[i],
> adapter->max_vport_nb - 1);
> + ret = -EINVAL;
This verify is not necessary, because we don't limit the vport id specified in
args need to be less than the number it supports.
RE: [PATCH v4] net/idpf: add VF support
> -Original Message- > From: Xing, Beilei > Sent: Monday, April 24, 2023 8:45 PM > To: Wu, Jingjing > Cc: dev@dpdk.org; Xing, Beilei > Subject: [PATCH v4] net/idpf: add VF support > > From: Beilei Xing > > Support VF whose device id is 0x145c. > > Signed-off-by: Beilei Xing Acked-by: Jingjing Wu
RE: [PATCH v5] common/idpf: refine capability get
> -Original Message- > From: Xing, Beilei > Sent: Monday, April 24, 2023 4:08 PM > To: Wu, Jingjing > Cc: dev@dpdk.org; Xing, Beilei > Subject: [PATCH v5] common/idpf: refine capability get > > From: Beilei Xing > > Initialize required capability in PMD, and refine > idpf_vc_caps_get function. Then different PMDs can > require different capability. > > Signed-off-by: Beilei Xing Acked-by: Jingjing Wu
RE: [PATCH v8 03/14] net/cpfl: add haipin queue group during vport init
> -Original Message-
> From: Xing, Beilei
> Sent: Monday, June 5, 2023 2:17 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei
>
> Subject: [PATCH v8 03/14] net/cpfl: add haipin queue group during vport init
>
> From: Beilei Xing
>
> This patch adds haipin queue group during vport init.
>
> Signed-off-by: Mingxia Liu
> Signed-off-by: Beilei Xing
> ---
> drivers/net/cpfl/cpfl_ethdev.c | 133 +
> drivers/net/cpfl/cpfl_ethdev.h | 18 +
> drivers/net/cpfl/cpfl_rxtx.h | 7 ++
> 3 files changed, 158 insertions(+)
>
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index e587155db6..c1273a7478 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -840,6 +840,20 @@ cpfl_dev_stop(struct rte_eth_dev *dev)
> return 0;
> }
>
> +static int
> +cpfl_p2p_queue_grps_del(struct idpf_vport *vport)
> +{
> + struct virtchnl2_queue_group_id qg_ids[CPFL_P2P_NB_QUEUE_GRPS] = {0};
> + int ret = 0;
> +
> + qg_ids[0].queue_group_id = CPFL_P2P_QUEUE_GRP_ID;
> + qg_ids[0].queue_group_type = VIRTCHNL2_QUEUE_GROUP_P2P;
> + ret = idpf_vc_queue_grps_del(vport, CPFL_P2P_NB_QUEUE_GRPS, qg_ids);
> + if (ret)
> + PMD_DRV_LOG(ERR, "Failed to delete p2p queue groups");
> + return ret;
> +}
> +
> static int
> cpfl_dev_close(struct rte_eth_dev *dev)
> {
> @@ -848,7 +862,12 @@ cpfl_dev_close(struct rte_eth_dev *dev)
> struct cpfl_adapter_ext *adapter = CPFL_ADAPTER_TO_EXT(vport->adapter);
>
> cpfl_dev_stop(dev);
> +
> + if (!adapter->base.is_rx_singleq && !adapter->base.is_tx_singleq)
> + cpfl_p2p_queue_grps_del(vport);
> +
> idpf_vport_deinit(vport);
> + rte_free(cpfl_vport->p2p_q_chunks_info);
>
> adapter->cur_vports &= ~RTE_BIT32(vport->devarg_id);
> adapter->cur_vport_nb--;
> @@ -1284,6 +1303,96 @@ cpfl_vport_idx_alloc(struct cpfl_adapter_ext *adapter)
> return vport_idx;
> }
>
> +static int
> +cpfl_p2p_q_grps_add(struct idpf_vport *vport,
> + struct virtchnl2_add_queue_groups *p2p_queue_grps_info,
> + uint8_t *p2p_q_vc_out_info)
> +{
> + int ret;
> +
> + p2p_queue_grps_info->vport_id = vport->vport_id;
> + p2p_queue_grps_info->qg_info.num_queue_groups =
> CPFL_P2P_NB_QUEUE_GRPS;
> + p2p_queue_grps_info->qg_info.groups[0].num_rx_q =
> CPFL_MAX_P2P_NB_QUEUES;
> + p2p_queue_grps_info->qg_info.groups[0].num_rx_bufq =
> CPFL_P2P_NB_RX_BUFQ;
> + p2p_queue_grps_info->qg_info.groups[0].num_tx_q =
> CPFL_MAX_P2P_NB_QUEUES;
> + p2p_queue_grps_info->qg_info.groups[0].num_tx_complq =
> CPFL_P2P_NB_TX_COMPLQ;
> + p2p_queue_grps_info->qg_info.groups[0].qg_id.queue_group_id =
> CPFL_P2P_QUEUE_GRP_ID;
> + p2p_queue_grps_info->qg_info.groups[0].qg_id.queue_group_type =
> VIRTCHNL2_QUEUE_GROUP_P2P;
> + p2p_queue_grps_info->qg_info.groups[0].rx_q_grp_info.rss_lut_size = 0;
> + p2p_queue_grps_info->qg_info.groups[0].tx_q_grp_info.tx_tc = 0;
> + p2p_queue_grps_info->qg_info.groups[0].tx_q_grp_info.priority = 0;
> + p2p_queue_grps_info->qg_info.groups[0].tx_q_grp_info.is_sp = 0;
> + p2p_queue_grps_info->qg_info.groups[0].tx_q_grp_info.pir_weight = 0;
> +
> + ret = idpf_vc_queue_grps_add(vport, p2p_queue_grps_info,
> p2p_q_vc_out_info);
> + if (ret != 0) {
> + PMD_DRV_LOG(ERR, "Failed to add p2p queue groups.");
> + return ret;
> + }
> +
> + return ret;
> +}
> +
> +static int
> +cpfl_p2p_queue_info_init(struct cpfl_vport *cpfl_vport,
> + struct virtchnl2_add_queue_groups *p2p_q_vc_out_info)
> +{
> + struct p2p_queue_chunks_info *p2p_q_chunks_info = cpfl_vport-
> >p2p_q_chunks_info;
> + struct virtchnl2_queue_reg_chunks *vc_chunks_out;
> + int i, type;
> +
> + if (p2p_q_vc_out_info->qg_info.groups[0].qg_id.queue_group_type !=
> + VIRTCHNL2_QUEUE_GROUP_P2P) {
> + PMD_DRV_LOG(ERR, "Add queue group response mismatch.");
> + return -EINVAL;
> + }
> +
> + vc_chunks_out = &p2p_q_vc_out_info->qg_info.groups[0].chunks;
> +
> + for (i = 0; i < vc_chunks_out->num_chunks; i++) {
> + type = vc_chunks_out->chunks[i].type;
> + switch (type) {
> + case VIRTCHNL2_QUEUE_TYPE_TX:
> + p2p_q_chunks_info->tx_start_qid =
> + vc_chunks_out->ch
RE: [PATCH v8 12/14] net/cpfl: support peer ports get
> -Original Message-
> From: Xing, Beilei
> Sent: Monday, June 5, 2023 2:17 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei
> ; Wang, Xiao W
> Subject: [PATCH v8 12/14] net/cpfl: support peer ports get
>
> From: Beilei Xing
>
> This patch supports get hairpin peer ports.
>
> Signed-off-by: Xiao Wang
> Signed-off-by: Beilei Xing
> ---
> drivers/net/cpfl/cpfl_ethdev.c | 41 ++
> 1 file changed, 41 insertions(+)
>
> diff --git a/drivers/net/cpfl/cpfl_ethdev.c b/drivers/net/cpfl/cpfl_ethdev.c
> index 850f1c0bc6..1a1ca4bc77 100644
> --- a/drivers/net/cpfl/cpfl_ethdev.c
> +++ b/drivers/net/cpfl/cpfl_ethdev.c
> @@ -1080,6 +1080,46 @@ cpfl_dev_close(struct rte_eth_dev *dev)
> return 0;
> }
>
> +static int
> +cpfl_hairpin_get_peer_ports(struct rte_eth_dev *dev, uint16_t *peer_ports,
> + size_t len, uint32_t tx)
> +{
> + struct cpfl_vport *cpfl_vport =
> + (struct cpfl_vport *)dev->data->dev_private;
> + struct idpf_tx_queue *txq;
> + struct idpf_rx_queue *rxq;
> + struct cpfl_tx_queue *cpfl_txq;
> + struct cpfl_rx_queue *cpfl_rxq;
> + int i;
> + int j = 0;
> +
> + if (len <= 0)
> + return -EINVAL;
> +
> + if (cpfl_vport->p2p_q_chunks_info == NULL)
> + return -ENOTSUP;
> +
> + if (tx > 0) {
> + for (i = cpfl_vport->nb_data_txq, j = 0; i <
> dev->data->nb_tx_queues; i++,
> j++) {
> + txq = dev->data->tx_queues[i];
> + if (txq == NULL)
> + return -EINVAL;
> + cpfl_txq = (struct cpfl_tx_queue *)txq;
> + peer_ports[j] = cpfl_txq->hairpin_info.peer_rxp;
Shouldn't access the peer_ports[j] if j >= len.
RE: [PATCH v10 00/14] net/cpfl: add hairpin queue support
> -Original Message- > From: Xing, Beilei > Sent: Tuesday, June 6, 2023 6:03 PM > To: Wu, Jingjing > Cc: dev@dpdk.org; Liu, Mingxia ; Xing, Beilei > > Subject: [PATCH v10 00/14] net/cpfl: add hairpin queue support > > From: Beilei Xing > > This patchset adds hairpin queue support. > > v2 changes: > - change hairpin rx queus configuration sequence. > - code refine. > > v3 changes: > - Refine the patchset based on the latest code. > > v4 change: > - Remove hairpin rx buffer queue's sw_ring. > - Change hairpin rx queus configuration sequence in cpfl_hairpin_bind > function. > - Refind hairpin queue setup and release. > > v5 change: > - Fix memory leak during queue setup. > - Refine hairpin Rxq/Txq start/stop. > > v6 change: > - Add sign-off. > > v7 change: > - Update cpfl.rst > > v8 change: > - Fix Intel-compilation failure. > > v9 change: > - Fix memory leak if fail to init queue group. > - Change log level. > > v10 change: > - Avoid accessing out-of-bounds. > > Beilei Xing (14): > net/cpfl: refine structures > common/idpf: support queue groups add/delete > net/cpfl: add haipin queue group during vport init > net/cpfl: support hairpin queue capbility get > net/cpfl: support hairpin queue setup and release > common/idpf: add queue config API > net/cpfl: support hairpin queue configuration > common/idpf: add switch queue API > net/cpfl: support hairpin queue start/stop > common/idpf: add irq map config API > net/cpfl: enable write back based on ITR expire > net/cpfl: support peer ports get > net/cpfl: support hairpin bind/unbind > doc: update the doc of CPFL PMD > > doc/guides/nics/cpfl.rst | 7 + > drivers/common/idpf/idpf_common_device.c | 75 ++ > drivers/common/idpf/idpf_common_device.h | 4 + > drivers/common/idpf/idpf_common_virtchnl.c | 138 +++- > drivers/common/idpf/idpf_common_virtchnl.h | 18 + > drivers/common/idpf/version.map| 6 + > drivers/net/cpfl/cpfl_ethdev.c | 613 ++-- > drivers/net/cpfl/cpfl_ethdev.h | 35 +- > drivers/net/cpfl/cpfl_rxtx.c | 789 +++-- > drivers/net/cpfl/cpfl_rxtx.h | 76 ++ > drivers/net/cpfl/cpfl_rxtx_vec_common.h| 21 +- > 11 files changed, 1663 insertions(+), 119 deletions(-) > > -- > 2.26.2 Acked-by: Jingjing Wu
RE: [PATCH] net/cpfl: fix fail to re-configure RSS
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, June 16, 2023 8:00 PM
> To: Wu, Jingjing ; Zhang, Yuying
>
> Cc: dev@dpdk.org; Xing, Beilei
> Subject: [PATCH] net/cpfl: fix fail to re-configure RSS
>
> From: Beilei Xing
>
> Currently, if launch testpmd with multiple queues and re-configure
> rxq with 'port config all rxq 1', Rx queue 0 may not receive packets.
> that's because RSS lookup tale is not re-configured when Rxq number
> is 1.
> Although Rxq number is 1 and multi queue mode is RTE_ETH_MQ_RX_NONE,
> cpfl PMD should init RSS to allow RSS re-configuration.
>
> Fixes: cfbc66551a14 ("net/cpfl: support RSS")
>
> Signed-off-by: Beilei Xing
Acked-by: Jingjing Wu
RE: [PATCH] doc: update release notes for Intel IPU
> -Original Message-
> From: Xing, Beilei
> Sent: Wednesday, June 28, 2023 11:39 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Xing, Beilei
> Subject: [PATCH] doc: update release notes for Intel IPU
>
> From: Beilei Xing
>
> Update release notes for Intel IPU new features:
> - Support VF whose device id is 0x145c.
> - Support hairpin queue.
>
> Fixes: 32bcd47e16fe ("net/idpf: support VF")
> Fixes: 1ec8064832db ("net/cpfl: add haipin queue group during vport init")
>
> Signed-off-by: Beilei Xing
Acked-by: Jingjing Wu
RE: [PATCH v2] raw/ntb: add check for disabling interrupt in dev close ops
> -Original Message-
> From: Guo, Junfeng
> Sent: Wednesday, June 28, 2023 5:12 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; sta...@dpdk.org; He, Xingguang ;
> Laatz, Kevin
> ; Guo, Junfeng
> Subject: [PATCH v2] raw/ntb: add check for disabling interrupt in dev close
> ops
>
> During EAL cleanup stage, all bus devices are cleaned up properly.
> In the meantime, the ntb example app will also do the device cleanup
> process, which may call the dev ops '*dev->dev_ops->dev_close' twice.
>
> If this dev ops for ntb was called twice, the interrupt handle for
> EAL will be disabled twice and will lead to error for the seconde
> time. Like this: "EAL: Error disabling MSI-X interrupts for fd xx"
>
> Thus, this patch added the check process for disabling interrupt in
> dev_close ops, to ensure that interrupt only be disabled once.
>
> Fixes: 1cab1a40ea9b ("bus: cleanup devices on shutdown")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Junfeng Guo
Acked-by: Jingjing Wu
RE: [PATCH] net/cpfl: fix RSS lookup table configuration
> -Original Message-
> From: Xing, Beilei
> Sent: Monday, July 3, 2023 7:28 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Xing, Beilei
> Subject: [PATCH] net/cpfl: fix RSS lookup table configuration
>
> From: Beilei Xing
>
> Ethdev Rx queues includes normal data queues and hairpin queues,
> RSS should direct traffic only to the normal data queues.
>
> Fixes: fda03330fcaa ("net/cpfl: support hairpin queue configuration")
>
> Signed-off-by: Beilei Xing
Acked-by: Jingjing Wu
RE: [PATCH] doc: update BIOS setting and supported HW list for NTB
> -Original Message-
> From: Guo, Junfeng
> Sent: Monday, July 3, 2023 5:25 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; sta...@dpdk.org; Guo, Junfeng
> Subject: [PATCH] doc: update BIOS setting and supported HW list for NTB
>
> Update BIOS settings and supported platform list for Intel NTB.
>
> Fixes: f5057be340e4 ("raw/ntb: support Intel Ice Lake")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Junfeng Guo
Acked-by: Jingjing Wu
Re: [dpdk-dev] [PATCH v4 2/6] net/iavf: rework tx path
> -Original Message- > From: Nicolau, Radu > Sent: Friday, October 1, 2021 5:51 PM > To: Wu, Jingjing ; Xing, Beilei > ; Richardson, > Bruce ; Ananyev, Konstantin > > Cc: dev@dpdk.org; Doherty, Declan ; Sinha, Abhijit > ; Zhang, Qi Z ; Nicolau, Radu > > Subject: [PATCH v4 2/6] net/iavf: rework tx path > > Rework the TX path and TX descriptor usage in order to > allow for better use of oflload flags and to facilitate enabling of > inline crypto offload feature. > > Signed-off-by: Declan Doherty > Signed-off-by: Abhijit Sinha > Signed-off-by: Radu Nicolau > --- > drivers/net/iavf/iavf_rxtx.c | 536 +++ > drivers/net/iavf/iavf_rxtx.h | 9 +- > drivers/net/iavf/iavf_rxtx_vec_sse.c | 10 +- > 3 files changed, 319 insertions(+), 236 deletions(-) Acked-by: Jingjing Wu
Re: [dpdk-dev] [PATCH v4 3/6] net/iavf: add support for asynchronous virt channel messages
> -Original Message- > From: Nicolau, Radu > Sent: Friday, October 1, 2021 5:51 PM > To: Wu, Jingjing ; Xing, Beilei > Cc: dev@dpdk.org; Doherty, Declan ; Sinha, Abhijit > ; Zhang, Qi Z ; Richardson, > Bruce > ; Ananyev, Konstantin > ; > Nicolau, Radu > Subject: [PATCH v4 3/6] net/iavf: add support for asynchronous virt channel > messages > > Add support for asynchronous virtual channel messages, specifically for > inline IPsec messages. > > Signed-off-by: Declan Doherty > Signed-off-by: Abhijit Sinha > Signed-off-by: Radu Nicolau > --- > drivers/net/iavf/iavf.h | 16 > drivers/net/iavf/iavf_vchnl.c | 137 +- > 2 files changed, 101 insertions(+), 52 deletions(-) Acked-by: Jingjing Wu
Re: [dpdk-dev] [PATCH v4 4/6] net/iavf: add iAVF IPsec inline crypto support
> -Original Message- > From: Nicolau, Radu > Sent: Friday, October 1, 2021 5:51 PM > To: Wu, Jingjing ; Xing, Beilei > ; Ray Kinsella > > Cc: dev@dpdk.org; Doherty, Declan ; Sinha, Abhijit > ; Zhang, Qi Z ; Richardson, > Bruce > ; Ananyev, Konstantin > ; > Nicolau, Radu > Subject: [PATCH v4 4/6] net/iavf: add iAVF IPsec inline crypto support > > Add support for inline crypto for IPsec, for ESP transport and > tunnel over IPv4 and IPv6, as well as supporting the offload for > ESP over UDP, and inconjunction with TSO for UDP and TCP flows. > Implement support for rte_security packet metadata > > Add definition for IPsec descriptors, extend support for offload > in data and context descriptor to support > > Add support to virtual channel mailbox for IPsec Crypto request > operations. IPsec Crypto requests receive an initial acknowledgement > from phsyical function driver of receipt of request and then an > asynchronous response with success/failure of request including any > response data. > > Add enhanced descriptor debugging > > Refactor of scalar tx burst function to support integration of offload > > Signed-off-by: Declan Doherty > Signed-off-by: Abhijit Sinha > Signed-off-by: Radu Nicolau Reviewed-by: Jingjing Wu
Re: [dpdk-dev] [PATCH v4 5/6] net/iavf: add xstats support for inline IPsec crypto
> -Original Message- > From: Nicolau, Radu > Sent: Friday, October 1, 2021 5:51 PM > To: Wu, Jingjing ; Xing, Beilei > Cc: dev@dpdk.org; Doherty, Declan ; Sinha, Abhijit > ; Zhang, Qi Z ; Richardson, > Bruce > ; Ananyev, Konstantin > ; > Nicolau, Radu > Subject: [PATCH v4 5/6] net/iavf: add xstats support for inline IPsec crypto > > Add per queue counters for maintaining statistics for inline IPsec > crypto offload, which can be retrieved through the > rte_security_session_stats_get() with more detailed errors through the > rte_ethdev xstats. > > Signed-off-by: Declan Doherty > Signed-off-by: Radu Nicolau Acked-by: Jingjing Wu
Re: [dpdk-dev] [PATCH v4 6/6] net/iavf: add watchdog for VFLR
> -Original Message-
> From: Nicolau, Radu
> Sent: Friday, October 1, 2021 5:52 PM
> To: Wu, Jingjing ; Xing, Beilei
> Cc: dev@dpdk.org; Doherty, Declan ; Sinha, Abhijit
> ; Zhang, Qi Z ; Richardson,
> Bruce
> ; Ananyev, Konstantin
> ;
> Nicolau, Radu
> Subject: [PATCH v4 6/6] net/iavf: add watchdog for VFLR
>
> Add watchdog to iAVF PMD which support monitoring the VFLR register. If
> the device is not already in reset then if a VF reset in progress is
> detected then notfiy user through callback and set into reset state.
> If the device is already in reset then poll for completion of reset.
>
> Signed-off-by: Declan Doherty
> Signed-off-by: Radu Nicolau
> ---
> drivers/net/iavf/iavf.h| 6 +++
> drivers/net/iavf/iavf_ethdev.c | 97 ++
> 2 files changed, 103 insertions(+)
>
> diff --git a/drivers/net/iavf/iavf.h b/drivers/net/iavf/iavf.h
> index d5f574b4b3..4481d2e134 100644
> --- a/drivers/net/iavf/iavf.h
> +++ b/drivers/net/iavf/iavf.h
> @@ -212,6 +212,12 @@ struct iavf_info {
> int cmd_retval; /* return value of the cmd response from PF */
> uint8_t *aq_resp; /* buffer to store the adminq response from PF */
>
> + struct {
> + uint8_t enabled:1;
> + uint64_t period_us;
> + } watchdog;
> + /** iAVF watchdog configuration */
> +
> /* Event from pf */
> bool dev_closed;
> bool link_up;
> diff --git a/drivers/net/iavf/iavf_ethdev.c b/drivers/net/iavf/iavf_ethdev.c
> index aad6a28585..d02aa9c1c5 100644
> --- a/drivers/net/iavf/iavf_ethdev.c
> +++ b/drivers/net/iavf/iavf_ethdev.c
> @@ -24,6 +24,7 @@
> #include
> #include
> #include
> +#include
>
> #include "iavf.h"
> #include "iavf_rxtx.h"
> @@ -239,6 +240,94 @@ iavf_tm_ops_get(struct rte_eth_dev *dev __rte_unused,
> return 0;
> }
>
> +
> +static int
> +iavf_vfr_inprogress(struct iavf_hw *hw)
> +{
> + int inprogress = 0;
> +
> + if ((IAVF_READ_REG(hw, IAVF_VFGEN_RSTAT) &
> + IAVF_VFGEN_RSTAT_VFR_STATE_MASK) ==
> + VIRTCHNL_VFR_INPROGRESS)
> + inprogress = 1;
> +
> + if (inprogress)
> + PMD_DRV_LOG(INFO, "Watchdog detected VFR in progress");
> +
> + return inprogress;
> +}
> +
> +static void
> +iavf_dev_watchdog(void *cb_arg)
> +{
> + struct iavf_adapter *adapter = cb_arg;
> + struct iavf_hw *hw = IAVF_DEV_PRIVATE_TO_HW(adapter);
> + int vfr_inprogress = 0, rc = 0;
> +
> + /* check if watchdog has been disabled since last call */
> + if (!adapter->vf.watchdog.enabled)
> + return;
> +
> + /* If in reset then poll vfr_inprogress register for completion */
> + if (adapter->vf.vf_reset) {
> + vfr_inprogress = iavf_vfr_inprogress(hw);
> +
> + if (!vfr_inprogress) {
> + PMD_DRV_LOG(INFO, "VF \"%s\" reset has completed",
> + adapter->eth_dev->data->name);
> + adapter->vf.vf_reset = false;
> + }
> + /* If not in reset then poll vfr_inprogress register for VFLR event */
> + } else {
> + vfr_inprogress = iavf_vfr_inprogress(hw);
> +
> + if (vfr_inprogress) {
> + PMD_DRV_LOG(INFO,
> + "VF \"%s\" reset event has been detected by
> watchdog",
> + adapter->eth_dev->data->name);
> +
> + /* enter reset state with VFLR event */
> + adapter->vf.vf_reset = true;
> +
> + rte_eth_dev_callback_process(adapter->eth_dev,
> + RTE_ETH_EVENT_INTR_RESET, NULL);
> + }
> + }
> +
> + /* re-alarm watchdog */
> + rc = rte_eal_alarm_set(adapter->vf.watchdog.period_us,
> + &iavf_dev_watchdog, cb_arg);
> +
> + if (rc)
> + PMD_DRV_LOG(ERR, "Failed \"%s\" to reset device watchdog alarm",
> + adapter->eth_dev->data->name);
> +}
> +
> +static void
> +iavf_dev_watchdog_enable(struct iavf_adapter *adapter, uint64_t period_us)
> +{
> + int rc;
> +
> + PMD_DRV_LOG(INFO, "Enabling device watchdog");
> +
> + adapter->vf.watchdog.enabled = 1;
> + adapter->vf.watchdog.period_us = period_us;
> +
> + rc = rte_eal_alarm_set(adapter->vf.watchdog.period_us,
> + &iavf_dev_watchdog, (void *)adapter
Re: [dpdk-dev] [PATCH v4 6/6] net/iavf: add watchdog for VFLR
> > Besides checking VFGEN_RSTAT, there is a process to handle > VIRTCHNL_OP_EVENT from PF. What is the change for? Any scenario which > VIRTCHNL_OP_EVENT doesn't cover? > > And how is the 500us been determined? > > Hi Jingjing, thanks for reviewing, I think this can be handled with the > VIRTCHNL_OP_EVENT with no need for a watchdog alarm, I will rework the > patch. > Hi, Radu, I saw the patch is reworked, but looks like watchdog is still there. So what is the scenario VIRTCHNL_OP_EVENT doesn't cover?
RE: [PATCH v2] net/idpf: fix crash when launching l3fwd
> -
> if (conf->txmode.mq_mode != RTE_ETH_MQ_TX_NONE) {
> PMD_INIT_LOG(ERR, "Multi-queue TX mode %d is not supported",
>conf->txmode.mq_mode);
> diff --git a/drivers/net/idpf/idpf_vchnl.c b/drivers/net/idpf/idpf_vchnl.c
> index ac6486d4ef..88770447f8 100644
> --- a/drivers/net/idpf/idpf_vchnl.c
> +++ b/drivers/net/idpf/idpf_vchnl.c
> @@ -1197,6 +1197,9 @@ idpf_vc_dealloc_vectors(struct idpf_vport *vport)
> int err, len;
>
> alloc_vec = vport->recv_vectors;
> + if (alloc_vec == NULL)
> + return -EINVAL;
> +
Would it be better to check before idpf_vc_dealloc_vectors?
RE: [PATCH] net/idpf: add supported ptypes get
> -Original Message-
> From: Xing, Beilei
> Sent: Friday, November 18, 2022 11:51 AM
> To: Wu, Jingjing
> Cc: dev@dpdk.org; Peng, Yuan ; Xing, Beilei
>
> Subject: [PATCH] net/idpf: add supported ptypes get
>
> From: Beilei Xing
>
> Failed to launch l3fwd, the log shows:
> port 0 cannot parse packet type, please add --parse-ptype
> This patch adds dev_supported_ptypes_get ops.
>
> Fixes: 549343c25db8 ("net/idpf: support device initialization")
>
> Signed-off-by: Beilei Xing
Reviewed-by: Jingjing Wu
RE: Building DPDK with IOVA_AS_VA
I guess driver may not handle the attribute enable_iova_as_pa well right now. Maybe you can have a try by disabling idpf driver by adding "-Ddisable_drivers=net/idpf". > -Original Message- > From: Morten Brørup > Sent: Wednesday, December 7, 2022 2:55 AM > To: Richardson, Bruce ; Wu, Jingjing > ; Xing, Beilei > Cc: dev@dpdk.org; Shijith Thotton > Subject: RE: Building DPDK with IOVA_AS_VA > > +To: Intel idpf maintainers Jingjing Wu, Beilei Xing > > > From: Morten Brørup [mailto:m...@smartsharesystems.com] > > Sent: Tuesday, 6 December 2022 17.58 > > > > Bruce, > > > > How do I build with IOVA_AS_VA, to benefit from Shijith Thotton's > > patch? (Bonus question: How do I make the CI build this way?) > > > > When I try this command: > > meson -Dplatform=generic -Dcheck_includes=true - > > Denable_iova_as_pa=false work > > > > It fails with: > > drivers/net/idpf/meson.build:36:8: ERROR: Unknown variable > > "static_rte_common_idpf". > > > > Here is the full output: > > > > The Meson build system > > Version: 0.60.3 > > Source dir: /home/morten/upstreaming/dpdk-experiment > > Build dir: /home/morten/upstreaming/dpdk-experiment/work > > Build type: native build > > Program cat found: YES (/usr/bin/cat) > > Project name: DPDK > > Project version: 23.03.0-rc0 > > C compiler for the host machine: cc (gcc 9.4.0 "cc (Ubuntu 9.4.0- > > 1ubuntu1~20.04.1) 9.4.0") > > C linker for the host machine: cc ld.bfd 2.34 > > Host machine cpu family: x86_64 > > Host machine cpu: x86_64 > > Message: ## Building in Developer Mode ## > > Program pkg-config pkgconf found: NO > > Program check-symbols.sh found: YES (/home/morten/upstreaming/dpdk- > > experiment/buildtools/check-symbols.sh) > > Program options-ibverbs-static.sh found: YES > > (/home/morten/upstreaming/dpdk-experiment/buildtools/options-ibverbs- > > static.sh) > > Program objdump found: YES (/usr/bin/objdump) > > Program python3 found: YES (/usr/bin/python3) > > Program cat found: YES (/usr/bin/cat) > > Program ../buildtools/symlink-drivers-solibs.sh found: YES (/bin/sh > > /home/morten/upstreaming/dpdk-experiment/config/../buildtools/symlink- > > drivers-solibs.sh) > > Checking for size of "void *" : 8 > > Checking for size of "void *" : 8 > > Library m found: YES > > Library numa found: YES > > Has header "numaif.h" : YES > > Library libfdt found: NO > > Library libexecinfo found: NO > > Did not find pkg-config by name 'pkg-config' > > Found Pkg-config: NO > > Run-time dependency libarchive found: NO (tried pkgconfig) > > Run-time dependency libbsd found: NO (tried pkgconfig) > > Run-time dependency jansson found: NO (tried pkgconfig) > > Run-time dependency openssl found: NO (tried pkgconfig) > > Run-time dependency libpcap found: NO (tried pkgconfig) > > Library pcap found: YES > > Has header "pcap.h" with dependency -lpcap: YES > > Compiler for C supports arguments -Wcast-qual: YES > > Compiler for C supports arguments -Wdeprecated: YES > > Compiler for C supports arguments -Wformat: YES > > Compiler for C supports arguments -Wformat-nonliteral: YES > > Compiler for C supports arguments -Wformat-security: YES > > Compiler for C supports arguments -Wmissing-declarations: YES > > Compiler for C supports arguments -Wmissing-prototypes: YES > > Compiler for C supports arguments -Wnested-externs: YES > > Compiler for C supports arguments -Wold-style-definition: YES > > Compiler for C supports arguments -Wpointer-arith: YES > > Compiler for C supports arguments -Wsign-compare: YES > > Compiler for C supports arguments -Wstrict-prototypes: YES > > Compiler for C supports arguments -Wundef: YES > > Compiler for C supports arguments -Wwrite-strings: YES > > Compiler for C supports arguments -Wno-address-of-packed-member: YES > > Compiler for C supports arguments -Wno-packed-not-aligned: YES > > Compiler for C supports arguments -Wno-missing-field-initializers: YES > > Compiler for C supports arguments -mavx512f: YES > > Checking if "AVX512 checking" : compiles: YES > > Fetching value of define "__SSE4_2__" : 1 > > Fetching value of define "__AES__" : > > Fetching value of define "__AVX__" : > > Fetching value of define "__AVX2__" : > > Fetching value of define "__AVX512BW__" : > > Fetching value of define "__AVX512CD__" : > > Fetching value of define "__AVX
RE: Building DPDK with IOVA_AS_VA
Yes, thanks for the suggestion.
> -Original Message-
> From: Shijith Thotton
> Sent: Wednesday, December 7, 2022 3:16 PM
> To: Wu, Jingjing ; Xing, Beilei
> Cc: dev@dpdk.org; Morten Brørup ; Richardson,
> Bruce
>
> Subject: RE: Building DPDK with IOVA_AS_VA
>
> Hi Jingjing Wu/Beilei Xing.
>
> >I guess driver may not handle the attribute enable_iova_as_pa well right now.
> >Maybe you can have a try by disabling idpf driver by adding "-
> >Ddisable_drivers=net/idpf".
> >
>
> Please send a fix. A check can be added similar to hns3 PMD.
>
> +if dpdk_conf.get('RTE_IOVA_AS_PA') == 0
> +build = false
> +reason = 'driver does not support disabling IOVA as PA mode'
> +subdir_done()
> +endif
> +
>
> Thanks,
> Shijith
>
> >>
> >> +To: Intel idpf maintainers Jingjing Wu, Beilei Xing
> >>
> >> > From: Morten Brørup [mailto:m...@smartsharesystems.com]
> >> > Sent: Tuesday, 6 December 2022 17.58
> >> >
> >> > Bruce,
> >> >
> >> > How do I build with IOVA_AS_VA, to benefit from Shijith Thotton's
> >> > patch? (Bonus question: How do I make the CI build this way?)
> >> >
> >> > When I try this command:
> >> > meson -Dplatform=generic -Dcheck_includes=true -
> >> > Denable_iova_as_pa=false work
> >> >
> >> > It fails with:
> >> > drivers/net/idpf/meson.build:36:8: ERROR: Unknown variable
> >> > "static_rte_common_idpf".
> >> >
> >> > Here is the full output:
> >> >
> >> > The Meson build system
> >> > Version: 0.60.3
> >> > Source dir: /home/morten/upstreaming/dpdk-experiment
> >> > Build dir: /home/morten/upstreaming/dpdk-experiment/work
> >> > Build type: native build
> >> > Program cat found: YES (/usr/bin/cat)
> >> > Project name: DPDK
> >> > Project version: 23.03.0-rc0
> >> > C compiler for the host machine: cc (gcc 9.4.0 "cc (Ubuntu 9.4.0-
> >> > 1ubuntu1~20.04.1) 9.4.0")
> >> > C linker for the host machine: cc ld.bfd 2.34
> >> > Host machine cpu family: x86_64
> >> > Host machine cpu: x86_64
> >> > Message: ## Building in Developer Mode ##
> >> > Program pkg-config pkgconf found: NO
> >> > Program check-symbols.sh found: YES (/home/morten/upstreaming/dpdk-
> >> > experiment/buildtools/check-symbols.sh)
> >> > Program options-ibverbs-static.sh found: YES
> >> > (/home/morten/upstreaming/dpdk-experiment/buildtools/options-ibverbs-
> >> > static.sh)
> >> > Program objdump found: YES (/usr/bin/objdump)
> >> > Program python3 found: YES (/usr/bin/python3)
> >> > Program cat found: YES (/usr/bin/cat)
> >> > Program ../buildtools/symlink-drivers-solibs.sh found: YES (/bin/sh
> >> > /home/morten/upstreaming/dpdk-experiment/config/../buildtools/symlink-
> >> > drivers-solibs.sh)
> >> > Checking for size of "void *" : 8
> >> > Checking for size of "void *" : 8
> >> > Library m found: YES
> >> > Library numa found: YES
> >> > Has header "numaif.h" : YES
> >> > Library libfdt found: NO
> >> > Library libexecinfo found: NO
> >> > Did not find pkg-config by name 'pkg-config'
> >> > Found Pkg-config: NO
> >> > Run-time dependency libarchive found: NO (tried pkgconfig)
> >> > Run-time dependency libbsd found: NO (tried pkgconfig)
> >> > Run-time dependency jansson found: NO (tried pkgconfig)
> >> > Run-time dependency openssl found: NO (tried pkgconfig)
> >> > Run-time dependency libpcap found: NO (tried pkgconfig)
> >> > Library pcap found: YES
> >> > Has header "pcap.h" with dependency -lpcap: YES
> >> > Compiler for C supports arguments -Wcast-qual: YES
> >> > Compiler for C supports arguments -Wdeprecated: YES
> >> > Compiler for C supports arguments -Wformat: YES
> >> > Compiler for C supports arguments -Wformat-nonliteral: YES
> >> > Compiler for C supports arguments -Wformat-security: YES
> >> > Compiler for C supports arguments -Wmissing-declarations: YES
> >> > Compiler for C supports arguments -Wmissing-prototypes: YES
> >> > Compiler for C supports arguments -Wnested-externs: YES
> >> > Compiler for C supports arguments -Wold-style-definition: YES
>
RE: [PATCH] net/iavf:fix slow memory allocation
> -Original Message-
> From: You, KaisenX
> Sent: Thursday, November 17, 2022 2:57 PM
> To: dev@dpdk.org
> Cc: sta...@dpdk.org; Yang, Qiming ; Zhou, YidingX
> ; You, KaisenX ; Wu, Jingjing
> ; Xing, Beilei ; Zhang, Qi Z
>
> Subject: [PATCH] net/iavf:fix slow memory allocation
>
> In some cases, the DPDK does not allocate hugepage heap memory to
> some sockets due to the user setting parameters
> (e.g. -l 40-79, SOCKET 0 has no memory).
> When the interrupt thread runs on the corresponding core of this
> socket, each allocation/release will execute a whole set of heap
> allocation/release operations,resulting in poor performance.
> Instead we call malloc() to get memory from the system's
> heap space to fix this problem.
>
> Fixes: cb5c1b91f76f ("net/iavf: add thread for event callbacks")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Kaisen You
Acked-by: Jingjing Wu
Re: [dpdk-dev] [PATCH] net/iavf: fix Rx issue for scalar Rx functions
> -Original Message-
> From: Xing, Beilei
> Sent: Tuesday, June 1, 2021 1:10 PM
> To: Wu, Jingjing ; Zhang, Qi Z
> Cc: dev@dpdk.org; Xing, Beilei ; sta...@dpdk.org
> Subject: [PATCH] net/iavf: fix Rx issue for scalar Rx functions
>
> From: Beilei Xing
>
> The new allocated mbuf should be updated to the SW
> ring.
>
> Fixes: a2b29a7733ef ("net/avf: enable basic Rx Tx")
> Fixes: b8b4c54ef9b0 ("net/iavf: support flexible Rx descriptor in normal
> path")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Beilei Xing
Acked-by: Jingjing Wu
[dpdk-dev] [PATCH][PMD][GENERIC_FILTER] add NIC filters support for generic filter feature
Fine, I will split it. -Original Message- From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] Sent: Monday, May 19, 2014 6:17 PM To: Wu, Jingjing Cc: dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH][PMD][GENERIC_FILTER] add NIC filters support for generic filter feature Hi Jingjing, 2014-05-17 17:35, Jingjing Wu: > A generic filter mechanism for handling special packet is required. It > will allows filters to be set in HW when available for so that > specific packets may be filtered by NICs to specific desriptor queues for > processing. > Currently only the Flow Director for Intel's 10GbE 82599 devices is > available. Other types of filter are not supported. > > This pacth adds following NIC filters used to assign different packets > to certain receive queues. ethertype filter/syn filter/2tuple > filter/flex filter for E1000(82580, i350) ethertype filter/syn > filter/5tuple filter for > 10G(82599) > > Signed-off-by: jingjing.wu > --- > app/test-pmd/cmdline.c | 905 > +++- app/test-pmd/config.c | > 143 ++ > app/test-pmd/testpmd.h | 6 + > lib/librte_ether/rte_ethdev.c | 300 > lib/librte_ether/rte_ethdev.h | 429 - > lib/librte_pmd_e1000/e1000_ethdev.h | 38 ++ > lib/librte_pmd_e1000/igb_ethdev.c | 512 > lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 365 +++ > lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 33 ++ > 9 files changed, 2729 insertions(+), 2 deletions(-) It would be really easier to review if you split it in 4 parts: - ethdev API - igb implementation - ixgbe implementation - testpmd Thanks -- Thomas
[dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter
Hi, Thomas This patch is mainly about multiple NIC filters implement. It has close relationship with NICs. As the patch says: NIC filters list below are implemented in this patchset: ethertype filter, syn filter, 2tuple filter and flex filter for 82580 and i350 ethertype filter, syn filter, 5tuple filter for 82576 ethertype filter, syn filter and 5tuple filter for 82599 The same type filter uses the same API for the NICs list above. About the generic filter feature, how to define the "generic" is still in discussing, and not included in this patch. These NIC filters implemented in this patch are first step. Even without generic, it also provides a way to configure these NIC filters to hardware in DPDK PMD. -Original Message- From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] Sent: Wednesday, May 28, 2014 7:22 AM To: Wu, Jingjing Cc: dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter Hi Jingjing, 2014-05-24 09:37, Jingjing Wu: > A generic filter mechanism for handling special packet is required. > It will allow filters to be set in HW when available so that specific > packet may be filtered by NICs to specific descriptor queues for > processing. Currently only Flow Director for Intel's 10GbE 82599 > devices is available. Other types of filter are not support. > NIC filters list below are implemented in this patchset: > ethertype filter, syn filter, 2tuple filter and flex filter for > 82580 and > i350 ethertype filter, syn filter, 5tuple filter for 82576 > ethertype filter, syn filter and 5tuple filter for 82599 I'd like we have a discussion about how this API is generic enough. I think many people would like to integrate drivers for other NICs in DPDK and I'd hate to see a global rework of this API because we haven't tried to think about it before. First, is there someone in the mailing list who knows other hardware which could fit in this filtering feature? Thanks -- Thomas
[dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter
Hi, Thomas The generic you said may be different from I mentioned in last mail. You are discussing whether the APIs provide for NIC filters is generic or not. About that we can use same API for a type of filter. For example, if we want to configure ethertype filter, we can use the same API, no matter the NIC is 82580, i350, 82576 or 82599. We think these NICs may be most common used. -Original Message- From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wu, Jingjing Sent: Wednesday, May 28, 2014 8:53 AM To: Thomas Monjalon Cc: dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter Hi, Thomas This patch is mainly about multiple NIC filters implement. It has close relationship with NICs. As the patch says: NIC filters list below are implemented in this patchset: ethertype filter, syn filter, 2tuple filter and flex filter for 82580 and i350 ethertype filter, syn filter, 5tuple filter for 82576 ethertype filter, syn filter and 5tuple filter for 82599 The same type filter uses the same API for the NICs list above. About the generic filter feature, how to define the "generic" is still in discussing, and not included in this patch. These NIC filters implemented in this patch are first step. Even without generic, it also provides a way to configure these NIC filters to hardware in DPDK PMD. -Original Message- From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] Sent: Wednesday, May 28, 2014 7:22 AM To: Wu, Jingjing Cc: dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter Hi Jingjing, 2014-05-24 09:37, Jingjing Wu: > A generic filter mechanism for handling special packet is required. > It will allow filters to be set in HW when available so that specific > packet may be filtered by NICs to specific descriptor queues for > processing. Currently only Flow Director for Intel's 10GbE 82599 > devices is available. Other types of filter are not support. > NIC filters list below are implemented in this patchset: > ethertype filter, syn filter, 2tuple filter and flex filter for > 82580 and > i350 ethertype filter, syn filter, 5tuple filter for 82576 > ethertype filter, syn filter and 5tuple filter for 82599 I'd like we have a discussion about how this API is generic enough. I think many people would like to integrate drivers for other NICs in DPDK and I'd hate to see a global rework of this API because we haven't tried to think about it before. First, is there someone in the mailing list who knows other hardware which could fit in this filtering feature? Thanks -- Thomas
Re: [dpdk-dev] [RFC PATCH] i40e: fix setting of default MAC address
Hi, Igor
Thanks for your contribute, my comments are in below, thanks.
> -Original Message-
> From: Igor Ryzhov [mailto:iryz...@nfware.com]
> Sent: Thursday, November 24, 2016 8:35 PM
> To: dev@dpdk.org
> Cc: Zhang, Helin ; Wu, Jingjing
>
> Subject: [RFC PATCH] i40e: fix setting of default MAC address
>
> While testing X710 cards in our lab I found that setting of default MAC
> address doesn't work correctly for i40e driver. I compared DPDK driver
> implementation with Linux driver implementation and found that a lot of
> code is lost in DPDK.
> I tried to make DPDK implementation similar to Linux implementation and it
> worked for me – now everything is working. But I'm not sure that my
> changes are correct so, please, maintainers, check the patch very careful.
>
> Signed-off-by: Igor Ryzhov
> ---
> drivers/net/i40e/i40e_ethdev.c | 30 --
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 67778ba..b73f9c8 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -9694,6 +9694,7 @@ static int i40e_get_eeprom(struct rte_eth_dev
> *dev, static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
> struct ether_addr *mac_addr)
> {
> + struct i40e_vsi *vsi =
> +I40E_DEV_PRIVATE_TO_MAIN_VSI(dev->data->dev_private);
> struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>
> if (!is_valid_assigned_ether_addr(mac_addr)) { @@ -9701,8 +9702,33
> @@ static void i40e_set_default_mac_addr(struct rte_eth_dev *dev,
> return;
> }
>
> - /* Flags: 0x3 updates port address */
> - i40e_aq_mac_address_write(hw, 0x3, mac_addr->addr_bytes,
> NULL);
> + i40e_aq_mac_address_write(hw, I40E_AQC_WRITE_TYPE_LAA_WOL,
> +mac_addr->addr_bytes, NULL);
> +
> + if (!memcmp(&dev->data->mac_addrs[0].addr_bytes, hw-
> >mac.addr, ETH_ADDR_LEN)) {
> + struct i40e_aqc_remove_macvlan_element_data element;
> +
> + memset(&element, 0, sizeof(element));
> + memcpy(element.mac_addr, &dev->data-
> >mac_addrs[0].addr_bytes, ETH_ADDR_LEN);
> + element.flags = I40E_AQC_MACVLAN_DEL_PERFECT_MATCH;
> + i40e_aq_remove_macvlan(hw, vsi->seid, &element, 1, NULL);
> + } else {
> + i40e_vsi_delete_mac(vsi, &dev->data->mac_addrs[0]);
> + }
> +
> + if (!memcmp(mac_addr->addr_bytes, hw->mac.addr,
> ETH_ADDR_LEN)) {
In rte_eth_dev_default_mac_addr_set, before call dev_ops->mac_addr_set,
The dev->data->mac_addrs[0] is already set to the input addr.
So the if loop is the same as above if, why not merge them?
If you look the code in eth_i40e_dev_init, you will found the
dev->data->mac_addrs[0]
Is just the hw->mac.addr. And I think we need to make them same all the time.
So hw->mac.addr may also need to be updated.
Thanks for figure it out!
/Jingjing
[dpdk-dev] Signature filter for virtual function
Hi Flow director is not supported on VF now. I think this may be the reason why ENOTSUP is returned. > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ilan Borenshtein > Sent: Thursday, August 14, 2014 8:34 PM > To: dev at dpdk.org > Subject: [dpdk-dev] Signature filter for virtual function > > Hello, > > I'm trying to configure the flow director of VF 82599 port using > rte_eth_dev_fdir_add_signature_filter (). > The function returns with ENOTSUP. > Doe's flow director supported for VF ? > What can be the reason for ENOTSUP ? > > Thanks, > Ilan B
[dpdk-dev] Signature filter for virtual function
Could you explain what exactly you want? Director packet to a VF? Or director packet to different queue in a VF? For the first one, I think if you know which queue the VF is using, the flow director can do it (it's my understanding, haven't try). But for the latter, I have no idea which way can be used instead. > -Original Message- > From: Ilan Borenshtein [mailto:ilan at asocstech.com] > Sent: Sunday, August 17, 2014 3:34 PM > To: Wu, Jingjing; dev at dpdk.org > Subject: RE: Signature filter for virtual function > > Hi, > > Is there any other way except of the Flow director to use the different Rx > queues by a VF ? > Will the next versions support flow director for VF ? > > Thanks, > Ilan B > > > -Original Message- > From: Wu, Jingjing [mailto:jingjing.wu at intel.com] > Sent: Friday, August 15, 2014 10:51 AM > To: Ilan Borenshtein; dev at dpdk.org > Subject: RE: Signature filter for virtual function > > Hi > > Flow director is not supported on VF now. > I think this may be the reason why ENOTSUP is returned. > > > > -Original Message- > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ilan Borenshtein > > Sent: Thursday, August 14, 2014 8:34 PM > > To: dev at dpdk.org > > Subject: [dpdk-dev] Signature filter for virtual function > > > > Hello, > > > > I'm trying to configure the flow director of VF 82599 port using > > rte_eth_dev_fdir_add_signature_filter (). > > The function returns with ENOTSUP. > > Doe's flow director supported for VF ? > > What can be the reason for ENOTSUP ? > > > > Thanks, > > Ilan B
[dpdk-dev] Signature filter for virtual function
Are you talking about 82599? I have no plan to support this currently. But I don't know there may be volunteers who is planning for it. I guess it also be great if you are eager to contribute to it. > -Original Message- > From: Ilan Borenshtein [mailto:ilan at asocstech.com] > Sent: Monday, August 18, 2014 3:17 PM > To: Wu, Jingjing; dev at dpdk.org > Subject: RE: Signature filter for virtual function > > Looking for director packet to different queues in a VF. > Will it be implemented in future releases ? > > -----Original Message- > From: Wu, Jingjing [mailto:jingjing.wu at intel.com] > Sent: Monday, August 18, 2014 3:40 AM > To: Ilan Borenshtein; dev at dpdk.org > Subject: RE: Signature filter for virtual function > > Could you explain what exactly you want? Director packet to a VF? Or director > packet to > different queue in a VF? > > For the first one, I think if you know which queue the VF is using, the flow > director can do it > (it's my understanding, haven't try). > But for the latter, I have no idea which way can be used instead. > > > > -Original Message- > > From: Ilan Borenshtein [mailto:ilan at asocstech.com] > > Sent: Sunday, August 17, 2014 3:34 PM > > To: Wu, Jingjing; dev at dpdk.org > > Subject: RE: Signature filter for virtual function > > > > Hi, > > > > Is there any other way except of the Flow director to use the different Rx > > queues by a VF ? > > Will the next versions support flow director for VF ? > > > > Thanks, > > Ilan B > > > > > > -Original Message- > > From: Wu, Jingjing [mailto:jingjing.wu at intel.com] > > Sent: Friday, August 15, 2014 10:51 AM > > To: Ilan Borenshtein; dev at dpdk.org > > Subject: RE: Signature filter for virtual function > > > > Hi > > > > Flow director is not supported on VF now. > > I think this may be the reason why ENOTSUP is returned. > > > > > > > -Original Message- > > > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Ilan > > > Borenshtein > > > Sent: Thursday, August 14, 2014 8:34 PM > > > To: dev at dpdk.org > > > Subject: [dpdk-dev] Signature filter for virtual function > > > > > > Hello, > > > > > > I'm trying to configure the flow director of VF 82599 port using > > > rte_eth_dev_fdir_add_signature_filter (). > > > The function returns with ENOTSUP. > > > Doe's flow director supported for VF ? > > > What can be the reason for ENOTSUP ? > > > > > > Thanks, > > > Ilan B
[dpdk-dev] [PATCH v2 1/7] i40e: flow director resource reserve and initialize on i40e
Hi, Thomas The "flow director resource" means we need to reserve a specific queue and VSI on HW to support. The queue and vsi is used for programming flow director filter, not common tx/rx queues. 2 separated items, one is about setup, another is for release. I will try to separate them. > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, August 27, 2014 10:18 PM > To: Wu, Jingjing > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 1/7] i40e: flow director resource reserve > and initialize on > i40e > > Hi Jingjing, > > I don't understand the title. What is a "flow director resource"? > > 2014-08-27 10:13, Jingjing Wu: > > flow director resource reserve and initialize on i40e, it includes > > - queue initialization and switch on and vsi creation during setup > > - queue vsi for flow director release during close > > If you have 2 separated items, it probably means it should be 2 patches. > > -- > Thomas
[dpdk-dev] [PATCH v2 3/7] i40e: function implement in i40e for flow director filter programming
Hi, Thomas I will rework on it. > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, August 27, 2014 10:25 PM > To: Wu, Jingjing > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 3/7] i40e: function implement in i40e for > flow director > filter programming > > 2014-08-27 10:13, Jingjing Wu: > > support the API ops defined in ethdev, the behavior according to each > > command: > > RTE_CMD_FDIR_RULE_ADD: add a new FDIR filter rule. > > RTE_CMD_FDIR_RULE_DEL: delete a FDIR filter rule. > > Here title is really too complicated. Be concise. > I'd change it to: > i40e: flow director filter > > -- > Thomas
[dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl
HI, Thomas Just as zhang, helin's said in his pacth "[dpdk-dev] [PATCH 2/5] ethdev: add new ops of 'check_command_supported' and 'rx_classification_filter_ctl'": 'rx_classification_filter_ctl' is for receive classification filter configuring. e.g. hash function configuration, flow director configuration. It is a common API where a lot of commands can be implemented for different sub features. We want to implement several common API for NIC specific features, to avoid creating quite a lot of ops in 'struct eth_dev_ops'. The idea came from ioctl. Because about flow director feature, there is large gap between i40e and ixgbe. The existed flow director APIs looks specific to IXGBE, so I choose this new API to implement i40e's flow director feature. Here, I briefly describe how the new 'rx_classification_filter_ctl' works: The API is like below: typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev, enum rte_eth_command cmd, void *arg); Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which contains the definition about structures specific to i40e, linked to the arg parameters above. Define a head file called rte_eth_features.h in lib/librte_ether, which contains the commands' definition linked to cmd parameters above. And if user want to use i40e specific features, then the head file rte_i40e.h need to be included user's application, for example, test-pmd. And user can encode these structures and call XXX_ctl API to configure their features. Do you think it make sense? And about the rename things, I will move it to a separate patch. > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, August 27, 2014 10:23 PM > To: Wu, Jingjing > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API > rx_classification_filter_ctl > > Hi Jingjing, > > 2014-08-27 10:13, Jingjing Wu: > > support a new ethdev API rx_classification_filter_ctl for all > > the configuration or queries for receive classification filters. > > this patch supports commands the API used below: > > RTE_CMD_FDIR_RULE_ADD > > RTE_CMD_FDIR_RULE_DEL > > RTE_CMD_FDIR_FLUSH > > RTE_CMD_FDIR_INFO_GET > > Could you explain why existing API (flow director + filters) is not enough? > I'd really like to see a common API for all filtering stuff. > > > -/* for 40G only */ > > -#define ETH_RSS_NONF_IPV4_UDP_SHIFT 31 > > -#define ETH_RSS_NONF_IPV4_TCP_SHIFT 33 > > -#define ETH_RSS_NONF_IPV4_SCTP_SHIFT 34 > > -#define ETH_RSS_NONF_IPV4_OTHER_SHIFT 35 > > -#define ETH_RSS_FRAG_IPV4_SHIFT 36 > > -#define ETH_RSS_NONF_IPV6_UDP_SHIFT 41 > > -#define ETH_RSS_NONF_IPV6_TCP_SHIFT 43 > > -#define ETH_RSS_NONF_IPV6_SCTP_SHIFT 44 > > -#define ETH_RSS_NONF_IPV6_OTHER_SHIFT 45 > > -#define ETH_RSS_FRAG_IPV6_SHIFT 46 > > -#define ETH_RSS_FCOE_OX_SHIFT 48 > > -#define ETH_RSS_FCOE_RX_SHIFT 49 > > -#define ETH_RSS_FCOE_OTHER_SHIFT 50 > > -#define ETH_RSS_L2_PAYLOAD_SHIFT 63 > > +/* Packet Classification Type for 40G only */ > > +#define ETH_PCTYPE_NONF_IPV4_UDP 31 > > +#define ETH_PCTYPE_NONF_IPV4_TCP 33 > > +#define ETH_PCTYPE_NONF_IPV4_SCTP 34 > > +#define ETH_PCTYPE_NONF_IPV4_OTHER35 > > +#define ETH_PCTYPE_FRAG_IPV4 36 > > +#define ETH_PCTYPE_NONF_IPV6_UDP 41 > > +#define ETH_PCTYPE_NONF_IPV6_TCP 43 > > +#define ETH_PCTYPE_NONF_IPV6_SCTP 44 > > +#define ETH_PCTYPE_NONF_IPV6_OTHER45 > > +#define ETH_PCTYPE_FRAG_IPV6 46 > > +#define ETH_PCTYPE_FCOE_OX48 /* not used */ > > +#define ETH_PCTYPE_FCOE_RX49 /* not used */ > > +#define ETH_PCTYPE_FCOE_OTHER 50 /* not used */ > > +#define ETH_PCTYPE_L2_PAYLOAD 63 > > Why is it specific to i40e? Could we have something generic? > Please take care at having only generic things in librte_ether. > > By the way, these renamings should be in a separated patch. > > -- > Thomas
[dpdk-dev] [PATCH v2 5/7] fix the Marco conflict
Hi, Thomas
Because these macros such as IPPROTO_TCP, IPPROTO_UDP are already defined in
. If user's application include and rte_ip.h at
the same time, there will be conflict error, for example cmdline.c in testpmd.
I remember there was someone also raised this issue few month ago.
So just use the way "#ifndef #endif" to avoid the conflict. And it is exactly
workaround as you said.
Oh, it should be macro, but not marco, my spelling mistake.
Sorry for that. I will rename this.
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Wednesday, August 27, 2014 10:28 PM
> To: Wu, Jingjing
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict
>
> 2014-08-27 10:13, Jingjing Wu:
> > fix the Marco conflict between rte_ip.h and netinet/in.h
>
> Who is Marco?
>
> > +#ifndef _NETINET_IN_H
> > +#ifndef _NETINET_IN_H_
> > /* IPv4 protocols */
> > #define IPPROTO_IP 0 /**< dummy for IP */
> > #define IPPROTO_HOPOPTS0 /**< IP6 hop-by-hop options */
> > @@ -226,7 +228,8 @@ struct ipv4_hdr {
> > #define IPPROTO_DIVERT 254 /**< divert pseudo-protocol */
> > #define IPPROTO_RAW 255 /**< raw IP packet */
> > #define IPPROTO_MAX 256 /**< maximum protocol number */
> > -
> > +#endif /* _NETINET_IN_H_ */
> > +#endif /* _NETINET_IN_H */
>
> Please explain your "fix" (which seems to be a workaround).
>
> --
> Thomas
[dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support
Hi, Thomas > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Wednesday, August 27, 2014 10:36 PM > To: Wu, Jingjing > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config > functions for > i40e flow director support > > Hi Jingjing, > > 2014-08-27 10:13, Jingjing Wu: > > add structure definition to construct programming packet. > > What is a "programming packet"? For Fortville, we need to set a flow director filter by sending a packet which contains the input set values through the queue belonging to flow director. > > > +#ifdef RTE_LIBRTE_I40E_PMD > > + "i40e_flow_director_filter (port_id) (add|del)" > > + " flow (ip4|ip6) src (src_ip_address) dst > > (dst_ip_address)" > > + " flexwords (flexwords_value) (drop|fwd)" > > + " queue (queue_id) fd_id (fd_id_value)\n" > > + "Add/Del a IP type flow director filter for i40e > > NIC.\n\n" > > + > > + "i40e_flow_director_filter (port_id) (add|del)" > > + " flow (udp4|tcp4|udp6|tcp6)" > > + " src (src_ip_address) (src_port)" > > + " dst (dst_ip_address) (dst_port)" > > + " flexwords (flexwords_value) (drop|fwd)" > > + " queue (queue_id) fd_id (fd_id_value)\n" > > + "Add/Del a UDP/TCP type flow director filter for > > i40e NIC.\n\n" > > + > > + "i40e_flush_flow_diretor (port_id)\n" > > + "Flush all flow director entries of a device on > > i40e NIC.\n\n" > > +#endif /* RTE_LIBRTE_I40E_PMD */ > > I'd really like to stop seeing this kind of thing. > We cannot add some ifdef for each PMD in generic code. > > I stopped reading after that. > > Sorry, I don't want to be rude but my feeling is that adding such feature > with global picture in mind is not easy. I know you want to offer all i40e > capabilities but you should think at future evolutions and how other drivers > will be integrated with yours. > Sorry to make you feel uncomfortable for such code. Just as you say, I want to offer more i40e capabilities. I will rework code in testpmd. > Thanks > -- > Thomas
[dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support
Hi, Thomas Thanks for your tips. I have another question: If we use the way 'rx_classification_filter_ctl' works, the specific structures defined in rte_i40e.h will be visible in user's application, such as testpmd. I know I shouldn't make commands linked with i40e like what I did before. But will the i40e specific structures become visible be acceptable? > -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, August 28, 2014 4:51 PM > To: Wu, Jingjing > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config > functions for > i40e flow director support > > 2014-08-28 03:51, Wu, Jingjing: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > 2014-08-27 10:13, Jingjing Wu: > > > > add structure definition to construct programming packet. > > > > > > What is a "programming packet"? > > > > For Fortville, we need to set a flow director filter by sending a > > packet which contains the input set values through the queue > > belonging to flow director. > > OK. To be more clear, some detailed explanations are required in the > commit log. Please try to be very descriptive. > You can think comments like this: > - if comments are absolutely needed to understand the code, you should > put comments in the code (or write the code differently) > - if some feature context can help for the review, you should explain > context and design in the commit log > > > > > +#ifdef RTE_LIBRTE_I40E_PMD > > > > + "i40e_flow_director_filter (port_id) (add|del)" > > > > + " flow (ip4|ip6) src (src_ip_address) dst > > > > (dst_ip_address)" > > > > + " flexwords (flexwords_value) (drop|fwd)" > > > > + " queue (queue_id) fd_id (fd_id_value)\n" > > > > + "Add/Del a IP type flow director filter for > > > > i40e NIC.\n\n" > > > > + > > > > + "i40e_flow_director_filter (port_id) (add|del)" > > > > + " flow (udp4|tcp4|udp6|tcp6)" > > > > + " src (src_ip_address) (src_port)" > > > > + " dst (dst_ip_address) (dst_port)" > > > > + " flexwords (flexwords_value) (drop|fwd)" > > > > + " queue (queue_id) fd_id (fd_id_value)\n" > > > > + "Add/Del a UDP/TCP type flow director > > > > filter for i40e NIC.\n\n" > > > > + > > > > + "i40e_flush_flow_diretor (port_id)\n" > > > > + "Flush all flow director entries of a > > > > device on i40e NIC.\n\n" > > > > +#endif /* RTE_LIBRTE_I40E_PMD */ > > > > > > I'd really like to stop seeing this kind of thing. > > > We cannot add some ifdef for each PMD in generic code. > > > > > > I stopped reading after that. > > > > > > Sorry, I don't want to be rude but my feeling is that adding such feature > > > with global picture in mind is not easy. I know you want to offer all i40e > > > capabilities but you should think at future evolutions and how other > > > drivers > > > will be integrated with yours. > > > > > > > Sorry to make you feel uncomfortable for such code. Just as you say, > > I want to offer more i40e capabilities. I will rework code in testpmd. > > Thanks > -- > Thomas
[dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl
Hi, Thomas
Please see my comments below.
Thanks a lot.
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Thursday, August 28, 2014 6:56 PM
> To: Wu, Jingjing
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API
> rx_classification_filter_ctl
>
> 2014-08-28 03:30, Wu, Jingjing:
> > We want to implement several common API for NIC specific features,
> > to avoid creating quite a lot of ops in 'struct eth_dev_ops'.
> > The idea came from ioctl.
>
> The approach can be interesting.
Yes, we have discussed this approach inside. And some other Fortville
Features are also based on it, such as RSS hash function support,
mac vlan filters.
Maybe it a good chance to discuss in open forum now.
>
> > Because about flow director feature, there is large gap between i40e
> > and ixgbe. The existed flow director APIs looks specific to IXGBE,
> > so I choose this new API to implement i40e's flow director feature.
>
> The API is not OK for you so you create another one.
> I'm OK to change APIs but you should remove the old one, or at least,
> implement your new API in existing drivers to allow deprecation of the
> old API.
> I think it would help if you start by doing ixgbe work and then apply it
> to i40e.
>
Yes, it will be perfect if we can use this new API to achieve flow director
setting all types of NICs. But the concern is downward compatibility.
Users who is planning update DPDK version need to change their code
to adapt such changes.
That's why we choose a new API instead of modifying current APIs. And
Of course, the ideal plan is adding such XXX_ctl function in Ixgbe and
Igb to moving smoothly without removing current APIs.
I'm not sure whether I understanding correctly about the importance of
compatibility.
> > The API is like below:
> > typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev,
> > enum rte_eth_command cmd,
> > void *arg);
> > Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which contains
> > the definition about structures specific to i40e, linked to the arg
> > parameters above.
> > Define a head file called rte_eth_features.h in lib/librte_ether, which
> > contains the commands' definition linked to cmd parameters above.
>
> Why creating a rte_eth_features.h? Don't you think rte_ethdev.h is a good
> place?
>
> > And if user want to use i40e specific features, then the head file
> > rte_i40e.h need to be included user's application, for example, test-pmd.
> > And user can encode these structures and call XXX_ctl API to configure
> > their features.
>
> OK, but the question is to know what is a specific feature?
> I don't think flow director is a specific feature. We shouldn't have
> to care if port is i40e or ixgbe to setup flow director.
> Is it possible to have a common API and maybe an inheritance of the
> common structure with PMD specific fields?
>
Yes, flow director is not a specific feature. Even ixgbe and i40 use the same
name. But the context and key have much difference. That's why I called it
specific.
> Example:
>
> struct fdir_entry {
> fdir_input input;
> fdir_action action;
> enum rte_driver driver;
> };
> fdir_entry_generic fdir_entry = {.driver = RTE_DRIVER_GENERIC};
> filter_ctl(port, FDIR_RULE_ADD, fdir_entry);
>
> struct i40e_fdir_entry {
> struct fdir_entry generic;
> i40e_fdir_input i40e_input;
> };
>
> So i40e_input will be handled by the PMD if driver == RTE_DRIVER_I40E.
>
> It's just an idea, comments are welcome.
Yes, it's a good idea about an inheritance of the common structure. I think it
may support new NIC integration in future. We can do it with the new API
architecture. But the concern is still how to be compatible with old version.
> --
> Thomas
[dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl
Hi, Konstantin
Thanks a lot.
> -Original Message-
> From: Ananyev, Konstantin
> Sent: Thursday, August 28, 2014 7:49 PM
> To: Thomas Monjalon; Wu, Jingjing
> Cc: dev at dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API
> rx_classification_filter_ctl
>
>
>
> > -Original Message-
> > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Thursday, August 28, 2014 11:56 AM
> > To: Wu, Jingjing
> > Cc: dev at dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API
> rx_classification_filter_ctl
> >
> > 2014-08-28 03:30, Wu, Jingjing:
> > > We want to implement several common API for NIC specific features,
> > > to avoid creating quite a lot of ops in 'struct eth_dev_ops'.
> > > The idea came from ioctl.
> >
> > The approach can be interesting.
> >
> > > Because about flow director feature, there is large gap between i40e
> > > and ixgbe. The existed flow director APIs looks specific to IXGBE,
> > > so I choose this new API to implement i40e's flow director feature.
> >
> > The API is not OK for you so you create another one.
> > I'm OK to change APIs but you should remove the old one, or at least,
> > implement your new API in existing drivers to allow deprecation of the
> > old API.
> > I think it would help if you start by doing ixgbe work and then apply it
> > to i40e.
> >
> > > The API is like below:
> > > typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev,
> > > enum rte_eth_command cmd,
> > > void *arg);
> > > Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which
> > > contains
> > > the definition about structures specific to i40e, linked to the arg
> > > parameters above.
> > > Define a head file called rte_eth_features.h in lib/librte_ether, which
> > > contains the commands' definition linked to cmd parameters above.
> >
> > Why creating a rte_eth_features.h? Don't you think rte_ethdev.h is a good
> > place?
>
> As I remember the long term idea was
> (Jingjing please correct me, if I am wrong here):
>
> Keep rte_ethdev.h for generic API.
> Put features specific for different NICs into rte_eth_features.h
> To make things clearer and avoid polluting of rte_ethdev.h.
>
> Provide API for the upper layer to query list of specific features(commands)
> supported by the
> underlying device.
> Something like:
> rte_eth_dev_get_rx_filter_supported(port, ...);
Yes, in helin's patch "[dpdk-dev] [PATCH v2 2/6] ethdev: add new ops of
'is_command_supported' and 'rx_classification_filter_ctl'", he already
defined rte_eth_dev_is_command_supported. It can be used to check
whether such command can be supported by the queried port.
Actually, some features are based on this architecture, including helin's
"Support configuring hash functions" and other non-ready patch.
We supposed after any patch of ours is applied, others need to rework
then. We are using the same approach.
> And ioctl-type API to configure HW specific features:
> rte_eth_dev_rx_classification_filter_ctl(port, cmd, cmd_spedific_arg);
>
> So, instead of application knows in advance "which specific NIC it is using",
> application would query which features/commannds the NIC provides and then
> configure
> them appropriately.
>
> >
> > > And if user want to use i40e specific features, then the head file
> > > rte_i40e.h need to be included user's application, for example, test-pmd.
> > > And user can encode these structures and call XXX_ctl API to configure
> > > their features.
> >
> > OK, but the question is to know what is a specific feature?
> > I don't think flow director is a specific feature. We shouldn't have
> > to care if port is i40e or ixgbe to setup flow director.
> > Is it possible to have a common API and maybe an inheritance of the
> > common structure with PMD specific fields?
> >
> > Example:
> >
> > struct fdir_entry {
> > fdir_input input;
> > fdir_action action;
> > enum rte_driver driver;
> > };
> > fdir_entry_generic fdir_entry = {.driver = RTE_DRIVER_GENERIC};
> > filter_ctl(port, FDIR_RULE_ADD, fdir_entry);
> >
> > struct i40e_fdir_entry {
> > struct fdir_entry generic;
> > i40e_fdir_input i40e_input;
> > };
> >
> > So i40e_input will be handled by the PMD if driver == RTE_DRIVER_I40E.
> >
> > It's just an idea, comments are welcome.
> >
> > --
> > Thomas
[dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, August 28, 2014 10:21 PM > To: Wu, Jingjing > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API > rx_classification_filter_ctl > > 2014-08-28 13:39, Wu, Jingjing: > > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > > > I'm OK to change APIs but you should remove the old one, or at least, > > > implement your new API in existing drivers to allow deprecation of the > > > old API. > > > I think it would help if you start by doing ixgbe work and then apply it > > > to i40e. > > > > > > > Yes, it will be perfect if we can use this new API to achieve flow director > > setting all types of NICs. But the concern is downward compatibility. > > In this case, cleanup is more important than compatibility. > > > Users who is planning update DPDK version need to change their code > > to adapt such changes. > > Yes, but we can keep deprecated function during 1 release. > > > That's why we choose a new API instead of modifying current APIs. And > > Of course, the ideal plan is adding such XXX_ctl function in Ixgbe and > > Igb to moving smoothly without removing current APIs. > > Yes > > > > I don't think flow director is a specific feature. We shouldn't have > > > to care if port is i40e or ixgbe to setup flow director. > > > Is it possible to have a common API and maybe an inheritance of the > > > common structure with PMD specific fields? > > > > Yes, flow director is not a specific feature. Even ixgbe and i40 use the > > same > > name. But the context and key have much difference. That's why I called it > > specific. > > > > Yes, it's a good idea about an inheritance of the common structure. I think > > it > > may support new NIC integration in future. We can do it with the new API > > architecture. But the concern is still how to be compatible with old > > version. > > There is no compatibility blocker here. > If we can keep deprecated functions a while, we'll do. Otherwise, just go with > the new API. > I prefer we concentrate on good design rather than on compatibility. > OK, now I have a rough understanding about your opinion, I guess there will be lots of rework need to be done. I will try. Thanks for your explanation. > Thanks > -- > Thomas Thanks Jingjing
[dpdk-dev] [PATCH v2 5/7] fix the Marco conflict
> -Original Message- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Thursday, August 28, 2014 4:56 PM > To: Wu, Jingjing > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict > > 2014-08-28 03:39, Wu, Jingjing: > > Because these macros such as IPPROTO_TCP, IPPROTO_UDP are already > > defined in . If user's application include > > and rte_ip.h at the same time, there will be conflict error, for > > example cmdline.c in testpmd. > > Yes > > > I remember there was someone also raised this issue few month ago. > > Yes, and the question was: "should we totally remove these definitions"? > I think yes. > Yes, it will be clear. But it also provides a way to user who doesn't use netinet/in.h. > > So just use the way "#ifndef #endif" to avoid the conflict. > > But you didn't explain difference between _NETINET_IN_H and _NETINET_IN_H_. It is due to the different versions of in.h, some use _NETINET_IN_H_ to define the head file, while some use _NETINET_IN_H > > > And it is exactly workaround as you said. > > Yes, it's a workaround. > If rte_ip.h is included before netinet/in.h, I think there is still a problem. Yes, it's just workaround. As my understanding, in DPDK's source code System head files includes first should be as following. So I think it's OK then. > > -- > Thomas Can I send a separate patch for this? Because it has no strict relationship with flow director. Thanks Jingjing
[dpdk-dev] [PATCH v2] i40e: fix of compile error
> -Original Message-
> From: Zhang, Helin
> Sent: Wednesday, December 03, 2014 9:13 AM
> To: dev at dpdk.org
> Cc: Cao, Waterman; Cao, Min; Wu, Jingjing; Zhang, Helin
> Subject: [PATCH v2] i40e: fix of compile error
>
> The compile error will occur as below when set
> 'RTE_LIBRTE_I40E_16BYTE_RX_DESC=y'.
> 'fd_id' should be used to replace 'fd', as 'fd' is not defined in that
> structure at
> all. In addition, local variable of 'flexbl' and 'flexbh' must be used only if
> 32 bytes RX descriptor is selected.
>
> error logs:
> lib/librte_pmd_i40e/i40e_rxtx.c: In function i40e_rxd_build_fdir:
> lib/librte_pmd_i40e/i40e_rxtx.c:431:28: error: volatile union
> has no member named fd
> lib/librte_pmd_i40e/i40e_rxtx.c:427:19: error: unused variable flexbl [-
> Werror=unused-variable]
> lib/librte_pmd_i40e/i40e_rxtx.c:427:11: error: unused variable flexbh [-
> Werror=unused-variable]
>
> Signed-off-by: Helin Zhang
> ---
> lib/librte_pmd_i40e/i40e_rxtx.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> v2 changes:
> * Removed the changes for code style fix, and kept it for compile error fix
> only.
> * Re-word the commit logs.
>
> diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
> index 2d2ef04..63c872d 100644
> --- a/lib/librte_pmd_i40e/i40e_rxtx.c
> +++ b/lib/librte_pmd_i40e/i40e_rxtx.c
> @@ -424,13 +424,9 @@ static inline uint64_t i40e_rxd_build_fdir(volatile
> union i40e_rx_desc *rxdp, struct rte_mbuf *mb) {
> uint64_t flags = 0;
> +#ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
> uint16_t flexbh, flexbl;
>
> -#ifdef RTE_LIBRTE_I40E_16BYTE_RX_DESC
> - mb->hash.fdir.hi =
> - rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd);
> - flags |= PKT_RX_FDIR_ID;
> -#else
> flexbh = (rte_le_to_cpu_32(rxdp->wb.qword2.ext_status) >>
> I40E_RX_DESC_EXT_STATUS_FLEXBH_SHIFT) &
> I40E_RX_DESC_EXT_STATUS_FLEXBH_MASK;
> @@ -453,6 +449,10 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc
> *rxdp, struct rte_mbuf *mb)
> rte_le_to_cpu_32(rxdp-
> >wb.qword3.lo_dword.flex_bytes_lo);
> flags |= PKT_RX_FDIR_FLX;
> }
> +#else
> + mb->hash.fdir.hi =
> + rte_le_to_cpu_32(rxdp->wb.qword0.hi_dword.fd_id);
> + flags |= PKT_RX_FDIR_ID;
> #endif
> return flags;
> }
> --
> 1.9.3
Acked-by: Jingjing Wu
[dpdk-dev] [PATCH 0/4] Integrate ethertype filter in igb/ixgbe driver to new API
Hi, Michael It's a long discuss in community. Due to in the development in i40e driver, we defined a new common API used for kinds of filters. In R1.8, because of time limit and compatibility, we just used the new API for i40e driver. While other driver still use old ones. We have planned to integrate filter to this new API to make the APIs generic for different types of NICs. Jingjing > -Original Message- > From: Qiu, Michael > Sent: Thursday, December 25, 2014 11:27 AM > To: Wu, Jingjing; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 0/4] Integrate ethertype filter in igb/ixgbe > driver to new API > > Hi Jingjing, > > Would you mind to tell me why need new APIs? Any functional or > performance increase? > Better to state in commit log. > > You know it should be careful to change APIs, especially for user interface. > > Thanks, > Michael > On 12/25/2014 11:14 AM, Jingjing Wu wrote: > > The patch set uses new filter_ctrl API to replace old ethertype filter APIs. > > It uses new functions and structure to replace old ones in igb/ixgbe > > driver, new commands to replace old ones in testpmd, and removes the > old APIs. > > > > Jingjing Wu (4): > > ixgbe: new functions replaces old ones for ethertype filters > > e1000: new functions replaces old ones for ethertype filters > > testpmd: new commands for ethertype filter > > ethdev: remove old APIs and structures of ethertype filters > > > > app/test-pmd/cmdline.c | 253 -- > > app/test-pmd/config.c | 27 --- > > lib/librte_ether/rte_ethdev.c | 57 -- > > lib/librte_ether/rte_ethdev.h | 88 - > > lib/librte_pmd_e1000/e1000_ethdev.h | 13 ++ > > lib/librte_pmd_e1000/igb_ethdev.c | 332 +- > --- > > lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 354 > > +++- > > lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 13 ++ > > 8 files changed, 579 insertions(+), 558 deletions(-) > >
[dpdk-dev] [PATCH v6 5/6] enicpmd: DPDK-ENIC PMD interface
Hi, ssujith
> + .tx_queue_release = enicpmd_dev_tx_queue_release,
> + .dev_led_on = NULL,
> + .dev_led_off = NULL,
> + .flow_ctrl_get= NULL,
> + .flow_ctrl_set= NULL,
> + .priority_flow_ctrl_set = NULL,
> + .mac_addr_add = enicpmd_add_mac_addr,
> + .mac_addr_remove = enicpmd_remove_mac_addr,
> + .fdir_add_signature_filter= NULL,
> + .fdir_update_signature_filter = NULL,
> + .fdir_remove_signature_filter = NULL,
> + .fdir_infos_get = enicpmd_fdir_info_get,
> + .fdir_add_perfect_filter = enicpmd_fdir_add_perfect_filter,
> + .fdir_update_perfect_filter = enicpmd_fdir_add_perfect_filter,
> + .fdir_remove_perfect_filter = enicpmd_fdir_remove_perfect_filter,
> + .fdir_set_masks = NULL,
> +};
> +
I found that in perfect fdir is also supported in enic driver.
During the R1.8 development, we defined a new dev_ops call filter_ctrl, which
can be used to control kinds of filters, flow director is included too. Which
is mentioned in http://www.dpdk.org/ml/archives/dev/2014-September/005179.html
.
In R1.8, filter_ctrl is only used by i40e driver. And we also planned use it in
the existing ixgbe/e1000 driver in the next days. The old APIs such as
fdir_add_perfect_filter, fdir_remove_perfect_filter can be replaced then.
So, do you have any plan to migrate the fdir in enic to the filter_ctrl API?
Jingjing
Thanks!
> +struct enic *enicpmd_list_head = NULL;
> +/* Initialize the driver
> + * It returns 0 on success.
> + */
> +static int eth_enicpmd_dev_init(
> + __attribute__((unused))struct eth_driver *eth_drv,
> + struct rte_eth_dev *eth_dev)
> +{
> + struct rte_pci_device *pdev;
> + struct rte_pci_addr *addr;
> + struct enic *enic = pmd_priv(eth_dev);
> +
> + ENICPMD_FUNC_TRACE();
> +
> + enic->rte_dev = eth_dev;
> + eth_dev->dev_ops = &enicpmd_eth_dev_ops;
> + eth_dev->rx_pkt_burst = &enicpmd_recv_pkts;
> + eth_dev->tx_pkt_burst = &enicpmd_xmit_pkts;
> +
> + pdev = eth_dev->pci_dev;
> + enic->pdev = pdev;
> + addr = &pdev->addr;
> +
> + snprintf(enic->bdf_name, ENICPMD_BDF_LENGTH,
> "%04x:%02x:%02x.%x",
> + addr->domain, addr->bus, addr->devid, addr->function);
> +
> + return enic_probe(enic);
> +}
> +
> +static struct eth_driver rte_enic_pmd = {
> + {
> + .name = "rte_enic_pmd",
> + .id_table = pci_id_enic_map,
> + .drv_flags = RTE_PCI_DRV_NEED_MAPPING,
> + },
> + .eth_dev_init = eth_enicpmd_dev_init,
> + .dev_private_size = sizeof(struct enic), };
> +
> +/* Driver initialization routine.
> + * Invoked once at EAL init time.
> + * Register as the [Poll Mode] Driver of Cisco ENIC device.
> + */
> +int rte_enic_pmd_init(const char *name __rte_unused,
> + const char *params __rte_unused)
> +{
> + ENICPMD_FUNC_TRACE();
> +
> + rte_eth_driver_register(&rte_enic_pmd);
> + return 0;
> +}
> +
> +static struct rte_driver rte_enic_driver = {
> + .type = PMD_PDEV,
> + .init = rte_enic_pmd_init,
> +};
> +
> +PMD_REGISTER_DRIVER(rte_enic_driver);
> +
> --
> 1.9.1
[dpdk-dev] symbol conflicts between netinet/in.h, arpa/inet.h, and rte_ip.h
Hello, We also notice these conflicts, we just planned to fix it in our new feature development. The proposal is like: #ifndef _NETINET_IN_H #ifndef _NETINET_IN_H_ #define IPPROTO_IP 0 ... ... #define IPPROTO_MAX 256 #endif #endif Do you think it is a good idea? > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Antti Kantee > Sent: Friday, July 25, 2014 6:56 AM > To: Matthew Hall; dev at dpdk.org > Subject: Re: [dpdk-dev] symbol conflicts between netinet/in.h, arpa/inet.h, > and rte_ip.h > > On 24/07/14 07:59, Matthew Hall wrote: > > Hello, > > > > I ran into some weird symbol conflicts between system netinet/in.h and DPDK > > rte_ip.h. They have a lot of duplicated definitions for stuff like > > IPPROTO_IP > > and so on. This breaks when you want to use inet_pton from arpa/inet.h, > > because it includes netinet/in.h to define struct in_addr. > > I would namespace the definitions in DPDK, i.e. make them > DPDK_IPPROTO_FOO etc. > > > Thus with all the conflicts it's impossible to use a DPDK IP struct instead > > of > > all the system's sockaddr stuff, to store a value from the system copy of > > inet_pton. This would be a common operation if, for example, you want to > > configure all the IP addresses on your box from a JSON file, which is what I > > was doing. > > > > The DPDK kludged around it internally by using a file called > > cmdline_parse_ipaddr.c with private copies of these functions. But it in my > > opinion very unwisely marked all of the functions as static except for > > cmdline_parse_ipaddr, which only works on the DPDK's proprietary argument > > handling, and not with anything the user might have which is a different > > format. > > In my experience from years of fighting with more or less this exact > same problem -- the fight is now thankfully over but the scars remain -- > you either want to expose a complete set of types and provide support > for everything, or you want to expose nothing. Approaches where you use > cute definitions and reuse some host routines is like asking for an > audience with Tyranthraxus when armed with a kitten. It's that doubly > so if you don't have to and do it anyway. > > > So, it would be a big help for users if the macros in librte_net files would > > check if the symbols already existed, or if they had subheader files > > available > > to grab only non conflicting symbols, or if they would make a proper .h and > > factor all the inet_pton and inet_ntop inside the cmdline lib into a place > > where users can access them. It would also be a help if they had a less ugly > > equivalent to struct sockaddr, which let you work with IP addresses a bit > > more > > easily, such as something like this: > > Again, I recommend steering away from any tightrope approaches that > "know" which types are non-conflicting, or pick out half-and-half from > the host and IP stack. "Do, or do not, there is no half-and-half"
[dpdk-dev] [PATCH v2 0/6] Support configuring hash functions
Reviewed-by: Jingjing Wu > -Original Message- > From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Helin Zhang > Sent: Monday, July 28, 2014 4:26 PM > To: dev at dpdk.org > Subject: [dpdk-dev] [PATCH v2 0/6] Support configuring hash functions > > These pathches mainly support configuring hash functions. > In detail, > - It can select Toeplitz or simple XOR hash functions. > - It can configure symmetric hash functions. >* Get/set symmetric hash enable per port. >* Get/set symmetric hash enable per 'PCTYPE'. >* Get/set filter swap configurations. > - 'ethdev' level interfaces are added. >* 'is_command_supported', to check if a feature (command) > is supported on a port. >* 'rx_classification_filter_ctl', a common API to execute > specific command of each feature. > - Seven commands are implemented in testpmd to support >testing above. > Note that 'PCTYPE' means 'Packet Classification Type'. > > Helin Zhang (6): > ethdev: rename macros of packet classification type > ethdev: add new ops of 'is_command_supported' and > 'rx_classification_filter_ctl' > i40e: support of 'rx_classification_filter_ctl' > i40e: support of 'is_command_supported' > i40e: Initialize hash function during port initialization. > app/testpmd: add commands for configuring hash functions > > app/test-pmd/cmdline.c | 579 > > lib/librte_ether/Makefile | 1 + > lib/librte_ether/rte_eth_features.h | 73 + > lib/librte_ether/rte_ethdev.c | 31 ++ > lib/librte_ether/rte_ethdev.h | 131 +--- > lib/librte_pmd_i40e/Makefile| 6 + > lib/librte_pmd_i40e/i40e_ethdev.c | 484 ++ > lib/librte_pmd_i40e/i40e_ethdev.h | 2 + > lib/librte_pmd_i40e/rte_i40e.h | 108 +++ > 9 files changed, 1377 insertions(+), 38 deletions(-) > create mode 100644 lib/librte_ether/rte_eth_features.h > create mode 100644 lib/librte_pmd_i40e/rte_i40e.h > > -- > 1.8.1.4
[dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter
Hi, Thomas Thanks for your reply. About your first concern: The pool field is used in virtualization scenario. It is acting as one of input set during filter matching in ixgbe. My patch didn't consider the virtualization scenario in generic filter feature. Because in 82599 datasheet, it is recommended to assign rx queues not used by DCB/RSS, that is virtualization without RSS and DCB mode. For this mode, current DPDK version makes the number of queue to 1 by default in IOV mode. So in this case it makes no sense make pool as a input set and the rx queue also need to be set to in this pool, so just keep the consistent with flow director who also ignore it in previous version. And further E1000/Niantic/Fortville have different definitions for VF, we need to think how to define it more generic. And even just need offer pool number in configuration of the filters as what Vladimir did, it also need to verify the interworking with Virtualization for different kinds of NICs, and the interworking with DCB and RSS which is not recommended in 82599's datasheet. So I think it will be a good choice to implement generic filter interworking with virtualization in future patch. If there is any volunteer to send patch for support this concern later, it will be also cool. About your second concern: I will send out a new version for that soon. -Original Message- From: Thomas Monjalon [mailto:thomas.monja...@6wind.com] Sent: Wednesday, June 11, 2014 11:45 PM To: Wu, Jingjing Cc: dev at dpdk.org; Vladimir Medvedkin Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter Hi Jingjing, Please reply below the question. 2014-05-28 01:12, Wu, Jingjing: > You are discussing whether the APIs provide for NIC filters is generic > or not. About that we can use same API for a type of filter. For > example, if we want to configure ethertype filter, we can use the same > API, no matter the NIC is 82580, i350, 82576 or 82599. We think these > NICs may be most common used. I was wondering if this API can apply to non-Intel devices. But nobody talked about it. So let's forget it. My main concern is that Vladimir Medvedkin suggested another API and I'd like you give your opinion about it: http://dpdk.org/ml/archives/dev/2014-June/003053.html It offers pool number in configuration of the filters. Last comment: patches would be more pleasant to read with right alignment and spaces in comments. This is an extract to illustrate what I'm talking about: + uint16_t priority; /**< used when more than one filter matches */ + uint8_t dst_port_mask : 1, /**
[dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter
Hi, Vladimir Thanks a lot for your remarks. Yes, your understanding is correct, in non-IOV mode, we can use 64pool, per pool can has 2 queues when ETH_MQ_RX_VMDQ_ONLY. While in IOV mode, current DPDK version makes the number of queue to 1 by default. The pools logic makes sense, but I didn?t consider it globally with the thinking we can do it in future. I will be great if you can generate a new patch based on mine. Or we can discuss it further? Due to it is close to the feature deliver time now and much verification work for it, it may not possible to add it in this patch. In API About your first remark, the reason why I didn?t put the queue in the filter structure is that the filter contains the fields used for comparison and the queue is acted as result, and another concern is to keep consistent with flow director?s implementation. About your second remark, I will accept it and integrate the change to patch in new version. Do your agree my proposal? From: Vladimir Medvedkin [mailto:medvedk...@gmail.com] Sent: Friday, June 13, 2014 7:52 PM To: Thomas Monjalon Cc: Wu, Jingjing; dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter Hi all, The 82599 datasheet (p. 284 and p.287) has only recommendations and only when possible about assign rx queue not used by RSS/DCB. I do not see any serious restrictions do not assign the rx queue used by RSS/DCB. For cases with only 1 queue if I understand correctly this patch http://dpdk.org/ml/archives/dev/2014-May/002589.html we can init second queue in pool and assign it by filter. In ETH_MQ_RX_VMDQ_ONLY mode init all possible queues (even if hardware route packets to zero queue in pools) so there no problem. Moreover, it is not necesary for rx queue to be set in the same pool. About genericity. I agree with Jingjing, different controllers have different definitions for pools or VFs. And it is only Intel controllers! It is very hard to predict hardware implementation. For example for Fortville I can not find 5-tuple filters at all. API. I have several remarks. 1. You use rx_queue as separate arg. For example: rte_eth_dev_add_ethertype_filter(uint8_t port_id, uint16_t index, struct rte_ethertype_filter *filter, uint8_t rx_queue) rte_eth_dev_get_ethertype_filter(uint8_t port_id, uint16_t index, struct rte_ethertype_filter *filter, uint8_t *rx_queue) you can move uint8_t rx_queue into struct rte_ethertype_filter *filter. 2. In SYN filter: rte_eth_dev_add_syn_filter(uint8_t port_id, uint8_t high_pri, uint8_t rx_queue) rte_eth_dev_get_syn_filter(uint8_t port_id, struct rte_syn_filter *filter, uint8_t *rx_queue) In first ADD func you alloc struct rte_syn_filter inside func, but in GET func you have to alloc struct rte_syn_filter in your app. May be better to do rte_eth_dev_add_syn_filter(uint8_t port_id, struct rte_syn_filter *filter, uint8_t *rx_queue) ? So, Jingjing made a lot of work, much more then I (igb filters, testpmd commands). It works the same as mine (not counting pools logic), so let's integrate it (it's will be great if jingjing change api according to my remarks). Regards, Vladimir 2014-06-12 19:36 GMT+04:00 Thomas Monjalon mailto:thomas.monjalon at 6wind.com>>: > 2014-06-11 17:45, Thomas Monjalon: > > My main concern is that Vladimir Medvedkin suggested another API and I'd > > like you give your opinion about it: > > http://dpdk.org/ml/archives/dev/2014-June/003053.html > > It offers pool number in configuration of the filters. 2014-06-12 08:08, Wu, Jingjing: > The pool field is used in virtualization scenario. It is acting as one of > input set during filter matching in ixgbe. > My patch didn't consider the virtualization scenario in generic filter > feature. Because in 82599 datasheet, it is recommended to assign rx queues > not used by DCB/RSS, that is virtualization without RSS and DCB mode. For > this mode, current DPDK version makes the number of queue to 1 by default in > IOV mode. So in this case it makes no sense make pool as a input set and the > rx queue also need to be set to in this pool, so just keep the consistent > with flow director who also ignore it in previous version. > And further E1000/Niantic/Fortville have different definitions for VF, we > need to think how to define it more generic. > And even just need offer pool number in configuration of the filters as what > Vladimir did, it also need to verify the interworking with Virtualization > for different kinds of NICs, and the interworking with DCB and RSS which is > not recommended in 82599's datasheet. > So I think it will be a good choice to implement generic filter interworking > with virtualization in future patch. If there is any volunteer to send patch > for support this concern later, it will be also cool. Vladimir, do you agree with this analysis? As you suggested another implementation, I need you acknowledgment for this patchset to be integrated. Thanks -- Thomas
[dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter
Hi, Vladimir Yes, for Fortville, uint8_t is not enough, it was also the concern is to keep consistent with flow director?s implementation. But I agree that we need to change. Let make an agreement like: I will make change for the remarks from you. One is the change the type of rx_queue to uint16_t. The other is change API like ?rte_eth_dev_add_syn_filter(uint8_t port_id, struct rte_syn_filter *filter, uint16_t rx_queue)?. And about the pool and virtualization case, maybe you will send a new patch about it, maybe me. Whatever, just leave it in future, not include in this patch. Thank you! Jingjing From: Vladimir Medvedkin [mailto:medvedk...@gmail.com] Sent: Saturday, June 14, 2014 12:19 AM To: Wu, Jingjing Cc: Thomas Monjalon; dev at dpdk.org Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter Hi Jingjing, Yes, I agree. I have one more remark. It is about type of rx_queue arg. Now it is uint8_t. I think we have to change it to uint16_t because for Fortville NIC it is not enough. Quote fro the datasheet: A PF VSI (Virtual Station Interfaces aka virtual NICs) can allocate and use up to 1536 LQPs (LAN queue pairs). Regards, Vladimir 2014-06-13 18:12 GMT+04:00 Wu, Jingjing mailto:jingjing.wu at intel.com>>: Hi, Vladimir Thanks a lot for your remarks. Yes, your understanding is correct, in non-IOV mode, we can use 64pool, per pool can has 2 queues when ETH_MQ_RX_VMDQ_ONLY. While in IOV mode, current DPDK version makes the number of queue to 1 by default. The pools logic makes sense, but I didn?t consider it globally with the thinking we can do it in future. I will be great if you can generate a new patch based on mine. Or we can discuss it further? Due to it is close to the feature deliver time now and much verification work for it, it may not possible to add it in this patch. In API About your first remark, the reason why I didn?t put the queue in the filter structure is that the filter contains the fields used for comparison and the queue is acted as result, and another concern is to keep consistent with flow director?s implementation. About your second remark, I will accept it and integrate the change to patch in new version. Do your agree my proposal? From: Vladimir Medvedkin [mailto:medvedkinv at gmail.com<mailto:medvedk...@gmail.com>] Sent: Friday, June 13, 2014 7:52 PM To: Thomas Monjalon Cc: Wu, Jingjing; dev at dpdk.org<mailto:dev at dpdk.org> Subject: Re: [dpdk-dev] [PATCH v2 0/4] NIC filters support for generic filter Hi all, The 82599 datasheet (p. 284 and p.287) has only recommendations and only when possible about assign rx queue not used by RSS/DCB. I do not see any serious restrictions do not assign the rx queue used by RSS/DCB. For cases with only 1 queue if I understand correctly this patch http://dpdk.org/ml/archives/dev/2014-May/002589.html we can init second queue in pool and assign it by filter. In ETH_MQ_RX_VMDQ_ONLY mode init all possible queues (even if hardware route packets to zero queue in pools) so there no problem. Moreover, it is not necesary for rx queue to be set in the same pool. About genericity. I agree with Jingjing, different controllers have different definitions for pools or VFs. And it is only Intel controllers! It is very hard to predict hardware implementation. For example for Fortville I can not find 5-tuple filters at all. API. I have several remarks. 1. You use rx_queue as separate arg. For example: rte_eth_dev_add_ethertype_filter(uint8_t port_id, uint16_t index, struct rte_ethertype_filter *filter, uint8_t rx_queue) rte_eth_dev_get_ethertype_filter(uint8_t port_id, uint16_t index, struct rte_ethertype_filter *filter, uint8_t *rx_queue) you can move uint8_t rx_queue into struct rte_ethertype_filter *filter. 2. In SYN filter: rte_eth_dev_add_syn_filter(uint8_t port_id, uint8_t high_pri, uint8_t rx_queue) rte_eth_dev_get_syn_filter(uint8_t port_id, struct rte_syn_filter *filter, uint8_t *rx_queue) In first ADD func you alloc struct rte_syn_filter inside func, but in GET func you have to alloc struct rte_syn_filter in your app. May be better to do rte_eth_dev_add_syn_filter(uint8_t port_id, struct rte_syn_filter *filter, uint8_t *rx_queue) ? So, Jingjing made a lot of work, much more then I (igb filters, testpmd commands). It works the same as mine (not counting pools logic), so let's integrate it (it's will be great if jingjing change api according to my remarks). Regards, Vladimir 2014-06-12 19:36 GMT+04:00 Thomas Monjalon mailto:thomas.monjalon at 6wind.com>>: > 2014-06-11 17:45, Thomas Monjalon: > > My main concern is that Vladimir Medvedkin suggested another API and I'd > > like you give your opinion about it: > > http://dpdk.org/ml/archives/dev/2014-June/003053.html > > It offers pool number in configuration of the filters. 2014-06-12 08:08, Wu, Jingjing: > The pool field is used in virt
[dpdk-dev] enable mirror functionality in i40e driver
Due to the PATCH prefix is missed in the cover-letter, I will resend the patch set. Please just ignore this one. > -Original Message- > From: Wu, Jingjing > Sent: Wednesday, May 13, 2015 4:38 PM > To: dev at dpdk.org > Cc: Wu, Jingjing; Jiajia, SunX; Zhang, Helin; Liu, Jijiang > Subject: enable mirror functionality in i40e driver > > This patch set enables mirror functionality in i40e driver, and redefine > structure and macros used to configure mirror. > > Jingjing Wu (3): > ethdev: rename rte_eth_vmdq_mirror_conf > ethdev: redefine the mirror type > i40e: enable mirror functionality in i40e driver > > app/test-pmd/cmdline.c | 60 --- > lib/librte_ether/rte_ethdev.c | 28 ++- > lib/librte_ether/rte_ethdev.h | 30 ++-- > lib/librte_pmd_i40e/i40e_ethdev.c | 334 > > lib/librte_pmd_i40e/i40e_ethdev.h | 23 +++ > lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 72 +--- > lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 4 +- > 7 files changed, 464 insertions(+), 87 deletions(-) > > -- > 1.9.3
[dpdk-dev] filter_ctl PMD API idea
Hi, all
When we develop filters feature in i40e driver for Intel? Ethernet Controller
XL710/X710 [Fortville] (For both 10G/40G), we found that there are lots of new
filters, there are also some changes on the existing filters, comparing to
ixgbe.
If we keep the way to add new ops in rte_eth_dev for each new filter, it can
work.
But we suggest to use a more generic API for all filters to avoid a superset
dev_ops. It needs to be cleaner and easy-to-use. There is a need for technical
discussion.
Here is the early design idea we are looking for comments. ?
1.?? Create two new APIs
-
rte_eth_filter_supported(uint8_t port, uint16_t filter_type);
/* check whether this filter type is supported for the queried port */
rte_eth_filter_ctl(uint8_t port, uint16_t filter_type, uint16_t filter_op, void
*arg);
/* configure filters, will call new ops eth_filter_ctl in eth_dev_ops */
-
2.?? Define filter types, operations, and structures in new header file
lib/librte_eth/rte_eth_filter.h.
-
#define RTE_ETH_FILTER_RSS 1
#define RTE_ETH_FILTER_SYN 2
#define RTE_ETH_FILTER_5TUPLE 3
#define RTE_ETH_FILTER_FDIR 4
#define RTE_ETH_FILTER_OP_GET 1
#define RTE_ETH_FILTER_OP_ADD 2
#define RTE_ETH_FILTER_OP_DELETE3
#define RTE_ETH_FILTER_OP_SET 4
< other operations if want to define>...
/* structures defined for corresponding filter type and operation */
/* take RTE_ETH_FILTER_FDIR and OP_SET for example*/
struct rte_eth_filter_fdir_cfg {?
#define RTE_ETH_FILTER_FDIR_SET_MASK 0
#define RTE_ETH_FILTER_FDIR_SET_OFFSET 1
??
uint16_t cfg_type;
??/* sub operation to defined what specific configuration it will take,
???and which following fields are meaningful*/
??/* fields, can be a union or combine of required specific items*/
??
};
-
By this way, It is easy to add more filter types or operation in future.
And the difference among the same filter and operation can be distinguish by
sub command in defined structure, e.g. ?cfg_type? in above
rte_eth_filter_fdir_cfg structure.
3.?? Define ops in driver (take i40e for example)
-
static struct eth_dev_ops i40e_eth_dev_ops = {
. filter_ctl = i40e_filter_ctl,
};
-
Then the functions in drivers can be implemented separately.
4.?? Use case In test-pmd/cmdline.c
-
#include
/* add or change commands e.g. fdir_set (arg1) (arg2) ?? */
static void
cmd_fdir_parsed()
{
??
??/* take setting fdir mask for example*/
??struct rte_eth_filter_fdir_cfg cfg;
??if (rte_eth_filter_supported(port, RTE_ETH_FILTER_FDIR)) {
?? cfg.cfg_type = RTE_ETH_FILTER_FDIR_SET_MASK;
?? /* fill the corresponding fields in cfg*/
?? ??
?? rte_eth_filter_ctl(port, RTE_ETH_FILTER_FDIR, RTE_ETH_FILTER_OP_SET,
&cfg);
??}
??
}
-
Any comments are welcome!
At the time being, only Intel PMD is only available on dpdk.org. We are lack of
understanding on the other non-Intel PMD, the current design did not take them
into account. But we are looking for the inputs from those PMD developers, we
strongly look forward to those PMD are released as open source.
Thanks!
Jingjing
[dpdk-dev] filter_ctl PMD API idea
Any comments or advises?
Thanks!
Fortville Filter features' development will be started based on this design
this week.
> -Original Message-
> From: Wu, Jingjing
> Sent: Thursday, September 4, 2014 8:05 PM
> To: dev at dpdk.org
> Cc: stephen at networkplumber.org; vincent.jardin at 6wind.com
> Subject: filter_ctl PMD API idea
>
> Hi, all
>
> When we develop filters feature in i40e driver for Intel? Ethernet Controller
> XL710/X710
> [Fortville] (For both 10G/40G), we found that there are lots of new filters,
> there are also
> some changes on the existing filters, comparing to ixgbe.
> If we keep the way to add new ops in rte_eth_dev for each new filter, it can
> work.
> But we suggest to use a more generic API for all filters to avoid a superset
> dev_ops. It needs
> to be cleaner and easy-to-use. There is a need for technical discussion.
>
> Here is the early design idea we are looking for comments.
>
> 1.?? Create two new APIs
> -
> rte_eth_filter_supported(uint8_t port, uint16_t filter_type);
> /* check whether this filter type is supported for the queried port */
> rte_eth_filter_ctl(uint8_t port, uint16_t filter_type, uint16_t filter_op,
> void *arg);
> /* configure filters, will call new ops eth_filter_ctl in eth_dev_ops */
> -
>
> 2.?? Define filter types, operations, and structures in new header file
> lib/librte_eth/rte_eth_filter.h.
> -
> #define RTE_ETH_FILTER_RSS1
> #define RTE_ETH_FILTER_SYN2
> #define RTE_ETH_FILTER_5TUPLE 3
> #define RTE_ETH_FILTER_FDIR 4
>
>
> #define RTE_ETH_FILTER_OP_GET 1
> #define RTE_ETH_FILTER_OP_ADD 2
> #define RTE_ETH_FILTER_OP_DELETE 3
> #define RTE_ETH_FILTER_OP_SET 4
> < other operations if want to define>...
>
> /* structures defined for corresponding filter type and operation */
> /* take RTE_ETH_FILTER_FDIR and OP_SET for example*/
>
> struct rte_eth_filter_fdir_cfg {
> #define RTE_ETH_FILTER_FDIR_SET_MASK 0
> #define RTE_ETH_FILTER_FDIR_SET_OFFSET 1
> ??
> uint16_t cfg_type;
> ??/* sub operation to defined what specific configuration it will take,
> ???and which following fields are meaningful*/
>
> ??/* fields, can be a union or combine of required specific items*/
>
>
> };
>
> -
> By this way, It is easy to add more filter types or operation in future.
> And the difference among the same filter and operation can be distinguish by
> sub command
> in defined structure, e.g. ?cfg_type? in above rte_eth_filter_fdir_cfg
> structure.
>
> 3.?? Define ops in driver (take i40e for example)
> -
> static struct eth_dev_ops i40e_eth_dev_ops = {
> . filter_ctl = i40e_filter_ctl,
> };
> -
> Then the functions in drivers can be implemented separately.
>
> 4.?? Use case In test-pmd/cmdline.c
> -
> #include
> /* add or change commands e.g. fdir_set (arg1) (arg2) ?? */
>
> static void
> cmd_fdir_parsed()
> {
> ??
> ??/* take setting fdir mask for example*/
> ??struct rte_eth_filter_fdir_cfg cfg;
>
> ??if (rte_eth_filter_supported(port, RTE_ETH_FILTER_FDIR)) {
> ??cfg.cfg_type = RTE_ETH_FILTER_FDIR_SET_MASK;
> ??/* fill the corresponding fields in cfg*/
> ????
> ??rte_eth_filter_ctl(port, RTE_ETH_FILTER_FDIR, RTE_ETH_FILTER_OP_SET,
> &cfg);
> ??}
> ??
> }
> -
>
>
> Any comments are welcome!
>
> At the time being, only Intel PMD is only available on dpdk.org. We are lack
> of understanding
> on the other non-Intel PMD, the current design did not take them into
> account. But we are
> looking for the inputs from those PMD developers, we strongly look forward to
> those PMD
> are released as open source.
>
> Thanks!
> Jingjing
[dpdk-dev] [RFC PATCH] ethdev: remove old flow director API
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Monday, April 20, 2015 10:12 PM
> To: Wu, Jingjing
> Cc: dev at dpdk.org
> Subject: [RFC PATCH] ethdev: remove old flow director API
>
> It's time to remove this old API.
> It seems some work is still needed to rely only on eth_ctrl API.
> At least ixgbe, i40e and testpmd must be fixed.
> Jingjing, do you think it's possible to remove all these structures
> from rte_ethdev.h?
>
[Wu, Jingjing] Yes, I agree.
But few comments list below.
Beside the following change, some commands also need to be removed in testpmd.
For the ixgbe, code to the old APIs are already fixed.
> Thanks
>
> ---
> lib/librte_ether/rte_ethdev.c | 260 -
> lib/librte_ether/rte_ethdev.h | 399
> --
> lib/librte_pmd_enic/enic_ethdev.c | 1 -
> lib/librte_pmd_mlx4/mlx4.c| 7 -
> 4 files changed, 667 deletions(-)
>
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index e20cca5..65173e7 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -2098,266 +2098,6 @@ rte_eth_dev_set_vlan_pvid(uint8_t port_id, uint16_t
> pvid, int on)
> }
>
> int
> -rte_eth_dev_fdir_add_signature_filter(uint8_t port_id,
> - struct rte_fdir_filter *fdir_filter,
> - uint8_t queue)
> -{
> - struct rte_eth_dev *dev;
> -
> - if (!rte_eth_dev_is_valid_port(port_id)) {
> - PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> - return (-ENODEV);
> - }
> -
> - dev = &rte_eth_devices[port_id];
> -
> - if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_SIGNATURE) {
> - PMD_DEBUG_TRACE("port %d: invalid FDIR mode=%u\n",
> - port_id, dev->data->dev_conf.fdir_conf.mode);
> - return (-ENOSYS);
> - }
> -
> - if ((fdir_filter->l4type == RTE_FDIR_L4TYPE_SCTP
> - || fdir_filter->l4type == RTE_FDIR_L4TYPE_NONE)
> - && (fdir_filter->port_src || fdir_filter->port_dst)) {
> - PMD_DEBUG_TRACE(" Port are meaningless for SCTP and " \
> - "None l4type, source & destinations ports " \
> - "should be null!\n");
> - return (-EINVAL);
> - }
> -
> - FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fdir_add_signature_filter, -ENOTSUP);
> - return (*dev->dev_ops->fdir_add_signature_filter)(dev, fdir_filter,
> - queue);
> -}
> -
> -int
> -rte_eth_dev_fdir_update_signature_filter(uint8_t port_id,
> - struct rte_fdir_filter *fdir_filter,
> - uint8_t queue)
> -{
> - struct rte_eth_dev *dev;
> -
> - if (!rte_eth_dev_is_valid_port(port_id)) {
> - PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> - return (-ENODEV);
> - }
> -
> - dev = &rte_eth_devices[port_id];
> -
> - if (dev->data->dev_conf.fdir_conf.mode != RTE_FDIR_MODE_SIGNATURE) {
> - PMD_DEBUG_TRACE("port %d: invalid FDIR mode=%u\n",
> - port_id, dev->data->dev_conf.fdir_conf.mode);
> - return (-ENOSYS);
> - }
> -
> - if ((fdir_filter->l4type == RTE_FDIR_L4TYPE_SCTP
> - || fdir_filter->l4type == RTE_FDIR_L4TYPE_NONE)
> - && (fdir_filter->port_src || fdir_filter->port_dst)) {
> - PMD_DEBUG_TRACE(" Port are meaningless for SCTP and " \
> - "None l4type, source & destinations ports " \
> - "should be null!\n");
> - return (-EINVAL);
> - }
> -
> - FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fdir_update_signature_filter,
> -ENOTSUP);
> - return (*dev->dev_ops->fdir_update_signature_filter)(dev, fdir_filter,
> - queue);
> -
> -}
> -
> -int
> -rte_eth_dev_fdir_remove_signature_filter(uint8_t port_id,
> - struct rte_fdir_filter *fdir_filter)
> -{
> - struct rte_eth_dev *dev;
> -
> - if (!rte_eth_dev_is_valid_port(port_id)) {
> - PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> - return (-ENODEV);
> - }
> -
> -
[dpdk-dev] [RFC PATCH] ethdev: remove old flow director API
> -Original Message-
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> Sent: Monday, April 20, 2015 11:00 PM
> To: Wu, Jingjing
> Cc: dev at dpdk.org
> Subject: Re: [RFC PATCH] ethdev: remove old flow director API
>
> 2015-04-20 14:32, Wu, Jingjing:
> > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> > > It's time to remove this old API.
> > > It seems some work is still needed to rely only on eth_ctrl API.
> > > At least ixgbe, i40e and testpmd must be fixed.
> > > Jingjing, do you think it's possible to remove all these structures
> > > from rte_ethdev.h?
> > >
> > [Wu, Jingjing] Yes, I agree.
> > But few comments list below.
> > Beside the following change, some commands also need to be removed in
> > testpmd. For the
> ixgbe, code to the old APIs are already fixed.
>
> > > --- a/lib/librte_ether/rte_ethdev.h
> > > +++ b/lib/librte_ether/rte_ethdev.h
> > > @@ -672,42 +672,6 @@ struct rte_eth_pfc_conf {
> > > };
> > >
> > > /**
> > > - * Memory space that can be configured to store Flow Director filters
> > > - * in the board memory.
> > > - */
> > > -enum rte_fdir_pballoc_type {
> > > - RTE_FDIR_PBALLOC_64K = 0, /**< 64k. */
> > > - RTE_FDIR_PBALLOC_128K, /**< 128k. */
> > > - RTE_FDIR_PBALLOC_256K, /**< 256k. */
> > > -};
>
> This is hardware-related. Why should it be part of the API?
>
> > > -
> > > -/**
> > > - * Select report mode of FDIR hash information in RX descriptors.
> > > - */
> > > -enum rte_fdir_status_mode {
> > > - RTE_FDIR_NO_REPORT_STATUS = 0, /**< Never report FDIR hash. */
> > > - RTE_FDIR_REPORT_STATUS, /**< Only report FDIR hash for matching pkts. */
> > > - RTE_FDIR_REPORT_STATUS_ALWAYS, /**< Always report FDIR hash. */
> > > -};
> > > -
> > > -/**
> > > - * A structure used to configure the Flow Director (FDIR) feature
> > > - * of an Ethernet port.
> > > - *
> > > - * If mode is RTE_FDIR_DISABLE, the pballoc value is ignored.
> > > - */
> > > -struct rte_fdir_conf {
> > > - enum rte_fdir_mode mode; /**< Flow Director mode. */
> > > - enum rte_fdir_pballoc_type pballoc; /**< Space for FDIR filters. */
> > > - enum rte_fdir_status_mode status; /**< How to report FDIR hash. */
> > > - /** RX queue of packets matching a "drop" filter in perfect mode. */
> > > - uint8_t drop_queue;
> > > - struct rte_eth_fdir_masks mask;
> > > - struct rte_eth_fdir_flex_conf flex_conf;
> > > - /**< Flex payload configuration. */
> > > -};
> > > -
> > [Wu, Jingjing] This structures and above types are useful about global
> > configuration. They
> can't be removed.
>
> Why? I thought eth_ctrl API would be enough.
> It seems strange to have a part of the flow director API in rte_eth_ctrl.h and
> another part in rte_ethdev.h. Other filters have no impact on rte_ethdev.h.
rte_fdir_conf in rte_eth_conf is used when rte_eth_dev_configure to set
initialization parameters which cann't be changed at running time.
For example, if we disable the flow director, we can set the mode in it to
RTR_FDIR_DISABLE, then the resource will not be allocated for flow director.
The way is the same as rss_conf in rte_eth_conf. While the filter_ctrl can
provide interface to configure parameters or add/delete rules at device running
time.
Re: [dpdk-dev] [PATCH v7 05/27] net/i40e: set Tx loopback from PF
Is the Tx lookback meaning as VEB mode ore VEPA mode? If so, how about change the API's name to set_switch_mode? Another comment is: you removed all mac vlan filters before changing The mode, and restore them after changing. How about vlan filters? The Patch for vlan anti-spoof enables it, correct? Thanks JIngjing
Re: [dpdk-dev] [PATCH v7 06/27] net/i40e: set VF unicast promisc mode from PF
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, January 3, 2017 2:54 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo
> Subject: [dpdk-dev] [PATCH v7 06/27] net/i40e: set VF unicast promisc mode
> from PF
>
> Support enabling/disabling VF unicast promiscuous mode from PF.
> User can call the API on PF to enable/disable a specific VF's unicast
> promiscuous
> mode.
>
> Signed-off-by: Wenzhuo Lu
> ---
> drivers/net/i40e/i40e_ethdev.c| 39
> +++
> drivers/net/i40e/rte_pmd_i40e.h | 19 +++
> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> 3 files changed, 59 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index a5d6d05..3d7ee03 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -10120,3 +10120,42 @@ static void i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
>
> return ret;
> }
> +
> +int
> +rte_pmd_i40e_set_vf_unicast_promisc(uint8_t port, uint16_t vf_id,
> +uint8_t on) {
> + struct rte_eth_dev *dev;
> + struct rte_eth_dev_info dev_info;
> + struct i40e_pf *pf;
> + struct i40e_vsi *vsi;
> + struct i40e_hw *hw;
> + int ret;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> + rte_eth_dev_info_get(port, &dev_info);
> +
> + if (vf_id >= dev_info.max_vfs)
> + return -EINVAL;
> +
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +
> + if (vf_id > pf->vf_num - 1 || !pf->vfs) {
> + PMD_DRV_LOG(ERR, "Invalid argument.");
> + return -EINVAL;
> + }
Same comments as previous patches.
> + vsi = pf->vfs[vf_id].vsi;
> + if (!vsi)
> + return -EINVAL;
> +
> + hw = I40E_VSI_TO_HW(vsi);
> +
> + ret = i40e_aq_set_vsi_unicast_promiscuous(hw, vsi->seid,
> + on, NULL, true);
> + if (ret != I40E_SUCCESS)
> + PMD_DRV_LOG(ERR, "Failed to set unicast promiscuous mode");
> +
> + return ret;
ret is the error code defined in i40e driver, please use the error code in eth
dev lib.
Thanks
Jingjing
Re: [dpdk-dev] [PATCH v7 10/27] net/i40e: set VF MAC from PF support
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, January 3, 2017 2:55 PM
> To: dev@dpdk.org
> Cc: Yigit, Ferruh
> Subject: [dpdk-dev] [PATCH v7 10/27] net/i40e: set VF MAC from PF support
>
> From: Ferruh Yigit
>
> Support setting VF MAC address from PF.
> User can call the API on PF to set a specific VF's MAC address.
>
> This will remove all existing MAC filters.
>
> Signed-off-by: Ferruh Yigit
> ---
> drivers/net/i40e/i40e_ethdev.c| 42
> +++
> drivers/net/i40e/rte_pmd_i40e.h | 19 ++
> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> 3 files changed, 62 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 9d050c8..758b574 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -10198,3 +10198,45 @@ static void i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
>
> return ret;
> }
> +
> +int
> +rte_pmd_i40e_set_vf_mac_addr(uint8_t port, uint16_t vf_id,
> + struct ether_addr *mac_addr)
> +{
> + struct rte_eth_dev_info dev_info;
> + struct i40e_mac_filter *f;
> + struct rte_eth_dev *dev;
> + struct i40e_pf_vf *vf;
> + struct i40e_vsi *vsi;
> + struct i40e_pf *pf;
> + void *temp;
> +
> + if (i40e_validate_mac_addr((u8 *)mac_addr) != I40E_SUCCESS)
> + return -EINVAL;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> + rte_eth_dev_info_get(port, &dev_info);
> +
> + if (vf_id >= dev_info.max_vfs)
> + return -EINVAL;
> +
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> +
> + if (vf_id > pf->vf_num - 1 || !pf->vfs)
> + return -EINVAL;
> +
> + vf = &pf->vfs[vf_id];
> + vsi = vf->vsi;
> + if (!vsi)
> + return -EINVAL;
> +
> + ether_addr_copy(mac_addr, &vf->mac_addr);
Only store the mac address in vf struct?
Are you supposing the API is called before VF is initialized? If so, it's
better to comment it.
Thanks
Jingjing
Re: [dpdk-dev] [PATCH v7 03/27] net/i40e: set VF MAC anti-spoofing from PF
> + > + vsi->info.valid_sections = > cpu_to_le16(I40E_AQ_VSI_PROP_SECURITY_VALID); > + if (on) > + vsi->info.sec_flags |= > I40E_AQ_VSI_SEC_FLAG_ENABLE_MAC_CHK; > + else > + vsi->info.sec_flags &= > ~I40E_AQ_VSI_SEC_FLAG_ENABLE_MAC_CHK; > + > + memset(&ctxt, 0, sizeof(ctxt)); > + (void)rte_memcpy(&ctxt.info, &vsi->info, sizeof(vsi->info)); > + ctxt.seid = vsi->seid; > + > + hw = I40E_VSI_TO_HW(vsi); > + ret = i40e_aq_update_vsi_params(hw, &ctxt, NULL); > + if (ret != I40E_SUCCESS) > + PMD_DRV_LOG(ERR, "Failed to update VSI params"); If fails, will you revert the info in vsi struct? > + > + return ret; Please return eth dev lib error code, but not I40E_XXX
Re: [dpdk-dev] [PATCH v7 14/27] net/i40e: set VF VLAN insertion from PF
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, January 3, 2017 2:55 PM
> To: dev@dpdk.org
> Cc: Iremonger, Bernard
> Subject: [dpdk-dev] [PATCH v7 14/27] net/i40e: set VF VLAN insertion from PF
>
> From: Bernard Iremonger
>
> Support inserting VF VLAN id from PF.
> User can call the API on PF to insert a VLAN id to a specific VF.
>
> Signed-off-by: Bernard Iremonger
> ---
> drivers/net/i40e/i40e_ethdev.c| 56
> +++
> drivers/net/i40e/rte_pmd_i40e.h | 19 +++
> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> 3 files changed, 76 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 7ab1c93..31c387d 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -10266,3 +10266,59 @@ static void i40e_set_default_mac_addr(struct
> rte_eth_dev *dev,
> else
> return -EINVAL;
> }
> +
> +int rte_pmd_i40e_set_vf_vlan_insert(uint8_t port, uint16_t vf_id,
> + uint16_t vlan_id)
> +{
> + struct rte_eth_dev *dev;
> + struct rte_eth_dev_info dev_info;
> + struct i40e_pf *pf;
> + struct i40e_hw *hw;
> + struct i40e_vsi *vsi;
> + struct i40e_vsi_context ctxt;
> + int ret;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> + rte_eth_dev_info_get(port, &dev_info);
It looks dev_info is not used in this function.
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> + hw = I40E_PF_TO_HW(pf);
> +
> + /**
> + * return -ENODEV if SRIOV not enabled, VF number not configured
> + * or no queue assigned.
> + */
> + if (!hw->func_caps.sr_iov_1_1 || pf->vf_num == 0 ||
> + pf->vf_nb_qps == 0)
> + return -ENODEV;
> +
> + if (vf_id >= pf->vf_num || !pf->vfs)
> + return -EINVAL;
> +
> + if (vlan_id > ETHER_MAX_VLAN_ID)
> + return -EINVAL;
> +
> + vsi = pf->vfs[vf_id].vsi;
> + if (!vsi)
> + return -EINVAL;
> +
> + vsi->info.valid_sections =
> cpu_to_le16(I40E_AQ_VSI_PROP_VLAN_VALID);
> + vsi->info.pvid = vlan_id;
> + if (vlan_id > 0)
> + vsi->info.port_vlan_flags |= I40E_AQ_VSI_PVLAN_INSERT_PVID;
> + else
> + vsi->info.port_vlan_flags &=
> ~I40E_AQ_VSI_PVLAN_INSERT_PVID;
So, Pvid is used here for insert. Does it has any relationship with vlan
anti-spoof patch?
If so, it's better to consider how to deal with that.
Thanks
Jingjing
Re: [dpdk-dev] [PATCH v7 17/27] net/i40e: set VF VLAN filter from PF
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, January 3, 2017 2:55 PM
> To: dev@dpdk.org
> Cc: Iremonger, Bernard
> Subject: [dpdk-dev] [PATCH v7 17/27] net/i40e: set VF VLAN filter from PF
>
> From: Bernard Iremonger
>
> add rte_pmd_i40e_set_vf_vlan_filter API.
> User can call the API on PF to enable/disable a set of VF's VLAN filters.
>
> Signed-off-by: Bernard Iremonger
> ---
> drivers/net/i40e/i40e_ethdev.c| 52
> +++
> drivers/net/i40e/rte_pmd_i40e.h | 22 +
> drivers/net/i40e/rte_pmd_i40e_version.map | 1 +
> 3 files changed, 75 insertions(+)
>
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index 4d2fb20..47e03d6 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -10428,3 +10428,55 @@ int rte_pmd_i40e_set_vf_vlan_tag(uint8_t port,
> uint16_t vf_id, uint8_t on)
>
> return ret;
> }
> +
> +int rte_pmd_i40e_set_vf_vlan_filter(uint8_t port, uint16_t vlan_id,
> + uint64_t vf_mask, uint8_t on)
> +{
> + struct rte_eth_dev *dev;
> + struct rte_eth_dev_info dev_info;
> + struct i40e_pf *pf;
> + uint16_t pool_idx;
> + int ret = I40E_SUCCESS;
> +
> + RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);
> +
> + dev = &rte_eth_devices[port];
> + rte_eth_dev_info_get(port, &dev_info);
> +
> + if (vlan_id > ETHER_MAX_VLAN_ID)
> + return -EINVAL;
> +
> + if (on > 1)
> + return -EINVAL;
> +
> + pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
> + if ((pf->flags & I40E_FLAG_VMDQ) == 0) {
> + PMD_INIT_LOG(ERR, "Firmware doesn't support VMDQ");
> + return -ENOTSUP;
> + }
> +
> + if (!pf->vmdq) {
> + PMD_INIT_LOG(INFO, "VMDQ not configured");
> + return -ENOTSUP;
> + }
> +
> + for (pool_idx = 0;
> + pool_idx < ETH_64_POOLS &&
> + pool_idx < pf->nb_cfg_vmdq_vsi &&
> + ret == I40E_SUCCESS;
> + pool_idx++) {
> + if (vf_mask & ((uint64_t)(1ULL << pool_idx))) {
> + if (on)
> + ret = i40e_vsi_add_vlan(pf->vmdq[pool_idx].vsi,
> + vlan_id);
> + else
> + ret = i40e_vsi_delete_vlan(
> + pf->vmdq[pool_idx].vsi, vlan_id);
> + }
> + }
The head is saying "set VF VLAN filter", why do you deal with VMDQ VSIs?
Re: [dpdk-dev] [PATCH v7 18/27] app/testpmd: use VFD APIs on i40e
> -Original Message- > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wenzhuo Lu > Sent: Tuesday, January 3, 2017 2:55 PM > To: dev@dpdk.org > Cc: Lu, Wenzhuo ; Chen, Jing D > ; Iremonger, Bernard > Subject: [dpdk-dev] [PATCH v7 18/27] app/testpmd: use VFD APIs on i40e > > The new VF Daemon (VFD) APIs is implemented on i40e. Change testpmd code > to use them, including VF MAC anti-spoofing, VF VLAN anti-spoofing, TX > loopback, VF VLAN strip, VF VLAN insert. > > Signed-off-by: Wenzhuo Lu > Signed-off-by: Chen Jing D(Mark) > Signed-off-by: Bernard Iremonger > --- > app/test-pmd/Makefile | 3 + > app/test-pmd/cmdline.c | 154 +++- > - > 2 files changed, 126 insertions(+), 31 deletions(-) > > diff --git a/app/test-pmd/Makefile b/app/test-pmd/Makefile index > 891b85a..66bd38a 100644 > --- a/app/test-pmd/Makefile > +++ b/app/test-pmd/Makefile > @@ -58,7 +58,10 @@ SRCS-y += csumonly.c > SRCS-y += icmpecho.c > SRCS-$(CONFIG_RTE_LIBRTE_IEEE1588) += ieee1588fwd.c > > +ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y) > _LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe > +_LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e endif > > CFLAGS_cmdline.o := -D_GNU_SOURCE > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index > ed84d7a..9a44b4f 100644 > --- a/app/test-pmd/cmdline.c > +++ b/app/test-pmd/cmdline.c > @@ -90,6 +90,9 @@ > #ifdef RTE_LIBRTE_IXGBE_PMD > #include > #endif > +#ifdef RTE_LIBRTE_I40E_PMD > +#include > +#endif > #include "testpmd.h" > > static struct cmdline *testpmd_cl; > @@ -262,19 +265,19 @@ static void cmd_help_long_parsed(void > *parsed_result, > "set portlist (x[,y]*)\n" > "Set the list of forwarding ports.\n\n" > > -#ifdef RTE_LIBRTE_IXGBE_PMD How about use #if defined(RTE_LIBRTE_IXGBE_PMD) || defined (RTE_LIBRTE_I40E_PMD) but not remove it, because this command only works for ixgbe and i40e pmd. > "set tx loopback (port_id) (on|off)\n" > "Enable or disable tx loopback.\n\n" > > +#ifdef RTE_LIBRTE_IXGBE_PMD > "set all queues drop (port_id) (on|off)\n" > "Set drop enable bit for all queues.\n\n" > > "set vf split drop (port_id) (vf_id) (on|off)\n" > "Set split drop enable bit for a VF from the > PF.\n\n" > +#endif > > "set vf mac antispoof (port_id) (vf_id) (on|off).\n" > "Set MAC antispoof for a VF from the PF.\n\n" > -#endif >
Re: [dpdk-dev] [PATCH v7 19/27] app/testpmd: use unicast promiscuous mode on i40e
> -Original Message-
> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Wenzhuo Lu
> Sent: Tuesday, January 3, 2017 2:55 PM
> To: dev@dpdk.org
> Cc: Lu, Wenzhuo
> Subject: [dpdk-dev] [PATCH v7 19/27] app/testpmd: use unicast promiscuous
> mode on i40e
>
> Add testpmd CLI to set VF unicast promiscuous mode on i40e.
>
> Signed-off-by: Wenzhuo Lu
> ---
> app/test-pmd/cmdline.c | 93
> +
> doc/guides/testpmd_app_ug/testpmd_funcs.rst | 9 +++
> 2 files changed, 102 insertions(+)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 9a44b4f..affe9d1 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -400,6 +400,9 @@ static void cmd_help_long_parsed(void *parsed_result,
> "set allmulti (port_id|all) (on|off)\n"
> "Set the allmulti mode on port_id, or all.\n\n"
>
> + "set vf promisc (port_id) (vf_id) (on|off)\n"
> + "Set unicast promiscuous mode for a VF from the
> PF.\n\n"
> +
> "set flow_ctrl rx (on|off) tx (on|off) (high_water)"
> " (low_water) (pause_time) (send_xon)
> mac_ctrl_frame_fwd"
> " (on|off) autoneg (on|off) (port_id)\n"
> @@ -11559,6 +11562,95 @@ struct cmd_set_vf_mac_addr_result {
> },
> };
>
> +/* VF unicast promiscuous mode configuration */
> +
> +/* Common result structure for VF unicast promiscuous mode */ struct
> +cmd_vf_promisc_result {
> + cmdline_fixed_string_t set;
> + cmdline_fixed_string_t vf;
> + cmdline_fixed_string_t promisc;
> + uint8_t port_id;
> + uint32_t vf_id;
> + cmdline_fixed_string_t on_off;
> +};
> +
> +/* Common CLI fields for VF unicast promiscuous mode enable disable */
> +cmdline_parse_token_string_t cmd_vf_promisc_set =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_vf_promisc_result,
> + set, "set");
> +cmdline_parse_token_string_t cmd_vf_promisc_vf =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_vf_promisc_result,
> + vf, "vf");
> +cmdline_parse_token_string_t cmd_vf_promisc_promisc =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_vf_promisc_result,
> + promisc, "promisc");
> +cmdline_parse_token_num_t cmd_vf_promisc_port_id =
> + TOKEN_NUM_INITIALIZER
> + (struct cmd_vf_promisc_result,
> + port_id, UINT8);
> +cmdline_parse_token_num_t cmd_vf_promisc_vf_id =
> + TOKEN_NUM_INITIALIZER
> + (struct cmd_vf_promisc_result,
> + vf_id, UINT32);
> +cmdline_parse_token_string_t cmd_vf_promisc_on_off =
> + TOKEN_STRING_INITIALIZER
> + (struct cmd_vf_promisc_result,
> + on_off, "on#off");
> +
> +static void
> +cmd_set_vf_promisc_parsed(
> + void *parsed_result,
> + __attribute__((unused)) struct cmdline *cl,
> + __attribute__((unused)) void *data)
> +{
> + struct cmd_vf_promisc_result *res = parsed_result;
> + int ret = -ENOTSUP;
> +
> + __rte_unused int is_on = (strcmp(res->on_off, "on") == 0) ? 1 : 0;
> +
> + if (port_id_is_invalid(res->port_id, ENABLED_WARN))
> + return;
> +
> +#ifdef RTE_LIBRTE_I40E_PMD
> + ret = rte_pmd_i40e_set_vf_unicast_promisc(res->port_id,
> + res->vf_id, is_on);
> +#endif
> +
It's better to wrap the command by +#ifdef RTE_LIBRTE_I40E_PMD #endif
Or at least, need to check if the port is handled i40e pmd.
