* Re: [RESEND PATCH net] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: Jann Horn @ 2019-02-14 21:26 UTC (permalink / raw)
To: David Miller, Alexander Duyck; +Cc: Network Development, kernel list
In-Reply-To: <20190214.091328.1687361207100252890.davem@davemloft.net>
On Thu, Feb 14, 2019 at 6:13 PM David Miller <davem@davemloft.net> wrote:
>
> From: Jann Horn <jannh@google.com>
> Date: Wed, 13 Feb 2019 22:45:59 +0100
>
> > The basic idea behind ->pagecnt_bias is: If we pre-allocate the maximum
> > number of references that we might need to create in the fastpath later,
> > the bump-allocation fastpath only has to modify the non-atomic bias value
> > that tracks the number of extra references we hold instead of the atomic
> > refcount. The maximum number of allocations we can serve (under the
> > assumption that no allocation is made with size 0) is nc->size, so that's
> > the bias used.
> >
> > However, even when all memory in the allocation has been given away, a
> > reference to the page is still held; and in the `offset < 0` slowpath, the
> > page may be reused if everyone else has dropped their references.
> > This means that the necessary number of references is actually
> > `nc->size+1`.
> >
> > Luckily, from a quick grep, it looks like the only path that can call
> > page_frag_alloc(fragsz=1) is TAP with the IFF_NAPI_FRAGS flag, which
> > requires CAP_NET_ADMIN in the init namespace and is only intended to be
> > used for kernel testing and fuzzing.
> >
> > To test for this issue, put a `WARN_ON(page_ref_count(page) == 0)` in the
> > `offset < 0` path, below the virt_to_page() call, and then repeatedly call
> > writev() on a TAP device with IFF_TAP|IFF_NO_PI|IFF_NAPI_FRAGS|IFF_NAPI,
> > with a vector consisting of 15 elements containing 1 byte each.
> >
> > Signed-off-by: Jann Horn <jannh@google.com>
>
> Applied and queued up for -stable.
I had sent a v2 at Alexander Duyck's request an hour before you
applied the patch (with a minor difference that, in Alexander's
opinion, might be slightly more efficient). I guess the net tree
doesn't work like the mm tree, where patches can get removed and
replaced with newer versions? So if Alexander wants that change
(s/size/PAGE_FRAG_CACHE_MAX_SIZE/ in the refcount), someone has to
send that as a separate patch?
^ permalink raw reply
* [PATCH net-next 2/2] net: phy: use phy_resolve_aneg_linkmode in genphy_read_status
From: Heiner Kallweit @ 2019-02-14 21:16 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <cc50884b-6271-6b93-605c-f451abeb0d7e@gmail.com>
Now that we have phy_resolve_aneg_linkmode() we can make
genphy_read_status() much simpler.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy_device.c | 24 +-----------------------
1 file changed, 1 insertion(+), 23 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2c61282a2..d7ccbd94f 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1747,8 +1747,6 @@ int genphy_read_status(struct phy_device *phydev)
int err;
int lpa;
int lpagb = 0;
- int common_adv;
- int common_adv_gb = 0;
/* Update the link, but return if there was an error */
err = genphy_update_link(phydev);
@@ -1780,7 +1778,6 @@ int genphy_read_status(struct phy_device *phydev)
mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising,
lpagb);
- common_adv_gb = lpagb & adv << 2;
}
lpa = phy_read(phydev, MII_LPA);
@@ -1793,31 +1790,12 @@ int genphy_read_status(struct phy_device *phydev)
if (adv < 0)
return adv;
- common_adv = lpa & adv;
-
phydev->speed = SPEED_10;
phydev->duplex = DUPLEX_HALF;
phydev->pause = 0;
phydev->asym_pause = 0;
- if (common_adv_gb & (LPA_1000FULL | LPA_1000HALF)) {
- phydev->speed = SPEED_1000;
-
- if (common_adv_gb & LPA_1000FULL)
- phydev->duplex = DUPLEX_FULL;
- } else if (common_adv & (LPA_100FULL | LPA_100HALF)) {
- phydev->speed = SPEED_100;
-
- if (common_adv & LPA_100FULL)
- phydev->duplex = DUPLEX_FULL;
- } else
- if (common_adv & LPA_10FULL)
- phydev->duplex = DUPLEX_FULL;
-
- if (phydev->duplex == DUPLEX_FULL) {
- phydev->pause = lpa & LPA_PAUSE_CAP ? 1 : 0;
- phydev->asym_pause = lpa & LPA_PAUSE_ASYM ? 1 : 0;
- }
+ phy_resolve_aneg_linkmode(phydev);
} else {
int bmcr = phy_read(phydev, MII_BMCR);
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 1/2] net: phy: improve phy_resolve_aneg_linkmode
From: Heiner Kallweit @ 2019-02-14 21:15 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <cc50884b-6271-6b93-605c-f451abeb0d7e@gmail.com>
We have the settings array of modes which is sorted based on aneg
priority. Instead of checking each mode manually let's simply iterate
over the sorted settings.
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/phy-core.c | 43 +++++++-------------------------------
1 file changed, 7 insertions(+), 36 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index cdea028d1..5d43106fe 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -349,45 +349,16 @@ size_t phy_speeds(unsigned int *speeds, size_t size,
void phy_resolve_aneg_linkmode(struct phy_device *phydev)
{
__ETHTOOL_DECLARE_LINK_MODE_MASK(common);
+ int i;
linkmode_and(common, phydev->lp_advertising, phydev->advertising);
- if (linkmode_test_bit(ETHTOOL_LINK_MODE_10000baseT_Full_BIT, common)) {
- phydev->speed = SPEED_10000;
- phydev->duplex = DUPLEX_FULL;
- } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_5000baseT_Full_BIT,
- common)) {
- phydev->speed = SPEED_5000;
- phydev->duplex = DUPLEX_FULL;
- } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
- common)) {
- phydev->speed = SPEED_2500;
- phydev->duplex = DUPLEX_FULL;
- } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Full_BIT,
- common)) {
- phydev->speed = SPEED_1000;
- phydev->duplex = DUPLEX_FULL;
- } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseT_Half_BIT,
- common)) {
- phydev->speed = SPEED_1000;
- phydev->duplex = DUPLEX_HALF;
- } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Full_BIT,
- common)) {
- phydev->speed = SPEED_100;
- phydev->duplex = DUPLEX_FULL;
- } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT,
- common)) {
- phydev->speed = SPEED_100;
- phydev->duplex = DUPLEX_HALF;
- } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Full_BIT,
- common)) {
- phydev->speed = SPEED_10;
- phydev->duplex = DUPLEX_FULL;
- } else if (linkmode_test_bit(ETHTOOL_LINK_MODE_10baseT_Half_BIT,
- common)) {
- phydev->speed = SPEED_10;
- phydev->duplex = DUPLEX_HALF;
- }
+ for (i = 0; i < ARRAY_SIZE(settings); i++)
+ if (test_bit(settings[i].bit, common)) {
+ phydev->speed = settings[i].speed;
+ phydev->duplex = settings[i].duplex;
+ break;
+ }
if (phydev->duplex == DUPLEX_FULL) {
phydev->pause = linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
--
2.20.1
^ permalink raw reply related
* [PATCH net-next 0/2] net: phy: improve and use phy_resolve_aneg_linkmode
From: Heiner Kallweit @ 2019-02-14 21:12 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org
Improve phy_resolve_aneg_linkmode and use it in genphy_read_status.
Heiner Kallweit (2):
net: phy: improve phy_resolve_aneg_linkmode
net: phy: use phy_resolve_aneg_linkmode in genphy_read_status
drivers/net/phy/phy-core.c | 43 ++++++------------------------------
drivers/net/phy/phy_device.c | 24 +-------------------
2 files changed, 8 insertions(+), 59 deletions(-)
--
2.20.1
^ permalink raw reply
* Have needs for?
From: Cindy @ 2019-02-11 8:59 UTC (permalink / raw)
To: netdev
Do you have needs for retouching your photos? Or do deep etching and
masking for your photos,
We are the image service provider who can do this for you.
Please send photos to start testing, then you cam judge the quality of our
service.
Thanks,
Cindy
Fuldad
Haldberstadt
^ permalink raw reply
* Re: [PATCH net-next 04/12] net: sched: flower: track filter deletion with flag
From: Stefano Brivio @ 2019-02-14 20:49 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem
In-Reply-To: <20190214074712.17846-5-vladbu@mellanox.com>
On Thu, 14 Feb 2019 09:47:04 +0200
Vlad Buslov <vladbu@mellanox.com> wrote:
> +static int __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
> + bool *last, struct netlink_ext_ack *extack)
This would be easier to follow (at least for me):
> {
> struct cls_fl_head *head = fl_head_dereference(tp);
> bool async = tcf_exts_get_net(&f->exts);
> - bool last;
> -
> - idr_remove(&head->handle_idr, f->handle);
> - list_del_rcu(&f->list);
> - last = fl_mask_put(head, f->mask, async);
> - if (!tc_skip_hw(f->flags))
> - fl_hw_destroy_filter(tp, f, extack);
> - tcf_unbind_filter(tp, &f->res);
> - __fl_put(f);
> + int err = 0;
without this
> +
> + (*last) = false;
with *last = false;
> +
> + if (!f->deleted) {
with:
if (f->deleted)
return -ENOENT;
> + f->deleted = true;
> + rhashtable_remove_fast(&f->mask->ht, &f->ht_node,
> + f->mask->filter_ht_params);
> + idr_remove(&head->handle_idr, f->handle);
> + list_del_rcu(&f->list);
> + (*last) = fl_mask_put(head, f->mask, async);
with:
*last = fl_mask_put(head, f->mask, async);
> + if (!tc_skip_hw(f->flags))
> + fl_hw_destroy_filter(tp, f, extack);
> + tcf_unbind_filter(tp, &f->res);
> + __fl_put(f);
and a return 0; here
> + } else {
> + err = -ENOENT;
> + }
>
> - return last;
> + return err;
> }
>
> [...]
>
> @@ -1520,14 +1541,14 @@ static int fl_delete(struct tcf_proto *tp, void *arg, bool *last,
> {
> struct cls_fl_head *head = fl_head_dereference(tp);
> struct cls_fl_filter *f = arg;
> + bool last_on_mask;
This is unused in this series, maybe change __fl_delete() to optionally
take NULL as 'bool *last' argument?
> + int err = 0;
Nit: no need to initialise this.
> - rhashtable_remove_fast(&f->mask->ht, &f->ht_node,
> - f->mask->filter_ht_params);
> - __fl_delete(tp, f, extack);
> + err = __fl_delete(tp, f, &last_on_mask, extack);
> *last = list_empty(&head->masks);
> __fl_put(f);
>
> - return 0;
> + return err;
> }
>
> static void fl_walk(struct tcf_proto *tp, struct tcf_walker *arg,
--
Stefano
^ permalink raw reply
* Have needs for?
From: Cindy @ 2019-02-11 11:42 UTC (permalink / raw)
To: netdev
Do you have needs for retouching your photos? Or do deep etching and
masking for your photos,
We are the image service provider who can do this for you.
Please send photos to start testing, then you cam judge the quality of our
service.
Thanks,
Cindy
Bottrodp
Haldberstadt
^ permalink raw reply
* [PATCHv4 2/6] Documentation: dt: socfpga: Add S10 System Manager binding
From: thor.thayer @ 2019-02-14 20:44 UTC (permalink / raw)
To: lee.jones, arnd, dinguyen, linux, catalin.marinas, will.deacon,
peppe.cavallaro, alexandre.torgue, joabreu, olof
Cc: davem, mcoquelin.stm32, mchehab+samsung, mark.rutland,
bjorn.andersson, devicetree, linux-kernel, linux-arm-kernel,
netdev, Thor Thayer
In-Reply-To: <1550177058-1860-1-git-send-email-thor.thayer@linux.intel.com>
From: Thor Thayer <thor.thayer@linux.intel.com>
Add the device tree bindings for the Stratix10 System Manager.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
v2 New compatible string and usage for Stratix10
v3 No change
v4 Add Reviewed-by from v2.
---
.../devicetree/bindings/arm/altera/socfpga-system.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt b/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt
index f4d04a067282..82edbaaa3f85 100644
--- a/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt
+++ b/Documentation/devicetree/bindings/arm/altera/socfpga-system.txt
@@ -11,3 +11,15 @@ Example:
reg = <0xffd08000 0x1000>;
cpu1-start-addr = <0xffd080c4>;
};
+
+ARM64 - Stratix10
+Required properties:
+- compatible : "altr,sys-mgr-s10"
+- reg : Should contain 1 register range(address and length)
+ for system manager register.
+
+Example:
+ sysmgr@ffd12000 {
+ compatible = "altr,sys-mgr-s10";
+ reg = <0xffd12000 0x228>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCHv4 1/6] mfd: altera-sysmgr: Add SOCFPGA System Manager
From: thor.thayer @ 2019-02-14 20:44 UTC (permalink / raw)
To: lee.jones, arnd, dinguyen, linux, catalin.marinas, will.deacon,
peppe.cavallaro, alexandre.torgue, joabreu, olof
Cc: davem, mcoquelin.stm32, mchehab+samsung, mark.rutland,
bjorn.andersson, devicetree, linux-kernel, linux-arm-kernel,
netdev, Thor Thayer
In-Reply-To: <1550177058-1860-1-git-send-email-thor.thayer@linux.intel.com>
From: Thor Thayer <thor.thayer@linux.intel.com>
The SOCFPGA System Manager register block aggregates different
peripheral functions into one area.
On 32 bit ARM parts, handle in the same way as syscon.
On 64 bit ARM parts, the System Manager can only be accessed by
EL3 secure mode. Since a SMC call to EL3 is required, this new
driver uses regmaps similar to syscon to handle the SMC call.
Since regmaps abstract out the underlying register access, the
changes to drivers accessing the System Manager are minimal.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
v2 Implement Arnd's changes.
1. Change socfpga_is_s10() to check compatible string.
Add new compatible string for Stratix10 in bindings
and add proper detection method.
2. Replace base cast with resource_size_t member.
3. Change s10_sysmgr_regmap_cfg to altr_sysmgr_regmap_cfg to
be generic.
4. Always use 4 byte width.
5. Initialize the .reg_read and .reg_write in S10 case only.
6. Remove call to syscon in 32bit ARM case and handle both
ARM32 and ARM64 in of_sysmgr_register().
7. Replace IS_ERR_OR_NULL() with IS_ERR().
8. Remove compatible check functions except phandle function.
v3 Implement 2nd set of Arnd's changes.
1. Use probe to register and create the regmap.
2. Remove global pointer and use traditional probe() method
of saving altr_sysmgr in private device data.
3. Lookup function using phandle finds altr_sysmgr and
returns its regmap.
4. Fix copyright dates.
5. Remove socfpga_is_s10() function since only used 1 time.
6. Remove unused function prototypes from header file.
7. Remove the SMC defines from header file and use the
defines from the recently accepted Intel Service Layer
header (stratix10-smc.h).
v4 Add of_put() required by of_parse_phandle().
---
MAINTAINERS | 6 ++
drivers/mfd/Kconfig | 10 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/altera-sysmgr.c | 211 ++++++++++++++++++++++++++++++++++++++
include/linux/mfd/altera-sysmgr.h | 29 ++++++
5 files changed, 257 insertions(+)
create mode 100644 drivers/mfd/altera-sysmgr.c
create mode 100644 include/linux/mfd/altera-sysmgr.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 4d04cebb4a71..0d2ccb710213 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -708,6 +708,12 @@ L: linux-gpio@vger.kernel.org
S: Maintained
F: drivers/gpio/gpio-altera.c
+ALTERA SYSTEM MANAGER DRIVER
+M: Thor Thayer <thor.thayer@linux.intel.com>
+S: Maintained
+F: drivers/mfd/altera-sysmgr.c
+F: include/linux/mfd/altera-sysgmr.h
+
ALTERA SYSTEM RESOURCE DRIVER FOR ARRIA10 DEVKIT
M: Thor Thayer <thor.thayer@linux.intel.com>
S: Maintained
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index f461460a2aeb..8629cf13520e 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -29,6 +29,16 @@ config MFD_ALTERA_A10SR
accessing the external gpio extender (LEDs & buttons) and
power supply alarms (hwmon).
+config MFD_ALTERA_SYSMGR
+ bool "Altera SOCFPGA System Manager"
+ depends on (ARCH_SOCFPGA || ARCH_STRATIX10) && OF
+ select MFD_SYSCON
+ help
+ Select this to get System Manager support for all Altera branded
+ SOCFPGAs. The SOCFPGA System Manager handles all SOCFPGAs by
+ using regmap_mmio accesses for ARM32 parts and SMC calls to
+ EL3 for ARM64 parts.
+
config MFD_ACT8945A
tristate "Active-semi ACT8945A"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 12980a4ad460..c649f6efed5f 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -233,6 +233,7 @@ obj-$(CONFIG_INTEL_SOC_PMIC_CHTDC_TI) += intel_soc_pmic_chtdc_ti.o
obj-$(CONFIG_MFD_MT6397) += mt6397-core.o
obj-$(CONFIG_MFD_ALTERA_A10SR) += altera-a10sr.o
+obj-$(CONFIG_MFD_ALTERA_SYSMGR) += altera-sysmgr.o
obj-$(CONFIG_MFD_SUN4I_GPADC) += sun4i-gpadc.o
obj-$(CONFIG_MFD_STM32_LPTIMER) += stm32-lptimer.o
diff --git a/drivers/mfd/altera-sysmgr.c b/drivers/mfd/altera-sysmgr.c
new file mode 100644
index 000000000000..8976f82785bb
--- /dev/null
+++ b/drivers/mfd/altera-sysmgr.c
@@ -0,0 +1,211 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2018-2019, Intel Corporation.
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ * Copyright (C) 2012 Linaro Ltd.
+ *
+ * Based on syscon driver.
+ */
+
+#include <linux/arm-smccc.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mfd/altera-sysmgr.h>
+#include <linux/mfd/syscon.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/regmap.h>
+#include <linux/slab.h>
+
+/**
+ * struct altr_sysmgr - Altera SOCFPGA System Manager
+ * @regmap: the regmap used for System Manager accesses.
+ * @base : the base address for the System Manager
+ */
+struct altr_sysmgr {
+ struct regmap *regmap;
+ resource_size_t *base;
+};
+
+static struct platform_driver altr_sysmgr_driver;
+
+/**
+ * s10_protected_reg_write
+ * Write to a protected SMC register.
+ * @base: Base address of System Manager
+ * @reg: Address offset of register
+ * @val: Value to write
+ * Return: INTEL_SIP_SMC_STATUS_OK (0) on success
+ * INTEL_SIP_SMC_REG_ERROR on error
+ * INTEL_SIP_SMC_RETURN_UNKNOWN_FUNCTION if not supported
+ */
+static int s10_protected_reg_write(void *base,
+ unsigned int reg, unsigned int val)
+{
+ struct arm_smccc_res result;
+ unsigned long sysmgr_base = (unsigned long)base;
+
+ arm_smccc_smc(INTEL_SIP_SMC_REG_WRITE, sysmgr_base + reg,
+ val, 0, 0, 0, 0, 0, &result);
+
+ return (int)result.a0;
+}
+
+/**
+ * s10_protected_reg_read
+ * Read the status of a protected SMC register
+ * @base: Base address of System Manager.
+ * @reg: Address of register
+ * @val: Value read.
+ * Return: INTEL_SIP_SMC_STATUS_OK (0) on success
+ * INTEL_SIP_SMC_REG_ERROR on error
+ * INTEL_SIP_SMC_RETURN_UNKNOWN_FUNCTION if not supported
+ */
+static int s10_protected_reg_read(void *base,
+ unsigned int reg, unsigned int *val)
+{
+ struct arm_smccc_res result;
+ unsigned long sysmgr_base = (unsigned long)base;
+
+ arm_smccc_smc(INTEL_SIP_SMC_REG_READ, sysmgr_base + reg,
+ 0, 0, 0, 0, 0, 0, &result);
+
+ *val = (unsigned int)result.a1;
+
+ return (int)result.a0;
+}
+
+static struct regmap_config altr_sysmgr_regmap_cfg = {
+ .name = "altr_sysmgr",
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .fast_io = true,
+ .use_single_read = true,
+ .use_single_write = true,
+};
+
+/**
+ * sysmgr_match_phandle
+ * Matching function used by driver_find_device().
+ * Return: True if match is found, otherwise false.
+ */
+static int sysmgr_match_phandle(struct device *dev, void *data)
+{
+ return dev->of_node == (struct device_node *)data;
+}
+
+/**
+ * altr_sysmgr_regmap_lookup_by_phandle
+ * Find the sysmgr previous configured in probe() and return regmap property.
+ * Return: regmap if found or error if not found.
+ */
+struct regmap *altr_sysmgr_regmap_lookup_by_phandle(struct device_node *np,
+ const char *property)
+{
+ struct device *dev;
+ struct altr_sysmgr *sysmgr;
+ struct device_node *sysmgr_np;
+
+ if (property)
+ sysmgr_np = of_parse_phandle(np, property, 0);
+ else
+ sysmgr_np = np;
+
+ if (!sysmgr_np)
+ return ERR_PTR(-ENODEV);
+
+ dev = driver_find_device(&altr_sysmgr_driver.driver, NULL,
+ (void *)sysmgr_np, sysmgr_match_phandle);
+ of_node_put(sysmgr_np);
+ if (!dev)
+ return ERR_PTR(-EPROBE_DEFER);
+
+ sysmgr = dev_get_drvdata(dev);
+
+ return sysmgr->regmap;
+}
+EXPORT_SYMBOL_GPL(altr_sysmgr_regmap_lookup_by_phandle);
+
+static int sysmgr_probe(struct platform_device *pdev)
+{
+ struct altr_sysmgr *sysmgr;
+ struct regmap *regmap;
+ struct resource *res;
+ struct regmap_config sysmgr_config = altr_sysmgr_regmap_cfg;
+ struct device *dev = &pdev->dev;
+ struct device_node *np = dev->of_node;
+
+ sysmgr = devm_kzalloc(dev, sizeof(*sysmgr), GFP_KERNEL);
+ if (!sysmgr)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -ENOENT;
+
+ sysmgr_config.max_register = resource_size(res) -
+ sysmgr_config.reg_stride;
+ if (of_device_is_compatible(np, "altr,sys-mgr-s10")) {
+ /* Need physical address for SMCC call */
+ sysmgr->base = (resource_size_t *)res->start;
+ sysmgr_config.reg_read = s10_protected_reg_read;
+ sysmgr_config.reg_write = s10_protected_reg_write;
+
+ regmap = devm_regmap_init(dev, NULL, sysmgr->base,
+ &sysmgr_config);
+ } else {
+ sysmgr->base = devm_ioremap(dev, res->start,
+ resource_size(res));
+ if (!sysmgr->base)
+ return -ENOMEM;
+
+ sysmgr_config.max_register = res->end - res->start - 3;
+ regmap = devm_regmap_init_mmio(dev, sysmgr->base,
+ &sysmgr_config);
+ }
+
+ if (IS_ERR(regmap)) {
+ pr_err("regmap init failed\n");
+ return PTR_ERR(regmap);
+ }
+
+ sysmgr->regmap = regmap;
+
+ platform_set_drvdata(pdev, sysmgr);
+
+ return 0;
+}
+
+static const struct of_device_id altr_sysmgr_of_match[] = {
+ { .compatible = "altr,sys-mgr" },
+ { .compatible = "altr,sys-mgr-s10" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, altr_sysmgr_of_match);
+
+static struct platform_driver altr_sysmgr_driver = {
+ .probe = sysmgr_probe,
+ .driver = {
+ .name = "altr,system_manager",
+ .of_match_table = altr_sysmgr_of_match,
+ },
+};
+
+static int __init altr_sysmgr_init(void)
+{
+ return platform_driver_register(&altr_sysmgr_driver);
+}
+core_initcall(altr_sysmgr_init);
+
+static void __exit altr_sysmgr_exit(void)
+{
+ platform_driver_unregister(&altr_sysmgr_driver);
+}
+module_exit(altr_sysmgr_exit);
+
+MODULE_AUTHOR("Thor Thayer <>");
+MODULE_DESCRIPTION("SOCFPGA System Manager driver");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/mfd/altera-sysmgr.h b/include/linux/mfd/altera-sysmgr.h
new file mode 100644
index 000000000000..b1ef11a83872
--- /dev/null
+++ b/include/linux/mfd/altera-sysmgr.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2018-2019 Intel Corporation
+ * Copyright (C) 2012 Freescale Semiconductor, Inc.
+ * Copyright (C) 2012 Linaro Ltd.
+ */
+
+#ifndef __LINUX_MFD_ALTERA_SYSMGR_H__
+#define __LINUX_MFD_ALTERA_SYSMGR_H__
+
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/firmware/intel/stratix10-smc.h>
+
+struct device_node;
+
+#ifdef CONFIG_MFD_ALTERA_SYSMGR
+struct regmap *altr_sysmgr_regmap_lookup_by_phandle(struct device_node *np,
+ const char *property);
+#else
+static inline struct regmap *
+altr_sysmgr_regmap_lookup_by_phandle(struct device_node *np,
+ const char *property)
+{
+ return ERR_PTR(-ENOTSUPP);
+}
+#endif
+
+#endif /* __LINUX_MFD_ALTERA_SYSMGR_H__ */
--
2.7.4
^ permalink raw reply related
* [PATCHv4 3/6] ARM: socfpga_defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
From: thor.thayer @ 2019-02-14 20:44 UTC (permalink / raw)
To: lee.jones, arnd, dinguyen, linux, catalin.marinas, will.deacon,
peppe.cavallaro, alexandre.torgue, joabreu, olof
Cc: davem, mcoquelin.stm32, mchehab+samsung, mark.rutland,
bjorn.andersson, devicetree, linux-kernel, linux-arm-kernel,
netdev, Thor Thayer
In-Reply-To: <1550177058-1860-1-git-send-email-thor.thayer@linux.intel.com>
From: Thor Thayer <thor.thayer@linux.intel.com>
Add System Manager driver by default for SOCFPGA ARM32 platforms.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
v2-4 No change
---
arch/arm/configs/socfpga_defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/configs/socfpga_defconfig b/arch/arm/configs/socfpga_defconfig
index 371fca4e1ab7..c510a32f9f0d 100644
--- a/arch/arm/configs/socfpga_defconfig
+++ b/arch/arm/configs/socfpga_defconfig
@@ -109,6 +109,7 @@ CONFIG_SENSORS_LTC2978_REGULATOR=y
CONFIG_WATCHDOG=y
CONFIG_DW_WATCHDOG=y
CONFIG_MFD_ALTERA_A10SR=y
+CONFIG_MFD_ALTERA_SYSMGR=y
CONFIG_MFD_STMPE=y
CONFIG_REGULATOR=y
CONFIG_REGULATOR_FIXED_VOLTAGE=y
--
2.7.4
^ permalink raw reply related
* [PATCHv4 5/6] net: stmmac: socfpga: Use shared System Manager driver
From: thor.thayer @ 2019-02-14 20:44 UTC (permalink / raw)
To: lee.jones, arnd, dinguyen, linux, catalin.marinas, will.deacon,
peppe.cavallaro, alexandre.torgue, joabreu, olof
Cc: davem, mcoquelin.stm32, mchehab+samsung, mark.rutland,
bjorn.andersson, devicetree, linux-kernel, linux-arm-kernel,
netdev, Thor Thayer
In-Reply-To: <1550177058-1860-1-git-send-email-thor.thayer@linux.intel.com>
From: Thor Thayer <thor.thayer@linux.intel.com>
The ARM64 System Manager requires a different method of reading
the System Manager than ARM32. A new System Manager driver was
created to steer ARM32 System Manager calls to regmap_mmio and
ARM64 System Manager calls to the new access method.
Convert from syscon to the shared System Manager driver so that
both ARM64 and ARM32 are supported.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
v2 No change to code. Update commit message.
v3 Remove the unused syscon.h header.
v4 No change
---
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 5b3b06a0a3bf..d466e33635b0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -15,7 +15,7 @@
* Adopted from dwmac-sti.c
*/
-#include <linux/mfd/syscon.h>
+#include <linux/mfd/altera-sysmgr.h>
#include <linux/of.h>
#include <linux/of_address.h>
#include <linux/of_net.h>
@@ -114,7 +114,8 @@ static int socfpga_dwmac_parse_data(struct socfpga_dwmac *dwmac, struct device *
dwmac->interface = of_get_phy_mode(np);
- sys_mgr_base_addr = syscon_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
+ sys_mgr_base_addr =
+ altr_sysmgr_regmap_lookup_by_phandle(np, "altr,sysmgr-syscon");
if (IS_ERR(sys_mgr_base_addr)) {
dev_info(dev, "No sysmgr-syscon node found\n");
return PTR_ERR(sys_mgr_base_addr);
--
2.7.4
^ permalink raw reply related
* [PATCHv4 6/6] arm64: dts: stratix10: New System Manager compatible
From: thor.thayer @ 2019-02-14 20:44 UTC (permalink / raw)
To: lee.jones, arnd, dinguyen, linux, catalin.marinas, will.deacon,
peppe.cavallaro, alexandre.torgue, joabreu, olof
Cc: davem, mcoquelin.stm32, mchehab+samsung, mark.rutland,
bjorn.andersson, devicetree, linux-kernel, linux-arm-kernel,
netdev, Thor Thayer
In-Reply-To: <1550177058-1860-1-git-send-email-thor.thayer@linux.intel.com>
From: Thor Thayer <thor.thayer@linux.intel.com>
Use the new compatible string defined for the Stratix10
System Manager. Remove syscon since it is not correct
on this platform.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
v2 New. Use new Stratix10 System Manager compatible
v3 Use "altr,sys-mgr" as the non-specific compatible.
v4 No change
---
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
index b2c9bb664595..18e4e54db0bb 100644
--- a/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
+++ b/arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi
@@ -363,7 +363,7 @@
};
sysmgr: sysmgr@ffd12000 {
- compatible = "altr,sys-mgr", "syscon";
+ compatible = "altr,sys-mgr-s10","altr,sys-mgr";
reg = <0xffd12000 0x228>;
};
--
2.7.4
^ permalink raw reply related
* [PATCHv4 4/6] arm64: defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
From: thor.thayer @ 2019-02-14 20:44 UTC (permalink / raw)
To: lee.jones, arnd, dinguyen, linux, catalin.marinas, will.deacon,
peppe.cavallaro, alexandre.torgue, joabreu, olof
Cc: davem, mcoquelin.stm32, mchehab+samsung, mark.rutland,
bjorn.andersson, devicetree, linux-kernel, linux-arm-kernel,
netdev, Thor Thayer
In-Reply-To: <1550177058-1860-1-git-send-email-thor.thayer@linux.intel.com>
From: Thor Thayer <thor.thayer@linux.intel.com>
Enable the Stratix10 System Manager by default.
Signed-off-by: Thor Thayer <thor.thayer@linux.intel.com>
---
v2-4 No change
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index c8432e24207e..48a312126cf7 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -417,6 +417,7 @@ CONFIG_MESON_WATCHDOG=m
CONFIG_RENESAS_WDT=y
CONFIG_UNIPHIER_WATCHDOG=y
CONFIG_BCM2835_WDT=y
+CONFIG_MFD_ALTERA_SYSMGR=y
CONFIG_MFD_BD9571MWV=y
CONFIG_MFD_AXP20X_I2C=y
CONFIG_MFD_AXP20X_RSB=y
--
2.7.4
^ permalink raw reply related
* [PATCHv4 0/6] Add SOCFPGA System Manager
From: thor.thayer @ 2019-02-14 20:44 UTC (permalink / raw)
To: lee.jones, arnd, dinguyen, linux, catalin.marinas, will.deacon,
peppe.cavallaro, alexandre.torgue, joabreu, olof
Cc: davem, mcoquelin.stm32, mchehab+samsung, mark.rutland,
bjorn.andersson, devicetree, linux-kernel, linux-arm-kernel,
netdev, Thor Thayer
From: Thor Thayer <thor.thayer@linux.intel.com>
Add MFD driver for SOCFPGA System Manager to handle
System Manager calls differently for ARM32 vs ARM64.
The SOCFPGA System Manager includes registers from several
SOC peripherals.
On ARM32, syscon handles this aggregated register grouping.
Implement System Manager calls as regmap_mmio similar to syscon
for ARM32 SOCFPGA systems.
The ARM64 System Manager can only be accessed from priority
level EL3 so this new MFD driver handles the calls to EL3.
v2 Changes:
Change socfpga_is_s10() to check compatible string.
Add new compatible string for Stratix10 in bindings
and add proper detection method.
Replace base cast with resource_size_t member.
Change s10_sysmgr_regmap_cfg to altr_sysmgr_regmap_cfg to
be generic.
Always use 4 byte width.
Initialize the .reg_read and .reg_write in S10 case only.
Remove call to syscon in 32bit ARM case and handle both
ARM32 and ARM64 in of_sysmgr_register().
Replace IS_ERR_OR_NULL() with IS_ERR().
Remove compatible check functions except phandle function.
v3 Changes:
Create and register regmap in probe().
Lookup functions find registered regmap.
Cleanup of header file.
Fix copyright dates.
Replace global pointer with traditional probe() methodology.
v4 Changes:
Add missing of_node_put().
Add Reviewed-by from v2.
Thor Thayer (6):
mfd: altera-sysmgr: Add SOCFPGA System Manager
Documentation: dt: socfpga: Add S10 System Manager binding
ARM: socfpga_defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
arm64: defconfig: Enable CONFIG_MTD_ALTERA_SYSMGR
net: stmmac: socfpga: Use shared System Manager driver
arm64: dts: stratix10: New System Manager compatible
.../bindings/arm/altera/socfpga-system.txt | 12 ++
MAINTAINERS | 6 +
arch/arm/configs/socfpga_defconfig | 1 +
arch/arm64/boot/dts/altera/socfpga_stratix10.dtsi | 2 +-
arch/arm64/configs/defconfig | 1 +
drivers/mfd/Kconfig | 10 +
drivers/mfd/Makefile | 1 +
drivers/mfd/altera-sysmgr.c | 211 +++++++++++++++++++++
.../net/ethernet/stmicro/stmmac/dwmac-socfpga.c | 5 +-
include/linux/mfd/altera-sysmgr.h | 29 +++
10 files changed, 275 insertions(+), 3 deletions(-)
create mode 100644 drivers/mfd/altera-sysmgr.c
create mode 100644 include/linux/mfd/altera-sysmgr.h
--
2.7.4
^ permalink raw reply
* Re: [PATCH net-next 03/12] net: sched: flower: introduce reference counting for filters
From: Stefano Brivio @ 2019-02-14 20:34 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem
In-Reply-To: <20190214074712.17846-4-vladbu@mellanox.com>
On Thu, 14 Feb 2019 09:47:03 +0200
Vlad Buslov <vladbu@mellanox.com> wrote:
> +static struct cls_fl_filter *fl_get_next_filter(struct tcf_proto *tp,
> + unsigned long *handle)
> +{
> + struct cls_fl_head *head = fl_head_dereference(tp);
> + struct cls_fl_filter *f;
> +
> + rcu_read_lock();
> + /* don't return filters that are being deleted */
> + while ((f = idr_get_next_ul(&head->handle_idr,
> + handle)) != NULL &&
> + !refcount_inc_not_zero(&f->refcnt))
> + ++(*handle);
This... hurts :) What about:
while ((f = idr_get_next_ul(&head->handle_idr, &handle))) {
if (refcount_inc_not_zero(&f->refcnt))
break;
++(*handle);
}
?
> + rcu_read_unlock();
> +
> + return f;
> +}
> +
> static bool __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
> struct netlink_ext_ack *extack)
> {
> @@ -456,10 +503,7 @@ static bool __fl_delete(struct tcf_proto *tp, struct cls_fl_filter *f,
> if (!tc_skip_hw(f->flags))
> fl_hw_destroy_filter(tp, f, extack);
> tcf_unbind_filter(tp, &f->res);
> - if (async)
> - tcf_queue_work(&f->rwork, fl_destroy_filter_work);
> - else
> - __fl_destroy_filter(f);
> + __fl_put(f);
>
> return last;
> }
> @@ -494,11 +538,18 @@ static void fl_destroy(struct tcf_proto *tp, bool rtnl_held,
> tcf_queue_work(&head->rwork, fl_destroy_sleepable);
> }
>
> +static void fl_put(struct tcf_proto *tp, void *arg)
> +{
> + struct cls_fl_filter *f = arg;
> +
> + __fl_put(f);
> +}
> +
> static void *fl_get(struct tcf_proto *tp, u32 handle)
> {
> struct cls_fl_head *head = fl_head_dereference(tp);
>
> - return idr_find(&head->handle_idr, handle);
> + return __fl_get(head, handle);
> }
>
> static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
> @@ -1321,12 +1372,16 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> struct nlattr **tb;
> int err;
>
> - if (!tca[TCA_OPTIONS])
> - return -EINVAL;
> + if (!tca[TCA_OPTIONS]) {
> + err = -EINVAL;
> + goto errout_fold;
> + }
>
> mask = kzalloc(sizeof(struct fl_flow_mask), GFP_KERNEL);
> - if (!mask)
> - return -ENOBUFS;
> + if (!mask) {
> + err = -ENOBUFS;
> + goto errout_fold;
> + }
>
> tb = kcalloc(TCA_FLOWER_MAX + 1, sizeof(struct nlattr *), GFP_KERNEL);
> if (!tb) {
> @@ -1349,6 +1404,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> err = -ENOBUFS;
> goto errout_tb;
> }
> + refcount_set(&fnew->refcnt, 1);
>
> err = tcf_exts_init(&fnew->exts, TCA_FLOWER_ACT, 0);
> if (err < 0)
> @@ -1381,6 +1437,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> if (!tc_in_hw(fnew->flags))
> fnew->flags |= TCA_CLS_FLAGS_NOT_IN_HW;
>
> + refcount_inc(&fnew->refcnt);
I guess I'm not getting the semantics but... why is it 2 now?
--
Stefano
^ permalink raw reply
* Re: [PATCH net-next 02/12] net: sched: flower: refactor fl_change
From: Stefano Brivio @ 2019-02-14 20:34 UTC (permalink / raw)
To: Vlad Buslov; +Cc: netdev, jhs, xiyou.wangcong, jiri, davem
In-Reply-To: <20190214074712.17846-3-vladbu@mellanox.com>
On Thu, 14 Feb 2019 09:47:02 +0200
Vlad Buslov <vladbu@mellanox.com> wrote:
> As a preparation for using classifier spinlock instead of relying on
> external rtnl lock, rearrange code in fl_change. The goal is to group the
> code which changes classifier state in single block in order to allow
> following commits in this set to protect it from parallel modification with
> tp->lock. Data structures that require tp->lock protection are mask
> hashtable and filters list, and classifier handle_idr.
>
> fl_hw_replace_filter() is a sleeping function and cannot be called while
> holding a spinlock. In order to execute all sequence of changes to shared
> classifier data structures atomically, call fl_hw_replace_filter() before
> modifying them.
>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Acked-by: Jiri Pirko <jiri@mellanox.com>
> ---
> net/sched/cls_flower.c | 85 ++++++++++++++++++++++++++------------------------
> 1 file changed, 44 insertions(+), 41 deletions(-)
>
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 88d7af78ba7e..91596a6271f8 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -1354,90 +1354,93 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
> if (err < 0)
> goto errout;
>
> - if (!handle) {
> - handle = 1;
> - err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
> - INT_MAX, GFP_KERNEL);
> - } else if (!fold) {
> - /* user specifies a handle and it doesn't exist */
> - err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
> - handle, GFP_KERNEL);
> - }
> - if (err)
> - goto errout;
> - fnew->handle = handle;
> -
>
> [...]
>
> if (fold) {
> + fnew->handle = handle;
I'm probably missing something, but what if fold is passed and the
handle isn't specified? That can still happen, right? In that case we
wouldn't be allocating the handle.
> +
> + err = rhashtable_insert_fast(&fnew->mask->ht, &fnew->ht_node,
> + fnew->mask->filter_ht_params);
> + if (err)
> + goto errout_hw;
> +
> rhashtable_remove_fast(&fold->mask->ht,
> &fold->ht_node,
> fold->mask->filter_ht_params);
> - if (!tc_skip_hw(fold->flags))
> - fl_hw_destroy_filter(tp, fold, NULL);
> - }
> -
> - *arg = fnew;
> -
> - if (fold) {
> idr_replace(&head->handle_idr, fnew, fnew->handle);
> list_replace_rcu(&fold->list, &fnew->list);
> +
> + if (!tc_skip_hw(fold->flags))
> + fl_hw_destroy_filter(tp, fold, NULL);
> tcf_unbind_filter(tp, &fold->res);
> tcf_exts_get_net(&fold->exts);
> tcf_queue_work(&fold->rwork, fl_destroy_filter_work);
> } else {
> + if (__fl_lookup(fnew->mask, &fnew->mkey)) {
> + err = -EEXIST;
> + goto errout_hw;
> + }
> +
> + if (handle) {
> + /* user specifies a handle and it doesn't exist */
> + err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
> + handle, GFP_ATOMIC);
> + } else {
> + handle = 1;
> + err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
> + INT_MAX, GFP_ATOMIC);
> + }
> + if (err)
> + goto errout_hw;
Just if you respin: a newline here would be nice to have.
> + fnew->handle = handle;
> +
> + err = rhashtable_insert_fast(&fnew->mask->ht, &fnew->ht_node,
> + fnew->mask->filter_ht_params);
> + if (err)
> + goto errout_idr;
> +
> list_add_tail_rcu(&fnew->list, &fnew->mask->filters);
> }
>
> + *arg = fnew;
> +
> kfree(tb);
> kfree(mask);
> return 0;
>
> -errout_mask_ht:
> - rhashtable_remove_fast(&fnew->mask->ht, &fnew->ht_node,
> - fnew->mask->filter_ht_params);
> -
> -errout_mask:
> - fl_mask_put(head, fnew->mask, false);
> -
> errout_idr:
> if (!fold)
This check could go away, I guess (not a strong preference though).
> idr_remove(&head->handle_idr, fnew->handle);
> +errout_hw:
> + if (!tc_skip_hw(fnew->flags))
> + fl_hw_destroy_filter(tp, fnew, NULL);
> +errout_mask:
> + fl_mask_put(head, fnew->mask, false);
> errout:
> tcf_exts_destroy(&fnew->exts);
> kfree(fnew);
--
Stefano
^ permalink raw reply
* [PATCH net-next] mlxsw: core: Extend thermal module with per QSFP module thermal zones
From: Ido Schimmel @ 2019-02-14 20:22 UTC (permalink / raw)
To: netdev@vger.kernel.org
Cc: davem@davemloft.net, Jiri Pirko, andrew@lunn.ch, mlxsw,
Vadim Pasternak, Ido Schimmel
From: Vadim Pasternak <vadimp@mellanox.com>
Add a dedicated thermal zone for each QSFP/SFP module. The current
temperature is obtained from the module's temperature sensor and the
trip points are set based on the warning and critical thresholds
read from the module.
A cooling device (fan) is bound to all the thermal zones. The
thermal zone governor is set to user space in order to avoid
collisions between thermal zones.
For example, one thermal zone might want to increase the speed of
the fan, whereas another one would like to decrease it.
Deferring this decision to user space allows the user to the take
the most suitable decision.
Signed-off-by: Vadim Pasternak <vadimp@mellanox.com>
Signed-off-by: Ido Schimmel <idosch@mellanox.com>
---
.../ethernet/mellanox/mlxsw/core_thermal.c | 400 ++++++++++++++++++
1 file changed, 400 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
index 821fef2e2230..0b85c7252f9e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core_thermal.c
@@ -9,8 +9,10 @@
#include <linux/sysfs.h>
#include <linux/thermal.h>
#include <linux/err.h>
+#include <linux/sfp.h>
#include "core.h"
+#include "core_env.h"
#define MLXSW_THERMAL_POLL_INT 1000 /* ms */
#define MLXSW_THERMAL_SLOW_POLL_INT 20000 /* ms */
@@ -19,6 +21,8 @@
#define MLXSW_THERMAL_ASIC_TEMP_HOT 105000 /* 105C */
#define MLXSW_THERMAL_ASIC_TEMP_CRIT 110000 /* 110C */
#define MLXSW_THERMAL_HYSTERESIS_TEMP 5000 /* 5C */
+#define MLXSW_THERMAL_MODULE_TEMP_SHIFT (MLXSW_THERMAL_HYSTERESIS_TEMP * 2)
+#define MLXSW_THERMAL_ZONE_MAX_NAME 16
#define MLXSW_THERMAL_MAX_STATE 10
#define MLXSW_THERMAL_MAX_DUTY 255
/* Minimum and maximum fan allowed speed in percent: from 20% to 100%. Values
@@ -36,6 +40,13 @@ static char * const mlxsw_thermal_external_allowed_cdev[] = {
"mlxreg_fan",
};
+enum mlxsw_thermal_trips {
+ MLXSW_THERMAL_TEMP_TRIP_NORM,
+ MLXSW_THERMAL_TEMP_TRIP_HIGH,
+ MLXSW_THERMAL_TEMP_TRIP_HOT,
+ MLXSW_THERMAL_TEMP_TRIP_CRIT,
+};
+
struct mlxsw_thermal_trip {
int type;
int temp;
@@ -80,6 +91,16 @@ static const struct mlxsw_thermal_trip default_thermal_trips[] = {
/* Make sure all trips are writable */
#define MLXSW_THERMAL_TRIP_MASK (BIT(MLXSW_THERMAL_NUM_TRIPS) - 1)
+struct mlxsw_thermal;
+
+struct mlxsw_thermal_module {
+ struct mlxsw_thermal *parent;
+ struct thermal_zone_device *tzdev;
+ struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
+ enum thermal_device_mode mode;
+ int module;
+};
+
struct mlxsw_thermal {
struct mlxsw_core *core;
const struct mlxsw_bus_info *bus_info;
@@ -89,6 +110,8 @@ struct mlxsw_thermal {
u8 cooling_levels[MLXSW_THERMAL_MAX_STATE + 1];
struct mlxsw_thermal_trip trips[MLXSW_THERMAL_NUM_TRIPS];
enum thermal_device_mode mode;
+ struct mlxsw_thermal_module *tz_module_arr;
+ unsigned int tz_module_num;
};
static inline u8 mlxsw_state_to_duty(int state)
@@ -122,6 +145,57 @@ static int mlxsw_get_cooling_device_idx(struct mlxsw_thermal *thermal,
return -ENODEV;
}
+static void
+mlxsw_thermal_module_trips_reset(struct mlxsw_thermal_module *tz)
+{
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_NORM].temp = 0;
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_HIGH].temp = 0;
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_HOT].temp = 0;
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_CRIT].temp = 0;
+}
+
+static int
+mlxsw_thermal_module_trips_update(struct device *dev, struct mlxsw_core *core,
+ struct mlxsw_thermal_module *tz)
+{
+ int crit_temp, emerg_temp;
+ int err;
+
+ err = mlxsw_env_module_temp_thresholds_get(core, tz->module,
+ SFP_TEMP_HIGH_WARN,
+ &crit_temp);
+ if (err)
+ return err;
+
+ err = mlxsw_env_module_temp_thresholds_get(core, tz->module,
+ SFP_TEMP_HIGH_ALARM,
+ &emerg_temp);
+ if (err)
+ return err;
+
+ /* According to the system thermal requirements, the thermal zones are
+ * defined with four trip points. The critical and emergency
+ * temperature thresholds, provided by QSFP module are set as "active"
+ * and "hot" trip points, "normal" and "critical" trip points are
+ * derived from "active" and "hot" by subtracting or adding double
+ * hysteresis value.
+ */
+ if (crit_temp >= MLXSW_THERMAL_MODULE_TEMP_SHIFT)
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_NORM].temp = crit_temp -
+ MLXSW_THERMAL_MODULE_TEMP_SHIFT;
+ else
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_NORM].temp = crit_temp;
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_HIGH].temp = crit_temp;
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_HOT].temp = emerg_temp;
+ if (emerg_temp > crit_temp)
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_CRIT].temp = emerg_temp +
+ MLXSW_THERMAL_MODULE_TEMP_SHIFT;
+ else
+ tz->trips[MLXSW_THERMAL_TEMP_TRIP_CRIT].temp = emerg_temp;
+
+ return 0;
+}
+
static int mlxsw_thermal_bind(struct thermal_zone_device *tzdev,
struct thermal_cooling_device *cdev)
{
@@ -291,6 +365,204 @@ static struct thermal_zone_device_ops mlxsw_thermal_ops = {
.set_trip_hyst = mlxsw_thermal_set_trip_hyst,
};
+static int mlxsw_thermal_module_bind(struct thermal_zone_device *tzdev,
+ struct thermal_cooling_device *cdev)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+ struct mlxsw_thermal *thermal = tz->parent;
+ int i, j, err;
+
+ /* If the cooling device is one of ours bind it */
+ if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
+ return 0;
+
+ for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
+ const struct mlxsw_thermal_trip *trip = &tz->trips[i];
+
+ err = thermal_zone_bind_cooling_device(tzdev, i, cdev,
+ trip->max_state,
+ trip->min_state,
+ THERMAL_WEIGHT_DEFAULT);
+ if (err < 0)
+ goto err_bind_cooling_device;
+ }
+ return 0;
+
+err_bind_cooling_device:
+ for (j = i - 1; j >= 0; j--)
+ thermal_zone_unbind_cooling_device(tzdev, j, cdev);
+ return err;
+}
+
+static int mlxsw_thermal_module_unbind(struct thermal_zone_device *tzdev,
+ struct thermal_cooling_device *cdev)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+ struct mlxsw_thermal *thermal = tz->parent;
+ int i;
+ int err;
+
+ /* If the cooling device is one of ours unbind it */
+ if (mlxsw_get_cooling_device_idx(thermal, cdev) < 0)
+ return 0;
+
+ for (i = 0; i < MLXSW_THERMAL_NUM_TRIPS; i++) {
+ err = thermal_zone_unbind_cooling_device(tzdev, i, cdev);
+ WARN_ON(err);
+ }
+ return err;
+}
+
+static int mlxsw_thermal_module_mode_get(struct thermal_zone_device *tzdev,
+ enum thermal_device_mode *mode)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+
+ *mode = tz->mode;
+
+ return 0;
+}
+
+static int mlxsw_thermal_module_mode_set(struct thermal_zone_device *tzdev,
+ enum thermal_device_mode mode)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+ struct mlxsw_thermal *thermal = tz->parent;
+
+ mutex_lock(&tzdev->lock);
+
+ if (mode == THERMAL_DEVICE_ENABLED)
+ tzdev->polling_delay = thermal->polling_delay;
+ else
+ tzdev->polling_delay = 0;
+
+ mutex_unlock(&tzdev->lock);
+
+ tz->mode = mode;
+ thermal_zone_device_update(tzdev, THERMAL_EVENT_UNSPECIFIED);
+
+ return 0;
+}
+
+static int mlxsw_thermal_module_temp_get(struct thermal_zone_device *tzdev,
+ int *p_temp)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+ struct mlxsw_thermal *thermal = tz->parent;
+ struct device *dev = thermal->bus_info->dev;
+ char mtbr_pl[MLXSW_REG_MTBR_LEN];
+ u16 temp;
+ int err;
+
+ /* Read module temperature. */
+ mlxsw_reg_mtbr_pack(mtbr_pl, MLXSW_REG_MTBR_BASE_MODULE_INDEX +
+ tz->module, 1);
+ err = mlxsw_reg_query(thermal->core, MLXSW_REG(mtbr), mtbr_pl);
+ if (err)
+ return err;
+
+ mlxsw_reg_mtbr_temp_unpack(mtbr_pl, 0, &temp, NULL);
+ /* Update temperature. */
+ switch (temp) {
+ case MLXSW_REG_MTBR_NO_CONN: /* fall-through */
+ case MLXSW_REG_MTBR_NO_TEMP_SENS: /* fall-through */
+ case MLXSW_REG_MTBR_INDEX_NA: /* fall-through */
+ case MLXSW_REG_MTBR_BAD_SENS_INFO:
+ temp = 0;
+ break;
+ default:
+ temp = MLXSW_REG_MTMP_TEMP_TO_MC(temp);
+ /* Reset all trip point. */
+ mlxsw_thermal_module_trips_reset(tz);
+ /* Update trip points. */
+ err = mlxsw_thermal_module_trips_update(dev, thermal->core,
+ tz);
+ if (err)
+ return err;
+ break;
+ }
+
+ *p_temp = (int) temp;
+ return 0;
+}
+
+static int
+mlxsw_thermal_module_trip_type_get(struct thermal_zone_device *tzdev, int trip,
+ enum thermal_trip_type *p_type)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+
+ if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
+ return -EINVAL;
+
+ *p_type = tz->trips[trip].type;
+ return 0;
+}
+
+static int
+mlxsw_thermal_module_trip_temp_get(struct thermal_zone_device *tzdev,
+ int trip, int *p_temp)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+
+ if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS)
+ return -EINVAL;
+
+ *p_temp = tz->trips[trip].temp;
+ return 0;
+}
+
+static int
+mlxsw_thermal_module_trip_temp_set(struct thermal_zone_device *tzdev,
+ int trip, int temp)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+
+ if (trip < 0 || trip >= MLXSW_THERMAL_NUM_TRIPS ||
+ temp > tz->trips[MLXSW_THERMAL_TEMP_TRIP_CRIT].temp)
+ return -EINVAL;
+
+ tz->trips[trip].temp = temp;
+ return 0;
+}
+
+static int
+mlxsw_thermal_module_trip_hyst_get(struct thermal_zone_device *tzdev, int trip,
+ int *p_hyst)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+
+ *p_hyst = tz->trips[trip].hyst;
+ return 0;
+}
+
+static int
+mlxsw_thermal_module_trip_hyst_set(struct thermal_zone_device *tzdev, int trip,
+ int hyst)
+{
+ struct mlxsw_thermal_module *tz = tzdev->devdata;
+
+ tz->trips[trip].hyst = hyst;
+ return 0;
+}
+
+static struct thermal_zone_params mlxsw_thermal_module_params = {
+ .governor_name = "user_space",
+};
+
+static struct thermal_zone_device_ops mlxsw_thermal_module_ops = {
+ .bind = mlxsw_thermal_module_bind,
+ .unbind = mlxsw_thermal_module_unbind,
+ .get_mode = mlxsw_thermal_module_mode_get,
+ .set_mode = mlxsw_thermal_module_mode_set,
+ .get_temp = mlxsw_thermal_module_temp_get,
+ .get_trip_type = mlxsw_thermal_module_trip_type_get,
+ .get_trip_temp = mlxsw_thermal_module_trip_temp_get,
+ .set_trip_temp = mlxsw_thermal_module_trip_temp_set,
+ .get_trip_hyst = mlxsw_thermal_module_trip_hyst_get,
+ .set_trip_hyst = mlxsw_thermal_module_trip_hyst_set,
+};
+
static int mlxsw_thermal_get_max_state(struct thermal_cooling_device *cdev,
unsigned long *p_state)
{
@@ -391,6 +663,123 @@ static const struct thermal_cooling_device_ops mlxsw_cooling_ops = {
.set_cur_state = mlxsw_thermal_set_cur_state,
};
+static int
+mlxsw_thermal_module_tz_init(struct mlxsw_thermal_module *module_tz)
+{
+ char tz_name[MLXSW_THERMAL_ZONE_MAX_NAME];
+ int err;
+
+ snprintf(tz_name, sizeof(tz_name), "mlxsw-module%d",
+ module_tz->module + 1);
+ module_tz->tzdev = thermal_zone_device_register(tz_name,
+ MLXSW_THERMAL_NUM_TRIPS,
+ MLXSW_THERMAL_TRIP_MASK,
+ module_tz,
+ &mlxsw_thermal_module_ops,
+ &mlxsw_thermal_module_params,
+ 0, 0);
+ if (IS_ERR(module_tz->tzdev)) {
+ err = PTR_ERR(module_tz->tzdev);
+ return err;
+ }
+
+ return 0;
+}
+
+static void mlxsw_thermal_module_tz_fini(struct thermal_zone_device *tzdev)
+{
+ thermal_zone_device_unregister(tzdev);
+}
+
+static int
+mlxsw_thermal_module_init(struct device *dev, struct mlxsw_core *core,
+ struct mlxsw_thermal *thermal, u8 local_port)
+{
+ struct mlxsw_thermal_module *module_tz;
+ char pmlp_pl[MLXSW_REG_PMLP_LEN];
+ u8 width, module;
+ int err;
+
+ mlxsw_reg_pmlp_pack(pmlp_pl, local_port);
+ err = mlxsw_reg_query(core, MLXSW_REG(pmlp), pmlp_pl);
+ if (err)
+ return err;
+
+ width = mlxsw_reg_pmlp_width_get(pmlp_pl);
+ if (!width)
+ return 0;
+
+ module = mlxsw_reg_pmlp_module_get(pmlp_pl, 0);
+ module_tz = &thermal->tz_module_arr[module];
+ module_tz->module = module;
+ module_tz->parent = thermal;
+ memcpy(module_tz->trips, default_thermal_trips,
+ sizeof(thermal->trips));
+ /* Initialize all trip point. */
+ mlxsw_thermal_module_trips_reset(module_tz);
+ /* Update trip point according to the module data. */
+ err = mlxsw_thermal_module_trips_update(dev, core, module_tz);
+ if (err)
+ return err;
+
+ thermal->tz_module_num++;
+
+ return 0;
+}
+
+static void mlxsw_thermal_module_fini(struct mlxsw_thermal_module *module_tz)
+{
+ if (module_tz && module_tz->tzdev) {
+ mlxsw_thermal_module_tz_fini(module_tz->tzdev);
+ module_tz->tzdev = NULL;
+ }
+}
+
+static int
+mlxsw_thermal_modules_init(struct device *dev, struct mlxsw_core *core,
+ struct mlxsw_thermal *thermal)
+{
+ unsigned int module_count = mlxsw_core_max_ports(core);
+ int i, err;
+
+ thermal->tz_module_arr = kcalloc(module_count,
+ sizeof(*thermal->tz_module_arr),
+ GFP_KERNEL);
+ if (!thermal->tz_module_arr)
+ return -ENOMEM;
+
+ for (i = 1; i < module_count; i++) {
+ err = mlxsw_thermal_module_init(dev, core, thermal, i);
+ if (err)
+ goto err_unreg_tz_module_arr;
+ }
+
+ for (i = 0; i < thermal->tz_module_num; i++) {
+ err = mlxsw_thermal_module_tz_init(&thermal->tz_module_arr[i]);
+ if (err)
+ goto err_unreg_tz_module_arr;
+ }
+
+ return 0;
+
+err_unreg_tz_module_arr:
+ for (i = module_count - 1; i >= 0; i--)
+ mlxsw_thermal_module_fini(&thermal->tz_module_arr[i]);
+ kfree(thermal->tz_module_arr);
+ return err;
+}
+
+static void
+mlxsw_thermal_modules_fini(struct mlxsw_thermal *thermal)
+{
+ unsigned int module_count = mlxsw_core_max_ports(thermal->core);
+ int i;
+
+ for (i = module_count - 1; i >= 0; i--)
+ mlxsw_thermal_module_fini(&thermal->tz_module_arr[i]);
+ kfree(thermal->tz_module_arr);
+}
+
int mlxsw_thermal_init(struct mlxsw_core *core,
const struct mlxsw_bus_info *bus_info,
struct mlxsw_thermal **p_thermal)
@@ -477,9 +866,19 @@ int mlxsw_thermal_init(struct mlxsw_core *core,
goto err_unreg_cdevs;
}
+ err = mlxsw_thermal_modules_init(dev, core, thermal);
+ if (err)
+ goto err_unreg_tzdev;
+
thermal->mode = THERMAL_DEVICE_ENABLED;
*p_thermal = thermal;
return 0;
+
+err_unreg_tzdev:
+ if (thermal->tzdev) {
+ thermal_zone_device_unregister(thermal->tzdev);
+ thermal->tzdev = NULL;
+ }
err_unreg_cdevs:
for (i = 0; i < MLXSW_MFCR_PWMS_MAX; i++)
if (thermal->cdevs[i])
@@ -493,6 +892,7 @@ void mlxsw_thermal_fini(struct mlxsw_thermal *thermal)
{
int i;
+ mlxsw_thermal_modules_fini(thermal);
if (thermal->tzdev) {
thermal_zone_device_unregister(thermal->tzdev);
thermal->tzdev = NULL;
--
2.20.1
^ permalink raw reply related
* Three questions about busy poll
From: Cong Wang @ 2019-02-14 20:15 UTC (permalink / raw)
To: Alexander Duyck
Cc: Eric Dumazet, sridhar.samudrala, Linux Kernel Network Developers
Hello,
While looking into the busy polling in Linux kernel, three questions
come into my mind:
1. In the document[1], it claims sysctl.net.busy_poll depends on
either SO_BUSY_POLL or sysctl.net.busy_read. However, from the code in
ep_set_busy_poll_napi_id(), I don't see such a dependency. It simply
checks sysctl_net_busy_poll and sk->sk_napi_id, but sk->sk_napi_id is
always set as long as we enable CONFIG_NET_RX_BUSY_POLL. So what I am
missing here?
2. Why there is no socket option for sysctl.net.busy_poll? Clearly
sysctl_net_busy_poll is global and SO_BUSY_POLL only works for
sysctl.net.busy_read.
3. How is SO_INCOMING_NAPI_ID supposed to be used? I can't find any
useful documents online. Any example or more detailed doc?
Thanks!
1. https://www.kernel.org/doc/Documentation/sysctl/net.txt
^ permalink raw reply
* Re: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Florian Fainelli @ 2019-02-14 19:33 UTC (permalink / raw)
To: Tristram.Ha; +Cc: sergio.paracuellos, pavel, UNGLinuxDriver, netdev, andrew
In-Reply-To: <SN1PR11MB0446E9A84FB312A2C6E6A097EC670@SN1PR11MB0446.namprd11.prod.outlook.com>
On 2/14/19 11:26 AM, Tristram.Ha@microchip.com wrote:
>>>>> + /* read only dropped counters when link is not up */
>>>>> + if (p->link_just_down)
>>>>> + p->link_just_down = 0;
>>>>> + else if (!p->phydev.link)
>>>>> + mib->cnt_ptr = dev->reg_mib_cnt;
>>>>
>>>> This link_just_down stuff is not clear at all. Why can the drop
>>>> counters not be read when the link is up?
>>>
>>> All of the MIB counters, except some that may be marked by driver, do
>>> not get updated when the link is down, so it is a waste of time to read
>>> them.
>>
>> Can you use netif_running() to determine that condition? Maintaining your
>> own set of variables when the PHY state machine should already determine
>> the link state sounds redundant if not error prone.
>>
>
> The driver can store the PHY device pointer passed to it when the port is enabled. But I am a little worried that pointer can be changed or completely gone as it is out of control of the driver.
The per-port network device is accessible from dp->slave so you can do
netif_running(dp->slave) from your driver, what are you talking about here?
>
>>> My intention is the driver eventually reads the MIB counters at least
>>> every second or faster so that the ethtool API called to show MIB
>>> counters gets them from memory rather than starting a read operation.
>>> For now that API is called from user space with the ethtool utility, so
>>> it may not be called too often and too fast. But theoretically that
>>> API can be called from a program continually.
>>>
>>> For simple switches that do not need to do anything special the MIB
>>> read operation does not cause any issue except CPU load, for more
>>> complicate switches that need to do some background operations too many
>>> read operation can affect some critical functions.
>>
>> Some switches have a MIB autocast feature taking a snapshot which AFAIR is
>> internally implemented as a fast read register with no contention on other
>> registers internally, do you have something similar?
>
> There is no such function in the switch. Every MIB counter read has to go through a single SPI transfer using indirect access. There are no table-like stored values that a single SPI transfer can retrieve all.
>
> For i2C the access is even slower, but then I do not expect this access mechanism is used when the switch can do more complex things.
>
Is that something you are considering to change for future designs?
--
Florian
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix memory leak in bpf_lwt_xmit_reroute
From: Peter Oskolkov @ 2019-02-14 19:28 UTC (permalink / raw)
To: David Ahern
Cc: Peter Oskolkov, Alexei Starovoitov, Daniel Borkmann, netdev,
Willem de Bruijn
In-Reply-To: <8f52efda-b502-2bfb-c881-8833bac461c2@gmail.com>
On Thu, Feb 14, 2019 at 11:10 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 2/14/19 11:42 AM, Peter Oskolkov wrote:
> > I'll refactor the test as you suggest here
> > when I add VRF and GRO tests in a couple of weeks, if this is OK.
>
> IMO, the tests should go in with the feature, not a release later. If we
> are at -rc6 this week then you might get next week as well.
>
> The unreachable toggle is a fairly quick integration. GRO really should
> also get in the same cycle as the feature. Preferably VRF tests as well
> since you have the commands.
OK, I'll work on the negative tests and GRO first.
>
> The pretty printing cleanup can be done later.
^ permalink raw reply
* RE: [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Tristram.Ha @ 2019-02-14 19:26 UTC (permalink / raw)
To: f.fainelli; +Cc: sergio.paracuellos, pavel, UNGLinuxDriver, netdev, andrew
In-Reply-To: <BC575D76-0419-4318-B924-81E29858B8E9@gmail.com>
> >> > + /* read only dropped counters when link is not up */
> >> > + if (p->link_just_down)
> >> > + p->link_just_down = 0;
> >> > + else if (!p->phydev.link)
> >> > + mib->cnt_ptr = dev->reg_mib_cnt;
> >>
> >> This link_just_down stuff is not clear at all. Why can the drop
> >> counters not be read when the link is up?
> >
> >All of the MIB counters, except some that may be marked by driver, do
> >not get updated when the link is down, so it is a waste of time to read
> >them.
>
> Can you use netif_running() to determine that condition? Maintaining your
> own set of variables when the PHY state machine should already determine
> the link state sounds redundant if not error prone.
>
The driver can store the PHY device pointer passed to it when the port is enabled. But I am a little worried that pointer can be changed or completely gone as it is out of control of the driver.
> >My intention is the driver eventually reads the MIB counters at least
> >every second or faster so that the ethtool API called to show MIB
> >counters gets them from memory rather than starting a read operation.
> >For now that API is called from user space with the ethtool utility, so
> >it may not be called too often and too fast. But theoretically that
> >API can be called from a program continually.
> >
> >For simple switches that do not need to do anything special the MIB
> >read operation does not cause any issue except CPU load, for more
> >complicate switches that need to do some background operations too many
> >read operation can affect some critical functions.
>
> Some switches have a MIB autocast feature taking a snapshot which AFAIR is
> internally implemented as a fast read register with no contention on other
> registers internally, do you have something similar?
There is no such function in the switch. Every MIB counter read has to go through a single SPI transfer using indirect access. There are no table-like stored values that a single SPI transfer can retrieve all.
For i2C the access is even slower, but then I do not expect this access mechanism is used when the switch can do more complex things.
^ permalink raw reply
* Re: [PATCH] NETWORKING: avoid use IPCB in cipso_v4_error
From: Stephen Smalley @ 2019-02-14 18:59 UTC (permalink / raw)
To: Nazarov Sergey, Paul Moore
Cc: netdev@vger.kernel.org, linux-security-module@vger.kernel.org,
davem@davemloft.net, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <258621550167251@sas1-46c84f197234.qloud-c.yandex.net>
On 2/14/19 1:00 PM, Nazarov Sergey wrote:
> Hi, Paul!
> I've found the problem and testing it with some very specific custom lsm module. The test case was simple:
> standard TCP/IP client-server application, where server opens CIPSO labeled TCP socket, and client connecting
> to this socket with forbidden labels. After several connections kernel crashing with general memory protection or
> kernel cache inconsistent error.
> I think, the similar behaviour should be with selinux or smack in the same conditions. But I don't know them
> so good to reproduce situation.
For SELinux, you can use
https://github.com/SELinuxProject/selinux-testsuite
That includes testing of CIPSO, both connecting from a client with an
authorized level and from a client with an unauthorized level.
Not sure about Smack; there were some tests in LTP but I don't know if
they would exercise it.
> After applying patch, I haven't kernel crashes.
> But now I've made additional checks and found no response icmp packets. The ip_options_compile requires
> CAP_NET_RAW capability when CIPSO option compiling, if skb is NULL. I have no other ideas than returning to
> the early patch version with ip_options_compile modified. What do you think about that?
>
> 14.02.2019, 00:42, "Paul Moore" <paul@paul-moore.com>:
>> On Tue, Feb 12, 2019 at 10:10 AM Nazarov Sergey <s-nazarov@yandex.ru> wrote:
>>> Since cipso_v4_error might be called from different network stack layers, we can't safely use icmp_send there.
>>> icmp_send copies IP options with ip_option_echo, which uses IPCB to take access to IP header compiled data.
>>> But after commit 971f10ec ("tcp: better TCP_SKB_CB layout to reduce cache line misses"), IPCB can't be used
>>> above IP layer.
>>> This patch fixes the problem by creating in cipso_v4_error a local copy of compiled IP options and using it with
>>> introduced __icmp_send function. This looks some overloaded, but in quite rare error conditions only.
>>>
>>> The original discussion is here:
>>> https://lore.kernel.org/linux-security-module/16659801547571984@sas1-890ba5c2334a.qloud-c.yandex.net/
>>>
>>> Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
>>> ---
>>> include/net/icmp.h | 9 ++++++++-
>>> net/ipv4/cipso_ipv4.c | 18 ++++++++++++++++--
>>> net/ipv4/icmp.c | 7 ++++---
>>> 3 files changed, 28 insertions(+), 6 deletions(-)
>>
>> Hi Sergey,
>>
>> Thanks for your work on finding this and putting a fix together. As
>> we discussed previously, I think this looks good, but can you describe
>> the testing you did to verify that this works correctly?
>>
>>> diff --git a/include/net/icmp.h b/include/net/icmp.h
>>> index 6ac3a5b..e0f709d 100644
>>> --- a/include/net/icmp.h
>>> +++ b/include/net/icmp.h
>>> @@ -22,6 +22,7 @@
>>>
>>> #include <net/inet_sock.h>
>>> #include <net/snmp.h>
>>> +#include <net/ip.h>
>>>
>>> struct icmp_err {
>>> int errno;
>>> @@ -39,7 +40,13 @@ struct icmp_err {
>>> struct sk_buff;
>>> struct net;
>>>
>>> -void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info);
>>> +void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
>>> + const struct ip_options *opt);
>>> +static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
>>> +{
>>> + __icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt);
>>> +}
>>> +
>>> int icmp_rcv(struct sk_buff *skb);
>>> int icmp_err(struct sk_buff *skb, u32 info);
>>> int icmp_init(void);
>>> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
>>> index 777fa3b..234d12e 100644
>>> --- a/net/ipv4/cipso_ipv4.c
>>> +++ b/net/ipv4/cipso_ipv4.c
>>> @@ -1735,13 +1735,27 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
>>> */
>>> void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
>>> {
>>> + unsigned char optbuf[sizeof(struct ip_options) + 40];
>>> + struct ip_options *opt = (struct ip_options *)optbuf;
>>> +
>>> if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
>>> return;
>>>
>>> + /*
>>> + * We might be called above the IP layer,
>>> + * so we can not use icmp_send and IPCB here.
>>> + */
>>> +
>>> + memset(opt, 0, sizeof(struct ip_options));
>>> + opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
>>> + memcpy(opt->__data, (unsigned char *)&(ip_hdr(skb)[1]), opt->optlen);
>>> + if (ip_options_compile(dev_net(skb->dev), opt, NULL))
>>> + return;
>>> +
>>> if (gateway)
>>> - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0);
>>> + __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
>>> else
>>> - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0);
>>> + __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
>>> }
>>>
>>> /**
>>> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
>>> index 065997f..3f24414 100644
>>> --- a/net/ipv4/icmp.c
>>> +++ b/net/ipv4/icmp.c
>>> @@ -570,7 +570,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
>>> * MUST reply to only the first fragment.
>>> */
>>>
>>> -void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
>>> +void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
>>> + const struct ip_options *opt)
>>> {
>>> struct iphdr *iph;
>>> int room;
>>> @@ -691,7 +692,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
>>> iph->tos;
>>> mark = IP4_REPLY_MARK(net, skb_in->mark);
>>>
>>> - if (ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in))
>>> + if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in, opt))
>>> goto out_unlock;
>>>
>>> @@ -742,7 +743,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
>>> local_bh_enable();
>>> out:;
>>> }
>>> -EXPORT_SYMBOL(icmp_send);
>>> +EXPORT_SYMBOL(__icmp_send);
>>>
>>> static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
>>> --
>>
>> --
>> paul moore
>> www.paul-moore.com
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix memory leak in bpf_lwt_xmit_reroute
From: David Ahern @ 2019-02-14 19:10 UTC (permalink / raw)
To: Peter Oskolkov
Cc: Peter Oskolkov, Alexei Starovoitov, Daniel Borkmann, netdev,
Willem de Bruijn
In-Reply-To: <CAFTs51ULXLnGoBhA_JU4L86RpZifykJtsseawHKvcTUYN+Ar0g@mail.gmail.com>
On 2/14/19 11:42 AM, Peter Oskolkov wrote:
> I'll refactor the test as you suggest here
> when I add VRF and GRO tests in a couple of weeks, if this is OK.
IMO, the tests should go in with the feature, not a release later. If we
are at -rc6 this week then you might get next week as well.
The unreachable toggle is a fairly quick integration. GRO really should
also get in the same cycle as the feature. Preferably VRF tests as well
since you have the commands.
The pretty printing cleanup can be done later.
^ permalink raw reply
* Re: [PATCH bpf-next v2] bpf: fix memory leak in bpf_lwt_xmit_reroute
From: David Ahern @ 2019-02-14 19:04 UTC (permalink / raw)
To: Peter Oskolkov, Alexei Starovoitov, Daniel Borkmann, netdev
Cc: Peter Oskolkov, Willem de Bruijn
In-Reply-To: <20190214183931.20293-1-posk@google.com>
On 2/14/19 11:39 AM, Peter Oskolkov wrote:
> On error the skb should be freed. Tested with diff/steps
> provided by David Ahern.
>
> v2: surface routing errors to the user instead of a generic EINVAL,
> as suggested by David Ahern.
>
> Reported-by: David Ahern <dsahern@gmail.com>
> Fixes: 3bd0b15281af ("bpf: add handling of BPF_LWT_REROUTE to lwt_bpf.c")
> Signed-off-by: Peter Oskolkov <posk@google.com>
> ---
> net/core/lwt_bpf.c | 29 ++++++++++++++++++++---------
> 1 file changed, 20 insertions(+), 9 deletions(-)
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: fix memory leak in bpf_lwt_xmit_reroute
From: Peter Oskolkov @ 2019-02-14 18:42 UTC (permalink / raw)
To: David Ahern
Cc: Peter Oskolkov, Alexei Starovoitov, Daniel Borkmann, netdev,
Willem de Bruijn
In-Reply-To: <733c9f8e-2262-dbff-6aa6-f960983812ab@gmail.com>
On Thu, Feb 14, 2019 at 10:11 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 2/13/19 11:09 PM, Peter Oskolkov wrote:
> > On error the skb should be freed. Tested with diff/steps
> > provided by David Ahern.
> >
> > Reported-by: David Ahern <dsahern@gmail.com>
> > Fixes: 3bd0b15281af ("bpf: add handling of BPF_LWT_REROUTE to lwt_bpf.c")
> > Signed-off-by: Peter Oskolkov <posk@google.com>
> > ---
> > net/core/lwt_bpf.c | 24 ++++++++++++++++--------
> > 1 file changed, 16 insertions(+), 8 deletions(-)
> >
> > diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> > index 32251f3fcda0..f3273cbb6b22 100644
> > --- a/net/core/lwt_bpf.c
> > +++ b/net/core/lwt_bpf.c
> > @@ -179,18 +179,19 @@ static int bpf_lwt_xmit_reroute(struct sk_buff *skb)
> > struct net_device *l3mdev = l3mdev_master_dev_rcu(skb_dst(skb)->dev);
> > int oif = l3mdev ? l3mdev->ifindex : 0;
> > struct dst_entry *dst = NULL;
> > + int err = -EAFNOSUPPORT;
> > struct sock *sk;
> > struct net *net;
> > bool ipv4;
> > - int err;
> >
> > if (skb->protocol == htons(ETH_P_IP))
> > ipv4 = true;
> > else if (skb->protocol == htons(ETH_P_IPV6))
> > ipv4 = false;
> > else
> > - return -EAFNOSUPPORT;
> > + goto err;
> >
> > + err = -EINVAL;
> > sk = sk_to_full_sk(skb->sk);
> > if (sk) {
> > if (sk->sk_bound_dev_if)
> > @@ -216,7 +217,7 @@ static int bpf_lwt_xmit_reroute(struct sk_buff *skb)
> >
> > rt = ip_route_output_key(net, &fl4);
> > if (IS_ERR(rt))
> > - return -EINVAL;
> > + goto err;
> > dst = &rt->dst;
> > } else {
> > struct ipv6hdr *iph6 = ipv6_hdr(skb);
> > @@ -231,12 +232,15 @@ static int bpf_lwt_xmit_reroute(struct sk_buff *skb)
> > fl6.saddr = iph6->saddr;
> >
> > err = ipv6_stub->ipv6_dst_lookup(net, skb->sk, &dst, &fl6);
> > - if (err || IS_ERR(dst))
> > - return -EINVAL;
> > + if (err || IS_ERR(dst)) {
> > + err = -EINVAL;
> > + goto err;
> > + }
> > }
> > if (unlikely(dst->error)) {
> > dst_release(dst);
> > - return -EINVAL;
> > + err = -EINVAL;
> > + goto err;
> > }
> >
> > /* Although skb header was reserved in bpf_lwt_push_ip_encap(), it
>
> EINVAL is a confusing return code; it is not an EINVAL problem, it is a
> routing problem:
Thanks, David! Sent a v2 of the patch.
>
> ...
> starting egress IPv4 encap test
> ping: sendmsg: Invalid argument
> FAIL: test_ping: 1
>
>
> Versus returning the error from the lookup:
> ...
> starting egress IPv4 encap test
> ping: sendmsg: No route to host
> FAIL: test_ping: 1
>
>
> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> index f3273cbb6b22..a1901ba319fc 100644
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
> @@ -191,7 +191,6 @@ static int bpf_lwt_xmit_reroute(struct sk_buff *skb)
> else
> goto err;
>
> - err = -EINVAL;
> sk = sk_to_full_sk(skb->sk);
> if (sk) {
> if (sk->sk_bound_dev_if)
> @@ -216,8 +215,10 @@ static int bpf_lwt_xmit_reroute(struct sk_buff *skb)
> fl4.saddr = iph->saddr;
>
> rt = ip_route_output_key(net, &fl4);
> - if (IS_ERR(rt))
> + if (IS_ERR(rt)) {
> + err = PTR_ERR(rt);
> goto err;
> + }
> dst = &rt->dst;
> } else {
> struct ipv6hdr *iph6 = ipv6_hdr(skb);
> @@ -232,14 +233,12 @@ static int bpf_lwt_xmit_reroute(struct sk_buff *skb)
> fl6.saddr = iph6->saddr;
>
> err = ipv6_stub->ipv6_dst_lookup(net, skb->sk, &dst, &fl6);
> - if (err || IS_ERR(dst)) {
> - err = -EINVAL;
> + if (err || IS_ERR(dst))
> goto err;
> - }
> }
> if (unlikely(dst->error)) {
> dst_release(dst);
> - err = -EINVAL;
> + err = dst->error;
> goto err;
> }
>
>
>
>
> > @@ -246,17 +250,21 @@ static int bpf_lwt_xmit_reroute(struct sk_buff *skb)
> > */
> > err = skb_cow_head(skb, LL_RESERVED_SPACE(dst->dev));
> > if (unlikely(err))
> > - return err;
> > + goto err;
> >
> > skb_dst_drop(skb);
> > skb_dst_set(skb, dst);
> >
> > err = dst_output(dev_net(skb_dst(skb)->dev), skb->sk, skb);
> > if (unlikely(err))
> > - return err;
> > + goto err;
> >
> > /* ip[6]_finish_output2 understand LWTUNNEL_XMIT_DONE */
> > return LWTUNNEL_XMIT_DONE;
> > +
> > +err:
> > + kfree_skb(skb);
> > + return err;
> > }
> >
> > static int bpf_xmit(struct sk_buff *skb)
> >
>
> I figured it was a leaked skb.
>
> Also, the test script needs to be updated as well with the negative
> tests -- ie., toggle the route from a dev/gateway to a reject
> (e.g.,unreachable) and back.
>
> Also, don't exit on the first failure - run all of them.
I'll refactor the test as you suggest here
when I add VRF and GRO tests in a couple of weeks, if this is OK.
>
> Having the result line up is more user friendly. e.g.,
>
> # ./fib_tests.sh
>
> Single path route test
> Start point
> TEST: IPv4 fibmatch [ OK ]
> TEST: IPv6 fibmatch [ OK ]
> Nexthop device deleted
> TEST: IPv4 fibmatch - no route [ OK ]
> TEST: IPv6 fibmatch - no route [ OK ]
> ...
^ 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