[ovs-dev] [PATCH 3/3] netdev-dpdk: Use uint8_t for port_id.

Ilya Maximets i.maximets at samsung.com
Thu May 18 13:14:22 UTC 2017


On 17.05.2017 18:32, Darrell Ball wrote:
> 
> 
> On 5/17/17, 7:59 AM, "Ilya Maximets" <i.maximets at 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.maximets at samsung.com> wrote:
>     > 
>     >     On 05.04.2017 22:34, Darrell Ball wrote:
>     >     > 
>     >     > 
>     >     > On 4/3/17, 8:04 AM, "ovs-dev-bounces at openvswitch.org on behalf of Ilya Maximets" <ovs-dev-bounces at openvswitch.org on behalf of i.maximets at 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.maximets at 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'?
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 at 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(&eth_addr, 0x0, sizeof(eth_addr));
>     >      rte_eth_macaddr_get(dev->port_id, &eth_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(&eth_addr, 0x0, sizeof(eth_addr));
>     >     >          rte_eth_macaddr_get(dev->port_id, &eth_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
>     >     >     dev at 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= 
>     >     >     
>     >     > 
>     >     
>     > 
>     
> 


More information about the dev mailing list