Netdev List
 help / color / mirror / Atom feed
* Re: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
From: Florian Fainelli @ 2017-07-27 17:57 UTC (permalink / raw)
  To: Andrew Lunn, Brown, Aaron F
  Cc: John W. Linville, netdev@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org
In-Reply-To: <20170727153713.GG18666@lunn.ch>

On 07/27/2017 08:37 AM, Andrew Lunn wrote:
> On Thu, Jul 27, 2017 at 12:40:01AM +0000, Brown, Aaron F wrote:
>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>>> Of John W. Linville
>>> Sent: Friday, July 21, 2017 11:12 AM
>>> To: netdev@vger.kernel.org
>>> Cc: intel-wired-lan@lists.osuosl.org; John W. Linville
>>> <linville@tuxdriver.com>
>>> Subject: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
>>>
>>> The management port on an Edgecore AS7712-32 switch uses an igb MAC,
>>> but
>>> it uses a BCM54616 PHY. Without a patch like this, loading the igb
>>> module produces dmesg output like this:
>>>
>>> [    3.439125] igb: Copyright (c) 2007-2014 Intel Corporation.
>>> [    3.439866] igb: probe of 0000:00:14.0 failed with error -2
>>>
>>> Signed-off-by: John W. Linville <linville@tuxdriver.com>
>>> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> ---
>>>  drivers/net/ethernet/intel/igb/e1000_82575.c   | 6 ++++++
>>>  drivers/net/ethernet/intel/igb/e1000_defines.h | 1 +
>>>  drivers/net/ethernet/intel/igb/e1000_hw.h      | 1 +
>>>  3 files changed, 8 insertions(+)
>>
>> I do not have the specific hardware (Edgecore switch) but as far as regression tests go this works fine.
>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> 
> Sorry, missed the initial post, so replying to a reply.
> 
> Linux has supported the BCM54616 PHY since April 2015. If the Intel
> drivers used the Linux PHY drivers, you would not of had this problem.
> 
> It would be good if somebody spent the time to migrate these MAC
> drivers to use the common Linux PHY infrastructure.

I suspect there is a design pattern within the Intel drivers to share as
much low-level code as possible between OSes and only have some
Linux-ism where necessary (e.g: net_device, ethtool etc.).

PHY code is a pain in general, especially if you are serious about
testing interoperability (which is where you can spend tons of $$$ with
little reward but just say: yes it works), so it may make sense to share
it across different OSes.

I too, wish there was more sharing, but considering that this works for
the Intel driver, there is little incentive in doing this I suppose...
-- 
Florian

^ permalink raw reply

* RE: [PATCH V4 net-next 6/8] net: hns3: Add MDIO support to HNS3 Ethernet driver for hip08 SoC
From: Salil Mehta @ 2017-07-27 17:56 UTC (permalink / raw)
  To: Florian Fainelli, davem@davemloft.net
  Cc: Zhuangyuzeng (Yisen), huangdaode, lipeng (Y),
	mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
	Linuxarm
In-Reply-To: <24043b65-27d3-13b9-1a25-8692ea9467af@gmail.com>

Hi Florian,

> -----Original Message-----
> From: Florian Fainelli [mailto:f.fainelli@gmail.com]
> Sent: Sunday, July 23, 2017 5:54 PM
> To: Salil Mehta; davem@davemloft.net
> Cc: Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V4 net-next 6/8] net: hns3: Add MDIO support to
> HNS3 Ethernet driver for hip08 SoC
> 
> 
> 
> On 07/22/2017 03:09 PM, Salil Mehta wrote:
> > This patch adds the support of MDIO bus interface for HNS3 driver.
> > Code provides various interfaces to start and stop the PHY layer
> > and to read and write the MDIO bus or PHY.
> >
> > Signed-off-by: Daode Huang <huangdaode@hisilicon.com>
> > Signed-off-by: lipeng <lipeng321@huawei.com>
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > Signed-off-by: Yisen Zhuang <yisen.zhuang@huawei.com>
> > ---
> > Patch V4: Addressed following comments:
> >  1. Andrew Lunn:
> >     https://lkml.org/lkml/2017/6/17/208
> > Patch V3: Addressed Below comments:
> >  1. Florian Fainelli:
> >     https://lkml.org/lkml/2017/6/13/963
> >  2. Andrew Lunn:
> >     https://lkml.org/lkml/2017/6/13/1039
> > Patch V2: Addressed below comments:
> >  1. Florian Fainelli:
> >     https://lkml.org/lkml/2017/6/10/130
> >  2. Andrew Lunn:
> >     https://lkml.org/lkml/2017/6/10/168
> > Patch V1: Initial Submit
> > ---
> >  .../ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c    | 230
> +++++++++++++++++++++
> >  1 file changed, 230 insertions(+)
> >  create mode 100644
> drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> >
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> > new file mode 100644
> > index 000000000000..6036a97f7de5
> > --- /dev/null
> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_mdio.c
> > @@ -0,0 +1,230 @@
> > +/*
> > + * Copyright (c) 2016~2017 Hisilicon Limited.
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify
> > + * it under the terms of the GNU General Public License as published
> by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +
> > +#include <linux/etherdevice.h>
> > +#include <linux/kernel.h>
> > +
> > +#include "hclge_cmd.h"
> > +#include "hclge_main.h"
> > +
> > +enum hclge_mdio_c22_op_seq {
> > +	HCLGE_MDIO_C22_WRITE = 1,
> > +	HCLGE_MDIO_C22_READ = 2
> > +};
> > +
> > +#define HCLGE_MDIO_CTRL_START_B		0
> > +#define HCLGE_MDIO_CTRL_ST_S		1
> > +#define HCLGE_MDIO_CTRL_ST_M		(0x3 << HCLGE_MDIO_CTRL_ST_S)
> > +#define HCLGE_MDIO_CTRL_OP_S		3
> > +#define HCLGE_MDIO_CTRL_OP_M		(0x3 << HCLGE_MDIO_CTRL_OP_S)
> > +
> > +#define HCLGE_MDIO_PHYID_S		0
> > +#define HCLGE_MDIO_PHYID_M		(0x1f << HCLGE_MDIO_PHYID_S)
> > +
> > +#define HCLGE_MDIO_PHYREG_S		0
> > +#define HCLGE_MDIO_PHYREG_M		(0x1f << HCLGE_MDIO_PHYREG_S)
> > +
> > +#define HCLGE_MDIO_STA_B		0
> > +
> > +struct hclge_mdio_cfg_cmd {
> > +	u8 ctrl_bit;
> > +	u8 phyid;
> > +	u8 phyad;
> > +	u8 rsvd;
> > +	__le16 reserve;
> > +	__le16 data_wr;
> > +	__le16 data_rd;
> > +	__le16 sta;
> > +};
> > +
> > +static int hclge_mdio_write(struct mii_bus *bus, int phyid, int
> regnum,
> > +			    u16 data)
> > +{
> > +	struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
> 
> Cast is not needed here since bus->priv is already a void *.
> 
> > +	struct hclge_mdio_cfg_cmd *mdio_cmd;
> > +	enum hclge_cmd_status status;
> > +	struct hclge_desc desc;
> > +
> > +	if (!bus)
> > +		return -EINVAL;
> 
> How can this be possible?
Agreed, will remove.

> 
> > +
> > +	hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_MDIO_CONFIG, false);
> > +
> > +	mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
> 
> Same here, can we not cast this into a struct hclge_mdio_cfg_cmd?
You mean should we assign directly without casting?

> 
> > +
> > +	hnae_set_field(mdio_cmd->phyid, HCLGE_MDIO_PHYID_M,
> > +		       HCLGE_MDIO_PHYID_S, phyid);
> > +	hnae_set_field(mdio_cmd->phyad, HCLGE_MDIO_PHYREG_M,
> > +		       HCLGE_MDIO_PHYREG_S, regnum);
> > +
> > +	hnae_set_bit(mdio_cmd->ctrl_bit, HCLGE_MDIO_CTRL_START_B, 1);
> > +	hnae_set_field(mdio_cmd->ctrl_bit, HCLGE_MDIO_CTRL_ST_M,
> > +		       HCLGE_MDIO_CTRL_ST_S, 1);
> > +	hnae_set_field(mdio_cmd->ctrl_bit, HCLGE_MDIO_CTRL_OP_M,
> > +		       HCLGE_MDIO_CTRL_OP_S, HCLGE_MDIO_C22_WRITE);
> > +
> > +	mdio_cmd->data_wr = cpu_to_le16(data);
> > +
> > +	status = hclge_cmd_send(&hdev->hw, &desc, 1);
> > +	if (status) {
> > +		dev_err(&hdev->pdev->dev,
> > +			"mdio write fail when sending cmd, status is %d.\n",
> > +			status);
> > +		return -EIO;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int hclge_mdio_read(struct mii_bus *bus, int phyid, int
> regnum)
> > +{
> > +	struct hclge_dev *hdev = (struct hclge_dev *)bus->priv;
> 
> Same here.
No cast required you mean?

> 
> > +	struct hclge_mdio_cfg_cmd *mdio_cmd;
> > +	enum hclge_cmd_status status;
> > +	struct hclge_desc desc;
> > +
> > +	if (!bus)
> > +		return -EINVAL;
> 
> And here.
Ok.

> 
> > +
> > +	hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_MDIO_CONFIG, true);
> > +
> > +	mdio_cmd = (struct hclge_mdio_cfg_cmd *)desc.data;
> 
> And here.
Ok.

> 
> > +
> > +	hnae_set_field(mdio_cmd->phyid, HCLGE_MDIO_PHYID_M,
> > +		       HCLGE_MDIO_PHYID_S, phyid);
> > +	hnae_set_field(mdio_cmd->phyad, HCLGE_MDIO_PHYREG_M,
> > +		       HCLGE_MDIO_PHYREG_S, regnum);
> > +
> > +	hnae_set_bit(mdio_cmd->ctrl_bit, HCLGE_MDIO_CTRL_START_B, 1);
> > +	hnae_set_field(mdio_cmd->ctrl_bit, HCLGE_MDIO_CTRL_ST_M,
> > +		       HCLGE_MDIO_CTRL_ST_S, 1);
> > +	hnae_set_field(mdio_cmd->ctrl_bit, HCLGE_MDIO_CTRL_OP_M,
> > +		       HCLGE_MDIO_CTRL_OP_S, HCLGE_MDIO_C22_READ);
> > +
> > +	/* Read out phy data */
> > +	status = hclge_cmd_send(&hdev->hw, &desc, 1);
> > +	if (status) {
> > +		dev_err(&hdev->pdev->dev,
> > +			"mdio read fail when get data, status is %d.\n",
> > +			status);
> > +		return status;
> > +	}
> > +
> > +	if (hnae_get_bit(le16_to_cpu(mdio_cmd->sta), HCLGE_MDIO_STA_B)) {
> > +		dev_err(&hdev->pdev->dev, "mdio read data error\n");
> > +		return -EIO;
> > +	}
> > +
> > +	return le16_to_cpu(mdio_cmd->data_rd);
> > +}
> > +
> > +int hclge_mac_mdio_config(struct hclge_dev *hdev)
> > +{
> > +	struct hclge_mac *mac = &hdev->hw.mac;
> > +	struct phy_device *phydev;
> > +	struct mii_bus *mdio_bus;
> > +	int ret;
> > +
> > +	if (hdev->hw.mac.phy_addr >= PHY_MAX_ADDR)
> > +		return 0;
> 
> Should not this be treated as a hard error instead?
What if SFP is not plugged in and our hardware treats
this as 0xff a special condition? I think we are trying
to detect that condition here.

> 
> > +
> > +	mdio_bus = devm_mdiobus_alloc(&hdev->pdev->dev);
> > +	if (!mdio_bus)
> > +		return -ENOMEM;
> > +
> > +	mdio_bus->name = "hisilicon MII bus";
> > +	mdio_bus->read = hclge_mdio_read;
> > +	mdio_bus->write = hclge_mdio_write;
> > +	snprintf(mdio_bus->id, MII_BUS_ID_SIZE, "%s-%s", "mii",
> > +		 dev_name(&hdev->pdev->dev));
> > +
> > +	mdio_bus->parent = &hdev->pdev->dev;
> > +	mdio_bus->priv = hdev;
> > +	mdio_bus->phy_mask = ~(1 << mac->phy_addr);
> > +	ret = mdiobus_register(mdio_bus);
> > +	if (ret) {
> > +		dev_err(mdio_bus->parent,
> > +			"Failed to register MDIO bus ret = %#x\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	phydev = mdiobus_get_phy(mdio_bus, mac->phy_addr);
> > +	if (!phydev || IS_ERR(phydev)) {
> > +		dev_err(mdio_bus->parent, "Failed to get phy device\n");
> > +		mdiobus_unregister(mdio_bus);
> > +		return -EIO;
> > +	}
> > +
> > +	mac->phydev = phydev;
> > +	mac->mdio_bus = mdio_bus;
> > +
> > +	return 0;
> > +}
> > +
> > +static void hclge_mac_adjust_link(struct net_device *netdev)
> > +{
> > +	struct hclge_mac *hw_mac;
> > +	struct hclge_dev *hdev;
> > +	struct hclge_hw *hw;
> > +	int duplex;
> > +	int speed;
> > +
> > +	if (!netdev)
> > +		return;
> 
> This should not happen, don't let your drivers register unless the
> net_device object is fully set up.
Agreed. Will remove.

> 
> > +
> > +	hw_mac = container_of(netdev, struct hclge_mac, netdev);
> > +	hw = container_of(hw_mac, struct hclge_hw, mac);
> > +	hdev = hw->back;
> > +
> > +	speed = netdev->phydev->speed;
> > +	duplex = netdev->phydev->duplex;
> > +
> > +	/* update antoneg. */
> > +	hw_mac->autoneg = netdev->phydev->autoneg;
> > +
> > +	if ((hw_mac->speed != speed) || (hw_mac->duplex != duplex))
> > +		(void)hclge_cfg_mac_speed_dup(hdev, speed, !!duplex);
> > +}
> > +
> > +int hclge_mac_start_phy(struct hclge_dev *hdev)
> > +{
> > +	struct hclge_mac *mac = &hdev->hw.mac;
> > +	struct net_device *netdev = &mac->netdev;
> > +	struct phy_device *phydev = mac->phydev;
> > +	int ret;
> > +
> > +	if (!phydev)
> > +		return 0;
> 
> Same thing here, this should not happen.
This is again when SFP is not present phydev will be NULL.

> 
> > +
> > +	phydev->dev_flags = 0;
> 
> This is the default.
Sure, will remove

> 
> > +
> > +	ret = phy_connect_direct(netdev, phydev,
> > +				 hclge_mac_adjust_link,
> > +				 PHY_INTERFACE_MODE_SGMII);
> > +	if (ret) {
> > +		pr_info("phy_connect_direct err");
> > +		return ret;
> > +	}
> > +
> > +	phy_start(phydev);
> > +
> > +	return 0;
> > +}
> > +
> > +void hclge_mac_stop_phy(struct hclge_dev *hdev)
> > +{
> > +	struct hclge_mac *mac = &hdev->hw.mac;
> > +	struct net_device *netdev = &mac->netdev;
> > +	struct phy_device *phydev = netdev->phydev;
> > +
> > +	if (!phydev)
> > +		return;
> 
> You should be symmetrical here, if you could phy_connect() +
> phy_start()
> then you should call phy_stop() + phy_disconnect().
Got it. Will fix it.

Thanks!
> 
> > +
> > +	phy_disconnect(phydev);
> > +}
> >
> 
> --
> Florian

^ permalink raw reply

* Re: [PATCH v7 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Alexander Duyck @ 2017-07-27 17:49 UTC (permalink / raw)
  To: Ding Tianhong
  Cc: Casey Leedom, Alex Williamson, Sinan Kaya, ashok.raj@intel.com,
	bhelgaas@google.com, helgaas@kernel.org, Michael Werner,
	Ganesh GR, asit.k.mallick@intel.com, patrick.j.cramer@intel.com,
	Suravee.Suthikulpanit@amd.com, Bob.Shaw@amd.com,
	l.stach@pengutronix.de, amira@mellanox.com,
	gabriele.paoloni@huawei.com, David.Laight@aculab.com,
	"jeffrey.t.kirsher@int
In-Reply-To: <75213fca-4522-2297-3cb8-338e643d3552@huawei.com>

On Wed, Jul 26, 2017 at 6:08 PM, Ding Tianhong <dingtianhong@huawei.com> wrote:
>
>
> On 2017/7/27 2:26, Casey Leedom wrote:
>>   By the way Ding, two issues:
>>
>>  1. Did we ever get any acknowledgement from either Intel or AMD
>>     on this patch?  I know that we can't ensure that, but it sure would
>>     be nice since the PCI Quirks that we're putting in affect their
>>     products.
>>
>
> Still no Intel and AMD guys has ack this, this is what I am worried about, should I
> ping some man again ?
>
> Thanks
> Ding


I probably wouldn't worry about it too much. If anything all this
patch is doing is disabling relaxed ordering on the platforms we know
have issues based on what Casey originally had. If nothing else we can
follow up once the patches are in the kernel and if somebody has an
issue then.

You can include my acked-by, but it is mostly related to how this
interacts with NICs, and not so much about the PCI chipsets
themselves.

Acked-by: Alexander Duyck <alexander.h.duyck@intel.com>

^ permalink raw reply

* Re: [PATCH v7 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Casey Leedom @ 2017-07-27 17:44 UTC (permalink / raw)
  To: Ding Tianhong, Alexander Duyck
  Cc: Netdev, Bjorn Helgaas, linux-arm-kernel@lists.infradead.org,
	David.Laight@aculab.com, ashok.raj@intel.com, Alex Williamson,
	l.stach@pengutronix.de, Suravee.Suthikulpanit@amd.com,
	catalin.marinas@arm.com, linux-pci@vger.kernel.org,
	will.deacon@arm.com, Sinan Kaya, robin.murphy@arm.com,
	linux-kernel@vger.kernel.org, davem@davemloft.net,
	Ganesh GR <g
In-Reply-To: <2feff1a5-25b7-eb6c-7628-7c2257c56e95@huawei.com>

| From: Ding Tianhong <dingtianhong@huawei.com>
| Sent: Wednesday, July 26, 2017 6:01 PM
|
| On 2017/7/27 3:05, Casey Leedom wrote:
| >
| > Ding, send me a note if you'd like me to work that [cxgb4vf patch] up
| > for you.
|
| Ok, you could send the change log and I could put it in the v8 version
| together, will you base on the patch 3/3 or build a independence patch?

Which ever you'd prefer.  It would basically mirror the same exact code that
you've got for cxgb4.  I.e. testing the setting of the VF's PCIe Capability
Device Control[Relaxed Ordering Enable], setting a new flag in
adpater->flags, testing that flag in cxgb4vf/sge.c:t4vf_sge_alloc_rxq().
But since the VF's PF will already have disabled the PF's Relaxed Ordering
Enable, the VF will also have it's Relaxed Ordering Enable disabled and any
effort by the internal chip to send TLPs with the Relaxed Ordering Attribute
will be gated by the PCIe logic.  So it's not critical that this be in the
first patch.  Your call.  Let me know if you'd like me to send that to you.


| From: Ding Tianhong <dingtianhong@huawei.com>
| Sent: Wednesday, July 26, 2017 6:08 PM
|
| On 2017/7/27 2:26, Casey Leedom wrote:
| >
| >  1. Did we ever get any acknowledgement from either Intel or AMD
| >     on this patch?  I know that we can't ensure that, but it sure would
| >     be nice since the PCI Quirks that we're putting in affect their
| >     products.
|
| Still no Intel and AMD guys has ack this, this is what I am worried about,
| should I ping some man again ?

By amusing coincidence, Patrik Cramer (now Cc'ed) from Intel sent me a note
yesterday with a link to the official Intel performance tuning documentation
which covers this issue:

https://software.intel.com/sites/default/files/managed/9e/bc/64-ia-32-architectures-optimization-manual.pdf

In section 3.9.1 we have:

    3.9.1 Optimizing PCIe Performance for Accesses Toward Coherent Memory
          and Toward MMIO Regions (P2P)

    In order to maximize performance for PCIe devices in the processors
    listed in Table 3-6 below, the soft- ware should determine whether the
    accesses are toward coherent memory (system memory) or toward MMIO
    regions (P2P access to other devices). If the access is toward MMIO
    region, then software can command HW to set the RO bit in the TLP
    header, as this would allow hardware to achieve maximum throughput for
    these types of accesses. For accesses toward coherent memory, software
    can command HW to clear the RO bit in the TLP header (no RO), as this
    would allow hardware to achieve maximum throughput for these types of
    accesses.

    Table 3-6. Intel Processor CPU RP Device IDs for Processors Optimizing
               PCIe Performance

    Processor                            CPU RP Device IDs

    Intel Xeon processors based on       6F01H-6F0EH
    Broadwell microarchitecture

    Intel Xeon processors based on       2F01H-2F0EH
    Haswell microarchitecture

Unfortunately that's a pretty thin section.  But it does expand the set of
Intel Root Complexes for which our Linux PCI Quirk will need to cover.  So
you should add those to the next (and hopefully final) spin of your patch.
And, it also verifies the need to handle the use of Relaxed Ordering more
subtlely than simply turning it off since the NVMe peer-to-peer example I
keep bringing up would fall into the "need to use Relaxed Ordering" case ...

It would have been nice to know why this is happening and if any future
processor would fix this.  After all, Relaxed Ordering, is just supposed to
be a hint.  At worst, a receiving device could just ignore the attribute
entirely.  Obviously someone made an effort to implement it but ... it
didn't go the way they wanted.

And, it also would have been nice to know if there was any hidden register
in these Intel Root Complexes which can completely turn off the effort to
pay attention to the Relaxed Ordering Attribute.  We've spend an enormous
amount of effort on this issue here on the Linux PCI email list struggling
mightily to come up with a way to determine when it's
safe/recommended/not-recommended/unsafe to use Relaxed Ordering when
directing TLPs towards the Root Complex.  And some architectures require RO
for decent performance so we can't just "turn it off" unilatterally.

Casey

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
From: Jonathan Toppins @ 2017-07-27 17:41 UTC (permalink / raw)
  To: Andrew Lunn, Brown, Aaron F
  Cc: John W. Linville, netdev@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org
In-Reply-To: <20170727153713.GG18666@lunn.ch>

On 07/27/2017 11:37 AM, Andrew Lunn wrote:
> On Thu, Jul 27, 2017 at 12:40:01AM +0000, Brown, Aaron F wrote:
>>> From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
>>> Of John W. Linville
>>> Sent: Friday, July 21, 2017 11:12 AM
>>> To: netdev@vger.kernel.org
>>> Cc: intel-wired-lan@lists.osuosl.org; John W. Linville
>>> <linville@tuxdriver.com>
>>> Subject: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
>>>
>>> The management port on an Edgecore AS7712-32 switch uses an igb MAC,
>>> but
>>> it uses a BCM54616 PHY. Without a patch like this, loading the igb
>>> module produces dmesg output like this:
>>>
>>> [    3.439125] igb: Copyright (c) 2007-2014 Intel Corporation.
>>> [    3.439866] igb: probe of 0000:00:14.0 failed with error -2
>>>
>>> Signed-off-by: John W. Linville <linville@tuxdriver.com>
>>> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>>> ---
>>>  drivers/net/ethernet/intel/igb/e1000_82575.c   | 6 ++++++
>>>  drivers/net/ethernet/intel/igb/e1000_defines.h | 1 +
>>>  drivers/net/ethernet/intel/igb/e1000_hw.h      | 1 +
>>>  3 files changed, 8 insertions(+)
>>
>> I do not have the specific hardware (Edgecore switch) but as far as regression tests go this works fine.
>> Tested-by: Aaron Brown <aaron.f.brown@intel.com>
> 
> Sorry, missed the initial post, so replying to a reply.
> 
> Linux has supported the BCM54616 PHY since April 2015. If the Intel
> drivers used the Linux PHY drivers, you would not of had this problem.
> 
> It would be good if somebody spent the time to migrate these MAC
> drivers to use the common Linux PHY infrastructure.
> 
> 	Andrew
> 

Thank you. This was a point I made in 2015 when I posted the original
patch for igb to support this phy.

http://marc.info/?l=linux-netdev&m=142870703206646&w=2
http://marc.info/?l=linux-netdev&m=142870703806647&w=2
http://marc.info/?l=linux-netdev&m=142870703806648&w=2

http://marc.info/?l=linux-netdev&m=142930225730399&w=2
http://marc.info/?l=linux-netdev&m=142930226230400&w=2

It would be good to accept this patch and then later work to port older
drivers to use phylib.

It might be worthwhile to start beating up on new driver submissions
that don't use phylib.

I recall someone mentioning that there were plans to have phylib support
[Q]SFP[+] modules as well. I am very interested in that work, if someone
has patches/plans I would like to take a look.

-Jon

^ permalink raw reply

* Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support
From: Corentin Labbe @ 2017-07-27 17:31 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Andrew Lunn, David Wu, mark.rutland-5wv7dgnIgG8,
	huangtao-TNX95d0MmH7DzftRWevZcw, hwg-TNX95d0MmH7DzftRWevZcw,
	heiko-4mtYJXux2i+zQB+pC5nmwQ, arnd-r2nGTMty4D4,
	devicetree-u79uwXL29TY76Z2rM5mHXA, catalin.marinas-5wv7dgnIgG8,
	will.deacon-5wv7dgnIgG8, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
	olof-nZhT3qVonbNeoWH0uzbU5w, peppe.cavallaro-qxv4g6HH51o,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	alexandre.torgue-qxv4g6HH51o
In-Reply-To: <65ae1747-4dff-b1d8-8aa3-684fc8c7809a-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

On Thu, Jul 27, 2017 at 09:54:01AM -0700, Florian Fainelli wrote:
> On 07/27/2017 06:48 AM, Andrew Lunn wrote:
> > On Thu, Jul 27, 2017 at 09:02:16PM +0800, David Wu wrote:
> >> To make internal phy work, need to configure the phy_clock,
> >> phy cru_reset and related registers.
> >>
> >> Signed-off-by: David Wu <david.wu-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >> ---
> >> changes in v2:
> >>  - Use the standard "phy-mode" property for internal phy. (Florian)
> > 
> > I think we need to discuss this. This PHY appears to be on an MDIO
> > bus, it uses a standard PHY driver, and it appears to be using an RMII
> > interface. So it is just an ordinary PHY.
> 
> First, the fact that the internal PHY also appears through MDIO is
> orthogonal to the fact that it is internal or external. Plenty of
> designs have internal PHYs exposed through MDIO because that is
> convenient. What matters though is how the data/clock lines are wired
> internally, which is what "phy-mode" describes.
> 
> > 
> > Internal is supposed to be something which is not ordinary, does not
> > use one of the standard phy modes, needs something special to make it
> > work.
> > 
> > Florain, it appears to be your suggestion to use internal. What do you
> > say?
> 
> phy-mode = "internal" really means that it is not a standard MII variant
> to connect the data/clock lines between the Ethernet MAC and the PHY,
> and this can happen in some designs (although quite unlikely). So from
> there we could do several things depending on the requirements:
> 
> - if you can have your Ethernet MAC driver perform the necessary
> configuration *after* you have been able to bind the PHY device with its
> PHY driver, then the PHY driver should have PHY_IS_INTERNAL in its
> flags, and you can use phy_is_internal() from PHYLIB to tell you that
> and we could imagine using: phy-mode = "rmii" because that would not too
> much of a stretch
> 
> - if you need knowledge about this PHY connection type prior to binding
> the PHY device and its driver (that is, before of_phy_connect()) we
> could add a boolean property e.g: "phy-is-internal" that allows us to
> know that, or we can have a new phy-mode value, e.g: "internal-rmii"
> which describes that, either way would probably be fine, but the former
> scales better
> 

Hello

We have the same problem on Allwinner SoCs for dwmac-sun8i, we need to set a syscon for chossing between internal/external PHY.
Having this phy-is-internal would be very helpfull. (adding internal-xmii will add too many flags in our case)

Thanks
Regards
Corentin Labbe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Performance regression with virtio_net
From: Seth Forshee @ 2017-07-27 17:09 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang; +Cc: virtualization, netdev

I'm seeing a performance regression with virtio_net that looks to have
started in 4.12-rc1. I only see it in one context though, downloading
snap packages from the Ubuntu snap store. For example:

 https://api.snapcraft.io/api/v1/snaps/download/b8X2psL1ryVrPt5WEmpYiqfr5emixTd7_1797.snap

which redirects to Internap's CDN. Normally this downloads in a few
seconds at ~10 MB/s, but with 4.12 and 4.13 it takes minutes with a rate
of ~150 KB/s. Everything else I've tried downloads as normal speeds.

I bisected this to 680557cf79f8 "virtio_net: rework mergeable buffer
handling". If I revert this on top of 4.13-rc2 (along with other changes
needed to successfully revert it) speeds return to normal.

Thanks,
Seth

^ permalink raw reply

* Re: [PATCH v2 05/11] net: stmmac: dwmac-rk: Add internal phy support
From: Florian Fainelli @ 2017-07-27 16:54 UTC (permalink / raw)
  To: Andrew Lunn, David Wu
  Cc: davem, heiko, robh+dt, mark.rutland, catalin.marinas, will.deacon,
	olof, linux, arnd, peppe.cavallaro, alexandre.torgue, huangtao,
	hwg, netdev, linux-arm-kernel, linux-rockchip, devicetree,
	linux-kernel
In-Reply-To: <20170727134834.GD18666@lunn.ch>

On 07/27/2017 06:48 AM, Andrew Lunn wrote:
> On Thu, Jul 27, 2017 at 09:02:16PM +0800, David Wu wrote:
>> To make internal phy work, need to configure the phy_clock,
>> phy cru_reset and related registers.
>>
>> Signed-off-by: David Wu <david.wu@rock-chips.com>
>> ---
>> changes in v2:
>>  - Use the standard "phy-mode" property for internal phy. (Florian)
> 
> I think we need to discuss this. This PHY appears to be on an MDIO
> bus, it uses a standard PHY driver, and it appears to be using an RMII
> interface. So it is just an ordinary PHY.

First, the fact that the internal PHY also appears through MDIO is
orthogonal to the fact that it is internal or external. Plenty of
designs have internal PHYs exposed through MDIO because that is
convenient. What matters though is how the data/clock lines are wired
internally, which is what "phy-mode" describes.

> 
> Internal is supposed to be something which is not ordinary, does not
> use one of the standard phy modes, needs something special to make it
> work.
> 
> Florain, it appears to be your suggestion to use internal. What do you
> say?

phy-mode = "internal" really means that it is not a standard MII variant
to connect the data/clock lines between the Ethernet MAC and the PHY,
and this can happen in some designs (although quite unlikely). So from
there we could do several things depending on the requirements:

- if you can have your Ethernet MAC driver perform the necessary
configuration *after* you have been able to bind the PHY device with its
PHY driver, then the PHY driver should have PHY_IS_INTERNAL in its
flags, and you can use phy_is_internal() from PHYLIB to tell you that
and we could imagine using: phy-mode = "rmii" because that would not too
much of a stretch

- if you need knowledge about this PHY connection type prior to binding
the PHY device and its driver (that is, before of_phy_connect()) we
could add a boolean property e.g: "phy-is-internal" that allows us to
know that, or we can have a new phy-mode value, e.g: "internal-rmii"
which describes that, either way would probably be fine, but the former
scales better

Then again, using phy-mode = "internal" even though this is Reduced MII
is not big of a deal IMHO as long as there is no loss of information and
that internal de-facto means internal reduced MII for instance.
-- 
Florian

^ permalink raw reply

* Re: [PATCH v2 01/11] net: phy: Add rockchip phy driver support
From: Florian Fainelli @ 2017-07-27 16:51 UTC (permalink / raw)
  To: David Wu, davem, heiko, andrew, robh+dt, mark.rutland,
	catalin.marinas, will.deacon, olof, linux, arnd
  Cc: peppe.cavallaro, alexandre.torgue, huangtao, hwg, netdev,
	linux-arm-kernel, linux-rockchip, devicetree, linux-kernel
In-Reply-To: <1501160156-30328-2-git-send-email-david.wu@rock-chips.com>

On 07/27/2017 05:55 AM, David Wu wrote:
> Support internal ephy currently.
> 
> Signed-off-by: David Wu <david.wu@rock-chips.com>
> ---
> changes in v2:
>  - Alphabetic order for Kconfig and Makefile.
>  - Add analog register init.
>  - Disable auto-mdix for workround.
>  - Rename config
> 
>  drivers/net/phy/Kconfig    |   5 ++
>  drivers/net/phy/Makefile   |   1 +
>  drivers/net/phy/rockchip.c | 128 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 134 insertions(+)
>  create mode 100644 drivers/net/phy/rockchip.c
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index 2dda720..8dc6cd7 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -334,6 +334,11 @@ config REALTEK_PHY
>  	---help---
>  	  Supports the Realtek 821x PHY.
>  
> +config ROCKCHIP_PHY
> +        tristate "Drivers for ROCKCHIP PHYs"

"Driver for Rockchip Ethernet PHYs" would seem more appropriate, this is
just one driver for now.

> +        ---help---
> +          Currently supports the internal ephy.

EPHY is usually how an Ethernet PHY is abbreviated.

> +
>  config SMSC_PHY
>  	tristate "SMSC PHYs"
>  	---help---
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index 8e9b9f3..350520e 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -66,6 +66,7 @@ obj-$(CONFIG_MICROSEMI_PHY)	+= mscc.o
>  obj-$(CONFIG_NATIONAL_PHY)	+= national.o
>  obj-$(CONFIG_QSEMI_PHY)		+= qsemi.o
>  obj-$(CONFIG_REALTEK_PHY)	+= realtek.o
> +obj-$(CONFIG_ROCKCHIP_PHY)	+= rockchip.o
>  obj-$(CONFIG_SMSC_PHY)		+= smsc.o
>  obj-$(CONFIG_STE10XP)		+= ste10Xp.o
>  obj-$(CONFIG_TERANETICS_PHY)	+= teranetics.o
> diff --git a/drivers/net/phy/rockchip.c b/drivers/net/phy/rockchip.c
> new file mode 100644
> index 0000000..3f74658
> --- /dev/null
> +++ b/drivers/net/phy/rockchip.c
> @@ -0,0 +1,128 @@
> +/**
> + * drivers/net/phy/rockchip.c
> + *
> + * Driver for ROCKCHIP PHY
> + *
> + * Copyright (c) 2017, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * David Wu<david.wu@rock-chips.com>

Missing space between your last name and your email address, there is
another typo like this in the MODULE_AUTHOR() macro.

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/mii.h>
> +#include <linux/ethtool.h>
> +#include <linux/phy.h>
> +#include <linux/netdevice.h>
> +
> +#define MII_INTERNAL_CTRL_STATUS	17
> +#define SMI_ADDR_TSTCNTL		20
> +#define SMI_ADDR_TSTREAD1		21
> +#define SMI_ADDR_TSTREAD2		22
> +#define SMI_ADDR_TSTWRITE		23
> +
> +#define AUTOMDIX_EN			BIT(7)
> +#define TSTCNTL_RD			(BIT(15) | BIT(10))
> +#define TSTCNTL_WR          		(BIT(14) | BIT(10))
> +
> +#define WR_ADDR_A7CFG			0x18
> +
> +static void rockchip_init_tstmode(struct phy_device *phydev)
> +{
> +	/* Enable access to Analog and DSP register banks */
> +	phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
> +	phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
> +	phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0400);
> +}
> +
> +static void  rockchip_close_tstmode(struct phy_device *phydev)
> +{
> +	/* Back to basic register bank */
> +	phy_write(phydev, SMI_ADDR_TSTCNTL, 0x0000);
> +}
> +
> +static void rockchip_internal_phy_analog_init(struct phy_device *phydev)
> +{
> +	rockchip_init_tstmode(phydev);

Technically MDIO writes can fail, but you are not propagating the return
value, so you could be stuck on a bad page/bank.

> +
> +	/*
> +	 * Adjust tx amplitude to make sginal better,
> +	 * the default value is 0x8.
> +	 */
> +	phy_write(phydev, SMI_ADDR_TSTWRITE, 0xB);
> +	phy_write(phydev, SMI_ADDR_TSTCNTL, TSTCNTL_WR | WR_ADDR_A7CFG);

Likewise.

> +
> +	rockchip_close_tstmode(phydev);

Same here.

> +}
> +
> +static int rockchip_internal_phy_config_init(struct phy_device *phydev)
> +{
> +	int val;
> +
> +	/*
> +	 * The auto MIDX has linked problem on some board,
> +	 * workround to disable auto MDIX.
> +	 */

If this a board-specific problem you may consider register a PHY fixup
for the affected boards, or introduce a specific property to illustrate
that MDI-X is broken.

> +	val = phy_read(phydev, MII_INTERNAL_CTRL_STATUS);
> +	val &= ~AUTOMDIX_EN;
> +	phy_write(phydev, MII_INTERNAL_CTRL_STATUS, val);

You also need to reject MDI configuration requests coming via
phy_ethtool_ksettings_set() which you should see in the
phyddrv::config_ane callback.

> +
> +	rockchip_internal_phy_analog_init(phydev);
> +
> +	return 0;
> +}
> +
> +static int rockchip_internal_phy_read_status(struct phy_device *phydev)
> +{
> +	int ret, old_speed;
> +
> +	old_speed = phydev->speed;
> +	ret = genphy_read_status(phydev);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * If mode switch happens from 10BT to 100BT, all DSP/AFE
> +	 * registers are set to default values. So any AFE/DSP
> +	 * registers have to be re-initialized in this case.
> +	 */
> +	if ((old_speed == SPEED_10) && (phydev->speed == SPEED_100))
> +		rockchip_internal_phy_analog_init(phydev);

link changes can be intercepted with a link_change_notify callback,
which would be more appropriate than doing this during read_status here.

> +
> +	return ret;
> +}
> +
> +static struct phy_driver rockchip_phy_driver[] = {
> +{
> +	.phy_id 	= 0x1234d400,
> +	.phy_id_mask	= 0xffff0000,

This mask is way too wide, the industry practice is to have the last 4
digits contain the PHY revision, so I would expect to see 0xffff_fff0
here as a mask.

> +	.name		= "rockchip internal ephy",

Rockchip internal EPHY?

> +	.features	= (PHY_BASIC_FEATURES | SUPPORTED_Pause
> +			  | SUPPORTED_Asym_Pause),
> +	.soft_reset 	= genphy_soft_reset,

Since this is a driver for an internal PHY you also need:

	.flags		= PHY_IS_INTERNAL

> +	.config_init	= rockchip_internal_phy_config_init,
> +	.config_aneg	= genphy_config_aneg,
> +	.read_status	= rockchip_internal_phy_read_status,
> +	.suspend	= genphy_suspend,
> +	.resume 	= genphy_resume,

You probably need your resume function to restore the AFE/DSP settings
that were done in rockchip_internal_phy_config_init()

> +},
> +};
> +
> +module_phy_driver(rockchip_phy_driver);
> +
> +static struct mdio_device_id __maybe_unused rockchip_phy_tbl[] = {
> +	{ 0x1234d400, 0xffff0000 },

Please also fix up the mask here.

> +	{ }
> +};
> +
> +MODULE_DEVICE_TABLE(mdio, rockchip_phy_tbl);
> +
> +MODULE_AUTHOR("David Wu<david.wu@rock-chips.com>");

Typo here (same as in the heading).

> +MODULE_DESCRIPTION("Rockchip phy driver");

"Rockchip Ethernet PHY driver" so make it clear what type of PHY it manages?

> +MODULE_LICENSE("GPL v2");
> 


-- 
Florian

^ permalink raw reply

* Re: [PATCH net] xen-netback: correctly schedule rate-limited queues
From: David Miller @ 2017-07-27 16:35 UTC (permalink / raw)
  To: jean-louis; +Cc: wei.liu2, netdev, xen-devel, Paul.Durrant
In-Reply-To: <ba4a971ce2c0891a6e911ef09b6a5d6e@dupond.be>

From: Jean-Louis Dupond <jean-louis@dupond.be>
Date: Thu, 27 Jul 2017 10:21:56 +0200

> Op 2017-06-22 17:16, schreef David Miller:
>> From: Wei Liu <wei.liu2@citrix.com>
>> Date: Wed, 21 Jun 2017 10:21:22 +0100
>> 
>>> Add a flag to indicate if a queue is rate-limited. Test the flag in
>>> NAPI poll handler and avoid rescheduling the queue if true, otherwise
>>> we risk locking up the host. The rescheduling will be done in the
>>> timer callback function.
>>> Reported-by: Jean-Louis Dupond <jean-louis@dupond.be>
>>> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
>>> Tested-by: Jean-Louis Dupond <jean-louis@dupond.be>
>> Applied.
> 
> Could this get applied to stable & LTS kernels also?
> Seems important enough in my opinion.

Sure, queued up.

^ permalink raw reply

* [PATCHv4 net] ipv6: no need to check rt->dst.error when get route info
From: Hangbin Liu @ 2017-07-27 16:25 UTC (permalink / raw)
  To: netdev; +Cc: Cong Wang, Roopa Prabhu, David Ahern, Hangbin Liu
In-Reply-To: <1500562286-14312-1-git-send-email-liuhangbin@gmail.com>

After commit 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib
result when requested"). When we get a prohibit ertry, we will return
-EACCES directly instead of dump route info.

Fix it by remove the rt->dst.error check.

Before fix:
\# ip -6 route add prohibit 2003::/64 dev eth1
\# ip -6 route get fibmatch 2003::1
RTNETLINK answers: Permission denied
\# ip -6 route add unreachable 2004::/64 dev eth1
\# ip -6 route get fibmatch 2004::1
RTNETLINK answers: No route to host

After fix:
\# ip -6 route add prohibit 2003::/64 dev eth1
\# ip -6 route get fibmatch 2003::1
prohibit 2003::/64 dev lo metric 1024 error -13 pref medium
\# ip -6 route add unreachable 2004::/64 dev eth1
\# ip -6 route get fibmatch 2004::1
unreachable 2004::/64 dev lo metric 1024 error -113 pref medium

Fixes: 18c3a61c4264 ("net: ipv6: RTM_GETROUTE: return matched fib...")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
 net/ipv6/route.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 4d30c96..8fc52de 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3637,12 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 		dst = ip6_route_lookup(net, &fl6, 0);
 
 	rt = container_of(dst, struct rt6_info, dst);
-	if (rt->dst.error) {
-		err = rt->dst.error;
-		ip6_rt_put(rt);
-		goto errout;
-	}
-
 	if (rt == net->ipv6.ip6_null_entry) {
 		err = rt->dst.error;
 		ip6_rt_put(rt);
-- 
2.5.5

^ permalink raw reply related

* Possible race in hysdn.ko
From: Anton Volkov @ 2017-07-27 16:19 UTC (permalink / raw)
  To: isdn; +Cc: netdev, linux-kernel, ldv-project, khoroshilov

Hello.

While searching for races in the Linux kernel I've come across
"drivers/isdn/hysdn/hysdn.ko" module. Here is a question that I came up
with while analysing results. Lines are given using the info from Linux
v4.12.

In hysdn_proclog.c file in put_log_buffer function a non-standard type
of synchronization is employed. It uses pd->del_lock as some kind of
semaphore (hysdn_proclog.c: lines 129 and 143). Consider the following
case:

Thread 1:                    Thread 2:
hysdn_log_write
-> hysdn_add_log
     -> put_log_buffer
          spin_lock()          hysdn_conf_open
          i = pd->del_lock++   -> hysdn_add_log
          spin_unlock()           -> put_log_buffer
          if (!i) <delete-loop>        spin_lock()
          pd->del_lock--               i = pd->del_lock++
                                       spin_unlock()
                                       if (!i) <delete-loop>
                                       pd->del_lock--

<delete-loop> - the loop that deletes unused buffer entries
(hysdn_proclog.c: lines 134-142).
pd->del_lock-- is not an atomic operation and is executed without any
locks. Thus it may interfere in the increment process of pd->del_lock in
another thread. There may be cases that lead to the inability of any
thread going through the <delete-loop>.

I see several possible solutions to this problem:
1) move the <delete-loop> under the spin_lock and delete
pd->del_lock synchronization;
2) wrap pd->del_lock-- with spin_lock protection.

What do you think should be done about it?

Thank you for your time.

^ permalink raw reply

* Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.
From: Hangbin Liu @ 2017-07-27 16:08 UTC (permalink / raw)
  To: David Ahern; +Cc: Roopa Prabhu, Cong Wang, network dev
In-Reply-To: <2d75317c-160c-01c2-5a5b-bc6111c099c6@gmail.com>

Hi David,

On Wed, Jul 26, 2017 at 01:00:26PM -0600, David Ahern wrote:
> >> I don't think so. If I add a prohibit route and use the fibmatch
> >> attribute, I want to see the route from the FIB that was matched.
> > 
> > 
> > yes, exactly. wouldn't  'rt != net->ipv6.ip6_prohibit_entry' above let
> > it fall through to the route fill code ?
> > 
> > ah...but i guess you are saying that they will have rt6_info's of
> > their own and will not match. got it. ack.
> > 
> 
> This:
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 4d30c96a819d..24de81c804c2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3637,11 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff
> *in_skb, struct nlmsghdr *nlh,
>                 dst = ip6_route_lookup(net, &fl6, 0);
> 
>         rt = container_of(dst, struct rt6_info, dst);
> -       if (rt->dst.error) {
> -               err = rt->dst.error;
> -               ip6_rt_put(rt);
> -               goto errout;
> -       }
> 
>         if (rt == net->ipv6.ip6_null_entry) {
>                 err = rt->dst.error;
> 
> Puts back the original behavior. In that case, only rt == null_entry
> drops to the error path which is correct. All other rt values will drop
> to rt6_fill_node and return rt data.

Thanks for your explains. Now I know where I made the mistake. I mis-looked
FR_ACT_UNREACHABLE to RTN_UNREACHABLE and thought we return rt =
net->ipv6.ip6_null_entry in fib6_rule_action().

With you help I know we just set rt->dst.error to -EACCES for prohibit
entry in ip6_route_info_create. So remove the rt->dst.error check is enought.

Thanks
Hangbin

^ permalink raw reply

* RE: [PATCH V4 net-next 8/8] net: hns3: Add HNS3 driver to kernel build framework & MAINTAINERS
From: Salil Mehta @ 2017-07-27 15:56 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: davem@davemloft.net, Zhuangyuzeng (Yisen), huangdaode, lipeng (Y),
	mehta.salil.lnk@gmail.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-rdma@vger.kernel.org,
	Linuxarm
In-Reply-To: <20170723131209.GI3259@mtr-leonro.local>

Hi Leon,

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Leon Romanovsky
> Sent: Sunday, July 23, 2017 2:12 PM
> To: Salil Mehta
> Cc: davem@davemloft.net; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil.lnk@gmail.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-rdma@vger.kernel.org; Linuxarm
> Subject: Re: [PATCH V4 net-next 8/8] net: hns3: Add HNS3 driver to
> kernel build framework & MAINTAINERS
> 
> On Sat, Jul 22, 2017 at 11:09:42PM +0100, Salil Mehta wrote:
> > This patch updates the MAINTAINERS file with HNS3 Ethernet driver
> > maintainers names and other details. This also introduces the new
> > Makefiles required to build the HNS3 Ethernet driver and updates
> > the existing Kconfig file in the hisilicon folder.
> >
> > Signed-off-by: Salil Mehta <salil.mehta@huawei.com>
> > ---
> > Patch V3: Addressed below errors:
> >  1. Intel kbuild: https://lkml.org/lkml/2017/6/14/313
> >  2. Intel Kbuild: https://lkml.org/lkml/2017/6/14/636
> > Patch V2: No change
> > Patch V1: Initial Submit
> > ---
> >  MAINTAINERS                                        |  8 +++++++
> >  drivers/net/ethernet/hisilicon/Kconfig             | 27
> ++++++++++++++++++++++
> >  drivers/net/ethernet/hisilicon/Makefile            |  1 +
> >  drivers/net/ethernet/hisilicon/hns3/Makefile       |  7 ++++++
> >  .../net/ethernet/hisilicon/hns3/hns3pf/Makefile    | 11 +++++++++
> >  5 files changed, 54 insertions(+)
> >  create mode 100644 drivers/net/ethernet/hisilicon/hns3/Makefile
> >  create mode 100644
> drivers/net/ethernet/hisilicon/hns3/hns3pf/Makefile
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 297e610c9163..a22d5b86c2b7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6197,6 +6197,14 @@ S:	Maintained
> >  F:	drivers/net/ethernet/hisilicon/
> >  F:	Documentation/devicetree/bindings/net/hisilicon*.txt
> >
> > +HISILICON NETWORK SUBSYSTEM 3 DRIVER (HNS3)
> > +M:	Yisen Zhuang <yisen.zhuang@huawei.com>
> > +M:	Salil Mehta <salil.mehta@huawei.com>
> > +L:	netdev@vger.kernel.org
> > +W:	http://www.hisilicon.com
> > +S:	Maintained
> > +F:	drivers/net/ethernet/hisilicon/hns3/
> > +
> >  HISILICON ROCE DRIVER
> >  M:	Lijun Ou <oulijun@huawei.com>
> >  M:	Wei Hu(Xavier) <xavier.huwei@huawei.com>
> > diff --git a/drivers/net/ethernet/hisilicon/Kconfig
> b/drivers/net/ethernet/hisilicon/Kconfig
> > index d11287e11371..9f8ea283c531 100644
> > --- a/drivers/net/ethernet/hisilicon/Kconfig
> > +++ b/drivers/net/ethernet/hisilicon/Kconfig
> > @@ -76,4 +76,31 @@ config HNS_ENET
> >  	  This selects the general ethernet driver for HNS.  This module
> make
> >  	  use of any HNS AE driver, such as HNS_DSAF
> >
> > +config HNS3
> > +	tristate "Hisilicon Network Subsystem Support HNS3 (Framework)"
> > +    depends on PCI
> > +	---help---
> > +	  This selects the framework support for Hisilicon Network
> Subsystem 3.
> > +	  This layer facilitates clients like ENET, RoCE and user-space
> ethernet
> > +	  drivers(like ODP)to register with HNAE devices and their
> associated
> > +	  operations.
> > +
> > +config HNS3_HCLGE
> > +	tristate "Hisilicon HNS3 HCLGE Acceleration Engine &
> Compatibility Layer Support"
> > +    depends on PCI_MSI
> > +	select HNS3
> 
> IMHO it should be "depends" and not "select".
Agreed, will fix in next patch.

Thanks
> 
> > +	---help---
> > +	  This selects the HNS3_HCLGE network acceleration engine & its
> hardware
> > +	  compatibility layer. The engine would be used in Hisilicon
> hip08 family of
> > +	  SoCs and further upcoming SoCs.
> > +
> > +config HNS3_ENET
> > +	tristate "Hisilicon HNS3 Ethernet Device Support"
> > +    depends on 64BIT && PCI
> > +	select HNS3
> 
> Ditto
Agreed, will fix in next patch.

Thanks 
> > +	---help---
> > +	  This selects the Ethernet Driver for Hisilicon Network
> Subsystem 3 for hip08
> > +	  family of SoCs. This module depends upon HNAE3 driver to access
> the HNAE3
> > +	  devices and their associated operations.
> > +
> >  endif # NET_VENDOR_HISILICON
> > diff --git a/drivers/net/ethernet/hisilicon/Makefile
> b/drivers/net/ethernet/hisilicon/Makefile
> > index 8661695024dc..3828c435c18f 100644
> > --- a/drivers/net/ethernet/hisilicon/Makefile
> > +++ b/drivers/net/ethernet/hisilicon/Makefile
> > @@ -6,4 +6,5 @@ obj-$(CONFIG_HIX5HD2_GMAC) += hix5hd2_gmac.o
> >  obj-$(CONFIG_HIP04_ETH) += hip04_eth.o
> >  obj-$(CONFIG_HNS_MDIO) += hns_mdio.o
> >  obj-$(CONFIG_HNS) += hns/
> > +obj-$(CONFIG_HNS3) += hns3/
> >  obj-$(CONFIG_HISI_FEMAC) += hisi_femac.o
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/Makefile
> b/drivers/net/ethernet/hisilicon/hns3/Makefile
> > new file mode 100644
> > index 000000000000..5e53735b2d4e
> > --- /dev/null
> > +++ b/drivers/net/ethernet/hisilicon/hns3/Makefile
> > @@ -0,0 +1,7 @@
> > +#
> > +# Makefile for the HISILICON network device drivers.
> > +#
> > +
> > +obj-$(CONFIG_HNS3) += hns3pf/
> > +
> > +obj-$(CONFIG_HNS3) +=hnae3.o
> 
> There is a missing space after "+="
Will fix. thanks.

> 
> > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/Makefile
> b/drivers/net/ethernet/hisilicon/hns3/hns3pf/Makefile
> > new file mode 100644
> > index 000000000000..c0a92b5690a9
> > --- /dev/null
> > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/Makefile
> > @@ -0,0 +1,11 @@
> > +#
> > +# Makefile for the HISILICON network device drivers.
> > +#
> > +
> > +ccflags-y := -Idrivers/net/ethernet/hisilicon/hns3
> > +
> > +obj-$(CONFIG_HNS3_HCLGE) += hclge.o
> > +hclge-objs =hclge_main.o hclge_cmd.o hclge_mdio.o hclge_tm.o
> 
> Missing space.
Okay. Will fix.

Thanks
> 
> > +
> > +obj-$(CONFIG_HNS3_ENET) += hns3.o
> > +hns3-objs = hns3_enet.o hns3_ethtool.o
> > --
> > 2.11.0
> >
> >

^ permalink raw reply

* RE: [PATCH V4 net-next 7/8] net: hns3: Add Ethtool support to HNS3 driver
From: Salil Mehta @ 2017-07-27 15:53 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
	Zhuangyuzeng (Yisen), huangdaode, lipeng (Y),
	mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
	netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linuxarm
In-Reply-To: <20170723102601.18ac81a1@xeon-e3>

Hi Stephen,

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen-OTpzqLSitTUnbdJkjeBofR2eb7JE58TQ@public.gmane.org]
> Sent: Sunday, July 23, 2017 6:26 PM
> To: Salil Mehta
> Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Zhuangyuzeng (Yisen); huangdaode; lipeng (Y);
> mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm
> Subject: Re: [PATCH V4 net-next 7/8] net: hns3: Add Ethtool support to
> HNS3 driver
> 
> On Sat, 22 Jul 2017 23:09:41 +0100
> Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> wrote:
> 
> > +	HNS3_NETDEV_STAT("rx_packets", rx_packets),
> > +	HNS3_NETDEV_STAT("tx_packets", tx_packets),
> > +	HNS3_NETDEV_STAT("rx_bytes", rx_bytes),
> > +	HNS3_NETDEV_STAT("tx_bytes", tx_bytes),
> > +	HNS3_NETDEV_STAT("rx_errors", rx_errors),
> > +	HNS3_NETDEV_STAT("tx_errors", tx_errors),
> > +	HNS3_NETDEV_STAT("rx_dropped", rx_dropped),
> > +	HNS3_NETDEV_STAT("tx_dropped", tx_dropped),
> > +	HNS3_NETDEV_STAT("multicast", multicast),
> > +	HNS3_NETDEV_STAT("collisions", collisions),
> > +
> > +	/* detailed Rx errors */
> 
> Do not put network statistics in ethtool statistics.
> This is redundant and unnecessary. Yes some other drivers may do it
> but it is not best practice.

Ok sure, will remove netdev stats from the ethtool.

Thanks
Salil
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: qmi_wwan: Null pointer dereference when removing driver
From: Nathaniel Roach @ 2017-07-27 15:50 UTC (permalink / raw)
  To: Dan Williams, netdev
In-Reply-To: <1501169946.7169.1.camel@redhat.com>

At some point in the suspend procedure the error occurs, so the first 
suspend works but subsequent ones fail with something like "timeout 
waiting for processes to suspend". I just assumed it happened before the 
suspend happens but was too late to be a hindrance.

Presumably the driver dies during the re-probe stage you mentioned, but 
a rmmod was how I found the issue (I was trying to pull out the driver 
to see if it was causing the suspend issues).


On 27/07/17 23:39, Dan Williams wrote:
> On Thu, 2017-07-27 at 13:31 +0800, Nathaniel Roach wrote:
>> Unsure at which point was added, but issue not present in stock
>> debian 4.11 kernel.
>>
>> Running on a Thinkpad X220 with coreboot.
>>
>> I'm building from upstream. When I attempt to remove the qmi_wwan
>> module (which also happens pre-suspend) the rmmod process gets
>> killed, and the following shows in dmesg:
> Unrelated to the crash (which should be fixed), why do you need to
> remove the module pre-suspend?  Typically on a laptop the device will
> either have all power cut to it over suspend and thus it'll get
> reprobed on resume, or else suspend gets handled OK by the driver.  I'm
> curious what the problem was that required an rmmod over suspend.
>
> Dan
>
>> [   59.979791] usb 2-1.4: USB disconnect, device number 4
>> [   59.980102] qmi_wwan 2-1.4:1.6 wwp0s29u1u4i6: unregister
>> 'qmi_wwan' usb-0000:00:1d.0-1.4, WWAN/QMI device
>> [   60.006821] BUG: unable to handle kernel NULL pointer dereference
>> at 00000000000000e0
>> [   60.006879] IP: qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan]
>> [   60.006911] PGD 0
>> [   60.006911] P4D 0
>> [   60.006957] Oops: 0000 [#1] SMP
>> [   60.006978] Modules linked in: fuse(E) ccm(E) rfcomm(E) cmac(E)
>> bnep(E) qmi_wwan(E) cdc_wdm(E) cdc_ether(E) usbnet(E) mii(E) btusb(E)
>> btrtl(E) btbcm(E) btintel(E) bluetooth(E) joydev(E) xpad(E)
>> ecdh_generic(E) ff_memless(E) binfmt_misc(E) snd_hda_codec_hdmi(E)
>> snd_hda_codec_conexant(E) snd_hda_codec_generic(E) arc4(E)
>> iTCO_wdt(E) iTCO_vendor_support(E) intel_rapl(E)
>> x86_pkg_temp_thermal(E) kvm_intel(E) kvm(E) irqbypass(E)
>> crct10dif_pclmul(E) crc32_pclmul(E) crc32c_intel(E)
>> ghash_clmulni_intel(E) aesni_intel(E) iwlmvm(E) aes_x86_64(E)
>> crypto_simd(E) mac80211(E) cryptd(E) glue_helper(E) snd_hda_intel(E)
>> snd_hda_codec(E) iwlwifi(E) snd_hwdep(E) psmouse(E) snd_hda_core(E)
>> snd_pcm(E) serio_raw(E) sdhci_pci(E) pcspkr(E) snd_timer(E)
>> ehci_pci(E) e1000e(E) i2c_i801(E) ehci_hcd(E) snd(E) sg(E) i915(E)
>> lpc_ich(E)
>> [   60.007366]  ptp(E) usbcore(E) cfg80211(E) mfd_core(E) pps_core(E)
>> shpchp(E) ac(E) battery(E) tpm_tis(E) tpm_tis_core(E) evdev(E) tpm(E)
>> parport_pc(E) ppdev(E) lp(E) parport(E) ip_tables(E) x_tables(E)
>> autofs4(E)
>> [   60.007474] CPU: 2 PID: 33 Comm: kworker/2:1 Tainted:
>> G            E   4.12.3-nr44-normandy-r1500619820+ #1
>> [   60.007524] Hardware name: LENOVO 4291LR7/4291LR7, BIOS CBET4000
>> 4.6-810-g50522254fb 07/21/2017
>> [   60.007580] Workqueue: usb_hub_wq hub_event [usbcore]
>> [   60.007609] task: ffff8c882b716040 task.stack: ffffb8e800d84000
>> [   60.007644] RIP: 0010:qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan]
>> [   60.007678] RSP: 0018:ffffb8e800d87b38 EFLAGS: 00010246
>> [   60.007711] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
>> 0000000000000000
>> [   60.007752] RDX: 0000000000000001 RSI: ffff8c8824f3f1d0 RDI:
>> ffff8c8824ef6400
>> [   60.007792] RBP: ffff8c8824ef6400 R08: 0000000000000000 R09:
>> 0000000000000000
>> [   60.007833] R10: ffffb8e800d87780 R11: 0000000000000011 R12:
>> ffffffffc07ea0e8
>> [   60.007874] R13: ffff8c8824e2e000 R14: ffff8c8824e2e098 R15:
>> 0000000000000000
>> [   60.007915] FS:  0000000000000000(0000) GS:ffff8c8835300000(0000)
>> knlGS:0000000000000000
>> [   60.007960] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [   60.007994] CR2: 00000000000000e0 CR3: 0000000229ca5000 CR4:
>> 00000000000406e0
>> [   60.008035] Call Trace:
>> [   60.008065]  ? usb_unbind_interface+0x71/0x270 [usbcore]
>> [   60.008101]  ? device_release_driver_internal+0x154/0x210
>> [   60.008135]  ? qmi_wwan_unbind+0x6d/0xc0 [qmi_wwan]
>> [   60.008168]  ? usbnet_disconnect+0x6c/0xf0 [usbnet]
>> [   60.008194]  ? qmi_wwan_disconnect+0x87/0xc0 [qmi_wwan]
>> [   60.008232]  ? usb_unbind_interface+0x71/0x270 [usbcore]
>> [   60.008264]  ? device_release_driver_internal+0x154/0x210
>> [   60.008296]  ? bus_remove_device+0xf5/0x160
>> [   60.008324]  ? device_del+0x1dc/0x310
>> [   60.008355]  ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
>> [   60.008393]  ? usb_disable_device+0x93/0x250 [usbcore]
>> [   60.008430]  ? usb_disconnect+0x90/0x260 [usbcore]
>> [   60.008468]  ? hub_event+0x1d9/0x14a0 [usbcore]
>> [   60.008500]  ? process_one_work+0x175/0x370
>> [   60.008528]  ? worker_thread+0x4a/0x380
>> [   60.008555]  ? kthread+0xfc/0x130
>> [   60.008579]  ? process_one_work+0x370/0x370
>> [   60.008606]  ? kthread_park+0x60/0x60
>> [   60.008631]  ? ret_from_fork+0x22/0x30
>> [   60.008656] Code: 66 0f 1f 44 00 00 66 66 66 66 90 55 48 89 fd 53
>> 48 83 ec 10 48 8b 9f c8 00 00 00 65 48 8b 04 25 28 00 00 00 48 89 44
>> 24 08 31 c0 <f6> 83 e0 00 00 00 02 74 51 e8 0d b3 2b cd 85 c0 74 67
>> 48 8b bb
>> [   60.011925] RIP: qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan] RSP:
>> ffffb8e800d87b38
>> [   60.013564] CR2: 00000000000000e0
>> [   60.022125] ---[ end trace e536b59f45bc0f25 ]---
>> [   60.025385] IPv6: ADDRCONF(NETDEV_UP): wlp2s0: link is not ready
>>
>> If I attempt a second rmmod, the process hangs. If I attempt it on
>> 4.11.x it works as expected:
>>
>> [   16.897783] fuse init (API version 7.26)
>> [   68.073552] usbcore: deregistering interface driver qmi_wwan
>> [   68.075808] qmi_wwan 2-1.4:1.6 wwp0s29u1u4i6: unregister
>> 'qmi_wwan' usb-0000:00:1d.0-1.4, WWAN/QMI device
>> [   72.431403] e1000e: enp0s25 NIC Link is Down
>>
>> So I'm pretty certain it's not coreboot causing the issue.
>>

^ permalink raw reply

* Re: qmi_wwan: Null pointer dereference when removing driver
From: Dan Williams @ 2017-07-27 15:39 UTC (permalink / raw)
  To: Nathaniel Roach, netdev
In-Reply-To: <0d39998a-dfa9-48c5-0c7f-19354f16a7c0@gmail.com>

On Thu, 2017-07-27 at 13:31 +0800, Nathaniel Roach wrote:
> Unsure at which point was added, but issue not present in stock
> debian 4.11 kernel.
> 
> Running on a Thinkpad X220 with coreboot.
> 
> I'm building from upstream. When I attempt to remove the qmi_wwan
> module (which also happens pre-suspend) the rmmod process gets
> killed, and the following shows in dmesg:

Unrelated to the crash (which should be fixed), why do you need to
remove the module pre-suspend?  Typically on a laptop the device will
either have all power cut to it over suspend and thus it'll get
reprobed on resume, or else suspend gets handled OK by the driver.  I'm
curious what the problem was that required an rmmod over suspend.

Dan

> [   59.979791] usb 2-1.4: USB disconnect, device number 4
> [   59.980102] qmi_wwan 2-1.4:1.6 wwp0s29u1u4i6: unregister
> 'qmi_wwan' usb-0000:00:1d.0-1.4, WWAN/QMI device
> [   60.006821] BUG: unable to handle kernel NULL pointer dereference
> at 00000000000000e0
> [   60.006879] IP: qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan]
> [   60.006911] PGD 0
> [   60.006911] P4D 0
> [   60.006957] Oops: 0000 [#1] SMP
> [   60.006978] Modules linked in: fuse(E) ccm(E) rfcomm(E) cmac(E)
> bnep(E) qmi_wwan(E) cdc_wdm(E) cdc_ether(E) usbnet(E) mii(E) btusb(E)
> btrtl(E) btbcm(E) btintel(E) bluetooth(E) joydev(E) xpad(E)
> ecdh_generic(E) ff_memless(E) binfmt_misc(E) snd_hda_codec_hdmi(E)
> snd_hda_codec_conexant(E) snd_hda_codec_generic(E) arc4(E)
> iTCO_wdt(E) iTCO_vendor_support(E) intel_rapl(E)
> x86_pkg_temp_thermal(E) kvm_intel(E) kvm(E) irqbypass(E)
> crct10dif_pclmul(E) crc32_pclmul(E) crc32c_intel(E)
> ghash_clmulni_intel(E) aesni_intel(E) iwlmvm(E) aes_x86_64(E)
> crypto_simd(E) mac80211(E) cryptd(E) glue_helper(E) snd_hda_intel(E)
> snd_hda_codec(E) iwlwifi(E) snd_hwdep(E) psmouse(E) snd_hda_core(E)
> snd_pcm(E) serio_raw(E) sdhci_pci(E) pcspkr(E) snd_timer(E)
> ehci_pci(E) e1000e(E) i2c_i801(E) ehci_hcd(E) snd(E) sg(E) i915(E)
> lpc_ich(E)
> [   60.007366]  ptp(E) usbcore(E) cfg80211(E) mfd_core(E) pps_core(E)
> shpchp(E) ac(E) battery(E) tpm_tis(E) tpm_tis_core(E) evdev(E) tpm(E)
> parport_pc(E) ppdev(E) lp(E) parport(E) ip_tables(E) x_tables(E)
> autofs4(E)
> [   60.007474] CPU: 2 PID: 33 Comm: kworker/2:1 Tainted:
> G            E   4.12.3-nr44-normandy-r1500619820+ #1
> [   60.007524] Hardware name: LENOVO 4291LR7/4291LR7, BIOS CBET4000
> 4.6-810-g50522254fb 07/21/2017
> [   60.007580] Workqueue: usb_hub_wq hub_event [usbcore]
> [   60.007609] task: ffff8c882b716040 task.stack: ffffb8e800d84000
> [   60.007644] RIP: 0010:qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan]
> [   60.007678] RSP: 0018:ffffb8e800d87b38 EFLAGS: 00010246
> [   60.007711] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
> 0000000000000000
> [   60.007752] RDX: 0000000000000001 RSI: ffff8c8824f3f1d0 RDI:
> ffff8c8824ef6400
> [   60.007792] RBP: ffff8c8824ef6400 R08: 0000000000000000 R09:
> 0000000000000000
> [   60.007833] R10: ffffb8e800d87780 R11: 0000000000000011 R12:
> ffffffffc07ea0e8
> [   60.007874] R13: ffff8c8824e2e000 R14: ffff8c8824e2e098 R15:
> 0000000000000000
> [   60.007915] FS:  0000000000000000(0000) GS:ffff8c8835300000(0000)
> knlGS:0000000000000000
> [   60.007960] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   60.007994] CR2: 00000000000000e0 CR3: 0000000229ca5000 CR4:
> 00000000000406e0
> [   60.008035] Call Trace:
> [   60.008065]  ? usb_unbind_interface+0x71/0x270 [usbcore]
> [   60.008101]  ? device_release_driver_internal+0x154/0x210
> [   60.008135]  ? qmi_wwan_unbind+0x6d/0xc0 [qmi_wwan]
> [   60.008168]  ? usbnet_disconnect+0x6c/0xf0 [usbnet]
> [   60.008194]  ? qmi_wwan_disconnect+0x87/0xc0 [qmi_wwan]
> [   60.008232]  ? usb_unbind_interface+0x71/0x270 [usbcore]
> [   60.008264]  ? device_release_driver_internal+0x154/0x210
> [   60.008296]  ? bus_remove_device+0xf5/0x160
> [   60.008324]  ? device_del+0x1dc/0x310
> [   60.008355]  ? usb_remove_ep_devs+0x1b/0x30 [usbcore]
> [   60.008393]  ? usb_disable_device+0x93/0x250 [usbcore]
> [   60.008430]  ? usb_disconnect+0x90/0x260 [usbcore]
> [   60.008468]  ? hub_event+0x1d9/0x14a0 [usbcore]
> [   60.008500]  ? process_one_work+0x175/0x370
> [   60.008528]  ? worker_thread+0x4a/0x380
> [   60.008555]  ? kthread+0xfc/0x130
> [   60.008579]  ? process_one_work+0x370/0x370
> [   60.008606]  ? kthread_park+0x60/0x60
> [   60.008631]  ? ret_from_fork+0x22/0x30
> [   60.008656] Code: 66 0f 1f 44 00 00 66 66 66 66 90 55 48 89 fd 53
> 48 83 ec 10 48 8b 9f c8 00 00 00 65 48 8b 04 25 28 00 00 00 48 89 44
> 24 08 31 c0 <f6> 83 e0 00 00 00 02 74 51 e8 0d b3 2b cd 85 c0 74 67
> 48 8b bb
> [   60.011925] RIP: qmi_wwan_disconnect+0x25/0xc0 [qmi_wwan] RSP:
> ffffb8e800d87b38
> [   60.013564] CR2: 00000000000000e0
> [   60.022125] ---[ end trace e536b59f45bc0f25 ]---
> [   60.025385] IPv6: ADDRCONF(NETDEV_UP): wlp2s0: link is not ready
> 
> If I attempt a second rmmod, the process hangs. If I attempt it on
> 4.11.x it works as expected:
> 
> [   16.897783] fuse init (API version 7.26)
> [   68.073552] usbcore: deregistering interface driver qmi_wwan
> [   68.075808] qmi_wwan 2-1.4:1.6 wwp0s29u1u4i6: unregister
> 'qmi_wwan' usb-0000:00:1d.0-1.4, WWAN/QMI device
> [   72.431403] e1000e: enp0s25 NIC Link is Down
> 
> So I'm pretty certain it's not coreboot causing the issue.
> 

^ permalink raw reply

* Re: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
From: Andrew Lunn @ 2017-07-27 15:37 UTC (permalink / raw)
  To: Brown, Aaron F
  Cc: John W. Linville, netdev@vger.kernel.org,
	intel-wired-lan@lists.osuosl.org
In-Reply-To: <309B89C4C689E141A5FF6A0C5FB2118B8C663B22@ORSMSX101.amr.corp.intel.com>

On Thu, Jul 27, 2017 at 12:40:01AM +0000, Brown, Aaron F wrote:
> > From: Intel-wired-lan [mailto:intel-wired-lan-bounces@osuosl.org] On Behalf
> > Of John W. Linville
> > Sent: Friday, July 21, 2017 11:12 AM
> > To: netdev@vger.kernel.org
> > Cc: intel-wired-lan@lists.osuosl.org; John W. Linville
> > <linville@tuxdriver.com>
> > Subject: [Intel-wired-lan] [PATCH] igb: support BCM54616 PHY
> > 
> > The management port on an Edgecore AS7712-32 switch uses an igb MAC,
> > but
> > it uses a BCM54616 PHY. Without a patch like this, loading the igb
> > module produces dmesg output like this:
> > 
> > [    3.439125] igb: Copyright (c) 2007-2014 Intel Corporation.
> > [    3.439866] igb: probe of 0000:00:14.0 failed with error -2
> > 
> > Signed-off-by: John W. Linville <linville@tuxdriver.com>
> > Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> > ---
> >  drivers/net/ethernet/intel/igb/e1000_82575.c   | 6 ++++++
> >  drivers/net/ethernet/intel/igb/e1000_defines.h | 1 +
> >  drivers/net/ethernet/intel/igb/e1000_hw.h      | 1 +
> >  3 files changed, 8 insertions(+)
> 
> I do not have the specific hardware (Edgecore switch) but as far as regression tests go this works fine.
> Tested-by: Aaron Brown <aaron.f.brown@intel.com>

Sorry, missed the initial post, so replying to a reply.

Linux has supported the BCM54616 PHY since April 2015. If the Intel
drivers used the Linux PHY drivers, you would not of had this problem.

It would be good if somebody spent the time to migrate these MAC
drivers to use the common Linux PHY infrastructure.

	Andrew

^ permalink raw reply

* Re: [PATCH net-next v2 07/10] net: dsa: lan9303: Added basic offloading of unicast traffic
From: Egil Hjelmeland @ 2017-07-27 14:07 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: corbet, vivien.didelot, f.fainelli, davem, kernel, linux-doc,
	linux-kernel, netdev
In-Reply-To: <20170727133113.GB18666@lunn.ch>

On 27. juli 2017 15:31, Andrew Lunn wrote:
>>> I think you are over-simplifying here. Say i have a layer 2 VPN and i
>>> bridge port 1 and the VPN? The software bridge still wants to do STP
>>> on port 1, in order to solve loops.
>>>
>>
>> Problem is that the mainline lan9303_separate_ports() does its
>> work by setting port 1 & 2 in STP BLOCKING state (and port 0 in
>> FORWARDING state). So my understanding is that it would break port
>> separation if LAN9303_SWE_PORT_STATE is written while the driver
>> is in the non-bridged state.
> 
> If the hardware cannot do it, that is a different matter. But if the
> hardware can do STP states per port, you should try to make use of it
> here.
> 

The HW does STP states per port, but not per pair of port.
I can set port 1 in learning, but I can not tell port 2
to ignore addresses learned on port 1. (Except by using VLAN).

Unless somebody can come up with an other way to implement the
port separation, I think this is how it has to be. I suppose
we don't want to break the port separation feature.


>> I thought the SW bridge would carry doing its STP work even if
>> there is a port_stp_state_set method on a DSA port?
> 
> It will, but it means you are dropping frames in software, adding
> extra load to the CPU, reducing the available bandwidth for the other
> port, etc.
> 

That is exactly the case with all traffic with the current mainline
driver.


>         Andrew
> 

Egil

^ permalink raw reply

* [net V2 04/14] net/mlx5: Fix command bad flow on command entry allocation failure
From: Saeed Mahameed @ 2017-07-27 13:56 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Moshe Shemesh, kernel-team, Saeed Mahameed
In-Reply-To: <20170727135617.18559-1-saeedm@mellanox.com>

From: Moshe Shemesh <moshe@mellanox.com>

When driver fail to allocate an entry to send command to FW, it must
notify the calling function and release the memory allocated for
this command.

Fixes: e126ba97dba9e ('mlx5: Add driver for Mellanox Connect-IB adapters')
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Cc: kernel-team@fb.com
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index 25fd32fcdf79..31cbe5e86a01 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -786,6 +786,10 @@ static void cb_timeout_handler(struct work_struct *work)
 	mlx5_cmd_comp_handler(dev, 1UL << ent->idx, true);
 }
 
+static void free_msg(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *msg);
+static void mlx5_free_cmd_msg(struct mlx5_core_dev *dev,
+			      struct mlx5_cmd_msg *msg);
+
 static void cmd_work_handler(struct work_struct *work)
 {
 	struct mlx5_cmd_work_ent *ent = container_of(work, struct mlx5_cmd_work_ent, work);
@@ -796,17 +800,28 @@ static void cmd_work_handler(struct work_struct *work)
 	struct semaphore *sem;
 	unsigned long flags;
 	bool poll_cmd = ent->polling;
+	int alloc_ret;
 
 
 	sem = ent->page_queue ? &cmd->pages_sem : &cmd->sem;
 	down(sem);
 	if (!ent->page_queue) {
-		ent->idx = alloc_ent(cmd);
-		if (ent->idx < 0) {
+		alloc_ret = alloc_ent(cmd);
+		if (alloc_ret < 0) {
 			mlx5_core_err(dev, "failed to allocate command entry\n");
+			if (ent->callback) {
+				ent->callback(-EAGAIN, ent->context);
+				mlx5_free_cmd_msg(dev, ent->out);
+				free_msg(dev, ent->in);
+				free_cmd(ent);
+			} else {
+				ent->ret = -EAGAIN;
+				complete(&ent->done);
+			}
 			up(sem);
 			return;
 		}
+		ent->idx = alloc_ret;
 	} else {
 		ent->idx = cmd->max_reg_cmds;
 		spin_lock_irqsave(&cmd->alloc_lock, flags);
-- 
2.13.0

^ permalink raw reply related

* [net V2 13/14] net/mlx5e: Schedule overflow check work to mlx5e workqueue
From: Saeed Mahameed @ 2017-07-27 13:56 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eugenia Emantayev, Saeed Mahameed
In-Reply-To: <20170727135617.18559-1-saeedm@mellanox.com>

From: Eugenia Emantayev <eugenia@mellanox.com>

This is done in order to ensure that work will not run after the cleanup.

Fixes: ef9814deafd0 ('net/mlx5e: Add HW timestamping (TS) support')
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_clock.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c b/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
index cde8e8e772ec..84dd63e74041 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
@@ -114,13 +114,14 @@ static void mlx5e_timestamp_overflow(struct work_struct *work)
 	struct delayed_work *dwork = to_delayed_work(work);
 	struct mlx5e_tstamp *tstamp = container_of(dwork, struct mlx5e_tstamp,
 						   overflow_work);
+	struct mlx5e_priv *priv = container_of(tstamp, struct mlx5e_priv, tstamp);
 	unsigned long flags;
 
 	write_lock_irqsave(&tstamp->lock, flags);
 	timecounter_read(&tstamp->clock);
 	write_unlock_irqrestore(&tstamp->lock, flags);
-	schedule_delayed_work(&tstamp->overflow_work,
-			      msecs_to_jiffies(tstamp->overflow_period * 1000));
+	queue_delayed_work(priv->wq, &tstamp->overflow_work,
+			   msecs_to_jiffies(tstamp->overflow_period * 1000));
 }
 
 int mlx5e_hwstamp_set(struct mlx5e_priv *priv, struct ifreq *ifr)
@@ -577,7 +578,7 @@ void mlx5e_timestamp_init(struct mlx5e_priv *priv)
 	INIT_WORK(&tstamp->pps_info.out_work, mlx5e_pps_out);
 	INIT_DELAYED_WORK(&tstamp->overflow_work, mlx5e_timestamp_overflow);
 	if (tstamp->overflow_period)
-		schedule_delayed_work(&tstamp->overflow_work, 0);
+		queue_delayed_work(priv->wq, &tstamp->overflow_work, 0);
 	else
 		mlx5_core_warn(priv->mdev, "invalid overflow period, overflow_work is not scheduled\n");
 
@@ -613,8 +614,6 @@ void mlx5e_timestamp_cleanup(struct mlx5e_priv *priv)
 	}
 
 	cancel_work_sync(&tstamp->pps_info.out_work);
-
-	kfree(tstamp->ptp_info.pin_config);
-
 	cancel_delayed_work_sync(&tstamp->overflow_work);
+	kfree(tstamp->ptp_info.pin_config);
 }
-- 
2.13.0

^ permalink raw reply related

* [net V2 01/14] net/mlx5: Clean SRIOV eswitch resources upon VF creation failure
From: Saeed Mahameed @ 2017-07-27 13:56 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eran Ben Elisha, Noa Osherovich, Saeed Mahameed
In-Reply-To: <20170727135617.18559-1-saeedm@mellanox.com>

From: Eran Ben Elisha <eranbe@mellanox.com>

Upon sriov enable, eswitch is always enabled.
Currently, if enable hca failed over all VFs, we would skip eswitch
disable as part of sriov disable, which will lead to resources leak.

Fix it by disabling eswitch if it was enabled (use indication from
eswitch mode).

Fixes: 6b6adee3dad2 ('net/mlx5: SRIOV core code refactoring')
Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
Signed-off-by: Noa Osherovich <noaos@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 3 ++-
 drivers/net/ethernet/mellanox/mlx5/core/sriov.c   | 5 +++++
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 89bfda419efe..8b18cc9ec026 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -1668,7 +1668,8 @@ void mlx5_eswitch_disable_sriov(struct mlx5_eswitch *esw)
 	int i;
 
 	if (!esw || !MLX5_CAP_GEN(esw->dev, vport_group_manager) ||
-	    MLX5_CAP_GEN(esw->dev, port_type) != MLX5_CAP_PORT_TYPE_ETH)
+	    MLX5_CAP_GEN(esw->dev, port_type) != MLX5_CAP_PORT_TYPE_ETH ||
+	    esw->mode == SRIOV_NONE)
 		return;
 
 	esw_info(esw->dev, "disable SRIOV: active vports(%d) mode(%d)\n",
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sriov.c b/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
index bcdf7779c48d..bf99d40e30b4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sriov.c
@@ -88,7 +88,11 @@ static void mlx5_device_disable_sriov(struct mlx5_core_dev *dev)
 	int vf;
 
 	if (!sriov->enabled_vfs)
+#ifdef CONFIG_MLX5_CORE_EN
+		goto disable_sriov_resources;
+#else
 		return;
+#endif
 
 	for (vf = 0; vf < sriov->num_vfs; vf++) {
 		if (!sriov->vfs_ctx[vf].enabled)
@@ -103,6 +107,7 @@ static void mlx5_device_disable_sriov(struct mlx5_core_dev *dev)
 	}
 
 #ifdef CONFIG_MLX5_CORE_EN
+disable_sriov_resources:
 	mlx5_eswitch_disable_sriov(dev->priv.eswitch);
 #endif
 
-- 
2.13.0

^ permalink raw reply related

* [pull request][net V2 00/14] Mellanox, mlx5 fixes 2017-07-27
From: Saeed Mahameed @ 2017-07-27 13:56 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Saeed Mahameed

Hi Dave,

This series contains some misc fixes to the mlx5 driver.

Please pull and let me know if there's any problem.

V1->V2:
 - removed redundant braces

for -stable:
4.7
net/mlx5: Fix command bad flow on command entry allocation failure

4.9
net/mlx5: Consider tx_enabled in all modes on remap
net/mlx5e: Fix outer_header_zero() check size

4.10
net/mlx5: Fix mlx5_add_flow_rules call with correct num of dests

4.11
net/mlx5: Fix mlx5_ifc_mtpps_reg_bits structure size
net/mlx5e: Add field select to MTPPS register
net/mlx5e: Fix broken disable 1PPS flow
net/mlx5e: Change 1PPS out scheme
net/mlx5e: Add missing support for PTP_CLK_REQ_PPS request
net/mlx5e: Fix wrong delay calculation for overflow check scheduling
net/mlx5e: Schedule overflow check work to mlx5e workqueue

4.12
net/mlx5: Fix command completion after timeout access invalid structure
net/mlx5e: IPoIB, Modify add/remove underlay QPN flows

I hope this is not too much, but most of the patches do apply cleanly on -stable.

Thanks,
Saeed.

---

The following changes since commit 6b84202c946cd3da3a8daa92c682510e9ed80321:

  sctp: fix the check for _sctp_walk_params and _sctp_walk_errors (2017-07-27 00:03:12 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux.git tags/mlx5-fixes-2017-07-27-V2

for you to fetch changes up to bcec601f30fb41e9233674942fa4040a6e63657a:

  net/mlx5: Fix mlx5_add_flow_rules call with correct num of dests (2017-07-27 16:40:17 +0300)

----------------------------------------------------------------
mlx5-fixes-2017-07-27-V2

----------------------------------------------------------------
Alex Vesker (1):
      net/mlx5e: IPoIB, Modify add/remove underlay QPN flows

Aviv Heller (1):
      net/mlx5: Consider tx_enabled in all modes on remap

Eran Ben Elisha (1):
      net/mlx5: Clean SRIOV eswitch resources upon VF creation failure

Eugenia Emantayev (7):
      net/mlx5: Fix mlx5_ifc_mtpps_reg_bits structure size
      net/mlx5e: Add field select to MTPPS register
      net/mlx5e: Fix broken disable 1PPS flow
      net/mlx5e: Change 1PPS out scheme
      net/mlx5e: Add missing support for PTP_CLK_REQ_PPS request
      net/mlx5e: Fix wrong delay calculation for overflow check scheduling
      net/mlx5e: Schedule overflow check work to mlx5e workqueue

Ilan Tayari (1):
      net/mlx5e: Fix outer_header_zero() check size

Moshe Shemesh (2):
      net/mlx5: Fix command completion after timeout access invalid structure
      net/mlx5: Fix command bad flow on command entry allocation failure

Paul Blakey (1):
      net/mlx5: Fix mlx5_add_flow_rules call with correct num of dests

 drivers/net/ethernet/mellanox/mlx5/core/cmd.c      |  25 ++-
 drivers/net/ethernet/mellanox/mlx5/core/en.h       |  10 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_clock.c | 222 +++++++++++++++------
 .../ethernet/mellanox/mlx5/core/en_fs_ethtool.c    |   4 +-
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c  |   1 -
 drivers/net/ethernet/mellanox/mlx5/core/eq.c       |   2 +-
 drivers/net/ethernet/mellanox/mlx5/core/eswitch.c  |   3 +-
 .../net/ethernet/mellanox/mlx5/core/ipoib/ipoib.c  |  16 +-
 drivers/net/ethernet/mellanox/mlx5/core/lag.c      |  25 +--
 .../net/ethernet/mellanox/mlx5/core/mlx5_core.h    |   5 +
 drivers/net/ethernet/mellanox/mlx5/core/sriov.c    |   5 +
 include/linux/mlx5/mlx5_ifc.h                      |  10 +-
 12 files changed, 232 insertions(+), 96 deletions(-)

^ permalink raw reply

* [net V2 03/14] net/mlx5: Fix command completion after timeout access invalid structure
From: Saeed Mahameed @ 2017-07-27 13:56 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Moshe Shemesh, kernel-team, Saeed Mahameed
In-Reply-To: <20170727135617.18559-1-saeedm@mellanox.com>

From: Moshe Shemesh <moshe@mellanox.com>

Completion on timeout should not free the driver command entry structure
as it will need to access it again once real completion event from FW
will occur.

Fixes: 73dd3a4839c1 ('net/mlx5: Avoid using pending command interface slots')
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Cc: kernel-team@fb.com
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index f5a2c605749f..25fd32fcdf79 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -967,7 +967,7 @@ static int mlx5_cmd_invoke(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *in,
 
 	err = wait_func(dev, ent);
 	if (err == -ETIMEDOUT)
-		goto out_free;
+		goto out;
 
 	ds = ent->ts2 - ent->ts1;
 	op = MLX5_GET(mbox_in, in->first.data, opcode);
@@ -1430,6 +1430,7 @@ void mlx5_cmd_comp_handler(struct mlx5_core_dev *dev, u64 vec, bool forced)
 					mlx5_core_err(dev, "Command completion arrived after timeout (entry idx = %d).\n",
 						      ent->idx);
 					free_ent(cmd, ent->idx);
+					free_cmd(ent);
 				}
 				continue;
 			}
@@ -1488,7 +1489,8 @@ void mlx5_cmd_comp_handler(struct mlx5_core_dev *dev, u64 vec, bool forced)
 				free_msg(dev, ent->in);
 
 				err = err ? err : ent->status;
-				free_cmd(ent);
+				if (!forced)
+					free_cmd(ent);
 				callback(err, context);
 			} else {
 				complete(&ent->done);
-- 
2.13.0

^ permalink raw reply related

* [net V2 12/14] net/mlx5e: Fix wrong delay calculation for overflow check scheduling
From: Saeed Mahameed @ 2017-07-27 13:56 UTC (permalink / raw)
  To: David S. Miller; +Cc: netdev, Eugenia Emantayev, Saeed Mahameed
In-Reply-To: <20170727135617.18559-1-saeedm@mellanox.com>

From: Eugenia Emantayev <eugenia@mellanox.com>

The overflow_period is calculated in seconds. In order to use it
for delayed work scheduling translation to jiffies is needed.

Fixes: ef9814deafd0 ('net/mlx5e: Add HW timestamping (TS) support')
Signed-off-by: Eugenia Emantayev <eugenia@mellanox.com>
Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_clock.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c b/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
index ba355c6b6e1f..cde8e8e772ec 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_clock.c
@@ -119,7 +119,8 @@ static void mlx5e_timestamp_overflow(struct work_struct *work)
 	write_lock_irqsave(&tstamp->lock, flags);
 	timecounter_read(&tstamp->clock);
 	write_unlock_irqrestore(&tstamp->lock, flags);
-	schedule_delayed_work(&tstamp->overflow_work, tstamp->overflow_period);
+	schedule_delayed_work(&tstamp->overflow_work,
+			      msecs_to_jiffies(tstamp->overflow_period * 1000));
 }
 
 int mlx5e_hwstamp_set(struct mlx5e_priv *priv, struct ifreq *ifr)
-- 
2.13.0

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox