* [dpdk-dev] [PATCH] net/mlx: remove link update lock
@ 2017-01-11 16:44 Olivier Matz
2017-01-16 14:03 ` Adrien Mazarguil
0 siblings, 1 reply; 3+ messages in thread
From: Olivier Matz @ 2017-01-11 16:44 UTC (permalink / raw)
To: dev, adrien.mazarguil; +Cc: Matthieu Ternisien d'Ouville
From: Matthieu Ternisien d'Ouville <matthieu.tdo@6wind.com>
Retrieving link status information through the link update callback should
be quick and non-blocking.
Mellanox PMDs retrieve this information through ioctl() calls on the
related kernel netdevice. This appears to take a long time to
complete and may cause significant slowdowns in applications.
While these system calls cannot be accelerated, removing the lock on the
private structure allows applications to perform other control operations
from separate threads in the meantime. This function remains safe without
locking as it does not write the private structure, it is only used to
retrieve the name of the netdevice.
Signed-off-by: Matthieu Ternisien d'Ouville <matthieu.tdo@6wind.com>
Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
drivers/net/mlx4/mlx4.c | 32 ++++++--------------------------
drivers/net/mlx5/mlx5.c | 2 +-
drivers/net/mlx5/mlx5.h | 1 -
drivers/net/mlx5/mlx5_ethdev.c | 30 ++++++------------------------
4 files changed, 13 insertions(+), 52 deletions(-)
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index eb06f56..941176c 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -4823,7 +4823,7 @@ mlx4_allmulticast_disable(struct rte_eth_dev *dev)
}
/**
- * DPDK callback to retrieve physical link information (unlocked version).
+ * DPDK callback to retrieve physical link information.
*
* @param dev
* Pointer to Ethernet device structure.
@@ -4831,9 +4831,9 @@ mlx4_allmulticast_disable(struct rte_eth_dev *dev)
* Wait for request completion (ignored).
*/
static int
-mlx4_link_update_unlocked(struct rte_eth_dev *dev, int wait_to_complete)
+mlx4_link_update(struct rte_eth_dev *dev, int wait_to_complete)
{
- struct priv *priv = mlx4_get_priv(dev);
+ const struct priv *priv = mlx4_get_priv(dev);
struct ethtool_cmd edata = {
.cmd = ETHTOOL_GSET
};
@@ -4841,6 +4841,8 @@ mlx4_link_update_unlocked(struct rte_eth_dev *dev, int wait_to_complete)
struct rte_eth_link dev_link;
int link_speed = 0;
+ /* priv_lock() is not taken to allow concurrent calls. */
+
if (priv == NULL)
return -EINVAL;
(void)wait_to_complete;
@@ -4876,28 +4878,6 @@ mlx4_link_update_unlocked(struct rte_eth_dev *dev, int wait_to_complete)
}
/**
- * DPDK callback to retrieve physical link information.
- *
- * @param dev
- * Pointer to Ethernet device structure.
- * @param wait_to_complete
- * Wait for request completion (ignored).
- */
-static int
-mlx4_link_update(struct rte_eth_dev *dev, int wait_to_complete)
-{
- struct priv *priv = mlx4_get_priv(dev);
- int ret;
-
- if (priv == NULL)
- return -EINVAL;
- priv_lock(priv);
- ret = mlx4_link_update_unlocked(dev, wait_to_complete);
- priv_unlock(priv);
- return ret;
-}
-
-/**
* DPDK callback to change the MTU.
*
* Setting the MTU affects hardware MRU (packets larger than the MTU cannot be
@@ -5413,7 +5393,7 @@ priv_dev_link_status_handler(struct priv *priv, struct rte_eth_dev *dev)
struct rte_eth_link *link = &dev->data->dev_link;
priv->pending_alarm = 0;
- mlx4_link_update_unlocked(dev, 0);
+ mlx4_link_update(dev, 0);
if (((link->link_speed == 0) && link->link_status) ||
((link->link_speed != 0) && !link->link_status)) {
/* Inconsistent status, check again later. */
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 6293c1f..d936e72 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -680,7 +680,7 @@ mlx5_pci_probe(struct rte_pci_driver *pci_drv, struct rte_pci_device *pci_dev)
/* Bring Ethernet device up. */
DEBUG("forcing Ethernet interface up");
priv_set_flags(priv, ~IFF_UP, IFF_UP);
- mlx5_link_update_unlocked(priv->dev, 1);
+ mlx5_link_update(priv->dev, 1);
continue;
port_error:
diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h
index ee62e04..d39b199 100644
--- a/drivers/net/mlx5/mlx5.h
+++ b/drivers/net/mlx5/mlx5.h
@@ -194,7 +194,6 @@ int priv_set_flags(struct priv *, unsigned int, unsigned int);
int mlx5_dev_configure(struct rte_eth_dev *);
void mlx5_dev_infos_get(struct rte_eth_dev *, struct rte_eth_dev_info *);
const uint32_t *mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev);
-int mlx5_link_update_unlocked(struct rte_eth_dev *, int);
int mlx5_link_update(struct rte_eth_dev *, int);
int mlx5_dev_set_mtu(struct rte_eth_dev *, uint16_t);
int mlx5_dev_get_flow_ctrl(struct rte_eth_dev *, struct rte_eth_fc_conf *);
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index fbb1b65..a46526e 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -628,7 +628,7 @@ mlx5_dev_supported_ptypes_get(struct rte_eth_dev *dev)
}
/**
- * Retrieve physical link information (unlocked version using legacy ioctl).
+ * DPDK callback to retrieve physical link information.
*
* @param dev
* Pointer to Ethernet device structure.
@@ -646,6 +646,8 @@ mlx5_link_update_unlocked_gset(struct rte_eth_dev *dev, int wait_to_complete)
struct rte_eth_link dev_link;
int link_speed = 0;
+ /* priv_lock() is not taken to allow concurrent calls. */
+
(void)wait_to_complete;
if (priv_ifreq(priv, SIOCGIFFLAGS, &ifr)) {
WARN("ioctl(SIOCGIFFLAGS) failed: %s", strerror(errno));
@@ -790,7 +792,7 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, int wait_to_complete)
}
/**
- * DPDK callback to retrieve physical link information (unlocked version).
+ * DPDK callback to retrieve physical link information.
*
* @param dev
* Pointer to Ethernet device structure.
@@ -798,7 +800,7 @@ mlx5_link_update_unlocked_gs(struct rte_eth_dev *dev, int wait_to_complete)
* Wait for request completion (ignored).
*/
int
-mlx5_link_update_unlocked(struct rte_eth_dev *dev, int wait_to_complete)
+mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete)
{
int ret;
@@ -809,26 +811,6 @@ mlx5_link_update_unlocked(struct rte_eth_dev *dev, int wait_to_complete)
}
/**
- * DPDK callback to retrieve physical link information.
- *
- * @param dev
- * Pointer to Ethernet device structure.
- * @param wait_to_complete
- * Wait for request completion (ignored).
- */
-int
-mlx5_link_update(struct rte_eth_dev *dev, int wait_to_complete)
-{
- struct priv *priv = mlx5_get_priv(dev);
- int ret;
-
- priv_lock(priv);
- ret = mlx5_link_update_unlocked(dev, wait_to_complete);
- priv_unlock(priv);
- return ret;
-}
-
-/**
* DPDK callback to change the MTU.
*
* Setting the MTU affects hardware MRU (packets larger than the MTU cannot be
@@ -1164,7 +1146,7 @@ priv_dev_link_status_handler(struct priv *priv, struct rte_eth_dev *dev)
struct rte_eth_link *link = &dev->data->dev_link;
priv->pending_alarm = 0;
- mlx5_link_update_unlocked(dev, 0);
+ mlx5_link_update(dev, 0);
if (((link->link_speed == 0) && link->link_status) ||
((link->link_speed != 0) && !link->link_status)) {
/* Inconsistent status, check again later. */
--
2.8.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx: remove link update lock
2017-01-11 16:44 [dpdk-dev] [PATCH] net/mlx: remove link update lock Olivier Matz
@ 2017-01-16 14:03 ` Adrien Mazarguil
2017-01-16 17:19 ` Ferruh Yigit
0 siblings, 1 reply; 3+ messages in thread
From: Adrien Mazarguil @ 2017-01-16 14:03 UTC (permalink / raw)
To: Olivier Matz; +Cc: dev, Matthieu Ternisien d'Ouville
On Wed, Jan 11, 2017 at 05:44:01PM +0100, Olivier Matz wrote:
> From: Matthieu Ternisien d'Ouville <matthieu.tdo@6wind.com>
>
> Retrieving link status information through the link update callback should
> be quick and non-blocking.
>
> Mellanox PMDs retrieve this information through ioctl() calls on the
> related kernel netdevice. This appears to take a long time to
> complete and may cause significant slowdowns in applications.
>
> While these system calls cannot be accelerated, removing the lock on the
> private structure allows applications to perform other control operations
> from separate threads in the meantime. This function remains safe without
> locking as it does not write the private structure, it is only used to
> retrieve the name of the netdevice.
>
> Signed-off-by: Matthieu Ternisien d'Ouville <matthieu.tdo@6wind.com>
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
> drivers/net/mlx4/mlx4.c | 32 ++++++--------------------------
> drivers/net/mlx5/mlx5.c | 2 +-
> drivers/net/mlx5/mlx5.h | 1 -
> drivers/net/mlx5/mlx5_ethdev.c | 30 ++++++------------------------
> 4 files changed, 13 insertions(+), 52 deletions(-)
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
--
Adrien Mazarguil
6WIND
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [dpdk-dev] [PATCH] net/mlx: remove link update lock
2017-01-16 14:03 ` Adrien Mazarguil
@ 2017-01-16 17:19 ` Ferruh Yigit
0 siblings, 0 replies; 3+ messages in thread
From: Ferruh Yigit @ 2017-01-16 17:19 UTC (permalink / raw)
To: Adrien Mazarguil, Olivier Matz; +Cc: dev, Matthieu Ternisien d'Ouville
On 1/16/2017 2:03 PM, Adrien Mazarguil wrote:
> On Wed, Jan 11, 2017 at 05:44:01PM +0100, Olivier Matz wrote:
>> From: Matthieu Ternisien d'Ouville <matthieu.tdo@6wind.com>
>>
>> Retrieving link status information through the link update callback should
>> be quick and non-blocking.
>>
>> Mellanox PMDs retrieve this information through ioctl() calls on the
>> related kernel netdevice. This appears to take a long time to
>> complete and may cause significant slowdowns in applications.
>>
>> While these system calls cannot be accelerated, removing the lock on the
>> private structure allows applications to perform other control operations
>> from separate threads in the meantime. This function remains safe without
>> locking as it does not write the private structure, it is only used to
>> retrieve the name of the netdevice.
>>
>> Signed-off-by: Matthieu Ternisien d'Ouville <matthieu.tdo@6wind.com>
>> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
>> ---
>> drivers/net/mlx4/mlx4.c | 32 ++++++--------------------------
>> drivers/net/mlx5/mlx5.c | 2 +-
>> drivers/net/mlx5/mlx5.h | 1 -
>> drivers/net/mlx5/mlx5_ethdev.c | 30 ++++++------------------------
>> 4 files changed, 13 insertions(+), 52 deletions(-)
>
> Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Applied to dpdk-next-net/master, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-16 17:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-11 16:44 [dpdk-dev] [PATCH] net/mlx: remove link update lock Olivier Matz
2017-01-16 14:03 ` Adrien Mazarguil
2017-01-16 17:19 ` Ferruh Yigit
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).