* Re: Sending 802.1Q packets using AF_PACKET socket on filtered bridge forwards with wrong MAC addresses
From: Brandon Carpenter @ 2017-11-29 22:01 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David S. Miller, bridge, netdev, linux-kernel
In-Reply-To: <CAPpVWGuVsUP-4Ty7db4DQueL23yjPBMPG4+1Ui+OBTjfUHmVVg@mail.gmail.com>
I narrowed the search to a memmove() called from
skb_reorder_vlan_header() in net/core/skbuff.c.
> memmove(skb->data - ETH_HLEN, skb->data - skb->mac_len - VLAN_HLEN,
> 2 * ETH_ALEN);
Calling skb_reset_mac_len() after skb_reset_mac_header() before
calling br_allowed_ingress() in net/bridge/br_device.c fixes the
problem.
diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index af5b8c87f590..e10131e2f68f 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -58,6 +58,7 @@ netdev_tx_t br_dev_xmit(struct sk_buff *skb, struct
net_device *dev)
BR_INPUT_SKB_CB(skb)->brdev = dev;
skb_reset_mac_header(skb);
+ skb_reset_mac_len(skb);
eth = eth_hdr(skb);
skb_pull(skb, ETH_HLEN);
I'll put together an official patch and submit it. Should I use
another email account? Are my emails being ignored because of that
stupid disclaimer my employer attaches to my messages (outside my
control)?
Brandon
--
CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is
for the sole use of the intended recipient(s) and may contain proprietary,
confidential or privileged information or otherwise be protected by law.
Any unauthorized review, use, disclosure or distribution is prohibited. If
you are not the intended recipient, please notify the sender and destroy
all copies and the original message.
^ permalink raw reply related
* Re: [PATCH 3/4] RFC: net: dsa: Add bindings for Realtek SMI DSAs
From: Andrew Lunn @ 2017-11-29 21:56 UTC (permalink / raw)
To: Linus Walleij
Cc: Vivien Didelot, Florian Fainelli, netdev, Antti Seppälä,
Roman Yeryomin, Colin Leitner, Gabor Juhos,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CACRpkdZVXgFMiHpyUqw7ONYDcq6Htn3rTMRaBJkzd6T3WtX36A@mail.gmail.com>
Hi Linus
> Just that the PHYs are on the MDIO bus inside the switch, of
> course.
I think the problem might be, you are using the DSA provided MDIO bus.
The Marvell switches has a similar setup in terms of interrupts. The
PHY interrupts appear within the switch. So i implemented an interrupt
controller, just the same as you.
The problem is, the DSA provided MDIO bus is not linked to device
tree. So you cannot have phy nodes in device tree associated to it.
What i did for the Marvell driver is that driver itself implements an
MDIO bus (two actually in some chips), and the internal or external
PHYs are placed on the switch drivers MDIO bus, rather than the DSA
MDIO bus. The switch driver MDIO bus links to an mdio node in device
tree. I can then have interrupt properties in the phys on this MDIO
bus in device tree.
What actually might make sense, is to have the DSA MDIO bus look
inside the switches device tree node and see if there is an mdio
node. If so allow dsa_switch_setup() to use of_mdiobus_register()
instead of mdiobus_register().
Andrew
^ permalink raw reply
* Re: [PATCH 3/4] RFC: net: dsa: Add bindings for Realtek SMI DSAs
From: Florian Fainelli @ 2017-11-29 21:48 UTC (permalink / raw)
To: Linus Walleij, Andrew Lunn
Cc: Vivien Didelot, netdev-u79uwXL29TY76Z2rM5mHXA,
Antti Seppälä, Roman Yeryomin, Colin Leitner,
Gabor Juhos,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <CACRpkdZVXgFMiHpyUqw7ONYDcq6Htn3rTMRaBJkzd6T3WtX36A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 11/29/2017 01:28 PM, Linus Walleij wrote:
> On Wed, Nov 29, 2017 at 4:56 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
>>> I have the phy-handle in the ethernet controller. This RTL8366RB
>>> thing is just one big PHY as far as I know.
>>
>> We don't model switches as PHYs. They are their own device type. And
>> the internal or external PHYs are just normal PHYs in the linux
>> model. Meaning their interrupt properties goes in the PHY node in
>> device tree, as documented in the phy.txt binding documentation.
>
> I do model the PHYs on the switch as PHYs.
> They are using the driver in drivers/phy/realtek.c.
That's good.
>
> The interrupts are assigned to the PHYs not to the Switch.
> Just that the PHYs are on the MDIO bus inside the switch, of
> course.
>
> The switch however provides an irqchip to demux the interrupts.
>
> I think there is some misunderstanding in what I'm trying to do..
>
> I have tried learning the DSA ideas by reading e.g. your paper:
> https://www.netdevconf.org/2.1/papers/distributed-switch-architecture.pdf
>
> So I try my best to conform with these ideas.
>
> I however have a hard time testing things since I don't really have a
> system to compare to. What would be useful is to know how
> commands like "ip" and "ifconfig" are used on a typical
> say home router.
There is a mock-up driver: drivers/net/dsa/dsa_loop.c which does not
pass any packets, but at least allows you to exercise user-space tools
and so on.
--
Florian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 3/4] RFC: net: dsa: Add bindings for Realtek SMI DSAs
From: Linus Walleij @ 2017-11-29 21:28 UTC (permalink / raw)
To: Andrew Lunn
Cc: Vivien Didelot, Florian Fainelli, netdev, Antti Seppälä,
Roman Yeryomin, Colin Leitner, Gabor Juhos,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
In-Reply-To: <20171129155627.GD24881@lunn.ch>
On Wed, Nov 29, 2017 at 4:56 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> I have the phy-handle in the ethernet controller. This RTL8366RB
>> thing is just one big PHY as far as I know.
>
> We don't model switches as PHYs. They are their own device type. And
> the internal or external PHYs are just normal PHYs in the linux
> model. Meaning their interrupt properties goes in the PHY node in
> device tree, as documented in the phy.txt binding documentation.
I do model the PHYs on the switch as PHYs.
They are using the driver in drivers/phy/realtek.c.
The interrupts are assigned to the PHYs not to the Switch.
Just that the PHYs are on the MDIO bus inside the switch, of
course.
The switch however provides an irqchip to demux the interrupts.
I think there is some misunderstanding in what I'm trying to do..
I have tried learning the DSA ideas by reading e.g. your paper:
https://www.netdevconf.org/2.1/papers/distributed-switch-architecture.pdf
So I try my best to conform with these ideas.
I however have a hard time testing things since I don't really have a
system to compare to. What would be useful is to know how
commands like "ip" and "ifconfig" are used on a typical
say home router.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect
From: Russell King - ARM Linux @ 2017-11-29 21:20 UTC (permalink / raw)
To: Yan Markman
Cc: Antoine Tenart, andrew@lunn.ch, f.fainelli@gmail.com,
davem@davemloft.net, gregory.clement@free-electrons.com,
thomas.petazzoni@free-electrons.com,
miquel.raynal@free-electrons.com, Nadav Haklai, mw@semihalf.com,
Stefan Chulski, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <21ec97be76d54a6c8a80fd5b56d35678@IL-EXCH01.marvell.com>
On Wed, Nov 29, 2017 at 09:06:56PM +0000, Yan Markman wrote:
> The attached p21 patch doesn't change anything.
> But another one from the mail-text is good
> void phylink_disconnect_phy(struct phylink *pl)
> + pl->phy_state.link = false;
>
> There still (not for my MRVL-PP2) problem:
> It is expected that on ifconfig-down the callback
> pl->ops->mac_link_down(ndev, pl->link_an_mode);
> would be called, but it isn't
Are you calling phylink_stop() or are you just calling phylink_disconnect() ?
You must call phylink_stop() prior to phylink_disconnect(). This
probably explains why the p21 patch did nothing.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* RE: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect
From: Yan Markman @ 2017-11-29 21:06 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Antoine Tenart, andrew@lunn.ch, f.fainelli@gmail.com,
davem@davemloft.net, gregory.clement@free-electrons.com,
thomas.petazzoni@free-electrons.com,
miquel.raynal@free-electrons.com, Nadav Haklai, mw@semihalf.com,
Stefan Chulski, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20171129195911.GG8356@n2100.armlinux.org.uk>
The attached p21 patch doesn't change anything.
But another one from the mail-text is good
void phylink_disconnect_phy(struct phylink *pl)
+ pl->phy_state.link = false;
There still (not for my MRVL-PP2) problem:
It is expected that on ifconfig-down the callback
pl->ops->mac_link_down(ndev, pl->link_an_mode);
would be called, but it isn't
-----Original Message-----
From: Russell King - ARM Linux [mailto:linux@armlinux.org.uk]
Sent: Wednesday, November 29, 2017 9:59 PM
To: Yan Markman <ymarkman@marvell.com>
Cc: Antoine Tenart <antoine.tenart@free-electrons.com>; andrew@lunn.ch; f.fainelli@gmail.com; davem@davemloft.net; gregory.clement@free-electrons.com; thomas.petazzoni@free-electrons.com; miquel.raynal@free-electrons.com; Nadav Haklai <nadavh@marvell.com>; mw@semihalf.com; Stefan Chulski <stefanc@marvell.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: [EXT] Re: [PATCH net] net: phylink: fix link state on phy-connect
External Email
----------------------------------------------------------------------
On Wed, Nov 29, 2017 at 07:33:44PM +0000, Yan Markman wrote:
> Hi Russel
>
> On my board I have [Marvell 88E1510] phy working with STATUS-POLLING I
> see some inconsistencies -- first ifconfig-up is different from furthers, no "link is down" reports.
> Please refer the behavior example below.
> My patch is a "simple solution" -- always reset/clear Link-state-parameters before going UP.
> Possibly, more correct (but much more complicated) solution would be in the phy state machine and phylink resolve modification.
> I just found that
> On ifconfig-down, the phy-state-machine and phylink-resolve
> are stopped before executing before passing over full graceful down/reset state.
> The further ifconfig-up starts with old state parameters.
> Special cases not-tested but logic 2 test-cases are:
> remote side changes speed whilst link is Down or Disconnected. But local ifconfig-up starts with old speed.
Hi,
I think this is covered in my "phy" branch - but could probably do with further testing, specifically this patch (which I've attached):
"phylink: ensure we take the link down when phylink_stop() is called"
This takes the link down on the MAC side synchronously when phylink_stop() is called. However, I think your case might also benefit from this patch - please test the patch referred to without this change, and let me know if you need this change to solve your problem:
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index 8f43f8779317..c90ad50204b0 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -798,6 +798,7 @@ void phylink_disconnect_phy(struct phylink *pl)
mutex_lock(&pl->state_mutex);
pl->netdev->phydev = NULL;
pl->phydev = NULL;
+ pl->phy_state.link = false;
mutex_unlock(&pl->state_mutex);
mutex_unlock(&phy->lock);
flush_work(&pl->resolve);
Thanks.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply
* Re: [PATCH 1/2] net: phy: core: use genphy version of callbacks read_status and config_aneg per default
From: David Miller @ 2017-11-29 20:59 UTC (permalink / raw)
To: hkallweit1; +Cc: f.fainelli, andrew, netdev
In-Reply-To: <8aab2425-9738-f54c-dd57-2cf38236ae3f@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Wed, 29 Nov 2017 21:47:16 +0100
> Am 15.11.2017 um 22:56 schrieb Florian Fainelli:
>> On 11/15/2017 01:42 PM, Heiner Kallweit wrote:
>>> read_status and config_aneg are the only mandatory callbacks and most
>>> of the time the generic implementation is used by drivers.
>>> So make the core fall back to the generic version if a driver doesn't
>>> implement the respective callback.
>>>
>>> Also currently the core doesn't seem to verify that drivers implement
>>> the mandatory calls. If a driver doesn't do so we'd just get a NPE.
>>
>> Right, which is not an unusual way to signal that something is mandatory.
>>
>>> With this patch this potential issue doesn't exit any longer.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>>
>> Note that net-next is closed at the moment, so you will have to resubmit
>> this when the tree opens back again.
>>
> I see that the two patches have status "deferred" in patchwork.
> So do I have to actually resubmit or are they going to be be picked up
> from patchwork?
You must resubmit when the net-next tree opens back up.
^ permalink raw reply
* Re: [PATCH RfC 1/2] net: phy: core: remove now uneeded disabling of interrupts
From: Heiner Kallweit @ 2017-11-29 20:50 UTC (permalink / raw)
To: Florian Fainelli, Andrew Lunn, David Miller
Cc: Ard Biesheuvel, <netdev@vger.kernel.org>
In-Reply-To: <CAKv+Gu904fbsRSZdnCU=qSbsuLamf-=Si5TtS7qX0RhH=0qmdA@mail.gmail.com>
Am 16.11.2017 um 10:51 schrieb Ard Biesheuvel:
> On 15 November 2017 at 22:19, Heiner Kallweit <hkallweit1@gmail.com> wrote:
>> Am 15.11.2017 um 23:04 schrieb Florian Fainelli:
>>> On 11/12/2017 01:08 PM, Heiner Kallweit wrote:
>>>> After commits c974bdbc3e "net: phy: Use threaded IRQ, to allow IRQ from
>>>> sleeping devices" and 664fcf123a30 "net: phy: Threaded interrupts allow
>>>> some simplification" all relevant code pieces run in process context
>>>> anyway and I don't think we need the disabling of interrupts any longer.
>>>>
>>>> Interestingly enough, latter commit already removed the comment
>>>> explaining why interrupts need to be temporarily disabled.
>>>>
>>>> On my system phy interrupt mode works fine with this patch.
>>>> However I may miss something, especially in the context of shared phy
>>>> interrupts, therefore I'd appreciate if more people could test this.
>>>
>>> I am not currently in a position to test this, but this looks very
>>> similar, if not identical to what Ard submitted a few days earlier:
>>>
>>
>> Thanks for the hint. Indeed it's exactly the same patch, so the one
>> sent by me can be disregarded.
>>
>
> Well, it does appear your patch is more complete. Another difference
> is that I actually need this change to fix an issue with a
> hierarchical irqchip stacked on top of the GIC.
>
>
>>> https://patchwork.kernel.org/patch/10048901/
>>>
>>> Since net-next is closed at the moment, that should allow us to give
>>> this some good amount of testing.
>>>
>>> Thanks
>>>
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> For the record
>
> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> Dear maintainers,
>
> Please take whichever of these patches looks more correct to you.
>
> Thanks,
> Ard.
>
These two patches have status RFC in patchwork. Based on Ard's review
and comment, any action to be taken from his or my side?
Rgds, Heiner
>>>> ---
>>>> drivers/net/phy/phy.c | 26 ++------------------------
>>>> include/linux/phy.h | 1 -
>>>> 2 files changed, 2 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>>> index 2b1e67bc1..b3784c9a2 100644
>>>> --- a/drivers/net/phy/phy.c
>>>> +++ b/drivers/net/phy/phy.c
>>>> @@ -629,9 +629,6 @@ static irqreturn_t phy_interrupt(int irq, void *phy_dat)
>>>> if (PHY_HALTED == phydev->state)
>>>> return IRQ_NONE; /* It can't be ours. */
>>>>
>>>> - disable_irq_nosync(irq);
>>>> - atomic_inc(&phydev->irq_disable);
>>>> -
>>>> phy_change(phydev);
>>>>
>>>> return IRQ_HANDLED;
>>>> @@ -689,7 +686,6 @@ static int phy_disable_interrupts(struct phy_device *phydev)
>>>> */
>>>> int phy_start_interrupts(struct phy_device *phydev)
>>>> {
>>>> - atomic_set(&phydev->irq_disable, 0);
>>>> if (request_threaded_irq(phydev->irq, NULL, phy_interrupt,
>>>> IRQF_ONESHOT | IRQF_SHARED,
>>>> phydev_name(phydev), phydev) < 0) {
>>>> @@ -716,13 +712,6 @@ int phy_stop_interrupts(struct phy_device *phydev)
>>>>
>>>> free_irq(phydev->irq, phydev);
>>>>
>>>> - /* If work indeed has been cancelled, disable_irq() will have
>>>> - * been left unbalanced from phy_interrupt() and enable_irq()
>>>> - * has to be called so that other devices on the line work.
>>>> - */
>>>> - while (atomic_dec_return(&phydev->irq_disable) >= 0)
>>>> - enable_irq(phydev->irq);
>>>> -
>>>> return err;
>>>> }
>>>> EXPORT_SYMBOL(phy_stop_interrupts);
>>>> @@ -736,7 +725,7 @@ void phy_change(struct phy_device *phydev)
>>>> if (phy_interrupt_is_valid(phydev)) {
>>>> if (phydev->drv->did_interrupt &&
>>>> !phydev->drv->did_interrupt(phydev))
>>>> - goto ignore;
>>>> + return;
>>>>
>>>> if (phy_disable_interrupts(phydev))
>>>> goto phy_err;
>>>> @@ -748,27 +737,16 @@ void phy_change(struct phy_device *phydev)
>>>> mutex_unlock(&phydev->lock);
>>>>
>>>> if (phy_interrupt_is_valid(phydev)) {
>>>> - atomic_dec(&phydev->irq_disable);
>>>> - enable_irq(phydev->irq);
>>>> -
>>>> /* Reenable interrupts */
>>>> if (PHY_HALTED != phydev->state &&
>>>> phy_config_interrupt(phydev, PHY_INTERRUPT_ENABLED))
>>>> - goto irq_enable_err;
>>>> + goto phy_err;
>>>> }
>>>>
>>>> /* reschedule state queue work to run as soon as possible */
>>>> phy_trigger_machine(phydev, true);
>>>> return;
>>>>
>>>> -ignore:
>>>> - atomic_dec(&phydev->irq_disable);
>>>> - enable_irq(phydev->irq);
>>>> - return;
>>>> -
>>>> -irq_enable_err:
>>>> - disable_irq(phydev->irq);
>>>> - atomic_inc(&phydev->irq_disable);
>>>> phy_err:
>>>> phy_error(phydev);
>>>> }
>>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>>> index dc82a07cb..8a87e441f 100644
>>>> --- a/include/linux/phy.h
>>>> +++ b/include/linux/phy.h
>>>> @@ -468,7 +468,6 @@ struct phy_device {
>>>> /* Interrupt and Polling infrastructure */
>>>> struct work_struct phy_queue;
>>>> struct delayed_work state_queue;
>>>> - atomic_t irq_disable;
>>>>
>>>> struct mutex lock;
>>>>
>>>>
>>>
>>>
>>
>
^ permalink raw reply
* Re: [PATCH 1/2] net: phy: core: use genphy version of callbacks read_status and config_aneg per default
From: Heiner Kallweit @ 2017-11-29 20:47 UTC (permalink / raw)
To: Florian Fainelli, David Miller, Andrew Lunn; +Cc: netdev@vger.kernel.org
In-Reply-To: <1b1e4842-690c-95a7-0d07-d596ce97bba5@gmail.com>
Am 15.11.2017 um 22:56 schrieb Florian Fainelli:
> On 11/15/2017 01:42 PM, Heiner Kallweit wrote:
>> read_status and config_aneg are the only mandatory callbacks and most
>> of the time the generic implementation is used by drivers.
>> So make the core fall back to the generic version if a driver doesn't
>> implement the respective callback.
>>
>> Also currently the core doesn't seem to verify that drivers implement
>> the mandatory calls. If a driver doesn't do so we'd just get a NPE.
>
> Right, which is not an unusual way to signal that something is mandatory.
>
>> With this patch this potential issue doesn't exit any longer.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
>
> Note that net-next is closed at the moment, so you will have to resubmit
> this when the tree opens back again.
>
I see that the two patches have status "deferred" in patchwork.
So do I have to actually resubmit or are they going to be be picked up
from patchwork?
Rgds, Heiner
>> ---
>> drivers/net/phy/phy.c | 5 ++++-
>> include/linux/phy.h | 33 ++++++++++++++++++---------------
>> 2 files changed, 22 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>> index 2b1e67bc1..a0e7605dc 100644
>> --- a/drivers/net/phy/phy.c
>> +++ b/drivers/net/phy/phy.c
>> @@ -493,7 +493,10 @@ static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
>> /* Invalidate LP advertising flags */
>> phydev->lp_advertising = 0;
>>
>> - err = phydev->drv->config_aneg(phydev);
>> + if (phydev->drv->config_aneg)
>> + err = phydev->drv->config_aneg(phydev);
>> + else
>> + err = genphy_config_aneg(phydev);
>> if (err < 0)
>> goto out_unlock;
>>
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index dc82a07cb..958b5162a 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -497,13 +497,13 @@ struct phy_device {
>> * flags: A bitfield defining certain other features this PHY
>> * supports (like interrupts)
>> *
>> - * The drivers must implement config_aneg and read_status. All
>> - * other functions are optional. Note that none of these
>> - * functions should be called from interrupt time. The goal is
>> - * for the bus read/write functions to be able to block when the
>> - * bus transaction is happening, and be freed up by an interrupt
>> - * (The MPC85xx has this ability, though it is not currently
>> - * supported in the driver).
>> + * All functions are optional. If config_aneg or read_status
>> + * are not implemented, the phy core uses the genphy versions.
>> + * Note that none of these functions should be called from
>> + * interrupt time. The goal is for the bus read/write functions
>> + * to be able to block when the bus transaction is happening,
>> + * and be freed up by an interrupt (The MPC85xx has this ability,
>> + * though it is not currently supported in the driver).
>> */
>> struct phy_driver {
>> struct mdio_driver_common mdiodrv;
>> @@ -841,14 +841,6 @@ int phy_aneg_done(struct phy_device *phydev);
>> int phy_stop_interrupts(struct phy_device *phydev);
>> int phy_restart_aneg(struct phy_device *phydev);
>>
>> -static inline int phy_read_status(struct phy_device *phydev)
>> -{
>> - if (!phydev->drv)
>> - return -EIO;
>> -
>> - return phydev->drv->read_status(phydev);
>> -}
>> -
>> #define phydev_err(_phydev, format, args...) \
>> dev_err(&_phydev->mdio.dev, format, ##args)
>>
>> @@ -890,6 +882,17 @@ int genphy_c45_read_pma(struct phy_device *phydev);
>> int genphy_c45_pma_setup_forced(struct phy_device *phydev);
>> int genphy_c45_an_disable_aneg(struct phy_device *phydev);
>>
>> +static inline int phy_read_status(struct phy_device *phydev)
>> +{
>> + if (!phydev->drv)
>> + return -EIO;
>> +
>> + if (phydev->drv->read_status)
>> + return phydev->drv->read_status(phydev);
>> + else
>> + return genphy_read_status(phydev);
>> +}
>> +
>> void phy_driver_unregister(struct phy_driver *drv);
>> void phy_drivers_unregister(struct phy_driver *drv, int n);
>> int phy_driver_register(struct phy_driver *new_driver, struct module *owner);
>>
>
>
^ permalink raw reply
* Re: KASAN: use-after-free Read in sock_release
From: Eric Dumazet @ 2017-11-29 20:49 UTC (permalink / raw)
To: Cong Wang, syzbot
Cc: David Miller, LKML, Linux Kernel Network Developers,
syzkaller-bugs, linux-fsdevel, Linus Torvalds, Al Viro
In-Reply-To: <CAM_iQpUuW=34pxJtkfw_sHTzmm3mUfrg-Fs3++kU+PktBfDJNg@mail.gmail.com>
On Wed, 2017-11-29 at 11:37 -0800, Cong Wang wrote:
> (Cc'ing fs people...)
>
> On Wed, Nov 29, 2017 at 12:33 AM, syzbot
> <bot+9abea25706ae35022385a41f61e579ed66e88a3f@syzkaller.appspotmail.c
> om>
> wrote:
> > Hello,
> >
> > syzkaller hit the following crash on
> > 1d3b78bbc6e983fabb3fbf91b76339bf66e4a12c
> > git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-
> > next.git/master
> > compiler: gcc (GCC) 7.1.1 20170620
> > .config is attached
> > Raw console output is attached.
> >
> > Unfortunately, I don't have any reproducer for this bug yet.
> >
> >
> > device syz3 left promiscuous mode
> > device syz3 entered promiscuous mode
> > ==================================================================
> > BUG: KASAN: use-after-free in sock_release+0x1c6/0x1e0
> > net/socket.c:601
> > Read of size 8 at addr ffff8801c8dd1d10 by task syz-executor4/31085
> >
> > CPU: 0 PID: 31085 Comm: syz-executor4 Not tainted 4.14.0+ #129
> > Hardware name: Google Google Compute Engine/Google Compute Engine,
> > BIOS
> > Google 01/01/2011
> > Call Trace:
> > __dump_stack lib/dump_stack.c:17 [inline]
> > dump_stack+0x194/0x257 lib/dump_stack.c:53
> > print_address_description+0x73/0x250 mm/kasan/report.c:252
> > kasan_report_error mm/kasan/report.c:351 [inline]
> > kasan_report+0x25b/0x340 mm/kasan/report.c:409
> > __asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:430
> > sock_release+0x1c6/0x1e0 net/socket.c:601
> > sock_close+0x16/0x20 net/socket.c:1125
> > __fput+0x333/0x7f0 fs/file_table.c:210
> > ____fput+0x15/0x20 fs/file_table.c:244
> > task_work_run+0x199/0x270 kernel/task_work.c:113
> > exit_task_work include/linux/task_work.h:22 [inline]
> > do_exit+0x9bb/0x1ae0 kernel/exit.c:865
> > do_group_exit+0x149/0x400 kernel/exit.c:968
> > get_signal+0x73f/0x16c0 kernel/signal.c:2335
> > do_signal+0x94/0x1ee0 arch/x86/kernel/signal.c:809
> > exit_to_usermode_loop+0x214/0x310 arch/x86/entry/common.c:158
> > prepare_exit_to_usermode arch/x86/entry/common.c:195 [inline]
> > syscall_return_slowpath+0x490/0x550 arch/x86/entry/common.c:264
> > entry_SYSCALL_64_fastpath+0x94/0x96
> > RIP: 0033:0x452879
> > RSP: 002b:00007fb1c2435ce8 EFLAGS: 00000246 ORIG_RAX:
> > 00000000000000ca
> > RAX: fffffffffffffe00 RBX: 0000000000758100 RCX: 0000000000452879
> > RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000758100
> > RBP: 0000000000758100 R08: 0000000000000304 R09: 00000000007580d8
> > R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
> > R13: 0000000000a6f7ff R14: 00007fb1c24369c0 R15: 000000000000000e
> >
> > Allocated by task 31066:
> > save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> > set_track mm/kasan/kasan.c:459 [inline]
> > kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
> > kmem_cache_alloc_trace+0x136/0x750 mm/slab.c:3613
> > kmalloc include/linux/slab.h:499 [inline]
> > sock_alloc_inode+0xb4/0x300 net/socket.c:253
> > alloc_inode+0x65/0x180 fs/inode.c:208
> > new_inode_pseudo+0x69/0x190 fs/inode.c:890
> > sock_alloc+0x41/0x270 net/socket.c:565
> > __sock_create+0x148/0x850 net/socket.c:1225
> > sock_create net/socket.c:1301 [inline]
> > SYSC_socket net/socket.c:1331 [inline]
> > SyS_socket+0xeb/0x200 net/socket.c:1311
> > entry_SYSCALL_64_fastpath+0x1f/0x96
> >
> > Freed by task 3039:
> > save_stack+0x43/0xd0 mm/kasan/kasan.c:447
> > set_track mm/kasan/kasan.c:459 [inline]
> > kasan_slab_free+0x71/0xc0 mm/kasan/kasan.c:524
> > __cache_free mm/slab.c:3491 [inline]
> > kfree+0xca/0x250 mm/slab.c:3806
> > __rcu_reclaim kernel/rcu/rcu.h:190 [inline]
> > rcu_do_batch kernel/rcu/tree.c:2758 [inline]
> > invoke_rcu_callbacks kernel/rcu/tree.c:3012 [inline]
> > __rcu_process_callbacks kernel/rcu/tree.c:2979 [inline]
> > rcu_process_callbacks+0xe79/0x17d0 kernel/rcu/tree.c:2996
> > __do_softirq+0x29d/0xbb2 kernel/softirq.c:285
>
> This looks more like a fs issue than network, my fs knowledge
> is not good enough to justify why the hell the inode could be
> destroyed before we release the fd.
>
> My _guess_ is that it is because we defer the ____fput() to a
> task work. If this is the case, then fs layer is not guilty for this.
>
> On the other hand, if we have to blame net layer, it does look
> suspicious on the RCU usage in sock_release() where we
> claim RCU protection but I don't see we hold any RCU lock
> there.
There is rcu protection for sock->wq, and the 1 in
rcu_dereference_protected(sock->wq, 1) is because we do not have a
lockdep convenient way to express that we are the last user of sock,
and about to free it.
> Also, the code that deferences sock->wq is pretty much
> useless now, at least I don't see it catches any bug though.
>
>
> diff --git a/net/socket.c b/net/socket.c
> index 42d8e9c9ccd5..b2390b5591a9 100644
> --- a/net/socket.c
> +++ b/net/socket.c
> @@ -598,9 +598,6 @@ void sock_release(struct socket *sock)
> module_put(owner);
> }
>
> - if (rcu_dereference_protected(sock->wq, 1)->fasync_list)
> - pr_err("%s: fasync list not empty!\n", __func__);
> -
>
At this point, sock->wq must be valid, and freed later (by us)
This really looks like some other bug, and a late effect.
^ permalink raw reply
* Re: [PATCH v2 25/35] nds32: Build infrastructure
From: Arnd Bergmann @ 2017-11-29 20:27 UTC (permalink / raw)
To: Greentime Hu
Cc: Geert Uytterhoeven, Greentime, Linux Kernel Mailing List,
linux-arch, Thomas Gleixner, Jason Cooper, Marc Zyngier,
Rob Herring, Networking, Vincent Chen, DTML, Al Viro,
David Howells, Will Deacon, Daniel Lezcano,
linux-serial@vger.kernel.org, Vincent Chen
In-Reply-To: <CAEbi=3fmXP=n_tS+-TSFhdht2G=VHOzVCVU=SRy4A6Ym_GToUQ@mail.gmail.com>
On Wed, Nov 29, 2017 at 3:10 PM, Greentime Hu <green.hu@gmail.com> wrote:
> 2017-11-29 19:57 GMT+08:00 Arnd Bergmann <arnd@arndb.de>:
>> On Wed, Nov 29, 2017 at 12:39 PM, Greentime Hu <green.hu@gmail.com> wrote:
>>>
>>> How about this?
>>>
>>> choice
>>> prompt "CPU type"
>>> default CPU_N13
>>> config CPU_N15
>>> bool "AndesCore N15"
>>> select CPU_CACHE_NONALIASING
>>> config CPU_N13
>>> bool "AndesCore N13"
>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> config CPU_N10
>>> bool "AndesCore N10"
>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> config CPU_D15
>>> bool "AndesCore D15"
>>> select CPU_CACHE_NONALIASING
>>> select HWZOL
>>> config CPU_D10
>>> bool "AndesCore D10"
>>> select CPU_CACHE_NONALIASING if ANDES_PAGE_SIZE_8KB
>>> endchoice
>>
>> With a 'choice' statement this works, but I would consider that
>> suboptimal for another reason: now you cannot build a kernel that
>> works on e.g. both N13 and N15.
>>
>> This is what we had on ARM a long time ago and on MIPS not so long
>> ago, but it's really a burden for testing and distribution once you get
>> support for more than a handful of machines supported in the mainline
>> kernel: If each CPU core is mutually incompatible with the other ones,
>> this means you likely end up having one defconfig per CPU core,
>> or even one defconfig per SoC or board.
>>
>> I would always try to get the largest amount of hardware to work
>> in the same kernel concurrently.
>>
>> One way of of this would be to define the "CPU type" as the minimum
>> supported CPU, e.g. selecting D15 would result in a kernel that
>> only works on D15, while selecting N15 would work on both N15 and
>> D15, and selecting D10 would work on both D10 and D15.
>>
>
> Hi, Arnd:
>
> Maybe we should keep the original implementation for this reason.
> The default value of CPU_CACHE_NONALIASING and ANDES_PAGE_SIZE_8KB is
> available for all CPU types for now.
> User can use these configs built kernel to boot on almost all nds32 CPUs.
>
> It might be a little bit weird if we config CPU_N10 but run on a N13 CPU.
> This might confuse our users.
I think it really depends on how much flexibility you want to give to users.
The way I suggested first was to allow selecting an arbitrary combination
of CPUs, and then let Kconfig derive the correct set of optimization flags
and other options that work with those. That is probably the easiest for
the users, but can be tricky to get right for all combinations.
When you put them in a sorted list like I mentioned for simplicity, you
could reduce the confusion by naming them differently, e.g.
CONFIG_CPU_N10_OR_NEWER.
Having only the CPU_CACHE_NONALIASING option is fine if you
never need to make any other decisions based on the CPU core
type, but then the help text should describe specifically which cases
are affected (N10/N13/D13 with 4K page size), and you can decide to
hide the option and make it always-on when using 8K page size.
Arnd
^ permalink raw reply
* Re: KASAN: use-after-free Read in sock_release
From: Linus Torvalds @ 2017-11-29 20:24 UTC (permalink / raw)
To: Cong Wang, Al Viro
Cc: syzbot, David Miller, LKML, Linux Kernel Network Developers,
syzkaller-bugs, linux-fsdevel
In-Reply-To: <CAM_iQpUuW=34pxJtkfw_sHTzmm3mUfrg-Fs3++kU+PktBfDJNg@mail.gmail.com>
On Wed, Nov 29, 2017 at 11:37 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> (Cc'ing fs people...)
>
> On Wed, Nov 29, 2017 at 12:33 AM, syzbot wrote:
>> BUG: KASAN: use-after-free in sock_release+0x1c6/0x1e0 net/socket.c:601
Lovely.
Yeah, that is:
601 if (rcu_dereference_protected(sock->wq, 1)->fasync_list)
and as you say, that "rcu_dereference_protected()" is confusing, but
that should be ok because we have a ref to the inode, and we're really
just testing that the pointer is zero.
The call trace here is:
>> sock_release+0x1c6/0x1e0 net/socket.c:601
>> sock_close+0x16/0x20 net/socket.c:1125
>> __fput+0x333/0x7f0 fs/file_table.c:210
>> ____fput+0x15/0x20 fs/file_table.c:244
>> task_work_run+0x199/0x270 kernel/task_work.c:113
and there is no RCU protection anywhere, but it's really just a sanity
check, and the access _should_ be ok.
The stale access does seem to be because 'sock' (embedded in the
inode) itself that has been free'd:
>> Allocated by task 31066:
>> kmalloc include/linux/slab.h:499 [inline]
>> sock_alloc_inode+0xb4/0x300 net/socket.c:253
>> alloc_inode+0x65/0x180 fs/inode.c:208
>> new_inode_pseudo+0x69/0x190 fs/inode.c:890
>> sock_alloc+0x41/0x270 net/socket.c:565
>> __sock_create+0x148/0x850 net/socket.c:1225
>> sock_create net/socket.c:1301 [inline]
>> SYSC_socket net/socket.c:1331 [inline]
>> SyS_socket+0xeb/0x200 net/socket.c:1311
>
> This looks more like a fs issue than network, my fs knowledge
> is not good enough to justify why the hell the inode could be
> destroyed before we release the fd.
Ugh. The inode freeing really is confusing and fairly involved, but
the last free *should* happen as part of the final dput() that is done
at the end of __fput().
So in __fput() calls into the
if (file->f_op->release)
file->f_op->release(inode, file);
then the inode should still be around, because the final ref won't be
done until later. And RCU simply shouldn't be an issue, because of
that reference count on the inode.
So it smells like some reference counting went wrong. The socket inode
creation is a bit confusing, and then in "sock_release()" we do have
that
if (!sock->file) {
iput(SOCK_INODE(sock));
return;
}
sock->file = NULL;
which *also* tries to free the inode. I'm not sure what the logic (and
what the locking) behind that code all is.
What *is* the locking for "sock->file" anyway?
Al, can you take a look on the vfs side? But I'm inclined to blame the
socket code, because if we really had a "inode free'd early" issue at
a vfs level, I'd have expected us to see infinite chaos.
Linus
^ permalink raw reply
* Re: [BUG] kernel stack corruption during/after Netlabel error
From: Eric Dumazet @ 2017-11-29 20:23 UTC (permalink / raw)
To: Stephen Smalley; +Cc: James Morris, Paul Moore, netdev, selinux
In-Reply-To: <1511985547.10464.22.camel@tycho.nsa.gov>
On Wed, Nov 29, 2017 at 11:59 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Wed, 2017-11-29 at 09:34 -0800, Eric Dumazet wrote:
>> On Wed, Nov 29, 2017 at 9:31 AM, Stephen Smalley <sds@tycho.nsa.gov>
>> wrote:
>> > On Wed, 2017-11-29 at 21:26 +1100, James Morris wrote:
>> > > I'm seeing a kernel stack corruption bug (detected via gcc) when
>> > > running
>> > > the SELinux testsuite on a 4.15-rc1 kernel, in the 2nd
>> > > inet_socket
>> > > test:
>> > >
>> > > https://github.com/SELinuxProject/selinux-testsuite/blob/master/t
>> > > ests
>> > > /inet_socket/test
>> > >
>> > > # Verify that unauthorized client cannot communicate with the
>> > > server.
>> > > $result = system
>> > > "runcon -t test_inet_bad_client_t -- $basedir/client stream
>> > > 127.0.0.1 65535 2>&1";
>> > >
>> > > This correctlly causes an access control error in the Netlabel
>> > > code,
>> > > and
>> > > the bug seems to be triggered during the ICMP send:
>> > >
>> > > [ 339.806024] SELinux: failure in selinux_parse_skb(), unable to
>> > > parse packet
>> > > [ 339.822505] Kernel panic - not syncing: stack-protector:
>> > > Kernel
>> > > stack is corrupted in: ffffffff81745af5
>> > > [ 339.822505]
>> > > [ 339.852250] CPU: 4 PID: 3642 Comm: client Not tainted 4.15.0-
>> > > rc1-
>> > > test #15
>> > > [ 339.868498] Hardware name: LENOVO 10FGS0VA1L/30BC, BIOS
>> > > FWKT68A 01/19/2017
>> > > [ 339.885060] Call Trace:
>> > > [ 339.896875] <IRQ>
>> > > [ 339.908103] dump_stack+0x63/0x87
>> > > [ 339.920645] panic+0xe8/0x248
>> > > [ 339.932668] ? ip_push_pending_frames+0x33/0x40
>> > > [ 339.946328] ? icmp_send+0x525/0x530
>> > > [ 339.958861] ? kfree_skbmem+0x60/0x70
>> > > [ 339.971431] __stack_chk_fail+0x1b/0x20
>> > > [ 339.984049] icmp_send+0x525/0x530
>> > > [ 339.996205] ? netlbl_skbuff_err+0x36/0x40
>> > > [ 340.008997] ? selinux_netlbl_err+0x11/0x20
>> > > [ 340.021816] ? selinux_socket_sock_rcv_skb+0x211/0x230
>> > > [ 340.035529] ? security_sock_rcv_skb+0x3b/0x50
>> > > [ 340.048471] ? sk_filter_trim_cap+0x44/0x1c0
>> > > [ 340.061246] ? tcp_v4_inbound_md5_hash+0x69/0x1b0
>> > > [ 340.074562] ? tcp_filter+0x2c/0x40
>> > > [ 340.086400] ? tcp_v4_rcv+0x820/0xa20
>> > > [ 340.098329] ? ip_local_deliver_finish+0x71/0x1a0
>> > > [ 340.111279] ? ip_local_deliver+0x6f/0xe0
>> > > [ 340.123535] ? ip_rcv_finish+0x3a0/0x3a0
>> > > [ 340.135523] ? ip_rcv_finish+0xdb/0x3a0
>> > > [ 340.147442] ? ip_rcv+0x27c/0x3c0
>> > > [ 340.158668] ? inet_del_offload+0x40/0x40
>> > > [ 340.170580] ? __netif_receive_skb_core+0x4ac/0x900
>> > > [ 340.183285] ? rcu_accelerate_cbs+0x5b/0x80
>> > > [ 340.195282] ? __netif_receive_skb+0x18/0x60
>> > > [ 340.207288] ? process_backlog+0x95/0x140
>> > > [ 340.218948] ? net_rx_action+0x26c/0x3b0
>> > > [ 340.230416] ? __do_softirq+0xc9/0x26a
>> > > [ 340.241625] ? do_softirq_own_stack+0x2a/0x40
>> > > [ 340.253368] </IRQ>
>> > > [ 340.262673] ? do_softirq+0x50/0x60
>> > > [ 340.273450] ? __local_bh_enable_ip+0x57/0x60
>> > > [ 340.285045] ? ip_finish_output2+0x175/0x350
>> > > [ 340.296403] ? ip_finish_output+0x127/0x1d0
>> > > [ 340.307665] ? nf_hook_slow+0x3c/0xb0
>> > > [ 340.318230] ? ip_output+0x72/0xe0
>> > > [ 340.328524] ? ip_fragment.constprop.54+0x80/0x80
>> > > [ 340.340070] ? ip_local_out+0x35/0x40
>> > > [ 340.350497] ? ip_queue_xmit+0x15c/0x3f0
>> > > [ 340.361060] ? __kmalloc_reserve.isra.40+0x31/0x90
>> > > [ 340.372484] ? __skb_clone+0x2e/0x130
>> > > [ 340.382633] ? tcp_transmit_skb+0x558/0xa10
>> > > [ 340.393262] ? tcp_connect+0x938/0xad0
>> > > [ 340.403370] ? ktime_get_with_offset+0x4c/0xb0
>> > > [ 340.414206] ? tcp_v4_connect+0x457/0x4e0
>> > > [ 340.424471] ? __inet_stream_connect+0xb3/0x300
>> > > [ 340.435195] ? inet_stream_connect+0x3b/0x60
>> > > [ 340.445607] ? SYSC_connect+0xd9/0x110
>> > > [ 340.455455] ? __audit_syscall_entry+0xaf/0x100
>> > > [ 340.466112] ? syscall_trace_enter+0x1d0/0x2b0
>> > > [ 340.476636] ? __audit_syscall_exit+0x209/0x290
>> > > [ 340.487151] ? SyS_connect+0xe/0x10
>> > > [ 340.496453] ? do_syscall_64+0x67/0x1b0
>> > > [ 340.506078] ? entry_SYSCALL64_slow_path+0x25/0x25
>> > > [ 340.516693] Kernel Offset: disabled
>> > > [ 340.526393] Rebooting in 11 seconds..
>> > >
>> > > This is mostly reliable, and I'm only seeing it on bare metal
>> > > (not in
>> > > a
>> > > virtualbox vm).
>> > >
>> > > The SELinux skb parse error at the start only sometimes appears,
>> > > and
>> > > looking at the code, I suspect some kind of memory corruption
>> > > being
>> > > the
>> > > cause at that point (basic packet header checks).
>> > >
>> > > I bisected the bug down to the following change:
>> > >
>> > > commit bffa72cf7f9df842f0016ba03586039296b4caaf
>> > > Author: Eric Dumazet <edumazet@google.com>
>> > > Date: Tue Sep 19 05:14:24 2017 -0700
>> > >
>> > > net: sk_buff rbnode reorg
>> > > ...
>> > >
>> > >
>> > > Anyone else able to reproduce this, or have any ideas on what's
>> > > happening?
>> >
>> > So far I haven't been able to reproduce with 4.15-rc1 or -linus.
>> >
>>
>> You might try adding KASAN in the picture ? ( CONFIG_KASAN=y )
>
> Good idea:
>
> ==================================================================
> BUG:
> KASAN: stack-out-of-bounds in __ip_options_echo+0x430/0x5e0
> Write of
> size 44 at addr ffff8803bc1c7560 by task ksoftirqd/1/16
>
> CPU: 1 PID: 16
> Comm: ksoftirqd/1 Not tainted 4.15.0-rc1 #27
> Hardware name: Dell Inc.
> Latitude E7470/0VNKRJ, BIOS 1.16.4 06/02/2017
> Call Trace:
>
> dump_stack+0x5c/0x7c
> print_address_description+0x6a/0x280
>
> kasan_report+0x254/0x370
> ? __ip_options_echo+0x430/0x5e0
>
> memcpy+0x34/0x50
> __ip_options_echo+0x430/0x5e0
> icmp_send+0x48d/0x7a0
>
> ? icmpv4_global_allow+0x50/0x50
> ?
> selinux_netlbl_sk_security_reset+0x20/0x20
> ? avc_has_perm+0x238/0x260
>
> ? avc_has_perm_noaudit+0x1d0/0x1d0
> ? selinux_peerlbl_enabled+0x50/0x50
> ? deref_stack_reg+0xd0/0xd0
> ? __save_stack_trace+0x82/0x100
>
> selinux_socket_sock_rcv_skb+0x341/0x350
> ?
> selinux_sock_rcv_skb_compat+0x200/0x200
> ? depot_save_stack+0x12f/0x460
> ? pskb_expand_head+0x9d/0x4d0
> ? save_stack+0x92/0xa0
> ?
> kasan_kmalloc+0xa0/0xd0
> ? __kmalloc_node_track_caller+0xf5/0x290
> ?
> skb_copy_and_csum_dev+0x142/0x180
> ? ip_rcv_finish+0x323/0x690
> ?
> __netif_receive_skb_core+0xe16/0x13d0
> ? process_backlog+0x10a/0x280
> ?
> net_rx_action+0x3ec/0x5a0
> ? __do_softirq+0x13f/0x36d
> ?
> __wake_up_common_lock+0xd7/0x130
> ? tcp_md5_do_lookup+0x27/0x240
>
> security_sock_rcv_skb+0x47/0x60
> sk_filter_trim_cap+0x45/0x4b0
> ?
> tcp4_proc_exit+0x11/0x11
> tcp_filter+0x5b/0x90
>
> tcp_v4_rcv+0x108a/0x1360
> ip_local_deliver_finish+0xf7/0x300
>
> ip_local_deliver+0xf2/0x1a0
> ? ip_call_ra_chain+0x220/0x220
> ?
> ip_rcv_finish+0x690/0x690
> ? ip_rcv_finish+0x1b9/0x690
>
> ip_rcv+0x4a6/0x660
> ? ip_local_deliver+0x1a0/0x1a0
> ?
> inet_del_offload+0x40/0x40
> ? cpumask_next_and+0x4e/0x70
> ?
> ip_local_deliver+0x1a0/0x1a0
> __netif_receive_skb_core+0xe16/0x13d0
> ?
> netdev_info+0x100/0x100
> ? __accumulate_pelt_segments+0x47/0xd0
> ?
> find_busiest_group+0x1100/0x1100
> ?
> __update_load_avg_se.isra.31+0x34e/0x360
> ?
> __update_load_avg_se.isra.31+0x201/0x360
> ?
> __accumulate_pelt_segments+0x47/0xd0
> ? process_backlog+0x10a/0x280
>
> process_backlog+0x10a/0x280
> net_rx_action+0x3ec/0x5a0
> ?
> napi_complete_done+0x180/0x180
> ? __schedule+0x4e0/0xd50
> ?
> sched_clock_cpu+0x14/0xe0
> __do_softirq+0x13f/0x36d
> ?
> takeover_tasklets+0x2b0/0x2b0
> run_ksoftirqd+0x25/0x40
>
> smpboot_thread_fn+0x212/0x2b0
> ? sort_range+0x20/0x20
> ?
> schedule+0x50/0xc0
> kthread+0x174/0x1c0
> ? sort_range+0x20/0x20
> ?
> kthread_create_worker_on_cpu+0xc0/0xc0
> ret_from_fork+0x1f/0x30
>
> The
> buggy address belongs to the page:
> page:ffffea000ef071c0 count:0
> mapcount:0 mapping: (null) index:0x0
> flags: 0x17ffffc0000000()
> raw: 0017ffffc0000000 0000000000000000 0000000000000000
> 00000000ffffffff
> raw: ffffea000ef071e0 ffffea000ef071e0
> 0000000000000000 0000000000000000
> page dumped because: kasan: bad
> access detected
>
> Memory state around the buggy address:
>
> ffff8803bc1c7480: 00 00 00 f2 f2 f2 f2 00 00 00 00 00 00 00 f4 f2
>
> ffff8803bc1c7500: f2 f2 f2 00 00 00 00 00 00 00 00 00 00 00 00 00
>
>>ffff8803bc1c7580: 00 f4 f4 00 00 00 00 00 00 00 00 00 00 00 00 00
>
> ^
> ffff8803bc1c7600: 00 00 f1 f1 f1 f1 04 f4 f4 f4 f2
> f2 f2 f2 00 f4
> ffff8803bc1c7680: f4 f4 f2 f2 f2 f2 00 00 00 00 f2 f2
> f2 f2 00 00
>
> ==================================================================
>
> Disabling lock debugging due to kernel taint
> SELinux: failure in
> selinux_parse_skb(), unable to parse packet
I suspect this exposes an ancient bug, caused by fact that TCP moves
IP[6]CB in skb->cb[]
Basically the 2nd tcp_filter() added in commit
8fac365f63c866a00015fa13932d8ffc584518b8
("tcp: Add a tcp_filter hook before handle ack packet") was not
expecting selinux code being called a 2nd time,
while skb->cb[] has been mangled [1]
[1]
memmove(&TCP_SKB_CB(skb)->header.h4, IPCB(skb),
sizeof(struct inet_skb_parm));
^ permalink raw reply
* [PATCH v2 02/17] radix tree test suite: Remove ARRAY_SIZE
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
This is now defined in tools/include/linux/kernel.h, so our
definition generates a warning.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
tools/testing/radix-tree/linux/kernel.h | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/testing/radix-tree/linux/kernel.h b/tools/testing/radix-tree/linux/kernel.h
index c3bc3f364f68..426f32f28547 100644
--- a/tools/testing/radix-tree/linux/kernel.h
+++ b/tools/testing/radix-tree/linux/kernel.h
@@ -17,6 +17,4 @@
#define pr_debug printk
#define pr_cont printk
-#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]))
-
#endif /* _KERNEL_H */
--
2.15.0
^ permalink raw reply related
* [PATCH v2 04/17] IDR test suite: Check handling negative end correctly
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
One of the charming quirks of the idr_alloc() interface is that you
can pass a negative end and it will be interpreted as "maximum". Ensure
we don't break that.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
tools/testing/radix-tree/idr-test.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/radix-tree/idr-test.c b/tools/testing/radix-tree/idr-test.c
index 193450b29bf0..892ef8855b02 100644
--- a/tools/testing/radix-tree/idr-test.c
+++ b/tools/testing/radix-tree/idr-test.c
@@ -207,6 +207,7 @@ void idr_checks(void)
assert(idr_alloc(&idr, item, i, i + 10, GFP_KERNEL) == i);
}
assert(idr_alloc(&idr, DUMMY_PTR, i - 2, i, GFP_KERNEL) == -ENOSPC);
+ assert(idr_alloc(&idr, DUMMY_PTR, i - 2, i + 10, GFP_KERNEL) == -ENOSPC);
idr_for_each(&idr, item_idr_free, &idr);
idr_destroy(&idr);
--
2.15.0
^ permalink raw reply related
* [PATCH v2 09/17] net sched actions: Convert to use idr_alloc_u32
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Use the new helper. Also untangle the error path, and in so doing
noticed that estimator generator failure would lead to us leaking an
ID. Fix that bug.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/act_api.c | 60 ++++++++++++++++++++++-------------------------------
1 file changed, 25 insertions(+), 35 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index efb90b8a3bf0..156302c110af 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -274,7 +274,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
struct tcf_idrinfo *idrinfo = tn->idrinfo;
struct idr *idr = &idrinfo->action_idr;
int err = -ENOMEM;
- unsigned long idr_index;
if (unlikely(!p))
return -ENOMEM;
@@ -284,45 +283,28 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
if (cpustats) {
p->cpu_bstats = netdev_alloc_pcpu_stats(struct gnet_stats_basic_cpu);
- if (!p->cpu_bstats) {
-err1:
- kfree(p);
- return err;
- }
- p->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
- if (!p->cpu_qstats) {
-err2:
- free_percpu(p->cpu_bstats);
+ if (!p->cpu_bstats)
goto err1;
- }
+ p->cpu_qstats = alloc_percpu(struct gnet_stats_queue);
+ if (!p->cpu_qstats)
+ goto err2;
}
spin_lock_init(&p->tcfa_lock);
+ idr_preload(GFP_KERNEL);
+ spin_lock_bh(&idrinfo->lock);
/* user doesn't specify an index */
if (!index) {
- idr_preload(GFP_KERNEL);
- spin_lock_bh(&idrinfo->lock);
- err = idr_alloc_ext(idr, NULL, &idr_index, 1, 0,
- GFP_ATOMIC);
- spin_unlock_bh(&idrinfo->lock);
- idr_preload_end();
- if (err) {
-err3:
- free_percpu(p->cpu_qstats);
- goto err2;
- }
- p->tcfa_index = idr_index;
+ index = 1;
+ err = idr_alloc_u32(idr, NULL, &index, UINT_MAX, GFP_ATOMIC);
} else {
- idr_preload(GFP_KERNEL);
- spin_lock_bh(&idrinfo->lock);
- err = idr_alloc_ext(idr, NULL, NULL, index, index + 1,
- GFP_ATOMIC);
- spin_unlock_bh(&idrinfo->lock);
- idr_preload_end();
- if (err)
- goto err3;
- p->tcfa_index = index;
+ err = idr_alloc_u32(idr, NULL, &index, index, GFP_ATOMIC);
}
+ spin_unlock_bh(&idrinfo->lock);
+ idr_preload_end();
+ if (err)
+ goto err3;
+ p->tcfa_index = index;
p->tcfa_tm.install = jiffies;
p->tcfa_tm.lastuse = jiffies;
p->tcfa_tm.firstuse = 0;
@@ -330,9 +312,8 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
err = gen_new_estimator(&p->tcfa_bstats, p->cpu_bstats,
&p->tcfa_rate_est,
&p->tcfa_lock, NULL, est);
- if (err) {
- goto err3;
- }
+ if (err)
+ goto err4;
}
p->idrinfo = idrinfo;
@@ -340,6 +321,15 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
INIT_LIST_HEAD(&p->list);
*a = p;
return 0;
+err4:
+ idr_remove(idr, index);
+err3:
+ free_percpu(p->cpu_qstats);
+err2:
+ free_percpu(p->cpu_bstats);
+err1:
+ kfree(p);
+ return err;
}
EXPORT_SYMBOL(tcf_idr_create);
--
2.15.0
^ permalink raw reply related
* [PATCH v2 00/17] IDR changes for v4.15-rc1
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
From: Matthew Wilcox <mawilcox@microsoft.com>
v2:
- Rebased on f6454f80e8a965fca203dab28723f68ec78db608 to resolve
conflicting changes with cls_bpf
- Fix whitespace
- Change a WARN_ON to WARN_ON_ONCE (git snafu yesterday)
Original cover letter:
The patches here are of three types:
- Enhancing the test suite (fixing the build, adding a couple of new
tests, fixing a bug in the test)
- Replacing the 'extended' IDR API
- Fixing some low-probability bugs
As far as the 'extended' IDR API goes, this was added by Chris Mi to
permit a savvy user to use IDs up to ULONG_MAX in size (the traditional
IDR API only permits IDs to be INT_MAX). It's harder to use, so we
wouldn't want to convert all users over to it. But it can be made
easier to use than it currently is, which is what I've done here. The
principal way that I've made it easier to use is by introducing
idr_alloc_u32(), which is what all but one of the existing users
actually want.
The last patch at the end I thought of just now -- what happens when
somebody adds an IDR entry with an ID > INT_MAX and then tries to
iterate over all entries in the IDR using an old interface that can't
return these large IDs? It's not safe to return those IDs, so I've
settled for a dmesg warning and terminating the iteration.
Matthew Wilcox (17):
idr: Fix build
radix tree test suite: Remove ARRAY_SIZE
idr test suite: Fix ida_test_random()
IDR test suite: Check handling negative end correctly
idr: Delete idr_remove_ext function
idr: Delete idr_replace_ext function
idr: Delete idr_find_ext function
idr: Add idr_alloc_u32 helper
net sched actions: Convert to use idr_alloc_u32
cls_basic: Convert to use idr_alloc_u32
cls_bpf: Convert to use idr_alloc_u32
cls_flower: Convert to idr_alloc_u32
cls_u32: Reinstate cyclic allocation
cls_u32: Convert to idr_alloc_u32
idr: Rename idr_alloc_ext to idr_alloc_ul
idr: Rename idr_for_each_entry_ext
idr: Warn if old iterators see large IDs
include/linux/idr.h | 109 ++++++++++--------------
include/linux/radix-tree.h | 17 +---
lib/idr.c | 143 +++++++++++++++++++++++---------
lib/radix-tree.c | 3 +-
net/sched/act_api.c | 72 +++++++---------
net/sched/cls_basic.c | 33 ++++----
net/sched/cls_bpf.c | 30 +++----
net/sched/cls_flower.c | 34 ++++----
net/sched/cls_u32.c | 51 +++++-------
tools/testing/radix-tree/idr-test.c | 22 ++++-
tools/testing/radix-tree/linux/kernel.h | 2 -
11 files changed, 266 insertions(+), 250 deletions(-)
--
2.15.0
^ permalink raw reply
* [PATCH v2 13/17] cls_u32: Reinstate cyclic allocation
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Commit e7614370d6f0 ("net_sched: use idr to allocate u32 filter handles)
converted htid allocation to use the IDR. The ID allocated by this
scheme changes; it used to be cyclic, but now always allocates the
lowest available. The IDR supports cyclic allocation, so just use
the right function.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/cls_u32.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 9d48674a70e0..e65b47483dc0 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -316,19 +316,13 @@ static void *u32_get(struct tcf_proto *tp, u32 handle)
return u32_lookup_key(ht, handle);
}
+/* Protected by rtnl lock */
static u32 gen_new_htid(struct tc_u_common *tp_c, struct tc_u_hnode *ptr)
{
- unsigned long idr_index;
- int err;
-
- /* This is only used inside rtnl lock it is safe to increment
- * without read _copy_ update semantics
- */
- err = idr_alloc_ext(&tp_c->handle_idr, ptr, &idr_index,
- 1, 0x7FF, GFP_KERNEL);
- if (err)
+ int id = idr_alloc_cyclic(&tp_c->handle_idr, ptr, 1, 0x7FF, GFP_KERNEL);
+ if (id < 0)
return 0;
- return (u32)(idr_index | 0x800) << 20;
+ return (id | 0x800U) << 20;
}
static struct hlist_head *tc_u_common_hash;
--
2.15.0
^ permalink raw reply related
* [PATCH v2 12/17] cls_flower: Convert to idr_alloc_u32
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Use the new helper which saves a temporary variable and a few lines
of code.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/cls_flower.c | 26 ++++++++++----------------
1 file changed, 10 insertions(+), 16 deletions(-)
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index ec0dc92f6104..adee3cf30bb3 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -858,7 +858,6 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
struct cls_fl_filter *fnew;
struct nlattr **tb;
struct fl_flow_mask mask = {};
- unsigned long idr_index;
int err;
if (!tca[TCA_OPTIONS])
@@ -889,21 +888,17 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
goto errout;
if (!handle) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- 1, 0x80000000, GFP_KERNEL);
- if (err)
- goto errout;
- fnew->handle = idr_index;
- }
-
- /* user specifies a handle and it doesn't exist */
- if (handle && !fold) {
- err = idr_alloc_ext(&head->handle_idr, fnew, &idr_index,
- handle, handle + 1, GFP_KERNEL);
- if (err)
- goto errout;
- fnew->handle = idr_index;
+ handle = 1;
+ err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
+ INT_MAX, GFP_KERNEL);
+ } else if (!fold) {
+ /* user specifies a handle and it doesn't exist */
+ err = idr_alloc_u32(&head->handle_idr, fnew, &handle,
+ handle, GFP_KERNEL);
}
+ if (err)
+ goto errout;
+ fnew->handle = handle;
if (tb[TCA_FLOWER_FLAGS]) {
fnew->flags = nla_get_u32(tb[TCA_FLOWER_FLAGS]);
@@ -957,7 +952,6 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
*arg = fnew;
if (fold) {
- fnew->handle = handle;
idr_replace(&head->handle_idr, fnew, fnew->handle);
list_replace_rcu(&fold->list, &fnew->list);
tcf_unbind_filter(tp, &fold->res);
--
2.15.0
^ permalink raw reply related
* [PATCH v2 06/17] idr: Delete idr_replace_ext function
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Changing idr_replace's 'id' argument to 'unsigned long' works for all
callers. Callers which passed a negative ID now get -ENOENT instead of
-EINVAL. No callers relied on this error value.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 3 +--
lib/idr.c | 15 +++------------
net/sched/act_api.c | 2 +-
net/sched/cls_basic.c | 2 +-
net/sched/cls_bpf.c | 2 +-
net/sched/cls_flower.c | 2 +-
net/sched/cls_u32.c | 2 +-
7 files changed, 9 insertions(+), 19 deletions(-)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 9a4042489ec6..90dbe7a3735c 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -136,8 +136,7 @@ int idr_for_each(const struct idr *,
int (*fn)(int id, void *p, void *data), void *data);
void *idr_get_next(struct idr *, int *nextid);
void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
-void *idr_replace(struct idr *, void *, int id);
-void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id);
+void *idr_replace(struct idr *, void *, unsigned long id);
void idr_destroy(struct idr *);
static inline void *idr_remove(struct idr *idr, unsigned long id)
diff --git a/lib/idr.c b/lib/idr.c
index 2593ce513a18..577bfd4fe5c2 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -147,18 +147,9 @@ EXPORT_SYMBOL(idr_get_next_ext);
* the one being replaced!).
*
* Returns: the old value on success. %-ENOENT indicates that @id was not
- * found. %-EINVAL indicates that @id or @ptr were not valid.
+ * found. %-EINVAL indicates that @ptr was not valid.
*/
-void *idr_replace(struct idr *idr, void *ptr, int id)
-{
- if (id < 0)
- return ERR_PTR(-EINVAL);
-
- return idr_replace_ext(idr, ptr, id);
-}
-EXPORT_SYMBOL(idr_replace);
-
-void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id)
+void *idr_replace(struct idr *idr, void *ptr, unsigned long id)
{
struct radix_tree_node *node;
void __rcu **slot = NULL;
@@ -175,7 +166,7 @@ void *idr_replace_ext(struct idr *idr, void *ptr, unsigned long id)
return entry;
}
-EXPORT_SYMBOL(idr_replace_ext);
+EXPORT_SYMBOL(idr_replace);
/**
* DOC: IDA description
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index bab81574a420..7e901e855d68 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -348,7 +348,7 @@ void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a)
struct tcf_idrinfo *idrinfo = tn->idrinfo;
spin_lock_bh(&idrinfo->lock);
- idr_replace_ext(&idrinfo->action_idr, a, a->tcfa_index);
+ idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
spin_unlock_bh(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_insert);
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index d2193304bad0..147700afcf31 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -231,7 +231,7 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
*arg = fnew;
if (fold) {
- idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
+ idr_replace(&head->handle_idr, fnew, fnew->handle);
list_replace_rcu(&fold->link, &fnew->link);
tcf_unbind_filter(tp, &fold->res);
tcf_exts_get_net(&fold->exts);
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index b017d99fd7e1..1660fc8294ef 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -526,7 +526,7 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
prog->gen_flags |= TCA_CLS_FLAGS_NOT_IN_HW;
if (oldprog) {
- idr_replace_ext(&head->handle_idr, prog, handle);
+ idr_replace(&head->handle_idr, prog, handle);
list_replace_rcu(&oldprog->link, &prog->link);
tcf_unbind_filter(tp, &oldprog->res);
tcf_exts_get_net(&oldprog->exts);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 3e89b0be1706..ca71823bee03 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -958,7 +958,7 @@ static int fl_change(struct net *net, struct sk_buff *in_skb,
if (fold) {
fnew->handle = handle;
- idr_replace_ext(&head->handle_idr, fnew, fnew->handle);
+ idr_replace(&head->handle_idr, fnew, fnew->handle);
list_replace_rcu(&fold->list, &fnew->list);
tcf_unbind_filter(tp, &fold->res);
tcf_exts_get_net(&fold->exts);
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index 6fe4e3549ad3..9d48674a70e0 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -833,7 +833,7 @@ static void u32_replace_knode(struct tcf_proto *tp, struct tc_u_common *tp_c,
if (pins->handle == n->handle)
break;
- idr_replace_ext(&ht->handle_idr, n, n->handle);
+ idr_replace(&ht->handle_idr, n, n->handle);
RCU_INIT_POINTER(n->next, pins->next);
rcu_assign_pointer(*ins, n);
}
--
2.15.0
^ permalink raw reply related
* [PATCH v2 08/17] idr: Add idr_alloc_u32 helper
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
All current users of idr_alloc_ext() actually want to allocate a u32 and
it's a little painful for them to use idr_alloc_ext(). This convenience
function makes it simple.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 12514ec0cd28..9b2fd6f408b2 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -139,6 +139,35 @@ void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
void *idr_replace(struct idr *, void *, unsigned long id);
void idr_destroy(struct idr *);
+/**
+ * idr_alloc_u32() - Allocate an ID.
+ * @idr: IDR handle.
+ * @ptr: Pointer to be associated with the new ID.
+ * @nextid: The new ID.
+ * @max: The maximum ID to allocate (inclusive).
+ * @gfp: Memory allocation flags.
+ *
+ * Allocates an unused ID in the range [*nextid, max] and updates the @nextid
+ * pointer with the newly assigned ID. Returns -ENOSPC and does not modify
+ * @nextid if there are no unused IDs in the range.
+ *
+ * The caller should provide their own locking to ensure that two concurrent
+ * modifications to the IDR are not possible. Read-only accesses to the
+ * IDR may be done under the RCU read lock or may exclude simultaneous
+ * writers.
+ *
+ * Return: 0 on success, -ENOMEM for memory allocation errors, -ENOSPC if
+ * there are no free IDs in the range.
+ */
+static inline int __must_check idr_alloc_u32(struct idr *idr, void *ptr,
+ u32 *nextid, unsigned long max, gfp_t gfp)
+{
+ unsigned long tmp = *nextid;
+ int ret = idr_alloc_ext(idr, ptr, &tmp, tmp, max + 1, gfp);
+ *nextid = tmp;
+ return ret;
+}
+
static inline void *idr_remove(struct idr *idr, unsigned long id)
{
return radix_tree_delete_item(&idr->idr_rt, id, NULL);
--
2.15.0
^ permalink raw reply related
* [PATCH v2 07/17] idr: Delete idr_find_ext function
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Simply changing idr_remove's 'id' argument to 'unsigned long' works
for all callers.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 7 +------
net/sched/act_api.c | 2 +-
net/sched/cls_flower.c | 2 +-
3 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 90dbe7a3735c..12514ec0cd28 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -179,16 +179,11 @@ static inline void idr_preload_end(void)
* This function can be called under rcu_read_lock(), given that the leaf
* pointers lifetimes are correctly managed.
*/
-static inline void *idr_find_ext(const struct idr *idr, unsigned long id)
+static inline void *idr_find(const struct idr *idr, unsigned long id)
{
return radix_tree_lookup(&idr->idr_rt, id);
}
-static inline void *idr_find(const struct idr *idr, int id)
-{
- return idr_find_ext(idr, id);
-}
-
/**
* idr_for_each_entry - iterate over an idr's elements of a given type
* @idr: idr handle
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 7e901e855d68..efb90b8a3bf0 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -222,7 +222,7 @@ static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo)
struct tc_action *p = NULL;
spin_lock_bh(&idrinfo->lock);
- p = idr_find_ext(&idrinfo->action_idr, index);
+ p = idr_find(&idrinfo->action_idr, index);
spin_unlock_bh(&idrinfo->lock);
return p;
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index ca71823bee03..ec0dc92f6104 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -329,7 +329,7 @@ static void *fl_get(struct tcf_proto *tp, u32 handle)
{
struct cls_fl_head *head = rtnl_dereference(tp->root);
- return idr_find_ext(&head->handle_idr, handle);
+ return idr_find(&head->handle_idr, handle);
}
static const struct nla_policy fl_policy[TCA_FLOWER_MAX + 1] = {
--
2.15.0
^ permalink raw reply related
* [PATCH v2 15/17] idr: Rename idr_alloc_ext to idr_alloc_ul
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
idr_alloc_ul fits better with other parts of the Linux kernel where we
need to name a function based on the types it operates on.
It uses a 'nextid' pointer argument instead of separate minimum ID and
output assigned ID, (like idr_get_next), reducing the number of arguments
by one. It also takes a 'max' argument rather than an 'end' argument
(unlike idr_alloc, but the semantics of 'end' don't work for unsigned long
arguments). And its return value is an errno, so mark it as __must_check.
Includes kernel-doc for idr_alloc_ul, which idr_alloc_ext didn't have,
and I realised we were missing a test-case where idr_alloc_cyclic wraps
around INT_MAX. Chris Mi <chrism@mellanox.com> has promised to contribute
test-cases for idr_alloc_ul.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 55 ++-------------------
include/linux/radix-tree.h | 17 +------
lib/idr.c | 99 +++++++++++++++++++++++++++++--------
lib/radix-tree.c | 3 +-
net/sched/cls_u32.c | 20 ++++----
tools/testing/radix-tree/idr-test.c | 17 +++++++
6 files changed, 111 insertions(+), 100 deletions(-)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 9b2fd6f408b2..344380fd0887 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -13,7 +13,6 @@
#define __IDR_H__
#include <linux/radix-tree.h>
-#include <linux/bug.h>
#include <linux/gfp.h>
#include <linux/percpu.h>
@@ -82,55 +81,9 @@ static inline void idr_set_cursor(struct idr *idr, unsigned int val)
void idr_preload(gfp_t gfp_mask);
-int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
- unsigned long start, unsigned long end, gfp_t gfp,
- bool ext);
-
-/**
- * idr_alloc - allocate an id
- * @idr: idr handle
- * @ptr: pointer to be associated with the new id
- * @start: the minimum id (inclusive)
- * @end: the maximum id (exclusive)
- * @gfp: memory allocation flags
- *
- * Allocates an unused ID in the range [start, end). Returns -ENOSPC
- * if there are no unused IDs in that range.
- *
- * Note that @end is treated as max when <= 0. This is to always allow
- * using @start + N as @end as long as N is inside integer range.
- *
- * Simultaneous modifications to the @idr are not allowed and should be
- * prevented by the user, usually with a lock. idr_alloc() may be called
- * concurrently with read-only accesses to the @idr, such as idr_find() and
- * idr_for_each_entry().
- */
-static inline int idr_alloc(struct idr *idr, void *ptr,
- int start, int end, gfp_t gfp)
-{
- unsigned long id;
- int ret;
-
- if (WARN_ON_ONCE(start < 0))
- return -EINVAL;
-
- ret = idr_alloc_cmn(idr, ptr, &id, start, end, gfp, false);
-
- if (ret)
- return ret;
-
- return id;
-}
-
-static inline int idr_alloc_ext(struct idr *idr, void *ptr,
- unsigned long *index,
- unsigned long start,
- unsigned long end,
- gfp_t gfp)
-{
- return idr_alloc_cmn(idr, ptr, index, start, end, gfp, true);
-}
-
+int idr_alloc(struct idr *, void *, int start, int end, gfp_t);
+int __must_check idr_alloc_ul(struct idr *, void *, unsigned long *nextid,
+ unsigned long max, gfp_t);
int idr_alloc_cyclic(struct idr *, void *entry, int start, int end, gfp_t);
int idr_for_each(const struct idr *,
int (*fn)(int id, void *p, void *data), void *data);
@@ -163,7 +116,7 @@ static inline int __must_check idr_alloc_u32(struct idr *idr, void *ptr,
u32 *nextid, unsigned long max, gfp_t gfp)
{
unsigned long tmp = *nextid;
- int ret = idr_alloc_ext(idr, ptr, &tmp, tmp, max + 1, gfp);
+ int ret = idr_alloc_ul(idr, ptr, &tmp, max, gfp);
*nextid = tmp;
return ret;
}
diff --git a/include/linux/radix-tree.h b/include/linux/radix-tree.h
index 23a9c89c7ad9..fc55ff31eca7 100644
--- a/include/linux/radix-tree.h
+++ b/include/linux/radix-tree.h
@@ -356,24 +356,9 @@ int radix_tree_split(struct radix_tree_root *, unsigned long index,
int radix_tree_join(struct radix_tree_root *, unsigned long index,
unsigned new_order, void *);
-void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
+void __rcu **idr_get_free(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max);
-static inline void __rcu **idr_get_free(struct radix_tree_root *root,
- struct radix_tree_iter *iter,
- gfp_t gfp,
- int end)
-{
- return idr_get_free_cmn(root, iter, gfp, end > 0 ? end - 1 : INT_MAX);
-}
-
-static inline void __rcu **idr_get_free_ext(struct radix_tree_root *root,
- struct radix_tree_iter *iter,
- gfp_t gfp,
- unsigned long end)
-{
- return idr_get_free_cmn(root, iter, gfp, end - 1);
-}
enum {
RADIX_TREE_ITER_TAG_MASK = 0x0f, /* tag index in lower nybble */
diff --git a/lib/idr.c b/lib/idr.c
index 577bfd4fe5c2..103afb97b4bd 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -1,4 +1,5 @@
#include <linux/bitmap.h>
+#include <linux/bug.h>
#include <linux/export.h>
#include <linux/idr.h>
#include <linux/slab.h>
@@ -7,32 +8,85 @@
DEFINE_PER_CPU(struct ida_bitmap *, ida_bitmap);
static DEFINE_SPINLOCK(simple_ida_lock);
-int idr_alloc_cmn(struct idr *idr, void *ptr, unsigned long *index,
- unsigned long start, unsigned long end, gfp_t gfp,
- bool ext)
+/**
+ * idr_alloc_ul() - Allocate a large ID.
+ * @idr: IDR handle.
+ * @ptr: Pointer to be associated with the new ID.
+ * @nextid: Pointer to minimum and new ID.
+ * @max: The maximum ID to allocate (inclusive).
+ * @gfp: Memory allocation flags.
+ *
+ * Allocates an unused ID in the range [*nextid, max] and updates the @nextid
+ * pointer with the newly assigned ID. Note that @max differs by 1 from the
+ * @end parameter to idr_alloc().
+ *
+ * The caller should provide their own locking to ensure that two concurrent
+ * modifications to the IDR are not possible. Read-only accesses to the
+ * IDR may be done under the RCU read lock or may exclude simultaneous
+ * writers.
+ *
+ * Return: 0 on success, -ENOMEM for memory allocation errors, -ENOSPC if
+ * there are no free IDs in the range.
+ */
+int idr_alloc_ul(struct idr *idr, void *ptr, unsigned long *nextid,
+ unsigned long max, gfp_t gfp)
{
struct radix_tree_iter iter;
void __rcu **slot;
if (WARN_ON_ONCE(radix_tree_is_internal_node(ptr)))
return -EINVAL;
+ if (WARN_ON_ONCE(!(idr->idr_rt.gfp_mask & ROOT_IS_IDR)))
+ idr->idr_rt.gfp_mask |= IDR_RT_MARKER;
- radix_tree_iter_init(&iter, start);
- if (ext)
- slot = idr_get_free_ext(&idr->idr_rt, &iter, gfp, end);
- else
- slot = idr_get_free(&idr->idr_rt, &iter, gfp, end);
+ radix_tree_iter_init(&iter, *nextid);
+ slot = idr_get_free(&idr->idr_rt, &iter, gfp, max);
if (IS_ERR(slot))
return PTR_ERR(slot);
radix_tree_iter_replace(&idr->idr_rt, &iter, slot, ptr);
radix_tree_iter_tag_clear(&idr->idr_rt, &iter, IDR_FREE);
- if (index)
- *index = iter.index;
+ *nextid = iter.index;
return 0;
}
-EXPORT_SYMBOL_GPL(idr_alloc_cmn);
+EXPORT_SYMBOL_GPL(idr_alloc_ul);
+
+/**
+ * idr_alloc - allocate an id
+ * @idr: idr handle
+ * @ptr: pointer to be associated with the new id
+ * @start: the minimum id (inclusive)
+ * @end: the maximum id (exclusive)
+ * @gfp: memory allocation flags
+ *
+ * Allocates an unused ID in the range [start, end). Returns -ENOSPC
+ * if there are no unused IDs in that range.
+ *
+ * Note that @end is treated as max when <= 0. This is to always allow
+ * using @start + N as @end as long as N is inside integer range.
+ *
+ * Simultaneous modifications to the @idr are not allowed and should be
+ * prevented by the user, usually with a lock. idr_alloc() may be called
+ * concurrently with read-only accesses to the @idr, such as idr_find() and
+ * idr_for_each_entry().
+ */
+int idr_alloc(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
+{
+ unsigned long id = start;
+ int ret;
+
+ if (WARN_ON_ONCE(start < 0))
+ return -EINVAL;
+
+ ret = idr_alloc_ul(idr, ptr, &id, end > 0 ? end - 1 : INT_MAX, gfp);
+
+ if (ret)
+ return ret;
+
+ return id;
+}
+EXPORT_SYMBOL_GPL(idr_alloc);
/**
* idr_alloc_cyclic - allocate new idr entry in a cyclical fashion
@@ -48,18 +102,21 @@ EXPORT_SYMBOL_GPL(idr_alloc_cmn);
*/
int idr_alloc_cyclic(struct idr *idr, void *ptr, int start, int end, gfp_t gfp)
{
- int id, curr = idr->idr_next;
-
- if (curr < start)
- curr = start;
+ unsigned long id = idr->idr_next;
+ int err, max = end > 0 ? end - 1 : INT_MAX;
- id = idr_alloc(idr, ptr, curr, end, gfp);
- if ((id == -ENOSPC) && (curr > start))
- id = idr_alloc(idr, ptr, start, curr, gfp);
+ if ((int)id < start)
+ id = start;
- if (id >= 0)
- idr->idr_next = id + 1U;
+ err = idr_alloc_ul(idr, ptr, &id, max, gfp);
+ if ((err == -ENOSPC) && (id > start)) {
+ id = start;
+ err = idr_alloc_ul(idr, ptr, &id, max, gfp);
+ }
+ if (err)
+ return err;
+ idr->idr_next = id + 1;
return id;
}
EXPORT_SYMBOL(idr_alloc_cyclic);
@@ -226,7 +283,7 @@ EXPORT_SYMBOL(idr_replace);
* bitmap, which is excessive.
*/
-#define IDA_MAX (0x80000000U / IDA_BITMAP_BITS)
+#define IDA_MAX (0x80000000U / IDA_BITMAP_BITS - 1)
/**
* ida_get_new_above - allocate new ID above or equal to a start id
diff --git a/lib/radix-tree.c b/lib/radix-tree.c
index c8d55565fafa..0a7ae3288a24 100644
--- a/lib/radix-tree.c
+++ b/lib/radix-tree.c
@@ -24,6 +24,7 @@
#include <linux/bitmap.h>
#include <linux/bitops.h>
+#include <linux/bug.h>
#include <linux/cpu.h>
#include <linux/errno.h>
#include <linux/export.h>
@@ -2135,7 +2136,7 @@ int ida_pre_get(struct ida *ida, gfp_t gfp)
}
EXPORT_SYMBOL(ida_pre_get);
-void __rcu **idr_get_free_cmn(struct radix_tree_root *root,
+void __rcu **idr_get_free(struct radix_tree_root *root,
struct radix_tree_iter *iter, gfp_t gfp,
unsigned long max)
{
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index e433d1adccc8..3b4523059862 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -730,19 +730,17 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last)
static u32 gen_new_kid(struct tc_u_hnode *ht, u32 htid)
{
- unsigned long idr_index;
- u32 start = htid | 0x800;
- u32 max = htid | 0xFFF;
- u32 min = htid;
-
- if (idr_alloc_ext(&ht->handle_idr, NULL, &idr_index,
- start, max + 1, GFP_KERNEL)) {
- if (idr_alloc_ext(&ht->handle_idr, NULL, &idr_index,
- min + 1, max + 1, GFP_KERNEL))
- return max;
+ unsigned long index = htid | 0x800;
+ unsigned long max = htid | 0xFFF;
+
+ if (idr_alloc_ul(&ht->handle_idr, NULL, &index, max, GFP_KERNEL)) {
+ index = htid + 1;
+ if (idr_alloc_ul(&ht->handle_idr, NULL, &index, max,
+ GFP_KERNEL))
+ index = max;
}
- return (u32)idr_index;
+ return index;
}
static const struct nla_policy u32_policy[TCA_U32_MAX + 1] = {
diff --git a/tools/testing/radix-tree/idr-test.c b/tools/testing/radix-tree/idr-test.c
index 892ef8855b02..36437ade429c 100644
--- a/tools/testing/radix-tree/idr-test.c
+++ b/tools/testing/radix-tree/idr-test.c
@@ -215,6 +215,23 @@ void idr_checks(void)
assert(idr_is_empty(&idr));
+ idr_set_cursor(&idr, INT_MAX - 3UL);
+ for (i = INT_MAX - 3UL; i < INT_MAX + 3UL; i++) {
+ struct item *item;
+ unsigned int id;
+ if (i <= INT_MAX)
+ item = item_create(i, 0);
+ else
+ item = item_create(i - INT_MAX - 1, 0);
+
+ id = idr_alloc_cyclic(&idr, item, 0, 0, GFP_KERNEL);
+ assert(id == item->index);
+ }
+
+ idr_for_each(&idr, item_idr_free, &idr);
+ idr_destroy(&idr);
+ assert(idr_is_empty(&idr));
+
for (i = 1; i < 10000; i++) {
struct item *item = item_create(i, 0);
assert(idr_alloc(&idr, item, 1, 20000, GFP_KERNEL) == i);
--
2.15.0
^ permalink raw reply related
* [PATCH v2 16/17] idr: Rename idr_for_each_entry_ext
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
Match idr_alloc_ul with idr_get_next_ul and idr_for_each_entry_ul.
Also add kernel-doc.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
include/linux/idr.h | 17 ++++++++++++++---
lib/idr.c | 20 +++++++++++++++-----
net/sched/act_api.c | 6 +++---
3 files changed, 32 insertions(+), 11 deletions(-)
diff --git a/include/linux/idr.h b/include/linux/idr.h
index 344380fd0887..91d27a9bcdf4 100644
--- a/include/linux/idr.h
+++ b/include/linux/idr.h
@@ -88,7 +88,7 @@ int idr_alloc_cyclic(struct idr *, void *entry, int start, int end, gfp_t);
int idr_for_each(const struct idr *,
int (*fn)(int id, void *p, void *data), void *data);
void *idr_get_next(struct idr *, int *nextid);
-void *idr_get_next_ext(struct idr *idr, unsigned long *nextid);
+void *idr_get_next_ul(struct idr *, unsigned long *nextid);
void *idr_replace(struct idr *, void *, unsigned long id);
void idr_destroy(struct idr *);
@@ -178,8 +178,19 @@ static inline void *idr_find(const struct idr *idr, unsigned long id)
*/
#define idr_for_each_entry(idr, entry, id) \
for (id = 0; ((entry) = idr_get_next(idr, &(id))) != NULL; ++id)
-#define idr_for_each_entry_ext(idr, entry, id) \
- for (id = 0; ((entry) = idr_get_next_ext(idr, &(id))) != NULL; ++id)
+
+/**
+ * idr_for_each_entry_ul() - iterate over an IDR's elements of a given type.
+ * @idr: IDR handle.
+ * @entry: The type * to use as cursor.
+ * @id: Entry ID.
+ *
+ * @entry and @id do not need to be initialized before the loop, and
+ * after normal terminatinon @entry is left with the value NULL. This
+ * is convenient for a "not found" value.
+ */
+#define idr_for_each_entry_ul(idr, entry, id) \
+ for (id = 0; ((entry) = idr_get_next_ul(idr, &(id))) != NULL; ++id)
/**
* idr_for_each_entry_continue - continue iteration over an idr's elements of a given type
diff --git a/lib/idr.c b/lib/idr.c
index 103afb97b4bd..772a24513d1e 100644
--- a/lib/idr.c
+++ b/lib/idr.c
@@ -155,9 +155,9 @@ int idr_for_each(const struct idr *idr,
EXPORT_SYMBOL(idr_for_each);
/**
- * idr_get_next - Find next populated entry
- * @idr: idr handle
- * @nextid: Pointer to lowest possible ID to return
+ * idr_get_next() - Find next populated entry.
+ * @idr: IDR handle.
+ * @nextid: Pointer to lowest possible ID to return.
*
* Returns the next populated entry in the tree with an ID greater than
* or equal to the value pointed to by @nextid. On exit, @nextid is updated
@@ -178,7 +178,17 @@ void *idr_get_next(struct idr *idr, int *nextid)
}
EXPORT_SYMBOL(idr_get_next);
-void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
+/**
+ * idr_get_next_ul() - Find next populated entry.
+ * @idr: IDR handle.
+ * @nextid: Pointer to lowest possible ID to return.
+ *
+ * Returns the next populated entry in the tree with an ID greater than
+ * or equal to the value pointed to by @nextid. On exit, @nextid is updated
+ * to the ID of the found value. To use in a loop, the value pointed to by
+ * nextid must be incremented by the user.
+ */
+void *idr_get_next_ul(struct idr *idr, unsigned long *nextid)
{
struct radix_tree_iter iter;
void __rcu **slot;
@@ -190,7 +200,7 @@ void *idr_get_next_ext(struct idr *idr, unsigned long *nextid)
*nextid = iter.index;
return rcu_dereference_raw(*slot);
}
-EXPORT_SYMBOL(idr_get_next_ext);
+EXPORT_SYMBOL(idr_get_next_ul);
/**
* idr_replace - replace pointer for given id
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 156302c110af..4133d91b7029 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -124,7 +124,7 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
s_i = cb->args[0];
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
index++;
if (index < s_i)
continue;
@@ -181,7 +181,7 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
if (nla_put_string(skb, TCA_KIND, ops->kind))
goto nla_put_failure;
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
ret = __tcf_idr_release(p, false, true);
if (ret == ACT_P_DELETED) {
module_put(ops->owner);
@@ -351,7 +351,7 @@ void tcf_idrinfo_destroy(const struct tc_action_ops *ops,
int ret;
unsigned long id = 1;
- idr_for_each_entry_ext(idr, p, id) {
+ idr_for_each_entry_ul(idr, p, id) {
ret = __tcf_idr_release(p, false, true);
if (ret == ACT_P_DELETED)
module_put(ops->owner);
--
2.15.0
^ permalink raw reply related
* [PATCH v2 14/17] cls_u32: Convert to idr_alloc_u32
From: Matthew Wilcox @ 2017-11-29 20:19 UTC (permalink / raw)
To: willy, netdev; +Cc: Matthew Wilcox
In-Reply-To: <20171129201922.24370-1-willy@infradead.org>
From: Matthew Wilcox <mawilcox@microsoft.com>
No real benefit to this classifier, but since we're allocating a u32
anyway, we should use this function.
Signed-off-by: Matthew Wilcox <mawilcox@microsoft.com>
---
net/sched/cls_u32.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c
index e65b47483dc0..e433d1adccc8 100644
--- a/net/sched/cls_u32.c
+++ b/net/sched/cls_u32.c
@@ -970,8 +970,8 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
return -ENOMEM;
}
} else {
- err = idr_alloc_ext(&tp_c->handle_idr, ht, NULL,
- handle, handle + 1, GFP_KERNEL);
+ err = idr_alloc_u32(&tp_c->handle_idr, ht, &handle,
+ handle, GFP_KERNEL);
if (err) {
kfree(ht);
return err;
@@ -1020,8 +1020,7 @@ static int u32_change(struct net *net, struct sk_buff *in_skb,
if (TC_U32_HTID(handle) && TC_U32_HTID(handle^htid))
return -EINVAL;
handle = htid | TC_U32_NODE(handle);
- err = idr_alloc_ext(&ht->handle_idr, NULL, NULL,
- handle, handle + 1,
+ err = idr_alloc_u32(&ht->handle_idr, NULL, &handle, handle,
GFP_KERNEL);
if (err)
return err;
--
2.15.0
^ 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