DPDK patches and discussions
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Yuanhan Liu <yliu@fridaylinux.org>, dev@dpdk.org
Cc: Thomas Monjalon <thomas@monjalon.net>,
	Adrien Mazarguil <adrien.mazarguil@6wind.com>,
	Ciara Loftus <ciara.loftus@intel.com>,
	Kevin Traynor <ktraynor@redhat.com>
Subject: Re: [dpdk-dev] [PATCH] [RFC] ether: standardize getting the port by name
Date: Tue, 16 Jan 2018 20:20:03 +0000	[thread overview]
Message-ID: <df671b23-c02c-0677-33bf-dedaee16851c@intel.com> (raw)
In-Reply-To: <1512027330-30030-1-git-send-email-yliu@fridaylinux.org>

On 11/30/2017 7:35 AM, Yuanhan Liu wrote:
> The ethdev name is taken from the "name" parameter from the helper
> function rte_eth_dev_allocate(name). The name is given by the caller,
> thus, there is no way to guarantee all the callers will follow the
> same syntax, leaving us the port name is not standardized.
> 
> For example, for all ports probed by the generic pci probe function,
> the name is set to the PCI id. For all others, it's set by the caller,
> aka, the PMD driver. Taking mlx PMD driver as the example, it's set
> to something like "mlx5_0 port 0".
> 
> Unfortunately, ovs-dpdk uses such name for referencing a specific port.
> Since there is no standard, user has to figure out what is the right
> name for the nic he is using. That adds extra (unnecessary) obstruction
> to users.

Can you please give more details about the ovs-dpdk usage?

I assume ovs-dpdk creates some virtual/physical devices while started, and
during switch configuration it need to access those devices and this is done by
getting device by name. That is why need to know the device name.

If above assumption correct, can supporting a generic id=x devargs help? Ethdev
can guarantie that unique ids provided are unique and provides API to get device
by id.
And in ovs-dpdk can use those ids directly, can this work?

> 
> Thus, the name should be standardized. We should give user a consistent
> interface for finding a specific port.
> 
> What this patch proposes is to use "name[,mac]" syntax. "name" is the
> PCI id for pci device. For vdev, it's the vdev name given by user. The
> reason "mac" is needed is for some devices (say ConnectX-3), 2 ports
> (in a single NIC) have the same PCI id.
> 
> There are 2 reasons I didn't make "mac" mandatory:
> - it keeps the compatibility
> - in most cases, the pci id is good enough to identify a port
> 
> However, while writing this commit log, I think it might be better to
> use something like UUID for standardizing the port name. This way, we
> will always have a very consistent naming, no matter whether it's PCI
> device or vdev device and whether a PCI devices has 2 ports share the
> same PIC id, or something we have considered so far (say, few ports
> sharing same PCI and mac address :/).
> 
> It's also simpler and cleaner. The only drawback is such ID is meaningless
> to human.
> 
> Please also note that this patch just comes up with an API to query
> a port from standard name suggested above. The ethdev name isn't really
> standardized here. This patch is asking for comments after all.
> 
> Thoughts?
> 
> Cc: Thomas Monjalon <thomas@monjalon.net>
> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Cc: Ciara Loftus <ciara.loftus@intel.com>
> Cc: Kevin Traynor <ktraynor@redhat.com>
> Signed-off-by: Yuanhan Liu <yliu@fridaylinux.org>
> ---
>  lib/librte_ether/rte_ethdev.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>  lib/librte_ether/rte_ethdev.h | 28 ++++++++++++++++++++
>  2 files changed, 89 insertions(+)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index 318af28..acf29e7 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -362,6 +362,67 @@ rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id)
>  	return -ENODEV;
>  }
>  
> +static int
> +str_to_ether(const char *mac, struct ether_addr *ea)
> +{
> +	unsigned int bytes[6];
> +	int i;
> +
> +	if (sscanf(mac, "%x:%x:%x:%x:%x:%x",
> +		   &bytes[0], &bytes[1], &bytes[2],
> +		   &bytes[3], &bytes[4], &bytes[5]) != 6) {
> +		return -1;
> +	}
> +
> +	for (i = 0; i < 6; i++)
> +		ea->addr_bytes[i] = bytes[i];
> +
> +	return 0;
> +}
> +
> +int
> +rte_eth_dev_get_port(const char *devarg, uint16_t *port_id)
> +{
> +	int i;
> +	char *name;
> +	char *mac;
> +	struct ether_addr ea;
> +	int nr_match = 0;
> +
> +	if (devarg == NULL)
> +		return -EINVAL;
> +
> +	name = strdup(devarg);
> +	mac  = strchr(name, ',');
> +	if (mac) {
> +		*mac++ = '\0';
> +		if (str_to_ether(mac, &ea) < 0)
> +			return -EINVAL;
> +	}
> +
> +	RTE_ETH_FOREACH_DEV(i) {
> +		if (strncmp(name, rte_eth_dev_data[i].name, strlen(name)) == 0) {
> +			*port_id = i;
> +
> +			/* if mac is given, mac has to be matched also */
> +			if (mac && !is_same_ether_addr(&ea,
> +					&rte_eth_dev_data[i].mac_addrs[0]))
> +				continue;
> +
> +			*port_id = i;
> +			nr_match += 1;
> +		}
> +	}
> +
> +	if (nr_match > 1) {
> +		RTE_LOG(ERR, EAL, "%d ports found with devarg: %s\n",
> +				nr_match, devarg);
> +		return -EINVAL;
> +	}
> +
> +	return -ENODEV;
> +}
> +
>  /* attach the new device, then store port_id of the device */
>  int
>  rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
> diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
> index 341c2d6..3e131b6 100644
> --- a/lib/librte_ether/rte_ethdev.h
> +++ b/lib/librte_ether/rte_ethdev.h
> @@ -4555,6 +4555,34 @@ int
>  rte_eth_dev_get_port_by_name(const char *name, uint16_t *port_id);
>  
>  /**
> +* Get the port id from the devarg
> +*
> +* The standard syntax for the devarg is "name[,mac]".
> +*
> +* For PCI device, "name" is the PCI id, for example, "0000:2:00.0".
> +* For vdev, it's the vdev name, for example, "net_pcap0".
> +*
> +* The "mac" is optional, as in most cases, the name (say, the PCI
> +* id) is good enough to identify a specific port. However, there
> +* are cases that multiple ports share a single PCI id. In such case,
> +* mac is needed to identify the specific port.
> +*
> +* If more than one port is found by the given devarg, -EINVAL will
> +* be returned. An error message will also be dumped.
> +*
> +* @param devarg
> +*   a standard devarg string
> +* @param port_id
> +*   pointer to port identifier of the device
> +* @return
> +*   - 0 if successful and port_id is filled.
> +*   - -ENODEV if no port found
> +*   - -EINVAL for invalid devarg
> +*/
> +int
> +rte_eth_dev_get_port(const char *devarg, uint16_t *port_id);
> +
> +/**
>  * Get the device name from port id
>  *
>  * @param port_id
> 

      parent reply	other threads:[~2018-01-16 20:20 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-30  7:35 Yuanhan Liu
2017-11-30 17:15 ` Stephen Hemminger
2017-11-30 17:35   ` Thomas Monjalon
2017-11-30 21:21     ` Stephen Hemminger
2017-11-30 21:44       ` Thomas Monjalon
2017-12-01  9:47         ` Gaëtan Rivet
2017-12-04 13:55           ` Yuanhan Liu
2017-12-05 11:04             ` Adrien Mazarguil
2017-12-05 13:20               ` Thomas Monjalon
2017-12-05 13:58                 ` Yuanhan Liu
2017-12-05 15:28                   ` Thomas Monjalon
2017-12-05 17:22                     ` Adrien Mazarguil
2017-12-06 15:49                       ` Yuanhan Liu
2017-12-18 22:25                 ` Thomas Monjalon
2017-12-18 22:30                   ` Stephen Hemminger
2017-12-18 22:41                     ` Thomas Monjalon
2017-12-18 23:05 ` Thomas Monjalon
2017-12-20 22:02   ` [dpdk-dev] standardize device identification Thomas Monjalon
2017-12-22  7:01     ` Shreyansh Jain
2017-12-22  9:00       ` Thomas Monjalon
2018-01-05  7:52     ` Finn Christensen
2018-01-05  8:39       ` Thomas Monjalon
2018-01-05 11:09         ` Finn Christensen
2018-01-05 12:01           ` Thomas Monjalon
2018-01-05 14:14             ` Finn Christensen
2018-01-05 15:34               ` Thomas Monjalon
2018-01-05 20:32                 ` Finn Christensen
2018-01-16 20:20 ` Ferruh Yigit [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=df671b23-c02c-0677-33bf-dedaee16851c@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=ciara.loftus@intel.com \
    --cc=dev@dpdk.org \
    --cc=ktraynor@redhat.com \
    --cc=thomas@monjalon.net \
    --cc=yliu@fridaylinux.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).