DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Zhiyong Yang <zhiyong.yang@intel.com>,
	dev@dpdk.org, Declan Doherty <declan.doherty@intel.com>,
	Wenzhuo Lu <wenzhuo.lu@intel.com>
Cc: thomas@monjalon.net, hemant.agrawal@nxp.com, david.hunt@intel.com
Subject: Re: [dpdk-dev] [PATCH v3 1/4] ethdev: increase port_id range
Date: Mon, 11 Sep 2017 11:21:32 +0100	[thread overview]
Message-ID: <ada991be-5eb7-59b1-1303-2b8e757d876c@intel.com> (raw)
In-Reply-To: <20170909144727.46388-2-zhiyong.yang@intel.com>

On 9/9/2017 3:47 PM, Zhiyong Yang wrote:
> Extend port_id definition from uint8_t to uint16_t in lib and drivers
> data structures, specifically rte_eth_dev_data.
> Modify the APIs, drivers and app using port_id at the same time.
> 
> Fix some checkpatch issues from the original code and remove some
> unnecessary cast operations.
> 
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>

<...>

> @@ -283,7 +283,7 @@ enum dcb_mode_enable
>  #define MAX_RX_QUEUE_STATS_MAPPINGS 4096 /* MAX_PORT of 32 @ 128 rx_queues/port */
>  
>  struct queue_stats_mappings {
> -	uint8_t port_id;
> +	uint16_t port_id;

Can this be "portid_t port_id;" ? For testpmd, portid_t can be used for
all port_id declarations.

<...>

> --- a/drivers/net/bnx2x/bnx2x.c
> +++ b/drivers/net/bnx2x/bnx2x.c
> @@ -703,7 +703,7 @@ bnx2x_gpio_mult_write(struct bnx2x_softc *sc, uint8_t pins, uint32_t mode)
>  
>  static int
>  bnx2x_gpio_int_write(struct bnx2x_softc *sc, int gpio_num, uint32_t mode,
> -		   uint8_t port)
> +		      uint8_t port)

If port storage type will not change, no need to update this line. It is
good to fix syntax the lines touched, but for the lines not updated
please don't fix the syntax in this patch.

>  {
>  	/* The GPIO should be swapped if swap register is set and active */
>  	int gpio_port = ((REG_RD(sc, NIG_REG_PORT_SWAP) &&
> @@ -749,7 +749,7 @@ bnx2x_gpio_int_write(struct bnx2x_softc *sc, int gpio_num, uint32_t mode,
>  }
>  
>  uint32_t
> -elink_cb_gpio_read(struct bnx2x_softc * sc, uint16_t gpio_num, uint8_t port)
> +elink_cb_gpio_read(struct bnx2x_softc *sc, uint16_t gpio_num, uint8_t port)

Same here.

>  {
>  	return bnx2x_gpio_read(sc, gpio_num, port);
>  }
> diff --git a/drivers/net/bnx2x/bnx2x_rxtx.h b/drivers/net/bnx2x/bnx2x_rxtx.h
> index 2e38ec26a..48d540476 100644
> --- a/drivers/net/bnx2x/bnx2x_rxtx.h
> +++ b/drivers/net/bnx2x/bnx2x_rxtx.h
> @@ -41,7 +41,7 @@ struct bnx2x_rx_queue {
>  	uint16_t                   rx_cq_head;           /**< Index of current rcq bd. */
>  	uint16_t                   rx_cq_tail;           /**< Index of last rcq bd. */
>  	uint16_t                   queue_id;             /**< RX queue index. */
> -	uint8_t                    port_id;              /**< Device port identifier. */
> +	uint16_t                   port_id;	/**< Device port identifier. */

Please fix comment allignment.

>  	struct bnx2x_softc           *sc;                  /**< Ptr to dev_private data. */
>  };

<...>

> @@ -500,7 +501,7 @@ elink_status_t elink_phy_probe(struct elink_params *params);
>  
>  /* Checks if fan failure detection is required on one of the phys on board */
>  uint8_t elink_fan_failure_det_req(struct bnx2x_softc *sc, uint32_t shmem_base,
> -			     uint32_t shmem2_base, uint8_t port);
> +				  uint32_t shmem2_base, uint8_t port);

no change, please drop.

<...>

> @@ -511,7 +511,6 @@ mux_machine(struct bond_dev_private *internals, uint8_t slave_id)
>  		ACTOR_STATE_CLR(port, SYNCHRONIZATION);
>  		MODE4_DEBUG("Out of sync -> ATTACHED\n");
>  	}
> -

Please drop this one.

<...>
> @@ -1022,12 +1022,12 @@ bond_mode_8023ad_activate_slave(struct rte_eth_dev *bond_dev, uint8_t slave_id)
>  
>  int
>  bond_mode_8023ad_deactivate_slave(struct rte_eth_dev *bond_dev,
> -		uint8_t slave_id)
> +				   uint16_t slave_id)

The coding style for multiple lines in a function call is two tabs or
alternatively allign to the paranthesis. Original code synyax looks good
here, no need to update.

<...>

> @@ -1536,17 +1536,12 @@ rte_eth_bond_8023ad_setup_v1708(uint8_t port_id,
>  	return 0;
>  }
>  BIND_DEFAULT_SYMBOL(rte_eth_bond_8023ad_setup, _v1708, 17.08);
> -MAP_STATIC_SYMBOL(int rte_eth_bond_8023ad_setup(uint8_t port_id,
> +MAP_STATIC_SYMBOL(int rte_eth_bond_8023ad_setup(uint16_t port_id,

Hmm, this is tricky!
The macro MAP_STATIC_SYMBOL is used for ABI versioning, but changing the
port_id storage type breaks the ABI already. ABI versioning can be
removed completely. Cc'ed Declan.

Which also reminds me that bonding LIBABIVER needs to be updated. This
is also required for all i40e, ixgbe and bnxt. Please let me know if you
need help here.

<...>

> @@ -1622,12 +1618,13 @@ rte_eth_bond_8023ad_ext_collect(uint8_t port_id, uint8_t slave_id, int enabled)
>  		ACTOR_STATE_SET(port, COLLECTING);
>  	else
>  		ACTOR_STATE_CLR(port, COLLECTING);
> -
> +	printf("enabled  port->actor_state = %d \r\n",  port->actor_state);

Is this a git rebase error ?

<...>

> @@ -586,13 +588,14 @@ rte_eth_bond_active_slaves_get(uint8_t bonded_port_id, uint8_t slaves[],
>  	if (internals->active_slave_count > len)
>  		return -1;
>  
> -	memcpy(slaves, internals->active_slaves, internals->active_slave_count);
> +	memcpy(slaves, internals->active_slaves,
> +	internals->active_slave_count * sizeof(internals->active_slaves[0]));

Good catch!
I wonder if there are more like this, did you traced all memcpy, memset,
etc..  ?

<...>

> --- a/drivers/net/ixgbe/rte_pmd_ixgbe.c
> +++ b/drivers/net/ixgbe/rte_pmd_ixgbe.c
> @@ -38,7 +38,7 @@
>  #include "rte_pmd_ixgbe.h"
>  
>  int
> -rte_pmd_ixgbe_set_vf_mac_addr(uint8_t port, uint16_t vf,
> +rte_pmd_ixgbe_set_vf_mac_addr(uint16_t port, uint16_t vf,
>  			      struct ether_addr *mac_addr)

ixgbe LIBABIVER also needs to be updated.

I have just recognized that release notes missing this library, I will add.

<...>

> --- a/drivers/net/vmxnet3/vmxnet3_ring.h
> +++ b/drivers/net/vmxnet3/vmxnet3_ring.h
> @@ -143,8 +143,8 @@ typedef struct vmxnet3_tx_queue {
>  	struct vmxnet3_txq_stats     stats;
>  	const struct rte_memzone     *mz;
>  	bool                         stopped;
> -	uint16_t                     queue_id;      /**< Device TX queue index. */
> -	uint8_t                      port_id;       /**< Device port identifier. */
> +	uint16_t                     queue_id; /**< Device TX queue index. */

No need to change "queue_id" here, if this is for comment allignment,
please allign the port_id one.

> +	uint16_t                     port_id;  /**< Device port identifier. */
>  	uint16_t		     txdata_desc_size;
>  } vmxnet3_tx_queue_t;
>  
> @@ -178,8 +178,8 @@ typedef struct vmxnet3_rx_queue {
>  	struct vmxnet3_rxq_stats    stats;
>  	const struct rte_memzone    *mz;
>  	bool                        stopped;
> -	uint16_t                    queue_id;      /**< Device RX queue index. */
> -	uint8_t                     port_id;       /**< Device port identifier. */
> +	uint16_t                    queue_id; /**< Device RX queue index. */

same as above.

<...>

> @@ -94,8 +94,7 @@ rte_port_ethdev_reader_create(void *params, int socket_id)
>  static int
>  rte_port_ethdev_reader_rx(void *port, struct rte_mbuf **pkts, uint32_t n_pkts)
>  {
> -	struct rte_port_ethdev_reader *p =
> -		port;
> +	struct rte_port_ethdev_reader *p = port;

This is a good syntax correction, but this patch is already big, please
drop these ones.

>  	uint16_t rx_pkt_cnt;
>  
>  	rx_pkt_cnt = rte_eth_rx_burst(p->port_id, p->queue_id, pkts, n_pkts);
> @@ -119,8 +118,7 @@ rte_port_ethdev_reader_free(void *port)
>  static int rte_port_ethdev_reader_stats_read(void *port,
>  		struct rte_port_in_stats *stats, int clear)
>  {
> -	struct rte_port_ethdev_reader *p =
> -			port;
> +	struct rte_port_ethdev_reader *p = port;

same as above, and there are few more below.

<...>

  parent reply	other threads:[~2017-09-11 10:21 UTC|newest]

Thread overview: 101+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-09  8:42 [dpdk-dev] [PATCH 0/2] " Zhiyong Yang
2017-08-09  8:42 ` [dpdk-dev] [PATCH 1/2] ethdev: " Zhiyong Yang
2017-08-09 12:52   ` Ferruh Yigit
2017-08-09 12:57     ` Wiles, Keith
2017-08-10  0:53       ` Yang, Zhiyong
2017-08-10  0:51     ` Yang, Zhiyong
2017-08-09  8:42 ` [dpdk-dev] [PATCH 2/2] examples: " Zhiyong Yang
2017-08-09 14:48   ` Stephen Hemminger
2017-08-10  1:03     ` Yang, Zhiyong
2017-08-09  9:00 ` [dpdk-dev] [PATCH 0/2] " De Lara Guarch, Pablo
2017-08-09  9:17   ` Yang, Zhiyong
2017-09-04  5:57 ` [dpdk-dev] [PATCH v2 0/4] " Zhiyong Yang
2017-09-04  5:57   ` [dpdk-dev] [PATCH v2 1/4] ethdev: " Zhiyong Yang
2017-09-04  9:06     ` Bruce Richardson
2017-09-04  9:29       ` Ferruh Yigit
2017-09-04  9:47         ` Yang, Zhiyong
2017-09-04 13:12           ` Adrien Mazarguil
2017-09-04 13:17             ` Richardson, Bruce
2017-09-04 13:36               ` Adrien Mazarguil
2017-09-04 13:59                 ` Yang, Zhiyong
2017-09-04 14:41                   ` Adrien Mazarguil
2017-09-05  6:51       ` Yang, Zhiyong
2017-09-06  8:32     ` Hemant Agrawal
2017-09-06  8:48       ` Yang, Zhiyong
     [not found]     ` <CALZ3Guikt9x8sz-oEKCuDCSp_wtKa64bSXTrMhqcWyg7f_dS7g@mail.gmail.com>
2017-09-07  0:45       ` Yang, Zhiyong
2017-09-04  5:57   ` [dpdk-dev] [PATCH v2 2/4] examples: " Zhiyong Yang
2017-09-04 14:15     ` Hunt, David
2017-09-04 15:01       ` Yang, Zhiyong
2017-09-04  5:57   ` [dpdk-dev] [PATCH v2 3/4] common_base: extend RTE_MAX_ETHPORTS from 32 to 1024 Zhiyong Yang
2017-09-04  7:46     ` Yao, Lei A
2017-09-04  7:59       ` Yang, Zhiyong
2017-09-04  9:09         ` Bruce Richardson
2017-09-04 10:05           ` Yang, Zhiyong
2017-09-04 10:27             ` Ananyev, Konstantin
2017-09-04 14:18               ` Yang, Zhiyong
2017-09-06  8:42               ` Hemant Agrawal
2017-09-06  8:52                 ` Yang, Zhiyong
2017-09-04 10:29             ` Bruce Richardson
2017-09-04  9:27       ` Ananyev, Konstantin
2017-09-04  5:57   ` [dpdk-dev] [PATCH v2 4/4] testpmd: add flexibility to mbuf allocation Zhiyong Yang
2017-09-09 14:47   ` [dpdk-dev] [PATCH v3 0/4] increase port_id range Zhiyong Yang
2017-09-09 14:47     ` [dpdk-dev] [PATCH v3 1/4] ethdev: " Zhiyong Yang
2017-09-11  9:37       ` Adrien Mazarguil
2017-09-11 10:51         ` Yang, Zhiyong
2017-09-11 10:21       ` Ferruh Yigit [this message]
2017-09-13  2:26         ` Yang, Zhiyong
2017-09-13 11:56           ` Ferruh Yigit
2017-09-13 12:15             ` Yang, Zhiyong
2017-09-13 12:18             ` Thomas Monjalon
2017-09-13 13:33               ` Ferruh Yigit
2017-09-19  6:05                 ` Yang, Zhiyong
2017-09-19 12:30                   ` Wiles, Keith
2017-09-14 12:49           ` Ferruh Yigit
2017-09-15  5:11             ` Yang, Zhiyong
2017-09-09 14:47     ` [dpdk-dev] [PATCH v3 2/4] test: " Zhiyong Yang
2017-09-09 14:47     ` [dpdk-dev] [PATCH v3 3/4] examples: " Zhiyong Yang
2017-09-14 14:41       ` Ferruh Yigit
2017-09-09 14:47     ` [dpdk-dev] [PATCH v3 4/4] librte_mbuf: modify port initialization value Zhiyong Yang
2017-09-11 10:23     ` [dpdk-dev] [PATCH v3 0/4] increase port_id range Ferruh Yigit
2017-09-11 11:25       ` Yang, Zhiyong
2017-09-13  8:14       ` Matej Vido
2017-09-13  8:21         ` Yang, Zhiyong
2017-09-18 14:54           ` Laatz, Kevin
2017-09-19  1:39             ` Yang, Zhiyong
2017-09-11 10:26     ` Ferruh Yigit
2017-09-11 10:55       ` Yang, Zhiyong
2017-09-11 11:24         ` Ferruh Yigit
2017-09-21  8:32     ` [dpdk-dev] [PATCH v4 0/5] " Zhiyong Yang
2017-09-21  8:32       ` [dpdk-dev] [PATCH v4 1/5] net/bonding: remove bonding APIs using ABI versioning Zhiyong Yang
2017-09-21 10:36         ` Ferruh Yigit
2017-09-22  2:02           ` Yang, Zhiyong
2017-09-21  8:32       ` [dpdk-dev] [PATCH v4 2/5] ethdev: increase port_id range Zhiyong Yang
2017-09-21 11:49         ` Adrien Mazarguil
2017-10-06 14:34           ` Nélio Laranjeiro
2017-09-21  8:32       ` [dpdk-dev] [PATCH v4 3/5] examples: " Zhiyong Yang
2017-09-21  8:32       ` [dpdk-dev] [PATCH v4 4/5] test: " Zhiyong Yang
2017-09-21  8:32       ` [dpdk-dev] [PATCH v4 5/5] librte_mbuf: modify port initialization value Zhiyong Yang
2017-09-25  3:22       ` [dpdk-dev] [PATCH v5 0/5] increase port_id range Zhiyong Yang
2017-09-25  3:22         ` [dpdk-dev] [PATCH v5 1/5] net/bonding: remove bonding APIs using ABI versioning Zhiyong Yang
2017-09-25 11:34           ` Ferruh Yigit
2017-09-25  3:22         ` [dpdk-dev] [PATCH v5 2/5] ethdev: increase port_id range Zhiyong Yang
2017-09-25 11:37           ` Ferruh Yigit
2017-09-25 12:06           ` Ferruh Yigit
2017-09-26  7:01             ` Yang, Zhiyong
2017-09-27 18:44               ` Ferruh Yigit
2017-09-28  2:12                 ` Yang, Zhiyong
2017-09-25  3:22         ` [dpdk-dev] [PATCH v5 3/5] examples: " Zhiyong Yang
2017-09-25  3:22         ` [dpdk-dev] [PATCH v5 4/5] test: " Zhiyong Yang
2017-09-25  3:22         ` [dpdk-dev] [PATCH v5 5/5] librte_mbuf: modify port initialization value Zhiyong Yang
2017-09-29  7:17         ` [dpdk-dev] [PATCH v6 0/5] increase port_id range Zhiyong Yang
2017-09-29  7:17           ` [dpdk-dev] [PATCH v6 1/5] net/bonding: remove bonding APIs using ABI versioning Zhiyong Yang
2017-09-29  7:17           ` [dpdk-dev] [PATCH v6 2/5] ethdev: increase port_id range Zhiyong Yang
2017-09-29  7:17           ` [dpdk-dev] [PATCH v6 3/5] examples: " Zhiyong Yang
2017-09-29  7:17           ` [dpdk-dev] [PATCH v6 4/5] test: " Zhiyong Yang
2017-09-29  7:17           ` [dpdk-dev] [PATCH v6 5/5] librte_mbuf: modify port initialization value Zhiyong Yang
2017-10-06  2:15           ` [dpdk-dev] [PATCH v6 0/5] increase port_id range Ferruh Yigit
2017-10-06 13:31             ` Gaëtan Rivet
2017-10-06 14:29             ` Thomas Monjalon
2017-10-06 16:02             ` Thomas Monjalon
2017-10-11 21:21             ` Ferruh Yigit
2017-10-12  1:33               ` Yang, Zhiyong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ada991be-5eb7-59b1-1303-2b8e757d876c@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=david.hunt@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=hemant.agrawal@nxp.com \
    --cc=thomas@monjalon.net \
    --cc=wenzhuo.lu@intel.com \
    --cc=zhiyong.yang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).