| Message ID | 1495201052-2941-4-git-send-email-i.maximets@samsung.com |
|---|---|
| State | Accepted |
| Headers | show |
>From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of >Ilya Maximets >Sent: Friday, May 19, 2017 2:38 PM >To: dev@openvswitch.org; Daniele Di Proietto <diproiettod@ovn.org>; Darrell Ball ><dball@vmware.com> >Cc: Ilya Maximets <i.maximets@samsung.com>; Heetae Ahn <heetae82.ahn@samsung.com> >Subject: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: Use uint8_t for port_id. > >Currently, signed integer is used for 'port_id' variable and >'-1' as identifier of bad or uninitialized 'port_id'. > >This inconsistent with dpdk library and, also, in few cases, >leads to passing '-1' to dpdk functions where uint8_t expected. > >Such behaviour doesn't produce any issues, but it's better to >use same type as in dpdk library for consistency. >Introduced 'dpdk_port_t' typedef for better maintainability. > >Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID >macro. > >Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Hi Ilya, Darrell has pretty much covered any concerns that I had with these changes in his comments on V3. On initial reading of V4, I found 'dpdk_port_t' slightly non-intuitive; however, having read your rationale for this typedef, and given that it adheres to existing port ID conventions, I'm happy with this change. I also like the introduction of 'DPDK_ETH_PORT_ID_INVALID'. I've also conducted the following sanity checks on the patch, and found no issues: - checkpatch.py - compilation with clang - compilation with gcc - sparse - make check (no additional tests fail after application of this patch) Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Thanks, Mark >--- > lib/netdev-dpdk.c | 63 ++++++++++++++++++++++++++++++++----------------------- > 1 file changed, 37 insertions(+), 26 deletions(-) > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >index a41679f..b770b70 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -141,6 +141,11 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / >ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF)) > #define OVS_VHOST_QUEUE_DISABLED (-2) /* Queue was disabled by guest and not > * yet mapped to another queue. */ > >+#define DPDK_ETH_PORT_ID_INVALID RTE_MAX_ETHPORTS >+ >+/* DPDK library uses uint8_t for port_id. */ >+typedef uint8_t dpdk_port_t; >+ > #define VHOST_ENQ_RETRY_NUM 8 > #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) > >@@ -310,7 +315,7 @@ struct dpdk_ring { > struct rte_ring *cring_tx; > struct rte_ring *cring_rx; > unsigned int user_port_id; /* User given port no, parsed from port name */ >- int eth_port_id; /* ethernet device port id */ >+ dpdk_port_t eth_port_id; /* ethernet device port id */ > struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); > }; > >@@ -326,7 +331,7 @@ enum dpdk_hw_ol_features { > > struct netdev_dpdk { > struct netdev up; >- int port_id; >+ dpdk_port_t port_id; > int max_packet_len; > enum dpdk_dev_type type; > >@@ -403,7 +408,7 @@ struct netdev_dpdk { > > struct netdev_rxq_dpdk { > struct netdev_rxq up; >- int port_id; >+ dpdk_port_t port_id; > }; > > static int netdev_dpdk_class_init(void); >@@ -604,12 +609,12 @@ check_link_status(struct netdev_dpdk *dev) > dev->link_reset_cnt++; > dev->link = link; > if (dev->link.link_status) { >- VLOG_DBG_RL(&rl, "Port %d Link Up - speed %u Mbps - %s", >+ VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Up - speed %u Mbps - %s", > dev->port_id, (unsigned) dev->link.link_speed, > (dev->link.link_duplex == ETH_LINK_FULL_DUPLEX) ? > ("full-duplex") : ("half-duplex")); > } else { >- VLOG_DBG_RL(&rl, "Port %d Link Down", dev->port_id); >+ VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Down", dev->port_id); > } > } > } >@@ -727,8 +732,8 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev) > if (rx_csum_ol_flag && > (info.rx_offload_capa & rx_chksm_offload_capa) != > rx_chksm_offload_capa) { >- VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d", >- dev->port_id); >+ VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8, >+ dev->port_id); > dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD; > return; > } >@@ -739,7 +744,8 @@ static void > dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex) > { > if (rte_eth_dev_flow_ctrl_set(dev->port_id, &dev->fc_conf)) { >- VLOG_WARN("Failed to enable flow control on device %d", dev->port_id); >+ VLOG_WARN("Failed to enable flow control on device %"PRIu8, >+ dev->port_id); > } > } > >@@ -777,7 +783,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) > > memset(ð_addr, 0x0, sizeof(eth_addr)); > rte_eth_macaddr_get(dev->port_id, ð_addr); >- VLOG_INFO_RL(&rl, "Port %d: "ETH_ADDR_FMT, >+ VLOG_INFO_RL(&rl, "Port %"PRIu8": "ETH_ADDR_FMT, > dev->port_id, ETH_ADDR_BYTES_ARGS(eth_addr.addr_bytes)); > > memcpy(dev->hwaddr.ea, eth_addr.addr_bytes, ETH_ADDR_LEN); >@@ -789,7 +795,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) > /* Get the Flow control configuration for DPDK-ETH */ > diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); > if (diag) { >- VLOG_DBG("cannot get flow control parameters on port=%d, err=%d", >+ VLOG_DBG("cannot get flow control parameters on port=%"PRIu8", err=%d", > dev->port_id, diag); > } > >@@ -834,7 +840,7 @@ netdev_dpdk_alloc_txq(unsigned int n_txqs) > } > > static int >-common_construct(struct netdev *netdev, unsigned int port_no, >+common_construct(struct netdev *netdev, dpdk_port_t port_no, > enum dpdk_dev_type type, int socket_id) > OVS_REQUIRES(dpdk_mutex) > { >@@ -919,7 +925,8 @@ vhost_common_construct(struct netdev *netdev) > return ENOMEM; > } > >- return common_construct(netdev, -1, DPDK_DEV_VHOST, socket_id); >+ return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID, >+ DPDK_DEV_VHOST, socket_id); > } > > static int >@@ -979,7 +986,8 @@ netdev_dpdk_construct(struct netdev *netdev) > int err; > > ovs_mutex_lock(&dpdk_mutex); >- err = common_construct(netdev, -1, DPDK_DEV_ETH, SOCKET0); >+ err = common_construct(netdev, DPDK_ETH_PORT_ID_INVALID, >+ DPDK_DEV_ETH, SOCKET0); > ovs_mutex_unlock(&dpdk_mutex); > return err; > } >@@ -1113,7 +1121,7 @@ netdev_dpdk_get_config(const struct netdev *netdev, struct smap *args) > } > > static struct netdev_dpdk * >-netdev_dpdk_lookup_by_port_id(int port_id) >+netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id) > OVS_REQUIRES(dpdk_mutex) > { > struct netdev_dpdk *dev; >@@ -1127,13 +1135,13 @@ netdev_dpdk_lookup_by_port_id(int port_id) > return NULL; > } > >-static int >+static dpdk_port_t > netdev_dpdk_process_devargs(struct netdev_dpdk *dev, > const char *devargs, char **errp) > { > /* Get the name up to the first comma. */ > char *name = xmemdup0(devargs, strcspn(devargs, ",")); >- uint8_t new_port_id = UINT8_MAX; >+ dpdk_port_t new_port_id = DPDK_ETH_PORT_ID_INVALID; > > if (!rte_eth_dev_count() > || rte_eth_dev_get_port_by_name(name, &new_port_id) >@@ -1145,9 +1153,9 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev, > VLOG_INFO("Device '%s' attached to DPDK", devargs); > } else { > /* Attach unsuccessful */ >+ new_port_id = DPDK_ETH_PORT_ID_INVALID; > VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", > devargs); >- new_port_id = UINT8_MAX; > } > } > >@@ -1228,8 +1236,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, > * is valid */ > if (!(dev->devargs && !strcmp(dev->devargs, new_devargs) > && rte_eth_dev_is_valid_port(dev->port_id))) { >- int new_port_id = netdev_dpdk_process_devargs(dev, new_devargs, >- errp); >+ dpdk_port_t new_port_id = netdev_dpdk_process_devargs(dev, >+ new_devargs, >+ errp); > if (!rte_eth_dev_is_valid_port(new_port_id)) { > err = EINVAL; > } else if (new_port_id == dev->port_id) { >@@ -1237,6 +1246,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, > err = 0; > } else { > struct netdev_dpdk *dup_dev; >+ > dup_dev = netdev_dpdk_lookup_by_port_id(new_port_id); > if (dup_dev) { > VLOG_WARN_BUF(errp, "'%s' is trying to use device '%s' " >@@ -1246,6 +1256,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, > err = EADDRINUSE; > } else { > int sid = rte_eth_dev_socket_id(new_port_id); >+ > dev->requested_socket_id = sid < 0 ? SOCKET0 : sid; > dev->devargs = xstrdup(new_devargs); > dev->port_id = new_port_id; >@@ -2053,7 +2064,7 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct netdev_stats >*stats) > int rte_xstats_len, rte_xstats_new_len, rte_xstats_ret; > > if (rte_eth_stats_get(dev->port_id, &rte_stats)) { >- VLOG_ERR("Can't get ETH statistics for port: %i.", dev->port_id); >+ VLOG_ERR("Can't get ETH statistics for port: %"PRIu8, dev->port_id); > ovs_mutex_unlock(&dev->mutex); > return EPROTO; > } >@@ -2061,7 +2072,7 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct netdev_stats >*stats) > /* Get length of statistics */ > rte_xstats_len = rte_eth_xstats_get_names(dev->port_id, NULL, 0); > if (rte_xstats_len < 0) { >- VLOG_WARN("Cannot get XSTATS values for port: %i", dev->port_id); >+ VLOG_WARN("Cannot get XSTATS values for port: %"PRIu8, dev->port_id); > goto out; > } > /* Reserve memory for xstats names and values */ >@@ -2073,7 +2084,7 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct netdev_stats >*stats) > rte_xstats_names, > rte_xstats_len); > if (rte_xstats_new_len != rte_xstats_len) { >- VLOG_WARN("Cannot get XSTATS names for port: %i.", dev->port_id); >+ VLOG_WARN("Cannot get XSTATS names for port: %"PRIu8, dev->port_id); > goto out; > } > /* Retreive xstats values */ >@@ -2084,7 +2095,7 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct netdev_stats >*stats) > netdev_dpdk_convert_xstats(stats, rte_xstats, rte_xstats_names, > rte_xstats_len); > } else { >- VLOG_WARN("Cannot get XSTATS values for port: %i.", dev->port_id); >+ VLOG_WARN("Cannot get XSTATS values for port: %"PRIu8, dev->port_id); > } > > out: >@@ -2762,7 +2773,7 @@ netdev_dpdk_vhost_class_init(void) > > static int > dpdk_ring_create(const char dev_name[], unsigned int port_no, >- unsigned int *eth_port_id) >+ dpdk_port_t *eth_port_id) > { > struct dpdk_ring *ring_pair; > char *ring_name; >@@ -2814,7 +2825,7 @@ dpdk_ring_create(const char dev_name[], unsigned int port_no, > } > > static int >-dpdk_ring_open(const char dev_name[], unsigned int *eth_port_id) >+dpdk_ring_open(const char dev_name[], dpdk_port_t *eth_port_id) > OVS_REQUIRES(dpdk_mutex) > { > struct dpdk_ring *ring_pair; >@@ -2863,7 +2874,7 @@ netdev_dpdk_ring_send(struct netdev *netdev, int qid, > static int > netdev_dpdk_ring_construct(struct netdev *netdev) > { >- unsigned int port_no = 0; >+ dpdk_port_t port_no = 0; > int err = 0; > > ovs_mutex_lock(&dpdk_mutex); >-- >2.7.4 > >_______________________________________________ >dev mailing list >dev@openvswitch.org >https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Fri, May 26, 2017 at 12:37:14PM +0000, Kavanagh, Mark B wrote: > > >From: ovs-dev-bounces@openvswitch.org [mailto:ovs-dev-bounces@openvswitch.org] On Behalf Of > >Ilya Maximets > >Sent: Friday, May 19, 2017 2:38 PM > >To: dev@openvswitch.org; Daniele Di Proietto <diproiettod@ovn.org>; Darrell Ball > ><dball@vmware.com> > >Cc: Ilya Maximets <i.maximets@samsung.com>; Heetae Ahn <heetae82.ahn@samsung.com> > >Subject: [ovs-dev] [PATCH v4 3/3] netdev-dpdk: Use uint8_t for port_id. > > > >Currently, signed integer is used for 'port_id' variable and > >'-1' as identifier of bad or uninitialized 'port_id'. > > > >This inconsistent with dpdk library and, also, in few cases, > >leads to passing '-1' to dpdk functions where uint8_t expected. > > > >Such behaviour doesn't produce any issues, but it's better to > >use same type as in dpdk library for consistency. > >Introduced 'dpdk_port_t' typedef for better maintainability. > > > >Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID > >macro. > > > >Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > > Hi Ilya, > > Darrell has pretty much covered any concerns that I had with these changes in his comments on V3. > > On initial reading of V4, I found 'dpdk_port_t' slightly non-intuitive; however, having read your rationale for this typedef, and given that it adheres to existing port ID conventions, I'm happy with this change. > I also like the introduction of 'DPDK_ETH_PORT_ID_INVALID'. > > I've also conducted the following sanity checks on the patch, and found no issues: > - checkpatch.py > - compilation with clang > - compilation with gcc > - sparse > - make check (no additional tests fail after application of this patch) > > Acked-by: Mark Kavanagh <mark.b.kavanagh@intel.com> Thanks Ilya and Mark. I'd tend to go for a new "DPDK_PORT_FMT" macro, like this: #define DPDK_PORT_FMT PRIu8 to better abstract dpdk_port_t for use in printf() contexts. However, this patch makes sense to me anyway, so I applied this to master. Thanks, Ben.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index a41679f..b770b70 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -141,6 +141,11 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF)) #define OVS_VHOST_QUEUE_DISABLED (-2) /* Queue was disabled by guest and not * yet mapped to another queue. */ +#define DPDK_ETH_PORT_ID_INVALID RTE_MAX_ETHPORTS + +/* DPDK library uses uint8_t for port_id. */ +typedef uint8_t dpdk_port_t; + #define VHOST_ENQ_RETRY_NUM 8 #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) @@ -310,7 +315,7 @@ struct dpdk_ring { struct rte_ring *cring_tx; struct rte_ring *cring_rx; unsigned int user_port_id; /* User given port no, parsed from port name */ - int eth_port_id; /* ethernet device port id */ + dpdk_port_t eth_port_id; /* ethernet device port id */ struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); }; @@ -326,7 +331,7 @@ enum dpdk_hw_ol_features { struct netdev_dpdk { struct netdev up; - int port_id; + dpdk_port_t port_id; int max_packet_len; enum dpdk_dev_type type; @@ -403,7 +408,7 @@ struct netdev_dpdk { struct netdev_rxq_dpdk { struct netdev_rxq up; - int port_id; + dpdk_port_t port_id; }; static int netdev_dpdk_class_init(void); @@ -604,12 +609,12 @@ check_link_status(struct netdev_dpdk *dev) dev->link_reset_cnt++; dev->link = link; if (dev->link.link_status) { - VLOG_DBG_RL(&rl, "Port %d Link Up - speed %u Mbps - %s", + VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Up - speed %u Mbps - %s", dev->port_id, (unsigned) dev->link.link_speed, (dev->link.link_duplex == ETH_LINK_FULL_DUPLEX) ? ("full-duplex") : ("half-duplex")); } else { - VLOG_DBG_RL(&rl, "Port %d Link Down", dev->port_id); + VLOG_DBG_RL(&rl, "Port %"PRIu8" Link Down", dev->port_id); } } } @@ -727,8 +732,8 @@ dpdk_eth_checksum_offload_configure(struct netdev_dpdk *dev) if (rx_csum_ol_flag && (info.rx_offload_capa & rx_chksm_offload_capa) != rx_chksm_offload_capa) { - VLOG_WARN_ONCE("Rx checksum offload is not supported on device %d", - dev->port_id); + VLOG_WARN_ONCE("Rx checksum offload is not supported on device %"PRIu8, + dev->port_id); dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD; return; } @@ -739,7 +744,8 @@ static void dpdk_eth_flow_ctrl_setup(struct netdev_dpdk *dev) OVS_REQUIRES(dev->mutex) { if (rte_eth_dev_flow_ctrl_set(dev->port_id, &dev->fc_conf)) { - VLOG_WARN("Failed to enable flow control on device %d", dev->port_id); + VLOG_WARN("Failed to enable flow control on device %"PRIu8, + dev->port_id); } } @@ -777,7 +783,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) memset(ð_addr, 0x0, sizeof(eth_addr)); rte_eth_macaddr_get(dev->port_id, ð_addr); - VLOG_INFO_RL(&rl, "Port %d: "ETH_ADDR_FMT, + VLOG_INFO_RL(&rl, "Port %"PRIu8": "ETH_ADDR_FMT, dev->port_id, ETH_ADDR_BYTES_ARGS(eth_addr.addr_bytes)); memcpy(dev->hwaddr.ea, eth_addr.addr_bytes, ETH_ADDR_LEN); @@ -789,7 +795,7 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev) /* Get the Flow control configuration for DPDK-ETH */ diag = rte_eth_dev_flow_ctrl_get(dev->port_id, &dev->fc_conf); if (diag) { - VLOG_DBG("cannot get flow control parameters on port=%d, err=%d", + VLOG_DBG("cannot get flow control parameters on port=%"PRIu8", err=%d", dev->port_id, diag); } @@ -834,7 +840,7 @@ netdev_dpdk_alloc_txq(unsigned int n_txqs) } static int -common_construct(struct netdev *netdev, unsigned int port_no, +common_construct(struct netdev *netdev, dpdk_port_t port_no, enum dpdk_dev_type type, int socket_id) OVS_REQUIRES(dpdk_mutex) { @@ -919,7 +925,8 @@ vhost_common_construct(struct netdev *netdev) return ENOMEM; } - return common_construct(netdev, -1, DPDK_DEV_VHOST, socket_id); + return common_construct(netdev, DPDK_ETH_PORT_ID_INVALID, + DPDK_DEV_VHOST, socket_id); } static int @@ -979,7 +986,8 @@ netdev_dpdk_construct(struct netdev *netdev) int err; ovs_mutex_lock(&dpdk_mutex); - err = common_construct(netdev, -1, DPDK_DEV_ETH, SOCKET0); + err = common_construct(netdev, DPDK_ETH_PORT_ID_INVALID, + DPDK_DEV_ETH, SOCKET0); ovs_mutex_unlock(&dpdk_mutex); return err; } @@ -1113,7 +1121,7 @@ netdev_dpdk_get_config(const struct netdev *netdev, struct smap *args) } static struct netdev_dpdk * -netdev_dpdk_lookup_by_port_id(int port_id) +netdev_dpdk_lookup_by_port_id(dpdk_port_t port_id) OVS_REQUIRES(dpdk_mutex) { struct netdev_dpdk *dev; @@ -1127,13 +1135,13 @@ netdev_dpdk_lookup_by_port_id(int port_id) return NULL; } -static int +static dpdk_port_t netdev_dpdk_process_devargs(struct netdev_dpdk *dev, const char *devargs, char **errp) { /* Get the name up to the first comma. */ char *name = xmemdup0(devargs, strcspn(devargs, ",")); - uint8_t new_port_id = UINT8_MAX; + dpdk_port_t new_port_id = DPDK_ETH_PORT_ID_INVALID; if (!rte_eth_dev_count() || rte_eth_dev_get_port_by_name(name, &new_port_id) @@ -1145,9 +1153,9 @@ netdev_dpdk_process_devargs(struct netdev_dpdk *dev, VLOG_INFO("Device '%s' attached to DPDK", devargs); } else { /* Attach unsuccessful */ + new_port_id = DPDK_ETH_PORT_ID_INVALID; VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", devargs); - new_port_id = UINT8_MAX; } } @@ -1228,8 +1236,9 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, * is valid */ if (!(dev->devargs && !strcmp(dev->devargs, new_devargs) && rte_eth_dev_is_valid_port(dev->port_id))) { - int new_port_id = netdev_dpdk_process_devargs(dev, new_devargs, - errp); + dpdk_port_t new_port_id = netdev_dpdk_process_devargs(dev, + new_devargs, + errp); if (!rte_eth_dev_is_valid_port(new_port_id)) { err = EINVAL; } else if (new_port_id == dev->port_id) { @@ -1237,6 +1246,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, err = 0; } else { struct netdev_dpdk *dup_dev; + dup_dev = netdev_dpdk_lookup_by_port_id(new_port_id); if (dup_dev) { VLOG_WARN_BUF(errp, "'%s' is trying to use device '%s' " @@ -1246,6 +1256,7 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, err = EADDRINUSE; } else { int sid = rte_eth_dev_socket_id(new_port_id); + dev->requested_socket_id = sid < 0 ? SOCKET0 : sid; dev->devargs = xstrdup(new_devargs); dev->port_id = new_port_id; @@ -2053,7 +2064,7 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct netdev_stats *stats) int rte_xstats_len, rte_xstats_new_len, rte_xstats_ret; if (rte_eth_stats_get(dev->port_id, &rte_stats)) { - VLOG_ERR("Can't get ETH statistics for port: %i.", dev->port_id); + VLOG_ERR("Can't get ETH statistics for port: %"PRIu8, dev->port_id); ovs_mutex_unlock(&dev->mutex); return EPROTO; } @@ -2061,7 +2072,7 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct netdev_stats *stats) /* Get length of statistics */ rte_xstats_len = rte_eth_xstats_get_names(dev->port_id, NULL, 0); if (rte_xstats_len < 0) { - VLOG_WARN("Cannot get XSTATS values for port: %i", dev->port_id); + VLOG_WARN("Cannot get XSTATS values for port: %"PRIu8, dev->port_id); goto out; } /* Reserve memory for xstats names and values */ @@ -2073,7 +2084,7 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct netdev_stats *stats) rte_xstats_names, rte_xstats_len); if (rte_xstats_new_len != rte_xstats_len) { - VLOG_WARN("Cannot get XSTATS names for port: %i.", dev->port_id); + VLOG_WARN("Cannot get XSTATS names for port: %"PRIu8, dev->port_id); goto out; } /* Retreive xstats values */ @@ -2084,7 +2095,7 @@ netdev_dpdk_get_stats(const struct netdev *netdev, struct netdev_stats *stats) netdev_dpdk_convert_xstats(stats, rte_xstats, rte_xstats_names, rte_xstats_len); } else { - VLOG_WARN("Cannot get XSTATS values for port: %i.", dev->port_id); + VLOG_WARN("Cannot get XSTATS values for port: %"PRIu8, dev->port_id); } out: @@ -2762,7 +2773,7 @@ netdev_dpdk_vhost_class_init(void) static int dpdk_ring_create(const char dev_name[], unsigned int port_no, - unsigned int *eth_port_id) + dpdk_port_t *eth_port_id) { struct dpdk_ring *ring_pair; char *ring_name; @@ -2814,7 +2825,7 @@ dpdk_ring_create(const char dev_name[], unsigned int port_no, } static int -dpdk_ring_open(const char dev_name[], unsigned int *eth_port_id) +dpdk_ring_open(const char dev_name[], dpdk_port_t *eth_port_id) OVS_REQUIRES(dpdk_mutex) { struct dpdk_ring *ring_pair; @@ -2863,7 +2874,7 @@ netdev_dpdk_ring_send(struct netdev *netdev, int qid, static int netdev_dpdk_ring_construct(struct netdev *netdev) { - unsigned int port_no = 0; + dpdk_port_t port_no = 0; int err = 0; ovs_mutex_lock(&dpdk_mutex);
Currently, signed integer is used for 'port_id' variable and '-1' as identifier of bad or uninitialized 'port_id'. This inconsistent with dpdk library and, also, in few cases, leads to passing '-1' to dpdk functions where uint8_t expected. Such behaviour doesn't produce any issues, but it's better to use same type as in dpdk library for consistency. Introduced 'dpdk_port_t' typedef for better maintainability. Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID macro. Signed-off-by: Ilya Maximets <i.maximets@samsung.com> --- lib/netdev-dpdk.c | 63 ++++++++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 26 deletions(-)