* [PATCH] nfc: nci: fix potential NULL pointer dereference
From: Gustavo A. R. Silva @ 2017-06-12 22:02 UTC (permalink / raw)
To: Samuel Ortiz, David S. Miller
Cc: linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva
NULL check at line 76: if (conn_info) {, implies that pointer conn_info
might be NULL, but this pointer is being previously dereferenced,
which might cause a NULL pointer dereference.
Add NULL check before dereferencing pointer conn_info in order to
avoid a potential NULL pointer dereference.
Addresses-Coverity-ID: 1362349
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
net/nfc/nci/core.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/net/nfc/nci/core.c b/net/nfc/nci/core.c
index 61fff42..d2198ce 100644
--- a/net/nfc/nci/core.c
+++ b/net/nfc/nci/core.c
@@ -70,14 +70,13 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type,
struct nci_conn_info *conn_info;
list_for_each_entry(conn_info, &ndev->conn_info_list, list) {
- if (conn_info->dest_type == dest_type) {
+ if (conn_info && conn_info->dest_type == dest_type) {
if (!params)
return conn_info->conn_id;
- if (conn_info) {
- if (params->id == conn_info->dest_params->id &&
- params->protocol == conn_info->dest_params->protocol)
- return conn_info->conn_id;
- }
+
+ if (params->id == conn_info->dest_params->id &&
+ params->protocol == conn_info->dest_params->protocol)
+ return conn_info->conn_id;
}
}
--
2.5.0
^ permalink raw reply related
* Re: [PATCH nf-next] netns: add and use net_ns_barrier
From: Cong Wang @ 2017-06-12 21:47 UTC (permalink / raw)
To: Florian Westphal
Cc: Eric W. Biederman, netfilter-devel,
Linux Kernel Network Developers
In-Reply-To: <20170601085259.GA6067@breakpoint.cc>
On Thu, Jun 1, 2017 at 1:52 AM, Florian Westphal <fw@strlen.de> wrote:
> Joe described it nicely, problem is that after unload we may have
> conntracks that still have a nf_conn_help extension attached that
> has a pointer to a structure that resided in the (unloaded) module.
Why not hold a refcnt for its module?
^ permalink raw reply
* Re: [PATCH v4 3/3] net/cxgb4: Use new PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag
From: Alexander Duyck @ 2017-06-12 21:33 UTC (permalink / raw)
To: Casey Leedom
Cc: Ashok Raj, Ding Tianhong, Bjorn Helgaas, Michael Werner,
Ganesh Goudar, Asit K Mallick, Patrick J Cramer,
Suravee Suthikulpanit, Bob Shaw, h, Amir Ancel, Gabriele Paoloni,
David Laight, Jeff Kirsher, Catalin Marinas, Will Deacon,
Mark Rutland, Robin Murphy, David Miller, linux-arm-ke
On Mon, Jun 12, 2017 at 4:05 AM, Ding Tianhong <dingtianhong@huawei.com> wrote:
> From: Casey Leedom <leedom@chelsio.com>
>
> cxgb4 Ethernet driver now queries PCIe configuration space to determine
> if it can send TLPs to it with the Relaxed Ordering Attribute set.
>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
Casey, does this patch work for you? I just want to make sure Ding
didn't miss anything. The effect of this is that the relaxed ordering
bits being set from your original patch are now dependent on them
being set in the PCIe configuration space of the device. I recall you
mentioning something about peer to peer and this currently disables
that for the case where the root complex or any PCIe bridges in
between cannot support relaxed ordering. Does that work for you or
will you need additional changes for this driver to enable peer to
peer in the that case?
> ---
> drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 1 +
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 17 +++++++++++++++++
> drivers/net/ethernet/chelsio/cxgb4/sge.c | 5 +++--
> 3 files changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
> index e88c180..478f25a 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
> @@ -521,6 +521,7 @@ enum { /* adapter flags */
> USING_SOFT_PARAMS = (1 << 6),
> MASTER_PF = (1 << 7),
> FW_OFLD_CONN = (1 << 9),
> + ROOT_NO_RELAXED_ORDERING = (1 << 10),
> };
>
> enum {
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> index 38a5c67..1dd093d 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> @@ -4726,6 +4726,23 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
> adapter->msg_enable = DFLT_MSG_ENABLE;
> memset(adapter->chan_map, 0xff, sizeof(adapter->chan_map));
>
> + /* If possible, we use PCIe Relaxed Ordering Attribute to deliver
> + * Ingress Packet Data to Free List Buffers in order to allow for
> + * chipset performance optimizations between the Root Complex and
> + * Memory Controllers. (Messages to the associated Ingress Queue
> + * notifying new Packet Placement in the Free Lists Buffers will be
> + * send without the Relaxed Ordering Attribute thus guaranteeing that
> + * all preceding PCIe Transaction Layer Packets will be processed
> + * first.) But some Root Complexes have various issues with Upstream
> + * Transaction Layer Packets with the Relaxed Ordering Attribute set.
> + * The PCIe devices which under the Root Complexes will be cleared the
> + * Relaxed Ordering bit in the configuration space, So we check our
> + * PCIe configuration space to see if it's flagged with advice against
> + * using Relaxed Ordering.
> + */
> + if (pcie_relaxed_ordering_supported(pdev))
> + adapter->flags |= ROOT_NO_RELAXED_ORDERING;
> +
> spin_lock_init(&adapter->stats_lock);
> spin_lock_init(&adapter->tid_release_lock);
> spin_lock_init(&adapter->win0_lock);
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/sge.c b/drivers/net/ethernet/chelsio/cxgb4/sge.c
> index f05f0d4..ac229a3 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
> @@ -2571,6 +2571,7 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
> struct fw_iq_cmd c;
> struct sge *s = &adap->sge;
> struct port_info *pi = netdev_priv(dev);
> + int relaxed = !(adap->flags & ROOT_NO_RELAXED_ORDERING);
>
> /* Size needs to be multiple of 16, including status entry. */
> iq->size = roundup(iq->size, 16);
> @@ -2624,8 +2625,8 @@ int t4_sge_alloc_rxq(struct adapter *adap, struct sge_rspq *iq, bool fwevtq,
>
> flsz = fl->size / 8 + s->stat_len / sizeof(struct tx_desc);
> c.iqns_to_fl0congen |= htonl(FW_IQ_CMD_FL0PACKEN_F |
> - FW_IQ_CMD_FL0FETCHRO_F |
> - FW_IQ_CMD_FL0DATARO_F |
> + FW_IQ_CMD_FL0FETCHRO_V(relaxed) |
> + FW_IQ_CMD_FL0DATARO_V(relaxed) |
> FW_IQ_CMD_FL0PADEN_F);
> if (cong >= 0)
> c.iqns_to_fl0congen |=
> --
> 1.9.0
>
>
^ permalink raw reply
* Re: [PATCH v4 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Alexander Duyck @ 2017-06-12 21:28 UTC (permalink / raw)
To: Ding Tianhong
Cc: Casey Leedom, Ashok Raj, Bjorn Helgaas, Michael Werner,
Ganesh Goudar, Asit K Mallick, Patrick J Cramer,
Suravee Suthikulpanit, Bob Shaw, h, Amir Ancel, Gabriele Paoloni,
David Laight, Jeff Kirsher, Catalin Marinas, Will Deacon,
Mark Rutland, Robin Murphy, David Miller, linux-arm-kernel
In-Reply-To: <1497265525-4752-3-git-send-email-dingtianhong@huawei.com>
On Mon, Jun 12, 2017 at 4:05 AM, Ding Tianhong <dingtianhong@huawei.com> wrote:
> The PCIe Device Control Register use the bit 4 to indicate that
> whether the device is permitted to enable relaxed ordering or not.
> But relaxed ordering is not safe for some platform which could only
> use strong write ordering, so devices are allowed (but not required)
> to enable relaxed ordering bit by default.
>
> If a PCIe device didn't enable the relaxed ordering attribute default,
> we should not do anything in the PCIe configuration, otherwise we
> should check if any of the devices above us do not support relaxed
> ordering by the PCI_DEV_FLAGS_NO_RELAXED_ORDERING flag, then base on
> the result if we get a return that indicate that the relaxed ordering
> is not supported we should update our device to disable relaxed ordering
> in configuration space. If the device above us doesn't exist or isn't
> the PCIe device, we shouldn't do anything and skip updating relaxed ordering
> because we are probably running in a guest machine.
>
> Signed-off-by: Ding Tianhong <dingtianhong@huawei.com>
> ---
> drivers/pci/pci.c | 32 ++++++++++++++++++++++++++++++++
> drivers/pci/probe.c | 41 +++++++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 2 ++
> 3 files changed, 75 insertions(+)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b01bd5b..b44f34c 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4878,6 +4878,38 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
> EXPORT_SYMBOL(pcie_set_mps);
>
> /**
> + * pcie_clear_relaxed_ordering - clear PCI Express relaxed ordering bit
> + * @dev: PCI device to query
> + *
> + * If possible clear relaxed ordering
> + */
> +int pcie_clear_relaxed_ordering(struct pci_dev *dev)
> +{
> + return pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> + PCI_EXP_DEVCTL_RELAX_EN);
> +}
> +EXPORT_SYMBOL(pcie_clear_relaxed_ordering);
> +
> +/**
> + * pcie_relaxed_ordering_supported - Probe for PCIe relexed ordering support
> + * @dev: PCI device to query
> + *
> + * Returns true if the device support relaxed ordering attribute.
> + */
> +bool pcie_relaxed_ordering_supported(struct pci_dev *dev)
> +{
> + bool ro_supported = false;
> + u16 v;
> +
> + pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
> + if ((v & PCI_EXP_DEVCTL_RELAX_EN) >> 4)
> + ro_supported = true;
Instead of "return ro_supported" why not just "return !!(v &
PCIE_EXP_DEVCTL_RELAX_EN)"? You can cut out the extra steps and save
yourself some extra steps this way since the shift by 4 shouldn't even
really be needed since you are just testing for a bit anyway.
> +
> + return ro_supported;
> +}
> +EXPORT_SYMBOL(pcie_relaxed_ordering_supported);
> +
> +/**
> * pcie_get_minimum_link - determine minimum link settings of a PCI device
> * @dev: PCI device to query
> * @speed: storage for minimum speed
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..ed1f717 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1701,6 +1701,46 @@ static void pci_configure_extended_tags(struct pci_dev *dev)
> PCI_EXP_DEVCTL_EXT_TAG);
> }
>
> +/**
> + * pci_dev_should_disable_relaxed_ordering - check if the PCI device
> + * should disable the relaxed ordering attribute.
> + * @dev: PCI device
> + *
> + * Return true if any of the PCI devices above us do not support
> + * relaxed ordering.
> + */
> +static bool pci_dev_should_disable_relaxed_ordering(struct pci_dev *dev)
> +{
> + bool ro_disabled = false;
> +
> + while (dev) {
> + if (dev->dev_flags & PCI_DEV_FLAGS_NO_RELAXED_ORDERING) {
> + ro_disabled = true;
> + break;
> + }
> + dev = dev->bus->self;
> + }
> +
> + return ro_disabled;
Same thing here. I would suggest just returning either true or false,
and drop the ro_disabled value. It will return the lines of code and
make things a bit bit more direct.
> +}
> +
> +static void pci_configure_relaxed_ordering(struct pci_dev *dev)
> +{
> + struct pci_dev *bridge = pci_upstream_bridge(dev);
> +
> + if (!pci_is_pcie(dev) || !bridge || !pci_is_pcie(bridge))
> + return;
The pci_is_pcie check is actually redundant based on the
pcie_relaxed_ordering_supported check using pcie_capability_read_word.
Also I am not sure what the point is of the pci_upstream_bridge()
check is, it seems like you should be able to catch all the same stuff
in your pci_dev_should_disable_relaxed_ordering() call. Though it did
give me a thought. I don't think we can alter this for a VF, so you
might want to add a check for dev->is_virtfn to the list of checks and
if it is a virtual function just return since I don't think there are
any VFs that would let you alter this bit anyway.
> + /* If the releaxed ordering enable bit is not set, do nothing. */
> + if (!pcie_relaxed_ordering_supported(dev))
> + return;
> +
> + if (pci_dev_should_disable_relaxed_ordering(dev)) {
> + pcie_clear_relaxed_ordering(dev);
> + dev_info(&dev->dev, "Disable Relaxed Ordering\n");
> + }
> +}
> +
> static void pci_configure_device(struct pci_dev *dev)
> {
> struct hotplug_params hpp;
> @@ -1708,6 +1748,7 @@ static void pci_configure_device(struct pci_dev *dev)
>
> pci_configure_mps(dev);
> pci_configure_extended_tags(dev);
> + pci_configure_relaxed_ordering(dev);
>
> memset(&hpp, 0, sizeof(hpp));
> ret = pci_get_hp_params(dev, &hpp);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index e1e8428..9870781 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1105,6 +1105,8 @@ int __pci_enable_wake(struct pci_dev *dev, pci_power_t state,
> void pci_pme_wakeup_bus(struct pci_bus *bus);
> void pci_d3cold_enable(struct pci_dev *dev);
> void pci_d3cold_disable(struct pci_dev *dev);
> +int pcie_clear_relaxed_ordering(struct pci_dev *dev);
> +bool pcie_relaxed_ordering_supported(struct pci_dev *dev);
>
> static inline int pci_enable_wake(struct pci_dev *dev, pci_power_t state,
> bool enable)
> --
> 1.9.0
>
>
^ permalink raw reply
* [PATCH 2/2] mdio_bus: use devm_gpiod_get_optional()
From: Sergei Shtylyov @ 2017-06-12 20:55 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, netdev, Rob Herring, Frank Rowand,
devicetree
Cc: Sergei Shtylyov
[-- Attachment #1: mdio_bus-use-devm_gpiod_get_optional.patch --]
[-- Type: text/plain, Size: 1201 bytes --]
The MDIO reset GPIO is really a classical optional GPIO property case,
so devm_gpiod_get_optional() should have been used, not devm_gpiod_get().
Doing this saves several LoCs...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
drivers/net/phy/mdio_bus.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
Index: net-next/drivers/net/phy/mdio_bus.c
===================================================================
--- net-next.orig/drivers/net/phy/mdio_bus.c
+++ net-next/drivers/net/phy/mdio_bus.c
@@ -354,16 +354,12 @@ int __mdiobus_register(struct mii_bus *b
mutex_init(&bus->mdio_lock);
/* de-assert bus level PHY GPIO reset */
- gpiod = devm_gpiod_get(&bus->dev, "reset", GPIOD_OUT_LOW);
+ gpiod = devm_gpiod_get_optional(&bus->dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(gpiod)) {
- err = PTR_ERR(gpiod);
- if (err != -ENOENT) {
- dev_err(&bus->dev,
- "mii_bus %s couldn't get reset GPIO\n",
- bus->id);
- return err;
- }
- } else {
+ dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO\n",
+ bus->id);
+ return PTR_ERR(gpiod);
+ } else if (gpiod) {
bus->reset_gpiod = gpiod;
gpiod_set_value_cansleep(gpiod, 1);
^ permalink raw reply
* [PATCH net-next] ibmvnic: Remove netdev notify for failover resets
From: Nathan Fontenot @ 2017-06-13 0:47 UTC (permalink / raw)
To: netdev
When handling a driver reset due to a failover of the backing
server on the vios, doing the netdev_notify_peers() can cause
network traffic to stall or halt. Remove the netdev notify call
for failover resets.
Signed-off-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
---
drivers/net/ethernet/ibm/ibmvnic.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 7d84e20..1c647c0 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -1364,7 +1364,9 @@ static int do_reset(struct ibmvnic_adapter *adapter,
for (i = 0; i < adapter->req_rx_queues; i++)
napi_schedule(&adapter->napi[i]);
- netdev_notify_peers(netdev);
+ if (adapter->reset_reason != VNIC_RESET_FAILOVER)
+ netdev_notify_peers(netdev);
+
return 0;
}
^ permalink raw reply related
* [PATCH 2/2] mdio_bus: use devm_gpiod_get_optional()
From: Sergei Shtylyov @ 2017-06-12 20:55 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, netdev-u79uwXL29TY76Z2rM5mHXA,
Rob Herring, Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Sergei Shtylyov
[-- Attachment #1: mdio_bus-use-devm_gpiod_get_optional.patch --]
[-- Type: text/plain, Size: 1451 bytes --]
The MDIO reset GPIO is really a classical optional GPIO property case,
so devm_gpiod_get_optional() should have been used, not devm_gpiod_get().
Doing this saves several LoCs...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
---
drivers/net/phy/mdio_bus.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
Index: net-next/drivers/net/phy/mdio_bus.c
===================================================================
--- net-next.orig/drivers/net/phy/mdio_bus.c
+++ net-next/drivers/net/phy/mdio_bus.c
@@ -354,16 +354,12 @@ int __mdiobus_register(struct mii_bus *b
mutex_init(&bus->mdio_lock);
/* de-assert bus level PHY GPIO reset */
- gpiod = devm_gpiod_get(&bus->dev, "reset", GPIOD_OUT_LOW);
+ gpiod = devm_gpiod_get_optional(&bus->dev, "reset", GPIOD_OUT_LOW);
if (IS_ERR(gpiod)) {
- err = PTR_ERR(gpiod);
- if (err != -ENOENT) {
- dev_err(&bus->dev,
- "mii_bus %s couldn't get reset GPIO\n",
- bus->id);
- return err;
- }
- } else {
+ dev_err(&bus->dev, "mii_bus %s couldn't get reset GPIO\n",
+ bus->id);
+ return PTR_ERR(gpiod);
+ } else if (gpiod) {
bus->reset_gpiod = gpiod;
gpiod_set_value_cansleep(gpiod, 1);
--
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
* [PATCH 1/2] mdio_bus: handle only single PHY reset GPIO
From: Sergei Shtylyov @ 2017-06-12 20:55 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: Sergei Shtylyov
[-- Attachment #1: mdio_bus-handle-only-single-PHY-reset-GPIO.patch --]
[-- Type: text/plain, Size: 4056 bytes --]
Commit 4c5e7a2c0501 ("dt-bindings: mdio: Clarify binding document")
declared that a MDIO reset GPIO property should have only a single GPIO
reference/specifier, however the supporting code was left intact, still
burdening the kernel with now apparently useless loops -- get rid of them.
Signed-off-by: Sergei Shtylyov <sergei.shtylyov-M4DtvfQ/ZS1MRgGoP+s0PdBPR1lH4CV8@public.gmane.org>
---
drivers/net/phy/mdio_bus.c | 53 ++++++++++++++++-----------------------------
drivers/of/of_mdio.c | 1
include/linux/phy.h | 6 +----
3 files changed, 21 insertions(+), 39 deletions(-)
Index: net-next/drivers/net/phy/mdio_bus.c
===================================================================
--- net-next.orig/drivers/net/phy/mdio_bus.c
+++ net-next/drivers/net/phy/mdio_bus.c
@@ -353,33 +353,22 @@ int __mdiobus_register(struct mii_bus *b
mutex_init(&bus->mdio_lock);
- /* de-assert bus level PHY GPIO resets */
- if (bus->num_reset_gpios > 0) {
- bus->reset_gpiod = devm_kcalloc(&bus->dev,
- bus->num_reset_gpios,
- sizeof(struct gpio_desc *),
- GFP_KERNEL);
- if (!bus->reset_gpiod)
- return -ENOMEM;
- }
-
- for (i = 0; i < bus->num_reset_gpios; i++) {
- gpiod = devm_gpiod_get_index(&bus->dev, "reset", i,
- GPIOD_OUT_LOW);
- if (IS_ERR(gpiod)) {
- err = PTR_ERR(gpiod);
- if (err != -ENOENT) {
- dev_err(&bus->dev,
- "mii_bus %s couldn't get reset GPIO\n",
- bus->id);
- return err;
- }
- } else {
- bus->reset_gpiod[i] = gpiod;
- gpiod_set_value_cansleep(gpiod, 1);
- udelay(bus->reset_delay_us);
- gpiod_set_value_cansleep(gpiod, 0);
+ /* de-assert bus level PHY GPIO reset */
+ gpiod = devm_gpiod_get(&bus->dev, "reset", GPIOD_OUT_LOW);
+ if (IS_ERR(gpiod)) {
+ err = PTR_ERR(gpiod);
+ if (err != -ENOENT) {
+ dev_err(&bus->dev,
+ "mii_bus %s couldn't get reset GPIO\n",
+ bus->id);
+ return err;
}
+ } else {
+ bus->reset_gpiod = gpiod;
+
+ gpiod_set_value_cansleep(gpiod, 1);
+ udelay(bus->reset_delay_us);
+ gpiod_set_value_cansleep(gpiod, 0);
}
if (bus->reset)
@@ -414,10 +403,8 @@ error:
}
/* Put PHYs in RESET to save power */
- for (i = 0; i < bus->num_reset_gpios; i++) {
- if (bus->reset_gpiod[i])
- gpiod_set_value_cansleep(bus->reset_gpiod[i], 1);
- }
+ if (bus->reset_gpiod)
+ gpiod_set_value_cansleep(bus->reset_gpiod, 1);
device_del(&bus->dev);
return err;
@@ -442,10 +429,8 @@ void mdiobus_unregister(struct mii_bus *
}
/* Put PHYs in RESET to save power */
- for (i = 0; i < bus->num_reset_gpios; i++) {
- if (bus->reset_gpiod[i])
- gpiod_set_value_cansleep(bus->reset_gpiod[i], 1);
- }
+ if (bus->reset_gpiod)
+ gpiod_set_value_cansleep(bus->reset_gpiod, 1);
device_del(&bus->dev);
}
Index: net-next/drivers/of/of_mdio.c
===================================================================
--- net-next.orig/drivers/of/of_mdio.c
+++ net-next/drivers/of/of_mdio.c
@@ -226,7 +226,6 @@ int of_mdiobus_register(struct mii_bus *
/* Get bus level PHY reset GPIO details */
mdio->reset_delay_us = DEFAULT_GPIO_RESET_DELAY;
of_property_read_u32(np, "reset-delay-us", &mdio->reset_delay_us);
- mdio->num_reset_gpios = of_gpio_named_count(np, "reset-gpios");
/* Register the MDIO bus */
rc = mdiobus_register(mdio);
Index: net-next/include/linux/phy.h
===================================================================
--- net-next.orig/include/linux/phy.h
+++ net-next/include/linux/phy.h
@@ -226,10 +226,8 @@ struct mii_bus {
/* GPIO reset pulse width in microseconds */
int reset_delay_us;
- /* Number of reset GPIOs */
- int num_reset_gpios;
- /* Array of RESET GPIO descriptors */
- struct gpio_desc **reset_gpiod;
+ /* RESET GPIO descriptor pointer */
+ struct gpio_desc *reset_gpiod;
};
#define to_mii_bus(d) container_of(d, struct mii_bus, dev)
--
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
* [PATCH 0/2] MDIO bus reset GPIO cleanups
From: Sergei Shtylyov @ 2017-06-12 20:55 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand,
netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA
Hello!
Commit 4c5e7a2c0501 ("dt-bindings: mdio: Clarify binding document")
declared that a MDIO reset GPIO property should have only a single GPIO
reference/specifier, however the supporting code was left intact...
Here's a couple of the obvious cleanups to that code:
[1/2] mdio_bus: handle only single PHY reset GPIO
[2/2] mdio_bus: use devm_gpiod_get_optional()
MBR, Sergei
--
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
* Re: [PATCH] net: Fix inconsistent teardown and release of private netdev state.
From: Stephen Hemminger @ 2017-06-12 20:40 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20170609.145434.399590645965329804.davem@davemloft.net>
On Fri, 09 Jun 2017 14:54:34 -0400 (EDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <stephen@networkplumber.org>
> Date: Fri, 9 Jun 2017 10:21:04 -0700
>
> > Is there anything in Documentation/networking/netdevices.txt about this to
> > avoid any future issues?
>
> You asked me about this last time, and I did not forget about it.
>
> I sincerely lack the time to do a writeup about it, and I felt that
> delaying the fix for another week or two until I find that magical
> non-existing timeframe to write the docs was not beneficial for
> users at all.
I could take a stab at it, but my writing skills are not great.
We don't need to delay for Documentation, in fact it is easier to write after
the cleanup.
^ permalink raw reply
* [PATCH v1 2/2] ptp: Add a ptp clock driver for Broadcom DTE
From: Arun Parameswaran @ 2017-06-12 20:26 UTC (permalink / raw)
To: Richard Cochran, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, netdev, bcm-kernel-feedback-list,
Arun Parameswaran
In-Reply-To: <1497299161-6458-1-git-send-email-arun.parameswaran@broadcom.com>
This patch adds a ptp clock driver for the Broadcom SoCs using
the Digital timing Engine (DTE) nco.
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
---
drivers/ptp/Kconfig | 16 +++
drivers/ptp/Makefile | 1 +
drivers/ptp/ptp_dte.c | 353 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 370 insertions(+)
create mode 100644 drivers/ptp/ptp_dte.c
diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 384f661..a21ad10 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -25,6 +25,22 @@ config PTP_1588_CLOCK
To compile this driver as a module, choose M here: the module
will be called ptp.
+config PTP_1588_CLOCK_DTE
+ tristate "Broadcom DTE as PTP clock"
+ depends on PTP_1588_CLOCK
+ depends on NET && HAS_IOMEM
+ depends on ARCH_BCM_MOBILE || (ARCH_BCM_IPROC && !(ARCH_BCM_NSP || ARCH_BCM_5301X)) || COMPILE_TEST
+ default y
+ help
+ This driver adds support for using the Digital timing engine
+ (DTE) in the Broadcom SoC's as a PTP clock.
+
+ The clock can be used in both wired and wireless networks
+ for PTP purposes.
+
+ To compile this driver as a module, choose M here: the module
+ will be called ptp_dte.
+
config PTP_1588_CLOCK_GIANFAR
tristate "Freescale eTSEC as PTP clock"
depends on GIANFAR
diff --git a/drivers/ptp/Makefile b/drivers/ptp/Makefile
index 5307361..d1f2fb1 100644
--- a/drivers/ptp/Makefile
+++ b/drivers/ptp/Makefile
@@ -4,6 +4,7 @@
ptp-y := ptp_clock.o ptp_chardev.o ptp_sysfs.o
obj-$(CONFIG_PTP_1588_CLOCK) += ptp.o
+obj-$(CONFIG_PTP_1588_CLOCK_DTE) += ptp_dte.o
obj-$(CONFIG_PTP_1588_CLOCK_IXP46X) += ptp_ixp46x.o
obj-$(CONFIG_PTP_1588_CLOCK_PCH) += ptp_pch.o
obj-$(CONFIG_PTP_1588_CLOCK_KVM) += ptp_kvm.o
diff --git a/drivers/ptp/ptp_dte.c b/drivers/ptp/ptp_dte.c
new file mode 100644
index 0000000..00145a3
--- /dev/null
+++ b/drivers/ptp/ptp_dte.c
@@ -0,0 +1,353 @@
+/*
+ * Copyright 2017 Broadcom
+ *
+ * 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 version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/ptp_clock_kernel.h>
+#include <linux/types.h>
+
+#define DTE_NCO_LOW_TIME_REG 0x00
+#define DTE_NCO_TIME_REG 0x04
+#define DTE_NCO_OVERFLOW_REG 0x08
+#define DTE_NCO_INC_REG 0x0c
+
+#define DTE_NCO_SUM2_MASK 0xffffffff
+#define DTE_NCO_SUM2_SHIFT 4ULL
+
+#define DTE_NCO_SUM3_MASK 0xff
+#define DTE_NCO_SUM3_SHIFT 36ULL
+#define DTE_NCO_SUM3_WR_SHIFT 8
+
+#define DTE_NCO_TS_WRAP_MASK 0xfff
+#define DTE_NCO_TS_WRAP_LSHIFT 32
+
+#define DTE_NCO_INC_DEFAULT 0x80000000
+#define DTE_NUM_REGS_TO_RESTORE 4
+
+/* Full wrap around is 44bits in ns (~4.887 hrs) */
+#define DTE_WRAP_AROUND_NSEC_SHIFT 44
+
+/* 44 bits NCO */
+#define DTE_NCO_MAX_NS 0xFFFFFFFFFFF
+
+/* 125MHz with 3.29 reg cfg */
+#define DTE_PPB_ADJ(ppb) (u32)(div64_u64((((u64)abs(ppb) * BIT(28)) +\
+ 62500000ULL), 125000000ULL))
+
+/* ptp dte priv structure */
+struct ptp_dte {
+ void __iomem *regs;
+ struct ptp_clock *ptp_clk;
+ struct ptp_clock_info caps;
+ struct device *dev;
+ u32 ts_ovf_last;
+ u32 ts_wrap_cnt;
+ spinlock_t lock;
+ u32 reg_val[DTE_NUM_REGS_TO_RESTORE];
+};
+
+static void dte_write_nco(void __iomem *regs, s64 ns)
+{
+ u32 sum2, sum3;
+
+ sum2 = (u32)((ns >> DTE_NCO_SUM2_SHIFT) & DTE_NCO_SUM2_MASK);
+ /* compensate for ignoring sum1 */
+ if (sum2 != DTE_NCO_SUM2_MASK)
+ sum2++;
+
+ /* to write sum3, bits [15:8] needs to be written */
+ sum3 = (u32)(((ns >> DTE_NCO_SUM3_SHIFT) & DTE_NCO_SUM3_MASK) <<
+ DTE_NCO_SUM3_WR_SHIFT);
+
+ writel(0, (regs + DTE_NCO_LOW_TIME_REG));
+ writel(sum2, (regs + DTE_NCO_TIME_REG));
+ writel(sum3, (regs + DTE_NCO_OVERFLOW_REG));
+}
+
+static s64 dte_read_nco(void __iomem *regs)
+{
+ u32 sum2, sum3;
+ s64 ns;
+
+ /*
+ * ignoring sum1 (4 bits) gives a 16ns resolution, which
+ * works due to the async register read.
+ */
+ sum3 = readl(regs + DTE_NCO_OVERFLOW_REG) & DTE_NCO_SUM3_MASK;
+ sum2 = readl(regs + DTE_NCO_TIME_REG);
+ ns = ((s64)sum3 << DTE_NCO_SUM3_SHIFT) |
+ ((s64)sum2 << DTE_NCO_SUM2_SHIFT);
+
+ return ns;
+}
+
+static void dte_write_nco_delta(struct ptp_dte *ptp_dte, s64 delta)
+{
+ s64 ns;
+
+ ns = dte_read_nco(ptp_dte->regs);
+
+ /* handle wraparound conditions */
+ if ((delta < 0) && (abs(delta) > ns)) {
+ if (ptp_dte->ts_wrap_cnt) {
+ ns += DTE_NCO_MAX_NS + delta;
+ ptp_dte->ts_wrap_cnt--;
+ } else {
+ ns = 0;
+ }
+ } else {
+ ns += delta;
+ if (ns > DTE_NCO_MAX_NS) {
+ ptp_dte->ts_wrap_cnt++;
+ ns -= DTE_NCO_MAX_NS;
+ }
+ }
+
+ dte_write_nco(ptp_dte->regs, ns);
+
+ ptp_dte->ts_ovf_last = (ns >> DTE_NCO_TS_WRAP_LSHIFT) &
+ DTE_NCO_TS_WRAP_MASK;
+}
+
+static s64 dte_read_nco_with_ovf(struct ptp_dte *ptp_dte)
+{
+ u32 ts_ovf;
+ s64 ns = 0;
+
+ ns = dte_read_nco(ptp_dte->regs);
+
+ /*Timestamp overflow: 8 LSB bits of sum3, 4 MSB bits of sum2 */
+ ts_ovf = (ns >> DTE_NCO_TS_WRAP_LSHIFT) & DTE_NCO_TS_WRAP_MASK;
+
+ /* Check for wrap around */
+ if (ts_ovf < ptp_dte->ts_ovf_last)
+ ptp_dte->ts_wrap_cnt++;
+
+ ptp_dte->ts_ovf_last = ts_ovf;
+
+ /* adjust for wraparounds */
+ ns += (s64)(BIT_ULL(DTE_WRAP_AROUND_NSEC_SHIFT) * ptp_dte->ts_wrap_cnt);
+
+ return ns;
+}
+
+static int ptp_dte_adjfreq(struct ptp_clock_info *ptp, s32 ppb)
+{
+ u32 nco_incr;
+ unsigned long flags;
+ struct ptp_dte *ptp_dte = container_of(ptp, struct ptp_dte, caps);
+
+ if (abs(ppb) > ptp_dte->caps.max_adj) {
+ dev_err(ptp_dte->dev, "ppb adj too big\n");
+ return -EINVAL;
+ }
+
+ if (ppb < 0)
+ nco_incr = DTE_NCO_INC_DEFAULT - DTE_PPB_ADJ(ppb);
+ else
+ nco_incr = DTE_NCO_INC_DEFAULT + DTE_PPB_ADJ(ppb);
+
+ spin_lock_irqsave(&ptp_dte->lock, flags);
+ writel(nco_incr, ptp_dte->regs + DTE_NCO_INC_REG);
+ spin_unlock_irqrestore(&ptp_dte->lock, flags);
+
+ return 0;
+}
+
+static int ptp_dte_adjtime(struct ptp_clock_info *ptp, s64 delta)
+{
+ unsigned long flags;
+ struct ptp_dte *ptp_dte = container_of(ptp, struct ptp_dte, caps);
+
+ spin_lock_irqsave(&ptp_dte->lock, flags);
+ dte_write_nco_delta(ptp_dte, delta);
+ spin_unlock_irqrestore(&ptp_dte->lock, flags);
+
+ return 0;
+}
+
+static int ptp_dte_gettime(struct ptp_clock_info *ptp, struct timespec64 *ts)
+{
+ unsigned long flags;
+ struct ptp_dte *ptp_dte = container_of(ptp, struct ptp_dte, caps);
+
+ spin_lock_irqsave(&ptp_dte->lock, flags);
+ *ts = ns_to_timespec64(dte_read_nco_with_ovf(ptp_dte));
+ spin_unlock_irqrestore(&ptp_dte->lock, flags);
+
+ return 0;
+}
+
+static int ptp_dte_settime(struct ptp_clock_info *ptp,
+ const struct timespec64 *ts)
+{
+ unsigned long flags;
+ struct ptp_dte *ptp_dte = container_of(ptp, struct ptp_dte, caps);
+
+ spin_lock_irqsave(&ptp_dte->lock, flags);
+
+ /* Disable nco increment */
+ writel(0, ptp_dte->regs + DTE_NCO_INC_REG);
+
+ dte_write_nco(ptp_dte->regs, timespec64_to_ns(ts));
+
+ /* reset overflow and wrap counter */
+ ptp_dte->ts_ovf_last = 0;
+ ptp_dte->ts_wrap_cnt = 0;
+
+ /* Enable nco increment */
+ writel(DTE_NCO_INC_DEFAULT, ptp_dte->regs + DTE_NCO_INC_REG);
+
+ spin_unlock_irqrestore(&ptp_dte->lock, flags);
+
+ return 0;
+}
+
+static int ptp_dte_enable(struct ptp_clock_info *ptp,
+ struct ptp_clock_request *rq, int on)
+{
+ return -EOPNOTSUPP;
+}
+
+static struct ptp_clock_info ptp_dte_caps = {
+ .owner = THIS_MODULE,
+ .name = "DTE PTP timer",
+ .max_adj = 50000000,
+ .n_ext_ts = 0,
+ .n_pins = 0,
+ .pps = 0,
+ .adjfreq = ptp_dte_adjfreq,
+ .adjtime = ptp_dte_adjtime,
+ .gettime64 = ptp_dte_gettime,
+ .settime64 = ptp_dte_settime,
+ .enable = ptp_dte_enable,
+};
+
+static int ptp_dte_probe(struct platform_device *pdev)
+{
+ struct ptp_dte *ptp_dte;
+ struct device *dev = &pdev->dev;
+ struct resource *res;
+
+ ptp_dte = devm_kzalloc(dev, sizeof(struct ptp_dte), GFP_KERNEL);
+ if (!ptp_dte)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ ptp_dte->regs = devm_ioremap_resource(dev, res);
+ if (IS_ERR(ptp_dte->regs)) {
+ dev_err(dev,
+ "%s: io remap failed\n", __func__);
+ return PTR_ERR(ptp_dte->regs);
+ }
+
+ spin_lock_init(&ptp_dte->lock);
+
+ ptp_dte->dev = dev;
+ ptp_dte->caps = ptp_dte_caps;
+ ptp_dte->ptp_clk = ptp_clock_register(&ptp_dte->caps, &pdev->dev);
+ if (IS_ERR(ptp_dte->ptp_clk)) {
+ dev_err(dev,
+ "%s: Failed to register ptp clock\n", __func__);
+ return PTR_ERR(ptp_dte->ptp_clk);
+ }
+
+ platform_set_drvdata(pdev, ptp_dte);
+
+ dev_info(dev, "ptp clk probe done\n");
+
+ return 0;
+}
+
+static int ptp_dte_remove(struct platform_device *pdev)
+{
+ struct ptp_dte *ptp_dte = platform_get_drvdata(pdev);
+ u8 i;
+
+ ptp_clock_unregister(ptp_dte->ptp_clk);
+
+ for (i = 0; i < DTE_NUM_REGS_TO_RESTORE; i++)
+ writel(0, ptp_dte->regs + (i * sizeof(u32)));
+
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int ptp_dte_suspend(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ptp_dte *ptp_dte = platform_get_drvdata(pdev);
+ u8 i;
+
+ for (i = 0; i < DTE_NUM_REGS_TO_RESTORE; i++) {
+ ptp_dte->reg_val[i] =
+ readl(ptp_dte->regs + (i * sizeof(u32)));
+ }
+
+ /* disable the nco */
+ writel(0, ptp_dte->regs + DTE_NCO_INC_REG);
+
+ return 0;
+}
+
+static int ptp_dte_resume(struct device *dev)
+{
+ struct platform_device *pdev = to_platform_device(dev);
+ struct ptp_dte *ptp_dte = platform_get_drvdata(pdev);
+ u8 i;
+
+ for (i = 0; i < DTE_NUM_REGS_TO_RESTORE; i++) {
+ if ((i * sizeof(u32)) != DTE_NCO_OVERFLOW_REG)
+ writel(ptp_dte->reg_val[i],
+ (ptp_dte->regs + (i * sizeof(u32))));
+ else
+ writel(((ptp_dte->reg_val[i] &
+ DTE_NCO_SUM3_MASK) << DTE_NCO_SUM3_WR_SHIFT),
+ (ptp_dte->regs + (i * sizeof(u32))));
+ }
+
+ return 0;
+}
+
+static const struct dev_pm_ops ptp_dte_pm_ops = {
+ .suspend = ptp_dte_suspend,
+ .resume = ptp_dte_resume
+};
+
+#define PTP_DTE_PM_OPS (&ptp_dte_pm_ops)
+#else
+#define PTP_DTE_PM_OPS NULL
+#endif
+
+static const struct of_device_id ptp_dte_of_match[] = {
+ { .compatible = "brcm,ptp-dte", },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ptp_dte_of_match);
+
+static struct platform_driver ptp_dte_driver = {
+ .driver = {
+ .name = "ptp-dte",
+ .pm = PTP_DTE_PM_OPS,
+ .of_match_table = ptp_dte_of_match,
+ },
+ .probe = ptp_dte_probe,
+ .remove = ptp_dte_remove,
+};
+module_platform_driver(ptp_dte_driver);
+
+MODULE_AUTHOR("Broadcom");
+MODULE_DESCRIPTION("Broadcom DTE PTP Clock driver");
+MODULE_LICENSE("GPL v2");
--
1.9.1
^ permalink raw reply related
* [PATCH v1 1/2] dt-binding: ptp: add bindings document for dte based ptp clock
From: Arun Parameswaran @ 2017-06-12 20:26 UTC (permalink / raw)
To: Richard Cochran, Rob Herring, Mark Rutland
Cc: devicetree, linux-kernel, netdev, bcm-kernel-feedback-list,
Arun Parameswaran
In-Reply-To: <1497299161-6458-1-git-send-email-arun.parameswaran@broadcom.com>
Add device tree binding documentation for the Broadcom DTE
PTP clock driver.
Signed-off-by: Arun Parameswaran <arun.parameswaran@broadcom.com>
---
Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 +++++++++++++
1 file changed, 13 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt
diff --git a/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt
new file mode 100644
index 0000000..07590bc
--- /dev/null
+++ b/Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt
@@ -0,0 +1,13 @@
+* Broadcom Digital Timing Engine(DTE) based PTP clock driver
+
+Required properties:
+- compatible: should be "brcm,ptp-dte"
+- reg: address and length of the DTE block's NCO registers
+
+Example:
+
+ptp_dte: ptp_dte@180af650 {
+ compatible = "brcm,ptp-dte";
+ reg = <0x180af650 0x10>;
+ status = "okay";
+};
--
1.9.1
^ permalink raw reply related
* [PATCH v1 0/2] Add support for Broadcom DTE based PTP clock
From: Arun Parameswaran @ 2017-06-12 20:25 UTC (permalink / raw)
To: Richard Cochran, Rob Herring, Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w,
Arun Parameswaran
Hi,
This patchset adds support for the DTE based PTP clock for Broadcom SoCs.
The DTE nco based PTP clock can be used in both wired and wireless networks
for precision time-stmaping purposes.
Arun Parameswaran (2):
dt-binding: ptp: add bindings document for dte based ptp clock
ptp: Add a ptp clock driver for Broadcom DTE
.../devicetree/bindings/ptp/brcm,ptp-dte.txt | 13 +
drivers/ptp/Kconfig | 16 +
drivers/ptp/Makefile | 1 +
drivers/ptp/ptp_dte.c | 353 +++++++++++++++++++++
4 files changed, 383 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ptp/brcm,ptp-dte.txt
create mode 100644 drivers/ptp/ptp_dte.c
--
1.9.1
--
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
* EMAIL ALERT
From: HELP DESK @ 2017-06-12 19:55 UTC (permalink / raw)
To: netdev
Recently, we have detect some unusual activity on your account and as a result, all email users are urged to update their email account within 24 hours of receiving this e-mail, please click the link http://beam.to/1469 to confirm that your email account is up to date with the institution requirement.
---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus
^ permalink raw reply
* Re: [PATCH net-next v10 1/4] net netlink: Add new type NLA_FLAG_BITS
From: David Ahern @ 2017-06-12 19:58 UTC (permalink / raw)
To: Jiri Pirko
Cc: Jamal Hadi Salim, davem, netdev, xiyou.wangcong, eric.dumazet,
simon.horman, mrv
In-Reply-To: <20170612192238.GF1993@nanopsycho>
On 6/12/17 1:22 PM, Jiri Pirko wrote:
>
>> 3. IMO since these are nla prefixes and new NLA type they should be in
>> uapi/linux/netlink.h
> Including NLA_* type enum? I think it is reasonable.
well, maybe not the NLA_BITFIELD. That enum is for policy validation
kernel side so not really part of the API.
^ permalink raw reply
* Re: [PATCH net-next] ibmvnic: Client-initiated failover
From: Nathan Fontenot @ 2017-06-12 19:42 UTC (permalink / raw)
To: Thomas Falcon, netdev; +Cc: jallen
In-Reply-To: <1497288904-15259-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
On 06/12/2017 12:35 PM, Thomas Falcon wrote:
> The IBM vNIC protocol provides support for the user to initiate
> a failover from the client LPAR in case the current backing infrastructure
> is deemed inadequate or in an error state.
>
> Support for two H_VIOCTL sub-commands for vNIC devices are required
> to implement this function. These commands are H_GET_SESSION_TOKEN
> and H_SESSION_ERR_DETECTED.
>
> "[H_GET_SESSION_TOKEN] is used to obtain a session token from a VNIC client
> adapter. This token is opaque to the caller and is intended to be used in
> tandem with the SESSION_ERROR_DETECTED vioctl subfunction."
>
> "[H_SESSION_ERR_DETECTED] is used to report that the currently active
> backing device for a VNIC client adapter is behaving poorly, and that
> the hypervisor should attempt to fail over to a different backing device,
> if one is available."
>
> To provide tools access to this functionality the vNIC driver creates a
> sysfs file that, when written to, will send a request to pHyp to failover
> to a different backing device.
>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
Reviewed-by: Nathan Fontenot <nfont@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/hvcall.h | 2 ++
> drivers/net/ethernet/ibm/ibmvnic.c | 46 ++++++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
> index d73755f..57d38b5 100644
> --- a/arch/powerpc/include/asm/hvcall.h
> +++ b/arch/powerpc/include/asm/hvcall.h
> @@ -295,6 +295,8 @@
> #define H_DISABLE_ALL_VIO_INTS 0x0A
> #define H_DISABLE_VIO_INTERRUPT 0x0B
> #define H_ENABLE_VIO_INTERRUPT 0x0C
> +#define H_GET_SESSION_TOKEN 0x19
> +#define H_SESSION_ERR_DETECTED 0x1A
>
>
> /* Platform specific hcalls, used by KVM */
> diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
> index 7d84e20..fd3ef30 100644
> --- a/drivers/net/ethernet/ibm/ibmvnic.c
> +++ b/drivers/net/ethernet/ibm/ibmvnic.c
> @@ -3656,6 +3656,8 @@ static int ibmvnic_init(struct ibmvnic_adapter *adapter)
> return rc;
> }
>
> +static struct device_attribute dev_attr_failover;
> +
> static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
> {
> struct ibmvnic_adapter *adapter;
> @@ -3712,9 +3714,16 @@ static int ibmvnic_probe(struct vio_dev *dev, const struct vio_device_id *id)
>
> netdev->mtu = adapter->req_mtu - ETH_HLEN;
>
> + rc = device_create_file(&dev->dev, &dev_attr_failover);
> + if (rc) {
> + free_netdev(netdev);
> + return rc;
> + }
> +
> rc = register_netdev(netdev);
> if (rc) {
> dev_err(&dev->dev, "failed to register netdev rc=%d\n", rc);
> + device_remove_file(&dev->dev, &dev_attr_failover);
> free_netdev(netdev);
> return rc;
> }
> @@ -3740,12 +3749,49 @@ static int ibmvnic_remove(struct vio_dev *dev)
> adapter->state = VNIC_REMOVED;
>
> mutex_unlock(&adapter->reset_lock);
> + device_remove_file(&dev->dev, &dev_attr_failover);
> free_netdev(netdev);
> dev_set_drvdata(&dev->dev, NULL);
>
> return 0;
> }
>
> +static ssize_t failover_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct net_device *netdev = dev_get_drvdata(dev);
> + struct ibmvnic_adapter *adapter = netdev_priv(netdev);
> + unsigned long retbuf[PLPAR_HCALL_BUFSIZE];
> + __be64 session_token;
> + long rc;
> +
> + if (!sysfs_streq(buf, "1"))
> + return -EINVAL;
> +
> + rc = plpar_hcall(H_VIOCTL, retbuf, adapter->vdev->unit_address,
> + H_GET_SESSION_TOKEN, 0, 0, 0);
> + if (rc) {
> + netdev_err(netdev, "Couldn't retrieve session token, rc %ld\n",
> + rc);
> + return -EINVAL;
> + }
> +
> + session_token = (__be64)retbuf[0];
> + netdev_dbg(netdev, "Initiating client failover, session id %llx\n",
> + be64_to_cpu(session_token));
> + rc = plpar_hcall_norets(H_VIOCTL, adapter->vdev->unit_address,
> + H_SESSION_ERR_DETECTED, session_token, 0, 0);
> + if (rc) {
> + netdev_err(netdev, "Client initiated failover failed, rc %ld\n",
> + rc);
> + return -EINVAL;
> + }
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(failover, 0200, NULL, failover_store);
> +
> static unsigned long ibmvnic_get_desired_dma(struct vio_dev *vdev)
> {
> struct net_device *netdev = dev_get_drvdata(&vdev->dev);
>
^ permalink raw reply
* Re: [net v1 PATCH] hsr: fix incorrect warning
From: David Miller @ 2017-06-12 19:22 UTC (permalink / raw)
To: m-karicheri2; +Cc: arvid.brodin, netdev, linux-kernel
In-Reply-To: <1497294386-12829-1-git-send-email-m-karicheri2@ti.com>
From: Murali Karicheri <m-karicheri2@ti.com>
Date: Mon, 12 Jun 2017 15:06:26 -0400
> When HSR interface is setup using ip link command, an annoying warning
> appears with the trace as below:-
>
> [ 203.019828] hsr_get_node: Non-HSR frame
> [ 203.019833] Modules linked in:
> [ 203.019848] CPU: 0 PID: 158 Comm: sd-resolve Tainted: G W 4.12.0-rc3-00052-g9fa6bf70 #2
> [ 203.019853] Hardware name: Generic DRA74X (Flattened Device Tree)
> [ 203.019869] [<c0110280>] (unwind_backtrace) from [<c010c2f4>] (show_stack+0x10/0x14)
> [ 203.019880] [<c010c2f4>] (show_stack) from [<c04b9f64>] (dump_stack+0xac/0xe0)
> [ 203.019894] [<c04b9f64>] (dump_stack) from [<c01374e8>] (__warn+0xd8/0x104)
> [ 203.019907] [<c01374e8>] (__warn) from [<c0137548>] (warn_slowpath_fmt+0x34/0x44)
> root@am57xx-evm:~# [ 203.019921] [<c0137548>] (warn_slowpath_fmt) from [<c081126c>] (hsr_get_node+0x148/0x170)
> [ 203.019932] [<c081126c>] (hsr_get_node) from [<c0814240>] (hsr_forward_skb+0x110/0x7c0)
> [ 203.019942] [<c0814240>] (hsr_forward_skb) from [<c0811d64>] (hsr_dev_xmit+0x2c/0x34)
> [ 203.019954] [<c0811d64>] (hsr_dev_xmit) from [<c06c0828>] (dev_hard_start_xmit+0xc4/0x3bc)
> [ 203.019963] [<c06c0828>] (dev_hard_start_xmit) from [<c06c13d8>] (__dev_queue_xmit+0x7c4/0x98c)
> [ 203.019974] [<c06c13d8>] (__dev_queue_xmit) from [<c0782f54>] (ip6_finish_output2+0x330/0xc1c)
> [ 203.019983] [<c0782f54>] (ip6_finish_output2) from [<c0788f0c>] (ip6_output+0x58/0x454)
> [ 203.019994] [<c0788f0c>] (ip6_output) from [<c07b16cc>] (mld_sendpack+0x420/0x744)
>
> As this is an expected path to hsr_get_node() with frame coming from
> the master interface, add a check to ensure packet is not from the
> master port and then warn.
>
> Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v10 1/4] net netlink: Add new type NLA_FLAG_BITS
From: Jiri Pirko @ 2017-06-12 19:22 UTC (permalink / raw)
To: David Ahern
Cc: Jamal Hadi Salim, davem, netdev, xiyou.wangcong, eric.dumazet,
simon.horman, mrv
In-Reply-To: <80dd1bc2-80d6-d8fa-6032-d0641ae5d79b@gmail.com>
Mon, Jun 12, 2017 at 05:00:41PM CEST, dsahern@gmail.com wrote:
>On 6/12/17 8:14 AM, Jiri Pirko wrote:
>>>> The thing is, struct nla_flag_bits is tightly coupled with NLA_FLAG_BITS
>>>> enum value. They should be in the same uapi file. That makes sense to me.
>>>>
>>>
>>> Sure - they should be in the same file. But is it uapi/linux/netlink.h?
>>
>> Might be the netlink-types.h you mentioned above.
>>
>> ccing DavidA.
>>
>
>Just saw this patch set this morning. Few comments:
>
>1. I think nla_bitfield or nla_bitmap is a better name than nla_flag_bits
ack
>
>2. The length should be open ended with the size of the array determined
>by nla_len / sizeof(struct nla_bitfield). That allows this to be
>extended to an arbitrary large bitfield as needed.
Yeah, I was thinking about that as well. Seems handy to have this
generic len.
>
>3. IMO since these are nla prefixes and new NLA type they should be in
>uapi/linux/netlink.h
Including NLA_* type enum? I think it is reasonable.
^ permalink raw reply
* [net v1 PATCH] hsr: fix incorrect warning
From: Murali Karicheri @ 2017-06-12 19:06 UTC (permalink / raw)
To: davem, arvid.brodin, netdev, linux-kernel
When HSR interface is setup using ip link command, an annoying warning
appears with the trace as below:-
[ 203.019828] hsr_get_node: Non-HSR frame
[ 203.019833] Modules linked in:
[ 203.019848] CPU: 0 PID: 158 Comm: sd-resolve Tainted: G W 4.12.0-rc3-00052-g9fa6bf70 #2
[ 203.019853] Hardware name: Generic DRA74X (Flattened Device Tree)
[ 203.019869] [<c0110280>] (unwind_backtrace) from [<c010c2f4>] (show_stack+0x10/0x14)
[ 203.019880] [<c010c2f4>] (show_stack) from [<c04b9f64>] (dump_stack+0xac/0xe0)
[ 203.019894] [<c04b9f64>] (dump_stack) from [<c01374e8>] (__warn+0xd8/0x104)
[ 203.019907] [<c01374e8>] (__warn) from [<c0137548>] (warn_slowpath_fmt+0x34/0x44)
root@am57xx-evm:~# [ 203.019921] [<c0137548>] (warn_slowpath_fmt) from [<c081126c>] (hsr_get_node+0x148/0x170)
[ 203.019932] [<c081126c>] (hsr_get_node) from [<c0814240>] (hsr_forward_skb+0x110/0x7c0)
[ 203.019942] [<c0814240>] (hsr_forward_skb) from [<c0811d64>] (hsr_dev_xmit+0x2c/0x34)
[ 203.019954] [<c0811d64>] (hsr_dev_xmit) from [<c06c0828>] (dev_hard_start_xmit+0xc4/0x3bc)
[ 203.019963] [<c06c0828>] (dev_hard_start_xmit) from [<c06c13d8>] (__dev_queue_xmit+0x7c4/0x98c)
[ 203.019974] [<c06c13d8>] (__dev_queue_xmit) from [<c0782f54>] (ip6_finish_output2+0x330/0xc1c)
[ 203.019983] [<c0782f54>] (ip6_finish_output2) from [<c0788f0c>] (ip6_output+0x58/0x454)
[ 203.019994] [<c0788f0c>] (ip6_output) from [<c07b16cc>] (mld_sendpack+0x420/0x744)
As this is an expected path to hsr_get_node() with frame coming from
the master interface, add a check to ensure packet is not from the
master port and then warn.
Signed-off-by: Murali Karicheri <m-karicheri2@ti.com>
---
v1 - sending as a separate patch to apply to net as per comment from David
Miller.
net/hsr/hsr_forward.c | 3 +--
net/hsr/hsr_framereg.c | 9 +++++++--
net/hsr/hsr_framereg.h | 2 +-
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/net/hsr/hsr_forward.c b/net/hsr/hsr_forward.c
index 4ebe2aa..04b5450 100644
--- a/net/hsr/hsr_forward.c
+++ b/net/hsr/hsr_forward.c
@@ -324,8 +324,7 @@ static int hsr_fill_frame_info(struct hsr_frame_info *frame,
unsigned long irqflags;
frame->is_supervision = is_supervision_frame(port->hsr, skb);
- frame->node_src = hsr_get_node(&port->hsr->node_db, skb,
- frame->is_supervision);
+ frame->node_src = hsr_get_node(port, skb, frame->is_supervision);
if (frame->node_src == NULL)
return -1; /* Unknown node and !is_supervision, or no mem */
diff --git a/net/hsr/hsr_framereg.c b/net/hsr/hsr_framereg.c
index 7ea9258..284a9b8 100644
--- a/net/hsr/hsr_framereg.c
+++ b/net/hsr/hsr_framereg.c
@@ -158,9 +158,10 @@ struct hsr_node *hsr_add_node(struct list_head *node_db, unsigned char addr[],
/* Get the hsr_node from which 'skb' was sent.
*/
-struct hsr_node *hsr_get_node(struct list_head *node_db, struct sk_buff *skb,
+struct hsr_node *hsr_get_node(struct hsr_port *port, struct sk_buff *skb,
bool is_sup)
{
+ struct list_head *node_db = &port->hsr->node_db;
struct hsr_node *node;
struct ethhdr *ethhdr;
u16 seq_out;
@@ -186,7 +187,11 @@ struct hsr_node *hsr_get_node(struct list_head *node_db, struct sk_buff *skb,
*/
seq_out = hsr_get_skb_sequence_nr(skb) - 1;
} else {
- WARN_ONCE(1, "%s: Non-HSR frame\n", __func__);
+ /* this is called also for frames from master port and
+ * so warn only for non master ports
+ */
+ if (port->type != HSR_PT_MASTER)
+ WARN_ONCE(1, "%s: Non-HSR frame\n", __func__);
seq_out = HSR_SEQNR_START;
}
diff --git a/net/hsr/hsr_framereg.h b/net/hsr/hsr_framereg.h
index 438b40f..4e04f0e 100644
--- a/net/hsr/hsr_framereg.h
+++ b/net/hsr/hsr_framereg.h
@@ -18,7 +18,7 @@
struct hsr_node *hsr_add_node(struct list_head *node_db, unsigned char addr[],
u16 seq_out);
-struct hsr_node *hsr_get_node(struct list_head *node_db, struct sk_buff *skb,
+struct hsr_node *hsr_get_node(struct hsr_port *port, struct sk_buff *skb,
bool is_sup);
void hsr_handle_sup_frame(struct sk_buff *skb, struct hsr_node *node_curr,
struct hsr_port *port);
--
1.9.1
^ permalink raw reply related
* Re: Toggling link state breaks network connectivity
From: Mason @ 2017-06-12 18:58 UTC (permalink / raw)
To: Florian Fainelli, netdev; +Cc: Andrew Lunn, Mans Rullgard, Thibaud Cornic
In-Reply-To: <2c12bc7a-fd57-3d3d-7dc0-f522cfadd042@gmail.com>
Hello Florian,
On 12/06/2017 18:38, Florian Fainelli wrote:
> On 06/12/2017 06:22 AM, Mason wrote:
>
>> I am using the following drivers for Ethernet connectivity.
>> drivers/net/ethernet/aurora/nb8800.c
>> drivers/net/phy/at803x.c
>>
>> Pulling the cable and plugging it back works as expected.
>> (I can ping both before and after.)
>>
>> However, if I toggle the link state in software (using ip link set),
>> the board loses network connectivity.
>>
>> # Statically assign IP address
>> ip addr add 172.27.64.77/18 brd 172.27.127.255 dev eth0
>> # Set link state to "up"
>> ip link set eth0 up
>> # ping -c 3 172.27.64.1 > /tmp/v1
>>
>> PING 172.27.64.1 (172.27.64.1): 56 data bytes
>> 64 bytes from 172.27.64.1: seq=0 ttl=64 time=18.321 ms
>
> This delay seems abnormally long unless you are purposely introducing
> delay (e.g: with cls_netem) or this is a really remote host, does not
> seem to be based on your traces later on.
172.27.64.1 and 172.27.64.77 are connected to the
same switch. Purely local traffic. It seems to me
that the ARP request/reply could explain the delay.
Start op at 45.187346
Receive ICMP echo reply at 45.194662
Hmmm, that's only 7 ms
>> 172.27.64.1 is a desktop system.
>> Running
>> % tcpdump -n -i eth1-boards ether host 00:16:e8:4d:7f:c4
>> on the desktop, I get:
>>
>> 15:01:45.187346 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46
>> 15:01:45.187359 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28
>> 15:01:45.194633 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, seq 0, length 64
>> 15:01:45.194662 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, seq 0, length 64
>> 15:01:50.198564 ARP, Request who-has 172.27.64.77 tell 172.27.64.1, length 28
>> 15:01:50.205929 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, seq 1, length 64
>> 15:01:50.205951 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, seq 1, length 64
>> 15:01:50.213217 IP 172.27.64.77 > 172.27.64.1: ICMP echo request, id 41219, seq 2, length 64
>> 15:01:50.213232 IP 172.27.64.1 > 172.27.64.77: ICMP echo reply, id 41219, seq 2, length 64
>> 15:01:51.198563 ARP, Request who-has 172.27.64.77 tell 172.27.64.1, length 28
>> 15:01:51.209586 ARP, Reply 172.27.64.77 is-at 00:16:e8:4d:7f:c4, length 46
>> 15:01:51.209598 ARP, Reply 172.27.64.77 is-at 00:16:e8:4d:7f:c4, length 46
>>
>> Packet #1: the board asks for the desktop's MAC address
>> Packet #2: the desktop replies instantly
>> Packet #3: the board sends the first ping
>> Packet #4: the desktop replies instantly
>> Then the board goes quiet for a long time (why???)
>> Packet #5: the desktop asks for the board's MAC address (doesn't it have it already?)
>> Packet #6: this seems to unwedge the board, which sends the second ping
>> Packet #7: the desktop replies instantly
>> Packet #8: the board sends the third ping
>> Packet #9: the desktop replies instantly
>> Packet #10: the desktop asks again for the board's MAC address
>> Packet #11 and #12: the board answers twice (for the old and new requests?)
>>
>> Some oddities, but it seems to work.
>>
>> Now toggle the link state:
>>
>> % ip link set eth0 down
>> % ip link set eth0 up
>> % ping -c 3 172.27.64.1 > /tmp/v2
>>
>> PING 172.27.64.1 (172.27.64.1): 56 data bytes
>>
>> --- 172.27.64.1 ping statistics ---
>> 3 packets transmitted, 0 packets received, 100% packet loss
>>
>>
>> On the desktop, I see
>>
>> 15:14:03.900162 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46
>> 15:14:03.900175 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28
>> 15:14:05.017189 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46
>> 15:14:05.017200 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28
>> 15:14:06.030531 ARP, Request who-has 172.27.64.1 tell 172.27.64.77, length 46
>> 15:14:06.030541 ARP, Reply 172.27.64.1 is-at 00:15:17:24:e0:81, length 28
>>
>> So basically, the board is asking the desktop for its MAC address,
>> and the desktop is answering immediately. But the board doesn't seem
>> to be getting the replies... Any ideas, or words of wisdom, as they say?
>
> - check the Ethernet MAC counters to see if there is packet loss, or
> error, or both
I'll take a look, but I don't expect any packet loss
(LAN traffic on an idle switch).
> - consult with your HW engineers for possible flaws in your
> ndo_open/ndo_close paths and possible interactions with the MAC/PHY
> clocks, or reset etc.
(The HW engineers have no knowledge of Linux use-cases.)
The crazy thing is that I can use the same driver on the
previous chip, and I don't see this behavior... Will
retest tomorrow to be sure. What does change between
the two chips are a few clock frequencies though.
So maybe some race is now consistently lost on the
new chip...
> - see if your PHY needs a complete re-init after an up/down sequence and
> if you are doing this properly
Thanks for these suggestions. I'll take a closer look
tomorrow.
Regards.
^ permalink raw reply
* Re: [Patch net] igmp: acquire pmc lock for ip_mc_clear_src()
From: Cong Wang @ 2017-06-12 18:35 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, Andrey Konovalov, Eric Dumazet
In-Reply-To: <CADvbK_f=NePnrAvARnOMvg_o8G2bj-p1i1e_CXMZ6BooW3yhHA@mail.gmail.com>
On Mon, Jun 12, 2017 at 11:30 AM, Xin Long <lucien.xin@gmail.com> wrote:
> Hi, Cong.
>
> how about in ip_check_mc_rcu():
> for (psf = im->sources; psf; psf = psf->sf_next) {
> if (psf->sf_inaddr == src_addr)
> break;
> }
>
> I didn't see spinlock for it, is it safe to access them in parallel ?
> or these two places would never be in parallel ?
That is a different bug which needs more work, therefore
I defer it to net-next. And I already explained to you why
it needs more work than just a call_rcu().
^ permalink raw reply
* Bluetooth: might sleep error in hidp_session_thread
From: Rohit Vaswani @ 2017-06-12 18:31 UTC (permalink / raw)
To: Jeffy Chen, linux-bluetooth@vger.kernel.org
Cc: Brian Norris, Douglas Anderson, Johan Hedberg, Peter Hurley,
Jeffy Chen, Johan Hedberg, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, David S. Miller, Marcel Holtmann,
Gustavo Padovan
Hi Jeffy,
I was looking into the patch from Jeffy Chen from February 14 2017 :
[v4,3/3] Bluetooth: hidp: fix possible might sleep error in hidp_session_thread: https://patchwork.kernel.org/patch/9570931/
We faced a similar issue and this patch seems to fix the problem in our preliminary test.
I am trying to check if there was a reason this wasn't merged earlier ?
Thanks,
Rohit
nvpublic
^ permalink raw reply
* Re: [Patch net] igmp: acquire pmc lock for ip_mc_clear_src()
From: Xin Long @ 2017-06-12 18:30 UTC (permalink / raw)
To: Cong Wang; +Cc: network dev, Andrey Konovalov, Eric Dumazet
In-Reply-To: <1497286346-26888-1-git-send-email-xiyou.wangcong@gmail.com>
On Tue, Jun 13, 2017 at 12:52 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> Andrey reported a use-after-free in add_grec():
>
> for (psf = *psf_list; psf; psf = psf_next) {
> ...
> psf_next = psf->sf_next;
>
> where the struct ip_sf_list's were already freed by:
>
> kfree+0xe8/0x2b0 mm/slub.c:3882
> ip_mc_clear_src+0x69/0x1c0 net/ipv4/igmp.c:2078
> ip_mc_dec_group+0x19a/0x470 net/ipv4/igmp.c:1618
> ip_mc_drop_socket+0x145/0x230 net/ipv4/igmp.c:2609
> inet_release+0x4e/0x1c0 net/ipv4/af_inet.c:411
> sock_release+0x8d/0x1e0 net/socket.c:597
> sock_close+0x16/0x20 net/socket.c:1072
>
> This happens because we don't hold pmc->lock in ip_mc_clear_src()
> and a parallel mr_ifc_timer timer could jump in and access them.
>
> The RCU lock is there but it is merely for pmc itself, this
> spinlock could actually ensure we don't access them in parallel.
>
> Thanks to Eric and Long for discussion on this bug.
>
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
> net/ipv4/igmp.c | 21 +++++++++++++--------
> 1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 44fd86d..8f6b5bb 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -2071,21 +2071,26 @@ static int ip_mc_add_src(struct in_device *in_dev, __be32 *pmca, int sfmode,
>
> static void ip_mc_clear_src(struct ip_mc_list *pmc)
> {
> - struct ip_sf_list *psf, *nextpsf;
> + struct ip_sf_list *psf, *nextpsf, *tomb, *sources;
>
> - for (psf = pmc->tomb; psf; psf = nextpsf) {
> + spin_lock_bh(&pmc->lock);
> + tomb = pmc->tomb;
> + pmc->tomb = NULL;
> + sources = pmc->sources;
> + pmc->sources = NULL;
> + pmc->sfmode = MCAST_EXCLUDE;
> + pmc->sfcount[MCAST_INCLUDE] = 0;
> + pmc->sfcount[MCAST_EXCLUDE] = 1;
> + spin_unlock_bh(&pmc->lock);
> +
> + for (psf = tomb; psf; psf = nextpsf) {
> nextpsf = psf->sf_next;
> kfree(psf);
> }
> - pmc->tomb = NULL;
> - for (psf = pmc->sources; psf; psf = nextpsf) {
> + for (psf = sources; psf; psf = nextpsf) {
> nextpsf = psf->sf_next;
> kfree(psf);
> }
Hi, Cong.
how about in ip_check_mc_rcu():
for (psf = im->sources; psf; psf = psf->sf_next) {
if (psf->sf_inaddr == src_addr)
break;
}
I didn't see spinlock for it, is it safe to access them in parallel ?
or these two places would never be in parallel ?
I've already checked elsewhere, all other places where it accesses
or traverses im->sources are protected by this spinlock.
> - pmc->sources = NULL;
> - pmc->sfmode = MCAST_EXCLUDE;
> - pmc->sfcount[MCAST_INCLUDE] = 0;
> - pmc->sfcount[MCAST_EXCLUDE] = 1;
> }
>
> /* Join a multicast group
> --
> 2.5.5
>
^ permalink raw reply
* Re: [PATCH 1/2] hsr: fix coding style issues
From: Murali Karicheri @ 2017-06-12 18:21 UTC (permalink / raw)
To: David Miller; +Cc: arvid.brodin, netdev, linux-kernel
In-Reply-To: <20170606.160937.2113672006487973562.davem@davemloft.net>
On 06/06/2017 04:09 PM, David Miller wrote:
>
> Please do not mix cleanups with legitimate bug fixes. Also, when posting
> a multi-patch series, you must always provide an appropriate "[PATCH 0/N]"
> header posting that describes what you series is doing at a high level,
> how it is doing it, and why it is doing it that way.
>
> For this, submit the erroneous warning removal against 'net' as a single
> patch. And then once that propagates into the 'net-next' tree you can
> submit the coding style cleanups against 'net-next', thanks.
>
Thanks. Will do
--
Murali Karicheri
Linux Kernel, Keystone
^ permalink raw reply
* Re: [PATCH RFC net-next 4/4] net/mlx5: Add CONFIG_MLX5_ESWITCH Kconfig
From: Jes Sorensen @ 2017-06-12 18:20 UTC (permalink / raw)
To: Saeed Mahameed, netdev; +Cc: kernel-team, Or Gerlitz, Tzahi Oved
In-Reply-To: <20170607234214.24723-5-saeedm@mellanox.com>
On 06/07/2017 07:42 PM, Saeed Mahameed wrote:
> This patch gives the option to chose whether to compile the driver with or
> without eswitch/eswitch_offloads(switchdev mode)/en_rep(VF representors)
> and en_tc offloads.
>
> It also removes most of the above modules headers declarations and stubs
> out the API functions which are used outside these modules.
>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/Kconfig | 7 +++++
> drivers/net/ethernet/mellanox/mlx5/core/Makefile | 6 +++--
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 33 +++++++++++++++--------
> drivers/net/ethernet/mellanox/mlx5/core/en_rep.h | 8 ++++++
> drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 2 ++
> drivers/net/ethernet/mellanox/mlx5/core/en_tc.h | 7 +++++
> drivers/net/ethernet/mellanox/mlx5/core/eswitch.h | 23 +++++++++++-----
> drivers/net/ethernet/mellanox/mlx5/core/main.c | 10 +------
> 8 files changed, 68 insertions(+), 28 deletions(-)
Overall good, a few nits
> @@ -3316,6 +3317,7 @@ static int mlx5e_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
> }
> }
>
> +#ifdef CONFIG_MLX5_ESWITCH
> static int mlx5e_set_vf_mac(struct net_device *dev, int vf, u8 *mac)
> {
> struct mlx5e_priv *priv = netdev_priv(dev);
> @@ -3418,6 +3420,7 @@ static int mlx5e_get_vf_stats(struct net_device *dev,
> return mlx5_eswitch_get_vport_stats(mdev->priv.eswitch, vf + 1,
> vf_stats);
> }
> +#endif
>
> static void mlx5e_add_vxlan_port(struct net_device *netdev,
> struct udp_tunnel_info *ti)
> @@ -3659,6 +3662,7 @@ static const struct net_device_ops mlx5e_netdev_ops_basic = {
> #endif
> };
>
> +#ifdef CONFIG_MLX5_ESWITCH
> static const struct net_device_ops mlx5e_netdev_ops_sriov = {
> .ndo_open = mlx5e_open,
> .ndo_stop = mlx5e_close,
> @@ -3697,6 +3701,7 @@ static const struct net_device_ops mlx5e_netdev_ops_sriov = {
> .ndo_has_offload_stats = mlx5e_has_offload_stats,
> .ndo_get_offload_stats = mlx5e_get_offload_stats,
> };
> +#endif
>
> static int mlx5e_check_required_hca_cap(struct mlx5_core_dev *mdev)
> {
> @@ -3923,9 +3928,11 @@ static void mlx5e_set_netdev_dev_addr(struct net_device *netdev)
> }
> }
>
> +#if IS_ENABLED(CONFIG_NET_SWITCHDEV) && IS_ENABLED(CONFIG_MLX5_ESWITCH)
> static const struct switchdev_ops mlx5e_switchdev_ops = {
> .switchdev_port_attr_get = mlx5e_attr_get,
> };
> +#endif
>
> static void mlx5e_build_nic_netdev(struct net_device *netdev)
> {
Why not move these functions and the struct into one of the files that
is being compiled out. The less #ifdefs we leave in the code the better.
> @@ -3936,15 +3943,17 @@ static void mlx5e_build_nic_netdev(struct net_device *netdev)
>
> SET_NETDEV_DEV(netdev, &mdev->pdev->dev);
>
> - if (MLX5_CAP_GEN(mdev, vport_group_manager)) {
> - netdev->netdev_ops = &mlx5e_netdev_ops_sriov;
> #ifdef CONFIG_MLX5_CORE_EN_DCB
> - if (MLX5_CAP_GEN(mdev, qos))
> - netdev->dcbnl_ops = &mlx5e_dcbnl_ops;
> + if (MLX5_CAP_GEN(mdev, vport_group_manager) && MLX5_CAP_GEN(mdev, qos))
> + netdev->dcbnl_ops = &mlx5e_dcbnl_ops;
> +#endif
> +
> +#ifdef CONFIG_MLX5_ESWITCH
> + if (MLX5_CAP_GEN(mdev, vport_group_manager))
> + netdev->netdev_ops = &mlx5e_netdev_ops_sriov;
> + else
> #endif
> - } else {
> netdev->netdev_ops = &mlx5e_netdev_ops_basic;
> - }
>
> netdev->watchdog_timeo = 15 * HZ;
This kind of #ifdef is always bad, it's hard to read and easy to get
wrong. Why not have MLX5_CAP_GEN return 0 if MLX5_ESWITCH is not enabled
and have a dummy pointer?
> @@ -4016,7 +4025,7 @@ static void mlx5e_build_nic_netdev(struct net_device *netdev)
>
> mlx5e_set_netdev_dev_addr(netdev);
>
> -#ifdef CONFIG_NET_SWITCHDEV
> +#if IS_ENABLED(CONFIG_NET_SWITCHDEV) && IS_ENABLED(CONFIG_MLX5_ESWITCH)
> if (MLX5_CAP_GEN(mdev, vport_group_manager))
> netdev->switchdev_ops = &mlx5e_switchdev_ops;
> #endif
Can MLX5_ESWITCH be enabled without NET_SWITCHDEV?
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index 66b5fec15313..7d2860252dce 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -806,6 +806,7 @@ void mlx5e_handle_rx_cqe(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
> &wqe->next.next_wqe_index);
> }
>
> +#ifdef CONFIG_MLX5_ESWITCH
> void mlx5e_handle_rx_cqe_rep(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
> {
> struct net_device *netdev = rq->netdev;
> @@ -838,6 +839,7 @@ void mlx5e_handle_rx_cqe_rep(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
> mlx5_wq_ll_pop(&rq->wq, wqe_counter_be,
> &wqe->next.next_wqe_index);
> }
> +#endif
>
> static inline void mlx5e_mpwqe_fill_rx_skb(struct mlx5e_rq *rq,
> struct mlx5_cqe64 *cqe,
Another case of moving it to one of the disabled files.
Cheers,
Jes
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox