* Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload
From: Johan Hovold @ 2018-04-11 7:07 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: devel, samuel, gregkh, johan, linux-kernel, netdev,
arvind.yadav.cs, davem
In-Reply-To: <1523410174-1553-1-git-send-email-baijiaju1990@gmail.com>
On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> stir421x_fw_upload() is never called in atomic context.
>
> The call chain ending up at stir421x_fw_upload() is:
> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>
> irda_usb_probe() is set as ".probe" in struct usb_driver.
> This function is not called in atomic context.
>
> Despite never getting called from atomic context, stir421x_fw_upload()
> calls mdelay() to busily wait.
> This is not necessary and can be replaced with usleep_range() to
> avoid busy waiting.
>
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
This all looks good (also note the call to usb_control_msg(), which may
sleep, just above the mdelay()).
Reviewed-by: Johan Hovold <johan@kernel.org>
Thanks,
Johan
^ permalink raw reply
* Re: [PATCH 2/2] staging: irda: Replace mdelay with usleep_range in irda_usb_probe
From: Johan Hovold @ 2018-04-11 7:08 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: samuel, gregkh, davem, johan, arvind.yadav.cs, netdev, devel,
linux-kernel
In-Reply-To: <1523410435-1693-1-git-send-email-baijiaju1990@gmail.com>
On Wed, Apr 11, 2018 at 09:33:55AM +0800, Jia-Ju Bai wrote:
> irda_usb_probe() is never called in atomic context.
>
> irda_usb_probe() is only set as ".probe" in struct usb_driver.
>
> Despite never getting called from atomic context, irda_usb_probe()
> calls mdelay() to busily wait.
> This is not necessary and can be replaced with usleep_range() to
> avoid busy waiting.
>
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
Reviewed-by: Johan Hovold <johan@kernel.org>
^ permalink raw reply
* Re: [PATCH] net: dsa: b53: Replace mdelay with msleep in b53_switch_reset_gpio
From: Jia-Ju Bai @ 2018-04-11 7:14 UTC (permalink / raw)
To: Phil Reid, f.fainelli, andrew, vivien.didelot; +Cc: netdev, linux-kernel
In-Reply-To: <11d079ab-3c07-a10d-c321-f873c53e9690@electromag.com.au>
On 2018/4/11 13:30, Phil Reid wrote:
> On 11/04/2018 09:51, Jia-Ju Bai wrote:
>> b53_switch_reset_gpio() is never called in atomic context.
>>
>> The call chain ending up at b53_switch_reset_gpio() is:
>> [1] b53_switch_reset_gpio() <- b53_switch_reset() <-
>> b53_reset_switch() <- b53_setup()
>>
>> b53_switch_reset_gpio() is set as ".setup" in struct dsa_switch_ops.
>> This function is not called in atomic context.
>>
>> Despite never getting called from atomic context,
>> b53_switch_reset_gpio()
>> calls mdelay() to busily wait.
>> This is not necessary and can be replaced with msleep() to
>> avoid busy waiting.
>>
>> This is found by a static analysis tool named DCNS written by myself.
>> And I also manually check it.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>> ---
>> drivers/net/dsa/b53/b53_common.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/dsa/b53/b53_common.c
>> b/drivers/net/dsa/b53/b53_common.c
>> index 274f367..e070ff6 100644
>> --- a/drivers/net/dsa/b53/b53_common.c
>> +++ b/drivers/net/dsa/b53/b53_common.c
>> @@ -597,10 +597,10 @@ static void b53_switch_reset_gpio(struct
>> b53_device *dev)
>> /* Reset sequence: RESET low(50ms)->high(20ms)
>> */
>> gpio_set_value(gpio, 0);
>> - mdelay(50);
>> + msleep(50);
>> gpio_set_value(gpio, 1);
>> - mdelay(20);
>> + msleep(20);
>> dev->current_page = 0xff;
>> }
>>
> Would that also imply gpio_set_value could be gpio_set_value_cansleep?
>
Yes, I think gpio_set_value_cansleep() is okay here?
Do I need to send a V2 patch to replace gpio_set_value()?
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload
From: Jia-Ju Bai @ 2018-04-11 7:17 UTC (permalink / raw)
To: Greg KH; +Cc: devel, samuel, netdev, johan, linux-kernel, arvind.yadav.cs,
davem
In-Reply-To: <20180411064135.GA28354@kroah.com>
On 2018/4/11 14:41, Greg KH wrote:
> On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
>> stir421x_fw_upload() is never called in atomic context.
>>
>> The call chain ending up at stir421x_fw_upload() is:
>> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>>
>> irda_usb_probe() is set as ".probe" in struct usb_driver.
>> This function is not called in atomic context.
>>
>> Despite never getting called from atomic context, stir421x_fw_upload()
>> calls mdelay() to busily wait.
>> This is not necessary and can be replaced with usleep_range() to
>> avoid busy waiting.
>>
>> This is found by a static analysis tool named DCNS written by myself.
>> And I also manually check it.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>> ---
>> drivers/staging/irda/drivers/irda-usb.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
> Please, at the very least, work off of Linus's tree. There is no
> drivers/staging/irda/ anymore :)
>
Okay, sorry.
Could you please recommend me a right tree or its git address?
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* [PATCH v1 net 0/3] lan78xx: Fixes and enhancements
From: Raghuram Chary J @ 2018-04-11 7:24 UTC (permalink / raw)
To: davem; +Cc: netdev, unglinuxdriver, woojung.huh, raghuramchary.jallipalli
These series of patches have fix and enhancements for
lan78xx driver.
Raghuram Chary J (3):
lan78xx: PHY DSP registers initialization to address EEE link drop
issues with long cables
lan78xx: Add support to dump lan78xx registers
lan78xx: Lan7801 Support for Fixed PHY
drivers/net/phy/microchip.c | 178 ++++++++++++++++++++++++++++++++++++++++++-
drivers/net/usb/Kconfig | 1 +
drivers/net/usb/lan78xx.c | 96 ++++++++++++++++++++++-
include/linux/microchipphy.h | 8 ++
4 files changed, 278 insertions(+), 5 deletions(-)
--
2.16.2
^ permalink raw reply
* [PATCH v1 net 1/3] lan78xx: PHY DSP registers initialization to address EEE link drop issues with long cables
From: Raghuram Chary J @ 2018-04-11 7:24 UTC (permalink / raw)
To: davem; +Cc: netdev, unglinuxdriver, woojung.huh, raghuramchary.jallipalli
In-Reply-To: <20180411072450.9809-1-raghuramchary.jallipalli@microchip.com>
The patch is to configure DSP registers of PHY device
to handle Gbe-EEE failures with >40m cable length.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Raghuram Chary J <raghuramchary.jallipalli@microchip.com>
---
v0->v1:
* Use phy_save_page to save current page before switching to TR page.
* Use phy_restore_page to restore saved page.
* Add read_page and write_page callbacks.
* __phy_read, __phy_write to read,write phy registers.
* Handle error conditions.
---
drivers/net/phy/microchip.c | 178 ++++++++++++++++++++++++++++++++++++++++++-
include/linux/microchipphy.h | 8 ++
2 files changed, 185 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/microchip.c b/drivers/net/phy/microchip.c
index 0f293ef28935..4a8e91922eaa 100644
--- a/drivers/net/phy/microchip.c
+++ b/drivers/net/phy/microchip.c
@@ -20,6 +20,7 @@
#include <linux/ethtool.h>
#include <linux/phy.h>
#include <linux/microchipphy.h>
+#include <linux/delay.h>
#define DRIVER_AUTHOR "WOOJUNG HUH <woojung.huh@microchip.com>"
#define DRIVER_DESC "Microchip LAN88XX PHY driver"
@@ -30,6 +31,16 @@ struct lan88xx_priv {
__u32 wolopts;
};
+static int lan88xx_read_page(struct phy_device *phydev)
+{
+ return __phy_read(phydev, LAN88XX_EXT_PAGE_ACCESS);
+}
+
+static int lan88xx_write_page(struct phy_device *phydev, int page)
+{
+ return __phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS, page);
+}
+
static int lan88xx_phy_config_intr(struct phy_device *phydev)
{
int rc;
@@ -66,6 +77,150 @@ static int lan88xx_suspend(struct phy_device *phydev)
return 0;
}
+static int lan88xx_TR_reg_set(struct phy_device *phydev, u16 regaddr,
+ u32 data)
+{
+ int val, save_page, ret = 0;
+ u16 buf;
+
+ /* Save current page */
+ save_page = phy_save_page(phydev);
+ if (save_page < 0) {
+ pr_warn("Failed to get current page\n");
+ goto err;
+ }
+
+ /* Switch to TR page */
+ lan88xx_write_page(phydev, LAN88XX_EXT_PAGE_ACCESS_TR);
+
+ ret = __phy_write(phydev, LAN88XX_EXT_PAGE_TR_LOW_DATA,
+ (data & 0xFFFF));
+ if (ret < 0) {
+ pr_warn("Failed to write TR low data\n");
+ goto err;
+ }
+
+ ret = __phy_write(phydev, LAN88XX_EXT_PAGE_TR_HIGH_DATA,
+ (data & 0x00FF0000) >> 16);
+ if (ret < 0) {
+ pr_warn("Failed to write TR high data\n");
+ goto err;
+ }
+
+ /* Config control bits [15:13] of register */
+ buf = (regaddr & ~(0x3 << 13));/* Clr [14:13] to write data in reg */
+ buf |= 0x8000; /* Set [15] to Packet transmit */
+
+ ret = __phy_write(phydev, LAN88XX_EXT_PAGE_TR_CR, buf);
+ if (ret < 0) {
+ pr_warn("Failed to write data in reg\n");
+ goto err;
+ }
+
+ usleep_range(1000, 2000);/* Wait for Data to be written */
+ val = __phy_read(phydev, LAN88XX_EXT_PAGE_TR_CR);
+ if (!(val & 0x8000))
+ pr_warn("TR Register[0x%X] configuration failed\n", regaddr);
+err:
+ return phy_restore_page(phydev, save_page, ret);
+}
+
+static void lan88xx_config_TR_regs(struct phy_device *phydev)
+{
+ int err;
+
+ /* Get access to Channel 0x1, Node 0xF , Register 0x01.
+ * Write 24-bit value 0x12B00A to register. Setting MrvlTrFix1000Kf,
+ * MrvlTrFix1000Kp, MasterEnableTR bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x0F82, 0x12B00A);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x0F82]\n");
+
+ /* Get access to Channel b'10, Node b'1101, Register 0x06.
+ * Write 24-bit value 0xD2C46F to register. Setting SSTrKf1000Slv,
+ * SSTrKp1000Mas bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x168C, 0xD2C46F);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x168C]\n");
+
+ /* Get access to Channel b'10, Node b'1111, Register 0x11.
+ * Write 24-bit value 0x620 to register. Setting rem_upd_done_thresh
+ * bits
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x17A2, 0x620);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x17A2]\n");
+
+ /* Get access to Channel b'10, Node b'1101, Register 0x10.
+ * Write 24-bit value 0xEEFFDD to register. Setting
+ * eee_TrKp1Long_1000, eee_TrKp2Long_1000, eee_TrKp3Long_1000,
+ * eee_TrKp1Short_1000,eee_TrKp2Short_1000, eee_TrKp3Short_1000 bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x16A0, 0xEEFFDD);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x16A0]\n");
+
+ /* Get access to Channel b'10, Node b'1101, Register 0x13.
+ * Write 24-bit value 0x071448 to register. Setting
+ * slv_lpi_tr_tmr_val1, slv_lpi_tr_tmr_val2 bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x16A6, 0x071448);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x16A6]\n");
+
+ /* Get access to Channel b'10, Node b'1101, Register 0x12.
+ * Write 24-bit value 0x13132F to register. Setting
+ * slv_sigdet_timer_val1, slv_sigdet_timer_val2 bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x16A4, 0x13132F);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x16A4]\n");
+
+ /* Get access to Channel b'10, Node b'1101, Register 0x14.
+ * Write 24-bit value 0x0 to register. Setting eee_3level_delay,
+ * eee_TrKf_freeze_delay bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x16A8, 0x0);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x16A8]\n");
+
+ /* Get access to Channel b'01, Node b'1111, Register 0x34.
+ * Write 24-bit value 0x91B06C to register. Setting
+ * FastMseSearchThreshLong1000, FastMseSearchThreshShort1000,
+ * FastMseSearchUpdGain1000 bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x0FE8, 0x91B06C);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x0FE8]\n");
+
+ /* Get access to Channel b'01, Node b'1111, Register 0x3E.
+ * Write 24-bit value 0xC0A028 to register. Setting
+ * FastMseKp2ThreshLong1000, FastMseKp2ThreshShort1000,
+ * FastMseKp2UpdGain1000, FastMseKp2ExitEn1000 bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x0FFC, 0xC0A028);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x0FFC]\n");
+
+ /* Get access to Channel b'01, Node b'1111, Register 0x35.
+ * Write 24-bit value 0x041600 to register. Setting
+ * FastMseSearchPhShNum1000, FastMseSearchClksPerPh1000,
+ * FastMsePhChangeDelay1000 bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x0FEA, 0x041600);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x0FEA]\n");
+
+ /* Get access to Channel b'10, Node b'1101, Register 0x03.
+ * Write 24-bit value 0x000004 to register. Setting TrFreeze bits.
+ */
+ err = lan88xx_TR_reg_set(phydev, 0x1686, 0x000004);
+ if (err < 0)
+ pr_warn("Failed to Set Register[0x1686]\n");
+}
+
static int lan88xx_probe(struct phy_device *phydev)
{
struct device *dev = &phydev->mdio.dev;
@@ -132,6 +287,25 @@ static void lan88xx_set_mdix(struct phy_device *phydev)
phy_write(phydev, LAN88XX_EXT_PAGE_ACCESS, LAN88XX_EXT_PAGE_SPACE_0);
}
+static int lan88xx_config_init(struct phy_device *phydev)
+{
+ int val;
+
+ genphy_config_init(phydev);
+ /*Zerodetect delay enable */
+ val = phy_read_mmd(phydev, MDIO_MMD_PCS,
+ PHY_ARDENNES_MMD_DEV_3_PHY_CFG);
+ val |= PHY_ARDENNES_MMD_DEV_3_PHY_CFG_ZD_DLY_EN_;
+
+ phy_write_mmd(phydev, MDIO_MMD_PCS, PHY_ARDENNES_MMD_DEV_3_PHY_CFG,
+ val);
+
+ /* Config DSP registers */
+ lan88xx_config_TR_regs(phydev);
+
+ return 0;
+}
+
static int lan88xx_config_aneg(struct phy_device *phydev)
{
lan88xx_set_mdix(phydev);
@@ -151,7 +325,7 @@ static struct phy_driver microchip_phy_driver[] = {
.probe = lan88xx_probe,
.remove = lan88xx_remove,
- .config_init = genphy_config_init,
+ .config_init = lan88xx_config_init,
.config_aneg = lan88xx_config_aneg,
.ack_interrupt = lan88xx_phy_ack_interrupt,
@@ -160,6 +334,8 @@ static struct phy_driver microchip_phy_driver[] = {
.suspend = lan88xx_suspend,
.resume = genphy_resume,
.set_wol = lan88xx_set_wol,
+ .read_page = lan88xx_read_page,
+ .write_page = lan88xx_write_page,
} };
module_phy_driver(microchip_phy_driver);
diff --git a/include/linux/microchipphy.h b/include/linux/microchipphy.h
index eb492d47f717..8f9c90379732 100644
--- a/include/linux/microchipphy.h
+++ b/include/linux/microchipphy.h
@@ -70,4 +70,12 @@
#define LAN88XX_MMD3_CHIP_ID (32877)
#define LAN88XX_MMD3_CHIP_REV (32878)
+/* DSP registers */
+#define PHY_ARDENNES_MMD_DEV_3_PHY_CFG (0x806A)
+#define PHY_ARDENNES_MMD_DEV_3_PHY_CFG_ZD_DLY_EN_ (0x2000)
+#define LAN88XX_EXT_PAGE_ACCESS_TR (0x52B5)
+#define LAN88XX_EXT_PAGE_TR_CR 16
+#define LAN88XX_EXT_PAGE_TR_LOW_DATA 17
+#define LAN88XX_EXT_PAGE_TR_HIGH_DATA 18
+
#endif /* _MICROCHIPPHY_H */
--
2.16.2
^ permalink raw reply related
* [PATCH v1 net 2/3] lan78xx: Add support to dump lan78xx registers
From: Raghuram Chary J @ 2018-04-11 7:24 UTC (permalink / raw)
To: davem; +Cc: netdev, unglinuxdriver, woojung.huh, raghuramchary.jallipalli
In-Reply-To: <20180411072450.9809-1-raghuramchary.jallipalli@microchip.com>
In order to dump lan78xx family registers using ethtool, add
support at lan78xx driver level.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Raghuram Chary J <raghuramchary.jallipalli@microchip.com>
---
v0->v1:
* Return device regs len if phydev is null.
---
drivers/net/usb/lan78xx.c | 54 +++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 54 insertions(+)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 55a78eb96961..cbeec784f8b8 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -278,6 +278,30 @@ struct lan78xx_statstage64 {
u64 eee_tx_lpi_time;
};
+u32 lan78xx_regs[] = {
+ ID_REV,
+ INT_STS,
+ HW_CFG,
+ PMT_CTL,
+ E2P_CMD,
+ E2P_DATA,
+ USB_STATUS,
+ VLAN_TYPE,
+ MAC_CR,
+ MAC_RX,
+ MAC_TX,
+ FLOW,
+ ERR_STS,
+ MII_ACC,
+ MII_DATA,
+ EEE_TX_LPI_REQ_DLY,
+ EEE_TW_TX_SYS,
+ EEE_TX_LPI_REM_DLY,
+ WUCSR
+};
+
+#define PHY_REG_SIZE (32 * sizeof(u32))
+
struct lan78xx_net;
struct lan78xx_priv {
@@ -1604,6 +1628,34 @@ static int lan78xx_set_pause(struct net_device *net,
return ret;
}
+static int lan78xx_get_regs_len(struct net_device *netdev)
+{
+ if (!netdev->phydev)
+ return (sizeof(lan78xx_regs));
+ else
+ return (sizeof(lan78xx_regs) + PHY_REG_SIZE);
+}
+
+static void
+lan78xx_get_regs(struct net_device *netdev, struct ethtool_regs *regs,
+ void *buf)
+{
+ u32 *data = buf;
+ int i, j;
+ struct lan78xx_net *dev = netdev_priv(netdev);
+
+ /* Read Device/MAC registers */
+ for (i = 0, j = 0; i < (sizeof(lan78xx_regs) / sizeof(u32)); i++, j++)
+ lan78xx_read_reg(dev, lan78xx_regs[i], &data[j]);
+
+ if (!netdev->phydev)
+ return;
+
+ /* Read PHY registers */
+ for (i = 0; i < 32; i++, j++)
+ data[j] = phy_read(netdev->phydev, i);
+}
+
static const struct ethtool_ops lan78xx_ethtool_ops = {
.get_link = lan78xx_get_link,
.nway_reset = phy_ethtool_nway_reset,
@@ -1624,6 +1676,8 @@ static const struct ethtool_ops lan78xx_ethtool_ops = {
.set_pauseparam = lan78xx_set_pause,
.get_link_ksettings = lan78xx_get_link_ksettings,
.set_link_ksettings = lan78xx_set_link_ksettings,
+ .get_regs_len = lan78xx_get_regs_len,
+ .get_regs = lan78xx_get_regs,
};
static int lan78xx_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
--
2.16.2
^ permalink raw reply related
* [PATCH v1 net 3/3] lan78xx: Lan7801 Support for Fixed PHY
From: Raghuram Chary J @ 2018-04-11 7:24 UTC (permalink / raw)
To: davem; +Cc: netdev, unglinuxdriver, woojung.huh, raghuramchary.jallipalli
In-Reply-To: <20180411072450.9809-1-raghuramchary.jallipalli@microchip.com>
Adding Fixed PHY support to the lan78xx driver.
Fixes: 55d7de9de6c3 ("Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver")
Signed-off-by: Raghuram Chary J <raghuramchary.jallipalli@microchip.com>
---
drivers/net/usb/Kconfig | 1 +
drivers/net/usb/lan78xx.c | 42 ++++++++++++++++++++++++++++++++++++++----
2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
index f28bd74ac275..418b0904cecb 100644
--- a/drivers/net/usb/Kconfig
+++ b/drivers/net/usb/Kconfig
@@ -111,6 +111,7 @@ config USB_LAN78XX
select MII
select PHYLIB
select MICROCHIP_PHY
+ select FIXED_PHY
help
This option adds support for Microchip LAN78XX based USB 2
& USB 3 10/100/1000 Ethernet adapters.
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index cbeec784f8b8..0c87ac1b767f 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -36,7 +36,7 @@
#include <linux/irq.h>
#include <linux/irqchip/chained_irq.h>
#include <linux/microchipphy.h>
-#include <linux/phy.h>
+#include <linux/phy_fixed.h>
#include "lan78xx.h"
#define DRIVER_AUTHOR "WOOJUNG HUH <woojung.huh@microchip.com>"
@@ -426,6 +426,7 @@ struct lan78xx_net {
struct statstage stats;
struct irq_domain_data domain_data;
+ struct phy_device *fixedphy;
};
/* define external phy id */
@@ -2061,11 +2062,39 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
int ret;
u32 mii_adv;
struct phy_device *phydev;
+ struct fixed_phy_status fphy_status = {
+ .link = 1,
+ .speed = SPEED_1000,
+ .duplex = DUPLEX_FULL,
+ };
phydev = phy_find_first(dev->mdiobus);
if (!phydev) {
- netdev_err(dev->net, "no PHY found\n");
- return -EIO;
+ if (dev->chipid == ID_REV_CHIP_ID_7801_) {
+ u32 buf;
+
+ netdev_info(dev->net, "PHY Not Found!! Registering Fixed PHY\n");
+ phydev = fixed_phy_register(PHY_POLL, &fphy_status, -1,
+ NULL);
+ if (IS_ERR(phydev)) {
+ netdev_err(dev->net, "No PHY/fixed_PHY found\n");
+ return -ENODEV;
+ }
+ netdev_info(dev->net, "Registered FIXED PHY\n");
+ dev->interface = PHY_INTERFACE_MODE_RGMII;
+ dev->fixedphy = phydev;
+ ret = lan78xx_write_reg(dev, MAC_RGMII_ID,
+ MAC_RGMII_ID_TXC_DELAY_EN_);
+ ret = lan78xx_write_reg(dev, RGMII_TX_BYP_DLL, 0x3D00);
+ ret = lan78xx_read_reg(dev, HW_CFG, &buf);
+ buf |= HW_CFG_CLK125_EN_;
+ buf |= HW_CFG_REFCLK25_EN_;
+ ret = lan78xx_write_reg(dev, HW_CFG, buf);
+ goto phyinit;
+ } else {
+ netdev_err(dev->net, "no PHY found\n");
+ return -EIO;
+ }
}
if ((dev->chipid == ID_REV_CHIP_ID_7800_) ||
@@ -2103,7 +2132,7 @@ static int lan78xx_phy_init(struct lan78xx_net *dev)
ret = -EIO;
goto error;
}
-
+phyinit:
/* if phyirq is not set, use polling mode in phylib */
if (dev->domain_data.phyirq > 0)
phydev->irq = dev->domain_data.phyirq;
@@ -3562,6 +3591,11 @@ static void lan78xx_disconnect(struct usb_interface *intf)
udev = interface_to_usbdev(intf);
net = dev->net;
+
+ if (dev->fixedphy) {
+ fixed_phy_unregister(dev->fixedphy);
+ dev->fixedphy = NULL;
+ }
unregister_netdev(net);
cancel_delayed_work_sync(&dev->wq);
--
2.16.2
^ permalink raw reply related
* Re: KASAN: slab-out-of-bounds Read in pfkey_add
From: Dmitry Vyukov @ 2018-04-11 7:48 UTC (permalink / raw)
To: Kevin Easton
Cc: Eric Biggers, David Miller, Herbert Xu, LKML, netdev,
Steffen Klassert, syzkaller-bugs, syzbot
In-Reply-To: <20180411061824.GA28791@la.guarana.org>
On Wed, Apr 11, 2018 at 8:18 AM, Kevin Easton <kevin@guarana.org> wrote:
> On Mon, Apr 09, 2018 at 01:56:36AM -0400, Kevin Easton wrote:
>> On Sun, Apr 08, 2018 at 09:04:33PM -0700, Eric Biggers wrote:
>> ...
>> >
>> > Looks like this is going to be fixed by
>> > https://patchwork.kernel.org/patch/10327883/ ("af_key: Always verify length of
>> > provided sadb_key"), but it's not applied yet to the ipsec tree yet. Kevin, for
>> > future reference, for syzbot bugs it would be helpful to reply to the original
>> > bug report and say that a patch was sent out, or even better send the patch as a
>> > reply to the bug report email, e.g.
>> >
>> > git format-patch --in-reply-to="<001a114292fadd3e2505607060a8@google.com>"
>> >
>> > for this one (and the Message ID can be found in the syzkaller-bugs archive even
>> > if the email isn't in your inbox).
>>
>> Sure, I can do that.
>
> I recalled one reason I _didn't_ do this - the message ID is retrievable
> from the archived email, but because the archive is Google Groups the
> message recipients aren't (only masked).
>
> - Kevin
>
Hi Kevin,
This was mailed to other lists too:
To: davem@, herbert@, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, steffen.klassert@,
syzkaller-bugs@googlegroups.com
In the groups UI there is a drop down menu with "Show Original" option
which shows raw email which include Message-ID: header.
^ permalink raw reply
* Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
From: Jiri Pirko @ 2018-04-11 7:50 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stephen Hemminger, Sridhar Samudrala, davem, netdev,
virtualization, virtio-dev, jesse.brandeburg, alexander.h.duyck,
kubakici, jasowang, loseweigh
In-Reply-To: <20180411022807-mutt-send-email-mst@kernel.org>
Wed, Apr 11, 2018 at 01:28:51AM CEST, mst@redhat.com wrote:
>On Tue, Apr 10, 2018 at 02:26:08PM -0700, Stephen Hemminger wrote:
>> On Tue, 10 Apr 2018 11:59:50 -0700
>> Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>>
>> > Use the registration/notification framework supported by the generic
>> > bypass infrastructure.
>> >
>> > Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> > ---
>>
>> Thanks for doing this. Your current version has couple show stopper
>> issues.
>>
>> First, the slave device is instantly taking over the slave.
>> This doesn't allow udev/systemd to do its device rename of the slave
>> device. Netvsc uses a delayed work to workaround this.
>
>Interesting. Does this mean udev must act within a specific time window
>then?
Yeah. That is scarry. Also, wrong.
>
>> Secondly, the select queue needs to call queue selection in VF.
>> The bonding/teaming logic doesn't work well for UDP flows.
>> Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF")
>> fixed this performance problem.
>>
>> Lastly, more indirection is bad in current climate.
>>
>> I am not completely adverse to this but it needs to be fast, simple
>> and completely transparent.
^ permalink raw reply
* Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework
From: Jiri Pirko @ 2018-04-11 7:53 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Sridhar Samudrala, mst, davem, netdev, virtualization, virtio-dev,
jesse.brandeburg, alexander.h.duyck, kubakici, jasowang,
loseweigh
In-Reply-To: <20180410142608.50f15b45@xeon-e3>
Tue, Apr 10, 2018 at 11:26:08PM CEST, stephen@networkplumber.org wrote:
>On Tue, 10 Apr 2018 11:59:50 -0700
>Sridhar Samudrala <sridhar.samudrala@intel.com> wrote:
>
>> Use the registration/notification framework supported by the generic
>> bypass infrastructure.
>>
>> Signed-off-by: Sridhar Samudrala <sridhar.samudrala@intel.com>
>> ---
>
>Thanks for doing this. Your current version has couple show stopper
>issues.
>
>First, the slave device is instantly taking over the slave.
>This doesn't allow udev/systemd to do its device rename of the slave
>device. Netvsc uses a delayed work to workaround this.
Wait. Why the fact a device is enslaved has to affect the udev in any
way? If it does, smells like a bug in udev.
>
>Secondly, the select queue needs to call queue selection in VF.
>The bonding/teaming logic doesn't work well for UDP flows.
>Commit b3bf5666a510 ("hv_netvsc: defer queue selection to VF")
>fixed this performance problem.
>
>Lastly, more indirection is bad in current climate.
>
>I am not completely adverse to this but it needs to be fast, simple
>and completely transparent.
^ permalink raw reply
* [PATCH net] rds: MP-RDS may use an invalid c_path
From: Ka-Cheong Poon @ 2018-04-11 7:57 UTC (permalink / raw)
To: netdev; +Cc: santosh.shilimkar, davem, rds-devel
rds_sendmsg() calls rds_send_mprds_hash() to find a c_path to use to
send a message. Suppose the RDS connection is not yet up. In
rds_send_mprds_hash(), it does
if (conn->c_npaths == 0)
wait_event_interruptible(conn->c_hs_waitq,
(conn->c_npaths != 0));
If it is interrupted before the connection is set up,
rds_send_mprds_hash() will return a non-zero hash value. Hence
rds_sendmsg() will use a non-zero c_path to send the message. But if
the RDS connection ends up to be non-MP capable, the message will be
lost as only the zero c_path can be used.
Signed-off-by: Ka-Cheong Poon <ka-cheong.poon@oracle.com>
---
net/rds/send.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/net/rds/send.c b/net/rds/send.c
index acad042..94c7f74 100644
--- a/net/rds/send.c
+++ b/net/rds/send.c
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2006 Oracle. All rights reserved.
+ * Copyright (c) 2006, 2018 Oracle and/or its affiliates. All rights reserved.
*
* This software is available to you under a choice of one of two
* licenses. You may choose to be licensed under the terms of the GNU
@@ -1017,10 +1017,15 @@ static int rds_send_mprds_hash(struct rds_sock *rs, struct rds_connection *conn)
if (conn->c_npaths == 0 && hash != 0) {
rds_send_ping(conn, 0);
- if (conn->c_npaths == 0) {
- wait_event_interruptible(conn->c_hs_waitq,
- (conn->c_npaths != 0));
- }
+ /* The underlying connection is not up yet. Need to wait
+ * until it is up to be sure that the non-zero c_path can be
+ * used. But if we are interrupted, we have to use the zero
+ * c_path in case the connection ends up being non-MP capable.
+ */
+ if (conn->c_npaths == 0)
+ if (wait_event_interruptible(conn->c_hs_waitq,
+ conn->c_npaths != 0))
+ hash = 0;
if (conn->c_npaths == 1)
hash = 0;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload
From: Greg KH @ 2018-04-11 8:03 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: devel, samuel, netdev, johan, linux-kernel, arvind.yadav.cs,
davem
In-Reply-To: <d65d1116-7ffd-e289-5379-cc052782700d@gmail.com>
On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
>
>
> On 2018/4/11 14:41, Greg KH wrote:
> > On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> > > stir421x_fw_upload() is never called in atomic context.
> > >
> > > The call chain ending up at stir421x_fw_upload() is:
> > > [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> > >
> > > irda_usb_probe() is set as ".probe" in struct usb_driver.
> > > This function is not called in atomic context.
> > >
> > > Despite never getting called from atomic context, stir421x_fw_upload()
> > > calls mdelay() to busily wait.
> > > This is not necessary and can be replaced with usleep_range() to
> > > avoid busy waiting.
> > >
> > > This is found by a static analysis tool named DCNS written by myself.
> > > And I also manually check it.
> > >
> > > Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> > > ---
> > > drivers/staging/irda/drivers/irda-usb.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > Please, at the very least, work off of Linus's tree. There is no
> > drivers/staging/irda/ anymore :)
> >
>
> Okay, sorry.
> Could you please recommend me a right tree or its git address?
Have you looked in the MAINTAINERS file? Worst case, always use
linux-next.
greg k-h
^ permalink raw reply
* Re: [RFC PATCH net-next v5 3/4] virtio_net: Extend virtio to use VF datapath when available
From: Jiri Pirko @ 2018-04-11 8:03 UTC (permalink / raw)
To: Samudrala, Sridhar
Cc: mst, stephen, davem, netdev, virtualization, virtio-dev,
jesse.brandeburg, alexander.h.duyck, kubakici, jasowang,
loseweigh
In-Reply-To: <5d56e572-24f1-745d-49ae-c2dada5db03c@intel.com>
Wed, Apr 11, 2018 at 08:24:43AM CEST, sridhar.samudrala@intel.com wrote:
>On 4/10/2018 11:03 PM, Jiri Pirko wrote:
>> Tue, Apr 10, 2018 at 05:59:02PM CEST, sridhar.samudrala@intel.com wrote:
>> > On 4/10/2018 8:43 AM, Jiri Pirko wrote:
>> > > Tue, Apr 10, 2018 at 05:27:48PM CEST, sridhar.samudrala@intel.com wrote:
>> > > > On 4/10/2018 8:22 AM, Jiri Pirko wrote:
>> > > > > Tue, Apr 10, 2018 at 05:13:40PM CEST, sridhar.samudrala@intel.com wrote:
>> > > > > > On 4/10/2018 3:55 AM, Jiri Pirko wrote:
>> > > > > > > Mon, Apr 09, 2018 at 08:47:06PM CEST, sridhar.samudrala@intel.com wrote:
>> > > > > > > > On 4/9/2018 1:07 AM, Jiri Pirko wrote:
>> > > > > > > > > Sat, Apr 07, 2018 at 12:59:14AM CEST, sridhar.samudrala@intel.com wrote:
>> > > > > > > > > > On 4/6/2018 5:48 AM, Jiri Pirko wrote:
>> > > > > > > > > > > Thu, Apr 05, 2018 at 11:08:22PM CEST, sridhar.samudrala@intel.com wrote:
>> > > > > > > > > [...]
>> > > > > > > > >
>> > > > > > > > > > > > +static int virtnet_bypass_join_child(struct net_device *bypass_netdev,
>> > > > > > > > > > > > + struct net_device *child_netdev)
>> > > > > > > > > > > > +{
>> > > > > > > > > > > > + struct virtnet_bypass_info *vbi;
>> > > > > > > > > > > > + bool backup;
>> > > > > > > > > > > > +
>> > > > > > > > > > > > + vbi = netdev_priv(bypass_netdev);
>> > > > > > > > > > > > + backup = (child_netdev->dev.parent == bypass_netdev->dev.parent);
>> > > > > > > > > > > > + if (backup ? rtnl_dereference(vbi->backup_netdev) :
>> > > > > > > > > > > > + rtnl_dereference(vbi->active_netdev)) {
>> > > > > > > > > > > > + netdev_info(bypass_netdev,
>> > > > > > > > > > > > + "%s attempting to join bypass dev when %s already present\n",
>> > > > > > > > > > > > + child_netdev->name, backup ? "backup" : "active");
>> > > > > > > > > > > Bypass module should check if there is already some other netdev
>> > > > > > > > > > > enslaved and refuse right there.
>> > > > > > > > > > This will work for virtio-net with 3 netdev model, but this check has to be done by netvsc
>> > > > > > > > > > as its bypass_netdev is same as the backup_netdev.
>> > > > > > > > > > Will add a flag while registering with the bypass module to indicate if the driver is doing
>> > > > > > > > > > a 2 netdev or 3 netdev model and based on that flag this check can be done in bypass module
>> > > > > > > > > > for 3 netdev scenario.
>> > > > > > > > > Just let me undestand it clearly. What I expect the difference would be
>> > > > > > > > > between 2netdev and3 netdev model is this:
>> > > > > > > > > 2netdev:
>> > > > > > > > > bypass_master
>> > > > > > > > > /
>> > > > > > > > > /
>> > > > > > > > > VF_slave
>> > > > > > > > >
>> > > > > > > > > 3netdev:
>> > > > > > > > > bypass_master
>> > > > > > > > > / \
>> > > > > > > > > / \
>> > > > > > > > > VF_slave backup_slave
>> > > > > > > > >
>> > > > > > > > > Is that correct? If not, how does it look like?
>> > > > > > > > >
>> > > > > > > > >
>> > > > > > > > Looks correct.
>> > > > > > > > VF_slave and backup_slave are the original netdevs and are present in both the models.
>> > > > > > > > In the 3 netdev model, bypass_master netdev is created and VF_slave and backup_slave are
>> > > > > > > > marked as the 2 slaves of this new netdev.
>> > > > > > > You say it looks correct and in another sentence you provide completely
>> > > > > > > different description. Could you please look again?
>> > > > > > >
>> > > > > > To be exact, 2 netdev model with netvsc looks like this.
>> > > > > >
>> > > > > > netvsc_netdev
>> > > > > > /
>> > > > > > /
>> > > > > > VF_slave
>> > > > > >
>> > > > > > With virtio_net, 3 netdev model
>> > > > > >
>> > > > > > bypass_netdev
>> > > > > > / \
>> > > > > > / \
>> > > > > > VF_slave virtio_net netdev
>> > > > > Could you also mark the original netdev which is there now? is it
>> > > > > bypass_netdev or virtio_net_netdev ?
>> > > > bypass_netdev
>> > > > / \
>> > > > / \
>> > > > VF_slave virtio_net netdev (original)
>> > > That does not make sense.
>> > > 1) You diverge from the behaviour of the netvsc, where the original
>> > > netdev is a master of the VF
>> > > 2) If the original netdev is a slave, you cannot have any IP address
>> > > configured on it (well you could, but the rx_handler would eat every
>> > > incoming packet). So you will break the user bacause he would have to
>> > > move the configuration to the new master device.
>> > > This only makes sense that the original netdev becomes the master for both
>> > > netvsc and virtio_net.
>> > Forgot to mention that bypass_netdev takes over the name of the original
>> > netdev and
>> > virtio_net netdev will get the backup name.
>> What do you mean by "name"?
>
>bypass_netdev also is associated with the same pci device as the original virtio_net
>netdev via SET_NETDEV_DEV(). Also, we added ndo_get_phys_port_name() to virtio_net
>that will return _bkup when BACKUP feature is enabled.
Okay.
>
>So for ex: if virtio_net inteface was getting 'ens12' as the name assigned by udev
>without BACKUP feature, when BACKUP feature is enabled, the bypass_netdev will be
>named 'ens12' and the original virtio_net will get named as ens12n_bkup.
Got it.
I don't like the bypass_master to look differently in netvsc and
virtio_net :/ The best would be to convert netvsc to 3 netdev model and
treat them the same. The more I think about it, the more the 2 netdev
model feels wrong.
>
>
>>
>> > So the userspace network configuration doesn't need to change.
>> >
>> >
>
^ permalink raw reply
* Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload
From: Jia-Ju Bai @ 2018-04-11 8:11 UTC (permalink / raw)
To: Greg KH; +Cc: devel, samuel, netdev, johan, linux-kernel, arvind.yadav.cs,
davem
In-Reply-To: <20180411080311.GB2137@kroah.com>
On 2018/4/11 16:03, Greg KH wrote:
> On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
>>
>> On 2018/4/11 14:41, Greg KH wrote:
>>> On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
>>>> stir421x_fw_upload() is never called in atomic context.
>>>>
>>>> The call chain ending up at stir421x_fw_upload() is:
>>>> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>>>>
>>>> irda_usb_probe() is set as ".probe" in struct usb_driver.
>>>> This function is not called in atomic context.
>>>>
>>>> Despite never getting called from atomic context, stir421x_fw_upload()
>>>> calls mdelay() to busily wait.
>>>> This is not necessary and can be replaced with usleep_range() to
>>>> avoid busy waiting.
>>>>
>>>> This is found by a static analysis tool named DCNS written by myself.
>>>> And I also manually check it.
>>>>
>>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>>> ---
>>>> drivers/staging/irda/drivers/irda-usb.c | 2 +-
>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> Please, at the very least, work off of Linus's tree. There is no
>>> drivers/staging/irda/ anymore :)
>>>
>> Okay, sorry.
>> Could you please recommend me a right tree or its git address?
> Have you looked in the MAINTAINERS file? Worst case, always use
> linux-next.
>
> greg k-h
Oh, sorry, I did notice the git tree in the MAINTAINERS file.
I always used linux-stable.
Thanks for telling me this :)
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload
From: Greg KH @ 2018-04-11 8:17 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: devel, samuel, netdev, johan, linux-kernel, arvind.yadav.cs,
davem
In-Reply-To: <ecf21a6e-0e99-55dc-54be-3bd633bedbc5@gmail.com>
On Wed, Apr 11, 2018 at 04:11:00PM +0800, Jia-Ju Bai wrote:
>
>
> On 2018/4/11 16:03, Greg KH wrote:
> > On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
> > >
> > > On 2018/4/11 14:41, Greg KH wrote:
> > > > On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
> > > > > stir421x_fw_upload() is never called in atomic context.
> > > > >
> > > > > The call chain ending up at stir421x_fw_upload() is:
> > > > > [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
> > > > >
> > > > > irda_usb_probe() is set as ".probe" in struct usb_driver.
> > > > > This function is not called in atomic context.
> > > > >
> > > > > Despite never getting called from atomic context, stir421x_fw_upload()
> > > > > calls mdelay() to busily wait.
> > > > > This is not necessary and can be replaced with usleep_range() to
> > > > > avoid busy waiting.
> > > > >
> > > > > This is found by a static analysis tool named DCNS written by myself.
> > > > > And I also manually check it.
> > > > >
> > > > > Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
> > > > > ---
> > > > > drivers/staging/irda/drivers/irda-usb.c | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > Please, at the very least, work off of Linus's tree. There is no
> > > > drivers/staging/irda/ anymore :)
> > > >
> > > Okay, sorry.
> > > Could you please recommend me a right tree or its git address?
> > Have you looked in the MAINTAINERS file? Worst case, always use
> > linux-next.
> >
> > greg k-h
>
> Oh, sorry, I did notice the git tree in the MAINTAINERS file.
> I always used linux-stable.
linux-stable is almost never the tree to use as it is almost always
12000 patches behind what is in Linus's tree and about 20000 changes
behind linux-next.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH 1/2] staging: irda: Replace mdelay with usleep_range in stir421x_fw_upload
From: Jia-Ju Bai @ 2018-04-11 8:20 UTC (permalink / raw)
To: Greg KH; +Cc: devel, samuel, netdev, johan, linux-kernel, arvind.yadav.cs,
davem
In-Reply-To: <20180411081724.GA4155@kroah.com>
On 2018/4/11 16:17, Greg KH wrote:
> On Wed, Apr 11, 2018 at 04:11:00PM +0800, Jia-Ju Bai wrote:
>>
>> On 2018/4/11 16:03, Greg KH wrote:
>>> On Wed, Apr 11, 2018 at 03:17:10PM +0800, Jia-Ju Bai wrote:
>>>> On 2018/4/11 14:41, Greg KH wrote:
>>>>> On Wed, Apr 11, 2018 at 09:29:34AM +0800, Jia-Ju Bai wrote:
>>>>>> stir421x_fw_upload() is never called in atomic context.
>>>>>>
>>>>>> The call chain ending up at stir421x_fw_upload() is:
>>>>>> [1] stir421x_fw_upload() <- stir421x_patch_device() <- irda_usb_probe()
>>>>>>
>>>>>> irda_usb_probe() is set as ".probe" in struct usb_driver.
>>>>>> This function is not called in atomic context.
>>>>>>
>>>>>> Despite never getting called from atomic context, stir421x_fw_upload()
>>>>>> calls mdelay() to busily wait.
>>>>>> This is not necessary and can be replaced with usleep_range() to
>>>>>> avoid busy waiting.
>>>>>>
>>>>>> This is found by a static analysis tool named DCNS written by myself.
>>>>>> And I also manually check it.
>>>>>>
>>>>>> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
>>>>>> ---
>>>>>> drivers/staging/irda/drivers/irda-usb.c | 2 +-
>>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>> Please, at the very least, work off of Linus's tree. There is no
>>>>> drivers/staging/irda/ anymore :)
>>>>>
>>>> Okay, sorry.
>>>> Could you please recommend me a right tree or its git address?
>>> Have you looked in the MAINTAINERS file? Worst case, always use
>>> linux-next.
>>>
>>> greg k-h
>> Oh, sorry, I did notice the git tree in the MAINTAINERS file.
>> I always used linux-stable.
> linux-stable is almost never the tree to use as it is almost always
> 12000 patches behind what is in Linus's tree and about 20000 changes
> behind linux-next.
>
Okay, I now know why many of my patches were not replied.
I should choose correct tree in the MAINTAINERS file.
Thanks :)
Best wishes,
Jia-Ju Bai
^ permalink raw reply
* Re: [patch net] devlink: convert occ_get op to separate registration
From: gregkh @ 2018-04-11 8:46 UTC (permalink / raw)
To: Sasha Levin
Cc: David Miller, jiri@resnulli.us, jiri@mellanox.com,
netdev@vger.kernel.org, idosch@mellanox.com,
stable@vger.kernel.org
In-Reply-To: <20180410204329.GJ2341@sasha-vm>
On Tue, Apr 10, 2018 at 08:43:31PM +0000, Sasha Levin wrote:
> >Bots are starting to overwhelm actual content from human beings
> >on this list, and I want to put my foot on the brake right now
> >before it gets even more out of control.
>
> I think we're just hitting the limitations of using a mailing list. Bots
> aren't around to spam everyone for fun, right?
>
> 0day bot is spammy because patches fail to compile, syzbot is spammy
> because we have tons of bugs users can hit and the stable bot is
> spammy because we miss lots of commits that should be in stable.
>
> As the kernel grows, not only the codebase is expanding but also the
> tooling around it. While spammy, bots provide valuable input that in the
> past would take blood, sweat and tears to acquire. We just need a better
> way to consume it rather than blocking off these inputs.
As one of the primary abusers of the "I send too much email" flood of
mess, I'm going to start cutting down on what type of message I respond
to a public vger list from now on. I'll write more on the stable@ list
about this, but for your individual patches like this, how about just
responding to the developers themselves and not a public list? I do
that when I commit a patch to my tree, stripping out lists, as it's not
a useful message for anyone other than the person who wrote the patch
and the reviewers.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH net-next] netns: filter uevents correctly
From: Christian Brauner @ 2018-04-11 9:09 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Kirill Tkhai, davem, gregkh, netdev, linux-kernel, avagin, serge
In-Reply-To: <87in8zumpd.fsf@xmission.com>
On Tue, Apr 10, 2018 at 10:04:46AM -0500, Eric W. Biederman wrote:
> Christian Brauner <christian.brauner@canonical.com> writes:
>
> > On Mon, Apr 09, 2018 at 06:21:31PM -0500, Eric W. Biederman wrote:
> >> Christian Brauner <christian.brauner@canonical.com> writes:
> >>
> >> > On Thu, Apr 05, 2018 at 10:59:49PM -0500, Eric W. Biederman wrote:
> >> >> Christian Brauner <christian.brauner@canonical.com> writes:
> >> >>
> >> >> > On Thu, Apr 05, 2018 at 05:26:59PM +0300, Kirill Tkhai wrote:
> >> >> >> On 05.04.2018 17:07, Christian Brauner wrote:
> >> >> >> > On Thu, Apr 05, 2018 at 04:01:03PM +0300, Kirill Tkhai wrote:
> >> >> >> >> On 04.04.2018 22:48, Christian Brauner wrote:
> >> >> >> >>> commit 07e98962fa77 ("kobject: Send hotplug events in all network namespaces")
> >> >> >> >>>
> >> >> >> >>> enabled sending hotplug events into all network namespaces back in 2010.
> >> >> >> >>> Over time the set of uevents that get sent into all network namespaces has
> >> >> >> >>> shrunk. We have now reached the point where hotplug events for all devices
> >> >> >> >>> that carry a namespace tag are filtered according to that namespace.
> >> >> >> >>>
> >> >> >> >>> Specifically, they are filtered whenever the namespace tag of the kobject
> >> >> >> >>> does not match the namespace tag of the netlink socket. One example are
> >> >> >> >>> network devices. Uevents for network devices only show up in the network
> >> >> >> >>> namespaces these devices are moved to or created in.
> >> >> >> >>>
> >> >> >> >>> However, any uevent for a kobject that does not have a namespace tag
> >> >> >> >>> associated with it will not be filtered and we will *try* to broadcast it
> >> >> >> >>> into all network namespaces.
> >> >> >> >>>
> >> >> >> >>> The original patchset was written in 2010 before user namespaces were a
> >> >> >> >>> thing. With the introduction of user namespaces sending out uevents became
> >> >> >> >>> partially isolated as they were filtered by user namespaces:
> >> >> >> >>>
> >> >> >> >>> net/netlink/af_netlink.c:do_one_broadcast()
> >> >> >> >>>
> >> >> >> >>> if (!net_eq(sock_net(sk), p->net)) {
> >> >> >> >>> if (!(nlk->flags & NETLINK_F_LISTEN_ALL_NSID))
> >> >> >> >>> return;
> >> >> >> >>>
> >> >> >> >>> if (!peernet_has_id(sock_net(sk), p->net))
> >> >> >> >>> return;
> >> >> >> >>>
> >> >> >> >>> if (!file_ns_capable(sk->sk_socket->file, p->net->user_ns,
> >> >> >> >>> CAP_NET_BROADCAST))
> >> >> >> >>> j return;
> >> >> >> >>> }
> >> >> >> >>>
> >> >> >> >>> The file_ns_capable() check will check whether the caller had
> >> >> >> >>> CAP_NET_BROADCAST at the time of opening the netlink socket in the user
> >> >> >> >>> namespace of interest. This check is fine in general but seems insufficient
> >> >> >> >>> to me when paired with uevents. The reason is that devices always belong to
> >> >> >> >>> the initial user namespace so uevents for kobjects that do not carry a
> >> >> >> >>> namespace tag should never be sent into another user namespace. This has
> >> >> >> >>> been the intention all along. But there's one case where this breaks,
> >> >> >> >>> namely if a new user namespace is created by root on the host and an
> >> >> >> >>> identity mapping is established between root on the host and root in the
> >> >> >> >>> new user namespace. Here's a reproducer:
> >> >> >> >>>
> >> >> >> >>> sudo unshare -U --map-root
> >> >> >> >>> udevadm monitor -k
> >> >> >> >>> # Now change to initial user namespace and e.g. do
> >> >> >> >>> modprobe kvm
> >> >> >> >>> # or
> >> >> >> >>> rmmod kvm
> >> >> >> >>>
> >> >> >> >>> will allow the non-initial user namespace to retrieve all uevents from the
> >> >> >> >>> host. This seems very anecdotal given that in the general case user
> >> >> >> >>> namespaces do not see any uevents and also can't really do anything useful
> >> >> >> >>> with them.
> >> >> >> >>>
> >> >> >> >>> Additionally, it is now possible to send uevents from userspace. As such we
> >> >> >> >>> can let a sufficiently privileged (CAP_SYS_ADMIN in the owning user
> >> >> >> >>> namespace of the network namespace of the netlink socket) userspace process
> >> >> >> >>> make a decision what uevents should be sent.
> >> >> >> >>>
> >> >> >> >>> This makes me think that we should simply ensure that uevents for kobjects
> >> >> >> >>> that do not carry a namespace tag are *always* filtered by user namespace
> >> >> >> >>> in kobj_bcast_filter(). Specifically:
> >> >> >> >>> - If the owning user namespace of the uevent socket is not init_user_ns the
> >> >> >> >>> event will always be filtered.
> >> >> >> >>> - If the network namespace the uevent socket belongs to was created in the
> >> >> >> >>> initial user namespace but was opened from a non-initial user namespace
> >> >> >> >>> the event will be filtered as well.
> >> >> >> >>> Put another way, uevents for kobjects not carrying a namespace tag are now
> >> >> >> >>> always only sent to the initial user namespace. The regression potential
> >> >> >> >>> for this is near to non-existent since user namespaces can't really do
> >> >> >> >>> anything with interesting devices.
> >> >> >> >>>
> >> >> >> >>> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> >> >> >> >>> ---
> >> >> >> >>> lib/kobject_uevent.c | 10 +++++++++-
> >> >> >> >>> 1 file changed, 9 insertions(+), 1 deletion(-)
> >> >> >> >>>
> >> >> >> >>> diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
> >> >> >> >>> index 15ea216a67ce..cb98cddb6e3b 100644
> >> >> >> >>> --- a/lib/kobject_uevent.c
> >> >> >> >>> +++ b/lib/kobject_uevent.c
> >> >> >> >>> @@ -251,7 +251,15 @@ static int kobj_bcast_filter(struct sock *dsk, struct sk_buff *skb, void *data)
> >> >> >> >>> return sock_ns != ns;
> >> >> >> >>> }
> >> >> >> >>>
> >> >> >> >>> - return 0;
> >> >> >> >>> + /*
> >> >> >> >>> + * The kobject does not carry a namespace tag so filter by user
> >> >> >> >>> + * namespace below.
> >> >> >> >>> + */
> >> >> >> >>> + if (sock_net(dsk)->user_ns != &init_user_ns)
> >> >> >> >>> + return 1;
> >> >> >> >>> +
> >> >> >> >>> + /* Check if socket was opened from non-initial user namespace. */
> >> >> >> >>> + return sk_user_ns(dsk) != &init_user_ns;
> >> >> >> >>> }
> >> >> >> >>> #endif
> >> >> >> >>
> >> >> >> >> So, this prohibits to listen events of all devices except network-related
> >> >> >> >> in containers? If it's so, I don't think it's a good solution. Uevents is not
> >> >> >> >
> >> >> >> > No, this is not correct: As it is right now *without my patch* no
> >> >> >> > non-initial user namespace is receiving *any uevents* but those
> >> >> >> > specifically namespaced such as those for network devices. This patch
> >> >> >> > doesn't change that at all. The commit message outlines this in detail
> >> >> >> > how this comes about.
> >> >> >> > There is only one case where this currently breaks and this is as I
> >> >> >> > outlined explicitly in my commit message when you create a new user
> >> >> >> > namespace and map container(0) -> host(0). This patch fixes this.
> >> >> >>
> >> >> >> Could you please point the place, where non-initial user namespaces are filtered?
> >> >> >> I only see the kobj_bcast_filter() logic, and it used to return 0, which means "accepted".
> >> >> >> Now it will return 1 sometimes.
> >> >> >
> >> >> > Oh sure, it's in the commit message though. The callchain is
> >> >> > lib/kobject_uevent.c:kobject_uevent_net_broadcast() ->
> >> >> > nnet/netlink/af_netlink.c:netlink_broadcast_filtered() ->
> >> >> > net/netlink/af_netlink.c:do_one_broadcast():
> >> >> >
> >> >> > This codepiece will check whether the openened socket holds
> >> >> > CAP_NET_BROADCAST in the user namespace of the target network namespace
> >> >> > which it won't because we don't have device namespaces and all devices
> >> >> > belong to the initial set of namespaces.
> >> >> >
> >> >> > if (!file_ns_capable(sk->sk_socket->file, p->net->user_ns,
> >> >> > CAP_NET_BROADCAST))
> >> >> > j return;
> >> >> >
> >> >>
> >> >> The above that only applies if someone has set NETLINK_F_LISTEN_ALL_NSID
> >> >> on their socket and has had someone with the appropriate privileges
> >> >> assign a peerrnetid.
> >> >>
> >> >> All of which is to say that file_ns_capable is not nearly as applicable
> >> >> as it might be, and if you can pass the other two checks I think it is
> >> >> pointless (because the peernet attributes are not generated for
> >> >> kobj_uevents) but valid to receive events from outside your network
> >> >> namespace.
> >> >>
> >> >>
> >> >> I might be missing something but I don't see anything excluding network
> >> >> namespaces owned by !init_user_ns excluded from the kobject_uevent
> >> >> logic.
> >> >>
> >> >> The uevent_sock_list has one entry per network namespace. And
> >> >> kobject_uevent_net_broacast appears to walk each one.
> >> >>
> >> >> I had a memory of filtering uevent messages and I had a memory
> >> >> that the netlink_has_listeners had a per network namespace effect.
> >> >> Neither seems true from my inspection of the code tonight.
> >> >>
> >> >> If we are not filtering ordinary uevents at least at the user namespace
> >> >> level that does seem to be at least a nuisance.
> >> >>
> >> >>
> >> >> Christian can you dig a little deeper into this. I have a feeling that
> >> >> there are some real efficiency improvements that we could make to
> >> >> kobject_uevent_net_broadcast if nothing else.
> >> >>
> >> >> Perhaps you could see where uevents are broadcast by poking
> >> >> the sysfs uevent of an existing device, and triggering a retransmit.
> >> >
> >> > @Eric, so I did some intensive testing over the weekend and forget everything I
> >> > said before. Uevents are not filtered by the kernel at the moment. This is
> >> > currently - apart from network devices - a pure userspace thing. Specifically,
> >> > anyone on the host can listen to all uevents from everywhere. It's neither
> >> > filtered by user nor by network namespace. The fact that I used
> >> >
> >> > udevadm --debug monitor
> >> >
> >> > to test my prior hypothesis was what led me to believe that uevents are already
> >> > correctly filtered.
> >> > Instead, what is actually happening is that every udev implementation out there
> >> > is discarding uevents that were send by uids != 0 in the CMSG_DATA.
> >> > Specifically,
> >>
> >> Yes. I remember something of the sort. I believe udev also filters to
> >> ensure that the netlink port id == 0 to ensure the message came from
> >> the kernel and was not spoofed in any way.
> >>
> >> > - legacy standalone udevd:
> >> > https://git.kernel.org/pub/scm/linux/hotplug/udev.git/snapshot/udev-062.tar.gz
> >> > - eudevd
> >> > https://github.com/gentoo/eudev/blob/6f630d32bf494a457171b3f99e329592497bf271/src/libudev/libudev-monitor.c#L645
> >> > - systemd-udevd
> >> > https://github.com/systemd/systemd/blob/e89ab7f219a399ab719c78cf43c07c0da60bd151/src/libudev/libudev-monitor.c#L656
> >> > - ueventd (Android)
> >> > https://android.googlesource.com/platform/system/core.git/+/android-8.1.0_r22/libcutils/uevent.c#81
> >> >
> >> > For all of those I could trace this behavior back to the first released
> >> > version. (To be precise, for legacy udevd that eventually became systemd-udevd
> >> > I could trace it back to the first version that is still available on
> >> > git.kernel.org which is 062. Since eudevd is a fork of systemd-udevd it is
> >> > trivially true that it has the same behavior from the beginning.)
> >> >
> >> > In any case, userspace udev is not making use of uevents at all right now since
> >> > any uid != 0 events are **explicitly** discarded.
> >> > The fact that you receive uevents for
> >> >
> >> > sudo unshare -U --map-root -n
> >> > udevadm --debug monitor
> >> >
> >> > is simply explained by the fact that container(0) <=> host(0) at which point
> >> > the uid in CMSG_DATA will be 0 in the new user namespace and udev will not
> >> > discard it.
> >> > The use case for receiving uevents in containers/user namespaces is definitely
> >> > there but that's what the uevent injection patch series was for that we merged.
> >> > This is a much safer and saner solution.
> >> > The fact that all udev implementations filter uevents by uid != 0 very much
> >> > seems like a security mechanism in userspace that we probably should provide by
> >> > isolating uevents based on user and/or network namespaces.
> >>
> >> So in summary. Uevents are filtered in a user namespace (by userspace)
> >> because the received uid != 0. It instead == 65534 == "nobody" because
> >> the global root uid is not mapped.
> >
> > Exactly.
> >
> >>
> >> Which means that we can modify the kernel to not send to all network
> >> namespaces whose user_ns != &init_user_ns because we know that userspace
> >> will ignore the message because of the uid anyway. Which means when
> >
> > Yes.
> >
> >> net-next reopens you can send that patch. But please base it on just
> >> not including network namespaces in the list, as that is much more
> >> efficient than adding more conditions to the filter.
> >
> > I'll send a patch out once net-next reopens. I'll also make sure to
> > inform all udev userspace implementations of the change. It won't affect
> > them but it is nice for them to know that they're safer now.
>
> The real danger is in a user namespace or in a container really is too
> many daemons responding to events will generate a thundering hurd of
> activity when there is really nothing to do.
>
> > Something like this (Proper commit message and so on will be added once
> > I sent this out.):
>
> Exactly.
>
> I would make the comment say something like: "ignore all but the initial
> user namespace".
Yeah, agreed.
But I think the patch is not complete. To guarantee that no non-initial
user namespace actually receives uevents we need to:
1. only sent uevents to uevent sockets that are located in network
namespaces that are owned by init_user_ns
2. filter uevents that are sent to sockets in mc_list that have opened a
uevent socket that is owned by init_user_ns *from* a
non-init_user_ns
We account for 1. by only recording uevent sockets in the global uevent
socket list who are owned by init_user_ns.
But to account for 2. we need to filter by the user namespace who owns
the socket in mc_list. So in addition to that we also need to slightly
change the filter logic in kobj_bcast_filter() I think:
diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
index 22a2c1a98b8f..064d7d29ace5 100644
--- a/lib/kobject_uevent.c
+++ b/lib/kobject_uevent.c
@@ -251,7 +251,8 @@ static int kobj_bcast_filter(struct sock *dsk, struct sk_buff *skb, void *data)
return sock_ns != ns;
}
- return 0;
+ /* Check if socket was opened from non-initial user namespace. */
+ return sk_user_ns(dsk) != &init_user_ns;
}
#endif
But correct me if I'm wrong.
Christian
>
> Eric
>
>
> > From 68d3c27435520cd600874999b6d9d17572854a7a Mon Sep 17 00:00:00 2001
> > From: Christian Brauner <christian.brauner@ubuntu.com>
> > Date: Tue, 10 Apr 2018 11:56:49 +0200
> > Subject: [PATCH] netns: restrict uevents to initial user namespace
> >
> > /* Here'll be a useful commit message describing in detail what's
> > * happening once I sent it to net-next. */
> >
> > Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
> > ---
> > lib/kobject_uevent.c | 21 +++++++++++++++------
> > 1 file changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/kobject_uevent.c b/lib/kobject_uevent.c
> > index 15ea216a67ce..22a2c1a98b8f 100644
> > --- a/lib/kobject_uevent.c
> > +++ b/lib/kobject_uevent.c
> > @@ -703,9 +703,16 @@ static int uevent_net_init(struct net *net)
> >
> > net->uevent_sock = ue_sk;
> >
> > - mutex_lock(&uevent_sock_mutex);
> > - list_add_tail(&ue_sk->list, &uevent_sock_list);
> > - mutex_unlock(&uevent_sock_mutex);
> > + /*
> > + * Only sent uevents to uevent sockets that are located in network
> > + * namespaces owned by the initial user namespace.
> > + */
> > + if (sock_net(ue_sk->sk)->user_ns == &init_user_ns) {
> > + mutex_lock(&uevent_sock_mutex);
> > + list_add_tail(&ue_sk->list, &uevent_sock_list);
> > + mutex_unlock(&uevent_sock_mutex);
> > + }
> > +
> > return 0;
> > }
> >
> > @@ -713,9 +720,11 @@ static void uevent_net_exit(struct net *net)
> > {
> > struct uevent_sock *ue_sk = net->uevent_sock;
> >
> > - mutex_lock(&uevent_sock_mutex);
> > - list_del(&ue_sk->list);
> > - mutex_unlock(&uevent_sock_mutex);
> > + if (sock_net(ue_sk->sk)->user_ns == &init_user_ns) {
> > + mutex_lock(&uevent_sock_mutex);
> > + list_del(&ue_sk->list);
> > + mutex_unlock(&uevent_sock_mutex);
> > + }
> >
> > netlink_kernel_release(ue_sk->sk);
> > kfree(ue_sk);
^ permalink raw reply related
* [iproute PATCH] iproute: Abort if nexthop cannot be parsed
From: Jakub Sitnicki @ 2018-04-11 9:43 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Attempt to add a multipath route where a nexthop definition refers to a
non-existent device causes 'ip' to crash and burn due to stack buffer
overflow:
# ip -6 route add fd00::1/64 nexthop dev fake1
Cannot find device "fake1"
Cannot find device "fake1"
Cannot find device "fake1"
...
Segmentation fault (core dumped)
Don't ignore errors from the helper routine that parses the nexthop
definition, and abort immediately if parsing fails.
Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
---
ip/iproute.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/ip/iproute.c b/ip/iproute.c
index 1d8fd815..44351bc5 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -1038,7 +1038,10 @@ static int parse_nexthops(struct nlmsghdr *n, struct rtmsg *r,
memset(rtnh, 0, sizeof(*rtnh));
rtnh->rtnh_len = sizeof(*rtnh);
rta->rta_len += rtnh->rtnh_len;
- parse_one_nh(n, r, rta, rtnh, &argc, &argv);
+ if (parse_one_nh(n, r, rta, rtnh, &argc, &argv)) {
+ fprintf(stderr, "Error: cannot parse nexthop\n");
+ exit(-1);
+ }
rtnh = RTNH_NEXT(rtnh);
}
--
2.14.3
^ permalink raw reply related
* [PATCH] lan78xx: Correctly indicate invalid OTP
From: Phil Elwell @ 2018-04-11 9:59 UTC (permalink / raw)
To: Woojung Huh, Microchip Linux Driver Support, netdev, linux-usb,
linux-kernel
Cc: Phil Elwell
lan78xx_read_otp tries to return -EINVAL in the event of invalid OTP
content, but the value gets overwritten before it is returned and the
read goes ahead anyway. Make the read conditional as it should be
and preserve the error code.
Signed-off-by: Phil Elwell <phil@raspberrypi.org>
---
drivers/net/usb/lan78xx.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
index 55a78eb..32cf217 100644
--- a/drivers/net/usb/lan78xx.c
+++ b/drivers/net/usb/lan78xx.c
@@ -928,7 +928,8 @@ static int lan78xx_read_otp(struct lan78xx_net *dev, u32 offset,
offset += 0x100;
else
ret = -EINVAL;
- ret = lan78xx_read_raw_otp(dev, offset, length, data);
+ if (!ret)
+ ret = lan78xx_read_raw_otp(dev, offset, length, data);
}
return ret;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] net: tipc: Replace GFP_ATOMIC with GFP_KERNEL in tipc_mon_create
From: Ying Xue @ 2018-04-11 10:11 UTC (permalink / raw)
To: Jia-Ju Bai, jon.maloy, davem; +Cc: netdev, tipc-discussion, linux-kernel
In-Reply-To: <1523323053-28127-1-git-send-email-baijiaju1990@gmail.com>
On 04/10/2018 09:17 AM, Jia-Ju Bai wrote:
> tipc_mon_create() is never called in atomic context.
>
> The call chain ending up at dn_route_init() is:
Sorry, I don't think there is any relationship between the following
call chain with dn_route_init().
> [1] tipc_mon_create() <- tipc_enable_bearer() <- tipc_nl_bearer_enable()
> tipc_nl_bearer_enable() calls rtnl_lock(), which indicates this function
> is not called in atomic context.
>
> Despite never getting called from atomic context,
> tipc_mon_create() calls kzalloc() with GFP_ATOMIC,
> which does not sleep for allocation.
> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
> which can sleep and improve the possibility of sucessful allocation.
s/sucessful/successful
>
> This is found by a static analysis tool named DCNS written by myself.
> And I also manually check it.
>
> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>> ---
> v2:
> * Modify the description of GFP_ATOMIC in v1.
> Thank Eric for good advice.
> ---
> net/tipc/monitor.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c
> index 9e109bb..9714d80 100644
> --- a/net/tipc/monitor.c
> +++ b/net/tipc/monitor.c
> @@ -604,9 +604,9 @@ int tipc_mon_create(struct net *net, int bearer_id)
> if (tn->monitors[bearer_id])
> return 0;
>
> - mon = kzalloc(sizeof(*mon), GFP_ATOMIC);
> - self = kzalloc(sizeof(*self), GFP_ATOMIC);
> - dom = kzalloc(sizeof(*dom), GFP_ATOMIC);
> + mon = kzalloc(sizeof(*mon), GFP_KERNEL);
> + self = kzalloc(sizeof(*self), GFP_KERNEL);
> + dom = kzalloc(sizeof(*dom), GFP_KERNEL);
> if (!mon || !self || !dom) {
> kfree(mon);
> kfree(self);
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply
* Re: [RFC bpf-next v2 8/8] bpf: add documentation for eBPF helpers (58-64)
From: Jesper Dangaard Brouer @ 2018-04-11 10:17 UTC (permalink / raw)
To: Quentin Monnet
Cc: brouer, daniel, ast, netdev, oss-drivers, linux-doc, linux-man,
John Fastabend
In-Reply-To: <20180410144157.4831-9-quentin.monnet@netronome.com>
On Tue, 10 Apr 2018 15:41:57 +0100
Quentin Monnet <quentin.monnet@netronome.com> wrote:
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 7343af4196c8..db090ad03626 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -1250,6 +1250,51 @@ union bpf_attr {
> * Return
> * 0 on success, or a negative error in case of failure.
> *
> + * int bpf_redirect_map(struct bpf_map *map, u32 key, u64 flags)
> + * Description
> + * Redirect the packet to the endpoint referenced by *map* at
> + * index *key*. Depending on its type, his *map* can contain
> + * references to net devices (for forwarding packets through other
> + * ports), or to CPUs (for redirecting XDP frames to another CPU;
> + * but this is not fully implemented as of this writing).
Stating that CPUMAP redirect "is not fully implemented" is confusing.
The issue is that CPUMAP only works for "native" XDP.
What about saying:
"[...] or to CPUs (for redirecting XDP frames to another CPU;
but this is only implemented for native XDP as of this writing)"
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v2] net: tipc: Replace GFP_ATOMIC with GFP_KERNEL in tipc_mon_create
From: Jia-Ju Bai @ 2018-04-11 10:18 UTC (permalink / raw)
To: Ying Xue, jon.maloy, davem; +Cc: netdev, tipc-discussion, linux-kernel
In-Reply-To: <73c0b7cb-5148-9c38-d45b-750f711b5570@windriver.com>
On 2018/4/11 18:11, Ying Xue wrote:
> On 04/10/2018 09:17 AM, Jia-Ju Bai wrote:
>> tipc_mon_create() is never called in atomic context.
>>
>> The call chain ending up at dn_route_init() is:
> Sorry, I don't think there is any relationship between the following
> call chain with dn_route_init().
>
>> [1] tipc_mon_create() <- tipc_enable_bearer() <- tipc_nl_bearer_enable()
>> tipc_nl_bearer_enable() calls rtnl_lock(), which indicates this function
>> is not called in atomic context.
>>
>> Despite never getting called from atomic context,
>> tipc_mon_create() calls kzalloc() with GFP_ATOMIC,
>> which does not sleep for allocation.
>> GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
>> which can sleep and improve the possibility of sucessful allocation.
> s/sucessful/successful
Thanks for your reply.
I am sorry for my mistakes.
I will revised the text and send a V3.
Jia-Ju Bai
^ permalink raw reply
* [PATCH v3] net: tipc: Replace GFP_ATOMIC with GFP_KERNEL in tipc_mon_create
From: Jia-Ju Bai @ 2018-04-11 10:24 UTC (permalink / raw)
To: jon.maloy, ying.xue, davem
Cc: netdev, tipc-discussion, linux-kernel, Jia-Ju Bai
tipc_mon_create() is never called in atomic context.
The call chain ending up at tipc_mon_create() is:
[1] tipc_mon_create() <- tipc_enable_bearer() <- tipc_nl_bearer_enable()
tipc_nl_bearer_enable() calls rtnl_lock(), which indicates this function
is not called in atomic context.
Despite never getting called from atomic context,
tipc_mon_create() calls kzalloc() with GFP_ATOMIC,
which does not sleep for allocation.
GFP_ATOMIC is not necessary and can be replaced with GFP_KERNEL,
which can sleep and improve the possibility of successful allocation.
This is found by a static analysis tool named DCNS written by myself.
And I also manually check it.
Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com>
---
v2:
* Modify the description of GFP_ATOMIC in v1.
Thank Eric for good advice.
v3:
* Modify wrong text in description in v2.
Thank Ying for good advice.
---
net/tipc/monitor.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/net/tipc/monitor.c b/net/tipc/monitor.c
index 9e109bb..9714d80 100644
--- a/net/tipc/monitor.c
+++ b/net/tipc/monitor.c
@@ -604,9 +604,9 @@ int tipc_mon_create(struct net *net, int bearer_id)
if (tn->monitors[bearer_id])
return 0;
- mon = kzalloc(sizeof(*mon), GFP_ATOMIC);
- self = kzalloc(sizeof(*self), GFP_ATOMIC);
- dom = kzalloc(sizeof(*dom), GFP_ATOMIC);
+ mon = kzalloc(sizeof(*mon), GFP_KERNEL);
+ self = kzalloc(sizeof(*self), GFP_KERNEL);
+ dom = kzalloc(sizeof(*dom), GFP_KERNEL);
if (!mon || !self || !dom) {
kfree(mon);
kfree(self);
--
1.9.1
^ permalink raw reply related
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