From: "Yang, Zhiyong" <zhiyong.yang@intel.com>
To: "Yigit, Ferruh" <ferruh.yigit@intel.com>,
"dev@dpdk.org" <dev@dpdk.org>,
"Doherty, Declan" <declan.doherty@intel.com>,
"Lu, Wenzhuo" <wenzhuo.lu@intel.com>
Cc: "thomas@monjalon.net" <thomas@monjalon.net>,
"hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"Hunt, David" <david.hunt@intel.com>,
"Richardson, Bruce" <bruce.richardson@intel.com>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Subject: Re: [dpdk-dev] [PATCH v3 1/4] ethdev: increase port_id range
Date: Wed, 13 Sep 2017 02:26:40 +0000 [thread overview]
Message-ID: <E182254E98A5DA4EB1E657AC7CB9BD2A8AF132E4@BGSMSX101.gar.corp.intel.com> (raw)
In-Reply-To: <ada991be-5eb7-59b1-1303-2b8e757d876c@intel.com>
Hi Ferruh,
> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, September 11, 2017 6:22 PM
> To: Yang, Zhiyong <zhiyong.yang@intel.com>; dev@dpdk.org; Doherty, Declan
> <declan.doherty@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: thomas@monjalon.net; hemant.agrawal@nxp.com; Hunt, David
> <david.hunt@intel.com>
> Subject: Re: [PATCH v3 1/4] ethdev: increase port_id range
>
> 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.
>
Ferruh, the suggestion has been discussed in the following thread. Most of people agree on
The basic type uint16_t. :). Your suggestion was my preference previously.
At last, I make this decision to use uint16_t. You know, whatever I use, some ones will stand out and
Say the other is better. :)
http://www.dpdk.org/dev/patchwork/patch/23208/
> <...>
>
> > --- 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.
Ok
>
> > {
> > /* 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.
Ok.
>
> > {
> > 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.
>
Ok.
> > 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.
>
Ok
> <...>
>
> > @@ -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.
Ok
>
> <...>
> > @@ -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.
>
Ok
> <...>
>
> > @@ -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.
>
Do you mean that I should remove
> > -MAP_STATIC_SYMBOL(int rte_eth_bond_8023ad_setup(uint8_t port_id, ?
> 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.
>
Yes. I'm not clear about it. Need help Ferruh.
> <...>
>
> > @@ -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 ?
>
My bad. Remove it.
> <...>
>
> > @@ -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.. ?
>
The code caused failures when I test bonding driver in test code. and I will fix them if I trace.
> <...>
>
> > --- 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.
>
thanks
> <...>
>
> > --- 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.
>
Ok.
> > + 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.
>
Ok.
> <...>
>
> > @@ -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.
Ok. I should focus on port id range increase . :)
>
> > 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.
>
Ok
> <...>
next prev parent reply other threads:[~2017-09-13 2:26 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
2017-09-13 2:26 ` Yang, Zhiyong [this message]
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=E182254E98A5DA4EB1E657AC7CB9BD2A8AF132E4@BGSMSX101.gar.corp.intel.com \
--to=zhiyong.yang@intel.com \
--cc=bruce.richardson@intel.com \
--cc=david.hunt@intel.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=hemant.agrawal@nxp.com \
--cc=konstantin.ananyev@intel.com \
--cc=thomas@monjalon.net \
--cc=wenzhuo.lu@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).