* Re: [v3] libertas: Avoid reading past end of buffer
From: Kalle Valo @ 2017-05-24 13:44 UTC (permalink / raw)
To: Kees Cook
Cc: netdev, Joe Perches, libertas-dev, linux-wireless, netdev,
Daniel Micay, linux-kernel
In-Reply-To: <20170515212640.GA45443@beast>
Kees Cook <keescook@chromium.org> wrote:
> Using memcpy() from a string that is shorter than the length copied means
> the destination buffer is being filled with arbitrary data from the kernel
> rodata segment. Instead, redefine the stat strings to be ETH_GSTRING_LEN
> sizes, like other drivers. This lets us use a single memcpy that does not
> leak rodata contents. Additionally adjust indentation to keep checkpatch.pl
> happy.
>
> This was found with the future CONFIG_FORTIFY_SOURCE feature.
>
> Cc: Daniel Micay <danielmicay@gmail.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Patch applied to wireless-drivers-next.git, thanks.
12e3c0433e8a libertas: Avoid reading past end of buffer
--
https://patchwork.kernel.org/patch/9727997/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: libertas: Remove function entry/exit debugging
From: Kalle Valo @ 2017-05-24 13:44 UTC (permalink / raw)
To: Kees Cook; +Cc: netdev, Joe Perches, libertas-dev, linux-wireless, linux-kernel
In-Reply-To: <20170515213317.GA45562@beast>
Kees Cook <keescook@chromium.org> wrote:
> In at least one place, the enter/exit debugging was not being correctly
> matched. Based on mailing list feedback, it was desired to drop all of
> these in favor of using ftrace instead.
>
> Suggested-by: Joe Perches <joe@perches.com>
> Suggested-by: Kalle Valo <kvalo@codeaurora.org>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Patch applied to wireless-drivers-next.git, thanks.
4bc606af9fab libertas: Remove function entry/exit debugging
--
https://patchwork.kernel.org/patch/9728087/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: Alignment in BPF verifier
From: Edward Cree @ 2017-05-24 13:46 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, David Miller
Cc: alexei.starovoitov, netdev
In-Reply-To: <5924A938.2090808@iogearbox.net>
On 23/05/17 22:27, Daniel Borkmann wrote:
> On 05/23/2017 09:45 PM, Alexei Starovoitov wrote:
>> On 5/23/17 7:41 AM, Edward Cree wrote:
>>> Hmm, that means that we can't do arithmetic on a
>>> PTR_TO_MAP_VALUE_OR_NULL, we have to convert it to a PTR_TO_MAP_VALUE
>>> first by NULL-checking it. That's probably fine, but I can just about
>>> imagine some compiler optimisation reordering them. Any reason not to
>>> split this out into a different reg->field, rather than overloading id?
>>
>> 'id' is sort of like 'version' of a pointer and has the same meaning in
>> both cases. How exactly do you see this split?
I was thinking there would be reg->id and reg->map_id. Both could share the
env->id_gen, since that's not likely to run out, but they'd be separate
fields so that a PTR_TO_MAP_VALUE_OR_NULL could say "this is either map_value
plus a 4-byte-aligned offset less than 24, or NULL plus that same offset",
and then if another pointer with the same map_id and no variable-offset part
was NULL-checked, we could convert both pointers to PTR_TO_MAP_VALUE. (I'm
getting rid of PTR_TO_MAP_VALUE_ADJ in my patch, along with several other
types, by taking the 'we have an offset' part out of the bpf_reg_type.)
> So far we haven't run into this kind of optimization
> from llvm side yet[...] Out of curiosity, did you run into it with llvm?
No, purely theoretical. I haven't even built/installed llvm yet, I'm just
working with the bytecode in test_verifier.c for now. I'm merely trying to
not have restrictions that are unnecessary; but since allowing this kind of
construct would take a non-zero amount of work, I'll file it for later.
-Ed
^ permalink raw reply
* Re: [v2, 01/10] rt2x00: change function pointers for register accessors
From: Kalle Valo @ 2017-05-24 13:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-wireless, Arnd Bergmann, Stanislaw Gruszka, David Miller,
Helmut Schaa, Daniel Golle, Mathias Kresin, Johannes Berg,
Serge Vasilugin, Roman Yeryomin, Networking,
Linux Kernel Mailing List, Jes Sorensen, Tom Psyborg
In-Reply-To: <20170517144702.1381136-2-arnd@arndb.de>
Arnd Bergmann <arnd@arndb.de> wrote:
> This prepares the driver for changing all the 'read' register accessors
> to return the value instead of passing it by reference. Since a lot
> of them are used in callbacks, this takes care of the callbacks first,
> adding a couple of helpers that will be removed again one at a time.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
10 patches applied to wireless-drivers-next.git, thanks.
6b81745e36e3 rt2x00: change function pointers for register accessors
16d571bb0fe6 rt2x00: convert rt2800_rfcsr_read return type
aea8baa10ac5 rt2x00: convert rt2x00_rf_read return type
3954b4e30609 rt2x00: convert rt2x00mmio_register_read return type
48bde9c96972 rt2x00: convert rt2x00usb_register_read return type
eebd68e7824b rt2x00: convert rt2800_register_read return type
5fbbe378890f rt2x00: convert rt2*_bbp_read return type
38651683aa98 rt2x00: convert rt2x00_eeprom_read return type
5c6a25855cbd rt2x00: convert rt2800_eeprom_read return type
b9b238726716 rt2x00: convert rt2x00_desc_read return type
--
https://patchwork.kernel.org/patch/9731331/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Timur Tabi @ 2017-05-24 13:48 UTC (permalink / raw)
To: Andrew Lunn
Cc: Matthias May, Zefir Kurtisi, netdev, f.fainelli, David Miller,
Manoj Iyer, jhugo
In-Reply-To: <20170524134021.GA26577@lunn.ch>
On 5/24/17 8:40 AM, Andrew Lunn wrote:
> You need to prove this, that the link is not up. Any by link, we mean
> both the copper and the SGMII link.
I can post the log of my iperf run showing that the, when
at803x_aneg_done() returns zero, no packets can go through. And then
after I change at803x_aneg_done() so that it returns BMSR_ANEGCOMPLETE,
then packets do go through. Is that the proof you're looking for?
The current work-around that we're using internally is to blacklist the
at803x driver. This forces the kernel to use the genphy driver instead.
Everything works when we do this.
>> at803x_aneg_done() is never called again, and so I think
>> the kernel is disabling the interface is some secret way.
>
> Well, the driver has told the core that the link is not up. So the
> kernel is waiting for another interrupt indicating the link has gone
> up. And probably, this second interrupt never happens.
Exxactly. That's because the link IS up, and so there is no opportunity
to receive another interrupt.
> And it is not disabling the interface. Since the PHY is still down,
> the core has not called netif_carrier_on().
Ok, I should have said "not enabled" instead of "disabled". Thanks.
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the
Code Aurora Forum, hosted by The Linux Foundation.
^ permalink raw reply
* RE: [PATCH v2] drivers: phy: Add Cortina CS4340 driver
From: Bogdan Purcareata @ 2017-05-24 13:52 UTC (permalink / raw)
To: Andrew Lunn
Cc: f.fainelli@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170524125830.GB24341@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Wednesday, May 24, 2017 3:59 PM
> To: Bogdan Purcareata <bogdan.purcareata@nxp.com>
> Cc: f.fainelli@gmail.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2] drivers: phy: Add Cortina CS4340 driver
>
> On Wed, May 24, 2017 at 09:25:48AM +0000, Bogdan Purcareata wrote:
> > Add basic support for Cortina PHY drivers. Support only CS4340 for now.
> > The phys are not compatible with IEEE 802.3 clause 45 registers.
> Implement
> > proper read_status support, so that phy polling does not cause bus
> > register access errors.
> >
> > The driver should be described using the "ethernet-phy-id" device tree
> > compatible.
>
> Hi Bogdan
>
> Thanks for testing that ethernet-phy-id works.
>
> I suggest you write a
> Documentation/devicetree/binding/net/phy/cortina.txt giving an example
> device tree node.
Yes, I can do that. However, I don't see a "phy" folder under Documentation/devicetree/bindings/net/.
Should I change the location to Documentation/devicetree/bindings/net/cortina.txt instead?
> >
> > Signed-off-by: Bogdan Purcareata <bogdan.purcareata@nxp.com>
> > ---
> > v1 -> v2:
> > - Rename "mdio-cortina.c" to "cortina.c" since it's a phy driver.
> > - Test probing based on the "ethernet-phy-id" compatible. In the previous
> > version, getting the phy_id via get_phy_c45_ids() involved an
> additional
> > hack. Drop that approach and document probing in the commit message.
> >
> > drivers/net/phy/Kconfig | 5 +++
> > drivers/net/phy/Makefile | 1 +
> > drivers/net/phy/cortina.c | 90
> +++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 96 insertions(+)
> > create mode 100644 drivers/net/phy/cortina.c
> >
> > diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> > index 22dea7f..ad09e2d 100644
> > --- a/drivers/net/phy/Kconfig
> > +++ b/drivers/net/phy/Kconfig
> > @@ -240,6 +240,11 @@ config CICADA_PHY
> > ---help---
> > Currently supports the cis8204
> >
> > +config CORTINA_PHY
> > + tristate "Cortina quad-10G Ethernet PHY"
> > + ---help---
> > + Currently supports the CS4340 phy.
> > +
> > config DAVICOM_PHY
> > tristate "Davicom PHYs"
> > ---help---
> > diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> > index 79365be..0de3e20 100644
> > --- a/drivers/net/phy/Makefile
> > +++ b/drivers/net/phy/Makefile
> > @@ -47,6 +47,7 @@ obj-$(CONFIG_BCM_CYGNUS_PHY) += bcm-cygnus.o
> > obj-$(CONFIG_BCM_NET_PHYLIB) += bcm-phy-lib.o
> > obj-$(CONFIG_BROADCOM_PHY) += broadcom.o
> > obj-$(CONFIG_CICADA_PHY) += cicada.o
> > +obj-$(CONFIG_CORTINA_PHY) += cortina.o
> > obj-$(CONFIG_DAVICOM_PHY) += davicom.o
> > obj-$(CONFIG_DP83640_PHY) += dp83640.o
> > obj-$(CONFIG_DP83848_PHY) += dp83848.o
> > diff --git a/drivers/net/phy/cortina.c b/drivers/net/phy/cortina.c
> > new file mode 100644
> > index 0000000..6f054ed
> > --- /dev/null
> > +++ b/drivers/net/phy/cortina.c
> > @@ -0,0 +1,90 @@
> > +/*
> > + * Based on code from Cortina Systems, Inc.
> > + *
> > + * Copyright 2011 Cortina Systems, Inc.
> > + * Copyright 2017 NXP
> > + *
> > + * This program is free software; you can redistribute it and/or
> modify
> > + * it under the terms of the GNU General Public License as published
> by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> > + * GNU General Public License for more details.
> > + *
> > + */
> > +#include <linux/module.h>
> > +#include <linux/phy.h>
> > +
> > +#define PHY_ID_CS4340 0x13e51002
> > +
> > +#define CORTINA_GPIO_GPIO_INTS 0x16D
> > +
> > +static int cortina_read_x(struct phy_device *phydev, int off, u16
> regnum)
> > +{
> > + return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr + off,
> > + MII_ADDR_C45 | regnum);
> > +}
> > +
> > +static int cortina_read(struct phy_device *phydev, u16 regnum)
> > +{
> > + return cortina_read_x(phydev, 0, regnum);
> > +}
> > +
> > +static int cortina_config_aneg(struct phy_device *phydev)
> > +{
> > + phydev->supported = SUPPORTED_10000baseT_Full;
> > + phydev->advertising = SUPPORTED_10000baseT_Full;
> > +
> > + return 0;
> > +}
> > +
> > +static int cortina_read_status(struct phy_device *phydev)
> > +{
> > + int gpio_int_status;
> > + int ret = 0;
>
> I think there needs to be some explanation here. What exactly are you
> using to indicate link up? What does CORTINA_GPIO_GPIO_INTS mean?
I can only assume it's the location of an interrupt status register. The Cortina PHYs go mostly undocumented, I've found an implementation in an old thread [1], that also did the job of programming the Cortina PHY microcode. I understand that microcode programming is now part of the bootloader.
The register seems to provide the link status properly, and based on that the phydev is initialized with the only (currently) supported configuration, 10Gbps full duplex.
I can change the register name to something more meaningful, though - e.g. CORTINA_LINK_STATUS.
> > + gpio_int_status = cortina_read(phydev, CORTINA_GPIO_GPIO_INTS);
> > + if (gpio_int_status < 0) {
> > + ret = gpio_int_status;
> > + goto err;
> > + }
> > +
> > + if (gpio_int_status & 0x8) {
> > + phydev->speed = SPEED_10000;
> > + phydev->duplex = DUPLEX_FULL;
> > + phydev->link = 1;
> > + } else {
> > + phydev->link = 0;
> > + }
> > +
> > +err:
> > + return ret;
> > +}
> > +
> > +static int cortina_soft_reset(struct phy_device *phydev)
> > +{
> > + return 0;
> > +}
> > +
> > +static struct phy_driver cortina_driver[] = {
> > +{
> > + .phy_id = PHY_ID_CS4340,
> > + .phy_id_mask = 0xffffffff,
> > + .name = "Cortina CS4340",
> > + .config_aneg = cortina_config_aneg,
> > + .read_status = cortina_read_status,
> > + .soft_reset = cortina_soft_reset,
> > +},
> > +};
>
> Having two } at the same indentation level seems odd. Please can you
> fix this.
I've purposely left them on the same indentation level in case more Cortina PHY IDs will be added (similar to the situation in drivers/net/phy/aquantia.c). If it does look odd, I can refactor it to use the proper indentation.
[1] https://www.linux-mips.org/archives/linux-mips/2012-05/msg00314.html
Thank you!
Bogdan
^ permalink raw reply
* Re: [PATCH 2/2] at803x: double check SGMII side autoneg
From: Andrew Lunn @ 2017-05-24 14:09 UTC (permalink / raw)
To: Timur Tabi
Cc: Matthias May, Zefir Kurtisi, netdev, f.fainelli, David Miller,
Manoj Iyer, jhugo
In-Reply-To: <8cf4ca6d-3e1e-3c9c-605f-5d6baf3c7588@codeaurora.org>
On Wed, May 24, 2017 at 08:48:04AM -0500, Timur Tabi wrote:
> On 5/24/17 8:40 AM, Andrew Lunn wrote:
>
> >You need to prove this, that the link is not up. Any by link, we mean
> >both the copper and the SGMII link.
>
> I can post the log of my iperf run showing that the, when
> at803x_aneg_done() returns zero, no packets can go through. And
> then after I change at803x_aneg_done() so that it returns
> BMSR_ANEGCOMPLETE, then packets do go through. Is that the proof
> you're looking for?
No. I would like to see the status of the copper side and the status
of the SGMII side, at the point at803x_aneg_done() is called.
If the copper side is up, but the SGMII side is down, returning 0 is
correct.
> Exxactly. That's because the link IS up, and so there is no
> opportunity to receive another interrupt.
It could be, the copper side is up, but the SGMII side is down, at the
point at803x_aneg_done() is called. So it is correctly returning
0. Sometime later the SGMII side goes up, but there is not a second
interrupt. Hence the phy core does not know that the full, 2 stage MAC
to PHY to peer PHY link is now up.
Andrew
^ permalink raw reply
* Re: [PATCH v4 next 1/3] modules:capabilities: allow __request_module() to take a capability argument
From: Djalal Harouni @ 2017-05-24 14:16 UTC (permalink / raw)
To: Kees Cook, Serge E. Hallyn, Rusty Russell, David S . Miller,
Jessica Yu
Cc: LKML, Network Development, linux-security-module,
kernel-hardening@lists.openwall.com, Andy Lutomirski,
Andrew Morton, James Morris, Paul Moore, Stephen Smalley,
Greg Kroah-Hartman, Tetsuo Handa, Ingo Molnar, Linux API,
Dongsu Park, Casey Schaufler, Jonathan Corbet,
Arnaldo Carvalho de Melo, Mauro Carvalho Chehab, Peter Zijlstra,
Zendyani, "linux-doc@vger.kerne
In-Reply-To: <CAGXu5j+L1aEWqi7DC4StwAorfG4da0XqrouMvbz07cJ7gYi0AQ@mail.gmail.com>
On Tue, May 23, 2017 at 9:19 PM, Kees Cook <keescook@google.com> wrote:
> On Tue, May 23, 2017 at 3:29 AM, Djalal Harouni <tixxdz@gmail.com> wrote:
[...]
>> I think if there is an interface request_module_capable() , then code
>> will use it. The DCCP code path did not check capabilities at all and
>> called request_module(), other code does the same.
>>
>> A new interface can be abused, the result of this: we may break
>> "modules_autoload_mode" in mode 0 and 1. In the long term code will
>> want to change may_autoload_module() to also allow mode 1 to load a
>> module with CAP_NET_ADMIN or other caps in its own userns, resulting
>> in "modules_autoload_mode == 0 == 1". Without userns in the game we
>> may just see request_module_capable(CAP_SYS_ADMIN, ...) . There is
>> already some code maybe phonet sockets ? that require CAP_SYS_ADMIN to
>> get the appropriate protocol.... and no one will be able to review all
>> this code or track new patches with request_module_capable() callers.
>
> I'm having some trouble following what you're saying here, but if I
> understand, you're worried about getting the kernel into a state where
> autoload state 0 == 1. Autoload 0 is "business as usual", and autoload
> 1 is "CAP_SYS_MODULE required to be able to trigger a module auto-load
> operation, or CAP_NET_ADMIN for modules with a 'netdev-%s' alias."
Indeed.
> In the v4 patch, under autoload==1, CAP_NET_ADMIN is needed to load
> netdev- modules:
>
> if (no_module && capable(CAP_NET_ADMIN))
> no_module = __request_module(true, CAP_NET_ADMIN,
> "netdev-%s", name);
>
> and in the LSM hook, CAP_NET_ADMIN is passed as an allowable "alias"
> for the CAP_SYS_MODULE requirement:
>
> else if (modules_autoload_mode == MODULES_AUTOLOAD_PRIVILEGED) {
> /* Check CAP_SYS_MODULE then allow_cap if valid */
> if (capable(CAP_SYS_MODULE) ||
> (allow_cap > 0 && capable(allow_cap)))
> return 0;
> }
>
> What I see is some needless double-checking. Since you're making
> changes to the request_module() API, it would be possible to have
That check is *not* a double check and it is *really* needed in v4
since how may_autoload_module() was implemented. It first checks if
'autoload' == 0 == ALLOWED, if so then it allows the operation
regardless of the capability. That's why I didn't want to touch
current network logic and assumed that net code knows what it should
do.
> request_module_cap(), which could be checked instead of open-coding
> it:
>
> if (no_module)
> no_module = request_module_cap(CAP_NET_ADMIN, "netdev-%s", name);
>
> If I'm understanding your objection correctly, it's that you want to
> ONLY ever provide this one-time alias for CAP_SYS_MODULE with the
> netdev-%s things, and you don't want to risk having other module
> loading start using request_module_cap() which would lead to
> CAP_SYS_MODULE aliases in other places?
Yes. We can't really track capabilities usage or new code.
> If the goal is to make sure that only privileged processes are
> autoloading, I don't think adding a well defined interface for
> cap-checks (request_module_cap()) would lead to a slippery slope. The
> worst case scenario (which would never happen) would be all
> request_module() users would convert to request_module_cap(). This
I am also concerned a bit with new code. In the documentation we
explicitly say CAP_SYS_MODULE, and new code should not break that
assumption.
> would mean that all module loading would require specific privileges.
> That seems in line with autoload==1. They would not be tied to
> CAP_SYS_MODULE, though, which is, I suspect, what you're concerned
> about.
Indeed, it is just easy to say hey it needs CAP_SYS_MODULE. The
capability usage in the module subsystem more precisely with explicit
loading is clean. CAP_SYS_MODULE is not overloaded, it has clear
focus. As you say it, we should be concerned if we blindly trust
callers and end up *aliasing* CAP_SYS_MODULE with some other cap...
> Even in the existing code, there is a sense about CAP_NET_ADMIN and
> CAP_SYS_MODULE having different privilege levels, in that
> CAP_NET_ADMIN can only load netdev-%s modules, but CAP_SYS_MODULE can
> load any module. What about refining request_module_cap() to _require_
> an explicit string prefix instead of an arbitrary format string? e.g.
> request_module_cap(CAP_NET_ADMIN, "netdev", "%s", name) which would
> make requests for ("netdev-%s", name)
>
> I see a few options:
>
> 1) keep what you have for v4, and hope other places don't use
> __request_module. (I'm not a fan of this.)
Yes even if it is documented I wouldn't bet on it, though. :-)
> 2) switch the logic on autoload==1 from OR to AND: both the specified
> caps _and_ CAP_SYS_MODULE are required. (This seems like it might make
> autoload==1 less useful.)
That will restrict some userspace that works only with CAP_NET_ADMIN.
> 3) use the request_module_cap() outlined above, which requires that
> modules being loaded under a CAP_SYS_MODULE-aliased capability are at
> least restricted to a subset of kernel module names.
This one tends to allow usability.
> 4) same as 3 but also insert autoload==2 level that switches from OR
> to AND (bumping existing ==2 to ==3).
I wouldn't expose autoload to callers, I think it is better if it
stays a property of the module subsystem. But lets use the bump idea,
please see below.
> What do you think?
Ok so given that we already have modules_autoload_mode=2 disabled,
maybe we go with 3) like this ?
int __request_module(bool wait, int required_cap, const char *prefix,
const char *name, ...);
#define request_module(mod...) \
__request_module(true, -1, NULL, mod)
#define request_module_cap(required_cap, prefix, mod...) \
__request_module(true, required_cap, prefix, mod)
and we require allow_cap and prefix to be set.
request_module_cap(CAP_NET_ADMIN, "netdev-", "%s", name) for
net/core/dev_ioctl.c:dev_load()
request_module_cap(CAP_NET_ADMIN, "tcp_", "%s", name) for
net/ipv4/tcp_cong.c functions.
Then
__request_module()
-> security_kernel_module_request(module_name, required_cap, prefix)
-> may_autoload_module(current, module_name, required_cap, prefix)
And update may_autoload_module() as below ? we hard code CAP_NET_ADMIN
and CAP_SYS_MODULE inside and make them the only capabilities needed
for a privileged auto-load operation.
request_module_cap(CAP_SYS_MODULE, ...) or
request_module_cap(CAP_NET_ADMIN, ...) if the autoload should be a
privileged operation.
Kees will this work ?
Jessica, Rusty, Serge. What do you think ? I definitively think that
module_autoload should be contained only inside the module subsystem..
+int may_autoload_module(struct task_struct *task, char *kmod_name,
+ int require_cap, char *prefix)
+{
+ unsigned int autoload;
+ int module_require_cap = 0;
+
+ if (require_cap > 0) {
+ if (prefix == NULL || *prefix == '\0')
+ return -EPERM;
+
+ /*
+ * We only allow CAP_SYS_MODULE or CAP_NET_ADMIN for
+ * 'netdev-%s' modules for backward compatibility.
+ * Please do not overload capabilities.
+ */
+ if (require_cap == CAP_SYS_MODULE ||
+ require_cap == CAP_NET_ADMIN)
+ module_require_cap = require_cap;
+ else
+ return -EPERM;
+ }
+
+ /* Get max value of sysctl and task "modules_autoload_mode" */
+ autoload = max_t(unsigned int, modules_autoload_mode,
+ task->modules_autoload_mode);
+
+ /*
+ * If autoload is disabled then fail here and not bother at all
+ */
+ if (autoload == MODULES_AUTOLOAD_DISABLED)
+ return -EPERM;
+
+ /*
+ * If caller require capabilities then we may not allow
+ * automatic module loading. We should not bypass callers.
+ * This allows to support networking code that uses CAP_NET_ADMIN
+ * for some aliased 'netdev-%s' modules.
+ *
+ * Explicitly bump autoload here if necessary
+ */
+ if (module_require_cap && autoload == MODULES_AUTOLOAD_ALLOWED)
+ autoload = MODULES_AUTOLOAD_PRIVILEGED;
+
+ if (autoload == MODULES_AUTOLOAD_ALLOWED)
+ return 0;
+ else if(autoload == MODULES_AUTOLOAD_PRIVILEGED) {
+ /*
+ * If module auto-load is a privileged operation then check
+ * if capabilities are set.
+ */
+ if (capable(CAP_SYS_MODULE) ||
+ (module_require_cap && capable(module_require_cap)))
+ return 0;
+ }
+
+ return -EPERM;
+}
+
^ permalink raw reply
* Aw: Re: [PATCH v6 net-next 17/17] net: qualcomm: add QCA7000 UART driver
From: Lino Sanfilippo @ 2017-05-24 14:19 UTC (permalink / raw)
To: Stefan Wahren
Cc: Rob Herring, Mark Rutland, David S. Miller, linux-serial,
Jiri Slaby, Greg Kroah-Hartman, netdev, linux-kernel,
Jakub Kicinski, devicetree
In-Reply-To: <50e5a442-777f-1516-4e94-16db7fa28f8b@i2se.com>
Hi,
> Gesendet: Mittwoch, 24. Mai 2017 um 11:06 Uhr
> Von: "Stefan Wahren" <stefan.wahren@i2se.com>
> An: "Lino Sanfilippo" <LinoSanfilippo@gmx.de>, "Rob Herring" <robh+dt@kernel.org>, "Mark Rutland" <mark.rutland@arm.com>, "David S. Miller" <davem@davemloft.net>
> Cc: linux-serial@vger.kernel.org, "Jiri Slaby" <jslaby@suse.com>, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org>, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, "Jakub Kicinski" <kubakici@wp.pl>, devicetree@vger.kernel.org
> Betreff: Re: [PATCH v6 net-next 17/17] net: qualcomm: add QCA7000 UART driver
>
> Am 23.05.2017 um 23:01 schrieb Lino Sanfilippo:
> > On 23.05.2017 21:38, Stefan Wahren wrote:
> >>> Lino Sanfilippo <LinoSanfilippo@gmx.de> hat am 23. Mai 2017 um 20:16 geschrieben:
> >>>
> >>> I suggest to avoid this possible race by first unregistering the netdevice and then
> >>> calling cancel_work_sync().
> >> What makes you sure that's safe to unregister the netdev while the tx work queue is possibly active?
> > unregister_netdevice() calls netdev_close() if the interface is still up. netdev_close() calls flush_work()
> > so the unregistration is delayed until the tx work function is finished. Furthermore both close() and
> > tx work are synchronized by means of the qca->lock which also guarantees that unregister_netdevice() wont
> > be finished until the tx work is done.
> >
>
> Thanks for the explanation. I suspect there could be the same race
> between serdev_device_close() and the tx work queue.
>
> So i would propose a variant of your original suggestion:
>
> unregister_netdev(qca->net_dev);
>
> /* Flush any pending characters in the driver. */
> serdev_device_close(serdev);
> cancel_work_sync(&qca->tx_work);
>
> Since we have the same pattern in the error path of the probe function,
> the same applies there.
Agreed, it is much cleaner to have the same cleanup pattern in remove() as
we have in (error case of) probe().
Regards,
Lino
^ permalink raw reply
* Re: [PATCH RESEND v7 net-next 17/17] net: qualcomm: add QCA7000 UART driver
From: Lino Sanfilippo @ 2017-05-24 14:21 UTC (permalink / raw)
To: Stefan Wahren
Cc: Rob Herring, Mark Rutland, David S. Miller, Greg Kroah-Hartman,
Jiri Slaby, Jakub Kicinski, devicetree, netdev, linux-serial,
linux-kernel, Stefan Wahren
In-Reply-To: <1495618330-1314-1-git-send-email-stefan.wahren@i2se.com>
Hi,
> This patch adds the Ethernet over UART driver for the
> Qualcomm QCA7000 HomePlug GreenPHY.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> ---
Reviewed-by: Lino Sanfilippo <LinoSanfilippo@gmx.de>
Regards,
Lino
^ permalink raw reply
* Re: [PATCH v2] drivers: phy: Add Cortina CS4340 driver
From: Andrew Lunn @ 2017-05-24 14:25 UTC (permalink / raw)
To: Bogdan Purcareata
Cc: f.fainelli@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <DB5PR04MB1240541B2CFB2C99ED519CDFEAFE0@DB5PR04MB1240.eurprd04.prod.outlook.com>
> Yes, I can do that. However, I don't see a "phy" folder under Documentation/devicetree/bindings/net/.
> Should I change the location to Documentation/devicetree/bindings/net/cortina.txt instead?
Ah, sorry, my error. Yes, Documentation/devicetree/bindings/net/cortina.txt.
> > I think there needs to be some explanation here. What exactly are you
> > using to indicate link up? What does CORTINA_GPIO_GPIO_INTS mean?
>
> I can only assume it's the location of an interrupt status register. The Cortina PHYs go mostly undocumented, I've found an implementation in an old thread [1], that also did the job of programming the Cortina PHY microcode. I understand that microcode programming is now part of the bootloader.
>
> The register seems to provide the link status properly, and based on that the phydev is initialized with the only (currently) supported configuration, 10Gbps full duplex.
>
> I can change the register name to something more meaningful, though - e.g. CORTINA_LINK_STATUS.
No, leave the name as is. The MIPS driver you gave a reference to
seems to be using sensible names. My guess is, the boot loader is
configuring the PHY to generate an interrupt on link up via one of its
GPIO lines. And this code is reading that GPIO status. This is all
very fragile, you are making a lot of assumptions.
Have you tested pulling the cable out? And plugging it back in again?
There is a chance this interrupt is "link change", not "link up".
What i do like in that mips code is the probe function verifies the
product ID.
+ /*
+ * CS4312 keeps its ID values in non-standard registers, make
+ * sure we are talking to what we think we are.
+ */
+ id_lsb = cs4321_phy_read(phydev, CS4321_GLOBAL_CHIP_ID_LSB);
+ if (id_lsb < 0) {
+ ret = id_lsb;
+ goto err;
+ }
+
+ id_msb = cs4321_phy_read(phydev, CS4321_GLOBAL_CHIP_ID_MSB);
+ if (id_msb < 0) {
+ ret = id_msb;
+ goto err;
+ }
+
+ if (id_lsb != 0x23E5 || id_msb != 0x1002) {
+ ret = -ENODEV;
+ goto err;
+ }
You should do that as well.
Andrew
^ permalink raw reply
* RE: [PATCH v2] drivers: phy: Add Cortina CS4340 driver
From: Bogdan Purcareata @ 2017-05-24 14:34 UTC (permalink / raw)
To: Andrew Lunn
Cc: f.fainelli@gmail.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170524142543.GD26577@lunn.ch>
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Wednesday, May 24, 2017 5:26 PM
> To: Bogdan Purcareata <bogdan.purcareata@nxp.com>
> Cc: f.fainelli@gmail.com; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: Re: [PATCH v2] drivers: phy: Add Cortina CS4340 driver
>
> > Yes, I can do that. However, I don't see a "phy" folder under
> Documentation/devicetree/bindings/net/.
> > Should I change the location to
> Documentation/devicetree/bindings/net/cortina.txt instead?
>
> Ah, sorry, my error. Yes,
> Documentation/devicetree/bindings/net/cortina.txt.
>
> > > I think there needs to be some explanation here. What exactly are you
> > > using to indicate link up? What does CORTINA_GPIO_GPIO_INTS mean?
> >
> > I can only assume it's the location of an interrupt status register. The
> Cortina PHYs go mostly undocumented, I've found an implementation in an old
> thread [1], that also did the job of programming the Cortina PHY microcode.
> I understand that microcode programming is now part of the bootloader.
> >
> > The register seems to provide the link status properly, and based on that
> the phydev is initialized with the only (currently) supported
> configuration, 10Gbps full duplex.
> >
> > I can change the register name to something more meaningful, though -
> e.g. CORTINA_LINK_STATUS.
>
> No, leave the name as is. The MIPS driver you gave a reference to
> seems to be using sensible names. My guess is, the boot loader is
> configuring the PHY to generate an interrupt on link up via one of its
> GPIO lines. And this code is reading that GPIO status. This is all
> very fragile, you are making a lot of assumptions.
>
> Have you tested pulling the cable out? And plugging it back in again?
> There is a chance this interrupt is "link change", not "link up".
No, I have not. I had it planned but it somehow slipped. Will test.
> What i do like in that mips code is the probe function verifies the
> product ID.
>
> + /*
> + * CS4312 keeps its ID values in non-standard registers, make
> + * sure we are talking to what we think we are.
> + */
> + id_lsb = cs4321_phy_read(phydev, CS4321_GLOBAL_CHIP_ID_LSB);
> + if (id_lsb < 0) {
> + ret = id_lsb;
> + goto err;
> + }
> +
> + id_msb = cs4321_phy_read(phydev, CS4321_GLOBAL_CHIP_ID_MSB);
> + if (id_msb < 0) {
> + ret = id_msb;
> + goto err;
> + }
> +
> + if (id_lsb != 0x23E5 || id_msb != 0x1002) {
> + ret = -ENODEV;
> + goto err;
> + }
>
> You should do that as well.
Okay, I will include this check in a probe function in v3.
Thanks a lot!
Bogdan
^ permalink raw reply
* Re: [PATCH RESEND v7 net-next 17/17] net: qualcomm: add QCA7000 UART driver
From: Stefan Wahren @ 2017-05-24 14:34 UTC (permalink / raw)
To: Lino Sanfilippo
Cc: Rob Herring, Mark Rutland, David S. Miller, Greg Kroah-Hartman,
Jiri Slaby, Jakub Kicinski, devicetree-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <trinity-190c217d-ab18-4d21-b7a5-fd1d6a3ea051-1495635717353@3capp-gmx-bs75>
Am 24.05.2017 um 16:21 schrieb Lino Sanfilippo:
> Hi,
>
>
>> This patch adds the Ethernet over UART driver for the
>> Qualcomm QCA7000 HomePlug GreenPHY.
>>
>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>> ---
> Reviewed-by: Lino Sanfilippo <LinoSanfilippo-Mmb7MZpHnFY@public.gmane.org>
>
Thanks
Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net-next v3 5/5] net-next: dsa: add dsa support for Mediatek MT7530 switch
From: Andrey Melnikov @ 2017-05-24 14:44 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Sean Wang
In-Reply-To: <20170524132049.GB24980-g2DYL2Zd6BY@public.gmane.org>
2017-05-24 16:20 GMT+03:00 Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org>:
>> What about JUMBO frames and large MTU support? devlink support?
>
> To get the driver merged, it is better to start small, a minimal set
> of features. Other features can be added later. If these features are
> important to you, please feel free to submit patches.
There is no documentation from mediatek about this feature. Driver
from SDK v4.x contains only switching 19'th bit in GDMA1_FWD_CFG and
nothing more.
So, how to write patches? Peeking random bits in random SOC config registers?
--
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: [PATCHv2 net-next 5/5] dsa: mv88e6xxx: Enable/Disable SERDES on port enable/disable
From: Vivien Didelot @ 2017-05-24 14:58 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Andrew Lunn
In-Reply-To: <1495573486-31827-6-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> +static int mv88e6xxx_port_enable(struct dsa_switch *ds, int port,
> + struct phy_device *phydev)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> + int err = 0;
> +
> + mutex_lock(&chip->reg_lock);
> + if (chip->info->ops->serdes_power)
> + err = chip->info->ops->serdes_power(chip, port, true);
> + mutex_unlock(&chip->reg_lock);
> +
> + return err;
> +}
Please use your new mv88e6xxx_serdes_power() helper here.
> +
> +static void mv88e6xxx_port_disable(struct dsa_switch *ds, int port,
> + struct phy_device *phydev)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> +
> + mutex_lock(&chip->reg_lock);
> + if (chip->info->ops->serdes_power)
> + chip->info->ops->serdes_power(chip, port, false);
> + mutex_unlock(&chip->reg_lock);
> +}
Idem, please print a message in case of error as well:
if (mv88e6xxx_serdes_power())
dev_err(...)
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCHv2 net-next 4/5] net: dsa: mv88e6xxx: mv88e6390X SERDES support
From: Vivien Didelot @ 2017-05-24 14:59 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Andrew Lunn
In-Reply-To: <1495573486-31827-5-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> The mv88e6390X family has 8 SERDES lanes. These can be used for 2
> 10Gbps ports, ports 9 or 10. If these ports are used at slower speeds,
> the SERDES lanes become available for other ports for 1000Base-X.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCHv2 net-next 3/5] net: dsa: mv88e6xxx: Remove SERDES flag
From: Vivien Didelot @ 2017-05-24 14:59 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Andrew Lunn
In-Reply-To: <1495573486-31827-4-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> Now that we use an op for SERDES operations, we don't need a flag for
> it. Remove it.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCHv2 net-next 2/5] net: dsa: mv88e6xxx: Refactor mv88e6352 SERDES code into an op
From: Vivien Didelot @ 2017-05-24 15:03 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Andrew Lunn
In-Reply-To: <1495573486-31827-3-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> The mv88e6390 family has a different SERDES implementation. Refactor
> the mv88e6352 code into an ops function, so we can later add the
> mv88e6390 code.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Thanks,
Vivien
^ permalink raw reply
* Re: [PATCHv2 net-next 1/5] net: dsa: mv88e6xxx: Move phy functions into phy.[ch]
From: Vivien Didelot @ 2017-05-24 15:31 UTC (permalink / raw)
To: Andrew Lunn, David Miller; +Cc: netdev, Andrew Lunn
In-Reply-To: <1495573486-31827-2-git-send-email-andrew@lunn.ch>
Hi Andrew,
Andrew Lunn <andrew@lunn.ch> writes:
> The upcoming SERDES support will need to make use of PHY functions. Move
> them out into a file of there own. No code changes.
their
[...]
> +int mv88e6xxx_phy_page_get(struct mv88e6xxx_chip *chip, int phy, u8 page);
> +void mv88e6xxx_phy_page_put(struct mv88e6xxx_chip *chip, int phy);
I am not sure that you need to expose these functions. Can you confirm?
> +int mv88e6xxx_phy_page_read(struct mv88e6xxx_chip *chip, int phy,
> + u8 page, int reg, u16 *val);
> +int mv88e6xxx_phy_page_write(struct mv88e6xxx_chip *chip, int phy,
> + u8 page, int reg, u16 val);
> +int mv88e6xxx_ppu_disable(struct mv88e6xxx_chip *chip);
> +int mv88e6xxx_ppu_enable(struct mv88e6xxx_chip *chip);
> +void mv88e6xxx_ppu_state_init(struct mv88e6xxx_chip *chip);
> +void mv88e6xxx_ppu_state_destroy(struct mv88e6xxx_chip *chip);
If we want to move all PHY-related functions and wrappers into phy.[ch],
there are also mv88e6xxx_phy_init and mv88e6xxx_phy_destroy. This will
make most mv88e6xxx_ppu_* code static.
> +int mv88e6xxx_phy_ppu_read(struct mv88e6xxx_chip *chip, struct mii_bus *bus,
> + int addr, int reg, u16 *val);
> +int mv88e6xxx_phy_ppu_write(struct mv88e6xxx_chip *chip, struct mii_bus *bus,
> + int addr, int reg, u16 val);
Thanks,
Vivien
^ permalink raw reply
* [PATCH net-next] net/packet: remove unused parameter in prb_curr_blk_in_use().
From: Rami Rosen @ 2017-05-24 15:34 UTC (permalink / raw)
To: davem; +Cc: netdev, Rami Rosen
This patch removes unused parameter from prb_curr_blk_in_use() method
in net/packet/af_packet.c.
Signed-off-by: Rami Rosen <rami.rosen@intel.com>
---
net/packet/af_packet.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index e3eeed1..82ca49f 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -196,8 +196,7 @@ static void *packet_previous_frame(struct packet_sock *po,
struct packet_ring_buffer *rb,
int status);
static void packet_increment_head(struct packet_ring_buffer *buff);
-static int prb_curr_blk_in_use(struct tpacket_kbdq_core *,
- struct tpacket_block_desc *);
+static int prb_curr_blk_in_use(struct tpacket_block_desc *);
static void *prb_dispatch_next_block(struct tpacket_kbdq_core *,
struct packet_sock *);
static void prb_retire_current_block(struct tpacket_kbdq_core *,
@@ -721,7 +720,7 @@ static void prb_retire_rx_blk_timer_expired(unsigned long data)
/* Case 1. Queue was frozen because user-space was
* lagging behind.
*/
- if (prb_curr_blk_in_use(pkc, pbd)) {
+ if (prb_curr_blk_in_use(pbd)) {
/*
* Ok, user-space is still behind.
* So just refresh the timer.
@@ -972,8 +971,7 @@ static void prb_retire_current_block(struct tpacket_kbdq_core *pkc,
}
}
-static int prb_curr_blk_in_use(struct tpacket_kbdq_core *pkc,
- struct tpacket_block_desc *pbd)
+static int prb_curr_blk_in_use(struct tpacket_block_desc *pbd)
{
return TP_STATUS_USER & BLOCK_STATUS(pbd);
}
@@ -1064,7 +1062,7 @@ static void *__packet_lookup_frame_in_block(struct packet_sock *po,
* Check if that last block which caused the queue to freeze,
* is still in_use by user-space.
*/
- if (prb_curr_blk_in_use(pkc, pbd)) {
+ if (prb_curr_blk_in_use(pbd)) {
/* Can't record this packet */
return NULL;
} else {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next v9 3/5] rxrpc: check return value of skb_to_sgvec always
From: David Howells @ 2017-05-24 15:38 UTC (permalink / raw)
To: Jason A. Donenfeld; +Cc: dhowells, netdev, linux-kernel, davem
In-Reply-To: <20170523160550.5203-4-Jason@zx2c4.com>
Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Cc: David Howells <dhowells@redhat.com>
Acked-by: David Howells <dhowells@redhat.com>
^ permalink raw reply
* Re: [Patch net-next] net_sched: only create filter chains for new filters/actions
From: Cong Wang @ 2017-05-24 15:53 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Linux Kernel Network Developers, Jamal Hadi Salim, Jiri Pirko
In-Reply-To: <20170524063752.GA1908@nanopsycho>
On Tue, May 23, 2017 at 11:37 PM, Jiri Pirko <jiri@resnulli.us> wrote:
> Tue, May 23, 2017 at 06:42:37PM CEST, xiyou.wangcong@gmail.com wrote:
>>tcf_chain_get() always creates a new filter chain if not found
>>in existing ones. This is totally unnecessary when we get or
>>delete filters, new chain should be only created for new filters
>>(or new actions).
>>
>>Fixes: 5bc1701881e3 ("net: sched: introduce multichain support for filters")
>>Cc: Jamal Hadi Salim <jhs@mojatatu.com>
>>Cc: Jiri Pirko <jiri@mellanox.com>
>>Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
>>---
>> include/net/pkt_cls.h | 3 ++-
>> net/sched/act_api.c | 2 +-
>> net/sched/cls_api.c | 13 +++++++++----
>> 3 files changed, 12 insertions(+), 6 deletions(-)
>>
>>diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
>>index 2c213a6..f776229 100644
>>--- a/include/net/pkt_cls.h
>>+++ b/include/net/pkt_cls.h
>>@@ -18,7 +18,8 @@ int register_tcf_proto_ops(struct tcf_proto_ops *ops);
>> int unregister_tcf_proto_ops(struct tcf_proto_ops *ops);
>>
>> #ifdef CONFIG_NET_CLS
>>-struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index);
>>+struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
>>+ bool create);
>> void tcf_chain_put(struct tcf_chain *chain);
>> int tcf_block_get(struct tcf_block **p_block,
>> struct tcf_proto __rcu **p_filter_chain);
>>diff --git a/net/sched/act_api.c b/net/sched/act_api.c
>>index 0ecf2a8..aed6cf2 100644
>>--- a/net/sched/act_api.c
>>+++ b/net/sched/act_api.c
>>@@ -34,7 +34,7 @@ static int tcf_action_goto_chain_init(struct tc_action *a, struct tcf_proto *tp)
>>
>> if (!tp)
>> return -EINVAL;
>>- a->goto_chain = tcf_chain_get(tp->chain->block, chain_index);
>>+ a->goto_chain = tcf_chain_get(tp->chain->block, chain_index, true);
>> if (!a->goto_chain)
>> return -ENOMEM;
>> return 0;
>>diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
>>index 01a8b8b..23d2236 100644
>>--- a/net/sched/cls_api.c
>>+++ b/net/sched/cls_api.c
>>@@ -220,7 +220,8 @@ static void tcf_chain_destroy(struct tcf_chain *chain)
>> kfree(chain);
>> }
>>
>>-struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index)
>>+struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index,
>>+ bool create)
>> {
>> struct tcf_chain *chain;
>>
>>@@ -230,7 +231,10 @@ struct tcf_chain *tcf_chain_get(struct tcf_block *block, u32 chain_index)
>> return chain;
>> }
>> }
>>- return tcf_chain_create(block, chain_index);
>>+ if (create)
>>+ return tcf_chain_create(block, chain_index);
>>+ else
>>+ return NULL;
>> }
>> EXPORT_SYMBOL(tcf_chain_get);
>>
>>@@ -509,9 +513,10 @@ static int tc_ctl_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
>> err = -EINVAL;
>> goto errout;
>> }
>>- chain = tcf_chain_get(block, chain_index);
>>+ chain = tcf_chain_get(block, chain_index,
>>+ n->nlmsg_type == RTM_NEWTFILTER);
>
> First of all, I really hate all these true/false arg dances. Totaly
> confusing all the time.
Sounds like you are able to understand the code at all.
Sigh, I bet you never even read the changelog. ;)
>
>
>
>> if (!chain) {
>>- err = -ENOMEM;
>>+ err = n->nlmsg_type == RTM_NEWTFILTER ? -ENOMEM : -EINVAL;
>
> Confusing. Please do not obfuscate the code for a corner cases. Thanks.
Either you don't understand the changelog or you don't understand
ternary conditional operator. I can't help you if the latter.
Sorry.
^ permalink raw reply
* [PATCH net-next] rxrpc: Support network namespacing
From: David Howells @ 2017-05-24 16:02 UTC (permalink / raw)
To: netdev; +Cc: dhowells, linux-afs, linux-kernel
Support network namespacing in AF_RXRPC with the following changes:
(1) All the local endpoint, peer and call lists, locks, counters, etc. are
moved into the per-namespace record.
(2) All the connection tracking is moved into the per-namespace record
with the exception of the client connection ID tree, which is kept
global so that connection IDs are kept unique per-machine.
(3) Each namespace gets its own epoch. This allows each network namespace
to pretend to be a separate client machine.
(4) The /proc/net/rxrpc_xxx files are now called /proc/net/rxrpc/xxx and
the contents reflect the namespace.
fs/afs/ should be okay with this patch as it explicitly requires the current
net namespace to be init_net to permit a mount to proceed at the moment. It
will, however, need updating so that cells, IP addresses and DNS records are
per-namespace also.
Signed-off-by: David Howells <dhowells@redhat.com>
---
net/rxrpc/Makefile | 1
net/rxrpc/af_rxrpc.c | 35 +++++------
net/rxrpc/ar-internal.h | 65 ++++++++++++++++----
net/rxrpc/call_accept.c | 14 ++--
net/rxrpc/call_object.c | 39 +++++++-----
net/rxrpc/conn_client.c | 153 +++++++++++++++++++++++-----------------------
net/rxrpc/conn_object.c | 55 ++++++++---------
net/rxrpc/conn_service.c | 11 ++-
net/rxrpc/local_object.c | 48 +++++++-------
net/rxrpc/net_ns.c | 85 ++++++++++++++++++++++++++
net/rxrpc/peer_object.c | 26 ++++----
net/rxrpc/proc.c | 40 ++++++++----
12 files changed, 356 insertions(+), 216 deletions(-)
create mode 100644 net/rxrpc/net_ns.c
diff --git a/net/rxrpc/Makefile b/net/rxrpc/Makefile
index b9da4d6b914f..9c68d2f8ba39 100644
--- a/net/rxrpc/Makefile
+++ b/net/rxrpc/Makefile
@@ -19,6 +19,7 @@ rxrpc-y := \
local_event.o \
local_object.o \
misc.o \
+ net_ns.o \
output.o \
peer_event.o \
peer_object.o \
diff --git a/net/rxrpc/af_rxrpc.c b/net/rxrpc/af_rxrpc.c
index 7fb59c3f1542..cd34ffbff1d1 100644
--- a/net/rxrpc/af_rxrpc.c
+++ b/net/rxrpc/af_rxrpc.c
@@ -38,9 +38,6 @@ MODULE_PARM_DESC(debug, "RxRPC debugging mask");
static struct proto rxrpc_proto;
static const struct proto_ops rxrpc_rpc_ops;
-/* local epoch for detecting local-end reset */
-u32 rxrpc_epoch;
-
/* current debugging ID */
atomic_t rxrpc_debug_id;
@@ -155,7 +152,7 @@ static int rxrpc_bind(struct socket *sock, struct sockaddr *saddr, int len)
memcpy(&rx->srx, srx, sizeof(rx->srx));
- local = rxrpc_lookup_local(&rx->srx);
+ local = rxrpc_lookup_local(sock_net(sock->sk), &rx->srx);
if (IS_ERR(local)) {
ret = PTR_ERR(local);
goto error_unlock;
@@ -434,7 +431,7 @@ static int rxrpc_sendmsg(struct socket *sock, struct msghdr *m, size_t len)
ret = -EAFNOSUPPORT;
goto error_unlock;
}
- local = rxrpc_lookup_local(&rx->srx);
+ local = rxrpc_lookup_local(sock_net(sock->sk), &rx->srx);
if (IS_ERR(local)) {
ret = PTR_ERR(local);
goto error_unlock;
@@ -582,9 +579,6 @@ static int rxrpc_create(struct net *net, struct socket *sock, int protocol,
_enter("%p,%d", sock, protocol);
- if (!net_eq(net, &init_net))
- return -EAFNOSUPPORT;
-
/* we support transport protocol UDP/UDP6 only */
if (protocol != PF_INET &&
IS_ENABLED(CONFIG_AF_RXRPC_IPV6) && protocol != PF_INET6)
@@ -780,8 +774,6 @@ static int __init af_rxrpc_init(void)
BUILD_BUG_ON(sizeof(struct rxrpc_skb_priv) > FIELD_SIZEOF(struct sk_buff, cb));
- get_random_bytes(&rxrpc_epoch, sizeof(rxrpc_epoch));
- rxrpc_epoch |= RXRPC_RANDOM_EPOCH;
get_random_bytes(&tmp, sizeof(tmp));
tmp &= 0x3fffffff;
if (tmp == 0)
@@ -809,6 +801,10 @@ static int __init af_rxrpc_init(void)
goto error_security;
}
+ ret = register_pernet_subsys(&rxrpc_net_ops);
+ if (ret)
+ goto error_pernet;
+
ret = proto_register(&rxrpc_proto, 1);
if (ret < 0) {
pr_crit("Cannot register protocol\n");
@@ -839,11 +835,6 @@ static int __init af_rxrpc_init(void)
goto error_sysctls;
}
-#ifdef CONFIG_PROC_FS
- proc_create("rxrpc_calls", 0, init_net.proc_net, &rxrpc_call_seq_fops);
- proc_create("rxrpc_conns", 0, init_net.proc_net,
- &rxrpc_connection_seq_fops);
-#endif
return 0;
error_sysctls:
@@ -855,6 +846,8 @@ static int __init af_rxrpc_init(void)
error_sock:
proto_unregister(&rxrpc_proto);
error_proto:
+ unregister_pernet_subsys(&rxrpc_net_ops);
+error_pernet:
rxrpc_exit_security();
error_security:
destroy_workqueue(rxrpc_workqueue);
@@ -875,14 +868,16 @@ static void __exit af_rxrpc_exit(void)
unregister_key_type(&key_type_rxrpc);
sock_unregister(PF_RXRPC);
proto_unregister(&rxrpc_proto);
- rxrpc_destroy_all_calls();
- rxrpc_destroy_all_connections();
+ unregister_pernet_subsys(&rxrpc_net_ops);
ASSERTCMP(atomic_read(&rxrpc_n_tx_skbs), ==, 0);
ASSERTCMP(atomic_read(&rxrpc_n_rx_skbs), ==, 0);
- rxrpc_destroy_all_locals();
- remove_proc_entry("rxrpc_conns", init_net.proc_net);
- remove_proc_entry("rxrpc_calls", init_net.proc_net);
+ /* Make sure the local and peer records pinned by any dying connections
+ * are released.
+ */
+ rcu_barrier();
+ rxrpc_destroy_client_conn_ids();
+
destroy_workqueue(rxrpc_workqueue);
rxrpc_exit_security();
kmem_cache_destroy(rxrpc_call_jar);
diff --git a/net/rxrpc/ar-internal.h b/net/rxrpc/ar-internal.h
index 7486926e60a8..067dbb3121d0 100644
--- a/net/rxrpc/ar-internal.h
+++ b/net/rxrpc/ar-internal.h
@@ -11,6 +11,8 @@
#include <linux/atomic.h>
#include <linux/seqlock.h>
+#include <net/net_namespace.h>
+#include <net/netns/generic.h>
#include <net/sock.h>
#include <net/af_rxrpc.h>
#include <rxrpc/packet.h>
@@ -65,6 +67,37 @@ enum {
};
/*
+ * Per-network namespace data.
+ */
+struct rxrpc_net {
+ struct proc_dir_entry *proc_net; /* Subdir in /proc/net */
+ u32 epoch; /* Local epoch for detecting local-end reset */
+ struct list_head calls; /* List of calls active in this namespace */
+ rwlock_t call_lock; /* Lock for ->calls */
+
+ struct list_head conn_proc_list; /* List of conns in this namespace for proc */
+ struct list_head service_conns; /* Service conns in this namespace */
+ rwlock_t conn_lock; /* Lock for ->conn_proc_list, ->service_conns */
+ struct delayed_work service_conn_reaper;
+
+ unsigned int nr_client_conns;
+ unsigned int nr_active_client_conns;
+ bool kill_all_client_conns;
+ spinlock_t client_conn_cache_lock; /* Lock for ->*_client_conns */
+ spinlock_t client_conn_discard_lock; /* Prevent multiple discarders */
+ struct list_head waiting_client_conns;
+ struct list_head active_client_conns;
+ struct list_head idle_client_conns;
+ struct delayed_work client_conn_reaper;
+
+ struct list_head local_endpoints;
+ struct mutex local_mutex; /* Lock for ->local_endpoints */
+
+ spinlock_t peer_hash_lock; /* Lock for ->peer_hash */
+ DECLARE_HASHTABLE (peer_hash, 10);
+};
+
+/*
* Service backlog preallocation.
*
* This contains circular buffers of preallocated peers, connections and calls
@@ -211,6 +244,7 @@ struct rxrpc_security {
struct rxrpc_local {
struct rcu_head rcu;
atomic_t usage;
+ struct rxrpc_net *rxnet; /* The network ns in which this resides */
struct list_head link;
struct socket *socket; /* my UDP socket */
struct work_struct processor;
@@ -601,7 +635,6 @@ struct rxrpc_ack_summary {
* af_rxrpc.c
*/
extern atomic_t rxrpc_n_tx_skbs, rxrpc_n_rx_skbs;
-extern u32 rxrpc_epoch;
extern atomic_t rxrpc_debug_id;
extern struct workqueue_struct *rxrpc_workqueue;
@@ -634,8 +667,6 @@ extern const char *const rxrpc_call_states[];
extern const char *const rxrpc_call_completions[];
extern unsigned int rxrpc_max_call_lifetime;
extern struct kmem_cache *rxrpc_call_jar;
-extern struct list_head rxrpc_calls;
-extern rwlock_t rxrpc_call_lock;
struct rxrpc_call *rxrpc_find_call_by_user_ID(struct rxrpc_sock *, unsigned long);
struct rxrpc_call *rxrpc_alloc_call(gfp_t);
@@ -653,7 +684,7 @@ void rxrpc_see_call(struct rxrpc_call *);
void rxrpc_get_call(struct rxrpc_call *, enum rxrpc_call_trace);
void rxrpc_put_call(struct rxrpc_call *, enum rxrpc_call_trace);
void rxrpc_cleanup_call(struct rxrpc_call *);
-void __exit rxrpc_destroy_all_calls(void);
+void rxrpc_destroy_all_calls(struct rxrpc_net *);
static inline bool rxrpc_is_service_call(const struct rxrpc_call *call)
{
@@ -773,7 +804,8 @@ int rxrpc_connect_call(struct rxrpc_call *, struct rxrpc_conn_parameters *,
void rxrpc_expose_client_call(struct rxrpc_call *);
void rxrpc_disconnect_client_call(struct rxrpc_call *);
void rxrpc_put_client_conn(struct rxrpc_connection *);
-void __exit rxrpc_destroy_all_client_connections(void);
+void rxrpc_discard_expired_client_conns(struct work_struct *);
+void rxrpc_destroy_all_client_connections(struct rxrpc_net *);
/*
* conn_event.c
@@ -784,9 +816,6 @@ void rxrpc_process_connection(struct work_struct *);
* conn_object.c
*/
extern unsigned int rxrpc_connection_expiry;
-extern struct list_head rxrpc_connections;
-extern struct list_head rxrpc_connection_proc_list;
-extern rwlock_t rxrpc_connection_lock;
int rxrpc_extract_addr_from_skb(struct sockaddr_rxrpc *, struct sk_buff *);
struct rxrpc_connection *rxrpc_alloc_connection(gfp_t);
@@ -800,7 +829,8 @@ void rxrpc_see_connection(struct rxrpc_connection *);
void rxrpc_get_connection(struct rxrpc_connection *);
struct rxrpc_connection *rxrpc_get_connection_maybe(struct rxrpc_connection *);
void rxrpc_put_service_conn(struct rxrpc_connection *);
-void __exit rxrpc_destroy_all_connections(void);
+void rxrpc_service_connection_reaper(struct work_struct *);
+void rxrpc_destroy_all_connections(struct rxrpc_net *);
static inline bool rxrpc_conn_is_client(const struct rxrpc_connection *conn)
{
@@ -828,7 +858,7 @@ static inline void rxrpc_put_connection(struct rxrpc_connection *conn)
*/
struct rxrpc_connection *rxrpc_find_service_conn_rcu(struct rxrpc_peer *,
struct sk_buff *);
-struct rxrpc_connection *rxrpc_prealloc_service_connection(gfp_t);
+struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *, gfp_t);
void rxrpc_new_incoming_connection(struct rxrpc_connection *, struct sk_buff *);
void rxrpc_unpublish_service_conn(struct rxrpc_connection *);
@@ -861,9 +891,9 @@ extern void rxrpc_process_local_events(struct rxrpc_local *);
/*
* local_object.c
*/
-struct rxrpc_local *rxrpc_lookup_local(const struct sockaddr_rxrpc *);
+struct rxrpc_local *rxrpc_lookup_local(struct net *, const struct sockaddr_rxrpc *);
void __rxrpc_put_local(struct rxrpc_local *);
-void __exit rxrpc_destroy_all_locals(void);
+void rxrpc_destroy_all_locals(struct rxrpc_net *);
static inline void rxrpc_get_local(struct rxrpc_local *local)
{
@@ -902,6 +932,17 @@ extern unsigned int rxrpc_resend_timeout;
extern const s8 rxrpc_ack_priority[];
/*
+ * net_ns.c
+ */
+extern unsigned int rxrpc_net_id;
+extern struct pernet_operations rxrpc_net_ops;
+
+static inline struct rxrpc_net *rxrpc_net(struct net *net)
+{
+ return net_generic(net, rxrpc_net_id);
+}
+
+/*
* output.c
*/
int rxrpc_send_ack_packet(struct rxrpc_call *, bool);
diff --git a/net/rxrpc/call_accept.c b/net/rxrpc/call_accept.c
index 1752fcf8e8f1..a8515b0d4717 100644
--- a/net/rxrpc/call_accept.c
+++ b/net/rxrpc/call_accept.c
@@ -38,6 +38,7 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
{
const void *here = __builtin_return_address(0);
struct rxrpc_call *call;
+ struct rxrpc_net *rxnet = rxrpc_net(sock_net(&rx->sk));
int max, tmp;
unsigned int size = RXRPC_BACKLOG_MAX;
unsigned int head, tail, call_head, call_tail;
@@ -79,7 +80,7 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
if (CIRC_CNT(head, tail, size) < max) {
struct rxrpc_connection *conn;
- conn = rxrpc_prealloc_service_connection(gfp);
+ conn = rxrpc_prealloc_service_connection(rxnet, gfp);
if (!conn)
return -ENOMEM;
b->conn_backlog[head] = conn;
@@ -136,9 +137,9 @@ static int rxrpc_service_prealloc_one(struct rxrpc_sock *rx,
write_unlock(&rx->call_lock);
- write_lock(&rxrpc_call_lock);
- list_add_tail(&call->link, &rxrpc_calls);
- write_unlock(&rxrpc_call_lock);
+ write_lock(&rxnet->call_lock);
+ list_add_tail(&call->link, &rxnet->calls);
+ write_unlock(&rxnet->call_lock);
b->call_backlog[call_head] = call;
smp_store_release(&b->call_backlog_head, (call_head + 1) & (size - 1));
@@ -185,6 +186,7 @@ int rxrpc_service_prealloc(struct rxrpc_sock *rx, gfp_t gfp)
void rxrpc_discard_prealloc(struct rxrpc_sock *rx)
{
struct rxrpc_backlog *b = rx->backlog;
+ struct rxrpc_net *rxnet = rxrpc_net(sock_net(&rx->sk));
unsigned int size = RXRPC_BACKLOG_MAX, head, tail;
if (!b)
@@ -209,10 +211,10 @@ void rxrpc_discard_prealloc(struct rxrpc_sock *rx)
tail = b->conn_backlog_tail;
while (CIRC_CNT(head, tail, size) > 0) {
struct rxrpc_connection *conn = b->conn_backlog[tail];
- write_lock(&rxrpc_connection_lock);
+ write_lock(&rxnet->conn_lock);
list_del(&conn->link);
list_del(&conn->proc_link);
- write_unlock(&rxrpc_connection_lock);
+ write_unlock(&rxnet->conn_lock);
kfree(conn);
tail = (tail + 1) & (size - 1);
}
diff --git a/net/rxrpc/call_object.c b/net/rxrpc/call_object.c
index 47f7f4205653..692110808baa 100644
--- a/net/rxrpc/call_object.c
+++ b/net/rxrpc/call_object.c
@@ -44,8 +44,6 @@ const char *const rxrpc_call_completions[NR__RXRPC_CALL_COMPLETIONS] = {
};
struct kmem_cache *rxrpc_call_jar;
-LIST_HEAD(rxrpc_calls);
-DEFINE_RWLOCK(rxrpc_call_lock);
static void rxrpc_call_timer_expired(unsigned long _call)
{
@@ -207,6 +205,7 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
__releases(&rx->sk.sk_lock.slock)
{
struct rxrpc_call *call, *xcall;
+ struct rxrpc_net *rxnet = rxrpc_net(sock_net(&rx->sk));
struct rb_node *parent, **pp;
const void *here = __builtin_return_address(0);
int ret;
@@ -255,9 +254,9 @@ struct rxrpc_call *rxrpc_new_client_call(struct rxrpc_sock *rx,
write_unlock(&rx->call_lock);
- write_lock(&rxrpc_call_lock);
- list_add_tail(&call->link, &rxrpc_calls);
- write_unlock(&rxrpc_call_lock);
+ write_lock(&rxnet->call_lock);
+ list_add_tail(&call->link, &rxnet->calls);
+ write_unlock(&rxnet->call_lock);
/* From this point on, the call is protected by its own lock. */
release_sock(&rx->sk);
@@ -508,6 +507,7 @@ void rxrpc_release_calls_on_socket(struct rxrpc_sock *rx)
*/
void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace op)
{
+ struct rxrpc_net *rxnet;
const void *here = __builtin_return_address(0);
int n;
@@ -520,9 +520,12 @@ void rxrpc_put_call(struct rxrpc_call *call, enum rxrpc_call_trace op)
_debug("call %d dead", call->debug_id);
ASSERTCMP(call->state, ==, RXRPC_CALL_COMPLETE);
- write_lock(&rxrpc_call_lock);
- list_del_init(&call->link);
- write_unlock(&rxrpc_call_lock);
+ if (!list_empty(&call->link)) {
+ rxnet = rxrpc_net(sock_net(&call->socket->sk));
+ write_lock(&rxnet->call_lock);
+ list_del_init(&call->link);
+ write_unlock(&rxnet->call_lock);
+ }
rxrpc_cleanup_call(call);
}
@@ -570,21 +573,23 @@ void rxrpc_cleanup_call(struct rxrpc_call *call)
}
/*
- * Make sure that all calls are gone.
+ * Make sure that all calls are gone from a network namespace. To reach this
+ * point, any open UDP sockets in that namespace must have been closed, so any
+ * outstanding calls cannot be doing I/O.
*/
-void __exit rxrpc_destroy_all_calls(void)
+void rxrpc_destroy_all_calls(struct rxrpc_net *rxnet)
{
struct rxrpc_call *call;
_enter("");
- if (list_empty(&rxrpc_calls))
+ if (list_empty(&rxnet->calls))
return;
- write_lock(&rxrpc_call_lock);
+ write_lock(&rxnet->call_lock);
- while (!list_empty(&rxrpc_calls)) {
- call = list_entry(rxrpc_calls.next, struct rxrpc_call, link);
+ while (!list_empty(&rxnet->calls)) {
+ call = list_entry(rxnet->calls.next, struct rxrpc_call, link);
_debug("Zapping call %p", call);
rxrpc_see_call(call);
@@ -595,10 +600,10 @@ void __exit rxrpc_destroy_all_calls(void)
rxrpc_call_states[call->state],
call->flags, call->events);
- write_unlock(&rxrpc_call_lock);
+ write_unlock(&rxnet->call_lock);
cond_resched();
- write_lock(&rxrpc_call_lock);
+ write_lock(&rxnet->call_lock);
}
- write_unlock(&rxrpc_call_lock);
+ write_unlock(&rxnet->call_lock);
}
diff --git a/net/rxrpc/conn_client.c b/net/rxrpc/conn_client.c
index e8dea0d49e7f..c86f3202f967 100644
--- a/net/rxrpc/conn_client.c
+++ b/net/rxrpc/conn_client.c
@@ -31,7 +31,7 @@
* may freely grant available channels to new calls and calls may be
* waiting on it for channels to become available.
*
- * The connection is on the rxrpc_active_client_conns list which is kept
+ * The connection is on the rxnet->active_client_conns list which is kept
* in activation order for culling purposes.
*
* rxrpc_nr_active_client_conns is held incremented also.
@@ -46,7 +46,7 @@
* expires, the EXPOSED flag is cleared and the connection transitions to
* the INACTIVE state.
*
- * The connection is on the rxrpc_idle_client_conns list which is kept in
+ * The connection is on the rxnet->idle_client_conns list which is kept in
* order of how soon they'll expire.
*
* There are flags of relevance to the cache:
@@ -85,27 +85,13 @@ __read_mostly unsigned int rxrpc_reap_client_connections = 900;
__read_mostly unsigned int rxrpc_conn_idle_client_expiry = 2 * 60 * HZ;
__read_mostly unsigned int rxrpc_conn_idle_client_fast_expiry = 2 * HZ;
-static unsigned int rxrpc_nr_client_conns;
-static unsigned int rxrpc_nr_active_client_conns;
-static __read_mostly bool rxrpc_kill_all_client_conns;
-
-static DEFINE_SPINLOCK(rxrpc_client_conn_cache_lock);
-static DEFINE_SPINLOCK(rxrpc_client_conn_discard_mutex);
-static LIST_HEAD(rxrpc_waiting_client_conns);
-static LIST_HEAD(rxrpc_active_client_conns);
-static LIST_HEAD(rxrpc_idle_client_conns);
-
/*
* We use machine-unique IDs for our client connections.
*/
DEFINE_IDR(rxrpc_client_conn_ids);
static DEFINE_SPINLOCK(rxrpc_conn_id_lock);
-static void rxrpc_cull_active_client_conns(void);
-static void rxrpc_discard_expired_client_conns(struct work_struct *);
-
-static DECLARE_DELAYED_WORK(rxrpc_client_conn_reap,
- rxrpc_discard_expired_client_conns);
+static void rxrpc_cull_active_client_conns(struct rxrpc_net *);
/*
* Get a connection ID and epoch for a client connection from the global pool.
@@ -116,6 +102,7 @@ static DECLARE_DELAYED_WORK(rxrpc_client_conn_reap,
static int rxrpc_get_client_connection_id(struct rxrpc_connection *conn,
gfp_t gfp)
{
+ struct rxrpc_net *rxnet = conn->params.local->rxnet;
int id;
_enter("");
@@ -131,7 +118,7 @@ static int rxrpc_get_client_connection_id(struct rxrpc_connection *conn,
spin_unlock(&rxrpc_conn_id_lock);
idr_preload_end();
- conn->proto.epoch = rxrpc_epoch;
+ conn->proto.epoch = rxnet->epoch;
conn->proto.cid = id << RXRPC_CIDSHIFT;
set_bit(RXRPC_CONN_HAS_IDR, &conn->flags);
_leave(" [CID %x]", conn->proto.cid);
@@ -183,6 +170,7 @@ static struct rxrpc_connection *
rxrpc_alloc_client_connection(struct rxrpc_conn_parameters *cp, gfp_t gfp)
{
struct rxrpc_connection *conn;
+ struct rxrpc_net *rxnet = cp->local->rxnet;
int ret;
_enter("");
@@ -213,9 +201,9 @@ rxrpc_alloc_client_connection(struct rxrpc_conn_parameters *cp, gfp_t gfp)
if (ret < 0)
goto error_2;
- write_lock(&rxrpc_connection_lock);
- list_add_tail(&conn->proc_link, &rxrpc_connection_proc_list);
- write_unlock(&rxrpc_connection_lock);
+ write_lock(&rxnet->conn_lock);
+ list_add_tail(&conn->proc_link, &rxnet->conn_proc_list);
+ write_unlock(&rxnet->conn_lock);
/* We steal the caller's peer ref. */
cp->peer = NULL;
@@ -243,12 +231,13 @@ rxrpc_alloc_client_connection(struct rxrpc_conn_parameters *cp, gfp_t gfp)
*/
static bool rxrpc_may_reuse_conn(struct rxrpc_connection *conn)
{
+ struct rxrpc_net *rxnet = conn->params.local->rxnet;
int id_cursor, id, distance, limit;
if (test_bit(RXRPC_CONN_DONT_REUSE, &conn->flags))
goto dont_reuse;
- if (conn->proto.epoch != rxrpc_epoch)
+ if (conn->proto.epoch != rxnet->epoch)
goto mark_dont_reuse;
/* The IDR tree gets very expensive on memory if the connection IDs are
@@ -440,12 +429,13 @@ static int rxrpc_get_client_conn(struct rxrpc_call *call,
/*
* Activate a connection.
*/
-static void rxrpc_activate_conn(struct rxrpc_connection *conn)
+static void rxrpc_activate_conn(struct rxrpc_net *rxnet,
+ struct rxrpc_connection *conn)
{
trace_rxrpc_client(conn, -1, rxrpc_client_to_active);
conn->cache_state = RXRPC_CONN_CLIENT_ACTIVE;
- rxrpc_nr_active_client_conns++;
- list_move_tail(&conn->cache_link, &rxrpc_active_client_conns);
+ rxnet->nr_active_client_conns++;
+ list_move_tail(&conn->cache_link, &rxnet->active_client_conns);
}
/*
@@ -460,7 +450,8 @@ static void rxrpc_activate_conn(struct rxrpc_connection *conn)
* channels if it has been culled to make space and then re-requested by a new
* call.
*/
-static void rxrpc_animate_client_conn(struct rxrpc_connection *conn)
+static void rxrpc_animate_client_conn(struct rxrpc_net *rxnet,
+ struct rxrpc_connection *conn)
{
unsigned int nr_conns;
@@ -469,12 +460,12 @@ static void rxrpc_animate_client_conn(struct rxrpc_connection *conn)
if (conn->cache_state == RXRPC_CONN_CLIENT_ACTIVE)
goto out;
- spin_lock(&rxrpc_client_conn_cache_lock);
+ spin_lock(&rxnet->client_conn_cache_lock);
- nr_conns = rxrpc_nr_client_conns;
+ nr_conns = rxnet->nr_client_conns;
if (!test_and_set_bit(RXRPC_CONN_COUNTED, &conn->flags)) {
trace_rxrpc_client(conn, -1, rxrpc_client_count);
- rxrpc_nr_client_conns = nr_conns + 1;
+ rxnet->nr_client_conns = nr_conns + 1;
}
switch (conn->cache_state) {
@@ -494,21 +485,21 @@ static void rxrpc_animate_client_conn(struct rxrpc_connection *conn)
}
out_unlock:
- spin_unlock(&rxrpc_client_conn_cache_lock);
+ spin_unlock(&rxnet->client_conn_cache_lock);
out:
_leave(" [%d]", conn->cache_state);
return;
activate_conn:
_debug("activate");
- rxrpc_activate_conn(conn);
+ rxrpc_activate_conn(rxnet, conn);
goto out_unlock;
wait_for_capacity:
_debug("wait");
trace_rxrpc_client(conn, -1, rxrpc_client_to_waiting);
conn->cache_state = RXRPC_CONN_CLIENT_WAITING;
- list_move_tail(&conn->cache_link, &rxrpc_waiting_client_conns);
+ list_move_tail(&conn->cache_link, &rxnet->waiting_client_conns);
goto out_unlock;
}
@@ -660,18 +651,19 @@ int rxrpc_connect_call(struct rxrpc_call *call,
struct sockaddr_rxrpc *srx,
gfp_t gfp)
{
+ struct rxrpc_net *rxnet = cp->local->rxnet;
int ret;
_enter("{%d,%lx},", call->debug_id, call->user_call_ID);
- rxrpc_discard_expired_client_conns(NULL);
- rxrpc_cull_active_client_conns();
+ rxrpc_discard_expired_client_conns(&rxnet->client_conn_reaper.work);
+ rxrpc_cull_active_client_conns(rxnet);
ret = rxrpc_get_client_conn(call, cp, srx, gfp);
if (ret < 0)
return ret;
- rxrpc_animate_client_conn(call->conn);
+ rxrpc_animate_client_conn(rxnet, call->conn);
rxrpc_activate_channels(call->conn);
ret = rxrpc_wait_for_channel(call, gfp);
@@ -729,6 +721,7 @@ void rxrpc_disconnect_client_call(struct rxrpc_call *call)
unsigned int channel = call->cid & RXRPC_CHANNELMASK;
struct rxrpc_connection *conn = call->conn;
struct rxrpc_channel *chan = &conn->channels[channel];
+ struct rxrpc_net *rxnet = rxrpc_net(sock_net(&call->socket->sk));
trace_rxrpc_client(conn, channel, rxrpc_client_chan_disconnect);
call->conn = NULL;
@@ -750,7 +743,7 @@ void rxrpc_disconnect_client_call(struct rxrpc_call *call)
/* We must deactivate or idle the connection if it's now
* waiting for nothing.
*/
- spin_lock(&rxrpc_client_conn_cache_lock);
+ spin_lock(&rxnet->client_conn_cache_lock);
if (conn->cache_state == RXRPC_CONN_CLIENT_WAITING &&
list_empty(&conn->waiting_calls) &&
!conn->active_chans)
@@ -787,14 +780,14 @@ void rxrpc_disconnect_client_call(struct rxrpc_call *call)
* list. It might even get moved back to the active list whilst we're
* waiting for the lock.
*/
- spin_lock(&rxrpc_client_conn_cache_lock);
+ spin_lock(&rxnet->client_conn_cache_lock);
switch (conn->cache_state) {
case RXRPC_CONN_CLIENT_ACTIVE:
if (list_empty(&conn->waiting_calls)) {
rxrpc_deactivate_one_channel(conn, channel);
if (!conn->active_chans) {
- rxrpc_nr_active_client_conns--;
+ rxnet->nr_active_client_conns--;
goto idle_connection;
}
goto out;
@@ -820,7 +813,7 @@ void rxrpc_disconnect_client_call(struct rxrpc_call *call)
}
out:
- spin_unlock(&rxrpc_client_conn_cache_lock);
+ spin_unlock(&rxnet->client_conn_cache_lock);
out_2:
spin_unlock(&conn->channel_lock);
rxrpc_put_connection(conn);
@@ -835,11 +828,11 @@ void rxrpc_disconnect_client_call(struct rxrpc_call *call)
trace_rxrpc_client(conn, channel, rxrpc_client_to_idle);
conn->idle_timestamp = jiffies;
conn->cache_state = RXRPC_CONN_CLIENT_IDLE;
- list_move_tail(&conn->cache_link, &rxrpc_idle_client_conns);
- if (rxrpc_idle_client_conns.next == &conn->cache_link &&
- !rxrpc_kill_all_client_conns)
+ list_move_tail(&conn->cache_link, &rxnet->idle_client_conns);
+ if (rxnet->idle_client_conns.next == &conn->cache_link &&
+ !rxnet->kill_all_client_conns)
queue_delayed_work(rxrpc_workqueue,
- &rxrpc_client_conn_reap,
+ &rxnet->client_conn_reaper,
rxrpc_conn_idle_client_expiry);
} else {
trace_rxrpc_client(conn, channel, rxrpc_client_to_inactive);
@@ -857,6 +850,7 @@ rxrpc_put_one_client_conn(struct rxrpc_connection *conn)
{
struct rxrpc_connection *next = NULL;
struct rxrpc_local *local = conn->params.local;
+ struct rxrpc_net *rxnet = local->rxnet;
unsigned int nr_conns;
trace_rxrpc_client(conn, -1, rxrpc_client_cleanup);
@@ -875,18 +869,18 @@ rxrpc_put_one_client_conn(struct rxrpc_connection *conn)
if (test_bit(RXRPC_CONN_COUNTED, &conn->flags)) {
trace_rxrpc_client(conn, -1, rxrpc_client_uncount);
- spin_lock(&rxrpc_client_conn_cache_lock);
- nr_conns = --rxrpc_nr_client_conns;
+ spin_lock(&rxnet->client_conn_cache_lock);
+ nr_conns = --rxnet->nr_client_conns;
if (nr_conns < rxrpc_max_client_connections &&
- !list_empty(&rxrpc_waiting_client_conns)) {
- next = list_entry(rxrpc_waiting_client_conns.next,
+ !list_empty(&rxnet->waiting_client_conns)) {
+ next = list_entry(rxnet->waiting_client_conns.next,
struct rxrpc_connection, cache_link);
rxrpc_get_connection(next);
- rxrpc_activate_conn(next);
+ rxrpc_activate_conn(rxnet, next);
}
- spin_unlock(&rxrpc_client_conn_cache_lock);
+ spin_unlock(&rxnet->client_conn_cache_lock);
}
rxrpc_kill_connection(conn);
@@ -921,10 +915,10 @@ void rxrpc_put_client_conn(struct rxrpc_connection *conn)
/*
* Kill the longest-active client connections to make room for new ones.
*/
-static void rxrpc_cull_active_client_conns(void)
+static void rxrpc_cull_active_client_conns(struct rxrpc_net *rxnet)
{
struct rxrpc_connection *conn;
- unsigned int nr_conns = rxrpc_nr_client_conns;
+ unsigned int nr_conns = rxnet->nr_client_conns;
unsigned int nr_active, limit;
_enter("");
@@ -936,12 +930,12 @@ static void rxrpc_cull_active_client_conns(void)
}
limit = rxrpc_reap_client_connections;
- spin_lock(&rxrpc_client_conn_cache_lock);
- nr_active = rxrpc_nr_active_client_conns;
+ spin_lock(&rxnet->client_conn_cache_lock);
+ nr_active = rxnet->nr_active_client_conns;
while (nr_active > limit) {
- ASSERT(!list_empty(&rxrpc_active_client_conns));
- conn = list_entry(rxrpc_active_client_conns.next,
+ ASSERT(!list_empty(&rxnet->active_client_conns));
+ conn = list_entry(rxnet->active_client_conns.next,
struct rxrpc_connection, cache_link);
ASSERTCMP(conn->cache_state, ==, RXRPC_CONN_CLIENT_ACTIVE);
@@ -953,14 +947,14 @@ static void rxrpc_cull_active_client_conns(void)
trace_rxrpc_client(conn, -1, rxrpc_client_to_waiting);
conn->cache_state = RXRPC_CONN_CLIENT_WAITING;
list_move_tail(&conn->cache_link,
- &rxrpc_waiting_client_conns);
+ &rxnet->waiting_client_conns);
}
nr_active--;
}
- rxrpc_nr_active_client_conns = nr_active;
- spin_unlock(&rxrpc_client_conn_cache_lock);
+ rxnet->nr_active_client_conns = nr_active;
+ spin_unlock(&rxnet->client_conn_cache_lock);
ASSERTCMP(nr_active, >=, 0);
_leave(" [culled]");
}
@@ -972,22 +966,25 @@ static void rxrpc_cull_active_client_conns(void)
* This may be called from conn setup or from a work item so cannot be
* considered non-reentrant.
*/
-static void rxrpc_discard_expired_client_conns(struct work_struct *work)
+void rxrpc_discard_expired_client_conns(struct work_struct *work)
{
struct rxrpc_connection *conn;
+ struct rxrpc_net *rxnet =
+ container_of(to_delayed_work(work),
+ struct rxrpc_net, client_conn_reaper);
unsigned long expiry, conn_expires_at, now;
unsigned int nr_conns;
bool did_discard = false;
- _enter("%c", work ? 'w' : 'n');
+ _enter("");
- if (list_empty(&rxrpc_idle_client_conns)) {
+ if (list_empty(&rxnet->idle_client_conns)) {
_leave(" [empty]");
return;
}
/* Don't double up on the discarding */
- if (!spin_trylock(&rxrpc_client_conn_discard_mutex)) {
+ if (!spin_trylock(&rxnet->client_conn_discard_lock)) {
_leave(" [already]");
return;
}
@@ -995,19 +992,19 @@ static void rxrpc_discard_expired_client_conns(struct work_struct *work)
/* We keep an estimate of what the number of conns ought to be after
* we've discarded some so that we don't overdo the discarding.
*/
- nr_conns = rxrpc_nr_client_conns;
+ nr_conns = rxnet->nr_client_conns;
next:
- spin_lock(&rxrpc_client_conn_cache_lock);
+ spin_lock(&rxnet->client_conn_cache_lock);
- if (list_empty(&rxrpc_idle_client_conns))
+ if (list_empty(&rxnet->idle_client_conns))
goto out;
- conn = list_entry(rxrpc_idle_client_conns.next,
+ conn = list_entry(rxnet->idle_client_conns.next,
struct rxrpc_connection, cache_link);
ASSERT(test_bit(RXRPC_CONN_EXPOSED, &conn->flags));
- if (!rxrpc_kill_all_client_conns) {
+ if (!rxnet->kill_all_client_conns) {
/* If the number of connections is over the reap limit, we
* expedite discard by reducing the expiry timeout. We must,
* however, have at least a short grace period to be able to do
@@ -1030,7 +1027,7 @@ static void rxrpc_discard_expired_client_conns(struct work_struct *work)
conn->cache_state = RXRPC_CONN_CLIENT_INACTIVE;
list_del_init(&conn->cache_link);
- spin_unlock(&rxrpc_client_conn_cache_lock);
+ spin_unlock(&rxnet->client_conn_cache_lock);
/* When we cleared the EXPOSED flag, we took on responsibility for the
* reference that that had on the usage count. We deal with that here.
@@ -1050,14 +1047,14 @@ static void rxrpc_discard_expired_client_conns(struct work_struct *work)
* then things get messier.
*/
_debug("not yet");
- if (!rxrpc_kill_all_client_conns)
+ if (!rxnet->kill_all_client_conns)
queue_delayed_work(rxrpc_workqueue,
- &rxrpc_client_conn_reap,
+ &rxnet->client_conn_reaper,
conn_expires_at - now);
out:
- spin_unlock(&rxrpc_client_conn_cache_lock);
- spin_unlock(&rxrpc_client_conn_discard_mutex);
+ spin_unlock(&rxnet->client_conn_cache_lock);
+ spin_unlock(&rxnet->client_conn_discard_lock);
_leave("");
}
@@ -1065,17 +1062,17 @@ static void rxrpc_discard_expired_client_conns(struct work_struct *work)
* Preemptively destroy all the client connection records rather than waiting
* for them to time out
*/
-void __exit rxrpc_destroy_all_client_connections(void)
+void rxrpc_destroy_all_client_connections(struct rxrpc_net *rxnet)
{
_enter("");
- spin_lock(&rxrpc_client_conn_cache_lock);
- rxrpc_kill_all_client_conns = true;
- spin_unlock(&rxrpc_client_conn_cache_lock);
+ spin_lock(&rxnet->client_conn_cache_lock);
+ rxnet->kill_all_client_conns = true;
+ spin_unlock(&rxnet->client_conn_cache_lock);
- cancel_delayed_work(&rxrpc_client_conn_reap);
+ cancel_delayed_work(&rxnet->client_conn_reaper);
- if (!queue_delayed_work(rxrpc_workqueue, &rxrpc_client_conn_reap, 0))
+ if (!queue_delayed_work(rxrpc_workqueue, &rxnet->client_conn_reaper, 0))
_debug("destroy: queue failed");
_leave("");
diff --git a/net/rxrpc/conn_object.c b/net/rxrpc/conn_object.c
index b0ecb770fdce..ade4d3d0b2a7 100644
--- a/net/rxrpc/conn_object.c
+++ b/net/rxrpc/conn_object.c
@@ -22,13 +22,6 @@
*/
unsigned int rxrpc_connection_expiry = 10 * 60;
-static void rxrpc_connection_reaper(struct work_struct *work);
-
-LIST_HEAD(rxrpc_connections);
-LIST_HEAD(rxrpc_connection_proc_list);
-DEFINE_RWLOCK(rxrpc_connection_lock);
-static DECLARE_DELAYED_WORK(rxrpc_connection_reap, rxrpc_connection_reaper);
-
static void rxrpc_destroy_connection(struct rcu_head *);
/*
@@ -222,15 +215,17 @@ void rxrpc_disconnect_call(struct rxrpc_call *call)
*/
void rxrpc_kill_connection(struct rxrpc_connection *conn)
{
+ struct rxrpc_net *rxnet = conn->params.local->rxnet;
+
ASSERT(!rcu_access_pointer(conn->channels[0].call) &&
!rcu_access_pointer(conn->channels[1].call) &&
!rcu_access_pointer(conn->channels[2].call) &&
!rcu_access_pointer(conn->channels[3].call));
ASSERT(list_empty(&conn->cache_link));
- write_lock(&rxrpc_connection_lock);
+ write_lock(&rxnet->conn_lock);
list_del_init(&conn->proc_link);
- write_unlock(&rxrpc_connection_lock);
+ write_unlock(&rxnet->conn_lock);
/* Drain the Rx queue. Note that even though we've unpublished, an
* incoming packet could still be being added to our Rx queue, so we
@@ -309,14 +304,17 @@ rxrpc_get_connection_maybe(struct rxrpc_connection *conn)
*/
void rxrpc_put_service_conn(struct rxrpc_connection *conn)
{
+ struct rxrpc_net *rxnet;
const void *here = __builtin_return_address(0);
int n;
n = atomic_dec_return(&conn->usage);
trace_rxrpc_conn(conn, rxrpc_conn_put_service, n, here);
ASSERTCMP(n, >=, 0);
- if (n == 0)
- rxrpc_queue_delayed_work(&rxrpc_connection_reap, 0);
+ if (n == 0) {
+ rxnet = conn->params.local->rxnet;
+ rxrpc_queue_delayed_work(&rxnet->service_conn_reaper, 0);
+ }
}
/*
@@ -348,9 +346,12 @@ static void rxrpc_destroy_connection(struct rcu_head *rcu)
/*
* reap dead service connections
*/
-static void rxrpc_connection_reaper(struct work_struct *work)
+void rxrpc_service_connection_reaper(struct work_struct *work)
{
struct rxrpc_connection *conn, *_p;
+ struct rxrpc_net *rxnet =
+ container_of(to_delayed_work(work),
+ struct rxrpc_net, service_conn_reaper);
unsigned long reap_older_than, earliest, idle_timestamp, now;
LIST_HEAD(graveyard);
@@ -361,8 +362,8 @@ static void rxrpc_connection_reaper(struct work_struct *work)
reap_older_than = now - rxrpc_connection_expiry * HZ;
earliest = ULONG_MAX;
- write_lock(&rxrpc_connection_lock);
- list_for_each_entry_safe(conn, _p, &rxrpc_connections, link) {
+ write_lock(&rxnet->conn_lock);
+ list_for_each_entry_safe(conn, _p, &rxnet->service_conns, link) {
ASSERTCMP(atomic_read(&conn->usage), >, 0);
if (likely(atomic_read(&conn->usage) > 1))
continue;
@@ -393,12 +394,12 @@ static void rxrpc_connection_reaper(struct work_struct *work)
list_move_tail(&conn->link, &graveyard);
}
- write_unlock(&rxrpc_connection_lock);
+ write_unlock(&rxnet->conn_lock);
if (earliest != ULONG_MAX) {
_debug("reschedule reaper %ld", (long) earliest - now);
ASSERT(time_after(earliest, now));
- rxrpc_queue_delayed_work(&rxrpc_connection_reap,
+ rxrpc_queue_delayed_work(&rxnet->client_conn_reaper,
earliest - now);
}
@@ -418,36 +419,30 @@ static void rxrpc_connection_reaper(struct work_struct *work)
* preemptively destroy all the service connection records rather than
* waiting for them to time out
*/
-void __exit rxrpc_destroy_all_connections(void)
+void rxrpc_destroy_all_connections(struct rxrpc_net *rxnet)
{
struct rxrpc_connection *conn, *_p;
bool leak = false;
_enter("");
- rxrpc_destroy_all_client_connections();
+ rxrpc_destroy_all_client_connections(rxnet);
rxrpc_connection_expiry = 0;
- cancel_delayed_work(&rxrpc_connection_reap);
- rxrpc_queue_delayed_work(&rxrpc_connection_reap, 0);
+ cancel_delayed_work(&rxnet->client_conn_reaper);
+ rxrpc_queue_delayed_work(&rxnet->client_conn_reaper, 0);
flush_workqueue(rxrpc_workqueue);
- write_lock(&rxrpc_connection_lock);
- list_for_each_entry_safe(conn, _p, &rxrpc_connections, link) {
+ write_lock(&rxnet->conn_lock);
+ list_for_each_entry_safe(conn, _p, &rxnet->service_conns, link) {
pr_err("AF_RXRPC: Leaked conn %p {%d}\n",
conn, atomic_read(&conn->usage));
leak = true;
}
- write_unlock(&rxrpc_connection_lock);
+ write_unlock(&rxnet->conn_lock);
BUG_ON(leak);
- ASSERT(list_empty(&rxrpc_connection_proc_list));
-
- /* Make sure the local and peer records pinned by any dying connections
- * are released.
- */
- rcu_barrier();
- rxrpc_destroy_client_conn_ids();
+ ASSERT(list_empty(&rxnet->conn_proc_list));
_leave("");
}
diff --git a/net/rxrpc/conn_service.c b/net/rxrpc/conn_service.c
index eef551f40dc2..edfc633f7d5e 100644
--- a/net/rxrpc/conn_service.c
+++ b/net/rxrpc/conn_service.c
@@ -121,7 +121,8 @@ static void rxrpc_publish_service_conn(struct rxrpc_peer *peer,
* Preallocate a service connection. The connection is placed on the proc and
* reap lists so that we don't have to get the lock from BH context.
*/
-struct rxrpc_connection *rxrpc_prealloc_service_connection(gfp_t gfp)
+struct rxrpc_connection *rxrpc_prealloc_service_connection(struct rxrpc_net *rxnet,
+ gfp_t gfp)
{
struct rxrpc_connection *conn = rxrpc_alloc_connection(gfp);
@@ -132,10 +133,10 @@ struct rxrpc_connection *rxrpc_prealloc_service_connection(gfp_t gfp)
conn->state = RXRPC_CONN_SERVICE_PREALLOC;
atomic_set(&conn->usage, 2);
- write_lock(&rxrpc_connection_lock);
- list_add_tail(&conn->link, &rxrpc_connections);
- list_add_tail(&conn->proc_link, &rxrpc_connection_proc_list);
- write_unlock(&rxrpc_connection_lock);
+ write_lock(&rxnet->conn_lock);
+ list_add_tail(&conn->link, &rxnet->service_conns);
+ list_add_tail(&conn->proc_link, &rxnet->conn_proc_list);
+ write_unlock(&rxnet->conn_lock);
trace_rxrpc_conn(conn, rxrpc_conn_new_service,
atomic_read(&conn->usage),
diff --git a/net/rxrpc/local_object.c b/net/rxrpc/local_object.c
index ff4864d550b8..17d79fd73ade 100644
--- a/net/rxrpc/local_object.c
+++ b/net/rxrpc/local_object.c
@@ -25,9 +25,6 @@
static void rxrpc_local_processor(struct work_struct *);
static void rxrpc_local_rcu(struct rcu_head *);
-static DEFINE_MUTEX(rxrpc_local_mutex);
-static LIST_HEAD(rxrpc_local_endpoints);
-
/*
* Compare a local to an address. Return -ve, 0 or +ve to indicate less than,
* same or greater than.
@@ -77,13 +74,15 @@ static long rxrpc_local_cmp_key(const struct rxrpc_local *local,
/*
* Allocate a new local endpoint.
*/
-static struct rxrpc_local *rxrpc_alloc_local(const struct sockaddr_rxrpc *srx)
+static struct rxrpc_local *rxrpc_alloc_local(struct rxrpc_net *rxnet,
+ const struct sockaddr_rxrpc *srx)
{
struct rxrpc_local *local;
local = kzalloc(sizeof(struct rxrpc_local), GFP_KERNEL);
if (local) {
atomic_set(&local->usage, 1);
+ local->rxnet = rxnet;
INIT_LIST_HEAD(&local->link);
INIT_WORK(&local->processor, rxrpc_local_processor);
init_rwsem(&local->defrag_sem);
@@ -105,7 +104,7 @@ static struct rxrpc_local *rxrpc_alloc_local(const struct sockaddr_rxrpc *srx)
* create the local socket
* - must be called with rxrpc_local_mutex locked
*/
-static int rxrpc_open_socket(struct rxrpc_local *local)
+static int rxrpc_open_socket(struct rxrpc_local *local, struct net *net)
{
struct sock *sock;
int ret, opt;
@@ -114,7 +113,7 @@ static int rxrpc_open_socket(struct rxrpc_local *local)
local, local->srx.transport_type, local->srx.transport.family);
/* create a socket to represent the local endpoint */
- ret = sock_create_kern(&init_net, local->srx.transport.family,
+ ret = sock_create_kern(net, local->srx.transport.family,
local->srx.transport_type, 0, &local->socket);
if (ret < 0) {
_leave(" = %d [socket]", ret);
@@ -172,9 +171,11 @@ static int rxrpc_open_socket(struct rxrpc_local *local)
/*
* Look up or create a new local endpoint using the specified local address.
*/
-struct rxrpc_local *rxrpc_lookup_local(const struct sockaddr_rxrpc *srx)
+struct rxrpc_local *rxrpc_lookup_local(struct net *net,
+ const struct sockaddr_rxrpc *srx)
{
struct rxrpc_local *local;
+ struct rxrpc_net *rxnet = rxrpc_net(net);
struct list_head *cursor;
const char *age;
long diff;
@@ -183,10 +184,10 @@ struct rxrpc_local *rxrpc_lookup_local(const struct sockaddr_rxrpc *srx)
_enter("{%d,%d,%pISp}",
srx->transport_type, srx->transport.family, &srx->transport);
- mutex_lock(&rxrpc_local_mutex);
+ mutex_lock(&rxnet->local_mutex);
- for (cursor = rxrpc_local_endpoints.next;
- cursor != &rxrpc_local_endpoints;
+ for (cursor = rxnet->local_endpoints.next;
+ cursor != &rxnet->local_endpoints;
cursor = cursor->next) {
local = list_entry(cursor, struct rxrpc_local, link);
@@ -220,11 +221,11 @@ struct rxrpc_local *rxrpc_lookup_local(const struct sockaddr_rxrpc *srx)
goto found;
}
- local = rxrpc_alloc_local(srx);
+ local = rxrpc_alloc_local(rxnet, srx);
if (!local)
goto nomem;
- ret = rxrpc_open_socket(local);
+ ret = rxrpc_open_socket(local, net);
if (ret < 0)
goto sock_error;
@@ -232,7 +233,7 @@ struct rxrpc_local *rxrpc_lookup_local(const struct sockaddr_rxrpc *srx)
age = "new";
found:
- mutex_unlock(&rxrpc_local_mutex);
+ mutex_unlock(&rxnet->local_mutex);
_net("LOCAL %s %d {%pISp}",
age, local->debug_id, &local->srx.transport);
@@ -243,13 +244,13 @@ struct rxrpc_local *rxrpc_lookup_local(const struct sockaddr_rxrpc *srx)
nomem:
ret = -ENOMEM;
sock_error:
- mutex_unlock(&rxrpc_local_mutex);
+ mutex_unlock(&rxnet->local_mutex);
kfree(local);
_leave(" = %d", ret);
return ERR_PTR(ret);
addr_in_use:
- mutex_unlock(&rxrpc_local_mutex);
+ mutex_unlock(&rxnet->local_mutex);
_leave(" = -EADDRINUSE");
return ERR_PTR(-EADDRINUSE);
}
@@ -273,6 +274,7 @@ void __rxrpc_put_local(struct rxrpc_local *local)
static void rxrpc_local_destroyer(struct rxrpc_local *local)
{
struct socket *socket = local->socket;
+ struct rxrpc_net *rxnet = local->rxnet;
_enter("%d", local->debug_id);
@@ -286,9 +288,9 @@ static void rxrpc_local_destroyer(struct rxrpc_local *local)
}
local->dead = true;
- mutex_lock(&rxrpc_local_mutex);
+ mutex_lock(&rxnet->local_mutex);
list_del_init(&local->link);
- mutex_unlock(&rxrpc_local_mutex);
+ mutex_unlock(&rxnet->local_mutex);
ASSERT(RB_EMPTY_ROOT(&local->client_conns));
ASSERT(!local->service);
@@ -357,7 +359,7 @@ static void rxrpc_local_rcu(struct rcu_head *rcu)
/*
* Verify the local endpoint list is empty by this point.
*/
-void __exit rxrpc_destroy_all_locals(void)
+void rxrpc_destroy_all_locals(struct rxrpc_net *rxnet)
{
struct rxrpc_local *local;
@@ -365,15 +367,13 @@ void __exit rxrpc_destroy_all_locals(void)
flush_workqueue(rxrpc_workqueue);
- if (!list_empty(&rxrpc_local_endpoints)) {
- mutex_lock(&rxrpc_local_mutex);
- list_for_each_entry(local, &rxrpc_local_endpoints, link) {
+ if (!list_empty(&rxnet->local_endpoints)) {
+ mutex_lock(&rxnet->local_mutex);
+ list_for_each_entry(local, &rxnet->local_endpoints, link) {
pr_err("AF_RXRPC: Leaked local %p {%d}\n",
local, atomic_read(&local->usage));
}
- mutex_unlock(&rxrpc_local_mutex);
+ mutex_unlock(&rxnet->local_mutex);
BUG();
}
-
- rcu_barrier();
}
diff --git a/net/rxrpc/net_ns.c b/net/rxrpc/net_ns.c
new file mode 100644
index 000000000000..26449a6bb076
--- /dev/null
+++ b/net/rxrpc/net_ns.c
@@ -0,0 +1,85 @@
+/* rxrpc network namespace handling.
+ *
+ * Copyright (C) 2017 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#include <linux/proc_fs.h>
+#include "ar-internal.h"
+
+unsigned int rxrpc_net_id;
+
+/*
+ * Initialise a per-network namespace record.
+ */
+static __net_init int rxrpc_init_net(struct net *net)
+{
+ struct rxrpc_net *rxnet = rxrpc_net(net);
+ int ret;
+
+ get_random_bytes(&rxnet->epoch, sizeof(rxnet->epoch));
+ rxnet->epoch |= RXRPC_RANDOM_EPOCH;
+
+ INIT_LIST_HEAD(&rxnet->calls);
+ rwlock_init(&rxnet->call_lock);
+
+ INIT_LIST_HEAD(&rxnet->conn_proc_list);
+ INIT_LIST_HEAD(&rxnet->service_conns);
+ rwlock_init(&rxnet->conn_lock);
+ INIT_DELAYED_WORK(&rxnet->service_conn_reaper,
+ rxrpc_service_connection_reaper);
+
+ rxnet->nr_client_conns = 0;
+ rxnet->nr_active_client_conns = 0;
+ rxnet->kill_all_client_conns = false;
+ spin_lock_init(&rxnet->client_conn_cache_lock);
+ spin_lock_init(&rxnet->client_conn_discard_lock);
+ INIT_LIST_HEAD(&rxnet->waiting_client_conns);
+ INIT_LIST_HEAD(&rxnet->active_client_conns);
+ INIT_LIST_HEAD(&rxnet->idle_client_conns);
+ INIT_DELAYED_WORK(&rxnet->client_conn_reaper,
+ rxrpc_discard_expired_client_conns);
+
+ INIT_LIST_HEAD(&rxnet->local_endpoints);
+ mutex_init(&rxnet->local_mutex);
+ hash_init(rxnet->peer_hash);
+ spin_lock_init(&rxnet->peer_hash_lock);
+
+ ret = -ENOMEM;
+ rxnet->proc_net = proc_net_mkdir(net, "rxrpc", net->proc_net);
+ if (!rxnet->proc_net)
+ goto err_proc;
+
+ proc_create("calls", 0444, rxnet->proc_net, &rxrpc_call_seq_fops);
+ proc_create("conns", 0444, rxnet->proc_net, &rxrpc_connection_seq_fops);
+ return 0;
+
+ proc_remove(rxnet->proc_net);
+err_proc:
+ return ret;
+}
+
+/*
+ * Clean up a per-network namespace record.
+ */
+static __net_exit void rxrpc_exit_net(struct net *net)
+{
+ struct rxrpc_net *rxnet = rxrpc_net(net);
+
+ rxrpc_destroy_all_calls(rxnet);
+ rxrpc_destroy_all_connections(rxnet);
+ rxrpc_destroy_all_locals(rxnet);
+ proc_remove(rxnet->proc_net);
+}
+
+struct pernet_operations rxrpc_net_ops = {
+ .init = rxrpc_init_net,
+ .exit = rxrpc_exit_net,
+ .id = &rxrpc_net_id,
+ .size = sizeof(struct rxrpc_net),
+};
diff --git a/net/rxrpc/peer_object.c b/net/rxrpc/peer_object.c
index 862eea6b266c..cfed3b27adf0 100644
--- a/net/rxrpc/peer_object.c
+++ b/net/rxrpc/peer_object.c
@@ -26,9 +26,6 @@
#include <net/ip6_route.h>
#include "ar-internal.h"
-static DEFINE_HASHTABLE(rxrpc_peer_hash, 10);
-static DEFINE_SPINLOCK(rxrpc_peer_hash_lock);
-
/*
* Hash a peer key.
*/
@@ -124,8 +121,9 @@ static struct rxrpc_peer *__rxrpc_lookup_peer_rcu(
unsigned long hash_key)
{
struct rxrpc_peer *peer;
+ struct rxrpc_net *rxnet = local->rxnet;
- hash_for_each_possible_rcu(rxrpc_peer_hash, peer, hash_link, hash_key) {
+ hash_for_each_possible_rcu(rxnet->peer_hash, peer, hash_link, hash_key) {
if (rxrpc_peer_cmp_key(peer, local, srx, hash_key) == 0) {
if (atomic_read(&peer->usage) == 0)
return NULL;
@@ -301,13 +299,14 @@ struct rxrpc_peer *rxrpc_lookup_incoming_peer(struct rxrpc_local *local,
struct rxrpc_peer *prealloc)
{
struct rxrpc_peer *peer;
+ struct rxrpc_net *rxnet = local->rxnet;
unsigned long hash_key;
hash_key = rxrpc_peer_hash_key(local, &prealloc->srx);
prealloc->local = local;
rxrpc_init_peer(prealloc, hash_key);
- spin_lock(&rxrpc_peer_hash_lock);
+ spin_lock(&rxnet->peer_hash_lock);
/* Need to check that we aren't racing with someone else */
peer = __rxrpc_lookup_peer_rcu(local, &prealloc->srx, hash_key);
@@ -315,10 +314,10 @@ struct rxrpc_peer *rxrpc_lookup_incoming_peer(struct rxrpc_local *local,
peer = NULL;
if (!peer) {
peer = prealloc;
- hash_add_rcu(rxrpc_peer_hash, &peer->hash_link, hash_key);
+ hash_add_rcu(rxnet->peer_hash, &peer->hash_link, hash_key);
}
- spin_unlock(&rxrpc_peer_hash_lock);
+ spin_unlock(&rxnet->peer_hash_lock);
return peer;
}
@@ -329,6 +328,7 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_local *local,
struct sockaddr_rxrpc *srx, gfp_t gfp)
{
struct rxrpc_peer *peer, *candidate;
+ struct rxrpc_net *rxnet = local->rxnet;
unsigned long hash_key = rxrpc_peer_hash_key(local, srx);
_enter("{%pISp}", &srx->transport);
@@ -350,17 +350,17 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_local *local,
return NULL;
}
- spin_lock_bh(&rxrpc_peer_hash_lock);
+ spin_lock_bh(&rxnet->peer_hash_lock);
/* Need to check that we aren't racing with someone else */
peer = __rxrpc_lookup_peer_rcu(local, srx, hash_key);
if (peer && !rxrpc_get_peer_maybe(peer))
peer = NULL;
if (!peer)
- hash_add_rcu(rxrpc_peer_hash,
+ hash_add_rcu(rxnet->peer_hash,
&candidate->hash_link, hash_key);
- spin_unlock_bh(&rxrpc_peer_hash_lock);
+ spin_unlock_bh(&rxnet->peer_hash_lock);
if (peer)
kfree(candidate);
@@ -379,11 +379,13 @@ struct rxrpc_peer *rxrpc_lookup_peer(struct rxrpc_local *local,
*/
void __rxrpc_put_peer(struct rxrpc_peer *peer)
{
+ struct rxrpc_net *rxnet = peer->local->rxnet;
+
ASSERT(hlist_empty(&peer->error_targets));
- spin_lock_bh(&rxrpc_peer_hash_lock);
+ spin_lock_bh(&rxnet->peer_hash_lock);
hash_del_rcu(&peer->hash_link);
- spin_unlock_bh(&rxrpc_peer_hash_lock);
+ spin_unlock_bh(&rxnet->peer_hash_lock);
kfree_rcu(peer, rcu);
}
diff --git a/net/rxrpc/proc.c b/net/rxrpc/proc.c
index b9bcfbfb095c..e92d8405b15a 100644
--- a/net/rxrpc/proc.c
+++ b/net/rxrpc/proc.c
@@ -30,19 +30,25 @@ static const char *const rxrpc_conn_states[RXRPC_CONN__NR_STATES] = {
*/
static void *rxrpc_call_seq_start(struct seq_file *seq, loff_t *_pos)
{
+ struct rxrpc_net *rxnet = rxrpc_net(seq_file_net(seq));
+
rcu_read_lock();
- read_lock(&rxrpc_call_lock);
- return seq_list_start_head(&rxrpc_calls, *_pos);
+ read_lock(&rxnet->call_lock);
+ return seq_list_start_head(&rxnet->calls, *_pos);
}
static void *rxrpc_call_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
- return seq_list_next(v, &rxrpc_calls, pos);
+ struct rxrpc_net *rxnet = rxrpc_net(seq_file_net(seq));
+
+ return seq_list_next(v, &rxnet->calls, pos);
}
static void rxrpc_call_seq_stop(struct seq_file *seq, void *v)
{
- read_unlock(&rxrpc_call_lock);
+ struct rxrpc_net *rxnet = rxrpc_net(seq_file_net(seq));
+
+ read_unlock(&rxnet->call_lock);
rcu_read_unlock();
}
@@ -52,10 +58,11 @@ static int rxrpc_call_seq_show(struct seq_file *seq, void *v)
struct rxrpc_sock *rx;
struct rxrpc_peer *peer;
struct rxrpc_call *call;
+ struct rxrpc_net *rxnet = rxrpc_net(seq_file_net(seq));
rxrpc_seq_t tx_hard_ack, rx_hard_ack;
char lbuff[50], rbuff[50];
- if (v == &rxrpc_calls) {
+ if (v == &rxnet->calls) {
seq_puts(seq,
"Proto Local "
" Remote "
@@ -113,7 +120,8 @@ static const struct seq_operations rxrpc_call_seq_ops = {
static int rxrpc_call_seq_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &rxrpc_call_seq_ops);
+ return seq_open_net(inode, file, &rxrpc_call_seq_ops,
+ sizeof(struct seq_net_private));
}
const struct file_operations rxrpc_call_seq_fops = {
@@ -129,27 +137,34 @@ const struct file_operations rxrpc_call_seq_fops = {
*/
static void *rxrpc_connection_seq_start(struct seq_file *seq, loff_t *_pos)
{
- read_lock(&rxrpc_connection_lock);
- return seq_list_start_head(&rxrpc_connection_proc_list, *_pos);
+ struct rxrpc_net *rxnet = rxrpc_net(seq_file_net(seq));
+
+ read_lock(&rxnet->conn_lock);
+ return seq_list_start_head(&rxnet->conn_proc_list, *_pos);
}
static void *rxrpc_connection_seq_next(struct seq_file *seq, void *v,
loff_t *pos)
{
- return seq_list_next(v, &rxrpc_connection_proc_list, pos);
+ struct rxrpc_net *rxnet = rxrpc_net(seq_file_net(seq));
+
+ return seq_list_next(v, &rxnet->conn_proc_list, pos);
}
static void rxrpc_connection_seq_stop(struct seq_file *seq, void *v)
{
- read_unlock(&rxrpc_connection_lock);
+ struct rxrpc_net *rxnet = rxrpc_net(seq_file_net(seq));
+
+ read_unlock(&rxnet->conn_lock);
}
static int rxrpc_connection_seq_show(struct seq_file *seq, void *v)
{
struct rxrpc_connection *conn;
+ struct rxrpc_net *rxnet = rxrpc_net(seq_file_net(seq));
char lbuff[50], rbuff[50];
- if (v == &rxrpc_connection_proc_list) {
+ if (v == &rxnet->conn_proc_list) {
seq_puts(seq,
"Proto Local "
" Remote "
@@ -197,7 +212,8 @@ static const struct seq_operations rxrpc_connection_seq_ops = {
static int rxrpc_connection_seq_open(struct inode *inode, struct file *file)
{
- return seq_open(file, &rxrpc_connection_seq_ops);
+ return seq_open_net(inode, file, &rxrpc_connection_seq_ops,
+ sizeof(struct seq_net_private));
}
const struct file_operations rxrpc_connection_seq_fops = {
^ permalink raw reply related
* Re: [PATCH] rhashtable: Fix missing elements when inserting.
From: Taehee Yoo @ 2017-05-24 16:13 UTC (permalink / raw)
To: Herbert Xu; +Cc: davem, netdev
In-Reply-To: <20170524043617.GA22594@gondor.apana.org.au>
2017-05-24 13:36 GMT+09:00 Herbert Xu <herbert@gondor.apana.org.au>:
> Taehee Yoo <ap420073@gmail.com> wrote:
>> rhltable_insert_key() inserts a node into list of element,
>> if node's key is duplicated, so that it becomes the chain of
>> element(as known as rhead). Also bucket table points that element directly.
>> If a inserted node's element chain is located at third,
>> rhltable misses first and second element chain.
>> This issue is causion of to failture the rhltable_remove().
>>
>> After this patch, rhltable_insert_key() inserts a node into second of
>> element's list, so that rhlist do not misses elements.
>>
>> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
>
> I'm sorry but I don't understand your description of the problem.
> The new duplicate object is always inserted at the head of the
> list and therefore replaces the previous list head in the hash
> bucket chain.
>
> Do you have some code that reproduces the problem?
>
I tested this problem using netfilter, therefore a attached diff file
modifies netfilter source code.
That diff prints fail messages if return value of rhltable_remove() is -ENOENT.
I used below commands
sysctl -w net.netfilter.nf_conntrack_udp_timeout=100000
sysctl -w net.netfilter.nf_conntrack_max=1000000
iptables -t nat -A POSTROUTING -j MASQUERADE
hping3 < ip address > -2 -p 1 --flood
hping3 < ip address > -2 -p 2 --flood
conntrack -D > /dev/null
we can see "nf_nat_cleanup_conntrack, not found"
>> diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
>> index 7d56a7e..d3c24b9 100644
>> --- a/include/linux/rhashtable.h
>> +++ b/include/linux/rhashtable.h
>> @@ -762,11 +762,9 @@ static inline void *__rhashtable_insert_fast(
>> list = container_of(obj, struct rhlist_head, rhead);
>> plist = container_of(head, struct rhlist_head, rhead);
>>
>> - RCU_INIT_POINTER(list->next, plist);
>> - head = rht_dereference_bucket(head->next, tbl, hash);
>> - RCU_INIT_POINTER(list->rhead.next, head);
>> - rcu_assign_pointer(*pprev, obj);
>> -
>> + RCU_INIT_POINTER(list->next, rht_dereference_bucket(plist->next,
>> + tbl, hash));
>> + RCU_INIT_POINTER(plist->next, list);
>
> Your second RCU_INIT_POINTER needs to be rcu_assign_pointer.
>
Thank you, I won't forget this.
> Your approach of retaining the first duplicate object as the head
> of the list should work too but I don't really see any point in
> changing this.
>
> Cheers,
> --
> Email: Herbert Xu <herbert@gondor.apana.org.au>
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] rhashtable: Fix missing elements when inserting.
From: Taehee Yoo @ 2017-05-24 16:17 UTC (permalink / raw)
To: Herbert Xu; +Cc: davem, netdev
In-Reply-To: <CAMArcTVrGoopQseeOLFQoHheve-qMV2iE-KCPDT8UG5o97sa7g@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2928 bytes --]
2017-05-25 1:13 GMT+09:00 Taehee Yoo <ap420073@gmail.com>:
> 2017-05-24 13:36 GMT+09:00 Herbert Xu <herbert@gondor.apana.org.au>:
>> Taehee Yoo <ap420073@gmail.com> wrote:
>>> rhltable_insert_key() inserts a node into list of element,
>>> if node's key is duplicated, so that it becomes the chain of
>>> element(as known as rhead). Also bucket table points that element directly.
>>> If a inserted node's element chain is located at third,
>>> rhltable misses first and second element chain.
>>> This issue is causion of to failture the rhltable_remove().
>>>
>>> After this patch, rhltable_insert_key() inserts a node into second of
>>> element's list, so that rhlist do not misses elements.
>>>
>>> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
>>
>> I'm sorry but I don't understand your description of the problem.
>> The new duplicate object is always inserted at the head of the
>> list and therefore replaces the previous list head in the hash
>> bucket chain.
>>
>> Do you have some code that reproduces the problem?
>>
> I tested this problem using netfilter, therefore a attached diff file
> modifies netfilter source code.
> That diff prints fail messages if return value of rhltable_remove() is -ENOENT.
>
> I used below commands
>
> sysctl -w net.netfilter.nf_conntrack_udp_timeout=100000
> sysctl -w net.netfilter.nf_conntrack_max=1000000
> iptables -t nat -A POSTROUTING -j MASQUERADE
>
> hping3 < ip address > -2 -p 1 --flood
> hping3 < ip address > -2 -p 2 --flood
> conntrack -D > /dev/null
> we can see "nf_nat_cleanup_conntrack, not found"
>
>>> diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
>>> index 7d56a7e..d3c24b9 100644
>>> --- a/include/linux/rhashtable.h
>>> +++ b/include/linux/rhashtable.h
>>> @@ -762,11 +762,9 @@ static inline void *__rhashtable_insert_fast(
>>> list = container_of(obj, struct rhlist_head, rhead);
>>> plist = container_of(head, struct rhlist_head, rhead);
>>>
>>> - RCU_INIT_POINTER(list->next, plist);
>>> - head = rht_dereference_bucket(head->next, tbl, hash);
>>> - RCU_INIT_POINTER(list->rhead.next, head);
>>> - rcu_assign_pointer(*pprev, obj);
>>> -
>>> + RCU_INIT_POINTER(list->next, rht_dereference_bucket(plist->next,
>>> + tbl, hash));
>>> + RCU_INIT_POINTER(plist->next, list);
>>
>> Your second RCU_INIT_POINTER needs to be rcu_assign_pointer.
>>
> Thank you, I won't forget this.
>
>> Your approach of retaining the first duplicate object as the head
>> of the list should work too but I don't really see any point in
>> changing this.
>>
>> Cheers,
>> --
>> Email: Herbert Xu <herbert@gondor.apana.org.au>
>> Home Page: http://gondor.apana.org.au/~herbert/
>> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
I'm so sorry, I did not attach diff file.
[-- Attachment #2: rhashtable_test.diff --]
[-- Type: text/plain, Size: 733 bytes --]
diff --git a/net/netfilter/nf_nat_core.c b/net/netfilter/nf_nat_core.c
index b48d6b5..a1d9a59 100644
--- a/net/netfilter/nf_nat_core.c
+++ b/net/netfilter/nf_nat_core.c
@@ -701,9 +701,14 @@ EXPORT_SYMBOL_GPL(nf_nat_l3proto_unregister);
/* No one using conntrack by the time this called. */
static void nf_nat_cleanup_conntrack(struct nf_conn *ct)
{
- if (ct->status & IPS_SRC_NAT_DONE)
- rhltable_remove(&nf_nat_bysource_table, &ct->nat_bysource,
+ int err;
+
+ if (ct->status & IPS_SRC_NAT_DONE) {
+ err = rhltable_remove(&nf_nat_bysource_table, &ct->nat_bysource,
nf_nat_bysource_params);
+ if (err == -ENOENT)
+ printk("%s, not found\n", __func__);
+ }
}
static struct nf_ct_ext_type nat_extend __read_mostly = {
^ 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