* Re: [PATCH] sh_eth: kill useless initializers
From: David Miller @ 2016-03-14 19:50 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, linux-renesas-soc
In-Reply-To: <2737382.tXjN0OZRaC@wasted.cogentembedded.com>
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: Sun, 13 Mar 2016 01:29:45 +0300
> Some of the local variable intializers in the driver turned out to be
> pointless, kill 'em.
>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Applied, thanks Sergei.
^ permalink raw reply
* Re: [PATCH 1/2] mISDN: Order IPAC register defines
From: David Miller @ 2016-03-14 19:51 UTC (permalink / raw)
To: mail; +Cc: isdn, netdev, linux-kernel
In-Reply-To: <56E4A3AE.8050407@maciej.szmigiero.name>
From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Date: Sun, 13 Mar 2016 00:18:06 +0100
> It looks like IPAC/ISAC chips register defines weren't in any particular
> order.
>
> Order them by their number to make it easier to spot holes.
>
> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Applied.
^ permalink raw reply
* Re: [PATCH 2/2] mISDN: Support DR6 indication in mISDNipac driver
From: David Miller @ 2016-03-14 19:51 UTC (permalink / raw)
To: mail; +Cc: isdn, netdev, linux-kernel
In-Reply-To: <56E4A3EB.4050507@maciej.szmigiero.name>
From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
Date: Sun, 13 Mar 2016 00:19:07 +0100
> According to figure 39 in PEB3086 data sheet, version 1.4 this indication
> replaces DR when layer 1 transition source state is F6.
>
> This fixes mISDN layer 1 getting stuck in F6 state in TE mode on
> Dialogic Diva 2.02 card (and possibly others) when NT deactivates it.
>
> Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Applied.
^ permalink raw reply
* [PATCH v2] netfilter: fix race condition in ipset save, swap and delete
From: Vishwanath Pai @ 2016-03-14 19:54 UTC (permalink / raw)
To: pablo, kaber, kadlec, netfilter-devel; +Cc: coreteam, johunt, netdev
I have updated the patch according to comments by Jozsef. Renamed
ref_kernel to ref_netlink, renamed _put/_get functions and updated the
description in commit log.
Thanks,
Vishwanath
--
netfilter: fix race condition in ipset save, swap and delete
This fix adds a new reference counter (ref_netlink) for the struct ip_set.
The other reference counter (ref) can be swapped out by ip_set_swap and we
need a separate counter to keep track of references for netlink events
like dump. Using the same ref counter for dump causes a race condition
which can be demonstrated by the following script:
#!/bin/sh
ipset create hash_ip1 hash:ip family inet hashsize 1024 maxelem 500000 \
counters
ipset create hash_ip2 hash:ip family inet hashsize 300000 maxelem 500000 \
counters
ipset create hash_ip3 hash:ip family inet hashsize 1024 maxelem 500000 \
counters
ipset save &
ipset swap hash_ip3 hash_ip2
ipset destroy hash_ip3 /* will crash the machine */
Swap will exchange the values of ref so destroy will see ref = 0 instead of
ref = 1. With this fix in place swap will not succeed because ipset save
still has ref_netlink on the set (ip_set_swap doesn't swap ref_netlink).
Both delete and swap will error out if ref_netlink != 0 on the set.
Note: The changes to *_head functions is because previously we would
increment ref whenever we called these functions, we don't do that
anymore.
Reviewed-by: Joshua Hunt <johunt@akamai.com>
Signed-off-by: Vishwanath Pai <vpai@akamai.com>
--
diff --git a/include/linux/netfilter/ipset/ip_set.h b/include/linux/netfilter/ipset/ip_set.h
index 0e1f433..f48b8a6 100644
--- a/include/linux/netfilter/ipset/ip_set.h
+++ b/include/linux/netfilter/ipset/ip_set.h
@@ -234,6 +234,10 @@ struct ip_set {
spinlock_t lock;
/* References to the set */
u32 ref;
+ /* References to the set for netlink events like dump,
+ * ref can be swapped out by ip_set_swap
+ */
+ u32 ref_netlink;
/* The core set type */
struct ip_set_type *type;
/* The type variant doing the real job */
diff --git a/net/netfilter/ipset/ip_set_bitmap_gen.h b/net/netfilter/ipset/ip_set_bitmap_gen.h
index b0bc475..2e8e7e5 100644
--- a/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -95,7 +95,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
if (!nested)
goto nla_put_failure;
if (mtype_do_head(skb, map) ||
- nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
+ nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref)) ||
nla_put_net32(skb, IPSET_ATTR_MEMSIZE, htonl(memsize)))
goto nla_put_failure;
if (unlikely(ip_set_put_flags(skb, set)))
diff --git a/net/netfilter/ipset/ip_set_core.c b/net/netfilter/ipset/ip_set_core.c
index 95db43f..a558075 100644
--- a/net/netfilter/ipset/ip_set_core.c
+++ b/net/netfilter/ipset/ip_set_core.c
@@ -497,6 +497,26 @@ __ip_set_put(struct ip_set *set)
write_unlock_bh(&ip_set_ref_lock);
}
+/* set->ref can be swapped out by ip_set_swap, netlink events (like dump) need
+ * a separate reference counter
+ */
+static inline void
+__ip_set_get_netlink(struct ip_set *set)
+{
+ write_lock_bh(&ip_set_ref_lock);
+ set->ref_netlink++;
+ write_unlock_bh(&ip_set_ref_lock);
+}
+
+static inline void
+__ip_set_put_netlink(struct ip_set *set)
+{
+ write_lock_bh(&ip_set_ref_lock);
+ BUG_ON(set->ref_netlink == 0);
+ set->ref_netlink--;
+ write_unlock_bh(&ip_set_ref_lock);
+}
+
/* Add, del and test set entries from kernel.
*
* The set behind the index must exist and must be referenced
@@ -999,7 +1019,7 @@ static int ip_set_destroy(struct net *net, struct sock *ctnl,
if (!attr[IPSET_ATTR_SETNAME]) {
for (i = 0; i < inst->ip_set_max; i++) {
s = ip_set(inst, i);
- if (s && s->ref) {
+ if (s && (s->ref || s->ref_netlink)) {
ret = -IPSET_ERR_BUSY;
goto out;
}
@@ -1021,7 +1041,7 @@ static int ip_set_destroy(struct net *net, struct sock *ctnl,
if (!s) {
ret = -ENOENT;
goto out;
- } else if (s->ref) {
+ } else if (s->ref || s->ref_netlink) {
ret = -IPSET_ERR_BUSY;
goto out;
}
@@ -1168,6 +1188,9 @@ static int ip_set_swap(struct net *net, struct sock *ctnl, struct sk_buff *skb,
from->family == to->family))
return -IPSET_ERR_TYPE_MISMATCH;
+ if (from->ref_netlink || to->ref_netlink)
+ return -EBUSY;
+
strncpy(from_name, from->name, IPSET_MAXNAMELEN);
strncpy(from->name, to->name, IPSET_MAXNAMELEN);
strncpy(to->name, from_name, IPSET_MAXNAMELEN);
@@ -1203,7 +1226,7 @@ ip_set_dump_done(struct netlink_callback *cb)
if (set->variant->uref)
set->variant->uref(set, cb, false);
pr_debug("release set %s\n", set->name);
- __ip_set_put_byindex(inst, index);
+ __ip_set_put_netlink(set);
}
return 0;
}
@@ -1325,7 +1348,7 @@ dump_last:
if (!cb->args[IPSET_CB_ARG0]) {
/* Start listing: make sure set won't be destroyed */
pr_debug("reference set\n");
- set->ref++;
+ set->ref_netlink++;
}
write_unlock_bh(&ip_set_ref_lock);
nlh = start_msg(skb, NETLINK_CB(cb->skb).portid,
@@ -1393,7 +1416,7 @@ release_refcount:
if (set->variant->uref)
set->variant->uref(set, cb, false);
pr_debug("release set %s\n", set->name);
- __ip_set_put_byindex(inst, index);
+ __ip_set_put_netlink(set);
cb->args[IPSET_CB_ARG0] = 0;
}
out:
diff --git a/net/netfilter/ipset/ip_set_hash_gen.h b/net/netfilter/ipset/ip_set_hash_gen.h
index e5336ab..d32fd6b 100644
--- a/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/net/netfilter/ipset/ip_set_hash_gen.h
@@ -1082,7 +1082,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
if (nla_put_u32(skb, IPSET_ATTR_MARKMASK, h->markmask))
goto nla_put_failure;
#endif
- if (nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
+ if (nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref)) ||
nla_put_net32(skb, IPSET_ATTR_MEMSIZE, htonl(memsize)))
goto nla_put_failure;
if (unlikely(ip_set_put_flags(skb, set)))
diff --git a/net/netfilter/ipset/ip_set_list_set.c b/net/netfilter/ipset/ip_set_list_set.c
index bbede95..00f92ae 100644
--- a/net/netfilter/ipset/ip_set_list_set.c
+++ b/net/netfilter/ipset/ip_set_list_set.c
@@ -457,7 +457,7 @@ list_set_head(struct ip_set *set, struct sk_buff *skb)
if (!nested)
goto nla_put_failure;
if (nla_put_net32(skb, IPSET_ATTR_SIZE, htonl(map->size)) ||
- nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
+ nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref)) ||
nla_put_net32(skb, IPSET_ATTR_MEMSIZE,
htonl(sizeof(*map) + n * set->dsize)))
goto nla_put_failure;
^ permalink raw reply related
* Re: [PATCH net-next 1/2] rtnetlink: add new RTM_GETSTATS message to dump link stats
From: David Miller @ 2016-03-14 19:56 UTC (permalink / raw)
To: jiri; +Cc: roopa, netdev, jhs
In-Reply-To: <20160314190435.GA23419@nanopsycho.orion>
From: Jiri Pirko <jiri@resnulli.us>
Date: Mon, 14 Mar 2016 20:04:35 +0100
> I believe that using *any* structs to send over netlink is a mistake.
> Netlink is capable to transfer everything using attrs. Easy to compose,
> easy to parse. easy to extend. Couple of more bytes in the message? So what?
> For newly introduced things, I suggest to do this properly.
It is not so straight-forward.
What to put into the header is a tradeoff.
The most basic use cases should be as efficient as possible, and if we
can put reasonable knobs into the base commend header we should do that
as avoiding attribute processing makes things faster.
And I think in this case it is reasonable to put the mask in there.
The only problem I see with this series is the naming of the netlink
command (it isn't a "new" operation, and the "del" is unused).
Maybe the suggestion to use just "GET" as the name is ok.
^ permalink raw reply
* Re: [PATCH net-next 0/3] net: dsa: finer bridging control
From: David Miller @ 2016-03-14 20:05 UTC (permalink / raw)
To: vivien.didelot
Cc: netdev, linux-kernel, kernel, f.fainelli, andrew, jiri, idosch,
kevin.smith
In-Reply-To: <1457900494-10266-1-git-send-email-vivien.didelot@savoirfairelinux.com>
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Date: Sun, 13 Mar 2016 16:21:31 -0400
> This patchset renames the bridging routines of the DSA layer, make the
> unbridging routine return void, and rework the DSA netdev notifier handler,
> similar to what the Mellanox Spectrum driver does.
>
> Changes RFC -> v1:
> - drop unused NETDEV_PRECHANGEUPPER case
> - add Andrew's Tested-by tag
Series applied, thanks Vivien.
^ permalink raw reply
* Re: [PATCHv2] vsock: Fix blocking ops call in prepare_to_wait
From: Laura Abbott @ 2016-03-14 20:07 UTC (permalink / raw)
To: David Miller, imbrenda; +Cc: linux-kernel, netdev, labbott
In-Reply-To: <20160314.152403.1490973796884839758.davem@davemloft.net>
On 03/14/2016 12:24 PM, David Miller wrote:
> From: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
> Date: Fri, 11 Mar 2016 13:39:23 +0100
>
>> I think I found a problem with the patch submitted by Laura Abbott
>> ( https://lkml.org/lkml/2016/2/4/711 ): we might miss wakeups.
>> Since the condition is not checked between the prepare_to_wait and the
>> schedule(), if a wakeup happens after the condition is checked but before
>> the sleep happens, and we miss it. ( A description of the problem can be
>> found here: http://www.makelinux.net/ldd3/chp-6-sect-2 ).
>>
>> My solution (see patch below) is to shrink the area influenced by
>> prepare_to_wait, but keeping the fragile section around the condition, and
>> keep the rest of the code in "normal" running state. This way the sleep is
>> correct and the other functions don't need to worry. The only caveat here
>> is that the function(s) called to verify the conditions are really not
>> allowed to sleep, so if you need synchronization in the backend of e.g.
>> vsock_stream_has_space(), you should use spinlocks and not mutexes.
>>
>> In case we want to be able to sleep while waiting for conditions, we can
>> consider this instead: https://lwn.net/Articles/628628/ .
>>
>>
>> I stumbled on this problem while working on fixing the upcoming virtio
>> backend for vsock, below is the patch I had prepared, with the original
>> message.
>
> Can someone please look at this? Who maintains this code anyways?
>
Nobody was listed in MAINTAINERS. I tried cc-ing some of the e-mail addresses
of the original authors (vmware?) when sending the original patch and they
all bounced.
Thanks,
Laura
^ permalink raw reply
* Re: [PATCH v2 0/3] net/phy: Improvements to Cavium Thunder MDIO code.
From: David Daney @ 2016-03-14 20:12 UTC (permalink / raw)
To: David Miller
Cc: ddaney.cavm, netdev, linux-arm-kernel, f.fainelli, rric, sgoutham,
galak, ijc+devicetree, mark.rutland, pawel.moll, robh+dt,
linux-kernel, rchintakuntla, david.daney
In-Reply-To: <20160314.152751.2108458769066613871.davem@davemloft.net>
On 03/14/2016 12:27 PM, David Miller wrote:
> From: David Daney <ddaney.cavm@gmail.com>
> Date: Fri, 11 Mar 2016 09:53:08 -0800
>
>> Changes from v1:
>>
>> - In 1/3 Add back check for non-OF objects in bgx_init_of_phy(). It
>> is probably not necessary, but better safe than sorry...
>>
>> The firmware on many Cavium Thunder systems configures the MDIO bus
>> hardware to be probed as a PCI device. In order to use the MDIO bus
>> drivers in this configuration, we must add PCI probing to the driver.
>>
>> There are two parts to this set of three patches:
>>
>> 1) Cleanup the PHY probing code in thunder_bgx.c to handle the case
>> where there is no PHY attached to a port, as well as being more
>> robust in the face of driver loading order by use of
>> -EPROBE_DEFER.
>>
>> 2) Split mdio-octeon.c into two drivers, one with platform probing,
>> and the other with PCI probing. Common code is shared between the
>> two.
>>
>> Tested on several different Thunder and OCTEON systems, also compile
>> tested on x86_64.
>
> Series applied, thanks David.
Thanks, but ... I was going to send another revision.
See: https://lkml.org/lkml/2016/3/11/721
There were a couple of items I wanted to fix.
1) Missing MODULE_LICENSE() in new common code source file results in
taint warnings.
2) Fix device reference counts for -EPROBE_DEFER case.
At this point, I think the best path forward is for me to rebase against
net-next and send you a small fixup set to what was merged.
What do you think?
David Daney
^ permalink raw reply
* [PATCH net v2] ppp: ensure file->private_data can't be overridden
From: Guillaume Nault @ 2016-03-14 20:17 UTC (permalink / raw)
To: netdev; +Cc: Paul Mackerras, Alan Cox, David Miller
Locking ppp_mutex must be done before dereferencing file->private_data,
otherwise it could be modified before ppp_unattached_ioctl() takes the
lock. This could lead ppp_unattached_ioctl() to override ->private_data,
thus leaking reference to the ppp_file previously pointed to.
v2: lock all ppp_ioctl() instead of just checking private_data in
ppp_unattached_ioctl(), to avoid ambiguous behaviour.
Fixes: f3ff8a4d80e8 ("ppp: push BKL down into the driver")
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
drivers/net/ppp/ppp_generic.c | 31 +++++++++++++++++--------------
1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index d61da9ec..8c8eedb 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -575,7 +575,7 @@ static int get_filter(void __user *arg, struct sock_filter **p)
static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
{
- struct ppp_file *pf = file->private_data;
+ struct ppp_file *pf;
struct ppp *ppp;
int err = -EFAULT, val, val2, i;
struct ppp_idle idle;
@@ -585,9 +585,14 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
void __user *argp = (void __user *)arg;
int __user *p = argp;
- if (!pf)
- return ppp_unattached_ioctl(current->nsproxy->net_ns,
- pf, file, cmd, arg);
+ mutex_lock(&ppp_mutex);
+
+ pf = file->private_data;
+ if (!pf) {
+ err = ppp_unattached_ioctl(current->nsproxy->net_ns,
+ pf, file, cmd, arg);
+ goto out;
+ }
if (cmd == PPPIOCDETACH) {
/*
@@ -602,7 +607,6 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
* this fd and reopening /dev/ppp.
*/
err = -EINVAL;
- mutex_lock(&ppp_mutex);
if (pf->kind == INTERFACE) {
ppp = PF_TO_PPP(pf);
rtnl_lock();
@@ -616,15 +620,13 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
} else
pr_warn("PPPIOCDETACH file->f_count=%ld\n",
atomic_long_read(&file->f_count));
- mutex_unlock(&ppp_mutex);
- return err;
+ goto out;
}
if (pf->kind == CHANNEL) {
struct channel *pch;
struct ppp_channel *chan;
- mutex_lock(&ppp_mutex);
pch = PF_TO_CHANNEL(pf);
switch (cmd) {
@@ -646,17 +648,16 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
err = chan->ops->ioctl(chan, cmd, arg);
up_read(&pch->chan_sem);
}
- mutex_unlock(&ppp_mutex);
- return err;
+ goto out;
}
if (pf->kind != INTERFACE) {
/* can't happen */
pr_err("PPP: not interface or channel??\n");
- return -EINVAL;
+ err = -EINVAL;
+ goto out;
}
- mutex_lock(&ppp_mutex);
ppp = PF_TO_PPP(pf);
switch (cmd) {
case PPPIOCSMRU:
@@ -831,7 +832,10 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
default:
err = -ENOTTY;
}
+
+out:
mutex_unlock(&ppp_mutex);
+
return err;
}
@@ -844,7 +848,6 @@ static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf,
struct ppp_net *pn;
int __user *p = (int __user *)arg;
- mutex_lock(&ppp_mutex);
switch (cmd) {
case PPPIOCNEWUNIT:
/* Create a new ppp unit */
@@ -894,7 +897,7 @@ static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf,
default:
err = -ENOTTY;
}
- mutex_unlock(&ppp_mutex);
+
return err;
}
--
2.7.0
^ permalink raw reply related
* Re: [PATCH v2 0/3] net/phy: Improvements to Cavium Thunder MDIO code.
From: David Miller @ 2016-03-14 20:18 UTC (permalink / raw)
To: ddaney
Cc: ddaney.cavm, netdev, linux-arm-kernel, f.fainelli, rric, sgoutham,
galak, ijc+devicetree, mark.rutland, pawel.moll, robh+dt,
linux-kernel, rchintakuntla, david.daney
In-Reply-To: <56E71B45.1010805@caviumnetworks.com>
From: David Daney <ddaney@caviumnetworks.com>
Date: Mon, 14 Mar 2016 13:12:53 -0700
> At this point, I think the best path forward is for me to rebase
> against net-next and send you a small fixup set to what was merged.
>
> What do you think?
That is in fact the one and only option.
^ permalink raw reply
* Re: [PATCH net-next 1/2] rtnetlink: add new RTM_GETSTATS message to dump link stats
From: Jiri Pirko @ 2016-03-14 20:22 UTC (permalink / raw)
To: David Miller; +Cc: roopa, netdev, jhs
In-Reply-To: <20160314.155640.1574437783302725000.davem@davemloft.net>
Mon, Mar 14, 2016 at 08:56:40PM CET, davem@davemloft.net wrote:
>From: Jiri Pirko <jiri@resnulli.us>
>Date: Mon, 14 Mar 2016 20:04:35 +0100
>
>> I believe that using *any* structs to send over netlink is a mistake.
>> Netlink is capable to transfer everything using attrs. Easy to compose,
>> easy to parse. easy to extend. Couple of more bytes in the message? So what?
>> For newly introduced things, I suggest to do this properly.
>
>It is not so straight-forward.
>
>What to put into the header is a tradeoff.
>
>The most basic use cases should be as efficient as possible, and if we
>can put reasonable knobs into the base commend header we should do that
>as avoiding attribute processing makes things faster.
Faster in which matter? Regarding the user app complexicity, I think that
processing attrs is very simple and straightforward. I might be missing
something very obvious, but I don't think that processing header struct
is that much easier that it advocates for the unclean approach.
I personally believe that introducing possibility to pass Netlink
headers was a mistake from the very beginning. If we have clean Netlink
interface, why to pollute that with ioclt-like struct approach. Okay,
the mistake was done. But as I said, for the future usage, I believe
that it should be avoided.
>
>And I think in this case it is reasonable to put the mask in there.
>
>The only problem I see with this series is the naming of the netlink
>command (it isn't a "new" operation, and the "del" is unused).
>
>Maybe the suggestion to use just "GET" as the name is ok.
+1
^ permalink raw reply
* Re: [PATCH net-next 08/13] net/mlx5e: Add fragmented memory support for RX multi packet WQE
From: Eric Dumazet @ 2016-03-14 20:23 UTC (permalink / raw)
To: Saeed Mahameed
Cc: Saeed Mahameed, David S. Miller, Linux Netdev List, Or Gerlitz,
Eran Ben Elisha, Tal Alon, Tariq Toukan, Jesper Dangaard Brouer
In-Reply-To: <CALzJLG9rB4iBnOC0f+8NopjdS407-WJ9b7KFUPO9hjbwFCyP-A@mail.gmail.com>
On Mon, 2016-03-14 at 20:16 +0200, Saeed Mahameed wrote:
> we can do special accounting for ooo like issues in the stack (maybe
> count page references and sum up page sizes as you suggest), device
> drivers shouldn't have special handling/accounting to protect against
> such cases.
The existing skb->truesize is doing this already.
The fact that some drivers use PAGE_SIZE/2 instead of PAGE_SIZE is an
heuristic that is mostly okay, and we accept the risk :
Even if a smart attack is happening, host will consume 200 XB instead of
100 XB.
But pretending to use 128 bytes is simply a dangerous weapon over your
head, since you end up consuming 1600 XB.
With tcp_mem[2] being 18% of physical memory, you end up consuming all
physical memory and crash.
I can tell you that these kind of attacks are very real. I´ve seen them
in action.
^ permalink raw reply
* Re: [PATCH net-next 08/13] net/mlx5e: Add fragmented memory support for RX multi packet WQE
From: Eric Dumazet @ 2016-03-14 20:26 UTC (permalink / raw)
To: achiad shochat
Cc: Saeed Mahameed, Saeed Mahameed, David S. Miller,
Linux Netdev List, Or Gerlitz, Eran Ben Elisha, Tal Alon,
Tariq Toukan, Jesper Dangaard Brouer
In-Reply-To: <CAEHy93JgcwKFRK+6ZVXgee77N6h+QbBNoAhE61sd0t6-0dnE1A@mail.gmail.com>
On Mon, 2016-03-14 at 21:16 +0200, achiad shochat wrote:
> Eric, am I missing something here or the new scheme was not clear to
> you previously?
I simply do not want to see drivers using
1) SKB_TRUESIZE()
or
2)
skb->truesize = some_expression
Drivers should not assume they know better than core networking stack.
^ permalink raw reply
* Re: [PATCH net-next 08/13] net/mlx5e: Add fragmented memory support for RX multi packet WQE
From: Eric Dumazet @ 2016-03-14 20:29 UTC (permalink / raw)
To: achiad shochat
Cc: Saeed Mahameed, Saeed Mahameed, David S. Miller,
Linux Netdev List, Or Gerlitz, Eran Ben Elisha, Tal Alon,
Tariq Toukan, Jesper Dangaard Brouer
In-Reply-To: <CAEHy93JgcwKFRK+6ZVXgee77N6h+QbBNoAhE61sd0t6-0dnE1A@mail.gmail.com>
On Mon, 2016-03-14 at 21:16 +0200, achiad shochat wrote:
> I really do not see why the new scheme is more DOSable than the common
> scheme of pre-allocating SKB using napi_alloc_skb().
Because sizeof(skb_shared_info) is big enough that if you allocate 128
bytes, you end up using 512 bytes or even more.
In practice it is good enough.
^ permalink raw reply
* Re: [PATCH 1/5] net: macb: Fix coding style error message
From: Michal Simek @ 2016-03-14 20:47 UTC (permalink / raw)
To: Moritz Fischer, nicolas.ferre
Cc: michal.simek, joe, davem, netdev, linux-kernel,
moritz.fischer.private
In-Reply-To: <1457896247-25934-2-git-send-email-moritz.fischer@ettus.com>
On 13.3.2016 20:10, Moritz Fischer wrote:
> checkpatch.pl gave the following error:
>
> ERROR: space required before the open parenthesis '('
> + for(; p < end; p++, offset += 4)
>
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 50c9410..4370f37 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -496,7 +496,7 @@ static void macb_update_stats(struct macb *bp)
>
> WARN_ON((unsigned long)(end - p - 1) != (MACB_TPF - MACB_PFR) / 4);
>
> - for(; p < end; p++, offset += 4)
> + for (; p < end; p++, offset += 4)
> *p += bp->macb_reg_readl(bp, offset);
> }
>
>
Acked-by: Michal Simek <michal.simek@xilinx.com>
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH 5/5] net: macb: Fix simple typo.
From: Michal Simek @ 2016-03-14 20:47 UTC (permalink / raw)
To: Moritz Fischer, nicolas.ferre
Cc: michal.simek, joe, davem, netdev, linux-kernel,
moritz.fischer.private
In-Reply-To: <1457896247-25934-6-git-send-email-moritz.fischer@ettus.com>
On 13.3.2016 20:10, Moritz Fischer wrote:
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index a0c01e5..681e5bf 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -127,7 +127,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value)
> }
>
> /* Find the CPU endianness by using the loopback bit of NCR register. When the
> - * CPU is in big endian we need to program swaped mode for management
> + * CPU is in big endian we need to program swapped mode for management
> * descriptor access.
> */
> static bool hw_is_native_io(void __iomem *addr)
>
Remove dot at the end of subject and feel free to add my:
Acked-by: Michal Simek <michal.simek@xilinx.com>
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH 4/5] net: macb: Use ether_addr_copy over memcpy
From: Michal Simek @ 2016-03-14 20:48 UTC (permalink / raw)
To: Moritz Fischer, nicolas.ferre
Cc: michal.simek, joe, davem, netdev, linux-kernel,
moritz.fischer.private
In-Reply-To: <1457896247-25934-5-git-send-email-moritz.fischer@ettus.com>
On 13.3.2016 20:10, Moritz Fischer wrote:
> Checkpatch suggests using ether_addr_copy over memcpy
> to copy the mac address.
>
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 53400f6..a0c01e5 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -2891,7 +2891,7 @@ static int macb_probe(struct platform_device *pdev)
>
> mac = of_get_mac_address(np);
> if (mac)
> - memcpy(bp->dev->dev_addr, mac, ETH_ALEN);
> + ether_addr_copy(bp->dev->dev_addr, mac);
> else
> macb_get_hwaddr(bp);
>
>
Acked-by: Michal Simek <michal.simek@xilinx.com>
M
^ permalink raw reply
* Re: [PATCH v7 net-next] ravb: Add dma queue interrupt support
From: Sergei Shtylyov @ 2016-03-14 20:48 UTC (permalink / raw)
To: Yoshihiro Kaneko, netdev
Cc: David S. Miller, Simon Horman, Magnus Damm, linux-renesas-soc
In-Reply-To: <1457892712-29024-1-git-send-email-ykaneko0929@gmail.com>
Hello.
On 03/13/2016 09:11 PM, Yoshihiro Kaneko wrote:
> From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
>
> This patch supports the following interrupts.
>
> - One interrupt for multiple (timestamp, error, gPTP)
> - One interrupt for emac
> - Four interrupts for dma queue (best effort rx/tx, network control rx/tx)
>
> This patch improve efficiency of the interrupt handler by adding the
> interrupt handler corresponding to each interrupt source described
> above. Additionally, it reduces the number of times of the access to
> EthernetAVB IF.
> Also this patch prevent this driver depends on the whim of a boot loader.
>
> [ykaneko0929@gmail.com: define bit names of registers]
> [ykaneko0929@gmail.com: add comment for gen3 only registers]
> [ykaneko0929@gmail.com: fix coding style]
> [ykaneko0929@gmail.com: update changelog]
> [ykaneko0929@gmail.com: gen3: fix initialization of interrupts]
> [ykaneko0929@gmail.com: gen3: fix clearing interrupts]
> [ykaneko0929@gmail.com: gen3: add helper function for request_irq()]
> [ykaneko0929@gmail.com: gen3: remove IRQF_SHARED flag for request_irq()]
> [ykaneko0929@gmail.com: revert ravb_close() and ravb_ptp_stop()]
> [ykaneko0929@gmail.com: avoid calling free_irq() to non-hooked interrupts]
> [ykaneko0929@gmail.com: make NC/BE interrupt handler a function]
> [ykaneko0929@gmail.com: make timestamp interrupt handler a function]
> [ykaneko0929@gmail.com: timestamp interrupt is handled in multiple
> interrupt handler instead of dma queue interrupt handler]
> Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
> Signed-off-by: Yoshihiro Kaneko <ykaneko0929@gmail.com>
[...]
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index 8f2c4fb..8fa8ffe 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
[...]
> @@ -757,14 +806,73 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id)
> result = IRQ_HANDLED;
> }
>
> - if (iss & ISS_CGIS)
> - result = ravb_ptp_interrupt(ndev);
> + /* gPTP interrupt status summary */
> + if ((iss & ISS_CGIS) && ravb_ptp_interrupt(ndev) == IRQ_HANDLED)
> + result = IRQ_HANDLED;
Wait, this seems like a bug in the existing driver! Please do fix it with
a separate patch against net.git. Sorry about missing (or even adding) it
while cleaning up the driver before submission...
[...]
Looks fine otherwise, however the new features and the fixes shouldn't be
mixed together, so I couldn't ACK yet. I'll go test it on gen2...
MBR, Sergei
^ permalink raw reply
* Re: [PATCH 3/5] net: macb: Address checkpatch 'check' suggestions
From: Michal Simek @ 2016-03-14 20:49 UTC (permalink / raw)
To: Moritz Fischer, nicolas.ferre
Cc: michal.simek, joe, davem, netdev, linux-kernel,
moritz.fischer.private
In-Reply-To: <1457896247-25934-4-git-send-email-moritz.fischer@ettus.com>
On 13.3.2016 20:10, Moritz Fischer wrote:
> This commit deals with a bunch of checkpatch suggestions
> that without changing behavior make checkpatch happier.
>
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 46 +++++++++++++++++++------------------
> 1 file changed, 24 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index c2d31c5..53400f6 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -184,7 +184,7 @@ static void macb_get_hwaddr(struct macb *bp)
>
> pdata = dev_get_platdata(&bp->pdev->dev);
>
> - /* Check all 4 address register for vaild address */
> + /* Check all 4 address register for valid address */
> for (i = 0; i < 4; i++) {
> bottom = macb_or_gem_readl(bp, SA1B + i * 8);
> top = macb_or_gem_readl(bp, SA1T + i * 8);
> @@ -292,7 +292,7 @@ static void macb_set_tx_clk(struct clk *clk, int speed, struct net_device *dev)
> ferr = DIV_ROUND_UP(ferr, rate / 100000);
> if (ferr > 5)
> netdev_warn(dev, "unable to generate target frequency: %ld Hz\n",
> - rate);
> + rate);
>
> if (clk_set_rate(clk, rate_rounded))
> netdev_err(dev, "adjusting tx_clk failed.\n");
> @@ -426,7 +426,7 @@ static int macb_mii_init(struct macb *bp)
> macb_writel(bp, NCR, MACB_BIT(MPE));
>
> bp->mii_bus = mdiobus_alloc();
> - if (bp->mii_bus == NULL) {
> + if (!bp->mii_bus) {
> err = -ENOMEM;
> goto err_out;
> }
> @@ -435,7 +435,7 @@ static int macb_mii_init(struct macb *bp)
> bp->mii_bus->read = &macb_mdio_read;
> bp->mii_bus->write = &macb_mdio_write;
> snprintf(bp->mii_bus->id, MII_BUS_ID_SIZE, "%s-%x",
> - bp->pdev->name, bp->pdev->id);
> + bp->pdev->name, bp->pdev->id);
> bp->mii_bus->priv = bp;
> bp->mii_bus->parent = &bp->dev->dev;
> pdata = dev_get_platdata(&bp->pdev->dev);
> @@ -656,7 +656,7 @@ static void macb_tx_interrupt(struct macb_queue *queue)
> queue_writel(queue, ISR, MACB_BIT(TCOMP));
>
> netdev_vdbg(bp->dev, "macb_tx_interrupt status = 0x%03lx\n",
> - (unsigned long)status);
> + (unsigned long)status);
>
> head = queue->tx_head;
> for (tail = queue->tx_tail; tail != head; tail++) {
> @@ -725,10 +725,10 @@ static void gem_rx_refill(struct macb *bp)
>
> bp->rx_prepared_head++;
>
> - if (bp->rx_skbuff[entry] == NULL) {
> + if (!bp->rx_skbuff[entry]) {
> /* allocate sk_buff for this free entry in ring */
> skb = netdev_alloc_skb(bp->dev, bp->rx_buffer_size);
> - if (unlikely(skb == NULL)) {
> + if (unlikely(!skb)) {
> netdev_err(bp->dev,
> "Unable to allocate sk_buff\n");
> break;
> @@ -762,7 +762,7 @@ static void gem_rx_refill(struct macb *bp)
> wmb();
>
> netdev_vdbg(bp->dev, "rx ring: prepared head %d, tail %d\n",
> - bp->rx_prepared_head, bp->rx_tail);
> + bp->rx_prepared_head, bp->rx_tail);
> }
>
> /* Mark DMA descriptors from begin up to and not including end as unused */
> @@ -876,8 +876,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> len = desc->ctrl & bp->rx_frm_len_mask;
>
> netdev_vdbg(bp->dev, "macb_rx_frame frags %u - %u (len %u)\n",
> - macb_rx_ring_wrap(first_frag),
> - macb_rx_ring_wrap(last_frag), len);
> + macb_rx_ring_wrap(first_frag),
> + macb_rx_ring_wrap(last_frag), len);
>
> /* The ethernet header starts NET_IP_ALIGN bytes into the
> * first buffer. Since the header is 14 bytes, this makes the
> @@ -916,7 +916,8 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> frag_len = len - offset;
> }
> skb_copy_to_linear_data_offset(skb, offset,
> - macb_rx_buffer(bp, frag), frag_len);
> + macb_rx_buffer(bp, frag),
> + frag_len);
> offset += bp->rx_buffer_size;
> desc = macb_rx_desc(bp, frag);
> desc->addr &= ~MACB_BIT(RX_USED);
> @@ -934,7 +935,7 @@ static int macb_rx_frame(struct macb *bp, unsigned int first_frag,
> bp->stats.rx_packets++;
> bp->stats.rx_bytes += skb->len;
> netdev_vdbg(bp->dev, "received skb of length %u, csum: %08x\n",
> - skb->len, skb->csum);
> + skb->len, skb->csum);
> netif_receive_skb(skb);
>
> return 0;
> @@ -999,7 +1000,7 @@ static int macb_poll(struct napi_struct *napi, int budget)
> work_done = 0;
>
> netdev_vdbg(bp->dev, "poll: status = %08lx, budget = %d\n",
> - (unsigned long)status, budget);
> + (unsigned long)status, budget);
>
> work_done = bp->macbgem_ops.mog_rx(bp, budget);
> if (work_done < budget) {
> @@ -1214,7 +1215,7 @@ static unsigned int macb_tx_map(struct macb *bp,
> }
>
> /* Should never happen */
> - if (unlikely(tx_skb == NULL)) {
> + if (unlikely(!tx_skb)) {
> netdev_err(bp->dev, "BUG! empty skb!\n");
> return 0;
> }
> @@ -1284,16 +1285,16 @@ static int macb_start_xmit(struct sk_buff *skb, struct net_device *dev)
>
> #if defined(DEBUG) && defined(VERBOSE_DEBUG)
> netdev_vdbg(bp->dev,
> - "start_xmit: queue %hu len %u head %p data %p tail %p end %p\n",
> - queue_index, skb->len, skb->head, skb->data,
> - skb_tail_pointer(skb), skb_end_pointer(skb));
> + "start_xmit: queue %hu len %u head %p data %p tail %p end %p\n",
> + queue_index, skb->len, skb->head, skb->data,
> + skb_tail_pointer(skb), skb_end_pointer(skb));
> print_hex_dump(KERN_DEBUG, "data: ", DUMP_PREFIX_OFFSET, 16, 1,
> skb->data, 16, true);
> #endif
>
> /* Count how many TX buffer descriptors are needed to send this
> * socket buffer: skb fragments of jumbo frames may need to be
> - * splitted into many buffer descriptors.
> + * split into many buffer descriptors.
> */
> count = DIV_ROUND_UP(skb_headlen(skb), bp->max_tx_length);
> nr_frags = skb_shinfo(skb)->nr_frags;
> @@ -1344,8 +1345,8 @@ static void macb_init_rx_buffer_size(struct macb *bp, size_t size)
>
> if (bp->rx_buffer_size % RX_BUFFER_MULTIPLE) {
> netdev_dbg(bp->dev,
> - "RX buffer must be multiple of %d bytes, expanding\n",
> - RX_BUFFER_MULTIPLE);
> + "RX buffer must be multiple of %d bytes, expanding\n",
> + RX_BUFFER_MULTIPLE);
> bp->rx_buffer_size =
> roundup(bp->rx_buffer_size, RX_BUFFER_MULTIPLE);
> }
> @@ -1368,7 +1369,7 @@ static void gem_free_rx_buffers(struct macb *bp)
> for (i = 0; i < RX_RING_SIZE; i++) {
> skb = bp->rx_skbuff[i];
>
> - if (skb == NULL)
> + if (!skb)
> continue;
>
> desc = &bp->rx_ring[i];
> @@ -1776,7 +1777,8 @@ static void macb_sethashtable(struct net_device *dev)
> unsigned int bitnr;
> struct macb *bp = netdev_priv(dev);
>
> - mc_filter[0] = mc_filter[1] = 0;
> + mc_filter[0] = 0;
> + mc_filter[1] = 0;
>
> netdev_for_each_mc_addr(ha, dev) {
> bitnr = hash_get_index(ha->addr);
>
Acked-by: Michal Simek <michal.simek@xilinx.com>
M
^ permalink raw reply
* Re: [PATCH RFC v2 00/32] Make DSA switches linux devices.
From: Andrew Lunn @ 2016-03-14 20:51 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Vivien Didelot, netdev
In-Reply-To: <56E712B2.9010905@gmail.com>
On Mon, Mar 14, 2016 at 12:36:18PM -0700, Florian Fainelli wrote:
> On 12/03/16 09:08, Andrew Lunn wrote:
> >> [snip]
> >>
> >>>
> >>> The third switch is as you would expect, dsa,member = <0 2>;
> >>
> >> I like that representation.
> >>
> >
> > ...
> >
> >> So does that mean you agree we do not need the DSA platform device
> >> anymore :)?
> >
> > It looks like it can be done without the DSA platform device.
> >
> > My previous approach was to keep the new binding as similar as
> > possible to the current one. If however, we decide we are going for
> > something totally new, we can remove this platform device.
>
> The old binding can and should remain available
Agreed. "marvell,dsa" will remain and will invoke a platform device,
as is today. That is the old binding.
I'm currently thinking that switch drivers will probe, and then call
something like
int dsa_switch_register(struct dsa_switch *ds, struct device_node *np);
The code behind that will be responsible for allocating the dst,
parsing the binding pointed to by np. It will also evaluate if all the
dsa links are available, and if so, call the setup() method in
ds->drv, etc.
> The new binding looks very similar to the previous one and this is
> certainly a good thing to do. With that in mind the new binding should
> probably be keeping the per-port Device Tree node representation and
> properties, because that one seems correct.
There are some changes. A cpu node will have the phandle to the
ethernet device. I really would like to only have a phandle to a phy,
but we will see, and i want to remove the limit of one cpu node. The
driver can then return -EINVAL during setup() if it cannot handle it.
> the parts that had gone
> wrong were definitively the "reg" property and the dsa,mii-bus property
> if not the dsa,ethernet as well, but code will talk.
reg at the switch level. Yes. We still need reg at the port level.
dsa,mii-bus is gone, and dsa,ethernet moved into the cpu port.
Andrew
^ permalink raw reply
* Re: [PATCH 2/5] net: macb: Fix coding style warnings
From: Michal Simek @ 2016-03-14 20:53 UTC (permalink / raw)
To: Moritz Fischer, nicolas.ferre
Cc: michal.simek, joe, davem, netdev, linux-kernel,
moritz.fischer.private
In-Reply-To: <1457896247-25934-3-git-send-email-moritz.fischer@ettus.com>
On 13.3.2016 20:10, Moritz Fischer wrote:
> This commit takes care of the coding style warnings
> that are mostly due to a different comment style and
> lines over 80 chars, as well as a dangling else.
>
> Signed-off-by: Moritz Fischer <moritz.fischer@ettus.com>
> ---
> drivers/net/ethernet/cadence/macb.c | 101 +++++++++++++++---------------------
> 1 file changed, 43 insertions(+), 58 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
> index 4370f37..c2d31c5 100644
> --- a/drivers/net/ethernet/cadence/macb.c
> +++ b/drivers/net/ethernet/cadence/macb.c
> @@ -58,8 +58,7 @@
>
> #define GEM_MTU_MIN_SIZE 68
>
> -/*
> - * Graceful stop timeouts in us. We should allow up to
> +/* Graceful stop timeouts in us. We should allow up to
> * 1 frame time (10 Mbits/s, full-duplex, ignoring collisions)
> */
> #define MACB_HALT_TIMEOUT 1230
> @@ -127,8 +126,7 @@ static void hw_writel(struct macb *bp, int offset, u32 value)
> writel_relaxed(value, bp->regs + offset);
> }
>
> -/*
> - * Find the CPU endianness by using the loopback bit of NCR register. When the
> +/* Find the CPU endianness by using the loopback bit of NCR register. When the
TBH: I would rather see this converting to kernel-doc format instead of
using this networking block.
Also splitting this to more patches will be better. Just by categories
but that's just my opinion.
Thanks,
Michal
^ permalink raw reply
* [PATCH] ipv6: Fix the pmtu path for connected UDP socket
From: Wei Wang @ 2016-03-14 20:59 UTC (permalink / raw)
To: David S . Miller; +Cc: Eric Dumazet, netdev, Wei Wang
From: Wei Wang <weiwan@google.com>
When ICMPV6_PKT_TOOBIG message is received by a connected UDP socket,
the new mtu value is not properly updated in the dst_entry associated
with the socket.
This leads to the issue that the mtu value returned by getsockopt(sockfd,
IPPROTO_IPV6, IPV6_MTU, ...) is wrong.
The fix is to update sk->sk_dst_cache and other corresponding fields
when a new routing cache is allocated for the new pmtu in UDP connected
socket case.
Signed-off-by: Wei Wang <weiwan@google.com>
---
include/net/ip6_route.h | 4 ++--
net/ipv6/ah6.c | 2 +-
net/ipv6/esp6.c | 2 +-
net/ipv6/icmp.c | 2 +-
net/ipv6/ip6_vti.c | 2 +-
net/ipv6/ipcomp6.c | 2 +-
net/ipv6/route.c | 21 +++++++++------------
7 files changed, 16 insertions(+), 19 deletions(-)
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 295d291..2b147a8 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -115,8 +115,8 @@ void rt6_purge_dflt_routers(struct net *net);
int rt6_route_rcv(struct net_device *dev, u8 *opt, int len,
const struct in6_addr *gwaddr);
-void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu, int oif,
- u32 mark);
+void ip6_update_pmtu(struct net *net, struct sock *sk, struct sk_buff *skb,
+ __be32 mtu, int oif, u32 mark);
void ip6_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, __be32 mtu);
void ip6_redirect(struct sk_buff *skb, struct net *net, int oif, u32 mark);
void ip6_redirect_no_header(struct sk_buff *skb, struct net *net, int oif,
diff --git a/net/ipv6/ah6.c b/net/ipv6/ah6.c
index 0630a4d5..2c926ec 100644
--- a/net/ipv6/ah6.c
+++ b/net/ipv6/ah6.c
@@ -664,7 +664,7 @@ static int ah6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, skb->dev->ifindex, 0);
else
- ip6_update_pmtu(skb, net, info, 0, 0);
+ ip6_update_pmtu(net, NULL, skb, info, 0, 0);
xfrm_state_put(x);
return 0;
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index 060a60b..b74847a 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -476,7 +476,7 @@ static int esp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, skb->dev->ifindex, 0);
else
- ip6_update_pmtu(skb, net, info, 0, 0);
+ ip6_update_pmtu(net, NULL, skb, info, 0, 0);
xfrm_state_put(x);
return 0;
diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 0a37ddc..03816f5 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -92,7 +92,7 @@ static void icmpv6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
struct net *net = dev_net(skb->dev);
if (type == ICMPV6_PKT_TOOBIG)
- ip6_update_pmtu(skb, net, info, 0, 0);
+ ip6_update_pmtu(net, NULL, skb, info, 0, 0);
else if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, skb->dev->ifindex, 0);
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index d90a11f..fa873ca 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -599,7 +599,7 @@ static int vti6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, skb->dev->ifindex, 0);
else
- ip6_update_pmtu(skb, net, info, 0, 0);
+ ip6_update_pmtu(net, NULL, skb, info, 0, 0);
xfrm_state_put(x);
return 0;
diff --git a/net/ipv6/ipcomp6.c b/net/ipv6/ipcomp6.c
index 1b9316e..c07a5ac 100644
--- a/net/ipv6/ipcomp6.c
+++ b/net/ipv6/ipcomp6.c
@@ -76,7 +76,7 @@ static int ipcomp6_err(struct sk_buff *skb, struct inet6_skb_parm *opt,
if (type == NDISC_REDIRECT)
ip6_redirect(skb, net, skb->dev->ifindex, 0);
else
- ip6_update_pmtu(skb, net, info, 0, 0);
+ ip6_update_pmtu(net, NULL, skb, info, 0, 0);
xfrm_state_put(x);
return 0;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index ed44663..8f6a5f1 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1346,7 +1346,7 @@ static bool rt6_cache_allowed_for_pmtu(const struct rt6_info *rt)
(rt->rt6i_flags & RTF_PCPU || rt->rt6i_node);
}
-static void __ip6_rt_update_pmtu(struct dst_entry *dst, const struct sock *sk,
+static void __ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
const struct ipv6hdr *iph, u32 mtu)
{
struct rt6_info *rt6 = (struct rt6_info *)dst;
@@ -1377,12 +1377,8 @@ static void __ip6_rt_update_pmtu(struct dst_entry *dst, const struct sock *sk,
nrt6 = ip6_rt_cache_alloc(rt6, daddr, saddr);
if (nrt6) {
rt6_do_update_pmtu(nrt6, mtu);
-
- /* ip6_ins_rt(nrt6) will bump the
- * rt6->rt6i_node->fn_sernum
- * which will fail the next rt6_check() and
- * invalidate the sk->sk_dst_cache.
- */
+ if (sk)
+ ip6_dst_store(sk, &nrt6->dst, daddr, saddr);
ip6_ins_rt(nrt6);
}
}
@@ -1394,8 +1390,8 @@ static void ip6_rt_update_pmtu(struct dst_entry *dst, struct sock *sk,
__ip6_rt_update_pmtu(dst, sk, skb ? ipv6_hdr(skb) : NULL, mtu);
}
-void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu,
- int oif, u32 mark)
+void ip6_update_pmtu(struct net *net, struct sock *sk,
+ struct sk_buff *skb, __be32 mtu, int oif, u32 mark)
{
const struct ipv6hdr *iph = (struct ipv6hdr *) skb->data;
struct dst_entry *dst;
@@ -1410,15 +1406,16 @@ void ip6_update_pmtu(struct sk_buff *skb, struct net *net, __be32 mtu,
dst = ip6_route_output(net, NULL, &fl6);
if (!dst->error)
- __ip6_rt_update_pmtu(dst, NULL, iph, ntohl(mtu));
+ __ip6_rt_update_pmtu(dst, sk, iph, ntohl(mtu));
dst_release(dst);
}
EXPORT_SYMBOL_GPL(ip6_update_pmtu);
void ip6_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, __be32 mtu)
{
- ip6_update_pmtu(skb, sock_net(sk), mtu,
- sk->sk_bound_dev_if, sk->sk_mark);
+ ip6_update_pmtu(sock_net(sk),
+ (sk->sk_state != TCP_ESTABLISHED) ? NULL : sk,
+ skb, mtu, sk->sk_bound_dev_if, sk->sk_mark);
}
EXPORT_SYMBOL_GPL(ip6_sk_update_pmtu);
--
2.7.0.rc3.207.g0ac5344
^ permalink raw reply related
* Re: [PATCH v6 net-next 2/2] tcp: Add Redundant Data Bundling (RDB)
From: Eric Dumazet @ 2016-03-14 21:15 UTC (permalink / raw)
To: Bendik Rønning Opstad
Cc: David S. Miller, netdev, Yuchung Cheng, Neal Cardwell,
Andreas Petlund, Carsten Griwodz, Pål Halvorsen,
Jonas Markussen, Kristian Evensen, Kenneth Klette Jonassen
In-Reply-To: <1457028388-18226-3-git-send-email-bro.devel+kernel@gmail.com>
On Thu, 2016-03-03 at 19:06 +0100, Bendik Rønning Opstad wrote:
> Redundant Data Bundling (RDB) is a mechanism for TCP aimed at reducing
> the latency for applications sending time-dependent data.
>
> Latency-sensitive applications or services, such as online games,
> remote control systems, and VoIP, produce traffic with thin-stream
> characteristics, characterized by small packets and relatively high
> inter-transmission times (ITT). When experiencing packet loss, such
> latency-sensitive applications are heavily penalized by the need to
> retransmit lost packets, which increases the latency by a minimum of
> one RTT for the lost packet. Packets coming after a lost packet are
> held back due to head-of-line blocking, causing increased delays for
> all data segments until the lost packet has been retransmitted.
Acked-by: Eric Dumazet <edumazet@google.com>
Note that RDB probably should get some SNMP counters,
so that we get an idea of how many times a loss could be repaired.
Ideally, if the path happens to be lossless, all these pro active
bundles are overhead. Might be useful to make RDB conditional to
tp->total_retrans or something.
^ permalink raw reply
* Re: [PATCH net-next 06/13] net/mlx5e: Support RX multi-packet WQE (Striding RQ)
From: Jesper Dangaard Brouer @ 2016-03-14 21:33 UTC (permalink / raw)
To: Saeed Mahameed
Cc: David S. Miller, netdev, Or Gerlitz, Eran Ben Elisha, Tal Alon,
Tariq Toukan, Achiad Shochat, brouer
In-Reply-To: <1457703594-9482-7-git-send-email-saeedm@mellanox.com>
On Fri, 11 Mar 2016 15:39:47 +0200 Saeed Mahameed <saeedm@mellanox.com> wrote:
> From: Tariq Toukan <tariqt@mellanox.com>
>
> Introduce the feature of multi-packet WQE (RX Work Queue Element)
> referred to as (MPWQE or Striding RQ), in which WQEs are larger
> and serve multiple packets each.
>
> Every WQE consists of many strides of the same size, every received
> packet is aligned to a beginning of a stride and is written to
> consecutive strides within a WQE.
I really like this HW support! :-)
I noticed the "Multi-Packet WQE" send format, but I could not find the
receive part in the programmers ref doc, until I started looking after
"stride".
> In the regular approach, each regular WQE is big enough to be capable
> of serving one received packet of any size up to MTU or 64K in case of
> device LRO is enabeled, making it very wasteful when dealing with
> small packets or device LRO is enabeled.
>
> For its flexibility, MPWQE allows a better memory utilization (implying
> improvements in CPU utilization and packet rate) as packets consume
> strides according to their size, preserving the rest of the WQE to be
> available for other packets.
It does allow significant better memory utilization (even if Eric
cannot see it, I can).
One issue with this approach is that we no-longer can use the
packet-data as the skb->data pointer. (AFAIK because we cannot use
dma_unmap any longer, and instead we need to use dma_sync).
Thus, for every single packet you are now allocating a new memory area
for skb->data.
> MPWQE default configuration:
> NUM WQEs = 16
> Strides Per WQE = 1024
> Stride Size = 128
> Performance tested on ConnectX4-Lx 50G.
>
> * Netperf single TCP stream:
> - message size = 1024, bw raised from ~12300 mbps to 14900 mbps (+20%)
> - message size = 65536, bw raised from ~21800 mbps to 33500 mbps (+50%)
> - with other message sized we saw some gain or no degradation.
>
> * Netperf multi TCP stream:
> - No degradation, line rate reached.
>
> * Pktgen: packet loss in bursts of N small messages (64byte), single
> stream
> - | num packets | packets loss before | packets loss after
> | 2K | ~ 1K | 0
> | 16K | ~13K | 0
> | 32K | ~29K | 14K
>
> As expected as the driver can recive as many small packets (<=128) as
> the number of total strides in the ring (default = 1024 * 16) vs. 1024
> (default ring size regardless of packets size) before this feautre.
>
> Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
> Signed-off-by: Achiad Shochat <achiad@mellanox.com>
> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
> ---
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 71 +++++++++++-
> .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 15 ++-
> drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 109 +++++++++++++----
> drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 126 ++++++++++++++++++--
> include/linux/mlx5/device.h | 39 ++++++-
> include/linux/mlx5/mlx5_ifc.h | 13 ++-
> 6 files changed, 327 insertions(+), 46 deletions(-)
>
[...]
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -76,6 +76,33 @@ err_free_skb:
> return -ENOMEM;
> }
>
> +int mlx5e_alloc_rx_mpwqe(struct mlx5e_rq *rq, struct mlx5e_rx_wqe *wqe, u16 ix)
> +{
> + struct mlx5e_mpw_info *wi = &rq->wqe_info[ix];
> + int ret = 0;
> +
> + wi->dma_info.page = alloc_pages(GFP_ATOMIC | __GFP_COMP | __GFP_COLD,
> + MLX5_MPWRQ_WQE_PAGE_ORDER);
Order 5 page = 131072 bytes, but we only alloc 16 of them.
> + if (unlikely(!wi->dma_info.page))
> + return -ENOMEM;
> +
> + wi->dma_info.addr = dma_map_page(rq->pdev, wi->dma_info.page, 0,
> + rq->wqe_sz, PCI_DMA_FROMDEVICE);
Mapping the entire page is going to make PowerPC owners happy.
> + if (dma_mapping_error(rq->pdev, wi->dma_info.addr)) {
> + ret = -ENOMEM;
> + goto err_put_page;
> + }
> +
> + wi->consumed_strides = 0;
> + wqe->data.addr = cpu_to_be64(wi->dma_info.addr);
> +
> + return 0;
> +
> +err_put_page:
> + put_page(wi->dma_info.page);
> + return ret;
> +}
> +
[...]
> +void mlx5e_handle_rx_cqe_mpwrq(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
> +{
> + u16 cstrides = mpwrq_get_cqe_consumed_strides(cqe);
> + u16 stride_ix = mpwrq_get_cqe_stride_index(cqe);
> + u32 consumed_bytes = cstrides * MLX5_MPWRQ_STRIDE_SIZE;
> + u32 stride_offset = stride_ix * MLX5_MPWRQ_STRIDE_SIZE;
> + u16 wqe_id = be16_to_cpu(cqe->wqe_id);
> + struct mlx5e_mpw_info *wi = &rq->wqe_info[wqe_id];
> + struct mlx5e_rx_wqe *wqe = mlx5_wq_ll_get_wqe(&rq->wq, wqe_id);
> + struct sk_buff *skb;
> + u16 byte_cnt;
> + u16 cqe_bcnt;
> + u16 headlen;
> +
> + wi->consumed_strides += cstrides;
Ok, moving N strides, for next round.
> +
> + if (unlikely((cqe->op_own >> 4) != MLX5_CQE_RESP_SEND)) {
> + rq->stats.wqe_err++;
> + goto mpwrq_cqe_out;
> + }
> +
> + if (mpwrq_is_filler_cqe(cqe)) {
> + rq->stats.mpwqe_filler++;
> + goto mpwrq_cqe_out;
> + }
> +
> + skb = netdev_alloc_skb(rq->netdev, MLX5_MPWRQ_SMALL_PACKET_THRESHOLD);
> + if (unlikely(!skb))
> + goto mpwrq_cqe_out;
> +
> + dma_sync_single_for_cpu(rq->pdev, wi->dma_info.addr + stride_offset,
> + consumed_bytes, DMA_FROM_DEVICE);
> +
> + cqe_bcnt = mpwrq_get_cqe_byte_cnt(cqe);
> + headlen = min_t(u16, MLX5_MPWRQ_SMALL_PACKET_THRESHOLD, cqe_bcnt);
> + skb_copy_to_linear_data(skb,
> + page_address(wi->dma_info.page) + stride_offset,
> + headlen);
> + skb_put(skb, headlen);
> +
> + byte_cnt = cqe_bcnt - headlen;
> + if (byte_cnt) {
> + skb_frag_t *f0 = &skb_shinfo(skb)->frags[0];
> +
> + skb_shinfo(skb)->nr_frags = 1;
> +
> + skb->data_len = byte_cnt;
> + skb->len += byte_cnt;
> + skb->truesize = SKB_TRUESIZE(skb->len);
> +
> + get_page(wi->dma_info.page);
> + skb_frag_set_page(skb, 0, wi->dma_info.page);
> + skb_frag_size_set(f0, skb->data_len);
> + f0->page_offset = stride_offset + headlen;
> + }
> +
> + mlx5e_complete_rx_cqe(rq, cqe, cqe_bcnt, skb);
> +
> +mpwrq_cqe_out:
> + if (likely(wi->consumed_strides < MLX5_MPWRQ_NUM_STRIDES))
> + return;
Due to return statement, we keep working on the same big page, only
dma_sync'ing what we need.
> +
> + dma_unmap_page(rq->pdev, wi->dma_info.addr, rq->wqe_sz,
> + PCI_DMA_FROMDEVICE);
Page is first fully dma_unmap'ed after all stride-entries have been
processed/consumed.
> + put_page(wi->dma_info.page);
> + mlx5_wq_ll_pop(&rq->wq, cqe->wqe_id, &wqe->next.next_wqe_index);
> +}
> +
> int mlx5e_poll_rx_cq(struct mlx5e_cq *cq, int budget)
> {
> struct mlx5e_rq *rq = container_of(cq, struct mlx5e_rq, cq);
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
Author of http://www.iptv-analyzer.org
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v6 net-next 2/2] tcp: Add Redundant Data Bundling (RDB)
From: Yuchung Cheng @ 2016-03-14 21:54 UTC (permalink / raw)
To: Bendik Rønning Opstad
Cc: David S. Miller, netdev, Eric Dumazet, Neal Cardwell,
Andreas Petlund, Carsten Griwodz, Pål Halvorsen,
Jonas Markussen, Kristian Evensen, Kenneth Klette Jonassen
In-Reply-To: <1457028388-18226-3-git-send-email-bro.devel+kernel@gmail.com>
On Thu, Mar 3, 2016 at 10:06 AM, Bendik Rønning Opstad
<bro.devel@gmail.com> wrote:
>
> Redundant Data Bundling (RDB) is a mechanism for TCP aimed at reducing
> the latency for applications sending time-dependent data.
>
> Latency-sensitive applications or services, such as online games,
> remote control systems, and VoIP, produce traffic with thin-stream
> characteristics, characterized by small packets and relatively high
> inter-transmission times (ITT). When experiencing packet loss, such
> latency-sensitive applications are heavily penalized by the need to
> retransmit lost packets, which increases the latency by a minimum of
> one RTT for the lost packet. Packets coming after a lost packet are
> held back due to head-of-line blocking, causing increased delays for
> all data segments until the lost packet has been retransmitted.
>
> RDB enables a TCP sender to bundle redundant (already sent) data with
> TCP packets containing small segments of new data. By resending
> un-ACKed data from the output queue in packets with new data, RDB
> reduces the need to retransmit data segments on connections
> experiencing sporadic packet loss. By avoiding a retransmit, RDB
> evades the latency increase of at least one RTT for the lost packet,
> as well as alleviating head-of-line blocking for the packets following
> the lost packet. This makes the TCP connection more resistant to
> latency fluctuations, and reduces the application layer latency
> significantly in lossy environments.
>
> Main functionality added:
>
> o When a packet is scheduled for transmission, RDB builds and
> transmits a new SKB containing both the unsent data as well as
> data of previously sent packets from the TCP output queue.
>
> o RDB will only be used for streams classified as thin by the
> function tcp_stream_is_thin_dpifl(). This enforces a lower bound
> on the ITT for streams that may benefit from RDB, controlled by
> the sysctl variable net.ipv4.tcp_thin_dpifl_itt_lower_bound.
>
> o Loss detection of hidden loss events: When bundling redundant data
> with each packet, packet loss can be hidden from the TCP engine due
> to lack of dupACKs. This is because the loss is "repaired" by the
> redundant data in the packet coming after the lost packet. Based on
> incoming ACKs, such hidden loss events are detected, and CWR state
> is entered.
>
> RDB can be enabled on a connection with the socket option TCP_RDB, or
> on all new connections by setting the sysctl variable
> net.ipv4.tcp_rdb=1
>
> Cc: Andreas Petlund <apetlund@simula.no>
> Cc: Carsten Griwodz <griff@simula.no>
> Cc: Pål Halvorsen <paalh@simula.no>
> Cc: Jonas Markussen <jonassm@ifi.uio.no>
> Cc: Kristian Evensen <kristian.evensen@gmail.com>
> Cc: Kenneth Klette Jonassen <kennetkl@ifi.uio.no>
> Signed-off-by: Bendik Rønning Opstad <bro.devel+kernel@gmail.com>
> ---
> Documentation/networking/ip-sysctl.txt | 15 +++
> include/linux/skbuff.h | 1 +
> include/linux/tcp.h | 3 +-
> include/net/tcp.h | 15 +++
> include/uapi/linux/tcp.h | 1 +
> net/core/skbuff.c | 2 +-
> net/ipv4/Makefile | 3 +-
> net/ipv4/sysctl_net_ipv4.c | 25 ++++
> net/ipv4/tcp.c | 14 +-
> net/ipv4/tcp_input.c | 3 +
> net/ipv4/tcp_output.c | 48 ++++---
> net/ipv4/tcp_rdb.c | 228 +++++++++++++++++++++++++++++++++
> 12 files changed, 335 insertions(+), 23 deletions(-)
> create mode 100644 net/ipv4/tcp_rdb.c
>
> diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
> index 6a92b15..8f3f3bf 100644
> --- a/Documentation/networking/ip-sysctl.txt
> +++ b/Documentation/networking/ip-sysctl.txt
> @@ -716,6 +716,21 @@ tcp_thin_dpifl_itt_lower_bound - INTEGER
> calculated, which is used to classify whether a stream is thin.
> Default: 10000
>
> +tcp_rdb - BOOLEAN
> + Enable RDB for all new TCP connections.
Please describe RDB briefly, perhaps with a pointer to your paper.
I suggest have three level of controls:
0: disable RDB completely
1: enable indiv. thin-stream conn. to use RDB via TCP_RDB socket
options
2: enable RDB on all thin-stream conn. by default
currently it only provides mode 1 and 2. but there may be cases where
the administrator wants to disallow it (e.g., broken middle-boxes).
> + Default: 0
> +
> +tcp_rdb_max_bytes - INTEGER
> + Enable restriction on how many bytes an RDB packet can contain.
> + This is the total amount of payload including the new unsent data.
> + Default: 0
> +
> +tcp_rdb_max_packets - INTEGER
> + Enable restriction on how many previous packets in the output queue
> + RDB may include data from. A value of 1 will restrict bundling to
> + only the data from the last packet that was sent.
> + Default: 1
why two metrics on redundancy? It also seems better to
allow individual socket to select the redundancy level (e.g.,
setsockopt TCP_RDB=3 means <=3 pkts per bundle) vs a global setting.
This requires more bits in tcp_sock but 2-3 more is suffice.
/
> +
> tcp_limit_output_bytes - INTEGER
> Controls TCP Small Queue limit per tcp socket.
> TCP bulk sender tends to increase packets in flight until it
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 797cefb..0f2c9d1 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -2927,6 +2927,7 @@ int zerocopy_sg_from_iter(struct sk_buff *skb, struct iov_iter *frm);
> void skb_free_datagram(struct sock *sk, struct sk_buff *skb);
> void skb_free_datagram_locked(struct sock *sk, struct sk_buff *skb);
> int skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags);
> +void copy_skb_header(struct sk_buff *new, const struct sk_buff *old);
> int skb_copy_bits(const struct sk_buff *skb, int offset, void *to, int len);
> int skb_store_bits(struct sk_buff *skb, int offset, const void *from, int len);
> __wsum skb_copy_and_csum_bits(const struct sk_buff *skb, int offset, u8 *to,
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index bcbf51d..c84de15 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -207,9 +207,10 @@ struct tcp_sock {
> } rack;
> u16 advmss; /* Advertised MSS */
> u8 unused;
> - u8 nonagle : 4,/* Disable Nagle algorithm? */
> + u8 nonagle : 3,/* Disable Nagle algorithm? */
> thin_lto : 1,/* Use linear timeouts for thin streams */
> thin_dupack : 1,/* Fast retransmit on first dupack */
> + rdb : 1,/* Redundant Data Bundling enabled */
> repair : 1,
> frto : 1;/* F-RTO (RFC5682) activated in CA_Loss */
> u8 repair_queue;
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index d38eae9..2d42f4a 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -267,6 +267,9 @@ extern int sysctl_tcp_slow_start_after_idle;
> extern int sysctl_tcp_thin_linear_timeouts;
> extern int sysctl_tcp_thin_dupack;
> extern int sysctl_tcp_thin_dpifl_itt_lower_bound;
> +extern int sysctl_tcp_rdb;
> +extern int sysctl_tcp_rdb_max_bytes;
> +extern int sysctl_tcp_rdb_max_packets;
> extern int sysctl_tcp_early_retrans;
> extern int sysctl_tcp_limit_output_bytes;
> extern int sysctl_tcp_challenge_ack_limit;
> @@ -539,6 +542,8 @@ void __tcp_push_pending_frames(struct sock *sk, unsigned int cur_mss,
> bool tcp_may_send_now(struct sock *sk);
> int __tcp_retransmit_skb(struct sock *, struct sk_buff *);
> int tcp_retransmit_skb(struct sock *, struct sk_buff *);
> +int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
> + gfp_t gfp_mask);
> void tcp_retransmit_timer(struct sock *sk);
> void tcp_xmit_retransmit_queue(struct sock *);
> void tcp_simple_retransmit(struct sock *);
> @@ -556,6 +561,7 @@ void tcp_send_ack(struct sock *sk);
> void tcp_send_delayed_ack(struct sock *sk);
> void tcp_send_loss_probe(struct sock *sk);
> bool tcp_schedule_loss_probe(struct sock *sk);
> +void tcp_skb_append_data(struct sk_buff *from_skb, struct sk_buff *to_skb);
>
> /* tcp_input.c */
> void tcp_resume_early_retransmit(struct sock *sk);
> @@ -565,6 +571,11 @@ void tcp_reset(struct sock *sk);
> void tcp_skb_mark_lost_uncond_verify(struct tcp_sock *tp, struct sk_buff *skb);
> void tcp_fin(struct sock *sk);
>
> +/* tcp_rdb.c */
> +void tcp_rdb_ack_event(struct sock *sk, u32 flags);
> +int tcp_transmit_rdb_skb(struct sock *sk, struct sk_buff *xmit_skb,
> + unsigned int mss_now, gfp_t gfp_mask);
> +
> /* tcp_timer.c */
> void tcp_init_xmit_timers(struct sock *);
> static inline void tcp_clear_xmit_timers(struct sock *sk)
> @@ -763,6 +774,7 @@ struct tcp_skb_cb {
> union {
> struct {
> /* There is space for up to 20 bytes */
> + __u32 rdb_start_seq; /* Start seq of rdb data */
> } tx; /* only used for outgoing skbs */
> union {
> struct inet_skb_parm h4;
> @@ -1497,6 +1509,9 @@ static inline struct sk_buff *tcp_write_queue_prev(const struct sock *sk,
> #define tcp_for_write_queue_from_safe(skb, tmp, sk) \
> skb_queue_walk_from_safe(&(sk)->sk_write_queue, skb, tmp)
>
> +#define tcp_for_write_queue_reverse_from_safe(skb, tmp, sk) \
> + skb_queue_reverse_walk_from_safe(&(sk)->sk_write_queue, skb, tmp)
> +
> static inline struct sk_buff *tcp_send_head(const struct sock *sk)
> {
> return sk->sk_send_head;
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index fe95446..6799875 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -115,6 +115,7 @@ enum {
> #define TCP_CC_INFO 26 /* Get Congestion Control (optional) info */
> #define TCP_SAVE_SYN 27 /* Record SYN headers for new connections */
> #define TCP_SAVED_SYN 28 /* Get SYN headers recorded for connection */
> +#define TCP_RDB 29 /* Enable Redundant Data Bundling mechanism */
>
> struct tcp_repair_opt {
> __u32 opt_code;
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 7af7ec6..50bc5b0 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -1055,7 +1055,7 @@ static void skb_headers_offset_update(struct sk_buff *skb, int off)
> skb->inner_mac_header += off;
> }
>
> -static void copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
> +void copy_skb_header(struct sk_buff *new, const struct sk_buff *old)
> {
> __copy_skb_header(new, old);
>
> diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
> index bfa1336..459048c 100644
> --- a/net/ipv4/Makefile
> +++ b/net/ipv4/Makefile
> @@ -12,7 +12,8 @@ obj-y := route.o inetpeer.o protocol.o \
> tcp_offload.o datagram.o raw.o udp.o udplite.o \
> udp_offload.o arp.o icmp.o devinet.o af_inet.o igmp.o \
> fib_frontend.o fib_semantics.o fib_trie.o \
> - inet_fragment.o ping.o ip_tunnel_core.o gre_offload.o
> + inet_fragment.o ping.o ip_tunnel_core.o gre_offload.o \
> + tcp_rdb.o
>
> obj-$(CONFIG_NET_IP_TUNNEL) += ip_tunnel.o
> obj-$(CONFIG_SYSCTL) += sysctl_net_ipv4.o
> diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
> index f04320a..43b4390 100644
> --- a/net/ipv4/sysctl_net_ipv4.c
> +++ b/net/ipv4/sysctl_net_ipv4.c
> @@ -581,6 +581,31 @@ static struct ctl_table ipv4_table[] = {
> .extra1 = &tcp_thin_dpifl_itt_lower_bound_min,
> },
> {
> + .procname = "tcp_rdb",
> + .data = &sysctl_tcp_rdb,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + .extra2 = &one,
> + },
> + {
> + .procname = "tcp_rdb_max_bytes",
> + .data = &sysctl_tcp_rdb_max_bytes,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + },
> + {
> + .procname = "tcp_rdb_max_packets",
> + .data = &sysctl_tcp_rdb_max_packets,
> + .maxlen = sizeof(int),
> + .mode = 0644,
> + .proc_handler = proc_dointvec_minmax,
> + .extra1 = &zero,
> + },
> + {
> .procname = "tcp_early_retrans",
> .data = &sysctl_tcp_early_retrans,
> .maxlen = sizeof(int),
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 8421f3d..b53d4cb 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -288,6 +288,8 @@ int sysctl_tcp_autocorking __read_mostly = 1;
>
> int sysctl_tcp_thin_dpifl_itt_lower_bound __read_mostly = TCP_THIN_DPIFL_ITT_LOWER_BOUND_MIN;
>
> +int sysctl_tcp_rdb __read_mostly;
> +
> struct percpu_counter tcp_orphan_count;
> EXPORT_SYMBOL_GPL(tcp_orphan_count);
>
> @@ -407,6 +409,7 @@ void tcp_init_sock(struct sock *sk)
> u64_stats_init(&tp->syncp);
>
> tp->reordering = sock_net(sk)->ipv4.sysctl_tcp_reordering;
> + tp->rdb = sysctl_tcp_rdb;
> tcp_enable_early_retrans(tp);
> tcp_assign_congestion_control(sk);
>
> @@ -2412,6 +2415,13 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
> }
> break;
>
> + case TCP_RDB:
> + if (val < 0 || val > 1)
> + err = -EINVAL;
> + else
> + tp->rdb = val;
> + break;
> +
> case TCP_REPAIR:
> if (!tcp_can_repair_sock(sk))
> err = -EPERM;
> @@ -2842,7 +2852,9 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
> case TCP_THIN_DUPACK:
> val = tp->thin_dupack;
> break;
> -
> + case TCP_RDB:
> + val = tp->rdb;
> + break;
> case TCP_REPAIR:
> val = tp->repair;
> break;
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index e6e65f7..7b52ce4 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3537,6 +3537,9 @@ static inline void tcp_in_ack_event(struct sock *sk, u32 flags)
>
> if (icsk->icsk_ca_ops->in_ack_event)
> icsk->icsk_ca_ops->in_ack_event(sk, flags);
> +
> + if (unlikely(tcp_sk(sk)->rdb))
> + tcp_rdb_ack_event(sk, flags);
> }
>
> /* Congestion control has updated the cwnd already. So if we're in
> diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
> index 7d2c7a4..6f92fae 100644
> --- a/net/ipv4/tcp_output.c
> +++ b/net/ipv4/tcp_output.c
> @@ -897,8 +897,8 @@ out:
> * We are working here with either a clone of the original
> * SKB, or a fresh unique copy made by the retransmit engine.
> */
> -static int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
> - gfp_t gfp_mask)
> +int tcp_transmit_skb(struct sock *sk, struct sk_buff *skb, int clone_it,
> + gfp_t gfp_mask)
> {
> const struct inet_connection_sock *icsk = inet_csk(sk);
> struct inet_sock *inet;
> @@ -2110,9 +2110,12 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
> break;
> }
>
> - if (unlikely(tcp_transmit_skb(sk, skb, 1, gfp)))
> + if (unlikely(tcp_sk(sk)->rdb)) {
> + if (tcp_transmit_rdb_skb(sk, skb, mss_now, gfp))
> + break;
> + } else if (unlikely(tcp_transmit_skb(sk, skb, 1, gfp))) {
> break;
> -
> + }
> repair:
> /* Advance the send_head. This one is sent out.
> * This call will increment packets_out.
> @@ -2439,15 +2442,32 @@ u32 __tcp_select_window(struct sock *sk)
> return window;
> }
>
> +/**
> + * tcp_skb_append_data() - copy the linear data from an SKB to the end
> + * of another and update end sequence number
> + * and checksum
> + * @from_skb: the SKB to copy data from
> + * @to_skb: the SKB to copy data to
> + */
> +void tcp_skb_append_data(struct sk_buff *from_skb, struct sk_buff *to_skb)
> +{
> + skb_copy_from_linear_data(from_skb, skb_put(to_skb, from_skb->len),
> + from_skb->len);
> + TCP_SKB_CB(to_skb)->end_seq = TCP_SKB_CB(from_skb)->end_seq;
> +
> + if (from_skb->ip_summed == CHECKSUM_PARTIAL)
> + to_skb->ip_summed = CHECKSUM_PARTIAL;
> +
> + if (to_skb->ip_summed != CHECKSUM_PARTIAL)
> + to_skb->csum = csum_block_add(to_skb->csum, from_skb->csum,
> + to_skb->len);
> +}
> +
> /* Collapses two adjacent SKB's during retransmission. */
> static void tcp_collapse_retrans(struct sock *sk, struct sk_buff *skb)
> {
> struct tcp_sock *tp = tcp_sk(sk);
> struct sk_buff *next_skb = tcp_write_queue_next(sk, skb);
> - int skb_size, next_skb_size;
> -
> - skb_size = skb->len;
> - next_skb_size = next_skb->len;
>
> BUG_ON(tcp_skb_pcount(skb) != 1 || tcp_skb_pcount(next_skb) != 1);
>
> @@ -2455,17 +2475,7 @@ static void tcp_collapse_retrans(struct sock *sk, struct sk_buff *skb)
>
> tcp_unlink_write_queue(next_skb, sk);
>
> - skb_copy_from_linear_data(next_skb, skb_put(skb, next_skb_size),
> - next_skb_size);
> -
> - if (next_skb->ip_summed == CHECKSUM_PARTIAL)
> - skb->ip_summed = CHECKSUM_PARTIAL;
> -
> - if (skb->ip_summed != CHECKSUM_PARTIAL)
> - skb->csum = csum_block_add(skb->csum, next_skb->csum, skb_size);
> -
> - /* Update sequence range on original skb. */
> - TCP_SKB_CB(skb)->end_seq = TCP_SKB_CB(next_skb)->end_seq;
> + tcp_skb_append_data(next_skb, skb);
>
> /* Merge over control information. This moves PSH/FIN etc. over */
> TCP_SKB_CB(skb)->tcp_flags |= TCP_SKB_CB(next_skb)->tcp_flags;
> diff --git a/net/ipv4/tcp_rdb.c b/net/ipv4/tcp_rdb.c
> new file mode 100644
> index 0000000..2b37957
> --- /dev/null
> +++ b/net/ipv4/tcp_rdb.c
> @@ -0,0 +1,228 @@
> +#include <linux/skbuff.h>
> +#include <net/tcp.h>
> +
> +int sysctl_tcp_rdb_max_bytes __read_mostly;
> +int sysctl_tcp_rdb_max_packets __read_mostly = 1;
> +
> +/**
> + * rdb_detect_loss() - perform RDB loss detection by analysing ACKs
> + * @sk: socket
> + *
> + * Traverse the output queue and check if the ACKed packet is an RDB
> + * packet and if the redundant data covers one or more un-ACKed SKBs.
> + * If the incoming ACK acknowledges multiple SKBs, we can presume
> + * packet loss has occurred.
> + *
> + * We can infer packet loss this way because we can expect one ACK per
> + * transmitted data packet, as delayed ACKs are disabled when a host
> + * receives packets where the sequence number is not the expected
> + * sequence number.
> + *
> + * Return: The number of packets that are presumed to be lost
> + */
> +static unsigned int rdb_detect_loss(struct sock *sk)
> +{
> + struct sk_buff *skb, *tmp;
> + struct tcp_skb_cb *scb;
> + u32 seq_acked = tcp_sk(sk)->snd_una;
> + unsigned int packets_lost = 0;
> +
> + tcp_for_write_queue(skb, sk) {
> + if (skb == tcp_send_head(sk))
> + break;
> +
> + scb = TCP_SKB_CB(skb);
> + /* The ACK acknowledges parts of the data in this SKB.
> + * Can be caused by:
> + * - TSO: We abort as RDB is not used on SKBs split across
> + * multiple packets on lower layers as these are greater
> + * than one MSS.
> + * - Retrans collapse: We've had a retrans, so loss has already
> + * been detected.
> + */
> + if (after(scb->end_seq, seq_acked))
> + break;
> + else if (scb->end_seq != seq_acked)
> + continue;
> +
> + /* We have found the ACKed packet */
> +
> + /* This packet was sent with no redundant data, or no prior
> + * un-ACKed SKBs is in the output queue, so break here.
> + */
> + if (scb->tx.rdb_start_seq == scb->seq ||
> + skb_queue_is_first(&sk->sk_write_queue, skb))
> + break;
> + /* Find number of prior SKBs whose data was bundled in this
> + * (ACKed) SKB. We presume any redundant data covering previous
> + * SKB's are due to loss. (An exception would be reordering).
> + */
> + skb = skb->prev;
> + tcp_for_write_queue_reverse_from_safe(skb, tmp, sk) {
> + if (before(TCP_SKB_CB(skb)->seq, scb->tx.rdb_start_seq))
> + break;
> + packets_lost++;
since we only care if there is packet loss or not, we can return early here?
> + }
> + break;
> + }
> + return packets_lost;
> +}
> +
> +/**
> + * tcp_rdb_ack_event() - initiate RDB loss detection
> + * @sk: socket
> + * @flags: flags
> + */
> +void tcp_rdb_ack_event(struct sock *sk, u32 flags)
flags are not used
> +{
> + if (rdb_detect_loss(sk))
> + tcp_enter_cwr(sk);
> +}
> +
> +/**
> + * rdb_build_skb() - build a new RDB SKB and copy redundant + unsent
> + * data to the linear page buffer
> + * @sk: socket
> + * @xmit_skb: the SKB processed for transmission in the output engine
> + * @first_skb: the first SKB in the output queue to be bundled
> + * @bytes_in_rdb_skb: the total number of data bytes for the new
> + * rdb_skb (NEW + Redundant)
> + * @gfp_mask: gfp_t allocation
> + *
> + * Return: A new SKB containing redundant data, or NULL if memory
> + * allocation failed
> + */
> +static struct sk_buff *rdb_build_skb(const struct sock *sk,
> + struct sk_buff *xmit_skb,
> + struct sk_buff *first_skb,
> + u32 bytes_in_rdb_skb,
> + gfp_t gfp_mask)
> +{
> + struct sk_buff *rdb_skb, *tmp_skb = first_skb;
> +
> + rdb_skb = sk_stream_alloc_skb((struct sock *)sk,
> + (int)bytes_in_rdb_skb,
> + gfp_mask, false);
> + if (!rdb_skb)
> + return NULL;
> + copy_skb_header(rdb_skb, xmit_skb);
> + rdb_skb->ip_summed = xmit_skb->ip_summed;
> + TCP_SKB_CB(rdb_skb)->seq = TCP_SKB_CB(first_skb)->seq;
> + TCP_SKB_CB(xmit_skb)->tx.rdb_start_seq = TCP_SKB_CB(rdb_skb)->seq;
> +
> + /* Start on first_skb and append payload from each SKB in the output
> + * queue onto rdb_skb until we reach xmit_skb.
> + */
> + tcp_for_write_queue_from(tmp_skb, sk) {
> + tcp_skb_append_data(tmp_skb, rdb_skb);
> +
> + /* We reached xmit_skb, containing the unsent data */
> + if (tmp_skb == xmit_skb)
> + break;
> + }
> + return rdb_skb;
> +}
> +
> +/**
> + * rdb_can_bundle_test() - test if redundant data can be bundled
> + * @sk: socket
> + * @xmit_skb: the SKB processed for transmission by the output engine
> + * @max_payload: the maximum allowed payload bytes for the RDB SKB
> + * @bytes_in_rdb_skb: store the total number of payload bytes in the
> + * RDB SKB if bundling can be performed
> + *
> + * Traverse the output queue and check if any un-acked data may be
> + * bundled.
> + *
> + * Return: The first SKB to be in the bundle, or NULL if no bundling
> + */
> +static struct sk_buff *rdb_can_bundle_test(const struct sock *sk,
> + struct sk_buff *xmit_skb,
> + unsigned int max_payload,
> + u32 *bytes_in_rdb_skb)
> +{
> + struct sk_buff *first_to_bundle = NULL;
> + struct sk_buff *tmp, *skb = xmit_skb->prev;
> + u32 skbs_in_bundle_count = 1; /* Start on 1 to account for xmit_skb */
> + u32 total_payload = xmit_skb->len;
> +
> + if (sysctl_tcp_rdb_max_bytes)
> + max_payload = min_t(unsigned int, max_payload,
> + sysctl_tcp_rdb_max_bytes);
> +
> + /* We start at xmit_skb->prev, and go backwards */
> + tcp_for_write_queue_reverse_from_safe(skb, tmp, sk) {
> + /* Including data from this SKB would exceed payload limit */
> + if ((total_payload + skb->len) > max_payload)
> + break;
> +
> + if (sysctl_tcp_rdb_max_packets &&
> + (skbs_in_bundle_count > sysctl_tcp_rdb_max_packets))
> + break;
> +
> + total_payload += skb->len;
> + skbs_in_bundle_count++;
> + first_to_bundle = skb;
> + }
> + *bytes_in_rdb_skb = total_payload;
> + return first_to_bundle;
> +}
> +
> +/**
> + * tcp_transmit_rdb_skb() - try to create and send an RDB packet
> + * @sk: socket
> + * @xmit_skb: the SKB processed for transmission by the output engine
> + * @mss_now: current mss value
> + * @gfp_mask: gfp_t allocation
> + *
> + * If an RDB packet could not be created and sent, transmit the
> + * original unmodified SKB (xmit_skb).
> + *
> + * Return: 0 if successfully sent packet, else error from
> + * tcp_transmit_skb
> + */
> +int tcp_transmit_rdb_skb(struct sock *sk, struct sk_buff *xmit_skb,
> + unsigned int mss_now, gfp_t gfp_mask)
> +{
> + struct sk_buff *rdb_skb = NULL;
> + struct sk_buff *first_to_bundle;
> + u32 bytes_in_rdb_skb = 0;
> +
> + /* How we detect that RDB was used. When equal, no RDB data was sent */
> + TCP_SKB_CB(xmit_skb)->tx.rdb_start_seq = TCP_SKB_CB(xmit_skb)->seq;
> +
> + if (!tcp_stream_is_thin_dpifl(tcp_sk(sk)))
During loss recovery tcp inflight fluctuates and would like to trigger
this check even for non-thin-stream connections. Since the loss
already occurs, RDB can only take advantage from limited-transmit,
which it likely does not have (b/c its a thin-stream). It might be
checking if the state is open.
> + goto xmit_default;
> +
> + /* No bundling if first in queue, or on FIN packet */
> + if (skb_queue_is_first(&sk->sk_write_queue, xmit_skb) ||
> + (TCP_SKB_CB(xmit_skb)->tcp_flags & TCPHDR_FIN))
seems there are still benefit to bundle packets up to FIN?
> + goto xmit_default;
> +
> + /* Find number of (previous) SKBs to get data from */
> + first_to_bundle = rdb_can_bundle_test(sk, xmit_skb, mss_now,
> + &bytes_in_rdb_skb);
> + if (!first_to_bundle)
> + goto xmit_default;
> +
> + /* Create an SKB that contains redundant data starting from
> + * first_to_bundle.
> + */
> + rdb_skb = rdb_build_skb(sk, xmit_skb, first_to_bundle,
> + bytes_in_rdb_skb, gfp_mask);
> + if (!rdb_skb)
> + goto xmit_default;
> +
> + /* Set skb_mstamp for the SKB in the output queue (xmit_skb) containing
> + * the yet unsent data. Normally this would be done by
> + * tcp_transmit_skb(), but as we pass in rdb_skb instead, xmit_skb's
> + * timestamp will not be touched.
> + */
> + skb_mstamp_get(&xmit_skb->skb_mstamp);
> + rdb_skb->skb_mstamp = xmit_skb->skb_mstamp;
> + return tcp_transmit_skb(sk, rdb_skb, 0, gfp_mask);
> +
> +xmit_default:
> + /* Transmit the unmodified SKB from output queue */
> + return tcp_transmit_skb(sk, xmit_skb, 1, gfp_mask);
> +}
> --
> 1.9.1
>
since RDB will cause DSACKs, and we only blindly count DSACKs to
perform CWND undo. How does RDB handle that false positives?
^ 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