On 5/19/17, 6:39 AM, "Ilya Maximets" <[email protected]> wrote:
On 18.05.2017 16:34, Aaron Conole wrote:
> Hi Ilya,
>
> Ilya Maximets <[email protected]> writes:
>
>> On 17.05.2017 18:32, Darrell Ball wrote:
>>>
>>>
>>> On 5/17/17, 7:59 AM, "Ilya Maximets" <[email protected]> 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" <[email protected]>
wrote:
>>> >
>>> > On 05.04.2017 22:34, Darrell Ball wrote:
>>> > >
>>> > >
>>>>> On 4/3/17, 8:04 AM, "[email protected] on behalf
>> of Ilya Maximets" <[email protected] on behalf of
>> [email protected]> 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 <[email protected]>
>>> > > ---
>>>>> 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
>>> > > [email protected]
>>>>>
>>
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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
