* Re: [PATCH net-next v2 02/13] net: phy: sfp: handle non-wired SFP connectors
From: Andrew Lunn @ 2018-05-04 17:17 UTC (permalink / raw)
To: Florian Fainelli
Cc: Antoine Tenart, davem, kishon, linux, gregory.clement, jason,
sebastian.hesselbarth, netdev, linux-kernel, thomas.petazzoni,
maxime.chevallier, miquel.raynal, nadavh, stefanc, ymarkman, mw,
linux-arm-kernel
In-Reply-To: <e338472c-1f26-e49b-9a2d-7942e6ed4288@gmail.com>
On Fri, May 04, 2018 at 10:04:48AM -0700, Florian Fainelli wrote:
> On 05/04/2018 06:56 AM, Antoine Tenart wrote:
> > SFP connectors can be solder on a board without having any of their pins
> > (LOS, i2c...) wired. In such cases the SFP link state cannot be guessed,
> > and the overall link status reporting is left to other layers.
> >
> > In order to achieve this, a new SFP_DEV status is added, named UNKNOWN.
> > This mode is set when it is not possible for the SFP code to get the
> > link status and as a result the link status is reported to be always UP
> > from the SFP point of view.
>
> Why represent the SFP in Device Tree then? Why not just declare this is
> a fixed link which would avoid having to introduce this "unknown" state.
Hi Antoine
I agree with Florian here.
It LOS was missing, but i2c worked, i could see some value in using
SFP, or order to be able to read the EEPROM. But if everything is
missing, fixed-link seems a better fit.
Andrew
^ permalink raw reply
* [PATCH 1/2] net: nixge: Fix error path for obtaining mac address
From: Moritz Fischer @ 2018-05-04 17:18 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, davem, alex.williams, Moritz Fischer
Fix issue where nixge_get_nvmem_address() returns a non-NULL
return value on a failed nvmem_cell_get() that causes an invalid
access when error value encoded in pointer is dereferenced.
Furthermore ensure that buffer allocated by nvmem_cell_read()
actually gets kfreed() if the function succeeds.
Fixes commit 492caffa8a1a ("net: ethernet: nixge: Add support for
National Instruments XGE netdev")
Reported-by: Alex Williams <alex.williams@ni.com>
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
drivers/net/ethernet/ni/nixge.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index 27364b7572fc..c41fea9253e3 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -1170,7 +1170,7 @@ static void *nixge_get_nvmem_address(struct device *dev)
cell = nvmem_cell_get(dev, "address");
if (IS_ERR(cell))
- return cell;
+ return NULL;
mac = nvmem_cell_read(cell, &cell_size);
nvmem_cell_put(cell);
@@ -1202,10 +1202,12 @@ static int nixge_probe(struct platform_device *pdev)
ndev->max_mtu = NIXGE_JUMBO_MTU;
mac_addr = nixge_get_nvmem_address(&pdev->dev);
- if (mac_addr && is_valid_ether_addr(mac_addr))
+ if (mac_addr && is_valid_ether_addr(mac_addr)) {
ether_addr_copy(ndev->dev_addr, mac_addr);
- else
+ kfree(mac_addr);
+ } else {
eth_hw_addr_random(ndev);
+ }
priv = netdev_priv(ndev);
priv->ndev = ndev;
--
2.17.0
^ permalink raw reply related
* [PATCH 2/2] net: nixge: Address compiler warnings about signedness
From: Moritz Fischer @ 2018-05-04 17:18 UTC (permalink / raw)
To: linux-kernel; +Cc: netdev, davem, alex.williams, Moritz Fischer
In-Reply-To: <20180504171834.9365-1-mdf@kernel.org>
Fixes the following warnings:
warning: pointer targets in passing argument 1 of
‘is_valid_ether_addr’ differ in signedness [-Wpointer-sign]
if (mac_addr && is_valid_ether_addr(mac_addr)) {
^~~~~~~~
expected ‘const u8 * {aka const unsigned char *}’ but argument
is of type ‘const char *’
static inline bool is_valid_ether_addr(const u8 *addr)
^~~~~~~~~~~~~~~~~~~
warning: pointer targets in passing argument 2 of
‘ether_addr_copy’ differ in signedness [-Wpointer-sign]
ether_addr_copy(ndev->dev_addr, mac_addr);
^~~~~~~~
expected ‘const u8 * {aka const unsigned char *}’ but argument
is of type ‘const char *’
static inline void ether_addr_copy(u8 *dst, const u8 *src)
Signed-off-by: Moritz Fischer <mdf@kernel.org>
---
drivers/net/ethernet/ni/nixge.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c
index c41fea9253e3..b092894dd128 100644
--- a/drivers/net/ethernet/ni/nixge.c
+++ b/drivers/net/ethernet/ni/nixge.c
@@ -1183,7 +1183,7 @@ static int nixge_probe(struct platform_device *pdev)
struct nixge_priv *priv;
struct net_device *ndev;
struct resource *dmares;
- const char *mac_addr;
+ const u8 *mac_addr;
int err;
ndev = alloc_etherdev(sizeof(*priv));
--
2.17.0
^ permalink raw reply related
* Re: [PATCH net-next v2 01/13] net: phy: sfp: make the i2c-bus property really optional
From: Antoine Tenart @ 2018-05-04 17:19 UTC (permalink / raw)
To: Florian Fainelli
Cc: Antoine Tenart, davem, kishon, linux, gregory.clement, andrew,
jason, sebastian.hesselbarth, netdev, linux-kernel,
thomas.petazzoni, maxime.chevallier, miquel.raynal, nadavh,
stefanc, ymarkman, mw, linux-arm-kernel
In-Reply-To: <fd91699d-c290-8821-b4a6-0789071cfba1@gmail.com>
Hi Florian,
On Fri, May 04, 2018 at 10:03:16AM -0700, Florian Fainelli wrote:
> On 05/04/2018 06:56 AM, Antoine Tenart wrote:
> >
> > static int sfp_read(struct sfp *sfp, bool a2, u8 addr, void *buf, size_t len)
> > {
> > + if (!sfp->read)
> > + return -EOPNOTSUPP;
>
> -ENODEV would be closer to the intended meaning IMHO, those this could
> be argue that this is yet another color to paint the bikeshed with.
I thought about -ENODEV as well, but ended up choosing -EOPNOTSUPP for
some reason. But I'm really fine with both solutions, it really depends
on if we want to return a callback isn't available from a s/w point of
view (-EOPNOTSUPP) or a h/w point of view (-ENODEV).
> > ret = sfp_read(sfp, false, 0, &id, sizeof(id));
> > + if (ret == -EOPNOTSUPP)
> > + return ret;
>
> Can you find a way such that only sfp_sm_mod_probe() needs to check
> whether the sfp read/write operations returned failure and then we just
> make sure the SFP state machine does not make any more progress? Having
> to check the sfp_read()/sfp_write() operations all over the place sounds
> error prone and won't scale in the future.
I tried doing this in this way (only having logic in the probe
function), but that wasn't as simple as this solution and it seemed
quite invasive as these read/write calls can be called from a few
functions but many code paths (as it's a state machine). So I choose the
easiest solution to maintain in the long run, as each future state
machine update could impact this.
Thanks!
Antoine
--
Antoine Ténart, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH] net: ethernet: sun: niu set correct packet size in skb
From: David Miller @ 2018-05-04 17:22 UTC (permalink / raw)
To: rob; +Cc: netdev
In-Reply-To: <20180504.130950.1760681294731661849.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Fri, 04 May 2018 13:09:50 -0400 (EDT)
> From: Rob Taglang <rob@taglang.io>
> Date: Fri, 04 May 2018 13:07:54 -0400
>
>>> Still corrupted. Your email client is chopping up long lines.
>>> Please, send a test patch to yourself and make sure you can apply the
>>> patch that arrives in that test email.
>>> Thank you.
>>
>> Hi David,
>>
>> I did go through the process of sending myself a test email before
>> submitting.
>>
>> I can copy-paste the patch from my message on the archive:
>> https://www.spinics.net/lists/netdev/msg500099.html
>> and apply it successfully, so I'm not sure what you need me to do
>> differently.
>>
>> Any help is appreciated.
>
> Weird, let me sort this out.
I ended up fixing it up by hand. I have no idea why the copy that showed
up on spinics looks completely different to what I received directly in
my inbox and what showed up on patchwork.ozlabs.org
Anyways, applied and queued up for -stable, thank you.
^ permalink raw reply
* Re: [PATCH net-next v2 02/13] net: phy: sfp: handle non-wired SFP connectors
From: Antoine Tenart @ 2018-05-04 17:23 UTC (permalink / raw)
To: Florian Fainelli
Cc: Antoine Tenart, davem, kishon, linux, gregory.clement, andrew,
jason, sebastian.hesselbarth, netdev, linux-kernel,
thomas.petazzoni, maxime.chevallier, miquel.raynal, nadavh,
stefanc, ymarkman, mw, linux-arm-kernel
In-Reply-To: <e338472c-1f26-e49b-9a2d-7942e6ed4288@gmail.com>
Hi Florian,
On Fri, May 04, 2018 at 10:04:48AM -0700, Florian Fainelli wrote:
> On 05/04/2018 06:56 AM, Antoine Tenart wrote:
> > SFP connectors can be solder on a board without having any of their pins
> > (LOS, i2c...) wired. In such cases the SFP link state cannot be guessed,
> > and the overall link status reporting is left to other layers.
> >
> > In order to achieve this, a new SFP_DEV status is added, named UNKNOWN.
> > This mode is set when it is not possible for the SFP code to get the
> > link status and as a result the link status is reported to be always UP
> > from the SFP point of view.
>
> Why represent the SFP in Device Tree then? Why not just declare this is
> a fixed link which would avoid having to introduce this "unknown" state.
The other solution would have been to represent this as a fixed-link.
But such a representation would report the link as being up all the
time, which is something we wanted to avoid as the GoP in PPv2 can
report some link status. This is achieved using SFP+phylink+PPv2.
And representing the SFP cage in the device tree, although it's a
"dummy" one, helps describing the hardware.
Thanks!
Antoine
--
Antoine Ténart, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH v3 net-next] net: dsa: mv88e6xxx: 88E6141/6341 SERDES support
From: Marek Behún @ 2018-05-04 17:26 UTC (permalink / raw)
To: netdev; +Cc: David S . Miller, Marek Behún
The 88E6141/6341 switches (also known as Topaz) have 1 SGMII lane,
which can be configured the same way as the SERDES lane on 88E6390.
Signed-off-by: Marek Behun <marek.behun@nic.cz>
---
drivers/net/dsa/mv88e6xxx/chip.c | 2 ++
drivers/net/dsa/mv88e6xxx/serdes.c | 20 ++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/serdes.h | 3 +++
3 files changed, 25 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 9d62e4acc01b..e7e079b1888c 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2529,6 +2529,7 @@ static const struct mv88e6xxx_ops mv88e6085_ops = {
.reset = mv88e6185_g1_reset,
.vtu_getnext = mv88e6352_g1_vtu_getnext,
.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
+ .serdes_power = mv88e6341_serdes_power,
};
static const struct mv88e6xxx_ops mv88e6095_ops = {
@@ -2848,6 +2849,7 @@ static const struct mv88e6xxx_ops mv88e6175_ops = {
.reset = mv88e6352_g1_reset,
.vtu_getnext = mv88e6352_g1_vtu_getnext,
.vtu_loadpurge = mv88e6352_g1_vtu_loadpurge,
+ .serdes_power = mv88e6341_serdes_power,
};
static const struct mv88e6xxx_ops mv88e6176_ops = {
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.c b/drivers/net/dsa/mv88e6xxx/serdes.c
index fb058fd35c0d..3a185ea1bf20 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.c
+++ b/drivers/net/dsa/mv88e6xxx/serdes.c
@@ -326,3 +326,23 @@ int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
return 0;
}
+
+int mv88e6341_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on)
+{
+ int err;
+ u8 cmode;
+
+ if (port != 5)
+ return 0;
+
+ err = mv88e6xxx_port_get_cmode(chip, port, &cmode);
+ if (err)
+ return err;
+
+ if (cmode == MV88E6XXX_PORT_STS_CMODE_1000BASE_X ||
+ cmode == MV88E6XXX_PORT_STS_CMODE_SGMII ||
+ cmode == MV88E6XXX_PORT_STS_CMODE_2500BASEX)
+ return mv88e6390_serdes_sgmii(chip, MV88E6341_ADDR_SERDES, on);
+
+ return 0;
+}
diff --git a/drivers/net/dsa/mv88e6xxx/serdes.h b/drivers/net/dsa/mv88e6xxx/serdes.h
index 1897c01c6e19..b6e5fbd46b5e 100644
--- a/drivers/net/dsa/mv88e6xxx/serdes.h
+++ b/drivers/net/dsa/mv88e6xxx/serdes.h
@@ -19,6 +19,8 @@
#define MV88E6352_ADDR_SERDES 0x0f
#define MV88E6352_SERDES_PAGE_FIBER 0x01
+#define MV88E6341_ADDR_SERDES 0x15
+
#define MV88E6390_PORT9_LANE0 0x09
#define MV88E6390_PORT9_LANE1 0x12
#define MV88E6390_PORT9_LANE2 0x13
@@ -42,6 +44,7 @@
#define MV88E6390_SGMII_CONTROL_LOOPBACK BIT(14)
#define MV88E6390_SGMII_CONTROL_PDOWN BIT(11)
+int mv88e6341_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
int mv88e6352_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
int mv88e6390_serdes_power(struct mv88e6xxx_chip *chip, int port, bool on);
int mv88e6352_serdes_get_sset_count(struct mv88e6xxx_chip *chip, int port);
--
2.16.1
^ permalink raw reply related
* Re: [PATCH net-next v2] net: core: rework basic flow dissection helper
From: David Miller @ 2018-05-04 17:26 UTC (permalink / raw)
To: pabeni; +Cc: netdev, edumazet, jasowang
In-Reply-To: <e87a05fed35c9cb8e60fe8f867c43b27b3044e21.1525426286.git.pabeni@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Fri, 4 May 2018 11:32:59 +0200
> When the core networking needs to detect the transport offset in a given
> packet and parse it explicitly, a full-blown flow_keys struct is used for
> storage.
> This patch introduces a smaller keys store, rework the basic flow dissect
> helper to use it, and apply this new helper where possible - namely in
> skb_probe_transport_header(). The used flow dissector data structures
> are renamed to match more closely the new role.
>
> The above gives ~50% performance improvement in micro benchmarking around
> skb_probe_transport_header() and ~30% around eth_get_headlen(), mostly due
> to the smaller memset. Small, but measurable improvement is measured also
> in macro benchmarking.
>
> v1 -> v2: use the new helper in eth_get_headlen() and skb_get_poff(),
> as per DaveM suggestion
>
> Suggested-by: David Miller <davem@davemloft.net>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Awesome. Applied, thanks Paolo.
^ permalink raw reply
* Re: [PATCH v2 2/4] net: hook socketpair() into LSM
From: David Miller @ 2018-05-04 17:31 UTC (permalink / raw)
To: dh.herrmann
Cc: linux-kernel, jmorris, paul, teg, sds, selinux,
linux-security-module, eparis, serge, casey, netdev
In-Reply-To: <20180504142822.15233-3-dh.herrmann@gmail.com>
From: David Herrmann <dh.herrmann@gmail.com>
Date: Fri, 4 May 2018 16:28:20 +0200
> Use the newly created LSM-hook for socketpair(). The default hook
> return-value is 0, so behavior stays the same unless LSMs start using
> this hook.
>
> Acked-by: Serge Hallyn <serge@hallyn.com>
> Signed-off-by: Tom Gundersen <teg@jklm.no>
> Signed-off-by: David Herrmann <dh.herrmann@gmail.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH net] net: phy: sfp: fix the BR,min computation
From: David Miller @ 2018-05-04 17:31 UTC (permalink / raw)
To: antoine.tenart
Cc: linux, netdev, linux-kernel, thomas.petazzoni, maxime.chevallier,
gregory.clement, miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20180504151054.17525-1-antoine.tenart@bootlin.com>
From: Antoine Tenart <antoine.tenart@bootlin.com>
Date: Fri, 4 May 2018 17:10:54 +0200
> In an SFP EEPROM values can be read to get information about a given SFP
> module. One of those is the bitrate, which can be determined using a
> nominal bitrate in addition with min and max values (in %). The SFP code
> currently compute both BR,min and BR,max values thanks to this nominal
> and min,max values.
>
> This patch fixes the BR,min computation as the min value should be
> subtracted to the nominal one, not added.
>
> Fixes: 9962acf7fb8c ("sfp: add support for 1000Base-PX and 1000Base-BX10")
> Signed-off-by: Antoine Tenart <antoine.tenart@bootlin.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Charity Gift !!!
From: Mrs Mavis L. Wanczyk @ 2018-05-03 16:23 UTC (permalink / raw)
--
This is the second time i am sending you this mail.
I, Mavis Wanczyk donates $ 5 Million Dollars from part of my Powerball
Jackpot Lottery of $ 758 Million Dollars, respond with your details
for claims.
I await your earliest response and God Bless you
Good luck.
Mavis Wanczyk
^ permalink raw reply
* Re: [PATCH v3 net-next] net: dsa: mv88e6xxx: 88E6141/6341 SERDES support
From: David Miller @ 2018-05-04 17:41 UTC (permalink / raw)
To: marek.behun; +Cc: netdev
In-Reply-To: <20180504172610.5406-1-marek.behun@nic.cz>
From: Marek Behún <marek.behun@nic.cz>
Date: Fri, 4 May 2018 19:26:10 +0200
> The 88E6141/6341 switches (also known as Topaz) have 1 SGMII lane,
> which can be configured the same way as the SERDES lane on 88E6390.
>
> Signed-off-by: Marek Behun <marek.behun@nic.cz>
Applied, thank you.
^ permalink raw reply
* [PATCH v5 0/6] firmware_loader: cleanups for v4.18
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
To: gregkh
Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
arend.vanspriel, zajec5, nbroeking, markivx, broonie,
dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
jewalt, oneukum, cantabile.desu, ast, hare, jejb, martin.petersen,
khc, davem, maco, arve, tkjos, linux
Greg,
I've reviewed the pending patches for the firmware_laoder and as for
v4.18, the following 3 patches from Andres have been iterated enough
that they're ready after I made some final minor changes, mostly just
style fixes and re-arrangements in terms of order. The new API he was
suggesting to add requires just a bit more review.
The last 3 patches are my own and are new, so I'd like further review
from others on them, but considering the changes Hans de Goede is having
us consider, I think this will prove useful to his work in terms of
splitting up code and documenting things properly. One thing that was
clear -- is our Kconfig entries for FW_LOADER were *extremely* outdated,
as such I've gone ahead and updated them.
The kconfig wording update for FW_LOADER includes prior conclusions made
to help justify keeping the split of the firmware fallback sysfs
interface in terms of size which was discussed with Josh Triplett a
while ago. It also includes modern recommendations, which would otherwise
get lost, on what to do about corner case firmware situations on
provisioning situations which folks have brought to my attention before
and architectural solutions based on firmwared [0] for a few years now.
Finally this work also reveals that a couple of candidate drivers could
likely move to staging considering their age, *or* we could just remove
the respective firmware build options. SCSI folks? Networking folks? To
my surprise *nothing* has been done about PREVENT_FIRMWARE_BUILD for
them since pre-git days! These sneaky litte buggers are:
* CONFIG_WANXL --> CONFIG_WANXL_BUILD_FIRMWARE
* CONFIG_SCSI_AIC79XX --> CONFIG_AIC79XX_BUILD_FIRMWARE
To this day both of these drivers are building driver *firmwares* when
the option CONFIG_PREVENT_FIRMWARE_BUILD is disabled, and they don't
even make use of the firmware API at all. I find it *highly unlikely*
pre-git day drivers are raging in new radical firmware updates these
days. I'll go put a knife into some of that unless I hear back from
SCSI or networking folks that WANXL_BUILD_FIRMWARE and
AIC79XX_BUILD_FIRMWARE are still hip and very much needed.
On my radar as well are Mimi's latest firmware_loader proposed changes,
but I think those need considerable review and attention from more security
folks, Android folks, and the linux-wireless community, our own
scattered random folks of firmware reviewer folks.
These patches are based on top of linux-next next-20180504, they are
also available in a respective git branch, both for linux-next [1] and
linux [2].
Question, and specially rants are greatly appreciated, and of course...
may the 4th be with you.
[0] https://github.com/teg/firmwared
[1] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux-next.git/log/?h=20180504-firmware_loader
[2] https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git/log/?h=20180504-firmware_loader
Luis
Andres Rodriguez (3):
firmware: wrap FW_OPT_* into an enum
firmware: use () to terminate kernel-doc function names
firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs()
Luis R. Rodriguez (3):
firmware_loader: document firmware_sysfs_fallback()
firmware_loader: enhance Kconfig documentation over FW_LOADER
firmware_loader: move kconfig FW_LOADER entries to its own file
drivers/base/Kconfig | 90 +++-----------
drivers/base/firmware_loader/Kconfig | 149 ++++++++++++++++++++++++
drivers/base/firmware_loader/fallback.c | 46 +++++---
drivers/base/firmware_loader/fallback.h | 18 +--
drivers/base/firmware_loader/firmware.h | 37 ++++--
drivers/base/firmware_loader/main.c | 28 ++---
6 files changed, 252 insertions(+), 116 deletions(-)
create mode 100644 drivers/base/firmware_loader/Kconfig
--
2.17.0
^ permalink raw reply
* [PATCH v5 2/6] firmware: use () to terminate kernel-doc function names
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
To: gregkh
Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
arend.vanspriel, zajec5, nbroeking, markivx, broonie,
dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
jewalt, oneukum, cantabile.desu, ast, hare, jejb, martin.petersen,
khc, davem, maco, arve, tkjos, linux
In-Reply-To: <20180504174356.13227-1-mcgrof@kernel.org>
From: Andres Rodriguez <andresx7@gmail.com>
The kernel-doc spec dictates a function name ends in ().
Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Randy Dunlap <rdunlap@infradead.org>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/fallback.c | 8 ++++----
drivers/base/firmware_loader/main.c | 20 ++++++++++----------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index b57a7b3b4122..529f7013616f 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -125,7 +125,7 @@ static ssize_t timeout_show(struct class *class, struct class_attribute *attr,
}
/**
- * firmware_timeout_store - set number of seconds to wait for firmware
+ * firmware_timeout_store() - set number of seconds to wait for firmware
* @class: device class pointer
* @attr: device attribute pointer
* @buf: buffer to scan for timeout value
@@ -239,7 +239,7 @@ static int map_fw_priv_pages(struct fw_priv *fw_priv)
}
/**
- * firmware_loading_store - set value in the 'loading' control file
+ * firmware_loading_store() - set value in the 'loading' control file
* @dev: device pointer
* @attr: device attribute pointer
* @buf: buffer to scan for loading control value
@@ -431,7 +431,7 @@ static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size)
}
/**
- * firmware_data_write - write method for firmware
+ * firmware_data_write() - write method for firmware
* @filp: open sysfs file
* @kobj: kobject for the device
* @bin_attr: bin_attr structure
@@ -537,7 +537,7 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
}
/**
- * fw_load_sysfs_fallback - load a firmware via the sysfs fallback mechanism
+ * fw_load_sysfs_fallback() - load a firmware via the sysfs fallback mechanism
* @fw_sysfs: firmware sysfs information for the firmware to load
* @opt_flags: flags of options, FW_OPT_*
* @timeout: timeout to wait for the load
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 9919f0e6a7cc..7f2bc7e8e3c0 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -597,7 +597,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
}
/**
- * request_firmware: - send firmware request and wait for it
+ * request_firmware() - send firmware request and wait for it
* @firmware_p: pointer to firmware image
* @name: name of firmware file
* @device: device for which firmware is being loaded
@@ -657,7 +657,7 @@ int request_firmware_direct(const struct firmware **firmware_p,
EXPORT_SYMBOL_GPL(request_firmware_direct);
/**
- * firmware_request_cache: - cache firmware for suspend so resume can use it
+ * firmware_request_cache() - cache firmware for suspend so resume can use it
* @name: name of firmware file
* @device: device for which firmware should be cached for
*
@@ -681,7 +681,7 @@ int firmware_request_cache(struct device *device, const char *name)
EXPORT_SYMBOL_GPL(firmware_request_cache);
/**
- * request_firmware_into_buf - load firmware into a previously allocated buffer
+ * request_firmware_into_buf() - load firmware into a previously allocated buffer
* @firmware_p: pointer to firmware image
* @name: name of firmware file
* @device: device for which firmware is being loaded and DMA region allocated
@@ -713,7 +713,7 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name,
EXPORT_SYMBOL(request_firmware_into_buf);
/**
- * release_firmware: - release the resource associated with a firmware image
+ * release_firmware() - release the resource associated with a firmware image
* @fw: firmware resource to release
**/
void release_firmware(const struct firmware *fw)
@@ -755,7 +755,7 @@ static void request_firmware_work_func(struct work_struct *work)
}
/**
- * request_firmware_nowait - asynchronous version of request_firmware
+ * request_firmware_nowait() - asynchronous version of request_firmware
* @module: module requesting the firmware
* @uevent: sends uevent to copy the firmware image if this flag
* is non-zero else the firmware copy must be done manually.
@@ -824,7 +824,7 @@ EXPORT_SYMBOL(request_firmware_nowait);
static ASYNC_DOMAIN_EXCLUSIVE(fw_cache_domain);
/**
- * cache_firmware - cache one firmware image in kernel memory space
+ * cache_firmware() - cache one firmware image in kernel memory space
* @fw_name: the firmware image name
*
* Cache firmware in kernel memory so that drivers can use it when
@@ -866,7 +866,7 @@ static struct fw_priv *lookup_fw_priv(const char *fw_name)
}
/**
- * uncache_firmware - remove one cached firmware image
+ * uncache_firmware() - remove one cached firmware image
* @fw_name: the firmware image name
*
* Uncache one firmware image which has been cached successfully
@@ -1042,7 +1042,7 @@ static void __device_uncache_fw_images(void)
}
/**
- * device_cache_fw_images - cache devices' firmware
+ * device_cache_fw_images() - cache devices' firmware
*
* If one device called request_firmware or its nowait version
* successfully before, the firmware names are recored into the
@@ -1075,7 +1075,7 @@ static void device_cache_fw_images(void)
}
/**
- * device_uncache_fw_images - uncache devices' firmware
+ * device_uncache_fw_images() - uncache devices' firmware
*
* uncache all firmwares which have been cached successfully
* by device_uncache_fw_images earlier
@@ -1092,7 +1092,7 @@ static void device_uncache_fw_images_work(struct work_struct *work)
}
/**
- * device_uncache_fw_images_delay - uncache devices firmwares
+ * device_uncache_fw_images_delay() - uncache devices firmwares
* @delay: number of milliseconds to delay uncache device firmwares
*
* uncache all devices's firmwares which has been cached successfully
--
2.17.0
^ permalink raw reply related
* [PATCH v5 3/6] firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs()
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
To: gregkh
Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
arend.vanspriel, zajec5, nbroeking, markivx, broonie,
dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
jewalt, oneukum, cantabile.desu, ast, hare, jejb, martin.petersen,
khc, davem, maco, arve, tkjos, linux
In-Reply-To: <20180504174356.13227-1-mcgrof@kernel.org>
From: Andres Rodriguez <andresx7@gmail.com>
This is done since this call is now exposed through kernel-doc,
and since this also paves the way for different future types of
fallback mechanims.
Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: small coding style changes]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/fallback.c | 8 ++++----
drivers/base/firmware_loader/fallback.h | 16 ++++++++--------
drivers/base/firmware_loader/firmware.h | 2 +-
drivers/base/firmware_loader/main.c | 2 +-
4 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 529f7013616f..3db9e0f225ac 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -662,10 +662,10 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
return fw_force_sysfs_fallback(opt_flags);
}
-int fw_sysfs_fallback(struct firmware *fw, const char *name,
- struct device *device,
- enum fw_opt opt_flags,
- int ret)
+int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+ struct device *device,
+ enum fw_opt opt_flags,
+ int ret)
{
if (!fw_run_sysfs_fallback(opt_flags))
return ret;
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index a3b73a09db6c..21063503e4ea 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -31,10 +31,10 @@ struct firmware_fallback_config {
};
#ifdef CONFIG_FW_LOADER_USER_HELPER
-int fw_sysfs_fallback(struct firmware *fw, const char *name,
- struct device *device,
- enum fw_opt opt_flags,
- int ret);
+int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+ struct device *device,
+ enum fw_opt opt_flags,
+ int ret);
void kill_pending_fw_fallback_reqs(bool only_kill_custom);
void fw_fallback_set_cache_timeout(void);
@@ -43,10 +43,10 @@ void fw_fallback_set_default_timeout(void);
int register_sysfs_loader(void);
void unregister_sysfs_loader(void);
#else /* CONFIG_FW_LOADER_USER_HELPER */
-static inline int fw_sysfs_fallback(struct firmware *fw, const char *name,
- struct device *device,
- enum fw_opt opt_flags,
- int ret)
+static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name,
+ struct device *device,
+ enum fw_opt opt_flags,
+ int ret)
{
/* Keep carrying over the same error */
return ret;
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 4f433b447367..4c1395f8e7ed 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -20,7 +20,7 @@
* @FW_OPT_NOWAIT: Used to describe the firmware request is asynchronous.
* @FW_OPT_USERHELPER: Enable the fallback mechanism, in case the direct
* filesystem lookup fails at finding the firmware. For details refer to
- * fw_sysfs_fallback().
+ * firmware_fallback_sysfs().
* @FW_OPT_NO_WARN: Quiet, avoid printing warning messages.
* @FW_OPT_NOCACHE: Disables firmware caching. Firmware caching is used to
* cache the firmware upon suspend, so that upon resume races against the
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index 7f2bc7e8e3c0..d951af29017a 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -581,7 +581,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
dev_warn(device,
"Direct firmware load for %s failed with error %d\n",
name, ret);
- ret = fw_sysfs_fallback(fw, name, device, opt_flags, ret);
+ ret = firmware_fallback_sysfs(fw, name, device, opt_flags, ret);
} else
ret = assign_fw(fw, device, opt_flags);
--
2.17.0
^ permalink raw reply related
* [PATCH v5 4/6] firmware_loader: document firmware_sysfs_fallback()
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
To: gregkh
Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
arend.vanspriel, zajec5, nbroeking, markivx, broonie,
dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
jewalt, oneukum, cantabile.desu, ast, hare, jejb, martin.petersen,
khc, davem, maco, arve, tkjos, linux
In-Reply-To: <20180504174356.13227-1-mcgrof@kernel.org>
This also sets the expecations for future fallback interfaces, even
if they are not exported.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/fallback.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 3db9e0f225ac..9169e7b9800c 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -662,6 +662,26 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
return fw_force_sysfs_fallback(opt_flags);
}
+/**
+ * firmware_fallback_sysfs() - use the fallback mechanism to find firmware
+ * @fw: pointer to firmware image
+ * @name: name of firmware file to look for
+ * @device: device for which firmware is being loaded
+ * @opt_flags: options to control firmware loading behaviour
+ * @ret: return value from direct lookup which triggered the fallback mechanism
+ *
+ * This function is called if direct lookup for the firmware failed, it enables
+ * a fallback mechanism through userspace by exposing a sysfs loading
+ * interface. Userspace is in charge of loading the firmware through the syfs
+ * loading interface. This syfs fallback mechanism may be disabled completely
+ * on a system by setting the proc sysctl value ignore_sysfs_fallback to true.
+ * If this false we check if the internal API caller set the @FW_OPT_NOFALLBACK
+ * flag, if so it would also disable the fallback mechanism. A system may want
+ * to enfoce the sysfs fallback mechanism at all times, it can do this by
+ * setting ignore_sysfs_fallback to false and force_sysfs_fallback to true.
+ * Enabling force_sysfs_fallback is functionally equivalent to build a kernel
+ * with CONFIG_FW_LOADER_USER_HELPER_FALLBACK.
+ **/
int firmware_fallback_sysfs(struct firmware *fw, const char *name,
struct device *device,
enum fw_opt opt_flags,
--
2.17.0
^ permalink raw reply related
* [PATCH v5 5/6] firmware_loader: enhance Kconfig documentation over FW_LOADER
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
To: gregkh
Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
arend.vanspriel, zajec5, nbroeking, markivx, broonie,
dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
jewalt, oneukum, cantabile.desu, ast, hare, jejb, martin.petersen,
khc, davem, maco, arve, tkjos, linux
In-Reply-To: <20180504174356.13227-1-mcgrof@kernel.org>
If you try to read FW_LOADER today it speaks of old riddles and
unless you have been following development closely you will loose
track of what is what. Even the documentation for PREVENT_FIRMWARE_BUILD
is a bit fuzzy and how it fits into this big picture.
Give the FW_LOADER kconfig documentation some love with more up to
date developments and recommendations. While at it, wrap the FW_LOADER
code into its own menu to compartamentalize and make it clearer which
components really are part of the FW_LOADER. This should also make
it easier to later move these kconfig entries into the firmware_loader/
directory later.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/Kconfig | 160 ++++++++++++++++++++++++++++++++++---------
1 file changed, 126 insertions(+), 34 deletions(-)
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 29b0eb452b3a..bf2d464b0e2c 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -70,39 +70,64 @@ config STANDALONE
If unsure, say Y.
config PREVENT_FIRMWARE_BUILD
- bool "Prevent firmware from being built"
+ bool "Disable drivers features which enable custom firmware building"
default y
help
- Say yes to avoid building firmware. Firmware is usually shipped
- with the driver and only when updating the firmware should a
- rebuild be made.
- If unsure, say Y here.
+ Say yes to disable driver features which enable building a custom
+ driver firmwar at kernel build time. These drivers do not use the
+ kernel firmware API to load firmware (CONFIG_FW_LOADER), instead they
+ use their own custom loading mechanism. The required firmware is
+ usually shipped with the driver, building the driver firmware
+ should only be needed if you have an updated firmware source.
+
+ Firmware should not be being built as part of kernel, these days
+ you should always prevent this and say Y here. There are only two
+ old drivers which enable building of its firmware at kernel build
+ time:
+
+ o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
+ o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
+
+menu "Firmware loader"
config FW_LOADER
- tristate "Userspace firmware loading support" if EXPERT
+ tristate "Firmware loading facility" if EXPERT
default y
---help---
- This option is provided for the case where none of the in-tree modules
- require userspace firmware loading support, but a module built
- out-of-tree does.
+ This enables the firmware loading facility in the kernel. The kernel
+ will first look for built-in firmware, if it has any. Next, it will
+ look for the requested firmware in a series of filesystem paths:
+
+ o firmware_class path module parameter or kernel boot param
+ o /lib/firmware/updates/UTS_RELEASE
+ o /lib/firmware/updates
+ o /lib/firmware/UTS_RELEASE
+ o /lib/firmware
+
+ Enabling this feature only increases your kernel image by about
+ 828 bytes, enable this option unless you are certain you don't
+ need firmware.
+
+ You typically want this built-in (=y) but you can also enable this
+ as a module, in which case the firmware_class module will be built.
+ You also want to be sure to enable this built-in if you are going to
+ enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
+
+if FW_LOADER
config EXTRA_FIRMWARE
- string "External firmware blobs to build into the kernel binary"
- depends on FW_LOADER
+ string "Build these firmware blobs into the kernel binary"
help
- Various drivers in the kernel source tree may require firmware,
- which is generally available in your distribution's linux-firmware
- package.
+ Device drivers which require firmware can typically deal with
+ having the kernel load firmware from the various supported
+ /lib/firmware/ paths. This option enables you to build into the
+ kernel firmware files. Built-in firmware searches are preceeded
+ over firmware lookups using your filesystem over the supported
+ /lib/firmware paths documented on CONFIG_FW_LOADER.
- The linux-firmware package should install firmware into
- /lib/firmware/ on your system, so they can be loaded by userspace
- helpers on request.
-
- This option allows firmware to be built into the kernel for the case
- where the user either cannot or doesn't want to provide it from
- userspace at runtime (for example, when the firmware in question is
- required for accessing the boot device, and the user doesn't want to
- use an initrd).
+ This may be useful for testing or if the firmware is required early on
+ in boot and cannot rely on the firmware being placed in an initrd or
+ initramfs.
This option is a string and takes the (space-separated) names of the
firmware files -- the same names that appear in MODULE_FIRMWARE()
@@ -113,7 +138,7 @@ config EXTRA_FIRMWARE
For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
the usb8388.bin file into /lib/firmware, and build the kernel. Then
any request_firmware("usb8388.bin") will be satisfied internally
- without needing to call out to userspace.
+ inside the kernel without ever looking at your filesystem at runtime.
WARNING: If you include additional firmware files into your binary
kernel image that are not available under the terms of the GPL,
@@ -130,22 +155,89 @@ config EXTRA_FIRMWARE_DIR
looks for the firmware files listed in the EXTRA_FIRMWARE option.
config FW_LOADER_USER_HELPER
- bool
+ bool "Enable the firmware sysfs fallback mechanism"
+ help
+ This option enables a sysfs loading facility to enable firmware
+ loading to the kernel through userspace as a fallback mechanism
+ if and only if the kernel's direct filesystem lookup for the
+ firmware failed using the different /lib/firmware/ paths, or the
+ path specified in the firmware_class path module parameter, or the
+ firmware_class path kernel boot parameter if the firmware_class is
+ built-in. For details on how to work with the sysfs fallback mechanism
+ refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
+
+ The direct filesystem lookup for firwmare is always used first now.
+
+ If the kernel's direct filesystem lookup for firware fails to find
+ the requested firmware a sysfs fallback loading facility is made
+ available and userspace is informed about this through uevents.
+ The uevent can be supressed if the driver explicitly requested it,
+ this is known as the driver using the custom fallback mechanism.
+ If the custom fallback mechanism is used userspace must always
+ acknowledge failure to find firmware as the timeout for the fallback
+ mechanism is disabled, and failed requests will linger forever.
+
+ This used to be the default firmware loading facility, and udev used
+ listen for uvents to load firmware for the kernel. The firmware
+ loading facility functionality in udev has been removed, as such it
+ can no longer be relied upon as a fallback mechanism. Linux no longer
+ relies on or uses a fallback mechanism in userspace.
+
+ Since this was the default firmware loading facility at one point,
+ old userspace may exist which relies upon it, and as such this
+ mechanism can never be removed from the kernel.
+
+ You should only enable this functionality if you are certain you
+ require a fallback mechanism and have a userspace mechanism ready to
+ load firmware in case it is not found. Another reason kernels may
+ have this feature enabled is to support a driver which explicitly
+ relies on this fallback mechanism. Only two drivers need this today:
+
+ o CONFIG_LEDS_LP55XX_COMMON
+ o CONFIG_DELL_RBU
+
+ Outside of supporting the above drivers, another reason for needing
+ this may be that your firmware resides outside of the paths the kernel
+ looks for and cannot possibily be specified using the firmware_class
+ path module parameter or kernel firmware_class path boot parameter
+ if firmware_class is built-in.
+
+ A modern use case may be to temporarily mount a custom partition
+ during provisioning which is only accessible to userspace, and then
+ to use it to look for and fetch the required firmware. Such type of
+ driver functionality may not even ever be desirable upstream by
+ vendors, and as such is only required to be supported as an interface
+ for provisioning. Since udev's firmware loading facility has been
+ removed you can use firmwared or a fork of it to customize how you
+ want to load firmware based on uevents issued:
+ https://github.com/teg/firmwared
+
+ Enabling this option will increase your kernel image size by about
+ 13436 bytes.
+
+ If you are unsure about this, say N here, unless you are Linux
+ distribution and need to support the above two drivers, or you are
+ certain you need to support some really custom firmware loading
+ facility in userspace.
config FW_LOADER_USER_HELPER_FALLBACK
- bool "Fallback user-helper invocation for firmware loading"
- depends on FW_LOADER
- select FW_LOADER_USER_HELPER
+ bool "Force the firmware sysfs fallback mechanism when possible"
+ depends on FW_LOADER_USER_HELPER
help
- This option enables / disables the invocation of user-helper
- (e.g. udev) for loading firmware files as a fallback after the
- direct file loading in kernel fails. The user-mode helper is
- no longer required unless you have a special firmware file that
- resides in a non-standard path. Moreover, the udev support has
- been deprecated upstream.
+ Enabling this option forces a sysfs userspace fallback mechanism
+ to be used for all firmware requests which explicitly do not disable a
+ a fallback mechanism. Firmware calls which do prohibit a fallback
+ mechanism is request_firmware_direct(). This option is kept for
+ backward compatibility purposes given this precise mechanism can also
+ be enabled by setting the proc sysctl value to true:
+
+ /proc/sys/kernel/firmware_config/force_sysfs_fallback
If you are unsure about this, say N here.
+endif # FW_LOADER
+endmenu
+
config WANT_DEV_COREDUMP
bool
help
--
2.17.0
^ permalink raw reply related
* [PATCH v5 6/6] firmware_loader: move kconfig FW_LOADER entries to its own file
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
To: gregkh
Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
arend.vanspriel, zajec5, nbroeking, markivx, broonie,
dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
jewalt, oneukum, cantabile.desu, ast, hare, jejb, martin.petersen,
khc, davem, maco, arve, tkjos, linux
In-Reply-To: <20180504174356.13227-1-mcgrof@kernel.org>
This will make it easier to track and easier to understand
what components and features are part of the FW_LOADER. There
are some components related to firmware which have *nothing* to
do with the FW_LOADER, souch as PREVENT_FIRMWARE_BUILD.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/Kconfig | 150 +--------------------------
drivers/base/firmware_loader/Kconfig | 149 ++++++++++++++++++++++++++
2 files changed, 150 insertions(+), 149 deletions(-)
create mode 100644 drivers/base/firmware_loader/Kconfig
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index bf2d464b0e2c..06d6e27784fa 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -88,155 +88,7 @@ config PREVENT_FIRMWARE_BUILD
o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE
o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE
-menu "Firmware loader"
-
-config FW_LOADER
- tristate "Firmware loading facility" if EXPERT
- default y
- ---help---
- This enables the firmware loading facility in the kernel. The kernel
- will first look for built-in firmware, if it has any. Next, it will
- look for the requested firmware in a series of filesystem paths:
-
- o firmware_class path module parameter or kernel boot param
- o /lib/firmware/updates/UTS_RELEASE
- o /lib/firmware/updates
- o /lib/firmware/UTS_RELEASE
- o /lib/firmware
-
- Enabling this feature only increases your kernel image by about
- 828 bytes, enable this option unless you are certain you don't
- need firmware.
-
- You typically want this built-in (=y) but you can also enable this
- as a module, in which case the firmware_class module will be built.
- You also want to be sure to enable this built-in if you are going to
- enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
-
-if FW_LOADER
-
-config EXTRA_FIRMWARE
- string "Build these firmware blobs into the kernel binary"
- help
- Device drivers which require firmware can typically deal with
- having the kernel load firmware from the various supported
- /lib/firmware/ paths. This option enables you to build into the
- kernel firmware files. Built-in firmware searches are preceeded
- over firmware lookups using your filesystem over the supported
- /lib/firmware paths documented on CONFIG_FW_LOADER.
-
- This may be useful for testing or if the firmware is required early on
- in boot and cannot rely on the firmware being placed in an initrd or
- initramfs.
-
- This option is a string and takes the (space-separated) names of the
- firmware files -- the same names that appear in MODULE_FIRMWARE()
- and request_firmware() in the source. These files should exist under
- the directory specified by the EXTRA_FIRMWARE_DIR option, which is
- /lib/firmware by default.
-
- For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
- the usb8388.bin file into /lib/firmware, and build the kernel. Then
- any request_firmware("usb8388.bin") will be satisfied internally
- inside the kernel without ever looking at your filesystem at runtime.
-
- WARNING: If you include additional firmware files into your binary
- kernel image that are not available under the terms of the GPL,
- then it may be a violation of the GPL to distribute the resulting
- image since it combines both GPL and non-GPL work. You should
- consult a lawyer of your own before distributing such an image.
-
-config EXTRA_FIRMWARE_DIR
- string "Firmware blobs root directory"
- depends on EXTRA_FIRMWARE != ""
- default "/lib/firmware"
- help
- This option controls the directory in which the kernel build system
- looks for the firmware files listed in the EXTRA_FIRMWARE option.
-
-config FW_LOADER_USER_HELPER
- bool "Enable the firmware sysfs fallback mechanism"
- help
- This option enables a sysfs loading facility to enable firmware
- loading to the kernel through userspace as a fallback mechanism
- if and only if the kernel's direct filesystem lookup for the
- firmware failed using the different /lib/firmware/ paths, or the
- path specified in the firmware_class path module parameter, or the
- firmware_class path kernel boot parameter if the firmware_class is
- built-in. For details on how to work with the sysfs fallback mechanism
- refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
-
- The direct filesystem lookup for firwmare is always used first now.
-
- If the kernel's direct filesystem lookup for firware fails to find
- the requested firmware a sysfs fallback loading facility is made
- available and userspace is informed about this through uevents.
- The uevent can be supressed if the driver explicitly requested it,
- this is known as the driver using the custom fallback mechanism.
- If the custom fallback mechanism is used userspace must always
- acknowledge failure to find firmware as the timeout for the fallback
- mechanism is disabled, and failed requests will linger forever.
-
- This used to be the default firmware loading facility, and udev used
- listen for uvents to load firmware for the kernel. The firmware
- loading facility functionality in udev has been removed, as such it
- can no longer be relied upon as a fallback mechanism. Linux no longer
- relies on or uses a fallback mechanism in userspace.
-
- Since this was the default firmware loading facility at one point,
- old userspace may exist which relies upon it, and as such this
- mechanism can never be removed from the kernel.
-
- You should only enable this functionality if you are certain you
- require a fallback mechanism and have a userspace mechanism ready to
- load firmware in case it is not found. Another reason kernels may
- have this feature enabled is to support a driver which explicitly
- relies on this fallback mechanism. Only two drivers need this today:
-
- o CONFIG_LEDS_LP55XX_COMMON
- o CONFIG_DELL_RBU
-
- Outside of supporting the above drivers, another reason for needing
- this may be that your firmware resides outside of the paths the kernel
- looks for and cannot possibily be specified using the firmware_class
- path module parameter or kernel firmware_class path boot parameter
- if firmware_class is built-in.
-
- A modern use case may be to temporarily mount a custom partition
- during provisioning which is only accessible to userspace, and then
- to use it to look for and fetch the required firmware. Such type of
- driver functionality may not even ever be desirable upstream by
- vendors, and as such is only required to be supported as an interface
- for provisioning. Since udev's firmware loading facility has been
- removed you can use firmwared or a fork of it to customize how you
- want to load firmware based on uevents issued:
- https://github.com/teg/firmwared
-
- Enabling this option will increase your kernel image size by about
- 13436 bytes.
-
- If you are unsure about this, say N here, unless you are Linux
- distribution and need to support the above two drivers, or you are
- certain you need to support some really custom firmware loading
- facility in userspace.
-
-config FW_LOADER_USER_HELPER_FALLBACK
- bool "Force the firmware sysfs fallback mechanism when possible"
- depends on FW_LOADER_USER_HELPER
- help
- Enabling this option forces a sysfs userspace fallback mechanism
- to be used for all firmware requests which explicitly do not disable a
- a fallback mechanism. Firmware calls which do prohibit a fallback
- mechanism is request_firmware_direct(). This option is kept for
- backward compatibility purposes given this precise mechanism can also
- be enabled by setting the proc sysctl value to true:
-
- /proc/sys/kernel/firmware_config/force_sysfs_fallback
-
- If you are unsure about this, say N here.
-
-endif # FW_LOADER
-endmenu
+source "drivers/base/firmware_loader/Kconfig"
config WANT_DEV_COREDUMP
bool
diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig
new file mode 100644
index 000000000000..b4cf42d6fe51
--- /dev/null
+++ b/drivers/base/firmware_loader/Kconfig
@@ -0,0 +1,149 @@
+menu "Firmware loader"
+
+config FW_LOADER
+ tristate "Firmware loading facility" if EXPERT
+ default y
+ ---help---
+ This enables the firmware loading facility in the kernel. The kernel
+ will first look for built-in firmware, if it has any. Next, it will
+ look for the requested firmware in a series of filesystem paths:
+
+ o firmware_class path module parameter or kernel boot param
+ o /lib/firmware/updates/UTS_RELEASE
+ o /lib/firmware/updates
+ o /lib/firmware/UTS_RELEASE
+ o /lib/firmware
+
+ Enabling this feature only increases your kernel image by about
+ 828 bytes, enable this option unless you are certain you don't
+ need firmware.
+
+ You typically want this built-in (=y) but you can also enable this
+ as a module, in which case the firmware_class module will be built.
+ You also want to be sure to enable this built-in if you are going to
+ enable built-in firmware (CONFIG_EXTRA_FIRMWARE).
+
+if FW_LOADER
+
+config EXTRA_FIRMWARE
+ string "Build these firmware blobs into the kernel binary"
+ help
+ Device drivers which require firmware can typically deal with
+ having the kernel load firmware from the various supported
+ /lib/firmware/ paths. This option enables you to build into the
+ kernel firmware files. Built-in firmware searches are preceeded
+ over firmware lookups using your filesystem over the supported
+ /lib/firmware paths documented on CONFIG_FW_LOADER.
+
+ This may be useful for testing or if the firmware is required early on
+ in boot and cannot rely on the firmware being placed in an initrd or
+ initramfs.
+
+ This option is a string and takes the (space-separated) names of the
+ firmware files -- the same names that appear in MODULE_FIRMWARE()
+ and request_firmware() in the source. These files should exist under
+ the directory specified by the EXTRA_FIRMWARE_DIR option, which is
+ /lib/firmware by default.
+
+ For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy
+ the usb8388.bin file into /lib/firmware, and build the kernel. Then
+ any request_firmware("usb8388.bin") will be satisfied internally
+ inside the kernel without ever looking at your filesystem at runtime.
+
+ WARNING: If you include additional firmware files into your binary
+ kernel image that are not available under the terms of the GPL,
+ then it may be a violation of the GPL to distribute the resulting
+ image since it combines both GPL and non-GPL work. You should
+ consult a lawyer of your own before distributing such an image.
+
+config EXTRA_FIRMWARE_DIR
+ string "Firmware blobs root directory"
+ depends on EXTRA_FIRMWARE != ""
+ default "/lib/firmware"
+ help
+ This option controls the directory in which the kernel build system
+ looks for the firmware files listed in the EXTRA_FIRMWARE option.
+
+config FW_LOADER_USER_HELPER
+ bool "Enable the firmware sysfs fallback mechanism"
+ help
+ This option enables a sysfs loading facility to enable firmware
+ loading to the kernel through userspace as a fallback mechanism
+ if and only if the kernel's direct filesystem lookup for the
+ firmware failed using the different /lib/firmware/ paths, or the
+ path specified in the firmware_class path module parameter, or the
+ firmware_class path kernel boot parameter if the firmware_class is
+ built-in. For details on how to work with the sysfs fallback mechanism
+ refer to Documentation/driver-api/firmware/fallback-mechanisms.rst.
+
+ The direct filesystem lookup for firwmare is always used first now.
+
+ If the kernel's direct filesystem lookup for firware fails to find
+ the requested firmware a sysfs fallback loading facility is made
+ available and userspace is informed about this through uevents.
+ The uevent can be supressed if the driver explicitly requested it,
+ this is known as the driver using the custom fallback mechanism.
+ If the custom fallback mechanism is used userspace must always
+ acknowledge failure to find firmware as the timeout for the fallback
+ mechanism is disabled, and failed requests will linger forever.
+
+ This used to be the default firmware loading facility, and udev used
+ listen for uvents to load firmware for the kernel. The firmware
+ loading facility functionality in udev has been removed, as such it
+ can no longer be relied upon as a fallback mechanism. Linux no longer
+ relies on or uses a fallback mechanism in userspace.
+
+ Since this was the default firmware loading facility at one point,
+ old userspace may exist which relies upon it, and as such this
+ mechanism can never be removed from the kernel.
+
+ You should only enable this functionality if you are certain you
+ require a fallback mechanism and have a userspace mechanism ready to
+ load firmware in case it is not found. Another reason kernels may
+ have this feature enabled is to support a driver which explicitly
+ relies on this fallback mechanism. Only two drivers need this today:
+
+ o CONFIG_LEDS_LP55XX_COMMON
+ o CONFIG_DELL_RBU
+
+ Outside of supporting the above drivers, another reason for needing
+ this may be that your firmware resides outside of the paths the kernel
+ looks for and cannot possibily be specified using the firmware_class
+ path module parameter or kernel firmware_class path boot parameter
+ if firmware_class is built-in.
+
+ A modern use case may be to temporarily mount a custom partition
+ during provisioning which is only accessible to userspace, and then
+ to use it to look for and fetch the required firmware. Such type of
+ driver functionality may not even ever be desirable upstream by
+ vendors, and as such is only required to be supported as an interface
+ for provisioning. Since udev's firmware loading facility has been
+ removed you can use firmwared or a fork of it to customize how you
+ want to load firmware based on uevents issued:
+ https://github.com/teg/firmwared
+
+ Enabling this option will increase your kernel image size by about
+ 13436 bytes.
+
+ If you are unsure about this, say N here, unless you are Linux
+ distribution and need to support the above two drivers, or you are
+ certain you need to support some really custom firmware loading
+ facility in userspace.
+
+config FW_LOADER_USER_HELPER_FALLBACK
+ bool "Force the firmware sysfs fallback mechanism when possible"
+ depends on FW_LOADER_USER_HELPER
+ help
+ Enabling this option forces a sysfs userspace fallback mechanism
+ to be used for all firmware requests which explicitly do not disable a
+ a fallback mechanism. Firmware calls which do prohibit a fallback
+ mechanism is request_firmware_direct(). This option is kept for
+ backward compatibility purposes given this precise mechanism can also
+ be enabled by setting the proc sysctl value to true:
+
+ /proc/sys/kernel/firmware_config/force_sysfs_fallback
+
+ If you are unsure about this, say N here.
+
+endif # FW_LOADER
+endmenu
--
2.17.0
^ permalink raw reply related
* [PATCH v5 1/6] firmware: wrap FW_OPT_* into an enum
From: Luis R. Rodriguez @ 2018-05-04 17:43 UTC (permalink / raw)
To: gregkh
Cc: akpm, keescook, josh, teg, wagi, hdegoede, andresx7, zohar,
kubakici, shuah, mfuzzey, dhowells, pali.rohar, tiwai, kvalo,
arend.vanspriel, zajec5, nbroeking, markivx, broonie,
dmitry.torokhov, dwmw2, torvalds, Abhay_Salunke, bjorn.andersson,
jewalt, oneukum, cantabile.desu, ast, hare, jejb, martin.petersen,
khc, davem, maco, arve, tkjos, linux
In-Reply-To: <20180504174356.13227-1-mcgrof@kernel.org>
From: Andres Rodriguez <andresx7@gmail.com>
This should let us associate enum kdoc to these values.
While at it, kdocify the fw_opt.
Signed-off-by: Andres Rodriguez <andresx7@gmail.com>
Acked-by: Luis R. Rodriguez <mcgrof@kernel.org>
[mcgrof: coding style fixes, merge kdoc with enum move]
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/base/firmware_loader/fallback.c | 12 ++++----
drivers/base/firmware_loader/fallback.h | 6 ++--
drivers/base/firmware_loader/firmware.h | 37 +++++++++++++++++++------
drivers/base/firmware_loader/main.c | 6 ++--
4 files changed, 42 insertions(+), 19 deletions(-)
diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c
index 358354148dec..b57a7b3b4122 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -512,7 +512,7 @@ static const struct attribute_group *fw_dev_attr_groups[] = {
static struct fw_sysfs *
fw_create_instance(struct firmware *firmware, const char *fw_name,
- struct device *device, unsigned int opt_flags)
+ struct device *device, enum fw_opt opt_flags)
{
struct fw_sysfs *fw_sysfs;
struct device *f_dev;
@@ -545,7 +545,7 @@ fw_create_instance(struct firmware *firmware, const char *fw_name,
* In charge of constructing a sysfs fallback interface for firmware loading.
**/
static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
- unsigned int opt_flags, long timeout)
+ enum fw_opt opt_flags, long timeout)
{
int retval = 0;
struct device *f_dev = &fw_sysfs->dev;
@@ -599,7 +599,7 @@ static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs,
static int fw_load_from_user_helper(struct firmware *firmware,
const char *name, struct device *device,
- unsigned int opt_flags)
+ enum fw_opt opt_flags)
{
struct fw_sysfs *fw_sysfs;
long timeout;
@@ -640,7 +640,7 @@ static int fw_load_from_user_helper(struct firmware *firmware,
return ret;
}
-static bool fw_force_sysfs_fallback(unsigned int opt_flags)
+static bool fw_force_sysfs_fallback(enum fw_opt opt_flags)
{
if (fw_fallback_config.force_sysfs_fallback)
return true;
@@ -649,7 +649,7 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags)
return true;
}
-static bool fw_run_sysfs_fallback(unsigned int opt_flags)
+static bool fw_run_sysfs_fallback(enum fw_opt opt_flags)
{
if (fw_fallback_config.ignore_sysfs_fallback) {
pr_info_once("Ignoring firmware sysfs fallback due to sysctl knob\n");
@@ -664,7 +664,7 @@ static bool fw_run_sysfs_fallback(unsigned int opt_flags)
int fw_sysfs_fallback(struct firmware *fw, const char *name,
struct device *device,
- unsigned int opt_flags,
+ enum fw_opt opt_flags,
int ret)
{
if (!fw_run_sysfs_fallback(opt_flags))
diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h
index f8255670a663..a3b73a09db6c 100644
--- a/drivers/base/firmware_loader/fallback.h
+++ b/drivers/base/firmware_loader/fallback.h
@@ -5,6 +5,8 @@
#include <linux/firmware.h>
#include <linux/device.h>
+#include "firmware.h"
+
/**
* struct firmware_fallback_config - firmware fallback configuration settings
*
@@ -31,7 +33,7 @@ struct firmware_fallback_config {
#ifdef CONFIG_FW_LOADER_USER_HELPER
int fw_sysfs_fallback(struct firmware *fw, const char *name,
struct device *device,
- unsigned int opt_flags,
+ enum fw_opt opt_flags,
int ret);
void kill_pending_fw_fallback_reqs(bool only_kill_custom);
@@ -43,7 +45,7 @@ void unregister_sysfs_loader(void);
#else /* CONFIG_FW_LOADER_USER_HELPER */
static inline int fw_sysfs_fallback(struct firmware *fw, const char *name,
struct device *device,
- unsigned int opt_flags,
+ enum fw_opt opt_flags,
int ret)
{
/* Keep carrying over the same error */
diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h
index 64acbb1a392c..4f433b447367 100644
--- a/drivers/base/firmware_loader/firmware.h
+++ b/drivers/base/firmware_loader/firmware.h
@@ -2,6 +2,7 @@
#ifndef __FIRMWARE_LOADER_H
#define __FIRMWARE_LOADER_H
+#include <linux/bitops.h>
#include <linux/firmware.h>
#include <linux/types.h>
#include <linux/kref.h>
@@ -10,13 +11,33 @@
#include <generated/utsrelease.h>
-/* firmware behavior options */
-#define FW_OPT_UEVENT (1U << 0)
-#define FW_OPT_NOWAIT (1U << 1)
-#define FW_OPT_USERHELPER (1U << 2)
-#define FW_OPT_NO_WARN (1U << 3)
-#define FW_OPT_NOCACHE (1U << 4)
-#define FW_OPT_NOFALLBACK (1U << 5)
+/**
+ * enum fw_opt - options to control firmware loading behaviour
+ *
+ * @FW_OPT_UEVENT: Enables the fallback mechanism to send a kobject uevent
+ * when the firmware is not found. Userspace is in charge to load the
+ * firmware using the sysfs loading facility.
+ * @FW_OPT_NOWAIT: Used to describe the firmware request is asynchronous.
+ * @FW_OPT_USERHELPER: Enable the fallback mechanism, in case the direct
+ * filesystem lookup fails at finding the firmware. For details refer to
+ * fw_sysfs_fallback().
+ * @FW_OPT_NO_WARN: Quiet, avoid printing warning messages.
+ * @FW_OPT_NOCACHE: Disables firmware caching. Firmware caching is used to
+ * cache the firmware upon suspend, so that upon resume races against the
+ * firmware file lookup on storage is avoided. Used for calls where the
+ * file may be too big, or where the driver takes charge of its own
+ * firmware caching mechanism.
+ * @FW_OPT_NOFALLBACK: Disable the fallback mechanism. Takes precedence over
+ * &FW_OPT_UEVENT and &FW_OPT_USERHELPER.
+ */
+enum fw_opt {
+ FW_OPT_UEVENT = BIT(0),
+ FW_OPT_NOWAIT = BIT(1),
+ FW_OPT_USERHELPER = BIT(2),
+ FW_OPT_NO_WARN = BIT(3),
+ FW_OPT_NOCACHE = BIT(4),
+ FW_OPT_NOFALLBACK = BIT(5),
+};
enum fw_status {
FW_STATUS_UNKNOWN,
@@ -110,6 +131,6 @@ static inline void fw_state_done(struct fw_priv *fw_priv)
}
int assign_fw(struct firmware *fw, struct device *device,
- unsigned int opt_flags);
+ enum fw_opt opt_flags);
#endif /* __FIRMWARE_LOADER_H */
diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c
index eb34089e4299..9919f0e6a7cc 100644
--- a/drivers/base/firmware_loader/main.c
+++ b/drivers/base/firmware_loader/main.c
@@ -443,7 +443,7 @@ static int fw_add_devm_name(struct device *dev, const char *name)
#endif
int assign_fw(struct firmware *fw, struct device *device,
- unsigned int opt_flags)
+ enum fw_opt opt_flags)
{
struct fw_priv *fw_priv = fw->priv;
int ret;
@@ -558,7 +558,7 @@ static void fw_abort_batch_reqs(struct firmware *fw)
static int
_request_firmware(const struct firmware **firmware_p, const char *name,
struct device *device, void *buf, size_t size,
- unsigned int opt_flags)
+ enum fw_opt opt_flags)
{
struct firmware *fw = NULL;
int ret;
@@ -734,7 +734,7 @@ struct firmware_work {
struct device *device;
void *context;
void (*cont)(const struct firmware *fw, void *context);
- unsigned int opt_flags;
+ enum fw_opt opt_flags;
};
static void request_firmware_work_func(struct work_struct *work)
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] net: Work around crash in ipv6 fib-walk-continue
From: David Ahern @ 2018-05-04 17:47 UTC (permalink / raw)
To: greearb, netdev
In-Reply-To: <1524160886-20401-1-git-send-email-greearb@candelatech.com>
On 4/19/18 12:01 PM, greearb@candelatech.com wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> This keeps us from crashing in certain test cases where we
> bring up many (1000, for instance) mac-vlans with IPv6
> enabled in the kernel. This bug has been around for a
> very long time.
>
> Until a real fix is found (and for stable), maybe it
> is better to return an incomplete fib walk instead
> of crashing.
>
> BUG: unable to handle kernel NULL pointer dereference at 8
> IP: fib6_walk_continue+0x5b/0x140 [ipv6]
> PGD 80000007dfc0c067 P4D 80000007dfc0c067 PUD 7e66ff067 PMD 0
> Oops: 0000 [#1] PREEMPT SMP PTI
> Modules linked in: nf_conntrack_netlink nf_conntrack nfnetlink nf_defrag_ipv4 libcrc32c vrf]
> CPU: 3 PID: 15117 Comm: ip Tainted: G O 4.16.0+ #5
> Hardware name: Iron_Systems,Inc CS-CAD-2U-A02/X10SRL-F, BIOS 2.0b 05/02/2017
> RIP: 0010:fib6_walk_continue+0x5b/0x140 [ipv6]
> RSP: 0018:ffffc90008c3bc10 EFLAGS: 00010287
> RAX: ffff88085ac45050 RBX: ffff8807e03008a0 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffc90008c3bc48 RDI: ffffffff8232b240
> RBP: ffff880819167600 R08: 0000000000000008 R09: ffff8807dff10071
> R10: ffffc90008c3bbd0 R11: 0000000000000000 R12: ffff8807e03008a0
> R13: 0000000000000002 R14: ffff8807e05744c8 R15: ffff8807e08ef000
> FS: 00007f2f04342700(0000) GS:ffff88087fcc0000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000008 CR3: 00000007e0556002 CR4: 00000000003606e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> inet6_dump_fib+0x14b/0x2c0 [ipv6]
> netlink_dump+0x216/0x2a0
> netlink_recvmsg+0x254/0x400
> ? copy_msghdr_from_user+0xb5/0x110
> ___sys_recvmsg+0xe9/0x230
> ? find_held_lock+0x3b/0xb0
> ? __handle_mm_fault+0x617/0x1180
> ? __audit_syscall_entry+0xb3/0x110
> ? __sys_recvmsg+0x39/0x70
> __sys_recvmsg+0x39/0x70
> do_syscall_64+0x63/0x120
> entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> RIP: 0033:0x7f2f03a72030
> RSP: 002b:00007fffab3de508 EFLAGS: 00000246 ORIG_RAX: 000000000000002f
> RAX: ffffffffffffffda RBX: 00007fffab3e641c RCX: 00007f2f03a72030
> RDX: 0000000000000000 RSI: 00007fffab3de570 RDI: 0000000000000004
> RBP: 0000000000000000 R08: 0000000000007e6c R09: 00007fffab3e63a8
> R10: 00007fffab3de5b0 R11: 0000000000000246 R12: 00007fffab3e6608
> R13: 000000000066b460 R14: 0000000000007e6c R15: 0000000000000000
> Code: 85 d2 74 17 f6 40 2a 04 74 11 8b 53 2c 85 d2 0f 84 d7 00 00 00 83 ea 01 89 53 2c c7 4
> RIP: fib6_walk_continue+0x5b/0x140 [ipv6] RSP: ffffc90008c3bc10
> CR2: 0000000000000008
> ---[ end trace bd03458864eb266c ]---
>
> Signed-off-by: Ben Greear <greearb@candelatech.com>
> ---
>
Does your use case that triggers this involve replacing routes? I just
noticed the route delete code in fib6_add_rt2node does not have the
'Adjust walkers' code that is in fib6_del_route.
Further, the adjust walkers code in fib6_del_route looks suspicious in
its timing with route deletes. If you have a reliable reproducer we can
try a few things with fib6_del_route and the walker code.
^ permalink raw reply
* [RFC PATCH 0/2] Introduce assert_in_softirq()
From: Sebastian Andrzej Siewior @ 2018-05-04 17:51 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, Peter Zijlstra, Ingo Molnar, David S. Miller, Johannes Berg,
Alexander Aring, Stefan Schmidt, netdev, linux-wireless,
linux-wpan
ieee80211_rx_napi() has a check to ensure that it is invoked in softirq
context / with BH disabled. It is there because it invokes
netif_receive_skb() which has this requirement.
On -RT this check does not work as expected so there is always this
warning.
Tree wide there are two users of this check: ieee80211_rx_napi() and
ieee802154_rx(). This approach introduces assert_in_softirq() which does
the check if lockdep is enabled. This check could then become a nop on
-RT.
As an alternative netif_receive_skb() (or ieee80211_rx_napi() could do
local_bh_disable() / local_bh_enable() unconditionally. The _disable()
part is very cheap. The _enable() part is more expensive because it
includes a function call. We could avoid that jump in the likely case
when BH was already disabled by something like:
static inline void local_bh_enable(void)
{
if (softirq_count() == SOFTIRQ_DISABLE_OFFSET)
__local_bh_enable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
else
preempt_count_sub(SOFTIRQ_DISABLE_OFFSET);
}
Which would make bh_enable() cheaper for everyone.
Sebastian
^ permalink raw reply
* [RFC PATCH 1/2] lockdep: Add a assert_in_softirq()
From: Sebastian Andrzej Siewior @ 2018-05-04 17:51 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, Peter Zijlstra, Ingo Molnar, David S. Miller, Johannes Berg,
Alexander Aring, Stefan Schmidt, netdev, linux-wireless,
linux-wpan, Anna-Maria Gleixner, Sebastian Andrzej Siewior
In-Reply-To: <20180504175144.12179-1-bigeasy@linutronix.de>
From: Anna-Maria Gleixner <anna-maria@linutronix.de>
Instead of directly warn on wrong context, check if softirq context is
set. This check could be a nop on RT.
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
include/linux/lockdep.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 6fc77d4dbdcd..59363c3047c6 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -608,11 +608,17 @@ do { \
"IRQs not disabled as expected\n"); \
} while (0)
+#define lockdep_assert_in_softirq() do { \
+ WARN_ONCE(debug_locks && !current->lockdep_recursion && \
+ !current->softirq_context, \
+ "Not in softirq context as expected\n"); \
+ } while (0)
#else
# define might_lock(lock) do { } while (0)
# define might_lock_read(lock) do { } while (0)
# define lockdep_assert_irqs_enabled() do { } while (0)
# define lockdep_assert_irqs_disabled() do { } while (0)
+# define lockdep_assert_in_softirq() do { } while (0)
#endif
#ifdef CONFIG_LOCKDEP
--
2.17.0
^ permalink raw reply related
* [RFC PATCH 2/2] net: mac808211: mac802154: use lockdep_assert_in_softirq() instead own warning
From: Sebastian Andrzej Siewior @ 2018-05-04 17:51 UTC (permalink / raw)
To: linux-kernel
Cc: tglx, Peter Zijlstra, Ingo Molnar, David S. Miller, Johannes Berg,
Alexander Aring, Stefan Schmidt, netdev, linux-wireless,
linux-wpan, Anna-Maria Gleixner, Sebastian Andrzej Siewior
In-Reply-To: <20180504175144.12179-1-bigeasy@linutronix.de>
From: Anna-Maria Gleixner <anna-maria@linutronix.de>
The warning in ieee802154_rx() and ieee80211_rx_napi() is there to ensure
the softirq context for the subsequent netif_receive_skb() call. The check
could be moved into the netif_receive_skb() function to prevent all calling
functions implement the checks on their own. Use the lockdep variant for
softirq context check. While at it, add a lockdep based check for irq
enabled as mentioned in the comment above netif_receive_skb().
Signed-off-by: Anna-Maria Gleixner <anna-maria@linutronix.de>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
net/core/dev.c | 3 +++
net/mac80211/rx.c | 2 --
net/mac802154/rx.c | 2 --
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index af0558b00c6c..7b4b8611cc21 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4750,6 +4750,9 @@ static int netif_receive_skb_internal(struct sk_buff *skb)
*/
int netif_receive_skb(struct sk_buff *skb)
{
+ lockdep_assert_irqs_enabled();
+ lockdep_assert_in_softirq();
+
trace_netif_receive_skb_entry(skb);
return netif_receive_skb_internal(skb);
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 03102aff0953..653332d81c17 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -4324,8 +4324,6 @@ void ieee80211_rx_napi(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta,
struct ieee80211_supported_band *sband;
struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
- WARN_ON_ONCE(softirq_count() == 0);
-
if (WARN_ON(status->band >= NUM_NL80211_BANDS))
goto drop;
diff --git a/net/mac802154/rx.c b/net/mac802154/rx.c
index 4dcf6e18563a..66916c270efc 100644
--- a/net/mac802154/rx.c
+++ b/net/mac802154/rx.c
@@ -258,8 +258,6 @@ void ieee802154_rx(struct ieee802154_local *local, struct sk_buff *skb)
{
u16 crc;
- WARN_ON_ONCE(softirq_count() == 0);
-
if (local->suspended)
goto drop;
--
2.17.0
^ permalink raw reply related
* Re: [PATCH] net: Work around crash in ipv6 fib-walk-continue
From: Ben Greear @ 2018-05-04 17:57 UTC (permalink / raw)
To: David Ahern, netdev
In-Reply-To: <513868fb-b548-0312-a276-482fed070d0f@gmail.com>
On 05/04/2018 10:47 AM, David Ahern wrote:
> On 4/19/18 12:01 PM, greearb@candelatech.com wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> This keeps us from crashing in certain test cases where we
>> bring up many (1000, for instance) mac-vlans with IPv6
>> enabled in the kernel. This bug has been around for a
>> very long time.
>>
>> Until a real fix is found (and for stable), maybe it
>> is better to return an incomplete fib walk instead
>> of crashing.
>>
>> BUG: unable to handle kernel NULL pointer dereference at 8
>> IP: fib6_walk_continue+0x5b/0x140 [ipv6]
>> PGD 80000007dfc0c067 P4D 80000007dfc0c067 PUD 7e66ff067 PMD 0
>> Oops: 0000 [#1] PREEMPT SMP PTI
>> Modules linked in: nf_conntrack_netlink nf_conntrack nfnetlink nf_defrag_ipv4 libcrc32c vrf]
>> CPU: 3 PID: 15117 Comm: ip Tainted: G O 4.16.0+ #5
>> Hardware name: Iron_Systems,Inc CS-CAD-2U-A02/X10SRL-F, BIOS 2.0b 05/02/2017
>> RIP: 0010:fib6_walk_continue+0x5b/0x140 [ipv6]
>> RSP: 0018:ffffc90008c3bc10 EFLAGS: 00010287
>> RAX: ffff88085ac45050 RBX: ffff8807e03008a0 RCX: 0000000000000000
>> RDX: 0000000000000000 RSI: ffffc90008c3bc48 RDI: ffffffff8232b240
>> RBP: ffff880819167600 R08: 0000000000000008 R09: ffff8807dff10071
>> R10: ffffc90008c3bbd0 R11: 0000000000000000 R12: ffff8807e03008a0
>> R13: 0000000000000002 R14: ffff8807e05744c8 R15: ffff8807e08ef000
>> FS: 00007f2f04342700(0000) GS:ffff88087fcc0000(0000) knlGS:0000000000000000
>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> CR2: 0000000000000008 CR3: 00000007e0556002 CR4: 00000000003606e0
>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
>> Call Trace:
>> inet6_dump_fib+0x14b/0x2c0 [ipv6]
>> netlink_dump+0x216/0x2a0
>> netlink_recvmsg+0x254/0x400
>> ? copy_msghdr_from_user+0xb5/0x110
>> ___sys_recvmsg+0xe9/0x230
>> ? find_held_lock+0x3b/0xb0
>> ? __handle_mm_fault+0x617/0x1180
>> ? __audit_syscall_entry+0xb3/0x110
>> ? __sys_recvmsg+0x39/0x70
>> __sys_recvmsg+0x39/0x70
>> do_syscall_64+0x63/0x120
>> entry_SYSCALL_64_after_hwframe+0x3d/0xa2
>> RIP: 0033:0x7f2f03a72030
>> RSP: 002b:00007fffab3de508 EFLAGS: 00000246 ORIG_RAX: 000000000000002f
>> RAX: ffffffffffffffda RBX: 00007fffab3e641c RCX: 00007f2f03a72030
>> RDX: 0000000000000000 RSI: 00007fffab3de570 RDI: 0000000000000004
>> RBP: 0000000000000000 R08: 0000000000007e6c R09: 00007fffab3e63a8
>> R10: 00007fffab3de5b0 R11: 0000000000000246 R12: 00007fffab3e6608
>> R13: 000000000066b460 R14: 0000000000007e6c R15: 0000000000000000
>> Code: 85 d2 74 17 f6 40 2a 04 74 11 8b 53 2c 85 d2 0f 84 d7 00 00 00 83 ea 01 89 53 2c c7 4
>> RIP: fib6_walk_continue+0x5b/0x140 [ipv6] RSP: ffffc90008c3bc10
>> CR2: 0000000000000008
>> ---[ end trace bd03458864eb266c ]---
>>
>> Signed-off-by: Ben Greear <greearb@candelatech.com>
>> ---
>>
>
> Does your use case that triggers this involve replacing routes? I just
> noticed the route delete code in fib6_add_rt2node does not have the
> 'Adjust walkers' code that is in fib6_del_route.
>
> Further, the adjust walkers code in fib6_del_route looks suspicious in
> its timing with route deletes. If you have a reliable reproducer we can
> try a few things with fib6_del_route and the walker code.
Yes, we replace routes, and yes we can reliably reproduce it and will
be happy to test patches.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH net-next v8 1/7] sched: Add Common Applications Kept Enhanced (cake) qdisc
From: Cong Wang @ 2018-05-04 17:57 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Linux Kernel Network Developers, Cake List
In-Reply-To: <152544255298.11750.16512595539680113590.stgit@alrua-kau>
On Fri, May 4, 2018 at 7:02 AM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:
> +struct cake_sched_data {
> + struct cake_tin_data *tins;
> +
> + struct cake_heap_entry overflow_heap[CAKE_QUEUES * CAKE_MAX_TINS];
> + u16 overflow_timeout;
> +
> + u16 tin_cnt;
> + u8 tin_mode;
> + u8 flow_mode;
> + u8 ack_filter;
> + u8 atm_mode;
> +
> + /* time_next = time_this + ((len * rate_ns) >> rate_shft) */
> + u16 rate_shft;
> + u64 time_next_packet;
> + u64 failsafe_next_packet;
> + u32 rate_ns;
> + u32 rate_bps;
> + u16 rate_flags;
> + s16 rate_overhead;
> + u16 rate_mpu;
> + u32 interval;
> + u32 target;
> +
> + /* resource tracking */
> + u32 buffer_used;
> + u32 buffer_max_used;
> + u32 buffer_limit;
> + u32 buffer_config_limit;
> +
> + /* indices for dequeue */
> + u16 cur_tin;
> + u16 cur_flow;
> +
> + struct qdisc_watchdog watchdog;
> + const u8 *tin_index;
> + const u8 *tin_order;
> +
> + /* bandwidth capacity estimate */
> + u64 last_packet_time;
> + u64 avg_packet_interval;
> + u64 avg_window_begin;
> + u32 avg_window_bytes;
> + u32 avg_peak_bandwidth;
> + u64 last_reconfig_time;
> +
> + /* packet length stats */
> + u32 avg_netoff;
> + u16 max_netlen;
> + u16 max_adjlen;
> + u16 min_netlen;
> + u16 min_adjlen;
> +};
So sch_cake doesn't accept normal tc filters? Is this intentional?
If so, why?
> +static u16 quantum_div[CAKE_QUEUES + 1] = {0};
> +
> +#define REC_INV_SQRT_CACHE (16)
> +static u32 cobalt_rec_inv_sqrt_cache[REC_INV_SQRT_CACHE] = {0};
> +
> +/* http://en.wikipedia.org/wiki/Methods_of_computing_square_roots
> + * new_invsqrt = (invsqrt / 2) * (3 - count * invsqrt^2)
> + *
> + * Here, invsqrt is a fixed point number (< 1.0), 32bit mantissa, aka Q0.32
> + */
> +
> +static void cobalt_newton_step(struct cobalt_vars *vars)
> +{
> + u32 invsqrt = vars->rec_inv_sqrt;
> + u32 invsqrt2 = ((u64)invsqrt * invsqrt) >> 32;
> + u64 val = (3LL << 32) - ((u64)vars->count * invsqrt2);
> +
> + val >>= 2; /* avoid overflow in following multiply */
> + val = (val * invsqrt) >> (32 - 2 + 1);
> +
> + vars->rec_inv_sqrt = val;
> +}
> +
> +static void cobalt_invsqrt(struct cobalt_vars *vars)
> +{
> + if (vars->count < REC_INV_SQRT_CACHE)
> + vars->rec_inv_sqrt = cobalt_rec_inv_sqrt_cache[vars->count];
> + else
> + cobalt_newton_step(vars);
> +}
Looks pretty much duplicated with codel...
> +
> +/* There is a big difference in timing between the accurate values placed in
> + * the cache and the approximations given by a single Newton step for small
> + * count values, particularly when stepping from count 1 to 2 or vice versa.
> + * Above 16, a single Newton step gives sufficient accuracy in either
> + * direction, given the precision stored.
> + *
> + * The magnitude of the error when stepping up to count 2 is such as to give
> + * the value that *should* have been produced at count 4.
> + */
> +
> +static void cobalt_cache_init(void)
> +{
> + struct cobalt_vars v;
> +
> + memset(&v, 0, sizeof(v));
> + v.rec_inv_sqrt = ~0U;
> + cobalt_rec_inv_sqrt_cache[0] = v.rec_inv_sqrt;
> +
> + for (v.count = 1; v.count < REC_INV_SQRT_CACHE; v.count++) {
> + cobalt_newton_step(&v);
> + cobalt_newton_step(&v);
> + cobalt_newton_step(&v);
> + cobalt_newton_step(&v);
> +
> + cobalt_rec_inv_sqrt_cache[v.count] = v.rec_inv_sqrt;
> + }
> +}
> +
> +static void cobalt_vars_init(struct cobalt_vars *vars)
> +{
> + memset(vars, 0, sizeof(*vars));
> +
> + if (!cobalt_rec_inv_sqrt_cache[0]) {
> + cobalt_cache_init();
> + cobalt_rec_inv_sqrt_cache[0] = ~0;
> + }
> +}
> +
> +/* CoDel control_law is t + interval/sqrt(count)
> + * We maintain in rec_inv_sqrt the reciprocal value of sqrt(count) to avoid
> + * both sqrt() and divide operation.
> + */
> +static cobalt_time_t cobalt_control(cobalt_time_t t,
> + cobalt_time_t interval,
> + u32 rec_inv_sqrt)
> +{
> + return t + reciprocal_scale(interval, rec_inv_sqrt);
> +}
> +
> +/* Call this when a packet had to be dropped due to queue overflow. Returns
> + * true if the BLUE state was quiescent before but active after this call.
> + */
> +static bool cobalt_queue_full(struct cobalt_vars *vars,
> + struct cobalt_params *p,
> + cobalt_time_t now)
> +{
> + bool up = false;
> +
> + if ((now - vars->blue_timer) > p->target) {
> + up = !vars->p_drop;
> + vars->p_drop += p->p_inc;
> + if (vars->p_drop < p->p_inc)
> + vars->p_drop = ~0;
> + vars->blue_timer = now;
> + }
> + vars->dropping = true;
> + vars->drop_next = now;
> + if (!vars->count)
> + vars->count = 1;
> +
> + return up;
> +}
> +
> +/* Call this when the queue was serviced but turned out to be empty. Returns
> + * true if the BLUE state was active before but quiescent after this call.
> + */
> +static bool cobalt_queue_empty(struct cobalt_vars *vars,
> + struct cobalt_params *p,
> + cobalt_time_t now)
> +{
> + bool down = false;
> +
> + if (vars->p_drop && (now - vars->blue_timer) > p->target) {
> + if (vars->p_drop < p->p_dec)
> + vars->p_drop = 0;
> + else
> + vars->p_drop -= p->p_dec;
> + vars->blue_timer = now;
> + down = !vars->p_drop;
> + }
> + vars->dropping = false;
> +
> + if (vars->count && (now - vars->drop_next) >= 0) {
> + vars->count--;
> + cobalt_invsqrt(vars);
> + vars->drop_next = cobalt_control(vars->drop_next,
> + p->interval,
> + vars->rec_inv_sqrt);
> + }
> +
> + return down;
> +}
> +
> +/* Call this with a freshly dequeued packet for possible congestion marking.
> + * Returns true as an instruction to drop the packet, false for delivery.
> + */
> +static bool cobalt_should_drop(struct cobalt_vars *vars,
> + struct cobalt_params *p,
> + cobalt_time_t now,
> + struct sk_buff *skb)
> +{
> + bool drop = false;
> +
> + /* Simplified Codel implementation */
> + cobalt_tdiff_t sojourn = now - cobalt_get_enqueue_time(skb);
> +
> +/* The 'schedule' variable records, in its sign, whether 'now' is before or
> + * after 'drop_next'. This allows 'drop_next' to be updated before the next
> + * scheduling decision is actually branched, without destroying that
> + * information. Similarly, the first 'schedule' value calculated is preserved
> + * in the boolean 'next_due'.
> + *
> + * As for 'drop_next', we take advantage of the fact that 'interval' is both
> + * the delay between first exceeding 'target' and the first signalling event,
> + * *and* the scaling factor for the signalling frequency. It's therefore very
> + * natural to use a single mechanism for both purposes, and eliminates a
> + * significant amount of reference Codel's spaghetti code. To help with this,
> + * both the '0' and '1' entries in the invsqrt cache are 0xFFFFFFFF, as close
> + * as possible to 1.0 in fixed-point.
> + */
> +
> + cobalt_tdiff_t schedule = now - vars->drop_next;
> +
> + bool over_target = sojourn > p->target &&
> + sojourn > p->mtu_time * 4;
> + bool next_due = vars->count && schedule >= 0;
> +
> + vars->ecn_marked = false;
> +
> + if (over_target) {
> + if (!vars->dropping) {
> + vars->dropping = true;
> + vars->drop_next = cobalt_control(now,
> + p->interval,
> + vars->rec_inv_sqrt);
> + }
> + if (!vars->count)
> + vars->count = 1;
> + } else if (vars->dropping) {
> + vars->dropping = false;
> + }
> +
> + if (next_due && vars->dropping) {
> + /* Use ECN mark if possible, otherwise drop */
> + drop = !(vars->ecn_marked = INET_ECN_set_ce(skb));
> +
> + vars->count++;
> + if (!vars->count)
> + vars->count--;
> + cobalt_invsqrt(vars);
> + vars->drop_next = cobalt_control(vars->drop_next,
> + p->interval,
> + vars->rec_inv_sqrt);
> + schedule = now - vars->drop_next;
> + } else {
> + while (next_due) {
> + vars->count--;
> + cobalt_invsqrt(vars);
> + vars->drop_next = cobalt_control(vars->drop_next,
> + p->interval,
> + vars->rec_inv_sqrt);
> + schedule = now - vars->drop_next;
> + next_due = vars->count && schedule >= 0;
> + }
> + }
> +
> + /* Simple BLUE implementation. Lack of ECN is deliberate. */
> + if (vars->p_drop)
> + drop |= (prandom_u32() < vars->p_drop);
> +
> + /* Overload the drop_next field as an activity timeout */
> + if (!vars->count)
> + vars->drop_next = now + p->interval;
> + else if (schedule > 0 && !drop)
> + vars->drop_next = now;
> +
> + return drop;
> +}
> +
> +/* Cake has several subtle multiple bit settings. In these cases you
> + * would be matching triple isolate mode as well.
> + */
> +
> +static bool cake_dsrc(int flow_mode)
> +{
> + return (flow_mode & CAKE_FLOW_DUAL_SRC) == CAKE_FLOW_DUAL_SRC;
> +}
> +
> +static bool cake_ddst(int flow_mode)
> +{
> + return (flow_mode & CAKE_FLOW_DUAL_DST) == CAKE_FLOW_DUAL_DST;
> +}
> +
> +static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb,
> + int flow_mode)
> +{
> + struct flow_keys keys, host_keys;
> + u32 flow_hash = 0, srchost_hash, dsthost_hash;
> + u16 reduced_hash, srchost_idx, dsthost_idx;
> +
> + if (unlikely(flow_mode == CAKE_FLOW_NONE))
> + return 0;
> +
> + skb_flow_dissect_flow_keys(skb, &keys,
> + FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
> +
> + /* flow_hash_from_keys() sorts the addresses by value, so we have
> + * to preserve their order in a separate data structure to treat
> + * src and dst host addresses as independently selectable.
> + */
> + host_keys = keys;
> + host_keys.ports.ports = 0;
> + host_keys.basic.ip_proto = 0;
> + host_keys.keyid.keyid = 0;
> + host_keys.tags.flow_label = 0;
> +
> + switch (host_keys.control.addr_type) {
> + case FLOW_DISSECTOR_KEY_IPV4_ADDRS:
> + host_keys.addrs.v4addrs.src = 0;
> + dsthost_hash = flow_hash_from_keys(&host_keys);
> + host_keys.addrs.v4addrs.src = keys.addrs.v4addrs.src;
> + host_keys.addrs.v4addrs.dst = 0;
> + srchost_hash = flow_hash_from_keys(&host_keys);
> + break;
> +
> + case FLOW_DISSECTOR_KEY_IPV6_ADDRS:
> + memset(&host_keys.addrs.v6addrs.src, 0,
> + sizeof(host_keys.addrs.v6addrs.src));
> + dsthost_hash = flow_hash_from_keys(&host_keys);
> + host_keys.addrs.v6addrs.src = keys.addrs.v6addrs.src;
> + memset(&host_keys.addrs.v6addrs.dst, 0,
> + sizeof(host_keys.addrs.v6addrs.dst));
> + srchost_hash = flow_hash_from_keys(&host_keys);
> + break;
> +
> + default:
> + dsthost_hash = 0;
> + srchost_hash = 0;
> + }
> +
> + /* This *must* be after the above switch, since as a
> + * side-effect it sorts the src and dst addresses.
> + */
> + if (flow_mode & CAKE_FLOW_FLOWS)
> + flow_hash = flow_hash_from_keys(&keys);
> +
> + if (!(flow_mode & CAKE_FLOW_FLOWS)) {
> + if (flow_mode & CAKE_FLOW_SRC_IP)
> + flow_hash ^= srchost_hash;
> +
> + if (flow_mode & CAKE_FLOW_DST_IP)
> + flow_hash ^= dsthost_hash;
> + }
> +
> + reduced_hash = flow_hash % CAKE_QUEUES;
> +
> + /* set-associative hashing */
> + /* fast path if no hash collision (direct lookup succeeds) */
> + if (likely(q->tags[reduced_hash] == flow_hash &&
> + q->flows[reduced_hash].set)) {
> + q->way_directs++;
> + } else {
> + u32 inner_hash = reduced_hash % CAKE_SET_WAYS;
> + u32 outer_hash = reduced_hash - inner_hash;
> + u32 i, k;
> + bool allocate_src = false;
> + bool allocate_dst = false;
> +
> + /* check if any active queue in the set is reserved for
> + * this flow.
> + */
> + for (i = 0, k = inner_hash; i < CAKE_SET_WAYS;
> + i++, k = (k + 1) % CAKE_SET_WAYS) {
> + if (q->tags[outer_hash + k] == flow_hash) {
> + if (i)
> + q->way_hits++;
> +
> + if (!q->flows[outer_hash + k].set) {
> + /* need to increment host refcnts */
> + allocate_src = cake_dsrc(flow_mode);
> + allocate_dst = cake_ddst(flow_mode);
> + }
> +
> + goto found;
> + }
> + }
> +
> + /* no queue is reserved for this flow, look for an
> + * empty one.
> + */
> + for (i = 0; i < CAKE_SET_WAYS;
> + i++, k = (k + 1) % CAKE_SET_WAYS) {
> + if (!q->flows[outer_hash + k].set) {
> + q->way_misses++;
> + allocate_src = cake_dsrc(flow_mode);
> + allocate_dst = cake_ddst(flow_mode);
> + goto found;
> + }
> + }
> +
> + /* With no empty queues, default to the original
> + * queue, accept the collision, update the host tags.
> + */
> + q->way_collisions++;
> + q->hosts[q->flows[reduced_hash].srchost].srchost_refcnt--;
> + q->hosts[q->flows[reduced_hash].dsthost].dsthost_refcnt--;
> + allocate_src = cake_dsrc(flow_mode);
> + allocate_dst = cake_ddst(flow_mode);
> +found:
> + /* reserve queue for future packets in same flow */
> + reduced_hash = outer_hash + k;
> + q->tags[reduced_hash] = flow_hash;
> +
> + if (allocate_src) {
> + srchost_idx = srchost_hash % CAKE_QUEUES;
> + inner_hash = srchost_idx % CAKE_SET_WAYS;
> + outer_hash = srchost_idx - inner_hash;
> + for (i = 0, k = inner_hash; i < CAKE_SET_WAYS;
> + i++, k = (k + 1) % CAKE_SET_WAYS) {
> + if (q->hosts[outer_hash + k].srchost_tag ==
> + srchost_hash)
> + goto found_src;
> + }
> + for (i = 0; i < CAKE_SET_WAYS;
> + i++, k = (k + 1) % CAKE_SET_WAYS) {
> + if (!q->hosts[outer_hash + k].srchost_refcnt)
> + break;
> + }
> + q->hosts[outer_hash + k].srchost_tag = srchost_hash;
> +found_src:
> + srchost_idx = outer_hash + k;
> + q->hosts[srchost_idx].srchost_refcnt++;
> + q->flows[reduced_hash].srchost = srchost_idx;
> + }
> +
> + if (allocate_dst) {
> + dsthost_idx = dsthost_hash % CAKE_QUEUES;
> + inner_hash = dsthost_idx % CAKE_SET_WAYS;
> + outer_hash = dsthost_idx - inner_hash;
> + for (i = 0, k = inner_hash; i < CAKE_SET_WAYS;
> + i++, k = (k + 1) % CAKE_SET_WAYS) {
> + if (q->hosts[outer_hash + k].dsthost_tag ==
> + dsthost_hash)
> + goto found_dst;
> + }
> + for (i = 0; i < CAKE_SET_WAYS;
> + i++, k = (k + 1) % CAKE_SET_WAYS) {
> + if (!q->hosts[outer_hash + k].dsthost_refcnt)
> + break;
> + }
> + q->hosts[outer_hash + k].dsthost_tag = dsthost_hash;
> +found_dst:
> + dsthost_idx = outer_hash + k;
> + q->hosts[dsthost_idx].dsthost_refcnt++;
> + q->flows[reduced_hash].dsthost = dsthost_idx;
> + }
> + }
> +
> + return reduced_hash;
> +}
> +
> +/* helper functions : might be changed when/if skb use a standard list_head */
> +/* remove one skb from head of slot queue */
> +
> +static struct sk_buff *dequeue_head(struct cake_flow *flow)
> +{
> + struct sk_buff *skb = flow->head;
> +
> + if (skb) {
> + flow->head = skb->next;
> + skb->next = NULL;
> +
> + if (skb == flow->ackcheck)
> + flow->ackcheck = NULL;
> + }
> +
> + return skb;
> +}
> +
> +/* add skb to flow queue (tail add) */
> +
> +static void flow_queue_add(struct cake_flow *flow, struct sk_buff *skb)
> +{
> + if (!flow->head)
> + flow->head = skb;
> + else
> + flow->tail->next = skb;
> + flow->tail = skb;
> + skb->next = NULL;
> +}
> +
> +static cobalt_time_t cake_ewma(cobalt_time_t avg, cobalt_time_t sample,
> + u32 shift)
> +{
> + avg -= avg >> shift;
> + avg += sample >> shift;
> + return avg;
> +}
> +
> +static void cake_heap_swap(struct cake_sched_data *q, u16 i, u16 j)
> +{
> + struct cake_heap_entry ii = q->overflow_heap[i];
> + struct cake_heap_entry jj = q->overflow_heap[j];
> +
> + q->overflow_heap[i] = jj;
> + q->overflow_heap[j] = ii;
> +
> + q->tins[ii.t].overflow_idx[ii.b] = j;
> + q->tins[jj.t].overflow_idx[jj.b] = i;
> +}
> +
> +static u32 cake_heap_get_backlog(const struct cake_sched_data *q, u16 i)
> +{
> + struct cake_heap_entry ii = q->overflow_heap[i];
> +
> + return q->tins[ii.t].backlogs[ii.b];
> +}
> +
> +static void cake_heapify(struct cake_sched_data *q, u16 i)
> +{
> + static const u32 a = CAKE_MAX_TINS * CAKE_QUEUES;
> + u32 m = i;
> + u32 mb = cake_heap_get_backlog(q, m);
> +
> + while (m < a) {
> + u32 l = m + m + 1;
> + u32 r = l + 1;
> +
> + if (l < a) {
> + u32 lb = cake_heap_get_backlog(q, l);
> +
> + if (lb > mb) {
> + m = l;
> + mb = lb;
> + }
> + }
> +
> + if (r < a) {
> + u32 rb = cake_heap_get_backlog(q, r);
> +
> + if (rb > mb) {
> + m = r;
> + mb = rb;
> + }
> + }
> +
> + if (m != i) {
> + cake_heap_swap(q, i, m);
> + i = m;
> + } else {
> + break;
> + }
> + }
> +}
> +
> +static void cake_heapify_up(struct cake_sched_data *q, u16 i)
> +{
> + while (i > 0 && i < CAKE_MAX_TINS * CAKE_QUEUES) {
> + u16 p = (i - 1) >> 1;
> + u32 ib = cake_heap_get_backlog(q, i);
> + u32 pb = cake_heap_get_backlog(q, p);
> +
> + if (ib > pb) {
> + cake_heap_swap(q, i, p);
> + i = p;
> + } else {
> + break;
> + }
> + }
> +}
> +
> +static int cake_advance_shaper(struct cake_sched_data *q,
> + struct cake_tin_data *b,
> + struct sk_buff *skb,
> + u64 now, bool drop)
> +{
> + u32 len = qdisc_pkt_len(skb);
> +
> + /* charge packet bandwidth to this tin
> + * and to the global shaper.
> + */
> + if (q->rate_ns) {
> + s64 tdiff1 = b->tin_time_next_packet - now;
> + s64 tdiff2 = (len * (u64)b->tin_rate_ns) >> b->tin_rate_shft;
> + s64 tdiff3 = (len * (u64)q->rate_ns) >> q->rate_shft;
> + s64 tdiff4 = tdiff3 + (tdiff3 >> 1);
> +
> + if (tdiff1 < 0)
> + b->tin_time_next_packet += tdiff2;
> + else if (tdiff1 < tdiff2)
> + b->tin_time_next_packet = now + tdiff2;
> +
> + q->time_next_packet += tdiff3;
> + if (!drop)
> + q->failsafe_next_packet += tdiff4;
> + }
> + return len;
> +}
> +
> +static unsigned int cake_drop(struct Qdisc *sch, struct sk_buff **to_free)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + struct sk_buff *skb;
> + u32 idx = 0, tin = 0, len;
> + struct cake_tin_data *b;
> + struct cake_flow *flow;
> + struct cake_heap_entry qq;
> + u64 now = cobalt_get_time();
> +
> + if (!q->overflow_timeout) {
> + int i;
> + /* Build fresh max-heap */
> + for (i = CAKE_MAX_TINS * CAKE_QUEUES / 2; i >= 0; i--)
> + cake_heapify(q, i);
> + }
> + q->overflow_timeout = 65535;
> +
> + /* select longest queue for pruning */
> + qq = q->overflow_heap[0];
> + tin = qq.t;
> + idx = qq.b;
> +
> + b = &q->tins[tin];
> + flow = &b->flows[idx];
> + skb = dequeue_head(flow);
> + if (unlikely(!skb)) {
> + /* heap has gone wrong, rebuild it next time */
> + q->overflow_timeout = 0;
> + return idx + (tin << 16);
> + }
> +
> + if (cobalt_queue_full(&flow->cvars, &b->cparams, now))
> + b->unresponsive_flow_count++;
> +
> + len = qdisc_pkt_len(skb);
> + q->buffer_used -= skb->truesize;
> + b->backlogs[idx] -= len;
> + b->tin_backlog -= len;
> + sch->qstats.backlog -= len;
> + qdisc_tree_reduce_backlog(sch, 1, len);
> +
> + b->tin_dropped++;
> + sch->qstats.drops++;
> +
> + __qdisc_drop(skb, to_free);
> + sch->q.qlen--;
> +
> + cake_heapify(q, 0);
> +
> + return idx + (tin << 16);
> +}
> +
> +static void cake_reconfigure(struct Qdisc *sch);
> +
> +static s32 cake_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> + struct sk_buff **to_free)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + u32 idx, tin;
> + struct cake_tin_data *b;
> + struct cake_flow *flow;
> + /* signed len to handle corner case filtered ACK larger than trigger */
> + int len = qdisc_pkt_len(skb);
> + u64 now = cobalt_get_time();
> +
> + tin = 0;
> + b = &q->tins[tin];
> +
> + /* choose flow to insert into */
> + idx = cake_hash(b, skb, q->flow_mode);
> + flow = &b->flows[idx];
> +
> + /* ensure shaper state isn't stale */
> + if (!b->tin_backlog) {
> + if (b->tin_time_next_packet < now)
> + b->tin_time_next_packet = now;
> +
> + if (!sch->q.qlen) {
> + if (q->time_next_packet < now) {
> + q->failsafe_next_packet = now;
> + q->time_next_packet = now;
> + } else if (q->time_next_packet > now &&
> + q->failsafe_next_packet > now) {
> + u64 next = min(q->time_next_packet,
> + q->failsafe_next_packet);
> + sch->qstats.overlimits++;
> + qdisc_watchdog_schedule_ns(&q->watchdog, next);
> + }
> + }
> + }
> +
> + if (unlikely(len > b->max_skblen))
> + b->max_skblen = len;
> +
> + cobalt_set_enqueue_time(skb, now);
> + flow_queue_add(flow, skb);
> +
> + sch->q.qlen++;
> + q->buffer_used += skb->truesize;
> +
> + /* stats */
> + b->packets++;
> + b->bytes += len;
> + b->backlogs[idx] += len;
> + b->tin_backlog += len;
> + sch->qstats.backlog += len;
> + q->avg_window_bytes += len;
> +
> + if (q->overflow_timeout)
> + cake_heapify_up(q, b->overflow_idx[idx]);
> +
> + /* incoming bandwidth capacity estimate */
> + q->avg_window_bytes = 0;
> + q->last_packet_time = now;
> +
> + /* flowchain */
> + if (!flow->set || flow->set == CAKE_SET_DECAYING) {
> + struct cake_host *srchost = &b->hosts[flow->srchost];
> + struct cake_host *dsthost = &b->hosts[flow->dsthost];
> + u16 host_load = 1;
> +
> + if (!flow->set) {
> + list_add_tail(&flow->flowchain, &b->new_flows);
> + } else {
> + b->decaying_flow_count--;
> + list_move_tail(&flow->flowchain, &b->new_flows);
> + }
> + flow->set = CAKE_SET_SPARSE;
> + b->sparse_flow_count++;
> +
> + if (cake_dsrc(q->flow_mode))
> + host_load = max(host_load, srchost->srchost_refcnt);
> +
> + if (cake_ddst(q->flow_mode))
> + host_load = max(host_load, dsthost->dsthost_refcnt);
> +
> + flow->deficit = (b->flow_quantum *
> + quantum_div[host_load]) >> 16;
> + } else if (flow->set == CAKE_SET_SPARSE_WAIT) {
> + /* this flow was empty, accounted as a sparse flow, but actually
> + * in the bulk rotation.
> + */
> + flow->set = CAKE_SET_BULK;
> + b->sparse_flow_count--;
> + b->bulk_flow_count++;
> + }
> +
> + if (q->buffer_used > q->buffer_max_used)
> + q->buffer_max_used = q->buffer_used;
> +
> + if (q->buffer_used > q->buffer_limit) {
> + u32 dropped = 0;
> +
> + while (q->buffer_used > q->buffer_limit) {
> + dropped++;
> + cake_drop(sch, to_free);
> + }
> + b->drop_overlimit += dropped;
> + }
> + return NET_XMIT_SUCCESS;
> +}
> +
> +static struct sk_buff *cake_dequeue_one(struct Qdisc *sch)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + struct cake_tin_data *b = &q->tins[q->cur_tin];
> + struct cake_flow *flow = &b->flows[q->cur_flow];
> + struct sk_buff *skb = NULL;
> + u32 len;
> +
> + if (flow->head) {
> + skb = dequeue_head(flow);
> + len = qdisc_pkt_len(skb);
> + b->backlogs[q->cur_flow] -= len;
> + b->tin_backlog -= len;
> + sch->qstats.backlog -= len;
> + q->buffer_used -= skb->truesize;
> + sch->q.qlen--;
> +
> + if (q->overflow_timeout)
> + cake_heapify(q, b->overflow_idx[q->cur_flow]);
> + }
> + return skb;
> +}
> +
> +/* Discard leftover packets from a tin no longer in use. */
> +static void cake_clear_tin(struct Qdisc *sch, u16 tin)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + struct sk_buff *skb;
> +
> + q->cur_tin = tin;
> + for (q->cur_flow = 0; q->cur_flow < CAKE_QUEUES; q->cur_flow++)
> + while (!!(skb = cake_dequeue_one(sch)))
> + kfree_skb(skb);
> +}
> +
> +static struct sk_buff *cake_dequeue(struct Qdisc *sch)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + struct sk_buff *skb;
> + struct cake_tin_data *b = &q->tins[q->cur_tin];
> + struct cake_flow *flow;
> + struct cake_host *srchost, *dsthost;
> + struct list_head *head;
> + u32 len;
> + u16 host_load;
> + cobalt_time_t now = ktime_get_ns();
> + cobalt_time_t delay;
> + bool first_flow = true;
> +
> +begin:
> + if (!sch->q.qlen)
> + return NULL;
> +
> + /* global hard shaper */
> + if (q->time_next_packet > now && q->failsafe_next_packet > now) {
> + u64 next = min(q->time_next_packet, q->failsafe_next_packet);
> +
> + sch->qstats.overlimits++;
> + qdisc_watchdog_schedule_ns(&q->watchdog, next);
> + return NULL;
> + }
> +
> + /* Choose a class to work on. */
> + if (!q->rate_ns) {
> + /* In unlimited mode, can't rely on shaper timings, just balance
> + * with DRR
> + */
> + while (b->tin_deficit < 0 ||
> + !(b->sparse_flow_count + b->bulk_flow_count)) {
> + if (b->tin_deficit <= 0)
> + b->tin_deficit += b->tin_quantum_band;
> +
> + q->cur_tin++;
> + b++;
> + if (q->cur_tin >= q->tin_cnt) {
> + q->cur_tin = 0;
> + b = q->tins;
> + }
> + }
> + } else {
> + /* In shaped mode, choose:
> + * - Highest-priority tin with queue and meeting schedule, or
> + * - The earliest-scheduled tin with queue.
> + */
> + int tin, best_tin = 0;
> + s64 best_time = 0xFFFFFFFFFFFFUL;
> +
> + for (tin = 0; tin < q->tin_cnt; tin++) {
> + b = q->tins + tin;
> + if ((b->sparse_flow_count + b->bulk_flow_count) > 0) {
> + s64 tdiff = b->tin_time_next_packet - now;
> +
> + if (tdiff <= 0 || tdiff <= best_time) {
> + best_time = tdiff;
> + best_tin = tin;
> + }
> + }
> + }
> +
> + q->cur_tin = best_tin;
> + b = q->tins + best_tin;
> + }
> +
> +retry:
> + /* service this class */
> + head = &b->decaying_flows;
> + if (!first_flow || list_empty(head)) {
> + head = &b->new_flows;
> + if (list_empty(head)) {
> + head = &b->old_flows;
> + if (unlikely(list_empty(head))) {
> + head = &b->decaying_flows;
> + if (unlikely(list_empty(head)))
> + goto begin;
> + }
> + }
> + }
> + flow = list_first_entry(head, struct cake_flow, flowchain);
> + q->cur_flow = flow - b->flows;
> + first_flow = false;
> +
> + /* triple isolation (modified DRR++) */
> + srchost = &b->hosts[flow->srchost];
> + dsthost = &b->hosts[flow->dsthost];
> + host_load = 1;
> +
> + if (cake_dsrc(q->flow_mode))
> + host_load = max(host_load, srchost->srchost_refcnt);
> +
> + if (cake_ddst(q->flow_mode))
> + host_load = max(host_load, dsthost->dsthost_refcnt);
> +
> + WARN_ON(host_load > CAKE_QUEUES);
> +
> + /* flow isolation (DRR++) */
> + if (flow->deficit <= 0) {
> + /* The shifted prandom_u32() is a way to apply dithering to
> + * avoid accumulating roundoff errors
> + */
> + flow->deficit += (b->flow_quantum * quantum_div[host_load] +
> + (prandom_u32() >> 16)) >> 16;
> + list_move_tail(&flow->flowchain, &b->old_flows);
> +
> + /* Keep all flows with deficits out of the sparse and decaying
> + * rotations. No non-empty flow can go into the decaying
> + * rotation, so they can't get deficits
> + */
> + if (flow->set == CAKE_SET_SPARSE) {
> + if (flow->head) {
> + b->sparse_flow_count--;
> + b->bulk_flow_count++;
> + flow->set = CAKE_SET_BULK;
> + } else {
> + /* we've moved it to the bulk rotation for
> + * correct deficit accounting but we still want
> + * to count it as a sparse flow, not a bulk one.
> + */
> + flow->set = CAKE_SET_SPARSE_WAIT;
> + }
> + }
> + goto retry;
> + }
> +
> + /* Retrieve a packet via the AQM */
> + while (1) {
> + skb = cake_dequeue_one(sch);
> + if (!skb) {
> + /* this queue was actually empty */
> + if (cobalt_queue_empty(&flow->cvars, &b->cparams, now))
> + b->unresponsive_flow_count--;
> +
> + if (flow->cvars.p_drop || flow->cvars.count ||
> + now < flow->cvars.drop_next) {
> + /* keep in the flowchain until the state has
> + * decayed to rest
> + */
> + list_move_tail(&flow->flowchain,
> + &b->decaying_flows);
> + if (flow->set == CAKE_SET_BULK) {
> + b->bulk_flow_count--;
> + b->decaying_flow_count++;
> + } else if (flow->set == CAKE_SET_SPARSE ||
> + flow->set == CAKE_SET_SPARSE_WAIT) {
> + b->sparse_flow_count--;
> + b->decaying_flow_count++;
> + }
> + flow->set = CAKE_SET_DECAYING;
> + } else {
> + /* remove empty queue from the flowchain */
> + list_del_init(&flow->flowchain);
> + if (flow->set == CAKE_SET_SPARSE ||
> + flow->set == CAKE_SET_SPARSE_WAIT)
> + b->sparse_flow_count--;
> + else if (flow->set == CAKE_SET_BULK)
> + b->bulk_flow_count--;
> + else
> + b->decaying_flow_count--;
> +
> + flow->set = CAKE_SET_NONE;
> + srchost->srchost_refcnt--;
> + dsthost->dsthost_refcnt--;
> + }
> + goto begin;
> + }
> +
> + /* Last packet in queue may be marked, shouldn't be dropped */
> + if (!cobalt_should_drop(&flow->cvars, &b->cparams, now, skb) ||
> + !flow->head)
> + break;
> +
> + b->tin_dropped++;
> + qdisc_tree_reduce_backlog(sch, 1, qdisc_pkt_len(skb));
> + qdisc_qstats_drop(sch);
> + kfree_skb(skb);
> + }
> +
> + b->tin_ecn_mark += !!flow->cvars.ecn_marked;
> + qdisc_bstats_update(sch, skb);
> +
> + /* collect delay stats */
> + delay = now - cobalt_get_enqueue_time(skb);
> + b->avge_delay = cake_ewma(b->avge_delay, delay, 8);
> + b->peak_delay = cake_ewma(b->peak_delay, delay,
> + delay > b->peak_delay ? 2 : 8);
> + b->base_delay = cake_ewma(b->base_delay, delay,
> + delay < b->base_delay ? 2 : 8);
> +
> + len = cake_advance_shaper(q, b, skb, now, false);
> + flow->deficit -= len;
> + b->tin_deficit -= len;
> +
> + if (q->time_next_packet > now && sch->q.qlen) {
> + u64 next = min(q->time_next_packet, q->failsafe_next_packet);
> +
> + qdisc_watchdog_schedule_ns(&q->watchdog, next);
> + } else if (!sch->q.qlen) {
> + int i;
> +
> + for (i = 0; i < q->tin_cnt; i++) {
> + if (q->tins[i].decaying_flow_count) {
> + u64 next = now + q->tins[i].cparams.target;
> +
> + qdisc_watchdog_schedule_ns(&q->watchdog, next);
> + break;
> + }
> + }
> + }
> +
> + if (q->overflow_timeout)
> + q->overflow_timeout--;
> +
> + return skb;
> +}
> +
> +static void cake_reset(struct Qdisc *sch)
> +{
> + u32 c;
> +
> + for (c = 0; c < CAKE_MAX_TINS; c++)
> + cake_clear_tin(sch, c);
> +}
> +
> +static const struct nla_policy cake_policy[TCA_CAKE_MAX + 1] = {
> + [TCA_CAKE_BASE_RATE] = { .type = NLA_U32 },
> + [TCA_CAKE_DIFFSERV_MODE] = { .type = NLA_U32 },
> + [TCA_CAKE_ATM] = { .type = NLA_U32 },
> + [TCA_CAKE_FLOW_MODE] = { .type = NLA_U32 },
> + [TCA_CAKE_OVERHEAD] = { .type = NLA_S32 },
> + [TCA_CAKE_RTT] = { .type = NLA_U32 },
> + [TCA_CAKE_TARGET] = { .type = NLA_U32 },
> + [TCA_CAKE_AUTORATE] = { .type = NLA_U32 },
> + [TCA_CAKE_MEMORY] = { .type = NLA_U32 },
> + [TCA_CAKE_NAT] = { .type = NLA_U32 },
> + [TCA_CAKE_RAW] = { .type = NLA_U32 },
> + [TCA_CAKE_WASH] = { .type = NLA_U32 },
> + [TCA_CAKE_MPU] = { .type = NLA_U32 },
> + [TCA_CAKE_INGRESS] = { .type = NLA_U32 },
> + [TCA_CAKE_ACK_FILTER] = { .type = NLA_U32 },
> +};
> +
> +static void cake_set_rate(struct cake_tin_data *b, u64 rate, u32 mtu,
> + cobalt_time_t ns_target, cobalt_time_t rtt_est_ns)
> +{
> + /* convert byte-rate into time-per-byte
> + * so it will always unwedge in reasonable time.
> + */
> + static const u64 MIN_RATE = 64;
> + u64 rate_ns = 0;
> + u8 rate_shft = 0;
> + cobalt_time_t byte_target_ns;
> + u32 byte_target = mtu;
> +
> + b->flow_quantum = 1514;
> + if (rate) {
> + b->flow_quantum = max(min(rate >> 12, 1514ULL), 300ULL);
> + rate_shft = 32;
> + rate_ns = ((u64)NSEC_PER_SEC) << rate_shft;
> + do_div(rate_ns, max(MIN_RATE, rate));
> + while (!!(rate_ns >> 32)) {
> + rate_ns >>= 1;
> + rate_shft--;
> + }
> + } /* else unlimited, ie. zero delay */
> +
> + b->tin_rate_bps = rate;
> + b->tin_rate_ns = rate_ns;
> + b->tin_rate_shft = rate_shft;
> +
> + byte_target_ns = (byte_target * rate_ns) >> rate_shft;
> +
> + b->cparams.target = max((byte_target_ns * 3) / 2, ns_target);
> + b->cparams.interval = max(rtt_est_ns +
> + b->cparams.target - ns_target,
> + b->cparams.target * 2);
> + b->cparams.mtu_time = byte_target_ns;
> + b->cparams.p_inc = 1 << 24; /* 1/256 */
> + b->cparams.p_dec = 1 << 20; /* 1/4096 */
> +}
> +
> +static void cake_reconfigure(struct Qdisc *sch)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + struct cake_tin_data *b = &q->tins[0];
> + int c, ft = 0;
> +
> + q->tin_cnt = 1;
> + cake_set_rate(b, q->rate_bps, psched_mtu(qdisc_dev(sch)),
> + US2TIME(q->target), US2TIME(q->interval));
> + b->tin_quantum_band = 65535;
> + b->tin_quantum_prio = 65535;
> +
> + for (c = q->tin_cnt; c < CAKE_MAX_TINS; c++) {
> + cake_clear_tin(sch, c);
> + q->tins[c].cparams.mtu_time = q->tins[ft].cparams.mtu_time;
> + }
> +
> + q->rate_ns = q->tins[ft].tin_rate_ns;
> + q->rate_shft = q->tins[ft].tin_rate_shft;
> +
> + if (q->buffer_config_limit) {
> + q->buffer_limit = q->buffer_config_limit;
> + } else if (q->rate_bps) {
> + u64 t = (u64)q->rate_bps * q->interval;
> +
> + do_div(t, USEC_PER_SEC / 4);
> + q->buffer_limit = max_t(u32, t, 4U << 20);
> + } else {
> + q->buffer_limit = ~0;
> + }
> +
> + sch->flags &= ~TCQ_F_CAN_BYPASS;
> +
> + q->buffer_limit = min(q->buffer_limit,
> + max(sch->limit * psched_mtu(qdisc_dev(sch)),
> + q->buffer_config_limit));
> +}
> +
> +static int cake_change(struct Qdisc *sch, struct nlattr *opt,
> + struct netlink_ext_ack *extack)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + struct nlattr *tb[TCA_CAKE_MAX + 1];
> + int err;
> +
> + if (!opt)
> + return -EINVAL;
> +
> + err = nla_parse_nested(tb, TCA_CAKE_MAX, opt, cake_policy, extack);
> + if (err < 0)
> + return err;
> +
> + if (tb[TCA_CAKE_BASE_RATE])
> + q->rate_bps = nla_get_u32(tb[TCA_CAKE_BASE_RATE]);
> +
> + if (tb[TCA_CAKE_FLOW_MODE])
> + q->flow_mode = (nla_get_u32(tb[TCA_CAKE_FLOW_MODE]) &
> + CAKE_FLOW_MASK);
> +
> + if (tb[TCA_CAKE_RTT]) {
> + q->interval = nla_get_u32(tb[TCA_CAKE_RTT]);
> +
> + if (!q->interval)
> + q->interval = 1;
> + }
> +
> + if (tb[TCA_CAKE_TARGET]) {
> + q->target = nla_get_u32(tb[TCA_CAKE_TARGET]);
> +
> + if (!q->target)
> + q->target = 1;
> + }
> +
> + if (tb[TCA_CAKE_MEMORY])
> + q->buffer_config_limit = nla_get_u32(tb[TCA_CAKE_MEMORY]);
> +
> + if (q->tins) {
> + sch_tree_lock(sch);
> + cake_reconfigure(sch);
> + sch_tree_unlock(sch);
> + }
> +
> + return 0;
> +}
> +
> +static void cake_free(void *addr)
> +{
> + if (addr)
> + kvfree(addr);
> +}
Are you sure you have to check NULL for kvfree()?
> +
> +static void cake_destroy(struct Qdisc *sch)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> +
> + qdisc_watchdog_cancel(&q->watchdog);
> +
> + if (q->tins)
> + cake_free(q->tins);
Duplicated NULL check.
> +}
> +
> +static int cake_init(struct Qdisc *sch, struct nlattr *opt,
> + struct netlink_ext_ack *extack)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + int i, j;
> +
> + sch->limit = 10240;
> + q->tin_mode = CAKE_DIFFSERV_BESTEFFORT;
> + q->flow_mode = CAKE_FLOW_TRIPLE;
> +
> + q->rate_bps = 0; /* unlimited by default */
> +
> + q->interval = 100000; /* 100ms default */
> + q->target = 5000; /* 5ms: codel RFC argues
> + * for 5 to 10% of interval
> + */
> +
> + q->cur_tin = 0;
> + q->cur_flow = 0;
> +
> + if (opt) {
> + int err = cake_change(sch, opt, extack);
> +
> + if (err)
> + return err;
Not sure if you really want to reallocate q->tines below for this
case.
> + }
> +
> + qdisc_watchdog_init(&q->watchdog, sch);
> +
> + quantum_div[0] = ~0;
> + for (i = 1; i <= CAKE_QUEUES; i++)
> + quantum_div[i] = 65535 / i;
> +
> + q->tins = kvzalloc(CAKE_MAX_TINS * sizeof(struct cake_tin_data),
> + GFP_KERNEL | __GFP_NOWARN);
Why __GFP_NOWARN?
> + if (!q->tins)
> + goto nomem;
> +
> + for (i = 0; i < CAKE_MAX_TINS; i++) {
> + struct cake_tin_data *b = q->tins + i;
> +
> + INIT_LIST_HEAD(&b->new_flows);
> + INIT_LIST_HEAD(&b->old_flows);
> + INIT_LIST_HEAD(&b->decaying_flows);
> + b->sparse_flow_count = 0;
> + b->bulk_flow_count = 0;
> + b->decaying_flow_count = 0;
> +
> + for (j = 0; j < CAKE_QUEUES; j++) {
> + struct cake_flow *flow = b->flows + j;
> + u32 k = j * CAKE_MAX_TINS + i;
> +
> + INIT_LIST_HEAD(&flow->flowchain);
> + cobalt_vars_init(&flow->cvars);
> +
> + q->overflow_heap[k].t = i;
> + q->overflow_heap[k].b = j;
> + b->overflow_idx[j] = k;
> + }
> + }
> +
> + cake_reconfigure(sch);
> + q->avg_peak_bandwidth = q->rate_bps;
> + q->min_netlen = ~0;
> + q->min_adjlen = ~0;
> + return 0;
> +
> +nomem:
> + cake_destroy(sch);
> + return -ENOMEM;
> +}
> +
> +static int cake_dump(struct Qdisc *sch, struct sk_buff *skb)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + struct nlattr *opts;
> +
> + opts = nla_nest_start(skb, TCA_OPTIONS);
> + if (!opts)
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_BASE_RATE, q->rate_bps))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_DIFFSERV_MODE, q->tin_mode))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_ATM, q->atm_mode))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_FLOW_MODE,
> + q->flow_mode & CAKE_FLOW_MASK))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_NAT,
> + !!(q->flow_mode & CAKE_FLOW_NAT_FLAG)))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_SPLIT_GSO,
> + !!(q->rate_flags & CAKE_FLAG_SPLIT_GSO)))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_WASH,
> + !!(q->rate_flags & CAKE_FLAG_WASH)))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_OVERHEAD, q->rate_overhead))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_MPU, q->rate_mpu))
> + goto nla_put_failure;
> +
> + if (!(q->rate_flags & CAKE_FLAG_OVERHEAD))
> + if (nla_put_u32(skb, TCA_CAKE_RAW, 0))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_RTT, q->interval))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_TARGET, q->target))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_AUTORATE,
> + !!(q->rate_flags & CAKE_FLAG_AUTORATE_INGRESS)))
> + goto nla_put_failure;
> +
> + if (nla_put_u32(skb, TCA_CAKE_INGRESS,
> + !!(q->rate_flags & CAKE_FLAG_INGRESS)))
> + goto nla_put_failure;
Is this used by a following patch? If so, please move it there.
> +
> + if (nla_put_u32(skb, TCA_CAKE_ACK_FILTER, q->ack_filter))
> + goto nla_put_failure;
Ditto.
> +
> + if (nla_put_u32(skb, TCA_CAKE_MEMORY, q->buffer_config_limit))
> + goto nla_put_failure;
> +
> + return nla_nest_end(skb, opts);
> +
> +nla_put_failure:
> + return -1;
> +}
> +
> +static int cake_dump_stats(struct Qdisc *sch, struct gnet_dump *d)
> +{
> + struct cake_sched_data *q = qdisc_priv(sch);
> + struct nlattr *stats = nla_nest_start(d->skb, TCA_STATS_APP);
> + struct nlattr *tstats, *ts;
> + int i;
> +
> + if (!stats)
> + return -1;
> +
> +#define PUT_STAT_U32(attr, data) do { \
> + if (nla_put_u32(d->skb, TCA_CAKE_STATS_ ## attr, data)) \
> + goto nla_put_failure; \
> + } while (0)
> +
> + PUT_STAT_U32(CAPACITY_ESTIMATE, q->avg_peak_bandwidth);
> + PUT_STAT_U32(MEMORY_LIMIT, q->buffer_limit);
> + PUT_STAT_U32(MEMORY_USED, q->buffer_max_used);
> + PUT_STAT_U32(AVG_NETOFF, ((q->avg_netoff + 0x8000) >> 16));
> + PUT_STAT_U32(MAX_NETLEN, q->max_netlen);
> + PUT_STAT_U32(MAX_ADJLEN, q->max_adjlen);
> + PUT_STAT_U32(MIN_NETLEN, q->min_netlen);
> + PUT_STAT_U32(MIN_ADJLEN, q->min_adjlen);
> +
> +#undef PUT_STAT_U32
> +
> + tstats = nla_nest_start(d->skb, TCA_CAKE_STATS_TIN_STATS);
> + if (!tstats)
> + goto nla_put_failure;
> +
> +#define PUT_TSTAT_U32(attr, data) do { \
> + if (nla_put_u32(d->skb, TCA_CAKE_TIN_STATS_ ## attr, data)) \
> + goto nla_put_failure; \
> + } while (0)
> +#define PUT_TSTAT_U64(attr, data) do { \
> + if (nla_put_u64_64bit(d->skb, TCA_CAKE_TIN_STATS_ ## attr, \
> + data, TCA_CAKE_TIN_STATS_PAD)) \
> + goto nla_put_failure; \
> + } while (0)
> +
> + for (i = 0; i < q->tin_cnt; i++) {
> + struct cake_tin_data *b = &q->tins[i];
> +
> + ts = nla_nest_start(d->skb, i + 1);
> + if (!ts)
> + goto nla_put_failure;
> +
> + PUT_TSTAT_U32(THRESHOLD_RATE, b->tin_rate_bps);
> + PUT_TSTAT_U32(TARGET_US, cobalt_time_to_us(b->cparams.target));
> + PUT_TSTAT_U32(INTERVAL_US,
> + cobalt_time_to_us(b->cparams.interval));
> +
> + PUT_TSTAT_U32(SENT_PACKETS, b->packets);
> + PUT_TSTAT_U64(SENT_BYTES64, b->bytes);
> + PUT_TSTAT_U32(DROPPED_PACKETS, b->tin_dropped);
> + PUT_TSTAT_U32(ECN_MARKED_PACKETS, b->tin_ecn_mark);
> + PUT_TSTAT_U64(BACKLOG_BYTES64, b->tin_backlog);
> + PUT_TSTAT_U32(ACKS_DROPPED_PACKETS, b->ack_drops);
> +
> + PUT_TSTAT_U32(PEAK_DELAY_US, cobalt_time_to_us(b->peak_delay));
> + PUT_TSTAT_U32(AVG_DELAY_US, cobalt_time_to_us(b->avge_delay));
> + PUT_TSTAT_U32(BASE_DELAY_US, cobalt_time_to_us(b->base_delay));
> +
> + PUT_TSTAT_U32(WAY_INDIRECT_HITS, b->way_hits);
> + PUT_TSTAT_U32(WAY_MISSES, b->way_misses);
> + PUT_TSTAT_U32(WAY_COLLISIONS, b->way_collisions);
> +
> + PUT_TSTAT_U32(SPARSE_FLOWS, b->sparse_flow_count +
> + b->decaying_flow_count);
> + PUT_TSTAT_U32(BULK_FLOWS, b->bulk_flow_count);
> + PUT_TSTAT_U32(UNRESPONSIVE_FLOWS, b->unresponsive_flow_count);
> + PUT_TSTAT_U32(MAX_SKBLEN, b->max_skblen);
> +
> + PUT_TSTAT_U32(FLOW_QUANTUM, b->flow_quantum);
> + nla_nest_end(d->skb, ts);
> + }
> +
> +#undef PUT_TSTAT_U32
> +#undef PUT_TSTAT_U64
> +
> + nla_nest_end(d->skb, tstats);
> + return nla_nest_end(d->skb, stats);
> +
> +nla_put_failure:
> + nla_nest_cancel(d->skb, stats);
> + return -1;
> +}
> +
> +static struct Qdisc_ops cake_qdisc_ops __read_mostly = {
> + .id = "cake",
> + .priv_size = sizeof(struct cake_sched_data),
> + .enqueue = cake_enqueue,
> + .dequeue = cake_dequeue,
> + .peek = qdisc_peek_dequeued,
> + .init = cake_init,
> + .reset = cake_reset,
> + .destroy = cake_destroy,
> + .change = cake_change,
> + .dump = cake_dump,
> + .dump_stats = cake_dump_stats,
> + .owner = THIS_MODULE,
> +};
> +
> +static int __init cake_module_init(void)
> +{
> + return register_qdisc(&cake_qdisc_ops);
> +}
> +
> +static void __exit cake_module_exit(void)
> +{
> + unregister_qdisc(&cake_qdisc_ops);
> +}
> +
> +module_init(cake_module_init)
> +module_exit(cake_module_exit)
> +MODULE_AUTHOR("Jonathan Morton");
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_DESCRIPTION("The CAKE shaper.");
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox