* [dpdk-dev] [PATCH] net/mlx5: fix wrong representor port link status
@ 2018-09-06 6:50 Xueming Li
2018-09-14 6:27 ` [dpdk-dev] [PATCH v2] " Xueming Li
0 siblings, 1 reply; 7+ messages in thread
From: Xueming Li @ 2018-09-06 6:50 UTC (permalink / raw)
To: Shahaf Shuler, Yongseok Koh; +Cc: Xueming Li, dev, adrien.mazarguil
Current code uses PF links status for representor port, not the representor
interface itself. This caused wrong representor port link status when
toggling linterface up or down.
Fixes: 5a4b8e2612c5 ("net/mlx5: probe all port representors")
Cc: adrien.mazarguil@6wind.com
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
drivers/net/mlx5/mlx5_ethdev.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 34c5b95..08b22ce 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -627,7 +627,7 @@ struct ethtool_link_settings {
int link_speed = 0;
int ret;
- ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
+ ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
if (ret) {
DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
dev->data->port_id, strerror(rte_errno));
@@ -636,6 +636,7 @@ struct ethtool_link_settings {
memset(&dev_link, 0, sizeof(dev_link));
dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
(ifr.ifr_flags & IFF_RUNNING));
+ memset(&ifr, 0, sizeof(ifr));
ifr.ifr_data = (void *)&edata;
ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
if (ret) {
@@ -698,7 +699,7 @@ struct ethtool_link_settings {
uint64_t sc;
int ret;
- ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
+ ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
if (ret) {
DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
dev->data->port_id, strerror(rte_errno));
@@ -707,6 +708,7 @@ struct ethtool_link_settings {
memset(&dev_link, 0, sizeof(dev_link));
dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
(ifr.ifr_flags & IFF_RUNNING));
+ memset(&ifr, 0, sizeof(ifr));
ifr.ifr_data = (void *)&gcmd;
ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
if (ret) {
--
1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v2] net/mlx5: fix wrong representor port link status
2018-09-06 6:50 [dpdk-dev] [PATCH] net/mlx5: fix wrong representor port link status Xueming Li
@ 2018-09-14 6:27 ` Xueming Li
2018-09-14 16:43 ` Yongseok Koh
2018-09-19 8:27 ` [dpdk-dev] [PATCH v3] " Xueming Li
0 siblings, 2 replies; 7+ messages in thread
From: Xueming Li @ 2018-09-14 6:27 UTC (permalink / raw)
To: Shahaf Shuler, Yongseok Koh; +Cc: Xueming Li, dev, adrien.mazarguil
Current code uses PF links status for representor port, not the representor
interface itself. This caused wrong representor port link status when
toggling linterface up or down.
Fixes: 5a4b8e2612c5 ("net/mlx5: probe all port representors")
Cc: adrien.mazarguil@6wind.com
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
drivers/net/mlx5/mlx5_ethdev.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 34c5b95..7391ab8 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -627,7 +627,7 @@ struct ethtool_link_settings {
int link_speed = 0;
int ret;
- ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
+ ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
if (ret) {
DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
dev->data->port_id, strerror(rte_errno));
@@ -636,6 +636,7 @@ struct ethtool_link_settings {
memset(&dev_link, 0, sizeof(dev_link));
dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
(ifr.ifr_flags & IFF_RUNNING));
+ memset(&ifr, 0, sizeof(ifr));
ifr.ifr_data = (void *)&edata;
ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
if (ret) {
@@ -666,8 +667,9 @@ struct ethtool_link_settings {
ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
ETH_LINK_SPEED_FIXED);
- if ((dev_link.link_speed && !dev_link.link_status) ||
- (!dev_link.link_speed && dev_link.link_status)) {
+ if (!priv->representor &&
+ ((dev_link.link_speed && !dev_link.link_status) ||
+ (!dev_link.link_speed && dev_link.link_status))) {
rte_errno = EAGAIN;
return -rte_errno;
}
@@ -698,7 +700,7 @@ struct ethtool_link_settings {
uint64_t sc;
int ret;
- ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
+ ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
if (ret) {
DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
dev->data->port_id, strerror(rte_errno));
@@ -707,6 +709,7 @@ struct ethtool_link_settings {
memset(&dev_link, 0, sizeof(dev_link));
dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
(ifr.ifr_flags & IFF_RUNNING));
+ memset(&ifr, 0, sizeof(ifr));
ifr.ifr_data = (void *)&gcmd;
ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
if (ret) {
@@ -775,8 +778,9 @@ struct ethtool_link_settings {
ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
ETH_LINK_SPEED_FIXED);
- if ((dev_link.link_speed && !dev_link.link_status) ||
- (!dev_link.link_speed && dev_link.link_status)) {
+ if (!priv->representor &&
+ ((dev_link.link_speed && !dev_link.link_status) ||
+ (!dev_link.link_speed && dev_link.link_status))) {
rte_errno = EAGAIN;
return -rte_errno;
}
--
1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix wrong representor port link status
2018-09-14 6:27 ` [dpdk-dev] [PATCH v2] " Xueming Li
@ 2018-09-14 16:43 ` Yongseok Koh
2018-09-15 5:23 ` Xueming(Steven) Li
2018-09-19 8:27 ` [dpdk-dev] [PATCH v3] " Xueming Li
1 sibling, 1 reply; 7+ messages in thread
From: Yongseok Koh @ 2018-09-14 16:43 UTC (permalink / raw)
To: Xueming(Steven) Li; +Cc: Shahaf Shuler, dev, Adrien Mazarguil
> On Sep 13, 2018, at 11:27 PM, Xueming Li <xuemingl@mellanox.com> wrote:
>
> Current code uses PF links status for representor port, not the representor
> interface itself. This caused wrong representor port link status when
> toggling linterface up or down.
>
> Fixes: 5a4b8e2612c5 ("net/mlx5: probe all port representors")
Wrong commit SHA.
Please always check it by running
./devtools/check-git-log.sh -$n
./devtools/checkpatches.sh -n$n
> Cc: adrien.mazarguil@6wind.com
>
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
> drivers/net/mlx5/mlx5_ethdev.c | 16 ++++++++++------
> 1 file changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 34c5b95..7391ab8 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -627,7 +627,7 @@ struct ethtool_link_settings {
> int link_speed = 0;
> int ret;
>
> - ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
> + ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
> if (ret) {
> DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
> dev->data->port_id, strerror(rte_errno));
> @@ -636,6 +636,7 @@ struct ethtool_link_settings {
> memset(&dev_link, 0, sizeof(dev_link));
> dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> (ifr.ifr_flags & IFF_RUNNING));
> + memset(&ifr, 0, sizeof(ifr));
> ifr.ifr_data = (void *)&edata;
It would be enough to be done like:
ifr = {
.ifr_data = (void *)&edata,
};
And please do the same for dev_link even though it isn't relevant in the patch.
> ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
> if (ret) {
> @@ -666,8 +667,9 @@ struct ethtool_link_settings {
> ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> ETH_LINK_SPEED_FIXED);
> - if ((dev_link.link_speed && !dev_link.link_status) ||
> - (!dev_link.link_speed && dev_link.link_status)) {
> + if (!priv->representor &&
> + ((dev_link.link_speed && !dev_link.link_status) ||
> + (!dev_link.link_speed && dev_link.link_status))) {
What does this change mean?
Is it allowed for representors?
> rte_errno = EAGAIN;
> return -rte_errno;
> }
> @@ -698,7 +700,7 @@ struct ethtool_link_settings {
> uint64_t sc;
> int ret;
>
> - ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
> + ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
> if (ret) {
> DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
> dev->data->port_id, strerror(rte_errno));
> @@ -707,6 +709,7 @@ struct ethtool_link_settings {
> memset(&dev_link, 0, sizeof(dev_link));
> dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> (ifr.ifr_flags & IFF_RUNNING));
> + memset(&ifr, 0, sizeof(ifr));
Same here for dev_link and ifr.
Thanks,
Yongseok
> ifr.ifr_data = (void *)&gcmd;
> ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
> if (ret) {
> @@ -775,8 +778,9 @@ struct ethtool_link_settings {
> ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> ETH_LINK_SPEED_FIXED);
> - if ((dev_link.link_speed && !dev_link.link_status) ||
> - (!dev_link.link_speed && dev_link.link_status)) {
> + if (!priv->representor &&
> + ((dev_link.link_speed && !dev_link.link_status) ||
> + (!dev_link.link_speed && dev_link.link_status))) {
> rte_errno = EAGAIN;
> return -rte_errno;
> }
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v2] net/mlx5: fix wrong representor port link status
2018-09-14 16:43 ` Yongseok Koh
@ 2018-09-15 5:23 ` Xueming(Steven) Li
0 siblings, 0 replies; 7+ messages in thread
From: Xueming(Steven) Li @ 2018-09-15 5:23 UTC (permalink / raw)
To: Yongseok Koh; +Cc: Shahaf Shuler, dev, Adrien Mazarguil
> -----Original Message-----
> From: Yongseok Koh
> Sent: Saturday, September 15, 2018 12:43 AM
> To: Xueming(Steven) Li <xuemingl@mellanox.com>
> Cc: Shahaf Shuler <shahafs@mellanox.com>; dev@dpdk.org; Adrien Mazarguil <adrien.mazarguil@6wind.com>
> Subject: Re: [PATCH v2] net/mlx5: fix wrong representor port link status
>
>
> > On Sep 13, 2018, at 11:27 PM, Xueming Li <xuemingl@mellanox.com> wrote:
> >
> > Current code uses PF links status for representor port, not the
> > representor interface itself. This caused wrong representor port link
> > status when toggling linterface up or down.
> >
> > Fixes: 5a4b8e2612c5 ("net/mlx5: probe all port representors")
>
> Wrong commit SHA.
> Please always check it by running
> ./devtools/check-git-log.sh -$n
> ./devtools/checkpatches.sh -n$n
>
> > Cc: adrien.mazarguil@6wind.com
> >
> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > ---
> > drivers/net/mlx5/mlx5_ethdev.c | 16 ++++++++++------
> > 1 file changed, 10 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/mlx5/mlx5_ethdev.c
> > b/drivers/net/mlx5/mlx5_ethdev.c index 34c5b95..7391ab8 100644
> > --- a/drivers/net/mlx5/mlx5_ethdev.c
> > +++ b/drivers/net/mlx5/mlx5_ethdev.c
> > @@ -627,7 +627,7 @@ struct ethtool_link_settings {
> > int link_speed = 0;
> > int ret;
> >
> > - ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
> > + ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
> > if (ret) {
> > DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
> > dev->data->port_id, strerror(rte_errno)); @@ -636,6 +636,7 @@
> > struct ethtool_link_settings {
> > memset(&dev_link, 0, sizeof(dev_link));
> > dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> > (ifr.ifr_flags & IFF_RUNNING));
> > + memset(&ifr, 0, sizeof(ifr));
> > ifr.ifr_data = (void *)&edata;
>
> It would be enough to be done like:
>
> ifr = {
> .ifr_data = (void *)&edata,
> };
>
> And please do the same for dev_link even though it isn't relevant in the patch.
>
> > ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
> > if (ret) {
> > @@ -666,8 +667,9 @@ struct ethtool_link_settings {
> > ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> > dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> > ETH_LINK_SPEED_FIXED);
> > - if ((dev_link.link_speed && !dev_link.link_status) ||
> > - (!dev_link.link_speed && dev_link.link_status)) {
> > + if (!priv->representor &&
> > + ((dev_link.link_speed && !dev_link.link_status) ||
> > + (!dev_link.link_speed && dev_link.link_status))) {
>
> What does this change mean?
> Is it allowed for representors?
Performance the check only if not representor port.
For representor port, the status not consistent here due to speed info retrieved from PF.
>
> > rte_errno = EAGAIN;
> > return -rte_errno;
> > }
> > @@ -698,7 +700,7 @@ struct ethtool_link_settings {
> > uint64_t sc;
> > int ret;
> >
> > - ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
> > + ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
> > if (ret) {
> > DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
> > dev->data->port_id, strerror(rte_errno)); @@ -707,6 +709,7 @@
> > struct ethtool_link_settings {
> > memset(&dev_link, 0, sizeof(dev_link));
> > dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> > (ifr.ifr_flags & IFF_RUNNING));
> > + memset(&ifr, 0, sizeof(ifr));
>
> Same here for dev_link and ifr.
>
> Thanks,
> Yongseok
>
> > ifr.ifr_data = (void *)&gcmd;
> > ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
> > if (ret) {
> > @@ -775,8 +778,9 @@ struct ethtool_link_settings {
> > ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> > dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> > ETH_LINK_SPEED_FIXED);
> > - if ((dev_link.link_speed && !dev_link.link_status) ||
> > - (!dev_link.link_speed && dev_link.link_status)) {
> > + if (!priv->representor &&
> > + ((dev_link.link_speed && !dev_link.link_status) ||
> > + (!dev_link.link_speed && dev_link.link_status))) {
> > rte_errno = EAGAIN;
> > return -rte_errno;
> > }
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH v3] net/mlx5: fix wrong representor port link status
2018-09-14 6:27 ` [dpdk-dev] [PATCH v2] " Xueming Li
2018-09-14 16:43 ` Yongseok Koh
@ 2018-09-19 8:27 ` Xueming Li
2018-09-28 0:13 ` Yongseok Koh
1 sibling, 1 reply; 7+ messages in thread
From: Xueming Li @ 2018-09-19 8:27 UTC (permalink / raw)
To: Shahaf Shuler, Yongseok Koh; +Cc: Xueming Li, dev, adrien.mazarguil
Current code uses PF links status for representor port, not the
representor
interface itself. This caused wrong representor port link status when
toggling linterface up or down.
Fixes: 2b7302638898 ("net/mlx5: probe all port representors")
Cc: adrien.mazarguil@6wind.com
Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
drivers/net/mlx5/mlx5_ethdev.c | 36 ++++++++++++++++++++++--------------
1 file changed, 22 insertions(+), 14 deletions(-)
diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
index 34c5b95..5515fdb 100644
--- a/drivers/net/mlx5/mlx5_ethdev.c
+++ b/drivers/net/mlx5/mlx5_ethdev.c
@@ -627,16 +627,19 @@ struct ethtool_link_settings {
int link_speed = 0;
int ret;
- ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
+ ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
if (ret) {
DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
dev->data->port_id, strerror(rte_errno));
return ret;
}
- memset(&dev_link, 0, sizeof(dev_link));
- dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
- (ifr.ifr_flags & IFF_RUNNING));
- ifr.ifr_data = (void *)&edata;
+ dev_link = (struct rte_eth_link) {
+ .link_status = ((ifr.ifr_flags & IFF_UP) &&
+ (ifr.ifr_flags & IFF_RUNNING)),
+ };
+ ifr = (struct ifreq) {
+ .ifr_data = (void *)&edata,
+ };
ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
if (ret) {
DRV_LOG(WARNING,
@@ -666,8 +669,9 @@ struct ethtool_link_settings {
ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
ETH_LINK_SPEED_FIXED);
- if ((dev_link.link_speed && !dev_link.link_status) ||
- (!dev_link.link_speed && dev_link.link_status)) {
+ if (!priv->representor &&
+ ((dev_link.link_speed && !dev_link.link_status) ||
+ (!dev_link.link_speed && dev_link.link_status))) {
rte_errno = EAGAIN;
return -rte_errno;
}
@@ -698,16 +702,19 @@ struct ethtool_link_settings {
uint64_t sc;
int ret;
- ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
+ ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
if (ret) {
DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
dev->data->port_id, strerror(rte_errno));
return ret;
}
- memset(&dev_link, 0, sizeof(dev_link));
- dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
- (ifr.ifr_flags & IFF_RUNNING));
- ifr.ifr_data = (void *)&gcmd;
+ dev_link = (struct rte_eth_link) {
+ .link_status = ((ifr.ifr_flags & IFF_UP) &&
+ (ifr.ifr_flags & IFF_RUNNING)),
+ };
+ ifr = (struct ifreq) {
+ .ifr_data = (void *)&gcmd,
+ };
ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
if (ret) {
DRV_LOG(DEBUG,
@@ -775,8 +782,9 @@ struct ethtool_link_settings {
ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
ETH_LINK_SPEED_FIXED);
- if ((dev_link.link_speed && !dev_link.link_status) ||
- (!dev_link.link_speed && dev_link.link_status)) {
+ if (!priv->representor &&
+ ((dev_link.link_speed && !dev_link.link_status) ||
+ (!dev_link.link_speed && dev_link.link_status))) {
rte_errno = EAGAIN;
return -rte_errno;
}
--
1.8.3.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/mlx5: fix wrong representor port link status
2018-09-19 8:27 ` [dpdk-dev] [PATCH v3] " Xueming Li
@ 2018-09-28 0:13 ` Yongseok Koh
2018-10-04 20:42 ` Thomas Monjalon
0 siblings, 1 reply; 7+ messages in thread
From: Yongseok Koh @ 2018-09-28 0:13 UTC (permalink / raw)
To: Xueming(Steven) Li; +Cc: Shahaf Shuler, dev, Adrien Mazarguil
> On Sep 19, 2018, at 1:27 AM, Xueming Li <xuemingl@mellanox.com> wrote:
>
> Current code uses PF links status for representor port, not the
> representor
> interface itself. This caused wrong representor port link status when
> toggling linterface up or down.
>
> Fixes: 2b7302638898 ("net/mlx5: probe all port representors")
> Cc: adrien.mazarguil@6wind.com
>
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> ---
Acked-by: Yongseok Koh <yskoh@mellanox.com>
Thanks
> drivers/net/mlx5/mlx5_ethdev.c | 36 ++++++++++++++++++++++--------------
> 1 file changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/mlx5/mlx5_ethdev.c b/drivers/net/mlx5/mlx5_ethdev.c
> index 34c5b95..5515fdb 100644
> --- a/drivers/net/mlx5/mlx5_ethdev.c
> +++ b/drivers/net/mlx5/mlx5_ethdev.c
> @@ -627,16 +627,19 @@ struct ethtool_link_settings {
> int link_speed = 0;
> int ret;
>
> - ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
> + ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
> if (ret) {
> DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
> dev->data->port_id, strerror(rte_errno));
> return ret;
> }
> - memset(&dev_link, 0, sizeof(dev_link));
> - dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> - (ifr.ifr_flags & IFF_RUNNING));
> - ifr.ifr_data = (void *)&edata;
> + dev_link = (struct rte_eth_link) {
> + .link_status = ((ifr.ifr_flags & IFF_UP) &&
> + (ifr.ifr_flags & IFF_RUNNING)),
> + };
> + ifr = (struct ifreq) {
> + .ifr_data = (void *)&edata,
> + };
> ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
> if (ret) {
> DRV_LOG(WARNING,
> @@ -666,8 +669,9 @@ struct ethtool_link_settings {
> ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> ETH_LINK_SPEED_FIXED);
> - if ((dev_link.link_speed && !dev_link.link_status) ||
> - (!dev_link.link_speed && dev_link.link_status)) {
> + if (!priv->representor &&
> + ((dev_link.link_speed && !dev_link.link_status) ||
> + (!dev_link.link_speed && dev_link.link_status))) {
> rte_errno = EAGAIN;
> return -rte_errno;
> }
> @@ -698,16 +702,19 @@ struct ethtool_link_settings {
> uint64_t sc;
> int ret;
>
> - ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 1);
> + ret = mlx5_ifreq(dev, SIOCGIFFLAGS, &ifr, 0);
> if (ret) {
> DRV_LOG(WARNING, "port %u ioctl(SIOCGIFFLAGS) failed: %s",
> dev->data->port_id, strerror(rte_errno));
> return ret;
> }
> - memset(&dev_link, 0, sizeof(dev_link));
> - dev_link.link_status = ((ifr.ifr_flags & IFF_UP) &&
> - (ifr.ifr_flags & IFF_RUNNING));
> - ifr.ifr_data = (void *)&gcmd;
> + dev_link = (struct rte_eth_link) {
> + .link_status = ((ifr.ifr_flags & IFF_UP) &&
> + (ifr.ifr_flags & IFF_RUNNING)),
> + };
> + ifr = (struct ifreq) {
> + .ifr_data = (void *)&gcmd,
> + };
> ret = mlx5_ifreq(dev, SIOCETHTOOL, &ifr, 1);
> if (ret) {
> DRV_LOG(DEBUG,
> @@ -775,8 +782,9 @@ struct ethtool_link_settings {
> ETH_LINK_HALF_DUPLEX : ETH_LINK_FULL_DUPLEX);
> dev_link.link_autoneg = !(dev->data->dev_conf.link_speeds &
> ETH_LINK_SPEED_FIXED);
> - if ((dev_link.link_speed && !dev_link.link_status) ||
> - (!dev_link.link_speed && dev_link.link_status)) {
> + if (!priv->representor &&
> + ((dev_link.link_speed && !dev_link.link_status) ||
> + (!dev_link.link_speed && dev_link.link_status))) {
> rte_errno = EAGAIN;
> return -rte_errno;
> }
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH v3] net/mlx5: fix wrong representor port link status
2018-09-28 0:13 ` Yongseok Koh
@ 2018-10-04 20:42 ` Thomas Monjalon
0 siblings, 0 replies; 7+ messages in thread
From: Thomas Monjalon @ 2018-10-04 20:42 UTC (permalink / raw)
To: Xueming(Steven) Li; +Cc: dev, Yongseok Koh, Shahaf Shuler, Adrien Mazarguil
28/09/2018 02:13, Yongseok Koh:
>
> > On Sep 19, 2018, at 1:27 AM, Xueming Li <xuemingl@mellanox.com> wrote:
> >
> > Current code uses PF links status for representor port, not the
> > representor
> > interface itself. This caused wrong representor port link status when
> > toggling linterface up or down.
> >
> > Fixes: 2b7302638898 ("net/mlx5: probe all port representors")
> > Cc: adrien.mazarguil@6wind.com
> >
> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > ---
> Acked-by: Yongseok Koh <yskoh@mellanox.com>
applied to dpdk-next-net-mlx
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-10-04 20:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-06 6:50 [dpdk-dev] [PATCH] net/mlx5: fix wrong representor port link status Xueming Li
2018-09-14 6:27 ` [dpdk-dev] [PATCH v2] " Xueming Li
2018-09-14 16:43 ` Yongseok Koh
2018-09-15 5:23 ` Xueming(Steven) Li
2018-09-19 8:27 ` [dpdk-dev] [PATCH v3] " Xueming Li
2018-09-28 0:13 ` Yongseok Koh
2018-10-04 20:42 ` Thomas Monjalon
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).