On 5/19/17, 6:39 AM, "Ilya Maximets" <i.maxim...@samsung.com> wrote:
On 18.05.2017 16:34, Aaron Conole wrote: > Hi Ilya, > > Ilya Maximets <i.maxim...@samsung.com> writes: > >> On 17.05.2017 18:32, Darrell Ball wrote: >>> >>> >>> On 5/17/17, 7:59 AM, "Ilya Maximets" <i.maxim...@samsung.com> wrote: >>> >>> I guess, we need some more opinions about this. >>> >>> My comments inline. >>> >>> Best regards, Ilya Maximets. >>> >>> On 13.05.2017 07:00, Darrell Ball wrote: >>> > >>> > >>> > On 5/12/17, 8:04 AM, "Ilya Maximets" <i.maxim...@samsung.com> wrote: >>> > >>> > On 05.04.2017 22:34, Darrell Ball wrote: >>> > > >>> > > >>>>> On 4/3/17, 8:04 AM, "ovs-dev-boun...@openvswitch.org on behalf >> of Ilya Maximets" <ovs-dev-boun...@openvswitch.org on behalf of >> i.maxim...@samsung.com> wrote: >>> > > >>> > > 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. >>> > > >>> > > Also, magic number '-1' replaced with DPDK_ETH_PORT_ID_INVALID >>> > > macro. >>> > > >>> > > Signed-off-by: Ilya Maximets <i.maxim...@samsung.com> >>> > > --- >>>>> lib/netdev-dpdk.c | 61 >> +++++++++++++++++++++++++++++++------------------------ >>> > > 1 file changed, 35 insertions(+), 26 deletions(-) >>> > > >>> > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >>> > > index 658a454..216ced8 100644 >>> > > --- a/lib/netdev-dpdk.c >>> > > +++ b/lib/netdev-dpdk.c >>>>> @@ -140,6 +140,8 @@ 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 >>> > > + >>> > > #define VHOST_ENQ_RETRY_NUM 8 >>>>> #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) >>> > > >>> > > @@ -309,7 +311,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 */ >>> > > + uint8_t eth_port_id; /* ethernet device port id */ >>> > > >>> > > The rte does not have a typedef for port_id. >>>>> One optional change is for OVS to have a typedef for this >> external type. >>> > > /* The dpdk rte uses uint8_t for port_id. */ >>> > > typedef uint8_t rte_port_id; >>> > >>> > I don't think that it is a good change to do because we will have to >>> > create additional typedef at least for PRIu8. This will look ugly. >>> > >>> > No, see my following diff >>> > >>> > Also, if DPDK someday will create typedef with different name >>> > we will have typedef of the typedef? >>> > >>> > I don’t follow this comment; see the diff below. >>> > >>> > The reasons for the typedef here are: >>> > 1) Easier to maintain if the size of type changes in future >>> >>> In case of future type change we will have to change all the "PRIu8" >>> format strings to something else. This is the half of all the changes. >>> So, maintainability is in question. >>> >>> The alternative is to change both PRIu8 and the other code as well… >>> This reasoning would imply that your proposed change is more maintainable >>> because all code using the datatype would need to be re-written ? >> >> Maybe you're right, but both approaches are more or less poorly maintainable. >> >>> The main purpose here is to differentiate b/w ovs and external port_id >>> namespaces. >>> The reason why this patch exists is due to confusion in this regard – >>> using the wrong datatype of int for port_ids derived from the RTE library. >>> >>> Your patch is trying again to manually replicate the RTE port_id >> datatype in OVS code and >>> doing this without documenting that the port_id namespace is RTE and >> not one of >>> OVS native port ids. >>> This is confusing to me and not maintainable. >>> >>> >>>> 2) Serves as documentation that the origin of the type is the rte >> library and >>> > and originates externally to ovs >>> >>> IMHO, using 'rte_' prefix for something defined not inside DPDK is a >>> bad style and may be misleading. We should remember that this type >>> defined locally in OVS. >>> >>> This patch is trying to replicate the datatype coming from the RTE >> library inside >>> OVS code. >>> As mentioned earlier, I would prefer the port_id being defined in >> RTE code and then used in OVS >>> but I don’t see that datatype defined in the RTE library. >>> >>> similarly to “struct rte_eth_link”, for example. >>> >>> Documenting the port_id is derived from rte is needed here, whether it be >>> rte_port_id >>> or >>> ovs_rte_port_id >> >> OK. >> I don't like 'rte_port_id' because it's not defined inside DPDK. >> 'ovs_rte_port_id' looks too complex. >> >> What do you think about 'dpdk_port_t'? > > +1 for this from me. Since DPDK likes to use rte_X, I was a little > uncomfortable with rte_port_id. I think this is a good compromise. I've sent v4 with this change. The particular name used is not the most important aspect here. In regards to the rte_ prefix by itself, I agree it is not a good choice. After all, this is not compat code. ovs_rte_ has some precedence and implies ovs version of the more specific library referenced, being “rte”. However, dpdk_ is already a commonly used prefix here and that is good for consistency. > No opinion on the print routines. > >> It will show that this port is from DPDK and will not create a false >> sensation that type defined inside the DPDK library. >> >> Additionally it will look similar to other port id types in OVS: >> >> include/openvswitch/types.h: >> >> typedef uint32_t OVS_BITWISE ofp_port_t; >> typedef uint32_t OVS_BITWISE odp_port_t; >> typedef uint32_t OVS_BITWISE ofp11_port_t; >> >> >>> > >>> > Here is a few examples of typedefs in the OVS code base: >>> > >>> > dpif-netdev.c:5587: typedef uint32_t map_type; >>> > stp.h:41:typedef uint64_t stp_identifier; >>> > timeval.c:45:typedef unsigned int clockid_t; >>> > >>> > >>> > dball@ubuntu:~/ovs$ git diff >>> > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >>> > index 609b8da..6667274 100644 >>> > --- a/lib/netdev-dpdk.c >>> > +++ b/lib/netdev-dpdk.c >>>> @@ -143,6 +143,9 @@ BUILD_ASSERT_DECL((MAX_NB_MBUF / >> ROUND_DOWN_POW2(MAX_NB_MBUF/MIN_NB_MBUF)) >>> > #define VHOST_ENQ_RETRY_NUM 8 >>> > #define IF_NAME_SZ (PATH_MAX > IFNAMSIZ ? PATH_MAX : IFNAMSIZ) >>> > >>> > +#define DPDK_ETH_PORT_ID_INVALID RTE_MAX_ETHPORTS >>> > +typedef uint8_t rte_port_id; >>> > + >>> > static const struct rte_eth_conf port_conf = { >>> > .rxmode = { >>> > .mq_mode = ETH_MQ_RX_RSS, >>> > @@ -309,7 +312,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 */ >>> > + rte_port_id eth_port_id; /* ethernet device port id */ >>> > struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); >>> > }; >>> > >>> > @@ -325,7 +328,7 @@ enum dpdk_hw_ol_features { >>> > >>> > struct netdev_dpdk { >>> > struct netdev up; >>> > - int port_id; >>> > + rte_port_id port_id; >>> > int max_packet_len; >>> > enum dpdk_dev_type type; >>> > >>> > @@ -399,7 +402,7 @@ struct netdev_dpdk { >>> > >>> > struct netdev_rxq_dpdk { >>> > struct netdev_rxq up; >>> > - int port_id; >>> > + rte_port_id port_id; >>> > }; >>> > >>> > static int netdev_dpdk_class_init(void); >>> > @@ -600,12 +603,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); >>> > } >>> > } >>> > } >>>> @@ -723,7 +726,7 @@ 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", >>>> + VLOG_WARN_ONCE("Rx checksum offload is not supported on device >> %"PRIu8, >>> > dev->port_id); >>> > dev->hw_ol_features &= ~NETDEV_RX_CHECKSUM_OFFLOAD; >>> > return; >>> > @@ -735,7 +738,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); >>> > } >>> > } >>> > >>> > @@ -773,7 +777,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); >>> > @@ -785,7 +789,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); >>> > } >>> > >>> > @@ -830,7 +834,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, rte_port_id port_no, >>> > enum dpdk_dev_type type, int socket_id) >>> > OVS_REQUIRES(dpdk_mutex) >>> > { >>> > @@ -914,7 +918,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 >>> > @@ -974,7 +979,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; >>> > } >>>> @@ -1097,7 +1103,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(rte_port_id port_id) >>> > OVS_REQUIRES(dpdk_mutex) >>> > { >>> > struct netdev_dpdk *dev; >>> > @@ -1111,7 +1117,7 @@ netdev_dpdk_lookup_by_port_id(int port_id) >>> > return NULL; >>> > } >>> > >>> > -static int >>> > +static rte_port_id >>> > netdev_dpdk_process_devargs(const char *devargs, char **errp) >>> > { >>> > uint8_t new_port_id = UINT8_MAX; >>>> @@ -1126,7 +1132,7 @@ netdev_dpdk_process_devargs(const char >> *devargs, char **errp) >>> > } else { >>> > /* Attach unsuccessful */ >>>> VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", >> devargs); >>> > - return -1; >>> > + new_port_id = DPDK_ETH_PORT_ID_INVALID; >>> > } >>> > } >>> > >>>> @@ -1206,7 +1212,8 @@ 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(new_devargs, >> errp); >>>> + rte_port_id new_port_id = >> netdev_dpdk_process_devargs(new_devargs, >>>> + errp); >>> > if (!rte_eth_dev_is_valid_port(new_port_id)) { >>> > err = EINVAL; >>> > } else if (new_port_id == dev->port_id) { >>>> @@ -2028,7 +2035,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; >>> > } >>>> @@ -2036,7 +2043,8 @@ 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 names for port: %"PRIu8, >> dev->port_id); >>> > + >>> > goto out; >>> > } >>> > /* Reserve memory for xstats names and values */ >>>> @@ -2059,7 +2067,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: >>> > @@ -2785,7 +2793,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) >>> > + rte_port_id *eth_port_id) >>> > { >>> > struct dpdk_ring *ring_pair; >>> > char *ring_name; >>>> @@ -2837,7 +2845,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[], rte_port_id *eth_port_id) >>> > OVS_REQUIRES(dpdk_mutex) >>> > { >>> > struct dpdk_ring *ring_pair; >>>> @@ -2886,7 +2894,7 @@ netdev_dpdk_ring_send(struct netdev *netdev, >> int qid, >>> > static int >>> > netdev_dpdk_ring_construct(struct netdev *netdev) >>> > { >>> > - unsigned int port_no = 0; >>> > + rte_port_id port_no = 0; >>> > int err = 0; >>> > >>> > ovs_mutex_lock(&dpdk_mutex); >>> > (END) >>> > >>> > >>> > >>> > >>> > >>> > >>> > > >>> > > struct ovs_list list_node OVS_GUARDED_BY(dpdk_mutex); >>> > > }; >>> > > >>> > > @@ -325,7 +327,7 @@ enum dpdk_hw_ol_features { >>> > > >>> > > struct netdev_dpdk { >>> > > struct netdev up; >>> > > - int port_id; >>> > > + uint8_t port_id; >>> > > int max_packet_len; >>> > > enum dpdk_dev_type type; >>> > > >>> > > @@ -402,7 +404,7 @@ struct netdev_dpdk { >>> > > >>> > > struct netdev_rxq_dpdk { >>> > > struct netdev_rxq up; >>> > > - int port_id; >>> > > + uint8_t port_id; >>> > > }; >>> > > >>> > > static int netdev_dpdk_class_init(void); >>>>> @@ -602,12 +604,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); >>> > > } >>> > > } >>> > > } >>>>> @@ -725,8 +727,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; >>> > > } >>> > > @@ -737,7 +739,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); >>> > > } >>> > > } >>> > > >>> > > @@ -775,7 +778,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); >>> > > @@ -787,7 +790,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); >>> > > } >>> > > >>> > > @@ -832,7 +835,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, uint8_t port_no, >>> > > enum dpdk_dev_type type, int socket_id) >>> > > OVS_REQUIRES(dpdk_mutex) >>> > > { >>>>> @@ -917,7 +920,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 >>>>> @@ -977,7 +981,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; >>> > > } >>>>> @@ -1111,7 +1116,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(uint8_t port_id) >>> > > OVS_REQUIRES(dpdk_mutex) >>> > > { >>> > > struct netdev_dpdk *dev; >>>>> @@ -1125,10 +1130,11 @@ netdev_dpdk_lookup_by_port_id(int >> port_id) >>> > > return NULL; >>> > > } >>> > > >>> > > -static int >>> > > -netdev_dpdk_process_devargs(const char *devargs, char **errp) >>> > > +static uint8_t >>> > > +netdev_dpdk_process_devargs(struct netdev_dpdk *dev, >>> > > + const char *devargs, char **errp) >>> > > { >>> > > - uint8_t new_port_id = UINT8_MAX; >>> > > + uint8_t new_port_id = DPDK_ETH_PORT_ID_INVALID; >>> > > char *ind, *name = xstrdup(devargs); >>> > > >>>>> /* Get the name from the comma separated list of arguments. */ >>>>> @@ -1148,7 +1154,7 @@ netdev_dpdk_process_devargs(const char >> *devargs, char **errp) >>> > > } else { >>> > > /* Attach unsuccessful */ >>>>> VLOG_WARN_BUF(errp, "Error attaching device '%s' to DPDK", >> devargs); >>> > > - return -1; >>> > > + new_port_id = DPDK_ETH_PORT_ID_INVALID; >>> > > } >>> > > } >>> > > >>>>> @@ -1229,7 +1235,8 @@ 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(new_devargs, >> errp); >>>>> + uint8_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 +1244,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 +1254,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; >>>>> @@ -2051,7 +2060,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; >>> > > } >>>>> @@ -2059,7 +2068,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 */ >>>>> @@ -2071,7 +2080,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 */ >>>>> @@ -2082,7 +2091,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: >>> > > @@ -2759,7 +2768,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) >>> > > + uint8_t *eth_port_id) >>> > > { >>> > > struct dpdk_ring *ring_pair; >>> > > char *ring_name; >>>>> @@ -2811,7 +2820,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[], uint8_t *eth_port_id) >>> > > OVS_REQUIRES(dpdk_mutex) >>> > > { >>> > > struct dpdk_ring *ring_pair; >>>>> @@ -2860,7 +2869,7 @@ netdev_dpdk_ring_send(struct netdev >> *netdev, int qid, >>> > > static int >>> > > netdev_dpdk_ring_construct(struct netdev *netdev) >>> > > { >>> > > - unsigned int port_no = 0; >>> > > + uint8_t port_no = 0; >>> > > int err = 0; >>> > > >>> > > ovs_mutex_lock(&dpdk_mutex); >>> > > -- >>> > > 2.7.4 >>> > > >>> > > _______________________________________________ >>> > > dev mailing list >>> > > d...@openvswitch.org >>>>> >> https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=CsrWZo9SNec-_DnojNdU2sRwAPpUXsyy5cTNk5hkhaQ&s=6OANH43fV6QTakoajvWrqyZoezwlGPQF4pLjlhA50ss&e= >>> > > >>> > > >>> > >>> > >>> >>> > > > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev