* [PATCH v2] kcm: remove any offset before parsing messages
From: Dominique Martinet @ 2018-09-11 9:21 UTC (permalink / raw)
To: Doron Roberts-Kedes, Tom Herbert, Dave Watson
Cc: Dominique Martinet, David S. Miller, netdev, linux-kernel
The current code assumes kcm users know they need to look for the
strparser offset within their bpf program, which is not documented
anywhere and examples laying around do not do.
The actual recv function does handle the offset well, so we can create a
temporary clone of the skb and pull that one up as required for parsing.
The pull itself has a cost if we are pulling beyond the head data,
measured to 2-3% latency in a noisy VM with a local client stressing
that path. The clone's impact seemed too small to measure.
This bug can be exhibited easily by implementing a "trivial" kcm parser
taking the first bytes as size, and on the client sending at least two
such packets in a single write().
Note that bpf sockmap has the same problem, both for parse and for recv,
so it would pulling twice or a real pull within the strparser logic if
anyone cares about that.
Signed-off-by: Dominique Martinet <asmadeus@codewreck.org>
---
Hmm, while trying to benchmark this, I sometimes got hangs in
kcm_wait_data() for the last packet somehow?
The sender program was done (exited (zombie) so I assumed the sender
socket flushed), but the receiver was in kcm_wait_data in kcm_recvmsg
indicating it parsed a header but there was no skb to peek at?
But the sock is locked so this shouldn't be racy...
I can get it fairly often with this patch and small messages with an
offset, but I think it's just because the pull changes some timing - I
can't hit it with just the clone, and I can hit it with a pull without
clone as well.... And I don't see how pulling a cloned skb can impact
the original socket, but I'm a bit fuzzy on this.
(it's interesting that I didn't seem to hit this race when pulling in
strparser, that shouldn't be any different)
I'll look at that a bit more, but there have been no activity here for
a while and I don't have the energy to keep pushing in the strparser
direction, so take this patch more as a change of direction and a bit
as a 'bump' on the subject - I think it's important but I have too much
on my plate right now to keep pushing if there is no interest from the
authors.
net/kcm/kcmsock.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 571d824e4e24..36c438b95955 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -381,8 +381,32 @@ static int kcm_parse_func_strparser(struct strparser *strp, struct sk_buff *skb)
{
struct kcm_psock *psock = container_of(strp, struct kcm_psock, strp);
struct bpf_prog *prog = psock->bpf_prog;
+ struct sk_buff *clone_skb = NULL;
+ struct strp_msg *stm;
+ int rc;
+
+ stm = strp_msg(skb);
+ if (stm->offset) {
+ skb = clone_skb = skb_clone(skb, GFP_ATOMIC);
+ if (!clone_skb)
+ return -ENOMEM;
+
+ if (!pskb_pull(clone_skb, stm->offset)) {
+ rc = -ENOMEM;
+ goto out;
+ }
+
+ /* reset cloned skb's offset for bpf programs using it */
+ stm = strp_msg(clone_skb);
+ stm->offset = 0;
+ }
+
+ rc = (*prog->bpf_func)(skb, prog->insnsi);
+out:
+ if (clone_skb)
+ kfree_skb(clone_skb);
- return (*prog->bpf_func)(skb, prog->insnsi);
+ return rc;
}
static int kcm_read_sock_done(struct strparser *strp, int err)
--
2.17.1
^ permalink raw reply related
* Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG
From: zhong jiang @ 2018-09-11 9:19 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: davem, edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <e1b9c36c-5459-a68e-e6fd-cca038e31d5f@cogentembedded.com>
On 2018/9/11 17:11, Sergei Shtylyov wrote:
> On 9/11/2018 11:59 AM, zhong jiang wrote:
>
>>>> The if condition can be removed if we use BUG_ON directly.
>>>> The issule is detected with the help of Coccinelle.
>>>
>>> Issue?
>>>
>>>>
>>>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>>>> ---
>>>> net/ipv4/tcp_input.c | 8 ++++----
>>>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>>>> index 62508a2..893bde3 100644
>>>> --- a/net/ipv4/tcp_input.c
>>>> +++ b/net/ipv4/tcp_input.c
>>>> @@ -4934,8 +4934,8 @@ void tcp_rbtree_insert(struct rb_root *root, struct sk_buff *skb)
>>>> BUG_ON(offset < 0);
>>>> if (size > 0) {
>>>> size = min(copy, size);
>>>> - if (skb_copy_bits(skb, offset, skb_put(nskb, size), size))
>>>> - BUG();
>>>> + BUG(skb_copy_bits(skb, offset,
>>>
>>> You said BUG_ON()?
>> Yep. Do you think that it is worthing to do
>
> I think BUG() doesn't take parameters, BUG_ON() does. Have you tried to build the kernel with your patch at all?
>
I know that the patch should be BUG_ON instead of BUG. This is my mistake. I just want to know that it is worthing to do so.
Thanks,
zhong jiang
>> Thanks,
>> zhong jiang
> [...]
>
> MBR, Sergei
>
>
>
^ permalink raw reply
* Re: [PATCH net-next v3 05/17] zinc: ChaCha20 x86_64 implementation
From: Thomas Gleixner @ 2018-09-11 9:09 UTC (permalink / raw)
To: Samuel Neves
Cc: Jason A. Donenfeld, linux-kernel, netdev, davem,
Greg Kroah-Hartman, Andy Lutomirski, Jean-Philippe Aumasson,
Andy Polyakov, Ingo Molnar, the arch/x86 maintainers,
Linux Crypto Mailing List
In-Reply-To: <CAEX_ruFFWsW897DuFEbz9+6av02bMwDgiA9GxV+=SBtyKG7LJw@mail.gmail.com>
On Tue, 11 Sep 2018, Samuel Neves wrote:
> On Tue, Sep 11, 2018 at 9:22 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Mon, 10 Sep 2018, Jason A. Donenfeld wrote:
> >> lib/zinc/Makefile | 4 +
> >> lib/zinc/chacha20/chacha20-x86_64-glue.h | 102 +
> >> lib/zinc/chacha20/chacha20-x86_64.S | 2632 ++++++++++++++++++++++
> >
> > Just a stupid question. What's the rationale of putting that into lib/zinc
> > instead of having it in arch/x86/crypto?
> >
>
> This is covered on the 02/17 commit message, whose relevant paragraph follows:
Well, being only cc'ed on only half of the patches does not really help.
> > It also organizes the implementations in a simple, straight-forward,
> > and direct manner, making it enjoyable and intuitive to work on.
> > Rather than moving optimized assembly implementations into arch/, it
> > keeps them all together in lib/zinc/, making it simple and obvious to
> > compare and contrast what's happening. This is, notably, exactly what
> > the lib/raid6/ tree does, and that seems to work out rather well. It's
> > also the pattern of most successful crypto libraries. The architecture-
> > specific glue-code is made a part of each translation unit, rather than
> > being in a separate one, so that generic and architecture-optimized code
> > are combined at compile-time, and incompatibility branches compiled out by
> > the optimizer.
Fair enough.
^ permalink raw reply
* Re: [PATCH] PCI: Reprogram bridge prefetch registers on resume
From: Rafael J. Wysocki @ 2018-09-11 9:08 UTC (permalink / raw)
To: Daniel Drake, Bjorn Helgaas
Cc: Peter Wu, linux-pci, Linux Upstreaming Team, nouveau, Linux PM,
kherbst, Andy Shevchenko, Wysocki, Rafael J, Keith Busch,
Mika Westerberg, Jon Derrick, Thomas Martitz, davem, hkallweit1,
netdev, nic_swsd, Linux Kernel
In-Reply-To: <CAD8Lp46eDaSF3fNQgYL=3vCXJ5VpV-JkYEu-YTtuh+w4rMU-5w@mail.gmail.com>
On Tuesday, September 11, 2018 5:35:13 AM CEST Daniel Drake wrote:
> I have created https://bugzilla.kernel.org/show_bug.cgi?id=201069 to
> archive the research done so far.
>
> On Fri, Sep 7, 2018 at 11:05 PM, Peter Wu <peter@lekensteyn.nl> wrote:
> > Windows 10 unconditionally rewrites these registers (BAR, I/O Base +
> > Limit, Memory Base + Limit, etc. from top to bottom), see annotations:
> > https://www.spinics.net/lists/linux-pci/msg75856.html
> >
> > Linux has a generic "restore" operation that works backwards from the
> > end of the PCI config space to the beginning, see
> > pci_restore_config_space. Do you have a dmesg where you see the
> > "restoring config space at offset" messages?
>
> Interesting, I had not spotted this code. The logs for the affected
> bridge on Asus X542UQ:
>
> 0000:00:1c.0: restoring config space at offset 0x3c (was 0x100,
> writing 0x1001ff)
> 0000:00:1c.0: restoring config space at offset 0x24 (was 0x10001,
> writing 0xe1f1d001)
> 0000:00:1c.0: restoring config space at offset 0x20 (was 0x0, writing
> 0xef00ee00)
> 0000:00:1c.0: restoring config space at offset 0xc (was 0x810000,
> writing 0x810010)
> 0000:00:1c.0: restoring config space at offset 0x4 (was 0x100007,
> writing 0x100407)
>
> So it didn't restore 0x28 (the magic register) and also register 0x2c
> (prefetch limit upper 32 bits) because their values were already 0 on
> resume.
>
> https://bugzilla.kernel.org/show_bug.cgi?id=116851#c23 has an
> assertion that Intel recognises this issue and calls for all those
> registers to be restored on resume.
>
> I propose for Linux 4.19 we apply a minimally intrusive change that
> "forcibly" restores dword registers 0x24, 0x28 and 0x2c for PCI
> bridges (regardless of their current value), while retaining the
> existing restore order (high to low) over all registers and doing the
> read-then-maybe-write thing for all of the other regs (per current
> behaviour).
I agree here. It would be good to cover this gap and possibly backport
the fix to "stable".
> Then we also consider swiftly applying a followup patch implementing a
> more thorough approach and we give it some time in linux-next before
> releasing in Linux 4.20 which does something more thorough. I think
> perhaps more discussion is needed there, or at least some more input
> from Bjorn. Seems like we have 3 approaches to consider:
>
> 1. Peter Wu suggests following what windows does. Windows appears to
> start with low registers and works its way upwards, which means it
> writes BAR addresses, I/O base, etc, before writing prefetch
> registers. It skips over read-only and write-to-clear registers and
> also only writes some of the registers at the very end - like the
> command register.
>
> To be thorough here we would probably also have to study and copy what
> Windows does for non-bridge devices (not sure how many device classes
> we would want to study here?). Also it is a bit risky in that Bjorn
> has pointed out that Linux's earlier approach with a high level of
> similarity here (restore registers in ascending order, regardless of
> their current value) caused a laptop to hang on resume.
>
>
> 2. Bjorn suggested tweaking the existing code to always write the
> saved value even when the hardware has that same value. The
> read-maybe-write logic was introduced to avoid a laptop hang, but at
> that time we were restoring registers in ascending order, now we are
> descending it might be worth giving this another try.
I agree with Bjorn here.
> 3. Do nothing else beyond the minimal change that I propose for v4.19?
> Looking a bit more into git history this seems to be a sensitive and
> problematic area, more changes might mean more trouble. For example
> right now pci_restore_config_space() does:
> pci_restore_config_space_range(pdev, 10, 15, 0, 0);
> /* Restore BARs before the command register. */
> pci_restore_config_space_range(pdev, 4, 9, 10, 0);
> pci_restore_config_space_range(pdev, 0, 3, 0, 0);
> but pci_restore_config_space_range() already goes in descending order,
> so the above is already equivalent to the code in the "else" path that
> follows:
> pci_restore_config_space_range(pdev, 0, 15, 0, 0);
>
> and if you look at the history behind this "mixup" there's stuff that
> goes back to 2012 like commit a6cb9ee7cabe ("PCI: Retry BARs
> restoration for Type 0 headers only") which was fixing up another
> problematic commit in this area, etc.
So let's first fix what's known broken and see how this goes IMO.
On top of that and later, try to make the changes suggested by Bjorn and see
how far we can get with that.
Having done that, revisit and see if there still are any problems to address
in this area.
Thanks,
Rafael
^ permalink raw reply
* Re: [PATCH net-next v3 05/17] zinc: ChaCha20 x86_64 implementation
From: Samuel Neves @ 2018-09-11 9:00 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Jason A. Donenfeld, linux-kernel, netdev, davem,
Greg Kroah-Hartman, Andy Lutomirski, Jean-Philippe Aumasson,
Andy Polyakov, Ingo Molnar, the arch/x86 maintainers,
Linux Crypto Mailing List
In-Reply-To: <alpine.DEB.2.21.1809111021250.5110@nanos.tec.linutronix.de>
On Tue, Sep 11, 2018 at 9:22 AM, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Mon, 10 Sep 2018, Jason A. Donenfeld wrote:
>> lib/zinc/Makefile | 4 +
>> lib/zinc/chacha20/chacha20-x86_64-glue.h | 102 +
>> lib/zinc/chacha20/chacha20-x86_64.S | 2632 ++++++++++++++++++++++
>
> Just a stupid question. What's the rationale of putting that into lib/zinc
> instead of having it in arch/x86/crypto?
>
This is covered on the 02/17 commit message, whose relevant paragraph follows:
> It also organizes the implementations in a simple, straight-forward,
> and direct manner, making it enjoyable and intuitive to work on.
> Rather than moving optimized assembly implementations into arch/, it
> keeps them all together in lib/zinc/, making it simple and obvious to
> compare and contrast what's happening. This is, notably, exactly what
> the lib/raid6/ tree does, and that seems to work out rather well. It's
> also the pattern of most successful crypto libraries. The architecture-
> specific glue-code is made a part of each translation unit, rather than
> being in a separate one, so that generic and architecture-optimized code
> are combined at compile-time, and incompatibility branches compiled out by
> the optimizer.
^ permalink raw reply
* Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG
From: zhong jiang @ 2018-09-11 8:59 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: davem, edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <a000b5a4-2907-70f9-70ed-7e1c6a797876@cogentembedded.com>
On 2018/9/11 16:51, Sergei Shtylyov wrote:
> On 9/10/2018 5:38 PM, zhong jiang wrote:
>
>> The if condition can be removed if we use BUG_ON directly.
>> The issule is detected with the help of Coccinelle.
>
> Issue?
>
>>
>> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
>> ---
>> net/ipv4/tcp_input.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
>> index 62508a2..893bde3 100644
>> --- a/net/ipv4/tcp_input.c
>> +++ b/net/ipv4/tcp_input.c
>> @@ -4934,8 +4934,8 @@ void tcp_rbtree_insert(struct rb_root *root, struct sk_buff *skb)
>> BUG_ON(offset < 0);
>> if (size > 0) {
>> size = min(copy, size);
>> - if (skb_copy_bits(skb, offset, skb_put(nskb, size), size))
>> - BUG();
>> + BUG(skb_copy_bits(skb, offset,
>
> You said BUG_ON()?
Yep. Do you think that it is worthing to do
Thanks,
zhong jiang
>
>> + skb_put(nskb, size), size));
>> TCP_SKB_CB(nskb)->end_seq += size;
>> copy -= size;
>> start += size;
>> @@ -5327,8 +5327,8 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, const struct tcphdr *t
>> /* Is the urgent pointer pointing into this packet? */
>> if (ptr < skb->len) {
>> u8 tmp;
>> - if (skb_copy_bits(skb, ptr, &tmp, 1))
>> - BUG();
>> +
>> + BUG(skb_copy_bits(skb, ptr, &tmp, 1));
>
> Likewise.
>
> [...]
>
> MBR, Sergei
>
> .
>
^ permalink raw reply
* Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG
From: Sergei Shtylyov @ 2018-09-11 8:51 UTC (permalink / raw)
To: zhong jiang, davem, edumazet; +Cc: kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <1536590282-23899-1-git-send-email-zhongjiang@huawei.com>
On 9/10/2018 5:38 PM, zhong jiang wrote:
> The if condition can be removed if we use BUG_ON directly.
> The issule is detected with the help of Coccinelle.
Issue?
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
> ---
> net/ipv4/tcp_input.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 62508a2..893bde3 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -4934,8 +4934,8 @@ void tcp_rbtree_insert(struct rb_root *root, struct sk_buff *skb)
> BUG_ON(offset < 0);
> if (size > 0) {
> size = min(copy, size);
> - if (skb_copy_bits(skb, offset, skb_put(nskb, size), size))
> - BUG();
> + BUG(skb_copy_bits(skb, offset,
You said BUG_ON()?
> + skb_put(nskb, size), size));
> TCP_SKB_CB(nskb)->end_seq += size;
> copy -= size;
> start += size;
> @@ -5327,8 +5327,8 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, const struct tcphdr *t
> /* Is the urgent pointer pointing into this packet? */
> if (ptr < skb->len) {
> u8 tmp;
> - if (skb_copy_bits(skb, ptr, &tmp, 1))
> - BUG();
> +
> + BUG(skb_copy_bits(skb, ptr, &tmp, 1));
Likewise.
[...]
MBR, Sergei
^ permalink raw reply
* Re: [PATCH net] ipv6: use rt6_info members when dst is set in rt6_fill_node
From: Hangbin Liu @ 2018-09-11 3:41 UTC (permalink / raw)
To: David Ahern; +Cc: Xin Long, network dev, davem, Roopa Prabhu
In-Reply-To: <879245ec-fd99-3b7f-6fc2-2d8d25cf2c57@cumulusnetworks.com>
Hi David,
On Mon, Sep 10, 2018 at 08:39:34PM -0600, David Ahern wrote:
> On 9/10/18 7:04 PM, Hangbin Liu wrote:
>
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 18e00ce..62621b4 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -998,6 +998,21 @@ static void ip6_rt_copy_init(struct rt6_info *rt, struct fib6_info *ort)
> > rt->rt6i_prefsrc = ort->fib6_prefsrc;
> > }
> >
> > +static void rt6_update_info(struct rt6_info *rt)
> > +{
> > + struct fib6_info *from;
> > +
> > + rcu_read_lock();
> > + from = rcu_dereference(rt->from);
> > + fib6_info_hold(from);
> > + rcu_read_unlock();
> > +
> > + from->fib6_flags = rt->rt6i_flags;
> > + from->fib6_nh.nh_gw = rt->rt6i_gateway;
>
> As I mentioned on your last patch, redirects do *not* update fib
> entries. Exceptions, yes, but not core data of a fib entry.
Thanks for the comments, I understand that we should not update original route.
And here I know the redirect (should?) do not update fib entries.
So Xin Long's patch looks more reasonable.
Thanks
Hangbin
^ permalink raw reply
* Re: [Intel-wired-lan] e1000e driver stuck at 10Mbps after reconnection
From: Benjamin Poirier @ 2018-09-11 8:31 UTC (permalink / raw)
To: Camille Bordignon
Cc: Neftin, Sasha, Alexander Duyck, Netdev, intel-wired-lan,
David S. Miller, linux-kernel
In-Reply-To: <20180907062851.GA7336@super_plancton>
On 2018/09/07 08:28, Camille Bordignon wrote:
> Le mercredi 08 août 2018 à 18:00:28 (+0300), Neftin, Sasha a écrit :
> > On 8/8/2018 17:24, Neftin, Sasha wrote:
> > > On 8/7/2018 09:42, Camille Bordignon wrote:
> > > > Le lundi 06 août 2018 à 15:45:29 (-0700), Alexander Duyck a écrit :
> > > > > On Mon, Aug 6, 2018 at 4:59 AM, Camille Bordignon
> > > > > <camille.bordignon@easymile.com> wrote:
> > > > > > Hello,
> > > > > >
> > > > > > Recently we experienced some issues with intel NIC (I219-LM
> > > > > > and I219-V).
> > > > > > It seems that after a wire reconnection, auto-negotation "fails" and
> > > > > > link speed drips to 10 Mbps.
> > > > > >
[...]
>
> I recently figured out that neither the previous patch nor commit
> 0b76aae741abb9d16d2c0e67f8b1e766576f897d fix this issue.
>
> In these cases, after reproducing the issue, when ethernet wire is connected
> kernel logs mention full speed (1000 Mbps) but actually it seems it is not.
> The problem persists.
>
Hmm, so the newer (post 4110e02eb45e) kernels are actually "better", in
that they accurately report that link speed is 10Mb/s.
In the end, do you know of a kernel version that doesn't exhibit the
problem of slower actual link speed?
I had a look at the code and I tried to reproduce the problem on the
hardware that I have (I217) but could not.
Also, out of curiosity, have you tried playing with the speed, autoneg
and advertise settings via ethtool -s to force the link to 1000Mb/s?
^ permalink raw reply
* RE: [PATCH] xen-netback: remove unecessary condition check before debugfs_remove_recursive
From: Paul Durrant @ 2018-09-11 8:00 UTC (permalink / raw)
To: 'zhong jiang', davem@davemloft.net, Wei Liu
Cc: xen-devel@lists.xenproject.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <1536414822-32911-1-git-send-email-zhongjiang@huawei.com>
> -----Original Message-----
> From: zhong jiang [mailto:zhongjiang@huawei.com]
> Sent: 08 September 2018 14:54
> To: davem@davemloft.net; Paul Durrant <Paul.Durrant@citrix.com>; Wei Liu
> <wei.liu2@citrix.com>
> Cc: xen-devel@lists.xenproject.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org
> Subject: [PATCH] xen-netback: remove unecessary condition check before
> debugfs_remove_recursive
>
> debugfs_remove_recursive has taken IS_ERR_OR_NULL into account. So just
> remove the condition check before debugfs_remove_recursive.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Reviewed-by: Paul Durrant <paul.durrant@citrix.com>
> ---
> drivers/net/xen-netback/netback.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-
> netback/netback.c
> index 3621e05..80aae3a 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -1660,8 +1660,7 @@ static int __init netback_init(void)
> static void __exit netback_fini(void)
> {
> #ifdef CONFIG_DEBUG_FS
> - if (!IS_ERR_OR_NULL(xen_netback_dbg_root))
> - debugfs_remove_recursive(xen_netback_dbg_root);
> + debugfs_remove_recursive(xen_netback_dbg_root);
> #endif /* CONFIG_DEBUG_FS */
> xenvif_xenbus_fini();
> }
> --
> 1.7.12.4
^ permalink raw reply
* Re: [PATCH net] ipv6: use rt6_info members when dst is set in rt6_fill_node
From: David Ahern @ 2018-09-11 2:39 UTC (permalink / raw)
To: Hangbin Liu; +Cc: Xin Long, network dev, davem, Roopa Prabhu
In-Reply-To: <20180911010449.GA24677@leo.usersys.redhat.com>
On 9/10/18 7:04 PM, Hangbin Liu wrote:
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 18e00ce..62621b4 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -998,6 +998,21 @@ static void ip6_rt_copy_init(struct rt6_info *rt, struct fib6_info *ort)
> rt->rt6i_prefsrc = ort->fib6_prefsrc;
> }
>
> +static void rt6_update_info(struct rt6_info *rt)
> +{
> + struct fib6_info *from;
> +
> + rcu_read_lock();
> + from = rcu_dereference(rt->from);
> + fib6_info_hold(from);
> + rcu_read_unlock();
> +
> + from->fib6_flags = rt->rt6i_flags;
> + from->fib6_nh.nh_gw = rt->rt6i_gateway;
As I mentioned on your last patch, redirects do *not* update fib
entries. Exceptions, yes, but not core data of a fib entry.
^ permalink raw reply
* [PATCH RESEND net-next] docs: net: Convert tcp.txt to RST format
From: Tobin C. Harding @ 2018-09-11 7:13 UTC (permalink / raw)
To: David S. Miller
Cc: Tobin C. Harding, Eric Dumazet, netdev, linux-doc, linux-kernel
Restructured text is the kernel documentation format of choice now.
Some text from tcp.txt is out of date, specifically the function
tcp_write() does not appear to be in the tree anymore. Also the
following data members have been removed
sk->tcp_pend_event
sk->transmit_queue
sk->transmit_new
sk->transmit_end
sk->tcp_last_tx_ack
sk->tcp_dup_ack
Remove section 'How the new TCP output machine [nyi] works'. This
leaves only a single section so we can name the document with that
section heading now.
Convert tcp.txt to RST format. Add GPLv2 SPDX tag.
Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
This is v1 re-sending since I sent the original during the merge window :(
CC'd Eric as maintainer of TCP (according to MAINTAINERS)
I was going to ask a question on netdev list as to whether this file was
out of date. I just removed the suspect section and did the patch
instead. I am not sure if it is correct to remove it or if the
structure documented has just been changed. Please see bottom section
of removed text ('How the new TCP output machine [nyi] works').
Also please note this file is not covered by MAINTAINERS, should it be?
thanks,
Tobin.
Documentation/networking/00-INDEX | 2 -
Documentation/networking/index.rst | 1 +
Documentation/networking/tcp.rst | 71 ++++++++++++++++++++
Documentation/networking/tcp.txt | 101 -----------------------------
4 files changed, 72 insertions(+), 103 deletions(-)
create mode 100644 Documentation/networking/tcp.rst
delete mode 100644 Documentation/networking/tcp.txt
diff --git a/Documentation/networking/00-INDEX b/Documentation/networking/00-INDEX
index 02a323c43261..dcbccae4043e 100644
--- a/Documentation/networking/00-INDEX
+++ b/Documentation/networking/00-INDEX
@@ -198,8 +198,6 @@ tc-actions-env-rules.txt
- rules for traffic control (tc) actions.
timestamping.txt
- overview of network packet timestamping variants.
-tcp.txt
- - short blurb on how TCP output takes place.
tcp-thin.txt
- kernel tuning options for low rate 'thin' TCP streams.
team.txt
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index fcd710f2cc7a..1cb9bcc36dd7 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -21,6 +21,7 @@ Contents:
net_failover
alias
bridge
+ tcp
.. only:: subproject
diff --git a/Documentation/networking/tcp.rst b/Documentation/networking/tcp.rst
new file mode 100644
index 000000000000..ae2094fc5de3
--- /dev/null
+++ b/Documentation/networking/tcp.rst
@@ -0,0 +1,71 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+======================
+TCP Congestion Control
+======================
+
+The following variables are used in the tcp_sock for congestion control
+
+.. flat-table:: Congestion Control
+ :widths: 1 2
+
+ * - tcp_sock struct member
+ - Usage
+
+ * - snd_cwnd
+ - The size of the congestion window
+
+ * - snd_ssthresh
+ - Slow start threshold. We are in slow start if snd_cwnd is less
+ than this.
+
+ * - snd_cwnd_cnt
+ - A counter used to slow down the rate of increase once we exceed
+ slow start threshold.
+
+ * - snd_cwnd_clamp
+ - This is the maximum size that snd_cwnd can grow to.
+
+ * - snd_cwnd_stamp
+ - Timestamp for when congestion window last validated.
+
+ * - snd_cwnd_used
+ - Used as a highwater mark for how much of the congestion window
+ is in use. It is used to adjust snd_cwnd down when the link is
+ limited by the application rather than the network.
+
+As of 2.6.13, Linux supports pluggable congestion control algorithms. A
+congestion control mechanism can be registered through functions in
+tcp_cong.c. The functions used by the congestion control mechanism are
+registered via passing a tcp_congestion_ops struct to
+tcp_register_congestion_control. As a minimum, the congestion control
+mechanism must provide a valid name and must implement either ssthresh,
+cong_avoid and undo_cwnd hooks or the "omnipotent" cong_control hook.
+
+Private data for a congestion control mechanism is stored in
+tp->ca_priv. tcp_ca(tp) returns a pointer to this space. This is
+preallocated space - it is important to check the size of your private
+data will fit this space, or alternatively, space could be allocated
+elsewhere and a pointer to it could be stored here.
+
+There are three kinds of congestion control algorithms currently: The
+simplest ones are derived from TCP reno (highspeed, scalable) and just
+provide an alternative congestion window calculation. More complex ones
+like BIC try to look at other events to provide better heuristics.
+There are also round trip time based algorithms like Vegas and
+Westwood+.
+
+Good TCP congestion control is a complex problem because the algorithm
+needs to maintain fairness and performance. Please review current
+research and RFC's before developing new modules.
+
+The default congestion control mechanism is chosen based on the
+DEFAULT_TCP_CONG Kconfig parameter. If you really want a particular
+default value then you can set it using sysctl
+net.ipv4.tcp_congestion_control. The module will be autoloaded if
+needed and you will get the expected protocol. If you ask for an
+unknown congestion method, then the sysctl attempt will fail.
+
+If you remove a TCP congestion control module, then you will get the
+next available one. Since reno cannot be built as a module, and cannot
+be removed, it will always be available.
diff --git a/Documentation/networking/tcp.txt b/Documentation/networking/tcp.txt
deleted file mode 100644
index 9c7139d57e57..000000000000
--- a/Documentation/networking/tcp.txt
+++ /dev/null
@@ -1,101 +0,0 @@
-TCP protocol
-============
-
-Last updated: 3 June 2017
-
-Contents
-========
-
-- Congestion control
-- How the new TCP output machine [nyi] works
-
-Congestion control
-==================
-
-The following variables are used in the tcp_sock for congestion control:
-snd_cwnd The size of the congestion window
-snd_ssthresh Slow start threshold. We are in slow start if
- snd_cwnd is less than this.
-snd_cwnd_cnt A counter used to slow down the rate of increase
- once we exceed slow start threshold.
-snd_cwnd_clamp This is the maximum size that snd_cwnd can grow to.
-snd_cwnd_stamp Timestamp for when congestion window last validated.
-snd_cwnd_used Used as a highwater mark for how much of the
- congestion window is in use. It is used to adjust
- snd_cwnd down when the link is limited by the
- application rather than the network.
-
-As of 2.6.13, Linux supports pluggable congestion control algorithms.
-A congestion control mechanism can be registered through functions in
-tcp_cong.c. The functions used by the congestion control mechanism are
-registered via passing a tcp_congestion_ops struct to
-tcp_register_congestion_control. As a minimum, the congestion control
-mechanism must provide a valid name and must implement either ssthresh,
-cong_avoid and undo_cwnd hooks or the "omnipotent" cong_control hook.
-
-Private data for a congestion control mechanism is stored in tp->ca_priv.
-tcp_ca(tp) returns a pointer to this space. This is preallocated space - it
-is important to check the size of your private data will fit this space, or
-alternatively, space could be allocated elsewhere and a pointer to it could
-be stored here.
-
-There are three kinds of congestion control algorithms currently: The
-simplest ones are derived from TCP reno (highspeed, scalable) and just
-provide an alternative congestion window calculation. More complex
-ones like BIC try to look at other events to provide better
-heuristics. There are also round trip time based algorithms like
-Vegas and Westwood+.
-
-Good TCP congestion control is a complex problem because the algorithm
-needs to maintain fairness and performance. Please review current
-research and RFC's before developing new modules.
-
-The default congestion control mechanism is chosen based on the
-DEFAULT_TCP_CONG Kconfig parameter. If you really want a particular default
-value then you can set it using sysctl net.ipv4.tcp_congestion_control. The
-module will be autoloaded if needed and you will get the expected protocol. If
-you ask for an unknown congestion method, then the sysctl attempt will fail.
-
-If you remove a TCP congestion control module, then you will get the next
-available one. Since reno cannot be built as a module, and cannot be
-removed, it will always be available.
-
-How the new TCP output machine [nyi] works.
-===========================================
-
-Data is kept on a single queue. The skb->users flag tells us if the frame is
-one that has been queued already. To add a frame we throw it on the end. Ack
-walks down the list from the start.
-
-We keep a set of control flags
-
-
- sk->tcp_pend_event
-
- TCP_PEND_ACK Ack needed
- TCP_ACK_NOW Needed now
- TCP_WINDOW Window update check
- TCP_WINZERO Zero probing
-
-
- sk->transmit_queue The transmission frame begin
- sk->transmit_new First new frame pointer
- sk->transmit_end Where to add frames
-
- sk->tcp_last_tx_ack Last ack seen
- sk->tcp_dup_ack Dup ack count for fast retransmit
-
-
-Frames are queued for output by tcp_write. We do our best to send the frames
-off immediately if possible, but otherwise queue and compute the body
-checksum in the copy.
-
-When a write is done we try to clear any pending events and piggy back them.
-If the window is full we queue full sized frames. On the first timeout in
-zero window we split this.
-
-On a timer we walk the retransmit list to send any retransmits, update the
-backoff timers etc. A change of route table stamp causes a change of header
-and recompute. We add any new tcp level headers and refinish the checksum
-before sending.
-
--
2.17.1
^ permalink raw reply related
* [PATCH] xen/netfront: don't bug in case of too many frags
From: Juergen Gross @ 2018-09-11 7:04 UTC (permalink / raw)
To: linux-kernel, netdev, xen-devel
Cc: davem, boris.ostrovsky, Juergen Gross, stable
Commit 57f230ab04d291 ("xen/netfront: raise max number of slots in
xennet_get_responses()") raised the max number of allowed slots by one.
This seems to be problematic in some configurations with netback using
a larger MAX_SKB_FRAGS value (e.g. old Linux kernel with MAX_SKB_FRAGS
defined as 18 instead of nowadays 17).
Instead of BUG_ON() in this case just fall back to retransmission.
Fixes: 57f230ab04d291 ("xen/netfront: raise max number of slots in xennet_get_responses()")
Cc: stable@vger.kernel.org
Signed-off-by: Juergen Gross <jgross@suse.com>
---
drivers/net/xen-netfront.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index 9407acbd19a9..f17f602e6171 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -908,7 +908,11 @@ static RING_IDX xennet_fill_frags(struct netfront_queue *queue,
BUG_ON(pull_to <= skb_headlen(skb));
__pskb_pull_tail(skb, pull_to - skb_headlen(skb));
}
- BUG_ON(skb_shinfo(skb)->nr_frags >= MAX_SKB_FRAGS);
+ if (unlikely(skb_shinfo(skb)->nr_frags >= MAX_SKB_FRAGS)) {
+ queue->rx.rsp_cons = ++cons;
+ kfree_skb(nskb);
+ return ~0U;
+ }
skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
skb_frag_page(nfrag),
@@ -1045,6 +1049,8 @@ static int xennet_poll(struct napi_struct *napi, int budget)
skb->len += rx->status;
i = xennet_fill_frags(queue, skb, &tmpq);
+ if (unlikely(i == ~0U))
+ goto err;
if (rx->flags & XEN_NETRXF_csum_blank)
skb->ip_summed = CHECKSUM_PARTIAL;
--
2.16.4
^ permalink raw reply related
* Re: [PATCH] ip6_gre: simplify gre header parsing in ip6gre_err
From: Jiri Benc @ 2018-09-11 6:47 UTC (permalink / raw)
To: Haishuang Yan; +Cc: David S. Miller, Alexey Kuznetsov, netdev, linux-kernel
In-Reply-To: <F6F07B41-345D-4416-B9C7-D5509715D33F@cmss.chinamobile.com>
On Tue, 11 Sep 2018 10:19:31 +0800, Haishuang Yan wrote:
> Since csum_err may not be used outside, how about refactoring gre_parse_header function like this:
>
> --- a/net/ipv4/gre_demux.c
> +++ b/net/ipv4/gre_demux.c
> @@ -86,7 +86,7 @@ int gre_parse_header(struct sk_buff *skb, struct tnl_ptk_info *tpi,
>
> options = (__be32 *)(greh + 1);
> if (greh->flags & GRE_CSUM) {
> - if (skb_checksum_simple_validate(skb)) {
> + if (csum_err && skb_checksum_simple_validate(skb)) {
Something like this, yes.
Thanks!
Jiri
^ permalink raw reply
* Re: [Patch net v2] rds: fix two RCU related problems
From: Santosh Shilimkar @ 2018-09-11 1:33 UTC (permalink / raw)
To: Cong Wang, netdev; +Cc: Sowmini Varadhan, rds-devel
In-Reply-To: <20180911012726.5353-1-xiyou.wangcong@gmail.com>
On 9/10/2018 6:27 PM, Cong Wang wrote:
> When a rds sock is bound, it is inserted into the bind_hash_table
> which is protected by RCU. But when releasing rds sock, after it
> is removed from this hash table, it is freed immediately without
> respecting RCU grace period. This could cause some use-after-free
> as reported by syzbot.
>
> Mark the rds sock with SOCK_RCU_FREE before inserting it into the
> bind_hash_table, so that it would be always freed after a RCU grace
> period.
>
> The other problem is in rds_find_bound(), the rds sock could be
> freed in between rhashtable_lookup_fast() and rds_sock_addref(),
> so we need to extend RCU read lock protection in rds_find_bound()
> to close this race condition.
>
> Reported-and-tested-by: syzbot+8967084bcac563795dc6@syzkaller.appspotmail.com
> Reported-by: syzbot+93a5839deb355537440f@syzkaller.appspotmail.com
> Cc: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com>
> Cc: rds-devel@oss.oracle.com
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
> ---
Thank you !!
Acked-by: Santosh Shilimkar <santosh.shilimkar@oarcle.com>
^ permalink raw reply
* [Patch net v2] rds: fix two RCU related problems
From: Cong Wang @ 2018-09-11 1:27 UTC (permalink / raw)
To: netdev; +Cc: Cong Wang, Sowmini Varadhan, Santosh Shilimkar, rds-devel
When a rds sock is bound, it is inserted into the bind_hash_table
which is protected by RCU. But when releasing rds sock, after it
is removed from this hash table, it is freed immediately without
respecting RCU grace period. This could cause some use-after-free
as reported by syzbot.
Mark the rds sock with SOCK_RCU_FREE before inserting it into the
bind_hash_table, so that it would be always freed after a RCU grace
period.
The other problem is in rds_find_bound(), the rds sock could be
freed in between rhashtable_lookup_fast() and rds_sock_addref(),
so we need to extend RCU read lock protection in rds_find_bound()
to close this race condition.
Reported-and-tested-by: syzbot+8967084bcac563795dc6@syzkaller.appspotmail.com
Reported-by: syzbot+93a5839deb355537440f@syzkaller.appspotmail.com
Cc: Sowmini Varadhan <sowmini.varadhan@oracle.com>
Cc: Santosh Shilimkar <santosh.shilimkar@oracle.com>
Cc: rds-devel@oss.oracle.com
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/rds/bind.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/net/rds/bind.c b/net/rds/bind.c
index 3ab55784b637..762d2c6788a3 100644
--- a/net/rds/bind.c
+++ b/net/rds/bind.c
@@ -76,11 +76,13 @@ struct rds_sock *rds_find_bound(const struct in6_addr *addr, __be16 port,
struct rds_sock *rs;
__rds_create_bind_key(key, addr, port, scope_id);
- rs = rhashtable_lookup_fast(&bind_hash_table, key, ht_parms);
+ rcu_read_lock();
+ rs = rhashtable_lookup(&bind_hash_table, key, ht_parms);
if (rs && !sock_flag(rds_rs_to_sk(rs), SOCK_DEAD))
rds_sock_addref(rs);
else
rs = NULL;
+ rcu_read_unlock();
rdsdebug("returning rs %p for %pI6c:%u\n", rs, addr,
ntohs(port));
@@ -235,6 +237,7 @@ int rds_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
goto out;
}
+ sock_set_flag(sk, SOCK_RCU_FREE);
ret = rds_add_bound(rs, binding_addr, &port, scope_id);
if (ret)
goto out;
--
2.14.4
^ permalink raw reply related
* [PATCHv2 iproute2] bridge/mdb: fix missing new line when show bridge mdb
From: Hangbin Liu @ 2018-09-11 1:26 UTC (permalink / raw)
To: netdev; +Cc: Stephen Hemminger, David Ahern, Phil Sutter, Hangbin Liu
In-Reply-To: <1536118423-20604-1-git-send-email-liuhangbin@gmail.com>
The bridge mdb show is broken on current iproute2. e.g.
]# bridge mdb show
34: br0 veth0_br 224.1.1.2 temp 34: br0 veth0_br 224.1.1.1 temp
After fix:
]# bridge mdb show
34: br0 veth0_br 224.1.1.2 temp
34: br0 veth0_br 224.1.1.1 temp
v2: use json print lib as Stephen suggested.
Reported-by: Ying Xu <yinxu@redhat.com>
Fixes: c7c1a1ef51aea ("bridge: colorize output and use JSON print library")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
bridge/mdb.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/bridge/mdb.c b/bridge/mdb.c
index f38dc67..408117d 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -107,6 +107,10 @@ static void br_print_router_ports(FILE *f, struct rtattr *attr,
fprintf(f, "%s ", port_ifname);
}
}
+
+ if (!is_json_context() && !show_stats)
+ print_string(PRINT_FP, NULL, "\n", NULL);
+
close_json_array(PRINT_JSON, NULL);
}
@@ -164,6 +168,10 @@ static void print_mdb_entry(FILE *f, int ifindex, const struct br_mdb_entry *e,
print_string(PRINT_ANY, "timer", " %s",
format_timer(timer));
}
+
+ if (!is_json_context())
+ print_string(PRINT_FP, NULL, "\n", NULL);
+
close_json_object();
}
--
2.5.5
^ permalink raw reply related
* Re: [PATCH net-next] virtio_net: ethtool tx napi configuration
From: Willem de Bruijn @ 2018-09-11 1:14 UTC (permalink / raw)
To: Jason Wang
Cc: Network Development, David Miller, caleb.raitto,
Michael S. Tsirkin, Jon Olson (Google Drive), Willem de Bruijn
In-Reply-To: <78a3f2f8-7c86-f2a1-4ffe-b9bb270ed186@redhat.com>
> >> I cook a fixup, and it looks works in my setup:
> >>
> >> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >> index b320b6b14749..9181c3f2f832 100644
> >> --- a/drivers/net/virtio_net.c
> >> +++ b/drivers/net/virtio_net.c
> >> @@ -2204,10 +2204,17 @@ static int virtnet_set_coalesce(struct
> >> net_device *dev,
> >> return -EINVAL;
> >>
> >> if (napi_weight ^ vi->sq[0].napi.weight) {
> >> - if (dev->flags & IFF_UP)
> >> - return -EBUSY;
> >> - for (i = 0; i < vi->max_queue_pairs; i++)
> >> + for (i = 0; i < vi->max_queue_pairs; i++) {
> >> + struct netdev_queue *txq =
> >> + netdev_get_tx_queue(vi->dev, i);
> >> +
> >> + virtnet_napi_tx_disable(&vi->sq[i].napi);
> >> + __netif_tx_lock_bh(txq);
> >> vi->sq[i].napi.weight = napi_weight;
> >> + __netif_tx_unlock_bh(txq);
> >> + virtnet_napi_tx_enable(vi, vi->sq[i].vq,
> >> + &vi->sq[i].napi);
> >> + }
> >> }
> >>
> >> return 0;
> > Thanks! It passes my simple stress test, too. Which consists of two
> > concurrent loops, one toggling the ethtool option, another running
> > TCP_RR.
> >
> >> The only left case is the speculative tx polling in RX NAPI. I think we
> >> don't need to care in this case since it was not a must for correctness.
> > As long as the txq lock is held that will be a noop, anyway. The other
> > concurrent action is skb_xmit_done. It looks correct to me, but need
> > to think about it a bit. The tricky transition is coming out of napi without
> > having >= 2 + MAX_SKB_FRAGS clean descriptors. If the queue is
> > stopped it may deadlock transmission in no-napi mode.
>
> Yes, maybe we can enable tx queue when napi weight is zero in
> virtnet_poll_tx().
Yes, that precaution should resolve that edge case.
> Let me do more stress test on this.
>
> >
> >>> Link: https://patchwork.ozlabs.org/patch/948149/
> >>> Suggested-by: Jason Wang <jasowang@redhat.com>
> >>> Signed-off-by: Willem de Bruijn <willemb@google.com>
> >>> ---
> >>> drivers/net/virtio_net.c | 52 ++++++++++++++++++++++++++++++++++++++++
> >>> 1 file changed, 52 insertions(+)
> >>>
> >>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> >>> index 765920905226..b320b6b14749 100644
> >>> --- a/drivers/net/virtio_net.c
> >>> +++ b/drivers/net/virtio_net.c
> >>> @@ -66,6 +66,8 @@ DECLARE_EWMA(pkt_len, 0, 64)
> >>>
> >>> #define VIRTNET_DRIVER_VERSION "1.0.0"
> >>>
> >>> +static const u32 ethtool_coalesce_napi_mask = (1UL << 10);
> >>> +
> >>> static const unsigned long guest_offloads[] = {
> >>> VIRTIO_NET_F_GUEST_TSO4,
> >>> VIRTIO_NET_F_GUEST_TSO6,
> >>> @@ -2181,6 +2183,54 @@ static int virtnet_get_link_ksettings(struct net_device *dev,
> >>> return 0;
> >>> }
> >>>
> >>> +static int virtnet_set_coalesce(struct net_device *dev,
> >>> + struct ethtool_coalesce *ec)
> >>> +{
> >>> + const struct ethtool_coalesce ec_default = {
> >>> + .cmd = ETHTOOL_SCOALESCE,
> >>> + .rx_max_coalesced_frames = 1,
> >> I think rx part is no necessary.
> > The definition of ethtool_coalesce has:
> >
> > "* It is illegal to set both usecs and max_frames to zero as this
> > * would cause interrupts to never be generated. To disable
> > * coalescing, set usecs = 0 and max_frames = 1."
> >
> > I'd rather not diverge from this prescribed behavior unless there's
> > a strong reason.
>
> I get this.
>
> >
> > On the related point in the other thread:
> >
> >> Rethink about this, how about something like:
> >>
> >> - UINT_MAX: no tx interrupt
> >> - other value: tx interrupt with possible interrupt moderation
> > Okay, that will be simpler to configure.
>
> I wonder maybe we can use ethtool_coalesce definition. E.g usecs = 0 &&
> max_frame = 0 means no interrupt? Just a thought, I'm ok with either.
Come to think of it, max_frames 0 is no worse than max_frames
UINT_MAX. Sure, let's do that.
^ permalink raw reply
* Re: [PATCH] net: ipv4: Use BUG_ON directly instead of a if condition followed by BUG
From: zhong jiang @ 2018-09-11 1:12 UTC (permalink / raw)
To: Jiri Benc; +Cc: davem, edumazet, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20180910173957.5f3728e6@redhat.com>
On 2018/9/10 23:39, Jiri Benc wrote:
> On Mon, 10 Sep 2018 22:38:02 +0800, zhong jiang wrote:
>> + BUG(skb_copy_bits(skb, ptr, &tmp, 1));
> I doubt this builds.
This is my fault. should Be BUG_ON
Thanks,
zhong jiang
> Jiri
>
> .
>
^ permalink raw reply
* [PATCH net-next v3 03/17] zinc: ChaCha20 generic C implementation
From: Jason A. Donenfeld @ 2018-09-11 1:08 UTC (permalink / raw)
To: linux-kernel, netdev, davem, gregkh
Cc: Jason A. Donenfeld, Andy Lutomirski, Samuel Neves,
Jean-Philippe Aumasson, linux-crypto
In-Reply-To: <20180911010838.8818-1-Jason@zx2c4.com>
This implements the ChaCha20 permutation as a single C statement, by way
of the comma operator, which the compiler is able to simplify
terrifically.
Information: https://cr.yp.to/chacha.html
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Greg KH <gregkh@linuxfoundation.org>
Cc: Samuel Neves <sneves@dei.uc.pt>
Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
Cc: linux-crypto@vger.kernel.org
---
include/zinc/chacha20.h | 53 +++++++++++
lib/zinc/Kconfig | 4 +
lib/zinc/Makefile | 4 +
lib/zinc/chacha20/chacha20.c | 168 +++++++++++++++++++++++++++++++++++
lib/zinc/main.c | 5 ++
5 files changed, 234 insertions(+)
create mode 100644 include/zinc/chacha20.h
create mode 100644 lib/zinc/chacha20/chacha20.c
diff --git a/include/zinc/chacha20.h b/include/zinc/chacha20.h
new file mode 100644
index 000000000000..d09afbccb601
--- /dev/null
+++ b/include/zinc/chacha20.h
@@ -0,0 +1,53 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ */
+
+#ifndef _ZINC_CHACHA20_H
+#define _ZINC_CHACHA20_H
+
+#include <asm/unaligned.h>
+#include <linux/simd.h>
+#include <linux/kernel.h>
+#include <linux/types.h>
+
+enum {
+ CHACHA20_IV_SIZE = 16,
+ CHACHA20_KEY_SIZE = 32,
+ CHACHA20_BLOCK_SIZE = 64,
+ HCHACHA20_KEY_SIZE = 32,
+ HCHACHA20_NONCE_SIZE = 16
+};
+
+struct chacha20_ctx {
+ u32 key[8];
+ u32 counter[4];
+} __aligned(32);
+
+void chacha20_fpu_init(void);
+
+static inline void chacha20_init(struct chacha20_ctx *state,
+ const u8 key[CHACHA20_KEY_SIZE],
+ const u64 nonce)
+{
+ state->key[0] = get_unaligned_le32(key + 0);
+ state->key[1] = get_unaligned_le32(key + 4);
+ state->key[2] = get_unaligned_le32(key + 8);
+ state->key[3] = get_unaligned_le32(key + 12);
+ state->key[4] = get_unaligned_le32(key + 16);
+ state->key[5] = get_unaligned_le32(key + 20);
+ state->key[6] = get_unaligned_le32(key + 24);
+ state->key[7] = get_unaligned_le32(key + 28);
+ state->counter[0] = state->counter[1] = 0;
+ state->counter[2] = nonce & U32_MAX;
+ state->counter[3] = nonce >> 32;
+}
+void chacha20(struct chacha20_ctx *state, u8 *dst, const u8 *src, u32 len,
+ simd_context_t simd_context);
+
+/* Derived key should be 32-bit aligned */
+void hchacha20(u8 derived_key[CHACHA20_KEY_SIZE],
+ const u8 nonce[HCHACHA20_NONCE_SIZE],
+ const u8 key[HCHACHA20_KEY_SIZE], simd_context_t simd_context);
+
+#endif /* _ZINC_CHACHA20_H */
diff --git a/lib/zinc/Kconfig b/lib/zinc/Kconfig
index aa4f8d449d6b..5311a0d6ba8b 100644
--- a/lib/zinc/Kconfig
+++ b/lib/zinc/Kconfig
@@ -6,6 +6,10 @@ config ZINC
select NEON
select KERNEL_MODE_NEON
+config ZINC_CHACHA20
+ bool
+ select ZINC
+
config ZINC_DEBUG
bool "Zinc cryptography library debugging and self-tests"
depends on ZINC
diff --git a/lib/zinc/Makefile b/lib/zinc/Makefile
index dad47573de42..0b5a964bfba6 100644
--- a/lib/zinc/Makefile
+++ b/lib/zinc/Makefile
@@ -3,6 +3,10 @@ ccflags-y += -Wframe-larger-than=8192
ccflags-y += -D'pr_fmt(fmt)=KBUILD_MODNAME ": " fmt'
ccflags-$(CONFIG_ZINC_DEBUG) += -DDEBUG
+ifeq ($(CONFIG_ZINC_CHACHA20),y)
+zinc-y += chacha20/chacha20.o
+endif
+
zinc-y += main.o
obj-$(CONFIG_ZINC) := zinc.o
diff --git a/lib/zinc/chacha20/chacha20.c b/lib/zinc/chacha20/chacha20.c
new file mode 100644
index 000000000000..ab5ef07dd4b5
--- /dev/null
+++ b/lib/zinc/chacha20/chacha20.c
@@ -0,0 +1,168 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
+ *
+ * Implementation of the ChaCha20 stream cipher.
+ *
+ * Information: https://cr.yp.to/chacha.html
+ */
+
+#include <zinc/chacha20.h>
+
+#include <linux/kernel.h>
+#include <crypto/algapi.h>
+
+#ifndef HAVE_CHACHA20_ARCH_IMPLEMENTATION
+void __init chacha20_fpu_init(void)
+{
+}
+static inline bool chacha20_arch(u8 *out, const u8 *in, const size_t len,
+ const u32 key[8], const u32 counter[4],
+ simd_context_t simd_context)
+{
+ return false;
+}
+static inline bool hchacha20_arch(u8 *derived_key, const u8 *nonce,
+ const u8 *key, simd_context_t simd_context)
+{
+ return false;
+}
+#endif
+
+#define EXPAND_32_BYTE_K 0x61707865U, 0x3320646eU, 0x79622d32U, 0x6b206574U
+
+#define QUARTER_ROUND(x, a, b, c, d) ( \
+ x[a] += x[b], \
+ x[d] = rol32((x[d] ^ x[a]), 16), \
+ x[c] += x[d], \
+ x[b] = rol32((x[b] ^ x[c]), 12), \
+ x[a] += x[b], \
+ x[d] = rol32((x[d] ^ x[a]), 8), \
+ x[c] += x[d], \
+ x[b] = rol32((x[b] ^ x[c]), 7) \
+)
+
+#define C(i, j) (i * 4 + j)
+
+#define DOUBLE_ROUND(x) ( \
+ /* Column Round */ \
+ QUARTER_ROUND(x, C(0, 0), C(1, 0), C(2, 0), C(3, 0)), \
+ QUARTER_ROUND(x, C(0, 1), C(1, 1), C(2, 1), C(3, 1)), \
+ QUARTER_ROUND(x, C(0, 2), C(1, 2), C(2, 2), C(3, 2)), \
+ QUARTER_ROUND(x, C(0, 3), C(1, 3), C(2, 3), C(3, 3)), \
+ /* Diagonal Round */ \
+ QUARTER_ROUND(x, C(0, 0), C(1, 1), C(2, 2), C(3, 3)), \
+ QUARTER_ROUND(x, C(0, 1), C(1, 2), C(2, 3), C(3, 0)), \
+ QUARTER_ROUND(x, C(0, 2), C(1, 3), C(2, 0), C(3, 1)), \
+ QUARTER_ROUND(x, C(0, 3), C(1, 0), C(2, 1), C(3, 2)) \
+)
+
+#define TWENTY_ROUNDS(x) ( \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x), \
+ DOUBLE_ROUND(x) \
+)
+
+static void chacha20_block_generic(__le32 *stream, u32 *state)
+{
+ u32 x[CHACHA20_BLOCK_SIZE / sizeof(u32)];
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(x); ++i)
+ x[i] = state[i];
+
+ TWENTY_ROUNDS(x);
+
+ for (i = 0; i < ARRAY_SIZE(x); ++i)
+ stream[i] = cpu_to_le32(x[i] + state[i]);
+
+ ++state[12];
+}
+
+static void chacha20_generic(u8 *out, const u8 *in, u32 len, const u32 key[8],
+ const u32 counter[4])
+{
+ __le32 buf[CHACHA20_BLOCK_SIZE / sizeof(__le32)];
+ u32 x[] = {
+ EXPAND_32_BYTE_K,
+ key[0], key[1], key[2], key[3],
+ key[4], key[5], key[6], key[7],
+ counter[0], counter[1], counter[2], counter[3]
+ };
+
+ if (out != in)
+ memmove(out, in, len);
+
+ while (len >= CHACHA20_BLOCK_SIZE) {
+ chacha20_block_generic(buf, x);
+ crypto_xor(out, (u8 *)buf, CHACHA20_BLOCK_SIZE);
+ len -= CHACHA20_BLOCK_SIZE;
+ out += CHACHA20_BLOCK_SIZE;
+ }
+ if (len) {
+ chacha20_block_generic(buf, x);
+ crypto_xor(out, (u8 *)buf, len);
+ }
+}
+
+void chacha20(struct chacha20_ctx *state, u8 *dst, const u8 *src, u32 len,
+ simd_context_t simd_context)
+{
+ if (!chacha20_arch(dst, src, len, state->key, state->counter,
+ simd_context))
+ chacha20_generic(dst, src, len, state->key, state->counter);
+ state->counter[0] += (len + 63) / 64;
+}
+EXPORT_SYMBOL(chacha20);
+
+static void hchacha20_generic(u8 derived_key[CHACHA20_KEY_SIZE],
+ const u8 nonce[HCHACHA20_NONCE_SIZE],
+ const u8 key[HCHACHA20_KEY_SIZE])
+{
+ __le32 *out = (__force __le32 *)derived_key;
+ u32 x[] = { EXPAND_32_BYTE_K,
+ get_unaligned_le32(key + 0),
+ get_unaligned_le32(key + 4),
+ get_unaligned_le32(key + 8),
+ get_unaligned_le32(key + 12),
+ get_unaligned_le32(key + 16),
+ get_unaligned_le32(key + 20),
+ get_unaligned_le32(key + 24),
+ get_unaligned_le32(key + 28),
+ get_unaligned_le32(nonce + 0),
+ get_unaligned_le32(nonce + 4),
+ get_unaligned_le32(nonce + 8),
+ get_unaligned_le32(nonce + 12)
+ };
+
+ TWENTY_ROUNDS(x);
+
+ out[0] = cpu_to_le32(x[0]);
+ out[1] = cpu_to_le32(x[1]);
+ out[2] = cpu_to_le32(x[2]);
+ out[3] = cpu_to_le32(x[3]);
+ out[4] = cpu_to_le32(x[12]);
+ out[5] = cpu_to_le32(x[13]);
+ out[6] = cpu_to_le32(x[14]);
+ out[7] = cpu_to_le32(x[15]);
+}
+
+/* Derived key should be 32-bit aligned */
+void hchacha20(u8 derived_key[CHACHA20_KEY_SIZE],
+ const u8 nonce[HCHACHA20_NONCE_SIZE],
+ const u8 key[HCHACHA20_KEY_SIZE], simd_context_t simd_context)
+{
+ if (!hchacha20_arch(derived_key, nonce, key, simd_context))
+ hchacha20_generic(derived_key, nonce, key);
+}
+/* Deliberately not EXPORT_SYMBOL'd, since there are few reasons why somebody
+ * should be using this directly, rather than via xchacha20. Revisit only in
+ * the unlikely event that somebody has a good reason to export this.
+ */
diff --git a/lib/zinc/main.c b/lib/zinc/main.c
index ceece33ff5a7..7e8e84b706b7 100644
--- a/lib/zinc/main.c
+++ b/lib/zinc/main.c
@@ -3,6 +3,8 @@
* Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
*/
+#include <zinc/chacha20.h>
+
#include <linux/init.h>
#include <linux/module.h>
@@ -17,6 +19,9 @@
static int __init mod_init(void)
{
+#ifdef CONFIG_ZINC_CHACHA20
+ chacha20_fpu_init();
+#endif
return 0;
}
--
2.18.0
^ permalink raw reply related
* Re: [PATCH net] ipv6: use rt6_info members when dst is set in rt6_fill_node
From: Hangbin Liu @ 2018-09-11 1:04 UTC (permalink / raw)
To: David Ahern; +Cc: Xin Long, network dev, davem, Roopa Prabhu
In-Reply-To: <c519ac67-a32e-4b68-76d4-49378e1440df@cumulusnetworks.com>
On Mon, Sep 10, 2018 at 01:07:11PM -0600, David Ahern wrote:
> On 9/10/18 11:55 AM, Xin Long wrote:
> > On Tue, Sep 11, 2018 at 12:13 AM David Ahern <dsa@cumulusnetworks.com> wrote:
> >>
> >> On 9/9/18 12:29 AM, Xin Long wrote:
> >>>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> >>>>> index 18e00ce..e554922 100644
> >>>>> --- a/net/ipv6/route.c
> >>>>> +++ b/net/ipv6/route.c
> >>>>> @@ -4670,20 +4670,33 @@ static int rt6_fill_node(struct net *net, struct sk_buff *skb,
> >>>>> int iif, int type, u32 portid, u32 seq,
> >>>>> unsigned int flags)
> >>>>> {
> >>>>> - struct rtmsg *rtm;
> >>>>> + struct rt6key *fib6_prefsrc, *fib6_dst, *fib6_src;
> >>>>> + struct rt6_info *rt6 = (struct rt6_info *)dst;
> >>>>> + u32 *pmetrics, table, fib6_flags;
> >>>>> struct nlmsghdr *nlh;
> >>>>> + struct rtmsg *rtm;
> >>>>> long expires = 0;
> >>>>> - u32 *pmetrics;
> >>>>> - u32 table;
> >>>>>
> >>>>> nlh = nlmsg_put(skb, portid, seq, type, sizeof(*rtm), flags);
> >>>>> if (!nlh)
> >>>>> return -EMSGSIZE;
> >>>>>
> >>>>> + if (rt6) {
> >>>>> + fib6_dst = &rt6->rt6i_dst;
> >>>>> + fib6_src = &rt6->rt6i_src;
> >>>>> + fib6_flags = rt6->rt6i_flags;
> >>>>> + fib6_prefsrc = &rt6->rt6i_prefsrc;
> >>>>> + } else {
> >>>>> + fib6_dst = &rt->fib6_dst;
> >>>>> + fib6_src = &rt->fib6_src;
> >>>>> + fib6_flags = rt->fib6_flags;
> >>>>> + fib6_prefsrc = &rt->fib6_prefsrc;
> >>>>> + }
> >>>>
> >>>> Unless I am missing something at the moment, an rt6_info can only have
> >>>> the same dst, src and prefsrc as the fib6_info on which it is based.
> >>>> Thus, only the flags is needed above. That simplifies this patch a lot.
> >>> If dst, src and prefsrc in rt6_info are always the same as these in fib6_info,
> >>> why do we need them in rt6_info? we could just get it by 'from'.
> >>>
> >>
> >> I just sent a patch removing rt6i_prefsrc. It is set with only 1 reader
> >> that can be converted.
> >>
> >> rt6i_src is checked against the fib6_info to invalidate a dst if the src
> >> has changed, so a valid rt will always have the same rt6i_src as the
> >> rt->from.
> >>
> >> rt6i_dst is set to the dest address / 128 in cases, so it should be used
> >> for rt6_info cases above.
> > So that means, I will use rt6i_dst and rt6i_flags when dst is set?
> > how about I use rt6i_src there as well? just to make it look clear.
> > and plus the gw/nh dump fix in rt6_fill_node():
> > - if (rt->fib6_nsiblings) {
> > + if (rt6) {
> > + if (fib6_flags & RTF_GATEWAY)
> > + if (nla_put_in6_addr(skb, RTA_GATEWAY,
> > + &rt6->rt6i_gateway) < 0)
> > + goto nla_put_failure;
> > +
> > + if (dst->dev && nla_put_u32(skb, RTA_OIF, dst->dev->ifindex))
> > + goto nla_put_failure;
> > + } else if (rt->fib6_nsiblings) {
> > struct fib6_info *sibling, *next_sibling;
> > struct nlattr *mp;
> >
> > looks good to you?
> >
>
> sure
Hmm... Then how to deal the other nh info filled by rt6_nexthop_info()?
I was planed to fix the gw issue[1] by syncing the gw and flags info. Like
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 18e00ce..62621b4 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -998,6 +998,21 @@ static void ip6_rt_copy_init(struct rt6_info *rt, struct fib6_info *ort)
rt->rt6i_prefsrc = ort->fib6_prefsrc;
}
+static void rt6_update_info(struct rt6_info *rt)
+{
+ struct fib6_info *from;
+
+ rcu_read_lock();
+ from = rcu_dereference(rt->from);
+ fib6_info_hold(from);
+ rcu_read_unlock();
+
+ from->fib6_flags = rt->rt6i_flags;
+ from->fib6_nh.nh_gw = rt->rt6i_gateway;
+
+ fib6_info_release(from);
+}
+
static struct fib6_node* fib6_backtrack(struct fib6_node *fn,
struct in6_addr *saddr)
{
@@ -3446,6 +3463,8 @@ static void rt6_do_redirect(struct dst_entry *dst, struct sock *sk, struct sk_bu
goto out;
}
+ rt6_update_info(nrt);
+
netevent.old = &rt->dst;
netevent.new = &nrt->dst;
netevent.daddr = &msg->dest;
--
2.5.5
[1] https://patchwork.ozlabs.org/patch/966972/
Thanks
Hangbin
^ permalink raw reply related
* [PATCH][net-next][v2] netlink: remove hash::nelems check in netlink_insert
From: Li RongQing @ 2018-09-11 1:05 UTC (permalink / raw)
To: netdev
The type of hash::nelems has been changed from size_t to atom_t
which in fact is int, so not need to check if BITS_PER_LONG, that
is bit number of size_t, is bigger than 32
and rht_grow_above_max() will be called to check if hashtable is
too big, ensure it can not bigger than 1<<31
Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
net/netlink/af_netlink.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index b4a29bcc33b9..e3a0538ec0be 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -574,11 +574,6 @@ static int netlink_insert(struct sock *sk, u32 portid)
if (nlk_sk(sk)->bound)
goto err;
- err = -ENOMEM;
- if (BITS_PER_LONG > 32 &&
- unlikely(atomic_read(&table->hash.nelems) >= UINT_MAX))
- goto err;
-
nlk_sk(sk)->portid = portid;
sock_hold(sk);
--
2.16.2
^ permalink raw reply related
* Re: [RFC PATCH bpf-next v2 0/4] Implement bpf queue/stack maps
From: Alexei Starovoitov @ 2018-09-11 1:04 UTC (permalink / raw)
To: Mauricio Vasquez
Cc: Alexei Starovoitov, Daniel Borkmann, Network Development,
Joe Stringer
On Fri, Sep 7, 2018 at 1:40 PM, Mauricio Vasquez
<mauricio.vasquez@polito.it> wrote:
>
> I read the Joe's proposal and using that for this problem looks like a nice
> solution.
>
> I think a good trade-off for now would be to go ahead with a queue/stack map
> without preallocating support (or maybe include it having always in mind
> that this issue has to be solved in the near future) and then, as a
> separated work, try to use Joe's proposal in the map helpers.
>
> What do you think?
the problem with such approach is that it would mean that
non-prealloc stack/queue api will be different from future one
after verifier will get smarter.
The alternative would be to support by-value api only.
Meaning let stack/queue support value_size = 1,2,4,8 byte only.
Then bpf_push|pop_elem() helper will be by-value
instead of returning a pointer.
No rcu callback issues and implementation on the kernel
side can be much simpler.
I think simple array of value_size elems with head/tail indices
will be enough.
Once we have Joe's verifier improvements
we can add alloc/free bpf object management facility
and use 8-byte stack/queue mapas a stack of pointers.
I think decoupling memory operations alloc/free from
stack push/pop would be additional benefit of such design.
^ permalink raw reply
* Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE
From: Cong Wang @ 2018-09-11 0:59 UTC (permalink / raw)
To: Santosh Shilimkar
Cc: Sowmini Varadhan, Linux Kernel Network Developers, rds-devel
In-Reply-To: <d97c6391-4ca7-e36a-09da-483b93218507@oracle.com>
On Mon, Sep 10, 2018 at 5:56 PM Santosh Shilimkar
<santosh.shilimkar@oracle.com> wrote:
>
> On 9/10/2018 5:45 PM, Cong Wang wrote:
> > On Mon, Sep 10, 2018 at 5:26 PM Santosh Shilimkar
> > <santosh.shilimkar@oracle.com> wrote:
> >> Would you mind posting an updated patch please with call_rcu and
> >> above extended RCU grace period with rcu_read_lock. Thanks !!
> >
> > If you prefer to fix _two_ problems in one patch, sure.
> >
> > For the record, the bug this patch fixes is NOT same with the one
> > in rds_find_bound(), because there is no rds_find_bound() in
> > the backtrace. If you want to see the full backtrace, here it is:
> >
> > https://marc.info/?l=linux-netdev&m=153644444808962&w=2
> >
> > This is why I believe they are two problems.
> >
> > Whether fixing two problems in one patch or two patches is
> > merely a preference, I leave it up to you.
> >
> I had a partial fix as well in past but since it wasn't covering
> complete window, it was abandoned.
>
> Since we know the other race, it would be best to address it
> together and hence requested a combine patch. Thanks for help !!
No problem! v2 is coming shortly after passing syzbot test. :)
^ permalink raw reply
* Re: [Patch net] rds: mark bound socket with SOCK_RCU_FREE
From: Santosh Shilimkar @ 2018-09-11 0:56 UTC (permalink / raw)
To: Cong Wang; +Cc: Sowmini Varadhan, Linux Kernel Network Developers, rds-devel
In-Reply-To: <CAM_iQpVn4pKCLM6O2251t-PFeUGNW45yY0Bn4XC4zXqL2C=Hgw@mail.gmail.com>
On 9/10/2018 5:45 PM, Cong Wang wrote:
> On Mon, Sep 10, 2018 at 5:26 PM Santosh Shilimkar
> <santosh.shilimkar@oracle.com> wrote:
>> Would you mind posting an updated patch please with call_rcu and
>> above extended RCU grace period with rcu_read_lock. Thanks !!
>
> If you prefer to fix _two_ problems in one patch, sure.
>
> For the record, the bug this patch fixes is NOT same with the one
> in rds_find_bound(), because there is no rds_find_bound() in
> the backtrace. If you want to see the full backtrace, here it is:
>
> https://marc.info/?l=linux-netdev&m=153644444808962&w=2
>
> This is why I believe they are two problems.
>
> Whether fixing two problems in one patch or two patches is
> merely a preference, I leave it up to you.
>
I had a partial fix as well in past but since it wasn't covering
complete window, it was abandoned.
Since we know the other race, it would be best to address it
together and hence requested a combine patch. Thanks for help !!
Regards,
Santosh
^ 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