[PATCH v3] net: Cavium: Fix MAC address setting in shutdown state

2015-06-23 Thread Pavel Fedin
This bug pops up with NetworkManager on Fedora 21. NetworkManager tends to
stop the interface (nicvf_stop() is called) before changing settings. In
stopped state MAC cannot be sent to a PF. However, when the interface is
restarted (nicvf_open() is called), we ping the PF using NIC_MBOX_MSG_READY
message, and the PF replies back with old MAC address, overriding what we
had after MAC setting from userspace. As a result, we cannot set MAC
address using NetworkManager.

This patch introduces special tracking of MAC change in stopped state so
that the correct new MAC address is sent to a PF when interface is reopen.

Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
Changes for v3:
- Fixed (actually hand-edited) file permissions to be 644. I occasionally 
pushed them as 755 to my
local repo.
Changes for v2:
- Fixed a couple of checkpatch.pl reports (was forgotten in v1)
- Adjusted email client prefs, hopefully will not corrupt patches any more.
---
 drivers/net/ethernet/cavium/thunder/nic.h|  1 +
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 14 --
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nic.h 
b/drivers/net/ethernet/cavium/thunder/nic.h
index a3b43e5..dda8a02 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -263,6 +263,7 @@ struct nicvf {
boolpf_acked;
boolpf_nacked;
boolbgx_stats_acked;
+   boolset_mac_pending;
 } cacheline_aligned_in_smp;
 
 /* PF -- VF Mailbox communication
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 02da802..8b119a0 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -201,7 +201,9 @@ static void  nicvf_handle_mbx_intr(struct nicvf *nic)
nic-vf_id = mbx.nic_cfg.vf_id  0x7F;
nic-tns_mode = mbx.nic_cfg.tns_mode  0x7F;
nic-node = mbx.nic_cfg.node_id;
-   ether_addr_copy(nic-netdev-dev_addr, mbx.nic_cfg.mac_addr);
+   if (!nic-set_mac_pending)
+   ether_addr_copy(nic-netdev-dev_addr,
+   mbx.nic_cfg.mac_addr);
nic-link_up = false;
nic-duplex = 0;
nic-speed = 0;
@@ -941,6 +943,11 @@ int nicvf_open(struct net_device *netdev)
nicvf_hw_set_mac_addr(nic, netdev);
}
 
+   if (nic-set_mac_pending) {
+   nic-set_mac_pending = false;
+   nicvf_hw_set_mac_addr(nic, netdev);
+   }
+
/* Init tasklet for handling Qset err interrupt */
tasklet_init(nic-qs_err_task, nicvf_handle_qs_err,
 (unsigned long)nic);
@@ -1040,9 +1047,12 @@ static int nicvf_set_mac_address(struct net_device 
*netdev, void *p)
 
memcpy(netdev-dev_addr, addr-sa_data, netdev-addr_len);
 
-   if (nic-msix_enabled)
+   if (nic-msix_enabled) {
if (nicvf_hw_set_mac_addr(nic, netdev))
return -EBUSY;
+   } else {
+   nic-set_mac_pending = true;
+   }
 
return 0;
 }
-- 
1.9.5.msysgit.0


--
To unsubscribe from this list: send the line unsubscribe netdev in

--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] net: Cavium: Fix MAC address setting in shutdown state

2015-06-23 Thread Pavel Fedin
This bug pops up with NetworkManager on Fedora 21. NetworkManager tends to
stop the interface (nicvf_stop() is called) before changing settings. In
stopped state MAC cannot be sent to a PF. However, when the interface is
restarted (nicvf_open() is called), we ping the PF using NIC_MBOX_MSG_READY
message, and the PF replies back with old MAC address, overriding what we
had after MAC setting from userspace. As a result, we cannot set MAC
address using NetworkManager.

This patch introduces special tracking of MAC change in stopped state so
that the correct new MAC address is sent to a PF when interface is reopen.

Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
 Differences from v1:
- Fixed a couple of checkpatch.pl reports (was forgotten in v1)
- Adjusted email client prefs, hopefully will not corrupt patches any more.
---
 drivers/net/ethernet/cavium/thunder/nic.h|  1 +
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 14 --
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nic.h 
b/drivers/net/ethernet/cavium/thunder/nic.h
index a3b43e5..dda8a02 100755
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -263,6 +263,7 @@ struct nicvf {
boolpf_acked;
boolpf_nacked;
boolbgx_stats_acked;
+   boolset_mac_pending;
 } cacheline_aligned_in_smp;
 
 /* PF -- VF Mailbox communication
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 02da802..8b119a0 100755
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -201,7 +201,9 @@ static void  nicvf_handle_mbx_intr(struct nicvf *nic)
nic-vf_id = mbx.nic_cfg.vf_id  0x7F;
nic-tns_mode = mbx.nic_cfg.tns_mode  0x7F;
nic-node = mbx.nic_cfg.node_id;
-   ether_addr_copy(nic-netdev-dev_addr, mbx.nic_cfg.mac_addr);
+   if (!nic-set_mac_pending)
+   ether_addr_copy(nic-netdev-dev_addr,
+   mbx.nic_cfg.mac_addr);
nic-link_up = false;
nic-duplex = 0;
nic-speed = 0;
@@ -941,6 +943,11 @@ int nicvf_open(struct net_device *netdev)
nicvf_hw_set_mac_addr(nic, netdev);
}
 
+   if (nic-set_mac_pending) {
+   nic-set_mac_pending = false;
+   nicvf_hw_set_mac_addr(nic, netdev);
+   }
+
/* Init tasklet for handling Qset err interrupt */
tasklet_init(nic-qs_err_task, nicvf_handle_qs_err,
 (unsigned long)nic);
@@ -1040,9 +1047,12 @@ static int nicvf_set_mac_address(struct net_device 
*netdev, void *p)
 
memcpy(netdev-dev_addr, addr-sa_data, netdev-addr_len);
 
-   if (nic-msix_enabled)
+   if (nic-msix_enabled) {
if (nicvf_hw_set_mac_addr(nic, netdev))
return -EBUSY;
+   } else {
+   nic-set_mac_pending = true;
+   }
 
return 0;
 }
-- 
1.9.5.msysgit.0


--
To unsubscribe from this list: send the line unsubscribe netdev in


[PATCH] net: Cavium: Bug fix: MAC address setting in shutdown state

2015-06-17 Thread Pavel Fedin
This bug pops up with NetworkManager on Fedora 21. NetworkManager tends to
stop the interface (nicvf_stop() is called) before changing settings. In
stopped state MAC cannot be sent to a PF. However, when the interface is
restarted (nicvf_open() is called), we ping the PF using NIC_MBOX_MSG_READY
message, and the PF replies back with old MAC address, overriding what we had
after MAC setting from userspace. As a result, we cannot set MAC address using
NetworkManager.

This patch introduces special tracking of MAC change in stopped state so that
the correct new MAC address is sent to a PF when interface is reopen.

Signed-off-by: Pavel Fedin p.fe...@samsung.com
---
 drivers/net/ethernet/cavium/thunder/nic.h|  1 +
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 12 ++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nic.h
b/drivers/net/ethernet/cavium/thunder/nic.h
index a3b43e5..dda8a02 100644
--- a/drivers/net/ethernet/cavium/thunder/nic.h
+++ b/drivers/net/ethernet/cavium/thunder/nic.h
@@ -263,6 +263,7 @@ struct nicvf {
boolpf_acked;
boolpf_nacked;
boolbgx_stats_acked;
+   boolset_mac_pending;
 } cacheline_aligned_in_smp;
 
 /* PF -- VF Mailbox communication
diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 02da802..49d7bcf 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -201,7 +201,8 @@ static void  nicvf_handle_mbx_intr(struct nicvf *nic)
nic-vf_id = mbx.nic_cfg.vf_id  0x7F;
nic-tns_mode = mbx.nic_cfg.tns_mode  0x7F;
nic-node = mbx.nic_cfg.node_id;
-   ether_addr_copy(nic-netdev-dev_addr, mbx.nic_cfg.mac_addr);
+   if (!nic-set_mac_pending)
+   ether_addr_copy(nic-netdev-dev_addr, 
mbx.nic_cfg.mac_addr);
nic-link_up = false;
nic-duplex = 0;
nic-speed = 0;
@@ -941,6 +942,11 @@ int nicvf_open(struct net_device *netdev)
nicvf_hw_set_mac_addr(nic, netdev);
}
 
+   if (nic-set_mac_pending) {
+   nic-set_mac_pending = false;
+   nicvf_hw_set_mac_addr(nic, netdev);
+   }
+
/* Init tasklet for handling Qset err interrupt */
tasklet_init(nic-qs_err_task, nicvf_handle_qs_err,
 (unsigned long)nic);
@@ -1040,9 +1046,11 @@ static int nicvf_set_mac_address(struct net_device 
*netdev, void
*p)
 
memcpy(netdev-dev_addr, addr-sa_data, netdev-addr_len);
 
-   if (nic-msix_enabled)
+   if (nic-msix_enabled) {
if (nicvf_hw_set_mac_addr(nic, netdev))
return -EBUSY;
+   } else
+   nic-set_mac_pending = true;
 
return 0;
 }
-- 
1.9.5.msysgit.0


--
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] net: smsc911x: Fix crash if loopback test fails

2015-10-29 Thread Pavel Fedin
On certain hardware in certain situations loopback test fails and the
driver gets removed. During mdiobus_unregister() instance of PHY driver
gets disposed. But by this time it has already been started using
phy_connect_direct().

PHY driver uses DELAYED_WORK in order to maintain its state. Attempting
to dispose the driver without calling phy_disconnect() causes deallocation
of DELAYED_WORK being active. This shortly causes a bad crash in timer
code.

The problem can be discovered by enabling CONFIG_DEBUG_OBJECTS_TIMERS and
CONFIG_DEBUG_OBJECTS_FREE

Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c 
b/drivers/net/ethernet/smsc/smsc911x.c
index 715f3ed..55d7a9c 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -1052,6 +1052,7 @@ static int smsc911x_mii_probe(struct net_device *dev)
 #ifdef USE_PHY_WORK_AROUND
if (smsc911x_phy_loopbacktest(dev) < 0) {
SMSC_WARN(pdata, hw, "Failed Loop Back Test");
+   phy_disconnect(phydev);
return -ENODEV;
}
SMSC_TRACE(pdata, hw, "Passed Loop Back Test");
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


PING: [PATCH] net: smsc911x: Reset PHY during initialization

2015-11-09 Thread Pavel Fedin
 Hello! So, what should we do with this?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
> Behalf Of Pavel
> Fedin
> Sent: Monday, November 02, 2015 10:19 AM
> To: 'David Miller'
> Cc: netdev@vger.kernel.org; linux-ker...@vger.kernel.org; 
> steve.glendinn...@shawell.net
> Subject: RE: [PATCH] net: smsc911x: Reset PHY during initialization
> 
>  Hello!
> 
> > > On certain hardware after software reboot the chip may get stuck and fail
> > > to reinitialize during reset. This can be fixed by ensuring that PHY is
> > > reset too.
> > >
> > > Old PHY resetting method required operational MDIO interface, therefore
> > > the chip should have been already set up. In order to be able to function
> > > during probe, it is changed to use PMT_CTRL register.
> > >
> > > The problem could be observed on SMDK5410 board.
> > >
> > > Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
> >
> > I'm pretty sure this is going to break the PHY loopback test.
> 
>  It's not (at least in normal situation), because first we do the test, and 
> only then, if it
> fails, we reset the PHY. So, during
> normal operation of the driver, when loopback test succeeds at first attempt, 
> we never attempt
> to reset the PHY. And, by the way, at
> least on my board this PHY reset did nothing useful, because after it 
> loopback test still
> failed, all 100 times.
>  And, we don't use PHY reset anywhere outside of loopback test.
> 
> > This is such a tricky and fragile area to get right, therefore I
> > want you to specifically guard any change in how PHY reset is
> > done with tests against the specific chips that have the problem.
> 
>  Well, i could do one (or some combination) of the following, if you really 
> want to:
>  a) Leave PHY reset inside loopback test as it is, but add a second routine 
> and call it only
> before smsc911x_soft_reset().
>  b) Reset PHY only conditionally, using the following algorithm:
> if smsc911x_soft_reset() {
> /* NIC reset failed, kick the PHY and retry */
> smsc911x_phy_reset()
> if (smsc_911x_soft_reset())
> return -ENODEV;
> }
>  c) Do extra PHY reset only if some hint in device tree is specified (or the 
> machine is known
> to have the problem)
> 
>  But, i dislike approach (a) for code duplication, because datasheet says 
> that both reset
> methods are equivalent; i dislike approach
> (b) for actually being a hack; and i dislike (c) for adding extra stuff which 
> seems to be not
> necessary. After all, what is wrong
> with just extra PHY reset before attempting to program anything? By the way, 
> i test my patch
> with both software reboot and hardware
> reboot, and even powercycle. Everything is quite stable.
>  Well, it's up to you to decide. But i'd like to get the fix upstreamed 
> somehow because from
> time to time we use these boards for
> tests, and it's quite annoiying to be unable to reboot them properly.
> 
> > Furthermore, I want you to test whether this has any negative
> > effects on the PHY loopback test.
> 
>  I did. I never disabled loopback test, so i actually discovered a problem 
> (even two) with it.
> First, the problem was chip reset
> timeout. By increasing it to 300ms this problem seemed to be fixed, but 
> loopback test started
> to fail. This was how i found and
> fixed crash with missing phy_disconnect(). I examined the code and discovered 
> that upon
> loopback test failure we reset the PHY and
> retry. But in my case this PHY reset never did the right thing, and all 
> loopback attempts
> (10x10 IIRC) were failing.
>  Some comments in the code gave me a clue that the main NIC can misbehave on 
> reset if e.g. PHY
> is in powerdown state. I printed
> value of its control register and discovered that it was not the case. But 
> then i discovered
> that we actually never try to reset the
> PHY before doing anything. Also, while studying the datasheed i discovered 
> that there is a
> shorthand for resetting PHY without MDIO
> bus set up, but our driver doesn't use it, preferring MDIO method instead, 
> which already
> requires operational NIC.
>  So, i suggested that PHY is just not being reset when board is rebooted by 
> software. I wrote
> the patch and it worked. I verified it
> by reverting my previous NIC reset timeout increase, and it continued to 
> work. After this
> loopback test on my board passes at first
> attempt.
> 
> Kind regards,
> Pavel Fedin

RE: PING: [PATCH] net: smsc911x: Reset PHY during initialization

2015-11-11 Thread Pavel Fedin
 Hello!

> >> If you think I should reconsider the patch, you should resubmit it.
> >
> >  I understand this, of course. But, before doing this i'd like to
> > clarify your concern, why exactly you think that loopback test will
> > break.
> 
> If I didn't reply it means I don't have anything constructive to say
> to you, and probably I'll end up agreeing with your analysis of the
> loopback test issue.
> 
> I'm not going to ask more than one more time for you to repost your
> patch.

 But, in this case, i don't have anything to change, do i? Or is it
just a formal requirement to RESEND? I can do this, if you want to.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] net: thunder: Fix crash upon shutdown after failed probe

2015-11-12 Thread Pavel Fedin
If device probe fails, driver remains bound to the PCI device. However,
driver data has been reset to NULL. This causes crash upon dereferencing
it in nicvf_remove()

Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c 
b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index a937772..372c39e 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1600,6 +1600,9 @@ static void nicvf_remove(struct pci_dev *pdev)
 
 static void nicvf_shutdown(struct pci_dev *pdev)
 {
+   if (!pci_get_drvdata(pdev))
+   return;
+
nicvf_remove(pdev);
 }
 
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH RESEND] net: smsc911x: Reset PHY during initialization

2015-11-12 Thread Pavel Fedin
On certain hardware after software reboot the chip may get stuck and fail
to reinitialize during reset. This can be fixed by ensuring that PHY is
reset too.

Old PHY resetting method required operational MDIO interface, therefore
the chip should have been already set up. In order to be able to function
during probe, it is changed to use PMT_CTRL register.

The problem could be observed on SMDK5410 board.

Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c 
b/drivers/net/ethernet/smsc/smsc911x.c
index c860c90..219a99b 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -809,22 +809,17 @@ static int smsc911x_phy_check_loopbackpkt(struct 
smsc911x_data *pdata)
 
 static int smsc911x_phy_reset(struct smsc911x_data *pdata)
 {
-   struct phy_device *phy_dev = pdata->phy_dev;
unsigned int temp;
unsigned int i = 10;
 
-   BUG_ON(!phy_dev);
-   BUG_ON(!phy_dev->bus);
-
-   SMSC_TRACE(pdata, hw, "Performing PHY BCR Reset");
-   smsc911x_mii_write(phy_dev->bus, phy_dev->addr, MII_BMCR, BMCR_RESET);
+   temp = smsc911x_reg_read(pdata, PMT_CTRL);
+   smsc911x_reg_write(pdata, PMT_CTRL, temp | PMT_CTRL_PHY_RST_);
do {
msleep(1);
-   temp = smsc911x_mii_read(phy_dev->bus, phy_dev->addr,
-   MII_BMCR);
-   } while ((i--) && (temp & BMCR_RESET));
+   temp = smsc911x_reg_read(pdata, PMT_CTRL);
+   } while ((i--) && (temp & PMT_CTRL_PHY_RST_));
 
-   if (temp & BMCR_RESET) {
+   if (unlikely(temp & PMT_CTRL_PHY_RST_)) {
SMSC_WARN(pdata, hw, "PHY reset failed to complete");
return -EIO;
}
@@ -2296,7 +2291,7 @@ static int smsc911x_init(struct net_device *dev)
}
 
/* Reset the LAN911x */
-   if (smsc911x_soft_reset(pdata))
+   if (smsc911x_phy_reset(pdata) || smsc911x_soft_reset(pdata))
return -ENODEV;
 
dev->flags |= IFF_MULTICAST;
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] net: thunder: Check for driver data in nicvf_remove()

2015-11-16 Thread Pavel Fedin
In some cases the crash is caused by nicvf_remove() being called from
outside. For example, if we try to feed the device to vfio after the
probe has failed for some reason. So, move the check to better place.

Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 13 -
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c 
b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index 372c39e..7f709cb 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1583,8 +1583,14 @@ err_disable_device:
 static void nicvf_remove(struct pci_dev *pdev)
 {
struct net_device *netdev = pci_get_drvdata(pdev);
-   struct nicvf *nic = netdev_priv(netdev);
-   struct net_device *pnetdev = nic->pnicvf->netdev;
+   struct nicvf *nic;
+   struct net_device *pnetdev;
+
+   if (!netdev)
+   return;
+
+   nic = netdev_priv(netdev);
+   pnetdev = nic->pnicvf->netdev;
 
/* Check if this Qset is assigned to different VF.
 * If yes, clean primary and all secondary Qsets.
@@ -1600,9 +1606,6 @@ static void nicvf_remove(struct pci_dev *pdev)
 
 static void nicvf_shutdown(struct pci_dev *pdev)
 {
-   if (!pci_get_drvdata(pdev))
-   return;
-
nicvf_remove(pdev);
 }
 
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] net: thunder: Fix crash upon shutdown after failed probe

2015-11-16 Thread Pavel Fedin
If device probe fails, driver remains bound to the PCI device. However,
driver data has been reset to NULL. This causes crash upon dereferencing
it in nicvf_remove()

Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
---
v1 => v2:
- Do the test in nicvf_remove(), this allows to cover more situations
---
 drivers/net/ethernet/cavium/thunder/nicvf_main.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c 
b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
index a937772..7f709cb 100644
--- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
@@ -1583,8 +1583,14 @@ err_disable_device:
 static void nicvf_remove(struct pci_dev *pdev)
 {
struct net_device *netdev = pci_get_drvdata(pdev);
-   struct nicvf *nic = netdev_priv(netdev);
-   struct net_device *pnetdev = nic->pnicvf->netdev;
+   struct nicvf *nic;
+   struct net_device *pnetdev;
+
+   if (!netdev)
+   return;
+
+   nic = netdev_priv(netdev);
+   pnetdev = nic->pnicvf->netdev;
 
/* Check if this Qset is assigned to different VF.
 * If yes, clean primary and all secondary Qsets.
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] net: thunder: Fix crash upon shutdown after failed probe

2015-11-16 Thread Pavel Fedin
 Hello!

> > If device probe fails, driver remains bound to the PCI device. However,
> > driver data has been reset to NULL. This causes crash upon dereferencing
> > it in nicvf_remove()
> >
> > Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
> 
> Applied, thanks.

 Thank you very much, however i've just found one more case where i still get 
this crash. I should have done this from the
beginning, but please revert it in your repo, i'm sending v2, which covers more 
cases.
 The new case is feeding the device to vfio after the probe failed. In this 
case nicvf_remove() is called upon unbind.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] net: smsc911x: Reset PHY during initialization

2015-10-30 Thread Pavel Fedin
On certain hardware after software reboot the chip may get stuck and fail
to reinitialize during reset. This can be fixed by ensuring that PHY is
reset too.

Old PHY resetting method required operational MDIO interface, therefore
the chip should have been already set up. In order to be able to function
during probe, it is changed to use PMT_CTRL register.

The problem could be observed on SMDK5410 board.

Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
---
 drivers/net/ethernet/smsc/smsc911x.c | 17 ++---
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/smsc/smsc911x.c 
b/drivers/net/ethernet/smsc/smsc911x.c
index c860c90..219a99b 100644
--- a/drivers/net/ethernet/smsc/smsc911x.c
+++ b/drivers/net/ethernet/smsc/smsc911x.c
@@ -809,22 +809,17 @@ static int smsc911x_phy_check_loopbackpkt(struct 
smsc911x_data *pdata)
 
 static int smsc911x_phy_reset(struct smsc911x_data *pdata)
 {
-   struct phy_device *phy_dev = pdata->phy_dev;
unsigned int temp;
unsigned int i = 10;
 
-   BUG_ON(!phy_dev);
-   BUG_ON(!phy_dev->bus);
-
-   SMSC_TRACE(pdata, hw, "Performing PHY BCR Reset");
-   smsc911x_mii_write(phy_dev->bus, phy_dev->addr, MII_BMCR, BMCR_RESET);
+   temp = smsc911x_reg_read(pdata, PMT_CTRL);
+   smsc911x_reg_write(pdata, PMT_CTRL, temp | PMT_CTRL_PHY_RST_);
do {
msleep(1);
-   temp = smsc911x_mii_read(phy_dev->bus, phy_dev->addr,
-   MII_BMCR);
-   } while ((i--) && (temp & BMCR_RESET));
+   temp = smsc911x_reg_read(pdata, PMT_CTRL);
+   } while ((i--) && (temp & PMT_CTRL_PHY_RST_));
 
-   if (temp & BMCR_RESET) {
+   if (unlikely(temp & PMT_CTRL_PHY_RST_)) {
SMSC_WARN(pdata, hw, "PHY reset failed to complete");
return -EIO;
}
@@ -2296,7 +2291,7 @@ static int smsc911x_init(struct net_device *dev)
}
 
/* Reset the LAN911x */
-   if (smsc911x_soft_reset(pdata))
+   if (smsc911x_phy_reset(pdata) || smsc911x_soft_reset(pdata))
return -ENODEV;
 
dev->flags |= IFF_MULTICAST;
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] net: smsc911x: Reset PHY during initialization

2015-11-01 Thread Pavel Fedin
 Hello!

> > On certain hardware after software reboot the chip may get stuck and fail
> > to reinitialize during reset. This can be fixed by ensuring that PHY is
> > reset too.
> >
> > Old PHY resetting method required operational MDIO interface, therefore
> > the chip should have been already set up. In order to be able to function
> > during probe, it is changed to use PMT_CTRL register.
> >
> > The problem could be observed on SMDK5410 board.
> >
> > Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
> 
> I'm pretty sure this is going to break the PHY loopback test.

 It's not (at least in normal situation), because first we do the test, and 
only then, if it fails, we reset the PHY. So, during
normal operation of the driver, when loopback test succeeds at first attempt, 
we never attempt to reset the PHY. And, by the way, at
least on my board this PHY reset did nothing useful, because after it loopback 
test still failed, all 100 times.
 And, we don't use PHY reset anywhere outside of loopback test.

> This is such a tricky and fragile area to get right, therefore I
> want you to specifically guard any change in how PHY reset is
> done with tests against the specific chips that have the problem.

 Well, i could do one (or some combination) of the following, if you really 
want to:
 a) Leave PHY reset inside loopback test as it is, but add a second routine and 
call it only before smsc911x_soft_reset().
 b) Reset PHY only conditionally, using the following algorithm:
if smsc911x_soft_reset() {
/* NIC reset failed, kick the PHY and retry */
smsc911x_phy_reset()
if (smsc_911x_soft_reset())
return -ENODEV;
}
 c) Do extra PHY reset only if some hint in device tree is specified (or the 
machine is known to have the problem)

 But, i dislike approach (a) for code duplication, because datasheet says that 
both reset methods are equivalent; i dislike approach
(b) for actually being a hack; and i dislike (c) for adding extra stuff which 
seems to be not necessary. After all, what is wrong
with just extra PHY reset before attempting to program anything? By the way, i 
test my patch with both software reboot and hardware
reboot, and even powercycle. Everything is quite stable.
 Well, it's up to you to decide. But i'd like to get the fix upstreamed somehow 
because from time to time we use these boards for
tests, and it's quite annoiying to be unable to reboot them properly.

> Furthermore, I want you to test whether this has any negative
> effects on the PHY loopback test.

 I did. I never disabled loopback test, so i actually discovered a problem 
(even two) with it. First, the problem was chip reset
timeout. By increasing it to 300ms this problem seemed to be fixed, but 
loopback test started to fail. This was how i found and
fixed crash with missing phy_disconnect(). I examined the code and discovered 
that upon loopback test failure we reset the PHY and
retry. But in my case this PHY reset never did the right thing, and all 
loopback attempts (10x10 IIRC) were failing.
 Some comments in the code gave me a clue that the main NIC can misbehave on 
reset if e.g. PHY is in powerdown state. I printed
value of its control register and discovered that it was not the case. But then 
i discovered that we actually never try to reset the
PHY before doing anything. Also, while studying the datasheed i discovered that 
there is a shorthand for resetting PHY without MDIO
bus set up, but our driver doesn't use it, preferring MDIO method instead, 
which already requires operational NIC.
 So, i suggested that PHY is just not being reset when board is rebooted by 
software. I wrote the patch and it worked. I verified it
by reverting my previous NIC reset timeout increase, and it continued to work. 
After this loopback test on my board passes at first
attempt.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v2] net: thunderx: Correctly distinguish between VF and LMAC count

2015-12-07 Thread Pavel Fedin
Commit bc69fdfc6c13
("net: thunderx: Enable BGX LMAC's RX/TX only after VF is up")
introduces lmac_cnt member and starts verifying VF number against it.
This is plain wrong, and works only because currently we have hardcoded
1:1 mapping between VFs and LMACs, and in this case num_vf_en and
lmac_cnt are always equal. However in future this may change, and the
code will badly misbehave. The worst consequence of this is failure to
deliver link status messages, causing VFs to go defunct because since
commit 0b72a9a1060e ("net: thunderx: Switchon carrier only upon
interface link up") VF will not fully bring itself up without it.

This patch fixes the potential problem by doing VF number checks against
the num_vf_en. Since lmac_cnt is not used anywhere else, it is removed.

Additionally some duplicated code is factored out into nic_enable_vf()

Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
---
v1 => v2:
- Updated subject and commit message for better understanding
---
 drivers/net/ethernet/cavium/thunder/nic_main.c | 39 --
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c 
b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 4b7fd63..5f24d11 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -37,7 +37,6 @@ struct nicpf {
 #defineNIC_GET_BGX_FROM_VF_LMAC_MAP(map)   ((map >> 4) & 0xF)
 #defineNIC_GET_LMAC_FROM_VF_LMAC_MAP(map)  (map & 0xF)
u8  vf_lmac_map[MAX_LMAC];
-   u8  lmac_cnt;
struct delayed_work dwork;
struct workqueue_struct *check_link;
u8  link[MAX_LMAC];
@@ -280,7 +279,6 @@ static void nic_set_lmac_vf_mapping(struct nicpf *nic)
u64 lmac_credit;
 
nic->num_vf_en = 0;
-   nic->lmac_cnt = 0;
 
for (bgx = 0; bgx < NIC_MAX_BGX; bgx++) {
if (!(bgx_map & (1 << bgx)))
@@ -290,7 +288,6 @@ static void nic_set_lmac_vf_mapping(struct nicpf *nic)
nic->vf_lmac_map[next_bgx_lmac++] =
NIC_SET_VF_LMAC_MAP(bgx, lmac);
nic->num_vf_en += lmac_cnt;
-   nic->lmac_cnt += lmac_cnt;
 
/* Program LMAC credits */
lmac_credit = (1ull << 1); /* channel credit enable */
@@ -618,6 +615,21 @@ static int nic_config_loopback(struct nicpf *nic, struct 
set_loopback *lbk)
return 0;
 }
 
+static void nic_enable_vf(struct nicpf *nic, int vf, bool enable)
+{
+   int bgx, lmac;
+
+   nic->vf_enabled[vf] = enable;
+
+   if (vf >= nic->num_vf_en)
+   return;
+
+   bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+   lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+
+   bgx_lmac_rx_tx_enable(nic->node, bgx, lmac, enable);
+}
+
 /* Interrupt handler to handle mailbox messages from VFs */
 static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
 {
@@ -717,29 +729,14 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
case NIC_MBOX_MSG_CFG_DONE:
/* Last message of VF config msg sequence */
-   nic->vf_enabled[vf] = true;
-   if (vf >= nic->lmac_cnt)
-   goto unlock;
-
-   bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
-   lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
-
-   bgx_lmac_rx_tx_enable(nic->node, bgx, lmac, true);
+   nic_enable_vf(nic, vf, true);
goto unlock;
case NIC_MBOX_MSG_SHUTDOWN:
/* First msg in VF teardown sequence */
-   nic->vf_enabled[vf] = false;
if (vf >= nic->num_vf_en)
nic->sqs_used[vf - nic->num_vf_en] = false;
nic->pqs_vf[vf] = 0;
-
-   if (vf >= nic->lmac_cnt)
-   break;
-
-   bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
-   lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
-
-   bgx_lmac_rx_tx_enable(nic->node, bgx, lmac, false);
+   nic_enable_vf(nic, vf, false);
break;
case NIC_MBOX_MSG_ALLOC_SQS:
nic_alloc_sqs(nic, _alloc);
@@ -958,7 +955,7 @@ static void nic_poll_for_link(struct work_struct *work)
 
mbx.link_status.msg = NIC_MBOX_MSG_BGX_LINK_CHANGE;
 
-   for (vf = 0; vf < nic->lmac_cnt; vf++) {
+   for (vf = 0; vf < nic->num_vf_en; vf++) {
/* Poll only if VF is UP */
if (!nic->vf_enabled[vf])
continue;
-- 
2.4.4

--
To unsubscribe from this 

RE: [PATCH 3/6] net: thunderx: Increase transmit queue length

2015-12-02 Thread Pavel Fedin
 Hello!

> Probably you might have to set "coherent_pool" size in bootargs to a
> higher value.
> Can you please check.

 I have tried to do this. I was able to enlarge the pool up to 4MB, and still 
got allocation failures. At 8MB pool preallocation stops working:
--- cut ---
Call trace:
[] __alloc_pages_nodemask+0x4f4/0x7d4
[] atomic_pool_init+0x60/0x1a4
[] arm64_dma_init+0x20/0x28
[] do_one_initcall+0x8c/0x1a4
[] kernel_init_freeable+0x154/0x1f4
[] kernel_init+0x10/0xd8
DMA: failed to allocate 8192 KiB pool for atomic coherent allocation
--- cut ---
 and i get even worse faults in the driver.

 I know that it is possible to allocate larger pools by setting 
CONFIG_FORCE_MAX_ZONEORDER, but:
a) This is done on per-platform basis. For ThunderX we used to have a patch 
(http://www.spinics.net/lists/arm-kernel/msg415457.html), which never made it 
upstream, because vGIC fixes stopped requiring it at some point. And also we 
may want to use the nicvf driver not only on actual hardware, but also inside 
virtual machine in KVM. So do we need to set CONFIG_FORCE_MAX_ZONEORDER for 
virt too? And what if at some point qemu emulates not only "virt", but some 
other machine (let's say AMD X-Gene), and we run it on ThunderX and want to use 
nicvf with this model?
b) IMHO it's not good to have a driver which simply does not work without some 
obscure option in boot arguments.

 So, i see several possible ways to solve this:

1. Introduce some mechanism which would allow the driver to tell the kernel 
that it needs coherent pool of large size. Can be problematic because the 
driver can be a module, and pool allocation happens early.
2. Can we use some other method for allocating queues, which would not require 
such a huge coherent pool?
3. The driver could check value of atomic_pool_size and adjust own memory 
requirements accordingly. This indeed looks like a quick hack, but would at 
least make things running quickly.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] net: thunderx: Remove unnecessary struct nicpf::lmac_cnt

2015-12-04 Thread Pavel Fedin
Commit bc69fdfc6c13
("net: thunderx: Enable BGX LMAC's RX/TX only after VF is up")
introduces lmac_cnt member and starts verifying VF number against it.
This is plain wrong, and works only because currently we have hardcoded
1:1 mapping between VFs and LMACs, and num_vf_en and lmac_cnt are always
equal. However in future this may change, and the code will badly
misbehave. The worst consequence of this is failure to deliver link status
messages, causing VFs to go defunct because since commit 0b72a9a1060e
("net: thunderx: Switchon carrier only upon interface link up") VF will
not fully bring itself up without it.

This patch fixes the potential problem by removing unnecessary structure
member and doing VF number checks against correct one.

Additionally some duplicated code is factored out into nic_enable_vf()

Signed-off-by: Pavel Fedin <p.fe...@samsung.com>
---
 drivers/net/ethernet/cavium/thunder/nic_main.c | 39 --
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/cavium/thunder/nic_main.c 
b/drivers/net/ethernet/cavium/thunder/nic_main.c
index 4b7fd63..5f24d11 100644
--- a/drivers/net/ethernet/cavium/thunder/nic_main.c
+++ b/drivers/net/ethernet/cavium/thunder/nic_main.c
@@ -37,7 +37,6 @@ struct nicpf {
 #defineNIC_GET_BGX_FROM_VF_LMAC_MAP(map)   ((map >> 4) & 0xF)
 #defineNIC_GET_LMAC_FROM_VF_LMAC_MAP(map)  (map & 0xF)
u8  vf_lmac_map[MAX_LMAC];
-   u8  lmac_cnt;
struct delayed_work dwork;
struct workqueue_struct *check_link;
u8  link[MAX_LMAC];
@@ -280,7 +279,6 @@ static void nic_set_lmac_vf_mapping(struct nicpf *nic)
u64 lmac_credit;
 
nic->num_vf_en = 0;
-   nic->lmac_cnt = 0;
 
for (bgx = 0; bgx < NIC_MAX_BGX; bgx++) {
if (!(bgx_map & (1 << bgx)))
@@ -290,7 +288,6 @@ static void nic_set_lmac_vf_mapping(struct nicpf *nic)
nic->vf_lmac_map[next_bgx_lmac++] =
NIC_SET_VF_LMAC_MAP(bgx, lmac);
nic->num_vf_en += lmac_cnt;
-   nic->lmac_cnt += lmac_cnt;
 
/* Program LMAC credits */
lmac_credit = (1ull << 1); /* channel credit enable */
@@ -618,6 +615,21 @@ static int nic_config_loopback(struct nicpf *nic, struct 
set_loopback *lbk)
return 0;
 }
 
+static void nic_enable_vf(struct nicpf *nic, int vf, bool enable)
+{
+   int bgx, lmac;
+
+   nic->vf_enabled[vf] = enable;
+
+   if (vf >= nic->num_vf_en)
+   return;
+
+   bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+   lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
+
+   bgx_lmac_rx_tx_enable(nic->node, bgx, lmac, enable);
+}
+
 /* Interrupt handler to handle mailbox messages from VFs */
 static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
 {
@@ -717,29 +729,14 @@ static void nic_handle_mbx_intr(struct nicpf *nic, int vf)
break;
case NIC_MBOX_MSG_CFG_DONE:
/* Last message of VF config msg sequence */
-   nic->vf_enabled[vf] = true;
-   if (vf >= nic->lmac_cnt)
-   goto unlock;
-
-   bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
-   lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
-
-   bgx_lmac_rx_tx_enable(nic->node, bgx, lmac, true);
+   nic_enable_vf(nic, vf, true);
goto unlock;
case NIC_MBOX_MSG_SHUTDOWN:
/* First msg in VF teardown sequence */
-   nic->vf_enabled[vf] = false;
if (vf >= nic->num_vf_en)
nic->sqs_used[vf - nic->num_vf_en] = false;
nic->pqs_vf[vf] = 0;
-
-   if (vf >= nic->lmac_cnt)
-   break;
-
-   bgx = NIC_GET_BGX_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
-   lmac = NIC_GET_LMAC_FROM_VF_LMAC_MAP(nic->vf_lmac_map[vf]);
-
-   bgx_lmac_rx_tx_enable(nic->node, bgx, lmac, false);
+   nic_enable_vf(nic, vf, false);
break;
case NIC_MBOX_MSG_ALLOC_SQS:
nic_alloc_sqs(nic, _alloc);
@@ -958,7 +955,7 @@ static void nic_poll_for_link(struct work_struct *work)
 
mbx.link_status.msg = NIC_MBOX_MSG_BGX_LINK_CHANGE;
 
-   for (vf = 0; vf < nic->lmac_cnt; vf++) {
+   for (vf = 0; vf < nic->num_vf_en; vf++) {
/* Poll only if VF is UP */
if (!nic->vf_enabled[vf])
continue;
-- 
2.4.4

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 0/2] net: thunderx: Miscellaneous cleanups

2015-12-07 Thread Pavel Fedin
 Tested-by: Pavel Fedin <p.fe...@samsung.com>

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
> Behalf Of Sunil
> Goutham
> Sent: Monday, December 07, 2015 8:01 AM
> To: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
> p.fe...@samsung.com;
> sunil.gout...@caviumnetworks.com; Sunil Goutham
> Subject: [PATCH 0/2] net: thunderx: Miscellaneous cleanups
> 
> From: Sunil Goutham <sgout...@cavium.com>
> 
> This patch series contains contains couple of cleanup patches.
> 
> Sunil Goutham (1):
>   net, thunderx: Remove unnecessary rcv buffer start address management
> 
> Yury Norov (1):
>   net: thunderx: nicvf_queues: nivc_*_intr: remove duplication
> 
>  drivers/net/ethernet/cavium/thunder/nicvf_queues.c |  189 
> +---
>  drivers/net/ethernet/cavium/thunder/nicvf_queues.h |   10 +-
>  2 files changed, 51 insertions(+), 148 deletions(-)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 2/2] net: thunderx: Enable CQE count threshold interrupt

2015-12-09 Thread Pavel Fedin
 Hello!

> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
> Behalf Of Sunil
> Goutham
> Sent: Wednesday, December 09, 2015 2:38 PM
> To: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
> p.fe...@samsung.com;
> sunil.gout...@caviumnetworks.com; Sunil Goutham
> Subject: [PATCH 2/2] net: thunderx: Enable CQE count threshold interrupt
> 
> From: Sunil Goutham <sgout...@cavium.com>
> 
> This feature is introduced in pass-2 chip and with this CQ interrupt
> coalescing will work based on both timer and count.
> 
> Signed-off-by: Sunil Goutham <sgout...@cavium.com>
> ---
>  drivers/net/ethernet/cavium/thunder/nic.h  |2 ++
>  drivers/net/ethernet/cavium/thunder/nicvf_main.c   |2 +-
>  drivers/net/ethernet/cavium/thunder/nicvf_queues.c |5 -
>  drivers/net/ethernet/cavium/thunder/nicvf_queues.h |3 ++-
>  4 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/nic.h
> b/drivers/net/ethernet/cavium/thunder/nic.h
> index 02571f4..e782856 100644
> --- a/drivers/net/ethernet/cavium/thunder/nic.h
> +++ b/drivers/net/ethernet/cavium/thunder/nic.h
> @@ -34,6 +34,8 @@
>  /* NIC priv flags */
>  #define  NIC_SRIOV_ENABLED   BIT(0)
> 
> +#define  VNIC_NAPI_WEIGHTNAPI_POLL_WEIGHT
> +
>  /* Min/Max packet size */
>  #define  NIC_HW_MIN_FRS  64
>  #define  NIC_HW_MAX_FRS  9200 /* 9216 max packet 
> including FCS */
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> index c24cb2a..e06a7f8 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> @@ -1155,7 +1155,7 @@ int nicvf_open(struct net_device *netdev)
>   cq_poll->cq_idx = qidx;
>   cq_poll->nicvf = nic;
>   netif_napi_add(netdev, _poll->napi, nicvf_poll,
> -NAPI_POLL_WEIGHT);
> +VNIC_NAPI_WEIGHT);

 What's the sense in introducing another constant which is aliased to the 
previous one? Making LOC bigger?

>   napi_enable(_poll->napi);
>   nic->napi[qidx] = cq_poll;
>   }
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> index b11fc09..4e9709e 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.c
> @@ -299,7 +299,10 @@ static int nicvf_init_cmp_queue(struct nicvf *nic,
>   return err;
> 
>   cq->desc = cq->dmem.base;
> - cq->thresh = CMP_QUEUE_CQE_THRESH;
> + if (!pass1_silicon(nic->pdev))
> + cq->thresh = CMP_QUEUE_CQE_THRESH;
> + else
> + cq->thresh = 0;

 IMHO "cq->thresh = pass1_silicon(nic->pdev) ? CMP_QUEUE_CQE_THRESH : 0" looks 
less bulky.

>   nic->cq_coalesce_usecs = (CMP_QUEUE_TIMER_THRESH * 0.05) - 1;
> 
>   return 0;
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
> b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
> index a4f6667..0fae6ad 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
> @@ -10,6 +10,7 @@
>  #define NICVF_QUEUES_H
> 
>  #include 
> +#include "nic.h"
>  #include "q_struct.h"
> 
>  #define MAX_QUEUE_SET128
> @@ -75,7 +76,7 @@
>   */
>  #define CMP_QSIZECMP_QUEUE_SIZE2
>  #define CMP_QUEUE_LEN(1ULL << (CMP_QSIZE + 10))
> -#define CMP_QUEUE_CQE_THRESH 0
> +#define CMP_QUEUE_CQE_THRESH (VNIC_NAPI_WEIGHT / 2)
>  #define CMP_QUEUE_TIMER_THRESH   80 /* ~2usec */
> 
>  #define RBDR_SIZERBDR_SIZE0
> --
> 1.7.1

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 1/2] net: thunderx: HW TSO support for pass-2 hardware

2015-12-09 Thread Pavel Fedin
dd_hdr_subdesc(nic, sq, qentry, subdesc_cnt - 1,
> +  skb, skb->len);
> 
>   /* Add SQ gather subdescs */
>   qentry = nicvf_get_nxt_sqentry(sq, qentry);
> diff --git a/drivers/net/ethernet/cavium/thunder/q_struct.h
> b/drivers/net/ethernet/cavium/thunder/q_struct.h
> index 3c1de97..9e6d987 100644
> --- a/drivers/net/ethernet/cavium/thunder/q_struct.h
> +++ b/drivers/net/ethernet/cavium/thunder/q_struct.h
> @@ -545,25 +545,28 @@ struct sq_hdr_subdesc {
>   u64subdesc_cnt:8;
>   u64csum_l4:2;
>   u64csum_l3:1;
> - u64rsvd0:5;
> + u64csum_inner_l4:2;
> + u64csum_inner_l3:1;
> + u64rsvd0:2;
>   u64l4_offset:8;
>   u64l3_offset:8;
>   u64rsvd1:4;
>   u64tot_len:20; /* W0 */
> 
> - u64tso_sdc_cont:8;
> - u64tso_sdc_first:8;
> - u64tso_l4_offset:8;
> - u64tso_flags_last:12;
> - u64tso_flags_first:12;
> - u64rsvd2:2;
> + u64rsvd2:24;
> + u64inner_l4_offset:8;
> + u64inner_l3_offset:8;
> + u64tso_start:8;
> + u64    rsvd3:2;
>   u64tso_max_paysize:14; /* W1 */
>  #elif defined(__LITTLE_ENDIAN_BITFIELD)
>   u64tot_len:20;
>   u64rsvd1:4;
>   u64l3_offset:8;
>   u64l4_offset:8;
> - u64rsvd0:5;
> + u64rsvd0:2;
> + u64csum_inner_l3:1;
> + u64csum_inner_l4:2;
>   u64csum_l3:1;
>   u64csum_l4:2;
>   u64subdesc_cnt:8;
> @@ -574,12 +577,11 @@ struct sq_hdr_subdesc {
>   u64subdesc_type:4; /* W0 */
> 
>   u64tso_max_paysize:14;
> - u64rsvd2:2;
> - u64tso_flags_first:12;
> - u64tso_flags_last:12;
> - u64tso_l4_offset:8;
> - u64tso_sdc_first:8;
> - u64tso_sdc_cont:8; /* W1 */
> + u64rsvd3:2;
> + u64tso_start:8;
> + u64inner_l3_offset:8;
> + u64inner_l4_offset:8;
> + u64rsvd2:24; /* W1 */
>  #endif
>  };
> 
> --
> 1.7.1
> 

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/6] net: thunderx: Increase transmit queue length

2015-12-02 Thread Pavel Fedin
 Hello!

> > Probably you might have to set "coherent_pool" size in bootargs to a
> > higher value.
> > Can you please check.
> 
>  I have tried to do this. I was able to enlarge the pool up to 4MB, and still 
> got allocation
> failures. At 8MB pool preallocation stops working:
> --- cut ---
> Call trace:
> [] __alloc_pages_nodemask+0x4f4/0x7d4
> [] atomic_pool_init+0x60/0x1a4
> [] arm64_dma_init+0x20/0x28
> [] do_one_initcall+0x8c/0x1a4
> [] kernel_init_freeable+0x154/0x1f4
> [] kernel_init+0x10/0xd8
> DMA: failed to allocate 8192 KiB pool for atomic coherent allocation
> --- cut ---
>  and i get even worse faults in the driver.
> 
>  I know that it is possible to allocate larger pools by setting 
> CONFIG_FORCE_MAX_ZONEORDER,
> but:
> a) This is done on per-platform basis. For ThunderX we used to have a patch
> (http://www.spinics.net/lists/arm-kernel/msg415457.html), which never made it 
> upstream,
> because vGIC fixes stopped requiring it at some point. And also we may want 
> to use the nicvf
> driver not only on actual hardware, but also inside virtual machine in KVM. 
> So do we need to
> set CONFIG_FORCE_MAX_ZONEORDER for virt too? And what if at some point qemu 
> emulates not only
> "virt", but some other machine (let's say AMD X-Gene), and we run it on 
> ThunderX and want to
> use nicvf with this model?
> b) IMHO it's not good to have a driver which simply does not work without 
> some obscure option
> in boot arguments.
> 
>  So, i see several possible ways to solve this:
> 
> 1. Introduce some mechanism which would allow the driver to tell the kernel 
> that it needs
> coherent pool of large size. Can be problematic because the driver can be a 
> module, and pool
> allocation happens early.
> 2. Can we use some other method for allocating queues, which would not 
> require such a huge
> coherent pool?
> 3. The driver could check value of atomic_pool_size and adjust own memory 
> requirements
> accordingly. This indeed looks like a quick hack, but would at least make 
> things running
> quickly.

 I have also noticed that CONFIG_DMA_CMA is turned off in my kernel. I guess it 
was a leftover from old defconfig, because i carry over my .config from version 
to version. I enabled it and rebuilt the kernel, but in order to get the driver 
working with this patch i had to also add cma=32M option to kernel arguments. 
With default of 16M the allocation still fails.
 Should we add Kconfig dependencies?

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 4/5] net: thunderx: Switchon carrier only upon interface link up

2015-12-02 Thread Pavel Fedin
 Just a reminder, we have issue with this one too, which is not addressed yet.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
> Behalf Of Sunil
> Goutham
> Sent: Wednesday, December 02, 2015 1:06 PM
> To: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> sunil.gout...@caviumnetworks.com; Sunil Goutham
> Subject: [PATCH v2 4/5] net: thunderx: Switchon carrier only upon interface 
> link up
> 
> From: Sunil Goutham <sgout...@cavium.com>
> 
> Call netif_carrier_on() only if interface's link is up. Switching this on
> upon IFF_UP by default, is causing issues with ethernet channel bonding
> in LACP mode. Initial NETDEV_CHANGE notification was being skipped.
> 
> Also fixed some issues with link/speed/duplex reporting via ethtool.
> 
> Signed-off-by: Sunil Goutham <sgout...@cavium.com>
> ---
>  .../net/ethernet/cavium/thunder/nicvf_ethtool.c|   16 +++-
>  drivers/net/ethernet/cavium/thunder/nicvf_main.c   |4 +---
>  drivers/net/ethernet/cavium/thunder/thunder_bgx.c  |2 ++
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
> b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
> index af54c10..a12b2e3 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
> @@ -112,6 +112,13 @@ static int nicvf_get_settings(struct net_device *netdev,
> 
>   cmd->supported = 0;
>   cmd->transceiver = XCVR_EXTERNAL;
> +
> + if (!nic->link_up) {
> + cmd->duplex = DUPLEX_UNKNOWN;
> + ethtool_cmd_speed_set(cmd, SPEED_UNKNOWN);
> + return 0;
> + }
> +
>   if (nic->speed <= 1000) {
>   cmd->port = PORT_MII;
>   cmd->autoneg = AUTONEG_ENABLE;
> @@ -125,6 +132,13 @@ static int nicvf_get_settings(struct net_device *netdev,
>   return 0;
>  }
> 
> +static u32 nicvf_get_link(struct net_device *netdev)
> +{
> + struct nicvf *nic = netdev_priv(netdev);
> +
> + return nic->link_up;
> +}
> +
>  static void nicvf_get_drvinfo(struct net_device *netdev,
> struct ethtool_drvinfo *info)
>  {
> @@ -660,7 +674,7 @@ static int nicvf_set_channels(struct net_device *dev,
> 
>  static const struct ethtool_ops nicvf_ethtool_ops = {
>   .get_settings   = nicvf_get_settings,
> - .get_link   = ethtool_op_get_link,
> + .get_link   = nicvf_get_link,
>   .get_drvinfo= nicvf_get_drvinfo,
>   .get_msglevel   = nicvf_get_msglevel,
>   .set_msglevel   = nicvf_set_msglevel,
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> index 7f709cb..dde8dc7 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> @@ -1057,6 +1057,7 @@ int nicvf_stop(struct net_device *netdev)
> 
>   netif_carrier_off(netdev);
>   netif_tx_stop_all_queues(nic->netdev);
> + nic->link_up = false;
> 
>   /* Teardown secondary qsets first */
>   if (!nic->sqs_mode) {
> @@ -1211,9 +1212,6 @@ int nicvf_open(struct net_device *netdev)
>   nic->drv_stats.txq_stop = 0;
>   nic->drv_stats.txq_wake = 0;
> 
> - netif_carrier_on(netdev);
> - netif_tx_start_all_queues(netdev);
> -
>   return 0;
>  cleanup:
>   nicvf_disable_intr(nic, NICVF_INTR_MBOX, 0);
> diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> index 6534b73..d77e41a 100644
> --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> @@ -612,6 +612,8 @@ static void bgx_poll_for_link(struct work_struct *work)
>   lmac->last_duplex = 1;
>   } else {
>   lmac->link_up = 0;
> + lmac->last_speed = SPEED_UNKNOWN;
> + lmac->last_duplex = DUPLEX_UNKNOWN;
>   }
> 
>   if (lmac->last_link != lmac->link_up) {
> --
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/6] net: thunderx: Increase transmit queue length

2015-12-02 Thread Pavel Fedin
 Hello!

> > swiotlb: coherent allocation failed for device 0002:01:08.4 size=4198400
> > CPU: 2 PID: 3655 Comm: NetworkManager Tainted: GW  O4.2.6+ #201
> > Hardware name: Cavium ThunderX CN88XX
> 
> Are you sure 4.2.6 kernel is suitable for backporting this patch aimed
> for linux-4.5 ?

 Why not? It's just a networking driver. And, i also have the same problem on 
4.3 running as KVM guest with VFIO.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/6] net: thunderx: Increase transmit queue length

2015-12-02 Thread Pavel Fedin
 Hello!

> >  So, i see several possible ways to solve this:
> >
> > 1. Introduce some mechanism which would allow the driver to tell the kernel 
> > that it needs
> > coherent pool of large size. Can be problematic because the driver can be a 
> > module, and pool
> > allocation happens early.
> > 2. Can we use some other method for allocating queues, which would not 
> > require such a huge
> > coherent pool?
> > 3. The driver could check value of atomic_pool_size and adjust own memory 
> > requirements
> > accordingly. This indeed looks like a quick hack, but would at least make 
> > things running
> > quickly.
> 
>  I have also noticed that CONFIG_DMA_CMA is turned off in my kernel. I guess 
> it was a leftover
> from old defconfig, because i carry over my .config from version to version. 
> I enabled it and
> rebuilt the kernel, but in order to get the driver working with this patch i 
> had to also add
> cma=32M option to kernel arguments. With default of 16M the allocation still 
> fails.
>  Should we add Kconfig dependencies?

 After getting it working in guest i tried to apply it to host. With total of 
128 virtual functions (= 128 interfaces) it does not work at all. Even after 
bumping cma region size to insane value of 2GB more than half of interfaces 
still failed to allocate queues. And after setting cma=3G i could not mount my 
rootfs.
 So, absolute NAK, unfortunately.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 4/5] net: thunderx: Switchon carrier only upon interface link up

2015-12-02 Thread Pavel Fedin
 Hello again!

> Subject: RE: [PATCH v2 4/5] net: thunderx: Switchon carrier only upon 
> interface link up
> 
>  Just a reminder, we have issue with this one too, which is not addressed yet.

 I have examined the problem thoroughly and discovered that it is a problem 
with experimental BGX driver patches on my 4.2 host
kernel build. It fails to send NIC_MBOX_MSG_BGX_LINK_CHANGE, so nothing works.
 I have retested this series on top of 4.3 running on ThunderX. It has good BGX 
driver and everything works fine. Therefore, the
whole series can be safely applied and...

 Tested-by: Pavel Fedin <p.fe...@samsung.com>

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/6] net: thunderx: Increase transmit queue length

2015-12-02 Thread Pavel Fedin
 Hello!

> >After getting it working in guest i tried to apply it to host. With total of 
> >128 virtual
> functions (= 128 interfaces) it does not work at all.
> > Even after bumping cma region size to insane value of 2GB more than half of 
> > interfaces still
> failed to allocate queues.
> > And after setting cma=3G i could not mount my rootfs.
> 
> Here what you are saying is half of the interfaces were initialized
> succesfully and rest didn't.

 After setting cma=2G. With default setting of 16M none of them initialized.

> So this issue is not something which is introduced by this patch.

 Before this patch all my interfaces were working.
 I would say the problem with your patch is that it introduces memory 
requirements which cannot be satisfied by the platform. It's combination of 
several factors which stops the thing from working, not a single factor. Using 
dma_alloc_coherent() is not all wrong by itself, of course.
 Perhaps you did some tricks with your configuration, which make it working. 
Then, i guess, you should have at least described them in commit message of 
your patch. Or describe all dependencies in KConfig of your driver, which is 
better. Or, if the platform needs some very special defconfig, add it to 
arch/arm64/configs (however, i guess, the goal of ARM64 Linux is to run on all 
possible hardware, so this would not be good from maintainers' POV).

 Sorry, but this is all i can say. In previous messages i have already 
suggested several ways to solve the problem (too lazy to quite here, 4 IIRC), 
or you can suggest your own one and let us test it, or you can even stick to 
"It works for me, i am the only right guy in the world, and i don't care if it 
doesn't work for you" position and let David decide who of us is right (and he 
already did that once).
 Basically, here is what i did: i took kernel 4.2, added ThunderX PCI drivers 
to it (they were posted but NAKed those days back, there's some lazy progress 
on them currently), added necessary errata patches (also posted on lists, all 
merged into 4.4), took defconfig, adjusted it according to my needs, and this 
is what i'm running on my board and this is what i'm using for development. If 
you point me at what i'm doing wrong way, i'll be glad to accept this.
 I'm over.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v2 0/2] net: thunderx: Support for pass-2 hw features

2015-12-10 Thread Pavel Fedin
 All series:

 Reviewed-by: Pavel Fedin <p.fe...@samsung.com>

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia

-Original Message-
From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
Behalf Of Sunil Goutham
Sent: Thursday, December 10, 2015 10:55 AM
To: netdev@vger.kernel.org
Cc: linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org; 
p.fe...@samsung.com; sunil.gout...@caviumnetworks.com; Sunil
Goutham
Subject: [PATCH v2 0/2] net: thunderx: Support for pass-2 hw features

From: Sunil Goutham <sgout...@cavium.com>

This patch set adds support for new features added in pass-2 revision of 
hardware like TSO and count based interrupt coalescing.

Changes from v1:
- Addressed comments received regarding boolean bit field changes
  by excluding them from this patch. Will submit a seperate
  patch along with cleanup of unsed field.
- Got rid of new macro 'VNIC_NAPI_WEIGHT' introduced in
  count threshold interrupt patch.

Sunil Goutham (2):
  net: thunderx: HW TSO support for pass-2 hardware
  net: thunderx: Enable CQE count threshold interrupt

 drivers/net/ethernet/cavium/thunder/nic.h  |6 
 drivers/net/ethernet/cavium/thunder/nic_main.c |   11 ++-
 drivers/net/ethernet/cavium/thunder/nicvf_main.c   |   15 -
 drivers/net/ethernet/cavium/thunder/nicvf_queues.c |   22 ++
 drivers/net/ethernet/cavium/thunder/nicvf_queues.h |2 +-
 drivers/net/ethernet/cavium/thunder/q_struct.h |   30 ++-
 6 files changed, 55 insertions(+), 31 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body 
of a message to majord...@vger.kernel.org More
majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 5/6] net: thunderx: Switchon carrier only upon interface link up

2015-12-01 Thread Pavel Fedin
 Hello!

 This one causes the network to stop working on Fedora 21. Probably has to do 
with NetworkManager, which sees something unexpected.
IP address is never set up and connection is never activated, despite it has UP 
flag.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
> Behalf Of Sunil
> Goutham
> Sent: Tuesday, December 01, 2015 12:14 PM
> To: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> sunil.gout...@caviumnetworks.com; Sunil Goutham
> Subject: [PATCH 5/6] net: thunderx: Switchon carrier only upon interface link 
> up
> 
> From: Sunil Goutham <sgout...@cavium.com>
> 
> Call netif_carrier_on() only if interface's link is up. Switching this on
> upon IFF_UP by default, is causing issues with ethernet channel bonding
> in LACP mode. Initial NETDEV_CHANGE notification was being skipped.
> 
> Also fixed some issues with link/speed/duplex reporting via ethtool.
> 
> Signed-off-by: Sunil Goutham <sgout...@cavium.com>
> ---
>  .../net/ethernet/cavium/thunder/nicvf_ethtool.c|   16 +++-
>  drivers/net/ethernet/cavium/thunder/nicvf_main.c   |4 +---
>  drivers/net/ethernet/cavium/thunder/thunder_bgx.c  |2 ++
>  3 files changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
> b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
> index af54c10..a12b2e3 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_ethtool.c
> @@ -112,6 +112,13 @@ static int nicvf_get_settings(struct net_device *netdev,
> 
>   cmd->supported = 0;
>   cmd->transceiver = XCVR_EXTERNAL;
> +
> + if (!nic->link_up) {
> + cmd->duplex = DUPLEX_UNKNOWN;
> + ethtool_cmd_speed_set(cmd, SPEED_UNKNOWN);
> + return 0;
> + }
> +
>   if (nic->speed <= 1000) {
>   cmd->port = PORT_MII;
>   cmd->autoneg = AUTONEG_ENABLE;
> @@ -125,6 +132,13 @@ static int nicvf_get_settings(struct net_device *netdev,
>   return 0;
>  }
> 
> +static u32 nicvf_get_link(struct net_device *netdev)
> +{
> + struct nicvf *nic = netdev_priv(netdev);
> +
> + return nic->link_up;
> +}
> +
>  static void nicvf_get_drvinfo(struct net_device *netdev,
> struct ethtool_drvinfo *info)
>  {
> @@ -660,7 +674,7 @@ static int nicvf_set_channels(struct net_device *dev,
> 
>  static const struct ethtool_ops nicvf_ethtool_ops = {
>   .get_settings   = nicvf_get_settings,
> - .get_link   = ethtool_op_get_link,
> + .get_link   = nicvf_get_link,
>   .get_drvinfo= nicvf_get_drvinfo,
>   .get_msglevel   = nicvf_get_msglevel,
>   .set_msglevel   = nicvf_set_msglevel,
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> index 7f709cb..dde8dc7 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_main.c
> @@ -1057,6 +1057,7 @@ int nicvf_stop(struct net_device *netdev)
> 
>   netif_carrier_off(netdev);
>   netif_tx_stop_all_queues(nic->netdev);
> + nic->link_up = false;
> 
>   /* Teardown secondary qsets first */
>   if (!nic->sqs_mode) {
> @@ -1211,9 +1212,6 @@ int nicvf_open(struct net_device *netdev)
>   nic->drv_stats.txq_stop = 0;
>   nic->drv_stats.txq_wake = 0;
> 
> - netif_carrier_on(netdev);
> - netif_tx_start_all_queues(netdev);
> -
>   return 0;
>  cleanup:
>   nicvf_disable_intr(nic, NICVF_INTR_MBOX, 0);
> diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> index 2076ac3..d9f27ad 100644
> --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> @@ -612,6 +612,8 @@ static void bgx_poll_for_link(struct work_struct *work)
>   lmac->last_duplex = 1;
>   } else {
>   lmac->link_up = 0;
> + lmac->last_speed = SPEED_UNKNOWN;
> + lmac->last_duplex = DUPLEX_UNKNOWN;
>   }
> 
>   if (lmac->last_link != lmac->link_up) {
> --
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH 3/6] net: thunderx: Increase transmit queue length

2015-12-01 Thread Pavel Fedin
 Hello!

 This one breaks networking on my machine:
--- cut ---
swiotlb: coherent allocation failed for device 0002:01:08.4 size=4198400
CPU: 2 PID: 3655 Comm: NetworkManager Tainted: GW  O4.2.6+ #201
Hardware name: Cavium ThunderX CN88XX board (DT)
Call trace:
[] dump_backtrace+0x0/0x124
[] show_stack+0x14/0x1c
[] dump_stack+0x88/0xc8
[] swiotlb_alloc_coherent+0x150/0x168
[] __dma_alloc+0x58/0x1e8
[] nicvf_alloc_q_desc_mem.isra.14+0xc0/0xdc
[] nicvf_config_data_transfer+0x448/0x728
[] nicvf_open+0x184/0x994
[] __dev_open+0xb4/0x120
[] __dev_change_flags+0x8c/0x150
[] dev_change_flags+0x20/0x5c
[] do_setlink+0x27c/0x8e0
[] rtnl_newlink+0x4b4/0x6c4
[] rtnetlink_rcv_msg+0x90/0x22c
[] netlink_rcv_skb+0xd8/0x100
[] rtnetlink_rcv+0x30/0x40
[] netlink_unicast+0x158/0x1f8
[] netlink_sendmsg+0x324/0x380
[] sock_sendmsg+0x18/0x58
[] ___sys_sendmsg+0x254/0x264
[] __sys_sendmsg+0x40/0x84
[] SyS_sendmsg+0x10/0x20
thunder-nicvf 0002:01:08.4 enP2p1s8f4: Failed to alloc/config VF's QSet 
resources
--- cut ---

 And none of interfaces work after this. Reverting this patch helps.

Kind regards,
Pavel Fedin
Expert Engineer
Samsung Electronics Research center Russia


> -Original Message-
> From: netdev-ow...@vger.kernel.org [mailto:netdev-ow...@vger.kernel.org] On 
> Behalf Of Sunil
> Goutham
> Sent: Tuesday, December 01, 2015 12:14 PM
> To: netdev@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> sunil.gout...@caviumnetworks.com; Sunil Goutham
> Subject: [PATCH 3/6] net: thunderx: Increase transmit queue length
> 
> From: Sunil Goutham <sgout...@cavium.com>
> 
> Under high transmit rates and with TSO enabled observing fluctuations
> in TX performance. Seen especially with iperf3 application.
> 
> Since TSO is taken care at driver level, with 64KB of TSO packets
> and when window size is also high the rate at which CPU fills in
> transmit descriptors is much higher than what HW is able to process.
> Each 64KB TSO packet occupies gets segmented to ~43 1500 byte MTU packets
> and occupies ~130 descriptors. Hence increasing transmit queue size.
> 
> Signed-off-by: Sunil Goutham <sgout...@cavium.com>
> ---
>  drivers/net/ethernet/cavium/thunder/nicvf_queues.h |4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
> b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
> index fb4957d..b1e93a9 100644
> --- a/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
> +++ b/drivers/net/ethernet/cavium/thunder/nicvf_queues.h
> @@ -62,7 +62,7 @@
>  #define SND_QUEUE_CNT8
>  #define CMP_QUEUE_CNT8 /* Max of RCV and SND qcount */
> 
> -#define SND_QSIZESND_QUEUE_SIZE2
> +#define SND_QSIZESND_QUEUE_SIZE3
>  #define SND_QUEUE_LEN(1ULL << (SND_QSIZE + 10))
>  #define MAX_SND_QUEUE_LEN(1ULL << (SND_QUEUE_SIZE6 + 10))
>  #define SND_QUEUE_THRESH 2ULL
> @@ -73,7 +73,7 @@
>  /* Keep CQ and SQ sizes same, if timestamping
>   * is enabled this equation will change.
>   */
> -#define CMP_QSIZECMP_QUEUE_SIZE2
> +#define CMP_QSIZECMP_QUEUE_SIZE3
>  #define CMP_QUEUE_LEN(1ULL << (CMP_QSIZE + 10))
>  #define CMP_QUEUE_CQE_THRESH 0
>  #define CMP_QUEUE_TIMER_THRESH   220 /* 10usec */
> --
> 1.7.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html