* RE: [PATCH net-next] net: hns3: Refactors "reset" handling code in HCLGE layer of HNS3 driver
From: Salil Mehta @ 2017-12-02 18:06 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org,
Zhuangyuzeng (Yisen), lipeng (Y),
mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linuxarm
In-Reply-To: <20171201134415.GD22599-g2DYL2Zd6BY@public.gmane.org>
Hi Andrew,
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew-g2DYL2Zd6BY@public.gmane.org]
> Sent: Friday, December 01, 2017 1:44 PM
> To: Salil Mehta <salil.mehta-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Cc: davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org; Zhuangyuzeng (Yisen)
> <yisen.zhuang-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>; lipeng (Y) <lipeng321-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>;
> mehta.salil.lnk-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-
> kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Linuxarm
> <linuxarm-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Subject: Re: [PATCH net-next] net: hns3: Refactors "reset" handling
> code in HCLGE layer of HNS3 driver
>
> On Fri, Dec 01, 2017 at 03:37:44AM +0000, Salil Mehta wrote:
> > This patch refactors the code of the reset feature in HCLGE layer
> > of HNS3 PF driver. Prime motivation to do this change is:
> > 1. To reduce the time for which common miscellaneous Vector 0
> > interrupt is disabled because of the reset.
> > 2. Simplification of reset request submission and pending reset
> > logic.
> > 3. Simplification of the common miscellaneous interrupt handler
> > routine(for Vector 0) used to handle reset and other sources
> > of Vector 0 interrupt.
> >
> > To achieve above below few things have been done:
> > 1. Interrupt is disabled while common miscellaneous interrupt
> > handler is entered and re-enabled before it is exit. This
> > reduces the interrupt handling latency as compared to older
> > interrupt handling scheme where interrupt was being disabled
> > in interrupt handler context and re-enabled in task context
> > some time later.
> > 2. Introduces new reset service task for honoring software reset
> > requests like from network stack related to timeout and serving
> > the pending reset request(to reset the driver and associated
> > clients).
> > 3. Made Miscellaneous interrupt handler more generic to handle
> > all sources including reset interrupt source.
>
> Hi Salil
>
> This is a rather large patch. Can you break it up? It seems like you
> should be able to break it up into at least three parts, maybe more.
>
> You are aiming to have small patches which are obviously correct. It
> is much easier to review than one big patch which is not obvious at
> all.
Ok. No issues. I will try to fix this in V2 version.
>
> Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: netfilter: xt_bpf: Fix XT_BPF_MODE_FD_PINNED mode of 'xt_bpf_info_v1'
From: Al Viro @ 2017-12-02 18:48 UTC (permalink / raw)
To: Daniel Borkmann
Cc: Kees Cook, Shmulik Ladkani, Willem de Bruijn, Pablo Neira Ayuso,
Linus Torvalds, David Miller, LKML, Network Development,
Christoph Hellwig, Thomas Garnier, Jann Horn
In-Reply-To: <7bbe72a8-dbbe-3343-765d-cc53eb40e0cd@iogearbox.net>
On Fri, Dec 01, 2017 at 09:47:00PM +0100, Daniel Borkmann wrote:
> > Might want to replace security_path_mknod() with something saner, while we are
> > at it.
> >
> > Objections?
>
> No, thanks for looking into this, and sorry for this fugly hack! :( Not
> that this doesn't make it any better, but I think back then I took it
> over from mqueue implementation ... should have known better and looking
> into making this generic instead, sigh. The above looks good to me, so
> no objections from my side and thanks for working on it!
>
> > PS: mqueue.c would also benefit from such primitive - do_create() there would
> > simply pass attr as callback's argument into vfs_mkobj(), with callback being
> > the guts of mqueue_create()...
OK... See vfs.git#untested.mkobj; it really needs testing, though - mq_open(2)
passes LTP tests, but that's not saying much, and BPF side is completely
untested.
^ permalink raw reply
* [PATCH 00/10] Handle return value of platform_get_*
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
- The platform_get_*_*() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq_byname() error
checking for zero is not correct.
- The change is to call free_netdev(), If of_match_node() will fail.
- Handle return value of platform_get_resource()
Arvind Yadav (10):
[PATCH 01/10] net: bcmgenet: Fix platform_get_irq's error checking
[PATCH 02/10] net: bcmgenet: free netdev on of_match_node() error
[PATCH 03/10] net: ezchip: nps_enet: Fix platform_get_irq's error checking
[PATCH 04/10] can: xilinx: Handle return value of platform_get_irq
[PATCH 05/10] net: ethernet: i825xx: Fix platform_get_irq's error checking
[PATCH 06/10] net: ethernet: natsemi: Handle return value of platform_get_irq
[PATCH 07/10] net: ethernet: smsc: Handle return value of platform_get_irq
[PATCH 08/10] net: fjes: Handle return value of platform_get_irq and platform_get_resource
[PATCH 09/10] net: ethernet: korina: Handle return value of platform_get_irq_byname
[PATCH 10/10] net: ethernet: cpmac: Handle return value of platform_get_irq_byname
drivers/net/can/xilinx_can.c | 4 ++++
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 8 +++++---
drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
drivers/net/ethernet/i825xx/sni_82596.c | 3 ++-
drivers/net/ethernet/korina.c | 9 +++++++++
drivers/net/ethernet/natsemi/jazzsonic.c | 5 +++++
drivers/net/ethernet/smsc/smc911x.c | 5 +++++
drivers/net/ethernet/ti/cpmac.c | 4 ++++
drivers/net/fjes/fjes_main.c | 10 ++++++++++
9 files changed, 47 insertions(+), 8 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 01/10] net: bcmgenet: Fix platform_get_irq's error checking
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
The platform_get_irq() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq() error checking
for zero is not correct.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index 24b4f4c..e2f1268 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3371,7 +3371,7 @@ static int bcmgenet_probe(struct platform_device *pdev)
priv->irq0 = platform_get_irq(pdev, 0);
priv->irq1 = platform_get_irq(pdev, 1);
priv->wol_irq = platform_get_irq(pdev, 2);
- if (!priv->irq0 || !priv->irq1) {
+ if (priv->irq0 <= 0 || priv->irq1 <= 0 || priv->wol_irq <= 0) {
dev_err(&pdev->dev, "can't find IRQs\n");
err = -EINVAL;
goto err;
--
2.7.4
^ permalink raw reply related
* [PATCH 02/10] net: bcmgenet: free netdev on of_match_node() error
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
The change is to call free_netdev(), If of_match_node() will fail.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/broadcom/genet/bcmgenet.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/genet/bcmgenet.c b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
index e2f1268..e0a8f79 100644
--- a/drivers/net/ethernet/broadcom/genet/bcmgenet.c
+++ b/drivers/net/ethernet/broadcom/genet/bcmgenet.c
@@ -3363,8 +3363,10 @@ static int bcmgenet_probe(struct platform_device *pdev)
if (dn) {
of_id = of_match_node(bcmgenet_match, dn);
- if (!of_id)
- return -EINVAL;
+ if (!of_id) {
+ err = -EINVAL;
+ goto err;
+ }
}
priv = netdev_priv(dev);
--
2.7.4
^ permalink raw reply related
* [PATCH 03/10] net: ezchip: nps_enet: Fix platform_get_irq's error checking
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: netdev, linux-kernel, linux-arm-kernel
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
The platform_get_irq() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq() error checking
for zero is not correct. And remove unnecessary check for free_netdev().
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/ezchip/nps_enet.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ezchip/nps_enet.c b/drivers/net/ethernet/ezchip/nps_enet.c
index 659f1ad..82dc6d0 100644
--- a/drivers/net/ethernet/ezchip/nps_enet.c
+++ b/drivers/net/ethernet/ezchip/nps_enet.c
@@ -623,9 +623,9 @@ static s32 nps_enet_probe(struct platform_device *pdev)
/* Get IRQ number */
priv->irq = platform_get_irq(pdev, 0);
- if (!priv->irq) {
+ if (priv->irq <= 0) {
dev_err(dev, "failed to retrieve <irq Rx-Tx> value from device tree\n");
- err = -ENODEV;
+ err = priv->irq ? priv->irq : -ENODEV;
goto out_netdev;
}
@@ -646,8 +646,7 @@ static s32 nps_enet_probe(struct platform_device *pdev)
out_netif_api:
netif_napi_del(&priv->napi);
out_netdev:
- if (err)
- free_netdev(ndev);
+ free_netdev(ndev);
return err;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 04/10] can: xilinx: Handle return value of platform_get_irq
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
platform_get_irq() can fail here and we must check its return value.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/can/xilinx_can.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/can/xilinx_can.c b/drivers/net/can/xilinx_can.c
index 89aec07..e36e2a2 100644
--- a/drivers/net/can/xilinx_can.c
+++ b/drivers/net/can/xilinx_can.c
@@ -1111,6 +1111,10 @@ static int xcan_probe(struct platform_device *pdev)
/* Get IRQ for the device */
ndev->irq = platform_get_irq(pdev, 0);
+ if (ndev->irq <= 0) {
+ ret = ndev->irq ? ndev->irq : -ENODEV;
+ goto err_free;
+ }
ndev->flags |= IFF_ECHO; /* We support local echo */
platform_set_drvdata(pdev, ndev);
--
2.7.4
^ permalink raw reply related
* [PATCH 05/10] net: ethernet: i825xx: Fix platform_get_irq's error checking
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
The platform_get_irq() function returns negative if an error occurs.
zero or positive number on success. platform_get_irq() error checking
for zero is not correct.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/i825xx/sni_82596.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/i825xx/sni_82596.c b/drivers/net/ethernet/i825xx/sni_82596.c
index b2c04a7..a6d56f5 100644
--- a/drivers/net/ethernet/i825xx/sni_82596.c
+++ b/drivers/net/ethernet/i825xx/sni_82596.c
@@ -120,9 +120,10 @@ static int sni_82596_probe(struct platform_device *dev)
netdevice->dev_addr[5] = readb(eth_addr + 0x06);
iounmap(eth_addr);
- if (!netdevice->irq) {
+ if (netdevice->irq <= 0) {
printk(KERN_ERR "%s: IRQ not found for i82596 at 0x%lx\n",
__FILE__, netdevice->base_addr);
+ retval = netdevice->irq ? netdevice->irq : -ENODEV;
goto probe_failed;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 06/10] net: ethernet: natsemi: Handle return value of platform_get_irq
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
platform_get_irq() can fail here and we must check its return value.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/natsemi/jazzsonic.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/natsemi/jazzsonic.c b/drivers/net/ethernet/natsemi/jazzsonic.c
index d5b2888..3cf0856 100644
--- a/drivers/net/ethernet/natsemi/jazzsonic.c
+++ b/drivers/net/ethernet/natsemi/jazzsonic.c
@@ -242,6 +242,11 @@ static int jazz_sonic_probe(struct platform_device *pdev)
dev->base_addr = res->start;
dev->irq = platform_get_irq(pdev, 0);
+ if (dev->irq <= 0) {
+ err = dev->irq ? dev->irq : -ENODEV;
+ goto out;
+ }
+
err = sonic_probe1(dev);
if (err)
goto out;
--
2.7.4
^ permalink raw reply related
* [PATCH 08/10] net: fjes: Handle return value of platform_get_irq and platform_get_resource
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
platform_get_irq() and platform_get_resource() can fail here and
we must check its return value.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/fjes/fjes_main.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
index 750954b..540dd51 100644
--- a/drivers/net/fjes/fjes_main.c
+++ b/drivers/net/fjes/fjes_main.c
@@ -1265,9 +1265,19 @@ static int fjes_probe(struct platform_device *plat_dev)
adapter->interrupt_watch_enable = false;
res = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
+ if (!res) {
+ err = -EINVAL;
+ goto err_free_netdev;
+ }
+
hw->hw_res.start = res->start;
hw->hw_res.size = resource_size(res);
hw->hw_res.irq = platform_get_irq(plat_dev, 0);
+ if (hw->hw_res.irq <= 0) {
+ err = hw->hw_res.irq ? hw->hw_res.irq : -ENODEV;
+ goto err_free_netdev;
+ }
+
err = fjes_hw_init(&adapter->hw);
if (err)
goto err_free_netdev;
--
2.7.4
^ permalink raw reply related
* [PATCH 09/10] net: ethernet: korina: Handle return value of platform_get_irq_byname
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
platform_get_irq_byname() can fail here and we must check its return
value.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/korina.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/ethernet/korina.c b/drivers/net/ethernet/korina.c
index ae195f8..e778504 100644
--- a/drivers/net/ethernet/korina.c
+++ b/drivers/net/ethernet/korina.c
@@ -1039,7 +1039,16 @@ static int korina_probe(struct platform_device *pdev)
memcpy(dev->dev_addr, bif->mac, ETH_ALEN);
lp->rx_irq = platform_get_irq_byname(pdev, "korina_rx");
+ if (lp->rx_irq < 0) {
+ rc = lp->rx_irq;
+ goto probe_err_out;
+ }
+
lp->tx_irq = platform_get_irq_byname(pdev, "korina_tx");
+ if (lp->tx_irq < 0) {
+ rc = lp->tx_irq;
+ goto probe_err_out;
+ }
r = platform_get_resource_byname(pdev, IORESOURCE_MEM, "korina_regs");
dev->base_addr = r->start;
--
2.7.4
^ permalink raw reply related
* [PATCH 07/10] net: ethernet: smsc: Handle return value of platform_get_irq
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
platform_get_irq() can fail here and we must check its return value.
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/smsc/smc911x.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
index 0515744..a1cf18c 100644
--- a/drivers/net/ethernet/smsc/smc911x.c
+++ b/drivers/net/ethernet/smsc/smc911x.c
@@ -2088,6 +2088,11 @@ static int smc911x_drv_probe(struct platform_device *pdev)
ndev->dma = (unsigned char)-1;
ndev->irq = platform_get_irq(pdev, 0);
+ if (ndev->irq <= 0) {
+ ret = ndev->irq ? ndev->irq : -ENODEV;
+ goto release_both;
+ }
+
lp = netdev_priv(ndev);
lp->netdev = ndev;
#ifdef SMC_DYNAMIC_BUS_CONFIG
--
2.7.4
^ permalink raw reply related
* [PATCH 10/10] net: ethernet: cpmac: Handle return value of platform_get_irq_byname
From: Arvind Yadav @ 2017-12-02 19:26 UTC (permalink / raw)
To: wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-1-git-send-email-arvind.yadav.cs@gmail.com>
platform_get_irq_byname() can fail here and we must check its return
value
Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
---
drivers/net/ethernet/ti/cpmac.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/ti/cpmac.c b/drivers/net/ethernet/ti/cpmac.c
index 9b8a30b..f3acfc0 100644
--- a/drivers/net/ethernet/ti/cpmac.c
+++ b/drivers/net/ethernet/ti/cpmac.c
@@ -1124,6 +1124,10 @@ static int cpmac_probe(struct platform_device *pdev)
}
dev->irq = platform_get_irq_byname(pdev, "irq");
+ if (dev->irq < 0) {
+ rc = dev->irq;
+ goto fail;
+ }
dev->netdev_ops = &cpmac_netdev_ops;
dev->ethtool_ops = &cpmac_ethtool_ops;
--
2.7.4
^ permalink raw reply related
* Re: [Patch net-next] act_mirred: use tcfm_dev in tcf_mirred_get_dev()
From: Cong Wang @ 2017-12-02 19:47 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <20171202085739.GE1821@nanopsycho>
On Sat, Dec 2, 2017 at 12:57 AM, Jiri Pirko <jiri@resnulli.us> wrote:
> Good. Please also use m->tcfm_dev->ifindex in tcf_mirred_dump and
> tcf_mirred_ifindex. Then you can remove tcfm_ifindex completely.
Sounds good. Will send v2.
^ permalink raw reply
* Re: [Patch net-next] act_mirred: use tcfm_dev in tcf_mirred_get_dev()
From: Cong Wang @ 2017-12-02 19:53 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Linux Kernel Network Developers, Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <CAM_iQpVyOmS7QARTT6+C4kdffrsoJMeeGqHb9P5LFO2WSuuymA@mail.gmail.com>
On Sat, Dec 2, 2017 at 11:47 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Sat, Dec 2, 2017 at 12:57 AM, Jiri Pirko <jiri@resnulli.us> wrote:
>> Good. Please also use m->tcfm_dev->ifindex in tcf_mirred_dump and
>> tcf_mirred_ifindex. Then you can remove tcfm_ifindex completely.
>
> Sounds good. Will send v2.
Hold on, m->tcfm_dev could be NULL, so we can't just def it here.
I think we can just use 0 when it is NULL:
.ifindex = m->tcfm_dev ? m->tcfm_dev->ifindex : 0,
This also "fixes" the garbage ifindex dump when the target device is gone.
^ permalink raw reply
* Re: [PATCH 08/10] net: fjes: Handle return value of platform_get_irq and platform_get_resource
From: Sergei Shtylyov @ 2017-12-02 20:06 UTC (permalink / raw)
To: Arvind Yadav, wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-9-git-send-email-arvind.yadav.cs@gmail.com>
Hello!
On 12/02/2017 10:26 PM, Arvind Yadav wrote:
> platform_get_irq() and platform_get_resource() can fail here and
> we must check its return value.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/net/fjes/fjes_main.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
> index 750954b..540dd51 100644
> --- a/drivers/net/fjes/fjes_main.c
> +++ b/drivers/net/fjes/fjes_main.c
> @@ -1265,9 +1265,19 @@ static int fjes_probe(struct platform_device *plat_dev)
> adapter->interrupt_watch_enable = false;
>
> res = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
> + if (!res) {
> + err = -EINVAL;
> + goto err_free_netdev;
> + }
> +
> hw->hw_res.start = res->start;
> hw->hw_res.size = resource_size(res);
> hw->hw_res.irq = platform_get_irq(plat_dev, 0);
> + if (hw->hw_res.irq <= 0) {
This function no longer returns 0 on error, no need to check for <= 0.
> + err = hw->hw_res.irq ? hw->hw_res.irq : -ENODEV;
> + goto err_free_netdev;
gcc allows a shorter way to write that.
err = hw->hw_res.irq ?: -ENODEV;
> + }
> +
> err = fjes_hw_init(&adapter->hw);
> if (err)
> goto err_free_netdev;
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 07/10] net: ethernet: smsc: Handle return value of platform_get_irq
From: Sergei Shtylyov @ 2017-12-02 20:06 UTC (permalink / raw)
To: Arvind Yadav, wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-8-git-send-email-arvind.yadav.cs@gmail.com>
On 12/02/2017 10:26 PM, Arvind Yadav wrote:
> platform_get_irq() can fail here and we must check its return value.
>
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/net/ethernet/smsc/smc911x.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/net/ethernet/smsc/smc911x.c b/drivers/net/ethernet/smsc/smc911x.c
> index 0515744..a1cf18c 100644
> --- a/drivers/net/ethernet/smsc/smc911x.c
> +++ b/drivers/net/ethernet/smsc/smc911x.c
> @@ -2088,6 +2088,11 @@ static int smc911x_drv_probe(struct platform_device *pdev)
>
> ndev->dma = (unsigned char)-1;
> ndev->irq = platform_get_irq(pdev, 0);
> + if (ndev->irq <= 0) {
> + ret = ndev->irq ? ndev->irq : -ENODEV;
Same comments as the next patch...
> + goto release_both;
> + }
> +
> lp = netdev_priv(ndev);
> lp->netdev = ndev;
> #ifdef SMC_DYNAMIC_BUS_CONFIG
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 05/10] net: ethernet: i825xx: Fix platform_get_irq's error checking
From: Sergei Shtylyov @ 2017-12-02 20:08 UTC (permalink / raw)
To: Arvind Yadav, wg, mkl, michal.simek, opendmb, f.fainelli, davem,
Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1512242782-7134-6-git-send-email-arvind.yadav.cs@gmail.com>
Hello.
On 12/02/2017 10:26 PM, Arvind Yadav wrote:
> The platform_get_irq() function returns negative if an error occurs.
> zero or positive number on success. platform_get_irq() error checking
> for zero is not correct.
The why you consider returning 0 a sign of failure?
> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
> ---
> drivers/net/ethernet/i825xx/sni_82596.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/i825xx/sni_82596.c b/drivers/net/ethernet/i825xx/sni_82596.c
> index b2c04a7..a6d56f5 100644
> --- a/drivers/net/ethernet/i825xx/sni_82596.c
> +++ b/drivers/net/ethernet/i825xx/sni_82596.c
> @@ -120,9 +120,10 @@ static int sni_82596_probe(struct platform_device *dev)
> netdevice->dev_addr[5] = readb(eth_addr + 0x06);
> iounmap(eth_addr);
>
> - if (!netdevice->irq) {
> + if (netdevice->irq <= 0) {
> printk(KERN_ERR "%s: IRQ not found for i82596 at 0x%lx\n",
> __FILE__, netdevice->base_addr);
> + retval = netdevice->irq ? netdevice->irq : -ENODEV;
> goto probe_failed;
> }
>
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 05/10] net: ethernet: i825xx: Fix platform_get_irq's error checking
From: arvindY @ 2017-12-02 20:22 UTC (permalink / raw)
To: Sergei Shtylyov, wg, mkl, michal.simek, opendmb, f.fainelli,
davem, Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <2b23637d-0d19-4a6a-4e67-e64968f85415@cogentembedded.com>
Hi Sergei,
On Sunday 03 December 2017 01:38 AM, Sergei Shtylyov wrote:
> Hello.
>
> On 12/02/2017 10:26 PM, Arvind Yadav wrote:
>
>> The platform_get_irq() function returns negative if an error occurs.
>> zero or positive number on success. platform_get_irq() error checking
>> for zero is not correct.
>
> The why you consider returning 0 a sign of failure?
Here, Returning 0 is a problem. Because IRQ0 is always a problem.
This function is for getting an IRQ for a device. So we should check for
0 also.
>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> ---
>> drivers/net/ethernet/i825xx/sni_82596.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/i825xx/sni_82596.c
>> b/drivers/net/ethernet/i825xx/sni_82596.c
>> index b2c04a7..a6d56f5 100644
>> --- a/drivers/net/ethernet/i825xx/sni_82596.c
>> +++ b/drivers/net/ethernet/i825xx/sni_82596.c
>> @@ -120,9 +120,10 @@ static int sni_82596_probe(struct
>> platform_device *dev)
>> netdevice->dev_addr[5] = readb(eth_addr + 0x06);
>> iounmap(eth_addr);
>> - if (!netdevice->irq) {
>> + if (netdevice->irq <= 0) {
>> printk(KERN_ERR "%s: IRQ not found for i82596 at 0x%lx\n",
>> __FILE__, netdevice->base_addr);
>> + retval = netdevice->irq ? netdevice->irq : -ENODEV;
>> goto probe_failed;
>> }
>
> MBR, Sergei
~arvind
^ permalink raw reply
* [PATCH net 1/2] netlink: add NLA_U8_BUGGY attribute type
From: Johannes Berg @ 2017-12-02 20:23 UTC (permalink / raw)
To: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
Cc: j, David Ahern, Johannes Berg
From: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
This netlink type is used only for backwards compatibility
with broken userspace that used the wrong size for a given
u8 attribute, which is now rejected. It would've been wrong
before already, since on big endian the wrong value (always
zero) would be used by the kernel, but we can't break the
existing deployed userspace - hostapd for example now fails
to initialize entirely.
We could try to fix up the big endian problem here, but we
don't know *how* userspace misbehaved - if using nla_put_u32
then we could, but we also found a debug tool (which we'll
ignore for the purposes of this regression) that was putting
the padding into the length.
Fixes: 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")
Signed-off-by: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
include/net/netlink.h | 1 +
lib/nlattr.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 0c154f98e987..448a9b86c959 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -180,6 +180,7 @@ enum {
NLA_S32,
NLA_S64,
NLA_BITFIELD32,
+ NLA_U8_BUGGY, /* don't use this - only for bug-ward compatibility */
__NLA_TYPE_MAX,
};
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 8bf78b4b78f0..2b89d25d4745 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -28,6 +28,7 @@ static const u8 nla_attr_len[NLA_TYPE_MAX+1] = {
};
static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = {
+ [NLA_U8_BUGGY] = sizeof(u8),
[NLA_MSECS] = sizeof(u64),
[NLA_NESTED] = NLA_HDRLEN,
};
--
2.14.2
^ permalink raw reply related
* [PATCH net 2/2] nl80211: use NLA_U8_BUGGY for two attributes
From: Johannes Berg @ 2017-12-02 20:23 UTC (permalink / raw)
To: linux-wireless, netdev; +Cc: j, David Ahern, Johannes Berg
In-Reply-To: <20171202202332.10205-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes.berg@intel.com>
We discovered that these are set incorrectly by the
corresponding userspace code, so keep compatible with
their bugs even if they'd always set the value to 0.
Reported-by: Jouni Malinen <j@w1.fi>
Fixes: 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/nl80211.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index b1ac23ca20c8..751b4efbf09a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -384,7 +384,7 @@ static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_TSID] = { .type = NLA_U8 },
[NL80211_ATTR_USER_PRIO] = { .type = NLA_U8 },
[NL80211_ATTR_ADMITTED_TIME] = { .type = NLA_U16 },
- [NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8 },
+ [NL80211_ATTR_SMPS_MODE] = { .type = NLA_U8_BUGGY },
[NL80211_ATTR_MAC_MASK] = { .len = ETH_ALEN },
[NL80211_ATTR_WIPHY_SELF_MANAGED_REG] = { .type = NLA_FLAG },
[NL80211_ATTR_NETNS_FD] = { .type = NLA_U32 },
@@ -5830,7 +5830,7 @@ static const struct nla_policy nl80211_meshconf_params_policy[NL80211_MESHCONF_A
[NL80211_MESHCONF_MAX_RETRIES] = { .type = NLA_U8 },
[NL80211_MESHCONF_TTL] = { .type = NLA_U8 },
[NL80211_MESHCONF_ELEMENT_TTL] = { .type = NLA_U8 },
- [NL80211_MESHCONF_AUTO_OPEN_PLINKS] = { .type = NLA_U8 },
+ [NL80211_MESHCONF_AUTO_OPEN_PLINKS] = { .type = NLA_U8_BUGGY },
[NL80211_MESHCONF_SYNC_OFFSET_MAX_NEIGHBOR] = { .type = NLA_U32 },
[NL80211_MESHCONF_HWMP_MAX_PREQ_RETRIES] = { .type = NLA_U8 },
[NL80211_MESHCONF_PATH_REFRESH_TIME] = { .type = NLA_U32 },
--
2.14.2
^ permalink raw reply related
* Re: [PATCH 08/10] net: fjes: Handle return value of platform_get_irq and platform_get_resource
From: arvindY @ 2017-12-02 20:26 UTC (permalink / raw)
To: Sergei Shtylyov, wg, mkl, michal.simek, opendmb, f.fainelli,
davem, Vladislav.Zakharov
Cc: linux-kernel, linux-arm-kernel, netdev
In-Reply-To: <1bbebd1a-4cca-0ef0-bdc1-ae50a4218f30@cogentembedded.com>
Hi Sergei,
On Sunday 03 December 2017 01:36 AM, Sergei Shtylyov wrote:
> Hello!
>
> On 12/02/2017 10:26 PM, Arvind Yadav wrote:
>
>> platform_get_irq() and platform_get_resource() can fail here and
>> we must check its return value.
>>
>> Signed-off-by: Arvind Yadav <arvind.yadav.cs@gmail.com>
>> ---
>> drivers/net/fjes/fjes_main.c | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/drivers/net/fjes/fjes_main.c b/drivers/net/fjes/fjes_main.c
>> index 750954b..540dd51 100644
>> --- a/drivers/net/fjes/fjes_main.c
>> +++ b/drivers/net/fjes/fjes_main.c
>> @@ -1265,9 +1265,19 @@ static int fjes_probe(struct platform_device
>> *plat_dev)
>> adapter->interrupt_watch_enable = false;
>> res = platform_get_resource(plat_dev, IORESOURCE_MEM, 0);
>> + if (!res) {
>> + err = -EINVAL;
>> + goto err_free_netdev;
>> + }
>> +
>> hw->hw_res.start = res->start;
>> hw->hw_res.size = resource_size(res);
>> hw->hw_res.irq = platform_get_irq(plat_dev, 0);
>> + if (hw->hw_res.irq <= 0) {
>
> This function no longer returns 0 on error, no need to check for <= 0.
>
>> + err = hw->hw_res.irq ? hw->hw_res.irq : -ENODEV;
>> + goto err_free_netdev;
>
> gcc allows a shorter way to write that.
>
> err = hw->hw_res.irq ?: -ENODEV;
Yes, you are right, But first is more readable. That's why I have used.
>
>> + }
>> +
>> err = fjes_hw_init(&adapter->hw);
>> if (err)
>> goto err_free_netdev;
>
> MBR, Sergei
Thanks,
~arvind
^ permalink raw reply
* [PATCH next-next 0/4] rtnetlink: rework handler (un)registering
From: Florian Westphal @ 2017-12-02 20:44 UTC (permalink / raw)
To: netdev
Peter Zijlstra reported (referring to commit 019a316992ee0d983,
"rtnetlink: add reference counting to prevent module unload while dump is in progress"):
1) it not in fact a refcount, so using refcount_t is silly
2) there is a distinct lack of memory barriers, so we can easily
observe the decrement while the msg_handler is still in progress.
3) waiting with a schedule()/yield() loop is complete crap and subject
life-locks, imagine doing that rtnl_unregister_all() from a RT task.
In ancient times rtnetlink exposed a statically-sized table with
preset doit/dumpit handlers to be called for a protocol/type pair.
Later the rtnl_register interface was added and the table was allocated
on demand. Eventually these were also used by modules.
Problem is that nothing prevents module unload while a netlink dump
is in progress. netlink dumps can be span multiple recv calls and
netlink core saves the to-be-repeated dumper address for later invocation.
To prevent rmmod the netlink core expects callers to pass in the owning
module so a reference can be taken.
So far rtnetlink wasn't doing this, add new interface to pass THIS_MODULE.
Moreover, when converting parts of the rtnetlink handling to rcu this code
gained way too many READ_ONCE spots, remove them and the extra refcounting.
Take a module reference when running dumpit and doit callbacks
and never alter content of rtnl_link structures after they have been
published via rcu_assign_pointer.
Based partially on earlier patch from Peter.
include/net/rtnetlink.h | 4
net/bridge/br_mdb.c | 6 -
net/can/gw.c | 14 +-
net/core/rtnetlink.c | 270 ++++++++++++++++++++++++++++++------------------
net/decnet/dn_dev.c | 9 +
net/decnet/dn_fib.c | 6 -
net/decnet/dn_route.c | 8 -
net/ipv6/addrconf.c | 44 +++++--
net/ipv6/addrlabel.c | 13 +-
net/ipv6/ip6_fib.c | 4
net/ipv6/route.c | 20 ++-
net/mpls/af_mpls.c | 15 +-
net/phonet/pn_netlink.c | 21 ++-
net/qrtr/qrtr.c | 8 +
14 files changed, 282 insertions(+), 160 deletions(-)
^ permalink raw reply
* [PATCH net-next 1/4] net: rtnetlink: use rcu to free rtnl message handlers
From: Florian Westphal @ 2017-12-02 20:44 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, Peter Zijlstra
In-Reply-To: <20171202204408.4166-1-fw@strlen.de>
rtnetlink is littered with READ_ONCE() because we can have read accesses
while another cpu can write to the structure we're reading by
(un)registering doit or dumpit handlers.
This patch changes this so that (un)registering cpu allocates a new
structure and then publishes it via rcu_assign_pointer, i.e. once
another cpu can see such pointer no modifications will occur anymore.
based on initial patch from Peter Zijlstra.
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/core/rtnetlink.c | 154 +++++++++++++++++++++++++++++++++------------------
1 file changed, 101 insertions(+), 53 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index dabba2a91fc8..ff292d3f2c41 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -63,6 +63,7 @@ struct rtnl_link {
rtnl_doit_func doit;
rtnl_dumpit_func dumpit;
unsigned int flags;
+ struct rcu_head rcu;
};
static DEFINE_MUTEX(rtnl_mutex);
@@ -127,7 +128,7 @@ bool lockdep_rtnl_is_held(void)
EXPORT_SYMBOL(lockdep_rtnl_is_held);
#endif /* #ifdef CONFIG_PROVE_LOCKING */
-static struct rtnl_link __rcu *rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
+static struct rtnl_link __rcu **rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
static refcount_t rtnl_msg_handlers_ref[RTNL_FAMILY_MAX + 1];
static inline int rtm_msgindex(int msgtype)
@@ -144,6 +145,20 @@ static inline int rtm_msgindex(int msgtype)
return msgindex;
}
+static struct rtnl_link *rtnl_get_link(int protocol, int msgtype)
+{
+ struct rtnl_link **tab;
+
+ if (protocol >= ARRAY_SIZE(rtnl_msg_handlers))
+ protocol = PF_UNSPEC;
+
+ tab = rcu_dereference_rtnl(rtnl_msg_handlers[protocol]);
+ if (!tab)
+ tab = rcu_dereference_rtnl(rtnl_msg_handlers[PF_UNSPEC]);
+
+ return tab[msgtype];
+}
+
/**
* __rtnl_register - Register a rtnetlink message type
* @protocol: Protocol family or PF_UNSPEC
@@ -166,28 +181,52 @@ int __rtnl_register(int protocol, int msgtype,
rtnl_doit_func doit, rtnl_dumpit_func dumpit,
unsigned int flags)
{
- struct rtnl_link *tab;
+ struct rtnl_link **tab, *link, *old;
int msgindex;
+ int ret = -ENOBUFS;
BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
msgindex = rtm_msgindex(msgtype);
- tab = rcu_dereference_raw(rtnl_msg_handlers[protocol]);
+ rtnl_lock();
+ tab = rtnl_msg_handlers[protocol];
if (tab == NULL) {
- tab = kcalloc(RTM_NR_MSGTYPES, sizeof(*tab), GFP_KERNEL);
- if (tab == NULL)
- return -ENOBUFS;
+ tab = kcalloc(RTM_NR_MSGTYPES, sizeof(void *), GFP_KERNEL);
+ if (!tab)
+ goto unlock;
+ /* ensures we see the 0 stores */
rcu_assign_pointer(rtnl_msg_handlers[protocol], tab);
}
+ old = rtnl_dereference(tab[msgindex]);
+ if (old) {
+ link = kmemdup(old, sizeof(*old), GFP_KERNEL);
+ if (!link)
+ goto unlock;
+ } else {
+ link = kzalloc(sizeof(*link), GFP_KERNEL);
+ if (!link)
+ goto unlock;
+ }
+
+ WARN_ON(doit && link->doit && link->doit != doit);
if (doit)
- tab[msgindex].doit = doit;
+ link->doit = doit;
+ WARN_ON(dumpit && link->dumpit && link->dumpit != dumpit);
if (dumpit)
- tab[msgindex].dumpit = dumpit;
- tab[msgindex].flags |= flags;
+ link->dumpit = dumpit;
- return 0;
+ link->flags |= flags;
+
+ /* publish protocol:msgtype */
+ rcu_assign_pointer(tab[msgindex], link);
+ ret = 0;
+ if (old)
+ kfree_rcu(old, rcu);
+unlock:
+ rtnl_unlock();
+ return ret;
}
EXPORT_SYMBOL_GPL(__rtnl_register);
@@ -220,24 +259,25 @@ EXPORT_SYMBOL_GPL(rtnl_register);
*/
int rtnl_unregister(int protocol, int msgtype)
{
- struct rtnl_link *handlers;
+ struct rtnl_link **tab, *link;
int msgindex;
BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
msgindex = rtm_msgindex(msgtype);
rtnl_lock();
- handlers = rtnl_dereference(rtnl_msg_handlers[protocol]);
- if (!handlers) {
+ tab = rtnl_dereference(rtnl_msg_handlers[protocol]);
+ if (!tab) {
rtnl_unlock();
return -ENOENT;
}
- handlers[msgindex].doit = NULL;
- handlers[msgindex].dumpit = NULL;
- handlers[msgindex].flags = 0;
+ link = tab[msgindex];
+ rcu_assign_pointer(tab[msgindex], NULL);
rtnl_unlock();
+ kfree_rcu(link, rcu);
+
return 0;
}
EXPORT_SYMBOL_GPL(rtnl_unregister);
@@ -251,20 +291,29 @@ EXPORT_SYMBOL_GPL(rtnl_unregister);
*/
void rtnl_unregister_all(int protocol)
{
- struct rtnl_link *handlers;
+ struct rtnl_link **tab, *link;
+ int msgindex;
BUG_ON(protocol < 0 || protocol > RTNL_FAMILY_MAX);
rtnl_lock();
- handlers = rtnl_dereference(rtnl_msg_handlers[protocol]);
+ tab = rtnl_msg_handlers[protocol];
RCU_INIT_POINTER(rtnl_msg_handlers[protocol], NULL);
+ for (msgindex = 0; msgindex < RTM_NR_MSGTYPES; msgindex++) {
+ link = tab[msgindex];
+ if (!link)
+ continue;
+
+ rcu_assign_pointer(tab[msgindex], NULL);
+ kfree_rcu(link, rcu);
+ }
rtnl_unlock();
synchronize_net();
while (refcount_read(&rtnl_msg_handlers_ref[protocol]) > 1)
schedule();
- kfree(handlers);
+ kfree(tab);
}
EXPORT_SYMBOL_GPL(rtnl_unregister_all);
@@ -2973,18 +3022,26 @@ static int rtnl_dump_all(struct sk_buff *skb, struct netlink_callback *cb)
s_idx = 1;
for (idx = 1; idx <= RTNL_FAMILY_MAX; idx++) {
+ struct rtnl_link **tab;
int type = cb->nlh->nlmsg_type-RTM_BASE;
- struct rtnl_link *handlers;
+ struct rtnl_link *link;
rtnl_dumpit_func dumpit;
if (idx < s_idx || idx == PF_PACKET)
continue;
- handlers = rtnl_dereference(rtnl_msg_handlers[idx]);
- if (!handlers)
+ if (type < 0 || type >= RTM_NR_MSGTYPES)
continue;
- dumpit = READ_ONCE(handlers[type].dumpit);
+ tab = rcu_dereference_rtnl(rtnl_msg_handlers[idx]);
+ if (!tab)
+ continue;
+
+ link = tab[type];
+ if (!link)
+ continue;
+
+ dumpit = link->dumpit;
if (!dumpit)
continue;
@@ -4314,7 +4371,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
struct netlink_ext_ack *extack)
{
struct net *net = sock_net(skb->sk);
- struct rtnl_link *handlers;
+ struct rtnl_link *link;
int err = -EOPNOTSUPP;
rtnl_doit_func doit;
unsigned int flags;
@@ -4338,32 +4395,20 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
if (kind != 2 && !netlink_net_capable(skb, CAP_NET_ADMIN))
return -EPERM;
- if (family >= ARRAY_SIZE(rtnl_msg_handlers))
- family = PF_UNSPEC;
-
rcu_read_lock();
- handlers = rcu_dereference(rtnl_msg_handlers[family]);
- if (!handlers) {
- family = PF_UNSPEC;
- handlers = rcu_dereference(rtnl_msg_handlers[family]);
- }
-
if (kind == 2 && nlh->nlmsg_flags&NLM_F_DUMP) {
struct sock *rtnl;
rtnl_dumpit_func dumpit;
u16 min_dump_alloc = 0;
- dumpit = READ_ONCE(handlers[type].dumpit);
- if (!dumpit) {
+ link = rtnl_get_link(family, type);
+ if (!link || !link->dumpit) {
family = PF_UNSPEC;
- handlers = rcu_dereference(rtnl_msg_handlers[PF_UNSPEC]);
- if (!handlers)
- goto err_unlock;
-
- dumpit = READ_ONCE(handlers[type].dumpit);
- if (!dumpit)
+ link = rtnl_get_link(family, type);
+ if (!link || !link->dumpit)
goto err_unlock;
}
+ dumpit = link->dumpit;
refcount_inc(&rtnl_msg_handlers_ref[family]);
@@ -4384,33 +4429,36 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
return err;
}
- doit = READ_ONCE(handlers[type].doit);
- if (!doit) {
+ link = rtnl_get_link(family, type);
+ if (!link || !link->doit) {
family = PF_UNSPEC;
- handlers = rcu_dereference(rtnl_msg_handlers[family]);
+ link = rtnl_get_link(PF_UNSPEC, type);
+ if (!link || !link->doit)
+ goto out_unlock;
}
- flags = READ_ONCE(handlers[type].flags);
+ flags = link->flags;
if (flags & RTNL_FLAG_DOIT_UNLOCKED) {
refcount_inc(&rtnl_msg_handlers_ref[family]);
- doit = READ_ONCE(handlers[type].doit);
+ doit = link->doit;
rcu_read_unlock();
if (doit)
err = doit(skb, nlh, extack);
refcount_dec(&rtnl_msg_handlers_ref[family]);
return err;
}
-
rcu_read_unlock();
rtnl_lock();
- handlers = rtnl_dereference(rtnl_msg_handlers[family]);
- if (handlers) {
- doit = READ_ONCE(handlers[type].doit);
- if (doit)
- err = doit(skb, nlh, extack);
- }
+ link = rtnl_get_link(family, type);
+ if (link && link->doit)
+ err = link->doit(skb, nlh, extack);
rtnl_unlock();
+
+ return err;
+
+out_unlock:
+ rcu_read_unlock();
return err;
err_unlock:
--
2.13.6
^ permalink raw reply related
* [PATCH net-next 2/4] rtnetlink: get reference on module before invoking handlers
From: Florian Westphal @ 2017-12-02 20:44 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal, Peter Zijlstra
In-Reply-To: <20171202204408.4166-1-fw@strlen.de>
Add yet another rtnl_register function. It will be used by modules
that can be removed.
The passed module struct is used to prevent module unload while
a netlink dump is in progress or when a DOIT_UNLOCKED doit callback
is called.
Cc: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/net/rtnetlink.h | 2 +
net/core/rtnetlink.c | 113 +++++++++++++++++++++++++++++++++---------------
2 files changed, 80 insertions(+), 35 deletions(-)
diff --git a/include/net/rtnetlink.h b/include/net/rtnetlink.h
index ead018744ff5..e326b3f9eb5f 100644
--- a/include/net/rtnetlink.h
+++ b/include/net/rtnetlink.h
@@ -17,6 +17,8 @@ int __rtnl_register(int protocol, int msgtype,
rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
void rtnl_register(int protocol, int msgtype,
rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
+int rtnl_register_module(struct module *owner, int protocol, int msgtype,
+ rtnl_doit_func, rtnl_dumpit_func, unsigned int flags);
int rtnl_unregister(int protocol, int msgtype);
void rtnl_unregister_all(int protocol);
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index ff292d3f2c41..de6390365c90 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -62,6 +62,7 @@
struct rtnl_link {
rtnl_doit_func doit;
rtnl_dumpit_func dumpit;
+ struct module *owner;
unsigned int flags;
struct rcu_head rcu;
};
@@ -129,7 +130,6 @@ EXPORT_SYMBOL(lockdep_rtnl_is_held);
#endif /* #ifdef CONFIG_PROVE_LOCKING */
static struct rtnl_link __rcu **rtnl_msg_handlers[RTNL_FAMILY_MAX + 1];
-static refcount_t rtnl_msg_handlers_ref[RTNL_FAMILY_MAX + 1];
static inline int rtm_msgindex(int msgtype)
{
@@ -159,27 +159,10 @@ static struct rtnl_link *rtnl_get_link(int protocol, int msgtype)
return tab[msgtype];
}
-/**
- * __rtnl_register - Register a rtnetlink message type
- * @protocol: Protocol family or PF_UNSPEC
- * @msgtype: rtnetlink message type
- * @doit: Function pointer called for each request message
- * @dumpit: Function pointer called for each dump request (NLM_F_DUMP) message
- * @flags: rtnl_link_flags to modifiy behaviour of doit/dumpit functions
- *
- * Registers the specified function pointers (at least one of them has
- * to be non-NULL) to be called whenever a request message for the
- * specified protocol family and message type is received.
- *
- * The special protocol family PF_UNSPEC may be used to define fallback
- * function pointers for the case when no entry for the specific protocol
- * family exists.
- *
- * Returns 0 on success or a negative error code.
- */
-int __rtnl_register(int protocol, int msgtype,
- rtnl_doit_func doit, rtnl_dumpit_func dumpit,
- unsigned int flags)
+static int rtnl_register_internal(struct module *owner,
+ int protocol, int msgtype,
+ rtnl_doit_func doit, rtnl_dumpit_func dumpit,
+ unsigned int flags)
{
struct rtnl_link **tab, *link, *old;
int msgindex;
@@ -210,6 +193,9 @@ int __rtnl_register(int protocol, int msgtype,
goto unlock;
}
+ WARN_ON(link->owner && link->owner != owner);
+ link->owner = owner;
+
WARN_ON(doit && link->doit && link->doit != doit);
if (doit)
link->doit = doit;
@@ -228,6 +214,54 @@ int __rtnl_register(int protocol, int msgtype,
rtnl_unlock();
return ret;
}
+
+/**
+ * rtnl_register_module - Register a rtnetlink message type
+ *
+ * @owner: module registering the hook (THIS_MODULE)
+ * @protocol: Protocol family or PF_UNSPEC
+ * @msgtype: rtnetlink message type
+ * @doit: Function pointer called for each request message
+ * @dumpit: Function pointer called for each dump request (NLM_F_DUMP) message
+ * @flags: rtnl_link_flags to modifiy behaviour of doit/dumpit functions
+ *
+ * Like rtnl_register, but for use by removable modules.
+ */
+int rtnl_register_module(struct module *owner,
+ int protocol, int msgtype,
+ rtnl_doit_func doit, rtnl_dumpit_func dumpit,
+ unsigned int flags)
+{
+ return rtnl_register_internal(owner, protocol, msgtype,
+ doit, dumpit, flags);
+}
+EXPORT_SYMBOL_GPL(rtnl_register_module);
+
+/**
+ * __rtnl_register - Register a rtnetlink message type
+ * @protocol: Protocol family or PF_UNSPEC
+ * @msgtype: rtnetlink message type
+ * @doit: Function pointer called for each request message
+ * @dumpit: Function pointer called for each dump request (NLM_F_DUMP) message
+ * @flags: rtnl_link_flags to modifiy behaviour of doit/dumpit functions
+ *
+ * Registers the specified function pointers (at least one of them has
+ * to be non-NULL) to be called whenever a request message for the
+ * specified protocol family and message type is received.
+ *
+ * The special protocol family PF_UNSPEC may be used to define fallback
+ * function pointers for the case when no entry for the specific protocol
+ * family exists.
+ *
+ * Returns 0 on success or a negative error code.
+ */
+int __rtnl_register(int protocol, int msgtype,
+ rtnl_doit_func doit, rtnl_dumpit_func dumpit,
+ unsigned int flags)
+{
+ return rtnl_register_internal(NULL, protocol, msgtype,
+ doit, dumpit, flags);
+}
EXPORT_SYMBOL_GPL(__rtnl_register);
/**
@@ -311,8 +345,6 @@ void rtnl_unregister_all(int protocol)
synchronize_net();
- while (refcount_read(&rtnl_msg_handlers_ref[protocol]) > 1)
- schedule();
kfree(tab);
}
EXPORT_SYMBOL_GPL(rtnl_unregister_all);
@@ -4372,6 +4404,7 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
{
struct net *net = sock_net(skb->sk);
struct rtnl_link *link;
+ struct module *owner;
int err = -EOPNOTSUPP;
rtnl_doit_func doit;
unsigned int flags;
@@ -4408,24 +4441,32 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
if (!link || !link->dumpit)
goto err_unlock;
}
+ owner = link->owner;
dumpit = link->dumpit;
- refcount_inc(&rtnl_msg_handlers_ref[family]);
-
if (type == RTM_GETLINK - RTM_BASE)
min_dump_alloc = rtnl_calcit(skb, nlh);
+ err = 0;
+ /* need to do this before rcu_read_unlock() */
+ if (!try_module_get(owner))
+ err = -EPROTONOSUPPORT;
+
rcu_read_unlock();
rtnl = net->rtnl;
- {
+ if (err == 0) {
struct netlink_dump_control c = {
.dump = dumpit,
.min_dump_alloc = min_dump_alloc,
+ .module = owner,
};
err = netlink_dump_start(rtnl, skb, nlh, &c);
+ /* netlink_dump_start() will keep a reference on
+ * module if dump is still in progress.
+ */
+ module_put(owner);
}
- refcount_dec(&rtnl_msg_handlers_ref[family]);
return err;
}
@@ -4437,14 +4478,19 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
goto out_unlock;
}
+ owner = link->owner;
+ if (!try_module_get(owner)) {
+ err = -EPROTONOSUPPORT;
+ goto out_unlock;
+ }
+
flags = link->flags;
if (flags & RTNL_FLAG_DOIT_UNLOCKED) {
- refcount_inc(&rtnl_msg_handlers_ref[family]);
doit = link->doit;
rcu_read_unlock();
if (doit)
err = doit(skb, nlh, extack);
- refcount_dec(&rtnl_msg_handlers_ref[family]);
+ module_put(owner);
return err;
}
rcu_read_unlock();
@@ -4455,6 +4501,8 @@ static int rtnetlink_rcv_msg(struct sk_buff *skb, struct nlmsghdr *nlh,
err = link->doit(skb, nlh, extack);
rtnl_unlock();
+ module_put(owner);
+
return err;
out_unlock:
@@ -4546,11 +4594,6 @@ static struct pernet_operations rtnetlink_net_ops = {
void __init rtnetlink_init(void)
{
- int i;
-
- for (i = 0; i < ARRAY_SIZE(rtnl_msg_handlers_ref); i++)
- refcount_set(&rtnl_msg_handlers_ref[i], 1);
-
if (register_pernet_subsys(&rtnetlink_net_ops))
panic("rtnetlink_init: cannot initialize rtnetlink\n");
--
2.13.6
^ 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