* Re: [PATCH ethtool] ethtool: Fix uninitialized variable use at qsfp dump
From: Eran Ben Elisha @ 2018-10-02 7:17 UTC (permalink / raw)
To: Andrew Lunn
Cc: netdev@vger.kernel.org, John W. Linville, Chris Preimesberger,
Neil Horman, Vidya Sagar Ravipati
In-Reply-To: <20180927160817.GH12979@lunn.ch>
On 9/27/2018 7:08 PM, Andrew Lunn wrote:
> On Thu, Sep 27, 2018 at 04:48:35PM +0300, Eran Ben Elisha wrote:
>> Struct sff_diags can be used uninitialized at sff8636_show_dom, this
>> caused the tool to show unreported fields (supports_alarms) by the lower
>> level driver.
>>
>> Fixes: a5e73bb05ee4 ("ethtool:QSFP Plus/QSFP28 Diagnostics Information Support")
>> Signed-off-by: Eran Ben Elisha <eranbe@mellanox.com>
>> ---
>> qsfp.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/qsfp.c b/qsfp.c
>> index 32e195d12dc0..d196aa1753de 100644
>> --- a/qsfp.c
>> +++ b/qsfp.c
>> @@ -671,7 +671,7 @@ static void sff8636_dom_parse(const __u8 *id, struct sff_diags *sd)
>>
>> static void sff8636_show_dom(const __u8 *id, __u32 eeprom_len)
>> {
>> - struct sff_diags sd;
>> + struct sff_diags sd = {0};
>> char *rx_power_string = NULL;
>> char power_string[MAX_DESC_SIZE];
>> int i;
>
> Hi Erin
>
> I wonder if similar bugs happen in sfpdiag.c?
Bug is not present here, all sd needed fields are being filled in
sff8472_parse_eeprom right at the beginning.
>
> void sff8472_show_all(const __u8 *id)
> {
> struct sff_diags sd;
> char *rx_power_string = NULL;
> int i;
>
> sd is also on the stack and not initialised. Could you expand your
> patch to also clear this one as well, just to be safe.
Although bug is not present, I agree that better initialize it to be on
the safe side. will post soon.
Eran
>
> Thanks
> Andrew
>
^ permalink raw reply
* Re: [PATCH net-next v6 01/23] asm: simd context helper API
From: Ard Biesheuvel @ 2018-10-02 7:18 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Andy Lutomirski, Joe Perches, LKML, Netdev,
Linux Crypto Mailing List, David Miller, Greg Kroah-Hartman,
Samuel Neves, Andrew Lutomirski, Thomas Gleixner, linux-arch
In-Reply-To: <CAHmME9o7_Xh=dZfnEHbPCJdqzXW1L5R7qjg=_sqPZiP2KmTc1Q@mail.gmail.com>
On 1 October 2018 at 03:43, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> On Sun, Sep 30, 2018 at 7:35 AM Andy Lutomirski <luto@amacapital.net> wrote:
>> >>>>>>> Oh, and another thing (and I'm surprised checkpatch.pl didn't complain
>> >>>>>>> about it): the use of typedef in new code is strongly discouraged.
>> >>>>>>> This policy predates my involvement, so perhaps Joe can elaborate on
>> >>>>>>> the rationale?
>> >>>>>>
>> >>>>>> In case it matters, the motivation for making this a typedef is I
>> >>>>>> could imagine this at some point turning into a more complicated
>> >>>>>> struct on certain platforms and that would make refactoring easier. I
>> >>>>>> could just make it `struct simd_context` now with 1 member though...
>> >>>>>
>> >>>>> Yes that makes sense
>> >>>>
>> >>>> The rationale for it being a typedef or moving to a struct now?
>> >>>
>> >>> Yes just switch to a struct.
>> >>
>> >> Okay. No problem with that, but will wait to hear from Joe first.
>> >
>> > Why do you need to hear from me again?
>> >
>> > As far as I know, the only info about typedef avoidance are in
>> > Documentation/process/coding-style.rst section 5.
>> >
>> >
>>
>> I personally prefer it with the typedef. If this were my code, I’d say the coding style is silly for opaque tiny structs like this.
>
> I'll stick with a typedef. Reading the style guide, this clearly falls
> into 5.a, 5.b, and maybe 5.c. For 5.a, at some point this will
> possibly contain architecture specific blobs. For 5.b, it is just an
> enum (integer) right now.
I can live with that, but other may still object.
In any case, since you are using the enum member as a bitfield, it
would be better to typedef it to int instead, and retain the enum
definition only for the symbolic constants.
^ permalink raw reply
* Re: bug: 'ethtool -m' reports spurious alarm & warning threshold values for QSFP28 transceivers
From: Eran Ben Elisha @ 2018-10-02 7:10 UTC (permalink / raw)
To: Andrew Lunn
Cc: Eran Ben Elisha, nhorman, chrisp, John W. Linville,
Linux Netdev List
In-Reply-To: <20180927153204.GE12979@lunn.ch>
On Thu, Sep 27, 2018 at 6:34 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> > Driver return 256 bytes (reading it correctly, I verified it, no overruns),
> > however the extra bytes are presented due to this bug (expecting to parse
> > 640 bytes).
> >
> > Do you see another bug here? Am I missing something?
>
> Hi Erin
Eran...
>
> Please could you try ethtool -m raw on so you get a binary dump. The
> file which Chris provided had more bytes in it than 256.
I ran '-m raw on' on QSFP28.
File size is 256 bytes.
(with and without my suggested patch...)
Eran
>
> Thanks
> Andrew
^ permalink raw reply
* Re: [PATCH net-next 1/5] net: phy: mscc: add ethtool statistics counters
From: Quentin Schulz @ 2018-10-02 13:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: davem, f.fainelli, allan.nielsen, linux-kernel, netdev,
thomas.petazzoni, Raju Lakkaraju, rmk+kernel
In-Reply-To: <20180914132959.GH14865@lunn.ch>
[-- Attachment #1: Type: text/plain, Size: 3972 bytes --]
Hi Russel,
Adding you to the discussion as you're the author and commiter of the
patch adding support for all the paged access in the phy core.
On Fri, Sep 14, 2018 at 03:29:59PM +0200, Andrew Lunn wrote:
> > When you change a page, you basically can access only the registers in
> > this page so if there are two functions requesting different pages at
> > the same time or registers of different pages, it won't work well
> > indeed.
> >
> > > phy_read_page() and phy_write_page() will do the needed locking if
> > > this is an issue.
> > >
> >
> > That's awesome! Didn't know it existed. Thanks a ton!
> >
> > Well, that means I should migrate the whole driver to use
> > phy_read/write_paged instead of the phy_read/write that is currently in
> > use.
> >
> > That's impacting performance though as per phy_read/write_paged we read
> > the current page, set the desired page, read/write the register, set the
> > old page back. That's 4 times more operations.
>
> You can use the lower level locking primatives. See m88e1318_set_wol()
> for example.
>
I'm converting the drivers/net/phy/mscc.c driver to make use of the
paged accesses but I'm hitting something confusing to me.
Firstly, just to be sure, I should use paged accesses only for read/write
outside of the standard page, right? I'm guessing that because we need
to be able to use the genphy functions which are using phy_write/read
and not __phy_write/read, thus we assume the mdio lock is not taken
(which is taken by phy_select/restore_page) and those functions
read/write to the standard page.
Secondly, I should refactor the driver to do the following:
oldpage = phy_select_page();
if (oldpage < 0) {
phy_restore_page();
error_path;
}
[...]
/* paged accesses */
__phy_write/read();
[...]
phy_restore_page();
I assume this is the correct way to handle paged accesses. Let me know
if it's not clear enough or wrong. (depending on the function, we could
of course put phy_restore_page in the error_path).
Now, I saw that phy_restore_page takes the phydev, the oldpage and a ret
parameters[1].
The (ret >= 0 && r < 0) condition of [2] seems counterintuitive to me.
ret being the argument passed to the function and r being the return of
__phy_write_page (which is phydev->drv->phy_write_page()).
In my understanding of C best practices, any return value equal to zero
marks a successful call to the function.
That would mean that with:
if (ret >= 0 && r < 0)
ret = r;
If ret is greather than 0, if __phy_write_page is successful (r == 0),
ret will be > 0, which would result in phy_restore_page not returning 0
thus signaling (IMO) an error occured in phy_restore_page.
One example is the following:
oldpage = phy_select_page(phydev, new_page);
[...]
return phy_restore_page(phydev, oldpage, oldpage);
If phy_select_page is successful, return phy_restore_page(phydev,
oldpage, oldpage) would return the value of oldpage which can be
different from 0.
This code could (I think) be working with `if (ret >= 0 && r <= 0)` (or
even `if (ret >= 0)`).
Now to have the same behaviour, I need to do:
oldpage = phy_select_page(phydev, new_page);
[...]
return phy_restore_page(phydev, oldpage, oldpage > 0 ? 0 : oldpage);
Another example is:
oldpage = phy_select_page(phydev, new_page);
ret = `any function returning a value > 0 in case of success and < 0 in
case of failure`().
return phy_restore_page(phydev, oldpage, ret);
Is there any reason for not wanting to overwrite the ret value when
__phy_write_page is successful in phy_restore_page?
I'd say that it could be more readable without the ternary condition in
the argument of phy_restore_page.
Let me know your thoughts on this.
Thanks,
Quentin
[1] https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy-core.c#L444
[2] https://elixir.bootlin.com/linux/latest/source/drivers/net/phy/phy-core.c#L454
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] net: mscc: allow extracting the FCS into the skb
From: Antoine Tenart @ 2018-10-02 6:59 UTC (permalink / raw)
To: Florian Fainelli
Cc: Antoine Tenart, davem, netdev, linux-kernel, thomas.petazzoni,
alexandre.belloni, quentin.schulz, allan.nielsen
In-Reply-To: <25a88126-2397-a1fb-0c0d-06504bb348ca@gmail.com>
Hi FLorian,
On Mon, Oct 01, 2018 at 09:35:45AM -0700, Florian Fainelli wrote:
> On 10/01/2018 02:57 AM, Antoine Tenart wrote:
> >
> > memcpy(dev->dev_addr, ocelot->base_mac, ETH_ALEN);
> > diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
> > index 3cdf63e35b53..245452a0f244 100644
> > --- a/drivers/net/ethernet/mscc/ocelot_board.c
> > +++ b/drivers/net/ethernet/mscc/ocelot_board.c
> > @@ -126,11 +126,16 @@ static irqreturn_t ocelot_xtr_irq_handler(int irq, void *arg)
> > len += sz;
> > } while (len < buf_len);
> >
> > - /* Read the FCS and discard it */
> > + /* Read the FCS */
> > sz = ocelot_rx_frame_word(ocelot, grp, false, &val);
> > /* Update the statistics if part of the FCS was read before */
> > len -= ETH_FCS_LEN - sz;
> >
> Don't this needs to be len -= sz;
No, part of the FCS could be read in the loop before. This subtraction
removes those bytes from the stats.
> > + if (unlikely(dev->features & NETIF_F_RXFCS)) {
> > + buf = (u32 *)skb_put(skb, ETH_FCS_LEN);
> > + *buf = val;
>
> and here len -= ETH_FCS_LEN
len doesn't contain the FCS len at this point, no need to remove it
again. The extraction logic is already designed to drop the FCS and to
not count it in the stats, this patch makes use of it depending on
NETIF_F_RXFCS.
> since "len" is later used for accounting how many bytes have been
> received by the network device?
The question could be "do we need len += ETH_FCS_LEN" to account for the
FCS when NETIF_F_RXFCS is used", but I looked at other drivers and it
seemed to me the FCS is not accounted in the stats. Should it be?
> I am bit confused by the use of "buf", but presumably if NETIF_F_RXCFS
> is turned on, the FCS needs to be part of the buffer passed to the
> network stack, so adjusting len accordingly is required anyway.
This is the way we extract the data to the skb, I'm following the same
logic (I could use buf++ as well, as it's done in the loop before).
Thanks!
Antoine
--
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH 2/4] usbnet: smsc95xx: align tx-buffer to word
From: Ben Dooks @ 2018-10-02 13:35 UTC (permalink / raw)
To: David Laight, netdev@vger.kernel.org
Cc: oneukum@suse.com, davem@davemloft.net, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kernel@lists.codethink.co.uk
In-Reply-To: <59988ed22559410881addfecf58335eb@AcuMS.aculab.com>
On 02/10/18 14:19, David Laight wrote:
> From: Ben Dooks
>> Sent: 02 October 2018 10:27
>>
>> The tegra driver requires alignment of the buffer, so try and
>> make this better by pushing the buffer start back to an word
>> aligned address. At the worst this makes memcpy() easier as
>> it is word aligned, at best it makes sure the usb can directly
>> map the buffer.
>>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> [todo - make this configurable]
>> ---
>> drivers/net/usb/Kconfig | 12 ++++++++++++
>> drivers/net/usb/smsc95xx.c | 22 ++++++++++++++++++++--
>> 2 files changed, 32 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> ...
>> +static bool align_tx = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TXALIGN);
>> +module_param(align_tx, bool, 0644);
>> +MODULE_PARM_DESC(align_tx, "Align TX buffers to word boundaries");
>
> DM doesn't like module parameters.
>
>> static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
>> module_param(turbo_mode, bool, 0644);
>> MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
>> @@ -2005,10 +2009,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
>> bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
>> int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
>> u32 tx_cmd_a, tx_cmd_b;
>> + u32 data_len;
>> + uintptr_t align = 0;
>>
>> /* We do not advertise SG, so skbs should be already linearized */
>> BUG_ON(skb_shinfo(skb)->nr_frags);
>>
>> + if (IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TXALIGN) && align_tx) {
>> + align = (uintptr_t)skb->data & 3;
>> + if (align)
>> + overhead += 4 - align;
>
> Better to calculate the pad size once:
> align = (-(long)skb->data) & 3;
> should do it - and you can unconditionally add it in.
>
>> + }
>> +
>> /* Make writable and expand header space by overhead if required */
>> if (skb_cow_head(skb, overhead)) {
>> /* Must deallocate here as returning NULL to indicate error
>> @@ -2037,16 +2049,22 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
>> }
>> }
>>
>> + data_len = skb->len;
>> + if (align)
>> + skb_push(skb, 4 - align);
>> +
>> skb_push(skb, 4);
>
> You don't want to call skb_push() twice.
> IIRC really horrid things happen if the data has to be copied.
> (Actually what happens to the alignment in that case??)
> And there is another skb_push() below....
The driver does it /multiple/ times depending on the path used.
Is it wise to try and make a separate patch to skb_push() once
and also move the tx_cmd_a and tx_cmd_b bit to a single point?
>> - tx_cmd_b = (u32)(skb->len - 4);
>> + tx_cmd_b = (u32)(data_len);
>
> You don't need the cast here at all (if it was ever needed).
> Actually you don't need the new 'data_len' variable.
> Just set tx_cmd_b earlier.
>
> ...
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
>
>
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
https://www.codethink.co.uk/privacy.html
^ permalink raw reply
* [PATCH] declance: Fix continuation with the adapter identification message
From: Maciej W. Rozycki @ 2018-10-02 13:23 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
Fix a commit 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing
continuation lines") regression with the `declance' driver, which caused
the adapter identification message to be split between two lines, e.g.:
declance.c: v0.011 by Linux MIPS DECstation task force
tc6: PMAD-AA
, addr = 08:00:2b:1b:2a:6a, irq = 14
tc6: registered as eth0.
Address that properly, by printing identification with a single call,
making the messages now look like:
declance.c: v0.011 by Linux MIPS DECstation task force
tc6: PMAD-AA, addr = 08:00:2b:1b:2a:6a, irq = 14
tc6: registered as eth0.
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
Fixes: 4bcc595ccd80 ("printk: reinstate KERN_CONT for printing continuation lines")
---
Hi,
Not requesting a backport as the impact of this issue is purely cosmetic.
Deferring driver review to use `netdev_info', etc. in place of `printk' to
another time. Please apply.
Maciej
---
drivers/net/ethernet/amd/declance.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
linux-net-declance-pr-cont.patch
Index: linux-20180930-4maxp64/drivers/net/ethernet/amd/declance.c
===================================================================
--- linux-20180930-4maxp64.orig/drivers/net/ethernet/amd/declance.c
+++ linux-20180930-4maxp64/drivers/net/ethernet/amd/declance.c
@@ -1031,6 +1031,7 @@ static int dec_lance_probe(struct device
int i, ret;
unsigned long esar_base;
unsigned char *esar;
+ const char *desc;
if (dec_lance_debug && version_printed++ == 0)
printk(version);
@@ -1216,19 +1217,20 @@ static int dec_lance_probe(struct device
*/
switch (type) {
case ASIC_LANCE:
- printk("%s: IOASIC onboard LANCE", name);
+ desc = "IOASIC onboard LANCE";
break;
case PMAD_LANCE:
- printk("%s: PMAD-AA", name);
+ desc = "PMAD-AA";
break;
case PMAX_LANCE:
- printk("%s: PMAX onboard LANCE", name);
+ desc = "PMAX onboard LANCE";
break;
}
for (i = 0; i < 6; i++)
dev->dev_addr[i] = esar[i * 4];
- printk(", addr = %pM, irq = %d\n", dev->dev_addr, dev->irq);
+ printk("%s: %s, addr = %pM, irq = %d\n",
+ name, desc, dev->dev_addr, dev->irq);
dev->netdev_ops = &lance_netdev_ops;
dev->watchdog_timeo = 5*HZ;
^ permalink raw reply
* RE: [PATCH 2/4] usbnet: smsc95xx: align tx-buffer to word
From: David Laight @ 2018-10-02 13:19 UTC (permalink / raw)
To: 'Ben Dooks', netdev@vger.kernel.org
Cc: oneukum@suse.com, davem@davemloft.net, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kernel@lists.codethink.co.uk
In-Reply-To: <20181002092645.1115-3-ben.dooks@codethink.co.uk>
From: Ben Dooks
> Sent: 02 October 2018 10:27
>
> The tegra driver requires alignment of the buffer, so try and
> make this better by pushing the buffer start back to an word
> aligned address. At the worst this makes memcpy() easier as
> it is word aligned, at best it makes sure the usb can directly
> map the buffer.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> [todo - make this configurable]
> ---
> drivers/net/usb/Kconfig | 12 ++++++++++++
> drivers/net/usb/smsc95xx.c | 22 ++++++++++++++++++++--
> 2 files changed, 32 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
...
> +static bool align_tx = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TXALIGN);
> +module_param(align_tx, bool, 0644);
> +MODULE_PARM_DESC(align_tx, "Align TX buffers to word boundaries");
DM doesn't like module parameters.
> static bool turbo_mode = IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TURBO);
> module_param(turbo_mode, bool, 0644);
> MODULE_PARM_DESC(turbo_mode, "Enable multiple frames per Rx transaction");
> @@ -2005,10 +2009,18 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
> bool csum = skb->ip_summed == CHECKSUM_PARTIAL;
> int overhead = csum ? SMSC95XX_TX_OVERHEAD_CSUM : SMSC95XX_TX_OVERHEAD;
> u32 tx_cmd_a, tx_cmd_b;
> + u32 data_len;
> + uintptr_t align = 0;
>
> /* We do not advertise SG, so skbs should be already linearized */
> BUG_ON(skb_shinfo(skb)->nr_frags);
>
> + if (IS_ENABLED(CONFIG_USB_NET_SMSC95XX_TXALIGN) && align_tx) {
> + align = (uintptr_t)skb->data & 3;
> + if (align)
> + overhead += 4 - align;
Better to calculate the pad size once:
align = (-(long)skb->data) & 3;
should do it - and you can unconditionally add it in.
> + }
> +
> /* Make writable and expand header space by overhead if required */
> if (skb_cow_head(skb, overhead)) {
> /* Must deallocate here as returning NULL to indicate error
> @@ -2037,16 +2049,22 @@ static struct sk_buff *smsc95xx_tx_fixup(struct usbnet *dev,
> }
> }
>
> + data_len = skb->len;
> + if (align)
> + skb_push(skb, 4 - align);
> +
> skb_push(skb, 4);
You don't want to call skb_push() twice.
IIRC really horrid things happen if the data has to be copied.
(Actually what happens to the alignment in that case??)
And there is another skb_push() below....
> - tx_cmd_b = (u32)(skb->len - 4);
> + tx_cmd_b = (u32)(data_len);
You don't need the cast here at all (if it was ever needed).
Actually you don't need the new 'data_len' variable.
Just set tx_cmd_b earlier.
...
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH net-next 0/3] ibmvnic: Implement driver-defined queue limits
From: David Miller @ 2018-10-02 6:31 UTC (permalink / raw)
To: tlfalcon; +Cc: netdev, nfont
In-Reply-To: <1538177906-17068-1-git-send-email-tlfalcon@linux.ibm.com>
From: Thomas Falcon <tlfalcon@linux.ibm.com>
Date: Fri, 28 Sep 2018 18:38:23 -0500
> In this patch series, update the ibmvnic driver to use driver-defined
> queue limits instead of limits imposed by the Virtual I/O server
> management partition. For some deviced, initial max queue size and
> amount limits, despite their definition, can actually be exceeded if
> the client driver requests it. With this in mind, define a private
> ethtool flag that toggles the use of driver-defined limits. These
> limits are currently more than what supported hardware will likely
> allow, so the driver will attempt to get as close as possible to
> the user request but may not fully succeed.
Series applied.
^ permalink raw reply
* Re: [PATCH net v2] r8169: fix network stalls due to missing bit TXCFG_AUTO_FIFO
From: David Miller @ 2018-10-02 6:29 UTC (permalink / raw)
To: hkallweit1; +Cc: nic_swsd, netdev, tatkinson, admin, odi
In-Reply-To: <1559697e-9dce-e162-21b1-763b1286cd1a@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 28 Sep 2018 23:51:54 +0200
> Some of the chip-specific hw_start functions set bit TXCFG_AUTO_FIFO
> in register TxConfig. The original patch changed the order of some
> calls resulting in these changes being overwritten by
> rtl_set_tx_config_registers() in rtl_hw_start(). This eventually
> resulted in network stalls especially under high load.
>
> Analyzing the chip-specific hw_start functions all chip version from
> 34, with the exception of version 39, need this bit set.
> This patch moves setting this bit to rtl_set_tx_config_registers().
>
> Fixes: 4fd48c4ac0a0 ("r8169: move common initializations to tp->hw_start")
> Reported-by: Ortwin Glück <odi@odi.ch>
> Reported-by: David Arendt <admin@prnet.org>
> Root-caused-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
> Tested-by: Tony Atkinson <tatkinson@linux.com>
> Tested-by: David Arendt <admin@prnet.org>
> Tested-by: Ortwin Glück <odi@odi.ch>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> Hint for applying this change to stable:
> It may collide with 05212ba8132b ("r8169: set RxConfig after tx/rx is
> enabled for RTL8169sb/8110sb devices") which renamed
> rtl_set_rx_tx_config_registers() to rtl_set_tx_config_registers().
> ---
> v2:
> - added Maciej as root-caused-by
Applied and queued up for -stable, thanks for the backporting hints.
^ permalink raw reply
* Re: [PATCH net 0/3] tun: address two syzbot reports
From: David Miller @ 2018-10-02 6:28 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20180928215149.22092-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Fri, 28 Sep 2018 14:51:46 -0700
> Small changes addressing races discovered by syzbot.
>
> First patch is a cleanup.
> Second patch moves a mutex init sooner.
> Third patch makes sure each tfile gets its own napi enable flags.
Series applied and queued up for -stable.
^ permalink raw reply
* Re: bond: take rcu lock in netpoll_send_skb_on_dev
From: David Miller @ 2018-10-02 6:26 UTC (permalink / raw)
To: davej; +Cc: netdev, xiyou.wangcong
In-Reply-To: <20180928202608.uycdlytob75iphfu@codemonkey.org.uk>
From: Dave Jones <davej@codemonkey.org.uk>
Date: Fri, 28 Sep 2018 16:26:08 -0400
> The bonding driver lacks the rcu lock when it calls down into
> netdev_lower_get_next_private_rcu from bond_poll_controller, which
> results in a trace like:
...
> We're also doing rcu dereferences a layer up in netpoll_send_skb_on_dev
> before we call down into netpoll_poll_dev, so just take the lock there.
>
> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
> Signed-off-by: Dave Jones <davej@codemonkey.org.uk>
Applied and queued up for -stable.
^ permalink raw reply
* [PATCH net-next] tcp: do not release socket ownership in tcp_close()
From: Eric Dumazet @ 2018-10-02 6:24 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
syzkaller was able to hit the WARN_ON(sock_owned_by_user(sk));
in tcp_close()
While a socket is being closed, it is very possible other
threads find it in rtnetlink dump.
tcp_get_info() will acquire the socket lock for a short amount
of time (slow = lock_sock_fast(sk)/unlock_sock_fast(sk, slow);),
enough to trigger the warning.
Fixes: 67db3e4bfbc9 ("tcp: no longer hold ehash lock while calling tcp_get_info()")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
---
include/net/sock.h | 1 +
net/core/sock.c | 2 +-
net/ipv4/tcp.c | 11 +++--------
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 38cae35f6e16056c862b29dee8ceb46fe16fff7b..751549ac0a849144ab0382203ee5c877374523e2 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1492,6 +1492,7 @@ static inline void lock_sock(struct sock *sk)
lock_sock_nested(sk, 0);
}
+void __release_sock(struct sock *sk);
void release_sock(struct sock *sk);
/* BH context may only use the following locking interface. */
diff --git a/net/core/sock.c b/net/core/sock.c
index 8537b6ca72c5013a75c70978c079646f6278aabf..7e8796a6a0892efbb7dfce67d12b8062b2d5daa9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2317,7 +2317,7 @@ static void __lock_sock(struct sock *sk)
finish_wait(&sk->sk_lock.wq, &wait);
}
-static void __release_sock(struct sock *sk)
+void __release_sock(struct sock *sk)
__releases(&sk->sk_lock.slock)
__acquires(&sk->sk_lock.slock)
{
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2827fa5643bde84633b174914bbb13a8273e9d32..43ef83b2330e6238a55c9843580a585d87708e0c 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2416,16 +2416,10 @@ void tcp_close(struct sock *sk, long timeout)
sock_hold(sk);
sock_orphan(sk);
- /* It is the last release_sock in its life. It will remove backlog. */
- release_sock(sk);
-
-
- /* Now socket is owned by kernel and we acquire BH lock
- * to finish close. No need to check for user refs.
- */
local_bh_disable();
bh_lock_sock(sk);
- WARN_ON(sock_owned_by_user(sk));
+ /* remove backlog if any, without releasing ownership. */
+ __release_sock(sk);
percpu_counter_inc(sk->sk_prot->orphan_count);
@@ -2494,6 +2488,7 @@ void tcp_close(struct sock *sk, long timeout)
out:
bh_unlock_sock(sk);
local_bh_enable();
+ release_sock(sk);
sock_put(sk);
}
EXPORT_SYMBOL(tcp_close);
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: [PATCH net] rtnetlink: Fail dump if target netnsid is invalid
From: David Miller @ 2018-10-02 6:22 UTC (permalink / raw)
To: dsahern; +Cc: netdev, jbenc, dsahern
In-Reply-To: <20180928192841.20410-1-dsahern@kernel.org>
From: David Ahern <dsahern@kernel.org>
Date: Fri, 28 Sep 2018 12:28:41 -0700
> From: David Ahern <dsahern@gmail.com>
>
> Link dumps can return results from a target namespace. If the namespace id
> is invalid, then the dump request should fail if get_target_net fails
> rather than continuing with a dump of the current namespace.
>
> Fixes: 79e1ad148c844 ("rtnetlink: use netnsid to query interface")
> Signed-off-by: David Ahern <dsahern@gmail.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] tcp/fq: move back to CLOCK_MONOTONIC
From: David Miller @ 2018-10-02 6:19 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet, leonard.crestez
In-Reply-To: <20180928172844.182542-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Fri, 28 Sep 2018 10:28:44 -0700
> In the recent TCP/EDT patch series, I switched TCP and sch_fq
> clocks from MONOTONIC to TAI, in order to meet the choice done
> earlier for sch_etf packet scheduler.
>
> But sure enough, this broke some setups were the TAI clock
> jumps forward (by almost 50 year...), as reported
> by Leonard Crestez.
>
> If we want to converge later, we'll probably need to add
> an skb field to differentiate the clock bases, or a socket option.
>
> In the meantime, an UDP application will need to use CLOCK_MONOTONIC
> base for its SCM_TXTIME timestamps if using fq packet scheduler.
>
> Fixes: 72b0094f9182 ("tcp: switch tcp_clock_ns() to CLOCK_TAI base")
> Fixes: 142537e41923 ("net_sched: sch_fq: switch to CLOCK_TAI")
> Fixes: fd2bca2aa789 ("tcp: switch internal pacing timer to CLOCK_TAI")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Leonard Crestez <leonard.crestez@nxp.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2 net-next] net: phy: improve handling delayed work
From: David Miller @ 2018-10-02 6:14 UTC (permalink / raw)
To: hkallweit1; +Cc: f.fainelli, andrew, netdev
In-Reply-To: <a35342df-36ad-9364-37f5-39176809bab0@gmail.com>
From: Heiner Kallweit <hkallweit1@gmail.com>
Date: Fri, 28 Sep 2018 08:51:09 +0200
> Using mod_delayed_work() allows to simplify handling delayed work and
> removes the need for the sync parameter in phy_trigger_machine().
> Also introduce a helper phy_queue_state_machine() to encapsulate the
> low-level delayed work calls. No functional change intended.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
> v2:
> - removed inline annotation from phy_queue_state_machine()
Applied.
^ permalink raw reply
* Re: [PATCH net-next 0/5] net: systemport: Turn on offloads by
From: David Miller @ 2018-10-02 6:12 UTC (permalink / raw)
To: f.fainelli; +Cc: netdev
In-Reply-To: <20180927223614.7116-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Thu, 27 Sep 2018 15:36:09 -0700
> Up until now, we had added all the code necessary to turn on RX/TX
> checksum offloads at runtime, but there is no reason why they have to be
> disabled by default given that this gives a slight performance
> improvement.
Series applied.
^ permalink raw reply
* Re: [PATCH 1/4] usbnet: smsc95xx: add kconfig for turbo mode
From: Andrew Lunn @ 2018-10-02 12:46 UTC (permalink / raw)
To: Ben Dooks; +Cc: netdev, oneukum, davem, linux-usb, linux-kernel, linux-kernel
In-Reply-To: <20181002092645.1115-2-ben.dooks@codethink.co.uk>
On Tue, Oct 02, 2018 at 10:26:42AM +0100, Ben Dooks wrote:
> Add a configuration option for the default state of turbo mode
> on the smsc95xx networking driver. Some systems it is better
> to default this to off as it causes significant increases in
> soft-irq load.
>
> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
> ---
> drivers/net/usb/Kconfig | 9 +++++++++
> drivers/net/usb/smsc95xx.c | 2 +-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/usb/Kconfig b/drivers/net/usb/Kconfig
> index 418b0904cecb..a32f1a446ce9 100644
> --- a/drivers/net/usb/Kconfig
> +++ b/drivers/net/usb/Kconfig
> @@ -351,6 +351,15 @@ config USB_NET_SMSC95XX
> This option adds support for SMSC LAN95XX based USB 2.0
> 10/100 Ethernet adapters.
>
> +config USB_NET_SMSC95XX_TURBO
> + bool "Use turbo receive mode by default"
> + depends on USB_NET_SMSC95XX
> + default y
> + help
> + This options sets the default turbo mode settings for the
> + driver's receive path. These can also be altered by the
> + turbo_mode module parameter.
> +
Hi Ben
Is it worth adding a comment here why you would want to turn it off?
To reduce soft-irq load?
Thanks
Andrew
^ permalink raw reply
* Re: [PATCH bpf] bpf: don't accept cgroup local storage with zero value size
From: Daniel Borkmann @ 2018-10-02 12:43 UTC (permalink / raw)
To: Roman Gushchin, netdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Kernel Team, Alexei Starovoitov
In-Reply-To: <20181002024003.25635-1-guro@fb.com>
On 10/02/2018 04:41 AM, Roman Gushchin wrote:
> Explicitly forbid creating cgroup local storage maps with zero value
> size, as it makes no sense and might even cause a panic.
>
> Reported-by: syzbot+18628320d3b14a5c459c@syzkaller.appspotmail.com
> Signed-off-by: Roman Gushchin <guro@fb.com>
> Cc: Alexei Starovoitov <ast@kernel.org>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
Applied to bpf, thanks Roman!
^ permalink raw reply
* Re: [PATCH net-next] net: mscc: allow extracting the FCS into the skb
From: Andrew Lunn @ 2018-10-02 12:43 UTC (permalink / raw)
To: Antoine Tenart
Cc: Florian Fainelli, davem, netdev, linux-kernel, thomas.petazzoni,
alexandre.belloni, quentin.schulz, allan.nielsen
In-Reply-To: <20181002065937.GB3368@kwain>
> The question could be "do we need len += ETH_FCS_LEN" to account for the
> FCS when NETIF_F_RXFCS is used", but I looked at other drivers and it
> seemed to me the FCS is not accounted in the stats. Should it be?
Hi Antoine
There does not appear to be a good answer to that. I submitted a patch
to a driver i'm using to not count it, so that the stats counters we
consistent with another driver. The patch was rejected.
I think the best you can do is flip a coin, and then document it using
a comment about if it is/is not included.
Andrew
^ permalink raw reply
* Re: [PATCH net] r8169: Enable MSI-X on RTL8106e
From: Jian-Hong Pan @ 2018-10-02 5:57 UTC (permalink / raw)
To: David S. Miller
Cc: Heiner Kallweit, Realtek linux nic maintainers, Linux Netdev List,
Linux Kernel, Kai-Heng Feng, Linux Upstreaming Team
In-Reply-To: <20181001.225140.1939583158282424865.davem@davemloft.net>
David Miller <davem@davemloft.net> 於 2018年10月2日 週二 下午1:51寫道：
>
> From: Jian-Hong Pan <jian-hong@endlessm.com>
> Date: Thu, 27 Sep 2018 12:09:48 +0800
>
> > However, there is a commit which resolves the drivers getting nothing in
> > PCI BAR=4 after system resumes. It is 04cb3ae895d7 "PCI: Reprogram
> > bridge prefetch registers on resume" by Daniel Drake.
>
> I don't see this upstream yet.
It is in linux-next repository:
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=04cb3ae895d7efdc60f0fe17182b200a3da20f09
Regrads,
Jian-Hong Pan
^ permalink raw reply
* Re: [PATCH v2 net-next 0/8] Continue towards using linkmode in phylib
From: David Miller @ 2018-10-02 5:55 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, maxime.chevallier
In-Reply-To: <1538255056-15114-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Sat, 29 Sep 2018 23:04:08 +0200
> These patches contain some further cleanup and helpers, and the first
> real patch towards using linkmode bitmaps in phylink.
>
> The macro magic in the RFC version has been replaced with run time
> initialisation.
Series applied, thanks Andrew.
^ permalink raw reply
* Re: [PATCH net-next] nfp: warn on experimental TLV types
From: David Miller @ 2018-10-02 5:50 UTC (permalink / raw)
To: jakub.kicinski; +Cc: netdev, oss-drivers
In-Reply-To: <20180926223531.21489-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Wed, 26 Sep 2018 15:35:31 -0700
> Reserve two TLV types for feature development, and warn in the driver
> if they ever leak into production.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
Applied.
^ permalink raw reply
* [PATCH v2 net] inet: frags: rework rhashtable dismantle
From: Eric Dumazet @ 2018-10-02 5:49 UTC (permalink / raw)
To: David S . Miller
Cc: netdev, Eric Dumazet, Eric Dumazet, Thomas Graf, Herbert Xu
syszbot found an interesting use-after-free [1] happening
while IPv4 fragment rhashtable was destroyed at netns dismantle.
While no insertions can possibly happen at the time a dismantling
netns is destroying this rhashtable, timers can still fire and
attempt to remove elements from this rhashtable.
This is forbidden, since rhashtable_free_and_destroy() has
no synchronization against concurrent inserts and deletes.
Add a new nf->dead flag so that timers do not attempt
a rhashtable_remove_fast() operation.
[1]
BUG: KASAN: use-after-free in __read_once_size include/linux/compiler.h:188 [inline]
BUG: KASAN: use-after-free in rhashtable_last_table+0x216/0x240 lib/rhashtable.c:217
Read of size 8 at addr ffff88019a4c8840 by task kworker/0:4/8279
CPU: 0 PID: 8279 Comm: kworker/0:4 Not tainted 4.19.0-rc5+ #61
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Workqueue: events rht_deferred_worker
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
__asan_report_load8_noabort+0x14/0x20 mm/kasan/report.c:433
__read_once_size include/linux/compiler.h:188 [inline]
rhashtable_last_table+0x216/0x240 lib/rhashtable.c:217
rht_deferred_worker+0x157/0x1de0 lib/rhashtable.c:410
process_one_work+0xc90/0x1b90 kernel/workqueue.c:2153
worker_thread+0x17f/0x1390 kernel/workqueue.c:2296
kthread+0x35a/0x420 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413
Allocated by task 5:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
__do_kmalloc_node mm/slab.c:3682 [inline]
__kmalloc_node+0x47/0x70 mm/slab.c:3689
kmalloc_node include/linux/slab.h:555 [inline]
kvmalloc_node+0xb9/0xf0 mm/util.c:423
kvmalloc include/linux/mm.h:577 [inline]
kvzalloc include/linux/mm.h:585 [inline]
bucket_table_alloc+0x9a/0x4e0 lib/rhashtable.c:176
rhashtable_rehash_alloc+0x73/0x100 lib/rhashtable.c:353
rht_deferred_worker+0x278/0x1de0 lib/rhashtable.c:413
process_one_work+0xc90/0x1b90 kernel/workqueue.c:2153
worker_thread+0x17f/0x1390 kernel/workqueue.c:2296
kthread+0x35a/0x420 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413
Freed by task 8283:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
kvfree+0x61/0x70 mm/util.c:452
bucket_table_free+0xda/0x250 lib/rhashtable.c:108
rhashtable_free_and_destroy+0x152/0x900 lib/rhashtable.c:1163
inet_frags_exit_net+0x3d/0x50 net/ipv4/inet_fragment.c:96
ipv4_frags_exit_net+0x73/0x90 net/ipv4/ip_fragment.c:914
ops_exit_list.isra.7+0xb0/0x160 net/core/net_namespace.c:153
cleanup_net+0x555/0xb10 net/core/net_namespace.c:551
process_one_work+0xc90/0x1b90 kernel/workqueue.c:2153
worker_thread+0x17f/0x1390 kernel/workqueue.c:2296
kthread+0x35a/0x420 kernel/kthread.c:246
ret_from_fork+0x3a/0x50 arch/x86/entry/entry_64.S:413
The buggy address belongs to the object at ffff88019a4c8800
which belongs to the cache kmalloc-16384 of size 16384
The buggy address is located 64 bytes inside of
16384-byte region [ffff88019a4c8800, ffff88019a4cc800)
The buggy address belongs to the page:
page:ffffea0006693200 count:1 mapcount:0 mapping:ffff8801da802200 index:0x0 compound_mapcount: 0
flags: 0x2fffc0000008100(slab|head)
raw: 02fffc0000008100 ffffea0006685608 ffffea0006617c08 ffff8801da802200
raw: 0000000000000000 ffff88019a4c8800 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88019a4c8700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88019a4c8780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff88019a4c8800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88019a4c8880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88019a4c8900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
Fixes: 648700f76b03 ("inet: frags: use rhashtables for reassembly units")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Thomas Graf <tgraf@suug.ch>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
---
include/net/inet_frag.h | 4 +++-
net/ipv4/inet_fragment.c | 31 +++++++++++++++++++++----------
2 files changed, 24 insertions(+), 11 deletions(-)
diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
index 1662cbc0b46b45296a367ecbdaf03c68854fdce7..ffe5e1be40212fa63e360f3e29a56c1b2ce897ee 100644
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -11,7 +11,7 @@ struct netns_frags {
int timeout;
int max_dist;
struct inet_frags *f;
-
+ bool dead;
struct rhashtable rhashtable ____cacheline_aligned_in_smp;
/* Keep atomic mem on separate cachelines in structs that include it */
@@ -24,11 +24,13 @@ struct netns_frags {
* @INET_FRAG_FIRST_IN: first fragment has arrived
* @INET_FRAG_LAST_IN: final fragment has arrived
* @INET_FRAG_COMPLETE: frag queue has been processed and is due for destruction
+ * @INET_FRAG_HASH_DEAD: inet_frag_kill() has not removed fq from rhashtable
*/
enum {
INET_FRAG_FIRST_IN = BIT(0),
INET_FRAG_LAST_IN = BIT(1),
INET_FRAG_COMPLETE = BIT(2),
+ INET_FRAG_HASH_DEAD = BIT(3),
};
struct frag_v4_compare_key {
diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
index bcb11f3a27c0c34115af05034a5a20f57842eb0a..887134425350cc7c8ee081d8c83a037074d96d93 100644
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -71,28 +71,31 @@ EXPORT_SYMBOL(inet_frags_fini);
static void inet_frags_free_cb(void *ptr, void *arg)
{
struct inet_frag_queue *fq = ptr;
+ int count = 0;
- /* If we can not cancel the timer, it means this frag_queue
- * is already disappearing, we have nothing to do.
- * Otherwise, we own a refcount until the end of this function.
- */
- if (!del_timer(&fq->timer))
- return;
+ if (del_timer_sync(&fq->timer))
+ count++;
spin_lock_bh(&fq->lock);
if (!(fq->flags & INET_FRAG_COMPLETE)) {
fq->flags |= INET_FRAG_COMPLETE;
- refcount_dec(&fq->refcnt);
+ count++;
+ } else if (fq->flags & INET_FRAG_HASH_DEAD) {
+ count++;
}
spin_unlock_bh(&fq->lock);
- inet_frag_put(fq);
+ if (refcount_sub_and_test(count, &fq->refcnt))
+ inet_frag_destroy(fq);
}
void inet_frags_exit_net(struct netns_frags *nf)
{
nf->high_thresh = 0; /* prevent creation of new frags */
+ /* paired with READ_ONCE() in inet_frag_kill() */
+ smp_store_release(&nf->dead, true);
+
rhashtable_free_and_destroy(&nf->rhashtable, inet_frags_free_cb, NULL);
}
EXPORT_SYMBOL(inet_frags_exit_net);
@@ -106,8 +109,16 @@ void inet_frag_kill(struct inet_frag_queue *fq)
struct netns_frags *nf = fq->net;
fq->flags |= INET_FRAG_COMPLETE;
- rhashtable_remove_fast(&nf->rhashtable, &fq->node, nf->f->rhash_params);
- refcount_dec(&fq->refcnt);
+ /* This READ_ONCE() is paired with smp_store_release()
+ * in inet_frags_exit_net().
+ */
+ if (!READ_ONCE(nf->dead)) {
+ rhashtable_remove_fast(&nf->rhashtable, &fq->node,
+ nf->f->rhash_params);
+ refcount_dec(&fq->refcnt);
+ } else {
+ fq->flags |= INET_FRAG_HASH_DEAD;
+ }
}
}
EXPORT_SYMBOL(inet_frag_kill);
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related
* Re: [PATCH net-next v6 00/23] WireGuard: Secure Network Tunnel
From: Jason A. Donenfeld @ 2018-10-02 12:22 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Herbert Xu, Eric Biggers, LKML, Netdev, Linux Crypto Mailing List,
David Miller, Greg Kroah-Hartman
In-Reply-To: <CAKv+Gu8fr_qADiTJH05nVWF7Yi3U722nfKJsMaRE9CP3YjNBbg@mail.gmail.com>
On Tue, Oct 2, 2018 at 8:04 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Also, I still think the name Zinc (zinc is not crypto/) is needlessly
> divisive and condescending, and unsaying it (in v2 and up) doesn't
> really work on the Internet (especially since you are still repeating
> it in your conference talk.)
> Jason, you seem to hate the existing crypto framework with passion,
> and the name reflects that.
It's not divisive or condescending. I don't hate the existing
framework with a passion -- this is patently false. The name even with
its original acronym doesn't imply anything essentially negative. I
see that you've repeatedly misinterpreted it this way -- which is why
I removed that from v2 for the avoidance of doubt -- but that doesn't
change the fact that its proper interpretation is not a condescending
or divisive one.
Look, people love to bikeshed about names. I'm sure you'll be able to
CC-in a large crew of people who have opinions in one way or another,
and this thread could begin to have many voices on that front or on
multiple fronts. There are real benefits of sticking with the name
I've given to the library I've spent enormous amounts of time writing.
And so I'm going to stick with Zinc, since that's why our library is
called. Sorry. We can talk about this at Plumbers in Vancouver if you
want, but I think you're not going to get very far with a mailing list
naming bikeshed.
Meanwhile, I'm working around the clock to integrate your very useful
technical suggestions, so please keep them coming if you still have
technical nits you'd like to be in v7 of this patchset.
Jason
^ 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