* [PATCH net-next] tcp: refine TSO autosizing
From: Eric Dumazet @ 2014-12-05 14:15 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Neal Cardwell, Yuchung Cheng, Nandita Dukkipati
From: Eric Dumazet <edumazet@google.com>
Commit 95bd09eb2750 ("tcp: TSO packets automatic sizing") tried to
control TSO size, but did this at the wrong place (sendmsg() time)
At sendmsg() time, we might have a pessimistic view of flow rate,
and we end up building very small skbs (with 2 MSS per skb).
This is bad because :
- It sends small TSO packets even in Slow Start where rate quickly
increases.
- It tends to make socket write queue very big, increasing tcp_ack()
processing time, but also increasing memory needs, not necessarily
accounted for, as fast clones overhead is currently ignored.
- Lower GRO efficiency and more ACK packets.
Servers with a lot of small lived connections suffer from this.
Lets instead fill skbs as much as possible (64KB of payload), but split
them at xmit time, when we have a precise idea of the flow rate.
skb split is actually quite efficient.
Patch looks bigger than necessary, because TCP Small Queue decision now
has to take place after the eventual split.
Tested:
40 ms rtt link
nstat >/dev/null
netperf -H remote -Cc -l -2000000 -- -s 1000000
nstat | egrep "IpInReceives|IpOutRequests|TcpOutSegs|IpExtOutOctets"
Before patch :
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
87380 2000000 2000000 0.36 44.22 0.00 0.06 0.000 5.007
IpInReceives 600 0.0
IpOutRequests 599 0.0
TcpOutSegs 1397 0.0
IpExtOutOctets 2033249 0.0
After patch :
Recv Send Send Utilization Service Demand
Socket Socket Message Elapsed Send Recv Send Recv
Size Size Size Time Throughput local remote local remote
bytes bytes bytes secs. 10^6bits/s % S % S us/KB us/KB
87380 2000000 2000000 0.36 44.09 0.00 0.00 0.000 0.000
IpInReceives 257 0.0
IpOutRequests 226 0.0
TcpOutSegs 1399 0.0
IpExtOutOctets 2013777 0.0
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/ipv4/tcp.c | 14 ++------------
net/ipv4/tcp_output.c | 38 ++++++++++++++++++++++++--------------
2 files changed, 26 insertions(+), 26 deletions(-)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index dc13a3657e8e..91e6c1406313 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -840,24 +840,14 @@ static unsigned int tcp_xmit_size_goal(struct sock *sk, u32 mss_now,
xmit_size_goal = mss_now;
if (large_allowed && sk_can_gso(sk)) {
- u32 gso_size, hlen;
+ u32 hlen;
/* Maybe we should/could use sk->sk_prot->max_header here ? */
hlen = inet_csk(sk)->icsk_af_ops->net_header_len +
inet_csk(sk)->icsk_ext_hdr_len +
tp->tcp_header_len;
- /* Goal is to send at least one packet per ms,
- * not one big TSO packet every 100 ms.
- * This preserves ACK clocking and is consistent
- * with tcp_tso_should_defer() heuristic.
- */
- gso_size = sk->sk_pacing_rate / (2 * MSEC_PER_SEC);
- gso_size = max_t(u32, gso_size,
- sysctl_tcp_min_tso_segs * mss_now);
-
- xmit_size_goal = min_t(u32, gso_size,
- sk->sk_gso_max_size - 1 - hlen);
+ xmit_size_goal = sk->sk_gso_max_size - 1 - hlen;
xmit_size_goal = tcp_bound_to_half_wnd(tp, xmit_size_goal);
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f5bd4bd3f7e6..dac9991bccbf 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -1533,6 +1533,16 @@ static unsigned int tcp_mss_split_point(const struct sock *sk,
{
const struct tcp_sock *tp = tcp_sk(sk);
u32 partial, needed, window, max_len;
+ u32 bytes = sk->sk_pacing_rate >> 10;
+ u32 segs;
+
+ /* Goal is to send at least one packet per ms,
+ * not one big TSO packet every 100 ms.
+ * This preserves ACK clocking and is consistent
+ * with tcp_tso_should_defer() heuristic.
+ */
+ segs = max_t(u32, bytes / mss_now, sysctl_tcp_min_tso_segs);
+ max_segs = min_t(u32, max_segs, segs);
window = tcp_wnd_end(tp) - TCP_SKB_CB(skb)->seq;
max_len = mss_now * max_segs;
@@ -2008,6 +2018,18 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
break;
}
+ limit = mss_now;
+ if (tso_segs > 1 && !tcp_urg_mode(tp))
+ limit = tcp_mss_split_point(sk, skb, mss_now,
+ min_t(unsigned int,
+ cwnd_quota,
+ sk->sk_gso_max_segs),
+ nonagle);
+
+ if (skb->len > limit &&
+ unlikely(tso_fragment(sk, skb, limit, mss_now, gfp)))
+ break;
+
/* TCP Small Queues :
* Control number of packets in qdisc/devices to two packets / or ~1 ms.
* This allows for :
@@ -2018,8 +2040,8 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
* of queued bytes to ensure line rate.
* One example is wifi aggregation (802.11 AMPDU)
*/
- limit = max_t(unsigned int, sysctl_tcp_limit_output_bytes,
- sk->sk_pacing_rate >> 10);
+ limit = max(2 * skb->truesize, sk->sk_pacing_rate >> 10);
+ limit = min_t(u32, limit, sysctl_tcp_limit_output_bytes);
if (atomic_read(&sk->sk_wmem_alloc) > limit) {
set_bit(TSQ_THROTTLED, &tp->tsq_flags);
@@ -2032,18 +2054,6 @@ static bool tcp_write_xmit(struct sock *sk, unsigned int mss_now, int nonagle,
break;
}
- limit = mss_now;
- if (tso_segs > 1 && !tcp_urg_mode(tp))
- limit = tcp_mss_split_point(sk, skb, mss_now,
- min_t(unsigned int,
- cwnd_quota,
- sk->sk_gso_max_segs),
- nonagle);
-
- if (skb->len > limit &&
- unlikely(tso_fragment(sk, skb, limit, mss_now, gfp)))
- break;
-
if (unlikely(tcp_transmit_skb(sk, skb, 1, gfp)))
break;
^ permalink raw reply related
* Re: [PATCH 1/3] netdev: introduce new NETIF_F_HW_SWITCH_OFFLOAD feature flag for switch device offloads
From: Roopa Prabhu @ 2014-12-05 14:16 UTC (permalink / raw)
To: Jiri Pirko
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh, aviadr,
netdev, davem, shm, gospo
In-Reply-To: <20141205074127.GB1866@nanopsycho.orion>
On 12/4/14, 11:41 PM, Jiri Pirko wrote:
> Fri, Dec 05, 2014 at 03:26:39AM CET, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This is a generic high level feature flag for all switch asic features today.
>>
>> switch drivers set this flag on switch ports. Logical devices like
>> bridge, bonds, vxlans can inherit this flag from their slaves/ports.
>
> Can you please elaborate on how exactly would this inheritance look
> like?
My thought there was, when a port with the hw offload flag is added to
the bridge, the same flag gets set on the bridge. And, for any bridge
attributes (not port attributes), this flag on the bridge can be used to
offload those bridge attributes.
bridge attribute examples: IFLA_BR_FORWARD_DELAY, IFLA_BR_HELLO_TIME,
IFLA_BR_MAX_AGE.
I don't think offloads for these are handled today. I was going to look
at them as part of continued work on this.
The current patches only target bridge port attributes and the flag for
this is already set by the port driver.
I believe netdev_update_features() takes care of the updating the flag
on the bridge part. I plan to check on that.
>
>
>> I had to use SWITCH in the name to avoid ambiguity with other feature
>> flags. But, since i have been harping about not calling it 'switch',
>> I am welcome to any suggestions :)
>>
>> An alternative to using a feature flag is to use a IFF_HW_OFFLOAD
>> in net_device_flags.
>> ---
>> include/linux/netdev_features.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/include/linux/netdev_features.h b/include/linux/netdev_features.h
>> index 8e30685..68db1de 100644
>> --- a/include/linux/netdev_features.h
>> +++ b/include/linux/netdev_features.h
>> @@ -66,6 +66,7 @@ enum {
>> NETIF_F_HW_VLAN_STAG_FILTER_BIT,/* Receive filtering on VLAN STAGs */
>> NETIF_F_HW_L2FW_DOFFLOAD_BIT, /* Allow L2 Forwarding in Hardware */
>> NETIF_F_BUSY_POLL_BIT, /* Busy poll */
>> + NETIF_F_HW_SWITCH_OFFLOAD_BIT, /* HW switch offload */
>>
>> /*
>> * Add your fresh new feature above and remember to update
>> @@ -124,6 +125,7 @@ enum {
>> #define NETIF_F_HW_VLAN_STAG_TX __NETIF_F(HW_VLAN_STAG_TX)
>> #define NETIF_F_HW_L2FW_DOFFLOAD __NETIF_F(HW_L2FW_DOFFLOAD)
>> #define NETIF_F_BUSY_POLL __NETIF_F(BUSY_POLL)
>> +#define NETIF_F_HW_SWITCH_OFFLOAD __NETIF_F(HW_SWITCH_OFFLOAD)
>>
>> /* Features valid for ethtool to change */
>> /* = all defined minus driver/device-class-related */
>> --
>> 1.7.10.4
>>
^ permalink raw reply
* Re: [PATCH][net-next] net: avoid to call skb_queue_len again
From: Eric Dumazet @ 2014-12-05 14:31 UTC (permalink / raw)
To: Sergei Shtylyov; +Cc: roy.qing.li, netdev
In-Reply-To: <5481BBF2.40806@cogentembedded.com>
On Fri, 2014-12-05 at 17:06 +0300, Sergei Shtylyov wrote:
> Hello.
>
> On 12/5/2014 12:49 PM, roy.qing.li@gmail.com wrote:
>
> > From: Li RongQing <roy.qing.li@gmail.com>
>
> > the queue length of sd->input_pkt_queue has been putted into qlen,
>
> s/putted/put/, it's irregular verb.
>
> > and impossible to change, since hold the lock
>
> I can't parse that. Who holds the lock?
This thread/cpu holds the lock to manipulate input_pkt_queue.
Otherwise, the following would break horribly....
__skb_queue_tail(&sd->input_pkt_queue, skb);
^ permalink raw reply
* Re: [PATCH 2/3] bridge: offload bridge port attributes to switch asic if feature flag set
From: Roopa Prabhu @ 2014-12-05 14:37 UTC (permalink / raw)
To: Jiri Pirko
Cc: sfeldma, jhs, bcrl, tgraf, john.fastabend, stephen, linville,
nhorman, nicolas.dichtel, vyasevic, f.fainelli, buytenh, aviadr,
netdev, davem, shm, gospo
In-Reply-To: <20141205073840.GA1866@nanopsycho.orion>
On 12/4/14, 11:38 PM, Jiri Pirko wrote:
> Fri, Dec 05, 2014 at 03:26:40AM CET, roopa@cumulusnetworks.com wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This allows offloading to switch asic without having the user to set
>> any flag. And this is done in the bridge driver to rollback kernel settings
>> on hw offload failure if required in the future.
>>
>> With this, it also makes sure a notification goes out only after the
>> attributes are set both in the kernel and hw.
>> ---
>> net/bridge/br_netlink.c | 27 ++++++++++++++++++++++++++-
>> 1 file changed, 26 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>> index 9f5eb55..ce173f0 100644
>> --- a/net/bridge/br_netlink.c
>> +++ b/net/bridge/br_netlink.c
>> @@ -407,9 +407,21 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh)
>> afspec, RTM_SETLINK);
>> }
>>
>> + if ((dev->features & NETIF_F_HW_SWITCH_OFFLOAD) &&
>> + dev->netdev_ops->ndo_bridge_setlink) {
>> + int ret = dev->netdev_ops->ndo_bridge_setlink(dev, nlh);
> This (and I suspect other patches as well) has many issues which
> are pointed out by scripts/checkpatch.pl sctipts. For example
> here, there should be an empty line. Please run your patches by
> this script before you send them.
>
>> + if (ret && ret != -EOPNOTSUPP) {
>> + /* XXX Fix this in the future to rollback
>> + * kernel settings and return error
>> + */
>> + br_warn(p->br, "error offloading bridge attributes "
>> + "on port %u(%s)\n", (unsigned int) p->port_no,
>> + p->dev->name);
>> + }
>> + }
>> +
>> if (err == 0)
>> br_ifinfo_notify(RTM_NEWLINK, p);
>> -
>> out:
>> return err;
>> }
>> @@ -433,6 +445,19 @@ int br_dellink(struct net_device *dev, struct nlmsghdr *nlh)
>> err = br_afspec((struct net_bridge *)netdev_priv(dev), p,
>> afspec, RTM_DELLINK);
>>
>> + if (dev->features & NETIF_F_HW_SWITCH_OFFLOAD
>> + && dev->netdev_ops->ndo_bridge_setlink) {
>> + int ret = dev->netdev_ops->ndo_bridge_dellink(dev, nlh);
> c&p issue, there should be check for dellink here.
>
>> + if (ret && ret != -EOPNOTSUPP) {
>> + /* XXX Fix this in the future to rollback
>> + * kernel settings and return error
>> + */
>> + br_warn(p->br, "error offloading bridge attributes "
>> + "on port %u(%s)\n", (unsigned int) p->port_no,
>> + p->dev->name);
>> + }
>> + }
>> +
>
> I agree with Scott that this code should be moved to rtnetlink.c
I moved it here to make rollback easier. Plus i don't want software
notification to go out before hardware is programmed.
And the software notification goes out from here.
I don't intend to implement rollback in v2 (that will be a separate series).
But, i do want to take care of the notification problem.
^ permalink raw reply
* Re: [PATCH 1/1] net: macb: Remove obsolete comment from Kconfig
From: Nicolas Ferre @ 2014-12-05 14:42 UTC (permalink / raw)
To: James Byrne, netdev, David Miller; +Cc: Cyrille Pitchen
In-Reply-To: <1417784633-13360-1-git-send-email-james.byrne@origamienergy.com>
Le 05/12/2014 14:03, James Byrne a écrit :
> The Kconfig file says that Gigabit mode is not supported, but it has been
> supported since commit 140b7552fdff04bbceeb842f0e04f0b4015fe97b ("net/macb:
> Add support for Gigabit Ethernet mode").
>
> Signed-off-by: James Byrne <james.byrne@origamienergy.com>
Yes indeed:
Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> ---
> drivers/net/ethernet/cadence/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/Kconfig b/drivers/net/ethernet/cadence/Kconfig
> index 9e089d2..6932be0 100644
> --- a/drivers/net/ethernet/cadence/Kconfig
> +++ b/drivers/net/ethernet/cadence/Kconfig
> @@ -35,8 +35,8 @@ config MACB
> ---help---
> The Cadence MACB ethernet interface is found on many Atmel AT32 and
> AT91 parts. This driver also supports the Cadence GEM (Gigabit
> - Ethernet MAC found in some ARM SoC devices). Note: the Gigabit mode
> - is not yet supported. Say Y to include support for the MACB/GEM chip.
> + Ethernet MAC found in some ARM SoC devices). Say Y to include
> + support for the MACB/GEM chip.
>
> To compile this driver as a module, choose M here: the module
> will be called macb.
>
--
Nicolas Ferre
^ permalink raw reply
* Re: [PATCH] net: fix the flow limitation computation
From: Eric Dumazet @ 2014-12-05 14:49 UTC (permalink / raw)
To: roy.qing.li, Willem de Bruijn; +Cc: netdev
In-Reply-To: <1417772919-17744-1-git-send-email-roy.qing.li@gmail.com>
On Fri, 2014-12-05 at 17:48 +0800, roy.qing.li@gmail.com wrote:
> From: Li RongQing <roy.qing.li@gmail.com>
>
> Once RPS is enabled, the skb maybe enqueue to different CPU, so the
> flow limitation computation should use the enqueued CPU, not the local
> CPU
>
> Signed-off-by: Li RongQing <roy.qing.li@gmail.com>
> ---
CC Willem de Bruijn <willemb@google.com>, author of this mechanism, for
comments.
> net/core/dev.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 945bbd0..e70507d 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -3250,7 +3250,7 @@ static int rps_ipi_queued(struct softnet_data *sd)
> int netdev_flow_limit_table_len __read_mostly = (1 << 12);
> #endif
>
> -static bool skb_flow_limit(struct sk_buff *skb, unsigned int qlen)
> +static bool skb_flow_limit(struct sk_buff *skb, unsigned int qlen, int cpu)
> {
> #ifdef CONFIG_NET_FLOW_LIMIT
> struct sd_flow_limit *fl;
> @@ -3260,7 +3260,7 @@ static bool skb_flow_limit(struct sk_buff *skb, unsigned int qlen)
> if (qlen < (netdev_max_backlog >> 1))
> return false;
>
> - sd = this_cpu_ptr(&softnet_data);
> + sd = &per_cpu(softnet_data, cpu);
What about passing sd instead of cpu, so that we do not have to compute
this again ?
^ permalink raw reply
* [patch net-next 1/2] net: sched: cls_basic: fix error path in basic_change()
From: Jiri Pirko @ 2014-12-05 14:50 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
net/sched/cls_basic.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
index 7cf0a62..5aed341 100644
--- a/net/sched/cls_basic.c
+++ b/net/sched/cls_basic.c
@@ -178,10 +178,9 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
return -EINVAL;
}
- err = -ENOBUFS;
fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
- if (fnew == NULL)
- goto errout;
+ if (!fnew)
+ return -ENOBUFS;
tcf_exts_init(&fnew->exts, TCA_BASIC_ACT, TCA_BASIC_POLICE);
err = -EINVAL;
--
1.9.3
^ permalink raw reply related
* [patch net-next 2/2] net: sched: cls: use nla_nest_cancel instead of nlmsg_trim
From: Jiri Pirko @ 2014-12-05 14:50 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs
In-Reply-To: <1417791023-28124-1-git-send-email-jiri@resnulli.us>
To cancel nesting, this function is more convenient.
Signed-off-by: Jiri Pirko <jiri@resnulli.us>
---
net/sched/cls_cgroup.c | 3 +--
net/sched/cls_flow.c | 2 +-
net/sched/cls_fw.c | 3 +--
net/sched/cls_route.c | 3 +--
net/sched/cls_rsvp.h | 3 +--
net/sched/cls_tcindex.c | 3 +--
6 files changed, 6 insertions(+), 11 deletions(-)
diff --git a/net/sched/cls_cgroup.c b/net/sched/cls_cgroup.c
index 741bfa7..221697a 100644
--- a/net/sched/cls_cgroup.c
+++ b/net/sched/cls_cgroup.c
@@ -177,7 +177,6 @@ static int cls_cgroup_dump(struct net *net, struct tcf_proto *tp, unsigned long
struct sk_buff *skb, struct tcmsg *t)
{
struct cls_cgroup_head *head = rtnl_dereference(tp->root);
- unsigned char *b = skb_tail_pointer(skb);
struct nlattr *nest;
t->tcm_handle = head->handle;
@@ -198,7 +197,7 @@ static int cls_cgroup_dump(struct net *net, struct tcf_proto *tp, unsigned long
return skb->len;
nla_put_failure:
- nlmsg_trim(skb, b);
+ nla_nest_cancel(skb, nest);
return -1;
}
diff --git a/net/sched/cls_flow.c b/net/sched/cls_flow.c
index 8e22718..15d68f2 100644
--- a/net/sched/cls_flow.c
+++ b/net/sched/cls_flow.c
@@ -638,7 +638,7 @@ static int flow_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
return skb->len;
nla_put_failure:
- nlmsg_trim(skb, nest);
+ nla_nest_cancel(skb, nest);
return -1;
}
diff --git a/net/sched/cls_fw.c b/net/sched/cls_fw.c
index 23fda2a..a5269f7 100644
--- a/net/sched/cls_fw.c
+++ b/net/sched/cls_fw.c
@@ -356,7 +356,6 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
{
struct fw_head *head = rtnl_dereference(tp->root);
struct fw_filter *f = (struct fw_filter *)fh;
- unsigned char *b = skb_tail_pointer(skb);
struct nlattr *nest;
if (f == NULL)
@@ -397,7 +396,7 @@ static int fw_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
return skb->len;
nla_put_failure:
- nlmsg_trim(skb, b);
+ nla_nest_cancel(skb, nest);
return -1;
}
diff --git a/net/sched/cls_route.c b/net/sched/cls_route.c
index 098a273..2ecd246 100644
--- a/net/sched/cls_route.c
+++ b/net/sched/cls_route.c
@@ -593,7 +593,6 @@ static int route4_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
struct sk_buff *skb, struct tcmsg *t)
{
struct route4_filter *f = (struct route4_filter *)fh;
- unsigned char *b = skb_tail_pointer(skb);
struct nlattr *nest;
u32 id;
@@ -635,7 +634,7 @@ static int route4_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
return skb->len;
nla_put_failure:
- nlmsg_trim(skb, b);
+ nla_nest_cancel(skb, nest);
return -1;
}
diff --git a/net/sched/cls_rsvp.h b/net/sched/cls_rsvp.h
index b7af362..edd8ade 100644
--- a/net/sched/cls_rsvp.h
+++ b/net/sched/cls_rsvp.h
@@ -653,7 +653,6 @@ static int rsvp_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
{
struct rsvp_filter *f = (struct rsvp_filter *)fh;
struct rsvp_session *s;
- unsigned char *b = skb_tail_pointer(skb);
struct nlattr *nest;
struct tc_rsvp_pinfo pinfo;
@@ -694,7 +693,7 @@ static int rsvp_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
return skb->len;
nla_put_failure:
- nlmsg_trim(skb, b);
+ nla_nest_cancel(skb, nest);
return -1;
}
diff --git a/net/sched/cls_tcindex.c b/net/sched/cls_tcindex.c
index 0d9d891..48d5e7b 100644
--- a/net/sched/cls_tcindex.c
+++ b/net/sched/cls_tcindex.c
@@ -489,7 +489,6 @@ static int tcindex_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
{
struct tcindex_data *p = rtnl_dereference(tp->root);
struct tcindex_filter_result *r = (struct tcindex_filter_result *) fh;
- unsigned char *b = skb_tail_pointer(skb);
struct nlattr *nest;
pr_debug("tcindex_dump(tp %p,fh 0x%lx,skb %p,t %p),p %p,r %p,b %p\n",
@@ -543,7 +542,7 @@ static int tcindex_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
return skb->len;
nla_put_failure:
- nlmsg_trim(skb, b);
+ nla_nest_cancel(skb, nest);
return -1;
}
--
1.9.3
^ permalink raw reply related
* Re: [PATCH 2/2] gianfar: handle map error in gfar_start_xmit()
From: Claudiu Manoil @ 2014-12-05 14:51 UTC (permalink / raw)
To: Arseny Solokha; +Cc: netdev, linux-kernel
In-Reply-To: <1417775874-17775-3-git-send-email-asolokha@kb.kras.ru>
On 12/5/2014 12:37 PM, Arseny Solokha wrote:
> From: Arseny Solokha <asolokha@kb.kras.ru>
>
> When DMA-API debugging is enabled in the kernel, it spews the following
> upon upping the link:
>
> WARNING: at lib/dma-debug.c:1135
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W O 3.18.0-rc7 #1
> task: c0720340 ti: effe2000 task.ti: c0750000
> NIP: c01d7c1c LR: c01d7c1c CTR: c02250fc
> REGS: effe3d40 TRAP: 0700 Tainted: G W O (3.18.0-rc7)
> MSR: 00021000 <CE,ME> CR: 22044242 XER: 20000000
>
> GPR00: c01d7c1c effe3df0 c0720340 00000095 c201e404 c201e9f0 00021000 01a9d000
> GPR08: 00000007 00000000 01a9d000 00000313 22044242 00583f60 05f41012 00000000
> GPR16: 00000000 000000ff 00000000 00000000 00000000 c5dc3b40 c5677720 00000001
> GPR24: c0730000 00029000 c0d0d828 c072c394 effe3e48 c075baec c0d0f020 ee31e600
> NIP [c01d7c1c] check_unmap+0x5b4/0xae4
> LR [c01d7c1c] check_unmap+0x5b4/0xae4
> Call Trace:
> [effe3df0] [c01d7c1c] check_unmap+0x5b4/0xae4 (unreliable)
> [effe3e40] [c01d81c4] debug_dma_unmap_page+0x78/0x8c
> [effe3ec0] [c0286ba0] gfar_clean_tx_ring+0x120/0x3c0
> [effe3f30] [c0286f90] gfar_poll_tx_sq+0x48/0x94
> o[effe3f50] [c030c388] net_rx_action+0x130/0x1ac
> [effe3f80] [c00319e0] __do_softirq+0x134/0x240
> [effe3fe0] [c0031dd0] irq_exit+0xa4/0xc8
> [effe3ff0] [c000e01c] call_do_irq+0x24/0x3c
> [c0751e70] [c0004a04] do_IRQ+0x8c/0x108
> [c0751e90] [c0010068] ret_from_except+0x0/0x18
> --- interrupt: 501 at arch_cpu_idle+0x24/0x5c
> LR = arch_cpu_idle+0x24/0x5c
> [c0751f50] [c007d2e4] rcu_idle_enter+0xc8/0xcc (unreliable)
> [c0751f60] [c006587c] cpu_startup_entry+0x1d4/0x29c
> [c0751fb0] [c054399c] start_kernel+0x338/0x34c
> [c0751ff0] [c000046c] set_ivor+0x154/0x190
> Instruction dump:
> 394adb30 80fc0018 811c001c 3c60c04f 5529103a 7cca482e 38639e60 813c0020
> 815c0024 90c10008 4cc63182 48240b01 <0fe00000> 3c60c04f 3863971c 4cc63182
> ---[ end trace 3eb7bf62ba1b80f9 ]---
> Mapped at:
> [<c0287420>] gfar_start_xmit+0x424/0x910
> [<c030e964>] dev_hard_start_xmit+0x20c/0x3d8
> [<c032cc3c>] sch_direct_xmit+0x124/0x22c
> [<c030ede8>] __dev_queue_xmit+0x2b8/0x674
>
> Or the following upon starting transmission of some large chunks
> of data:
>
> fsl-gianfar ffe25000.ethernet: DMA-API: device driver failed to check map error[device address=0x0000000005fa8000]
> ------------[ cut here ]------------
> WARNING: at lib/dma-debug.c:1135
> Modules linked in:
> CPU: 0 PID: 0 Comm: swapper/0 Tainted: G O 3.18.0-rc7 #35
> task: c071d340 ti: effe2000 task.ti: c074e000
> NIP: c01d7c1c LR: c01d7c1c CTR: c022339c
> REGS: effe3d40 TRAP: 0700 Tainted: G O (3.18.0-rc7)
> MSR: 00021000 <CE,ME> CR: 22044242 XER: 20000000
>
> GPR00: c01d7c1c effe3df0 c071d340 00000094 00000001 c0071750 00000000 00000001
> GPR08: 00000000 00000000 effe2000 00000000 20044242 00581f60 05fa8000 000001c4
> GPR16: 00000000 000000ff 00000000 000000e4 00000039 c5b1a9c0 c5679c60 00000002
> GPR24: c0730000 00029000 c0d0c528 c0729394 effe3e48 c0759aec c0d0d020 ee315900
> NIP [c01d7c1c] check_unmap+0x5b4/0xae4
> LR [c01d7c1c] check_unmap+0x5b4/0xae4
> Call Trace:
> [effe3df0] [c01d7c1c] check_unmap+0x5b4/0xae4 (unreliable)
> [effe3e40] [c01d81c4] debug_dma_unmap_page+0x78/0x8c
> [effe3ec0] [c0284c78] gfar_clean_tx_ring+0x1b4/0x3c0
> [effe3f30] [c0284fd4] gfar_poll_tx_sq+0x48/0x94
> [effe3f50] [c030a5c4] net_rx_action+0x130/0x1ac
> [effe3f80] [c00319e0] __do_softirq+0x134/0x240
> [effe3fe0] [c0031dd0] irq_exit+0xa4/0xc8
> [effe3ff0] [c000e01c] call_do_irq+0x24/0x3c
> [c074fe70] [c0004a04] do_IRQ+0x8c/0x108
> [c074fe90] [c0010068] ret_from_except+0x0/0x18
> --- interrupt: 501 at arch_cpu_idle+0x24/0x5c
> LR = arch_cpu_idle+0x24/0x5c
> [c074ff50] [c007d2e4] rcu_idle_enter+0xc8/0xcc (unreliable)
> [c074ff60] [c006587c] cpu_startup_entry+0x1d4/0x29c
> [c074ffb0] [c054199c] start_kernel+0x338/0x34c
> [c074fff0] [c000046c] set_ivor+0x154/0x190
> Instruction dump:
> 394abb30 80fc0018 811c001c 3c60c04e 5529103a 7cca482e 386379f0 813c0020
> 815c0024 90c10008 4cc63182 4823ed39 <0fe00000> 3c60c04e 386372ac 4cc63182
> ---[ end trace 008c59ca7ca1f712 ]---
> Mapped at:
> [<c0285264>] gfar_start_xmit+0x224/0x95c
> [<c030cba0>] dev_hard_start_xmit+0x20c/0x3d8
> [<c032ae78>] sch_direct_xmit+0x124/0x22c
> [<c032b008>] __qdisc_run+0x88/0x1c0
> [<c0307920>] net_tx_action+0xf0/0x19c
>
> Ignore these mapping failures in hope we'll have more luck next time.
>
> Signed-off-by: Arseny Solokha <asolokha@kb.kras.ru>
> ---
> drivers/net/ethernet/freescale/gianfar.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index f34ca55..9ea887e 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -2296,6 +2296,12 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> 0,
> frag_len,
> DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(priv->dev, bufaddr))) {
> + /* As DMA mapping failed, pretend the TX path
> + * is busy to retry later
> + */
> + return NETDEV_TX_BUSY;
This is not right.
Proper bailout code missing: un-mapping of skb fragments and
de-allocation of resources.
This is not a TX_BUSY error condition, it's a system failure.
(will resubmit this one:
http://permalink.gmane.org/gmane.linux.network/336274)
> + }
>
> /* set the TxBD length and buffer pointer */
> txbdp->bufPtr = bufaddr;
> @@ -2345,8 +2351,15 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> fcb->ptp = 1;
> }
>
> - txbdp_start->bufPtr = dma_map_single(priv->dev, skb->data,
> - skb_headlen(skb), DMA_TO_DEVICE);
> + bufaddr = dma_map_single(priv->dev, skb->data, skb_headlen(skb),
> + DMA_TO_DEVICE);
> + if (unlikely(dma_mapping_error(priv->dev, bufaddr))) {
> + /* As DMA mapping failed, pretend the TX path is busy to retry
> + * later
> + */
> + return NETDEV_TX_BUSY;
same here
> + }
> + txbdp_start->bufPtr = bufaddr;
>
> /* If time stamping is requested one additional TxBD must be set up. The
> * first TxBD points to the FCB and must have a data length of
>
^ permalink raw reply
* Re: [PATCH 0/2] DMA API usage fixes in gianfar
From: Claudiu Manoil @ 2014-12-05 14:48 UTC (permalink / raw)
To: Arseny Solokha; +Cc: netdev, linux-kernel, haokexin
In-Reply-To: <1417775874-17775-1-git-send-email-asolokha@kb.kras.ru>
On 12/5/2014 12:37 PM, Arseny Solokha wrote:
> Hello.
>
> This patch set fixes DMA API usage issues in gianfar ethernet driver
> reported by the kernel w/ DMA API debug enabled.
>
> There were even reports that the kernel sometimes oopsed in the past
> because of kernel paging request handling failures, though it was likely
> observed on some ancient versions. And while I personally doesn't have
> any strong evidence of this, there's no reason to let these possible
> failures live any longer.
>
> Arseny Solokha (2):
> gianfar: handle map error in gfar_new_rxbdp()
> gianfar: handle map error in gfar_start_xmit()
>
> drivers/net/ethernet/freescale/gianfar.c | 49 ++++++++++++++++++++++++++------
> 1 file changed, 41 insertions(+), 8 deletions(-)
>
Thanks but please note that Kevin Hao already provided a fix for this issue:
http://permalink.gmane.org/gmane.linux.network/336274
His patch was only deferred for testing (and bandwidth) reasons.
I will try to resend his patch to the netdev list today if possible,
I apologize for the delay.
Also note that there are some issues with your patches.
As said before, I will resubmit Kevin Hao's patch.
Thanks,
Claudiu
^ permalink raw reply
* [PATCH iproute2 REGRESSIONS v2] ss: Fix layout/output issues introduced by regression
From: Vadim Kochan @ 2014-12-05 15:03 UTC (permalink / raw)
To: netdev; +Cc: Vadim Kochan
This patch fixes the following issues which was introduced by me in commits:
#1 (2dc854854b7f1b) ss: Fixed broken output for Netlink 'Peer Address:Port' column
ISSUE: Broken layout when all sockets are printed out
#2 (eef43b5052afb7) ss: Identify more netlink protocol names
ISSUE: Protocol id is not printed if 'numbers only' output was specified (-n)
Also aligned the width of the local/peer ports to be more wider.
I tested with a lot of option combinations (I may miss some test cases),
but layout seems to me better than the previous released version of iproute2/ss.
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
---
misc/ss.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/misc/ss.c b/misc/ss.c
index a99294d..8abaaff 100644
--- a/misc/ss.c
+++ b/misc/ss.c
@@ -101,8 +101,6 @@ int state_width;
int addrp_width;
int addr_width;
int serv_width;
-int paddr_width;
-int pserv_width;
int screen_width;
static const char *TCP_PROTO = "tcp";
@@ -2912,11 +2910,12 @@ static void netlink_show_one(struct filter *f,
printf("%-*s ", state_width, "UNCONN");
printf("%-6d %-6d ", rq, wq);
- if (resolve_services)
- {
+ if (resolve_services) {
printf("%*s:", addr_width, nl_proto_n2a(prot, prot_name,
sizeof(prot_name)));
- }
+ } else
+ printf("%*d:", addr_width, prot);
+
if (pid == -1) {
printf("%-*s ", serv_width, "*");
@@ -2947,10 +2946,10 @@ static void netlink_show_one(struct filter *f,
if (state == NETLINK_CONNECTED) {
printf("%*d:%-*d",
- paddr_width, dst_group, pserv_width, dst_pid);
+ addr_width, dst_group, serv_width, dst_pid);
} else {
printf("%*s*%-*s",
- paddr_width, "", pserv_width, "");
+ addr_width, "", serv_width, "");
}
char *pid_context = NULL;
@@ -3684,22 +3683,13 @@ int main(int argc, char *argv[])
printf("%-*s ", state_width, "State");
printf("%-6s %-6s ", "Recv-Q", "Send-Q");
- paddr_width = addr_width;
- pserv_width = serv_width;
-
- /* Netlink service column can be resolved as process name/pid thus it
- * can be much wider than address column which is just a
- * protocol name/id.
- */
- if (current_filter.dbs & (1<<NETLINK_DB)) {
- serv_width = addr_width - 10;
- paddr_width = 13;
- pserv_width = 13;
- }
+ /* Make enough space for the local/remote port field */
+ addr_width -= 13;
+ serv_width += 13;
printf("%*s:%-*s %*s:%-*s\n",
addr_width, "Local Address", serv_width, "Port",
- paddr_width, "Peer Address", pserv_width, "Port");
+ addr_width, "Peer Address", serv_width, "Port");
fflush(stdout);
--
2.1.3
^ permalink raw reply related
* Re: [PATCH iproute2 REGRESSIONS] ss: Fix layout issues introduced by regression
From: Vadim Kochan @ 2014-12-05 15:14 UTC (permalink / raw)
To: netdev@vger.kernel.org; +Cc: Vadim Kochan
In-Reply-To: <1417778169-12339-1-git-send-email-vadim4j@gmail.com>
On Fri, Dec 5, 2014 at 1:16 PM, Vadim Kochan <vadim4j@gmail.com> wrote:
> This patch fixes the following issues which was introduced
> by me in commits:
>
> #1 (2dc854854b7f1b) ss: Fixed broken output for Netlink 'Peer Address:Port' column
> ISSUE: Broken layout when all sockets are printed out
>
> #2 (eef43b5052afb7) ss: Identify more netlink protocol names
> ISSUE: PID is not printed if only numbers output was specified (-n)
>
> Also aligned the width of the local/peer ports to be more wider.
>
> I tested with a lot of option combinations (I may miss some test cases),
> but layout seems to me better even on the previous released version
> of iproute2/ss.
>
> Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
> ---
> misc/ss.c | 30 ++++++++++--------------------
> 1 file changed, 10 insertions(+), 20 deletions(-)
Send v2 with updated commit message and subject.
Ragards,
^ permalink raw reply
* [PATCH net-next 1/4] net: tcp: refactor reinitialization of congestion control
From: Daniel Borkmann @ 2014-12-05 15:24 UTC (permalink / raw)
To: davem; +Cc: hannes, fw, netdev
In-Reply-To: <1417793092-6263-1-git-send-email-dborkman@redhat.com>
We can just move this to an extra function and make the code
a bit more readable, no functional change.
Joint work with Florian Westphal.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
net/ipv4/tcp_cong.c | 24 ++++++++++++++----------
1 file changed, 14 insertions(+), 10 deletions(-)
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 27ead0d..38f2f8a 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -107,6 +107,18 @@ void tcp_init_congestion_control(struct sock *sk)
icsk->icsk_ca_ops->init(sk);
}
+static void tcp_reinit_congestion_control(struct sock *sk,
+ const struct tcp_congestion_ops *ca)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+
+ tcp_cleanup_congestion_control(sk);
+ icsk->icsk_ca_ops = ca;
+
+ if (sk->sk_state != TCP_CLOSE && icsk->icsk_ca_ops->init)
+ icsk->icsk_ca_ops->init(sk);
+}
+
/* Manage refcounts on socket close. */
void tcp_cleanup_congestion_control(struct sock *sk)
{
@@ -262,21 +274,13 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
#endif
if (!ca)
err = -ENOENT;
-
else if (!((ca->flags & TCP_CONG_NON_RESTRICTED) ||
ns_capable(sock_net(sk)->user_ns, CAP_NET_ADMIN)))
err = -EPERM;
-
else if (!try_module_get(ca->owner))
err = -EBUSY;
-
- else {
- tcp_cleanup_congestion_control(sk);
- icsk->icsk_ca_ops = ca;
-
- if (sk->sk_state != TCP_CLOSE && icsk->icsk_ca_ops->init)
- icsk->icsk_ca_ops->init(sk);
- }
+ else
+ tcp_reinit_congestion_control(sk, ca);
out:
rcu_read_unlock();
return err;
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 3/4] net: tcp: add RTAX_CC_ALGO fib handling
From: Daniel Borkmann @ 2014-12-05 15:24 UTC (permalink / raw)
To: davem; +Cc: hannes, fw, netdev
In-Reply-To: <1417793092-6263-1-git-send-email-dborkman@redhat.com>
This patch adds the minimum necessary for the RTAX_CC_ALGO congestion
control metric to be set up and dumped back to user space.
While the internal representation of RTAX_CC_ALGO is handled as a u32
key, we avoided to expose this implementation detail to user space, thus
instead, we chose the netlink attribute that is being exchanged between
user space to be the actual congestion control algorithm name, similarly
as in the setsockopt(2) API in order to allow for maximum flexibility,
even for 3rd party modules.
It is a bit unfortunate that RTAX_QUICKACK used up a whole RTAX slot as
it should have been stored in RTAX_FEATURES instead, we first thought
about reusing it for the congestion control key, but it brings more
complications and/or confusion than worth it. Trying to load a non
present congestion algorithm name will be rejected.
Joint work with Florian Westphal.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
include/net/tcp.h | 7 +++++++
include/uapi/linux/rtnetlink.h | 2 ++
net/core/rtnetlink.c | 15 +++++++++++++--
net/decnet/dn_fib.c | 3 ++-
net/decnet/dn_table.c | 3 ++-
net/ipv4/fib_semantics.c | 14 ++++++++++++--
net/ipv6/ip6_fib.c | 15 ++++++++++++++-
net/ipv6/route.c | 3 ++-
8 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 135b70c..95bb237 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -846,7 +846,14 @@ extern struct tcp_congestion_ops tcp_reno;
struct tcp_congestion_ops *tcp_ca_find_key(u32 key);
u32 tcp_ca_get_key_by_name(const char *name);
+#ifdef CONFIG_INET
char *tcp_ca_get_name_by_key(u32 key, char *buffer);
+#else
+static inline char *tcp_ca_get_name_by_key(u32 key, char *buffer)
+{
+ return NULL;
+}
+#endif
static inline bool tcp_ca_needs_ecn(const struct sock *sk)
{
diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 9c9b8b4..d81f22d 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -389,6 +389,8 @@ enum {
#define RTAX_INITRWND RTAX_INITRWND
RTAX_QUICKACK,
#define RTAX_QUICKACK RTAX_QUICKACK
+ RTAX_CC_ALGO,
+#define RTAX_CC_ALGO RTAX_CC_ALGO
__RTAX_MAX
};
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 61cb7e7..3566f41 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -50,6 +50,7 @@
#include <net/arp.h>
#include <net/route.h>
#include <net/udp.h>
+#include <net/tcp.h>
#include <net/sock.h>
#include <net/pkt_sched.h>
#include <net/fib_rules.h>
@@ -669,9 +670,19 @@ int rtnetlink_put_metrics(struct sk_buff *skb, u32 *metrics)
for (i = 0; i < RTAX_MAX; i++) {
if (metrics[i]) {
+ if (i == RTAX_CC_ALGO - 1) {
+ char tmp[TCP_CA_NAME_MAX], *name;
+
+ name = tcp_ca_get_name_by_key(metrics[i], tmp);
+ if (!name)
+ continue;
+ if (nla_put_string(skb, i + 1, name))
+ goto nla_put_failure;
+ } else {
+ if (nla_put_u32(skb, i + 1, metrics[i]))
+ goto nla_put_failure;
+ }
valid++;
- if (nla_put_u32(skb, i+1, metrics[i]))
- goto nla_put_failure;
}
}
diff --git a/net/decnet/dn_fib.c b/net/decnet/dn_fib.c
index d332aef..df48034 100644
--- a/net/decnet/dn_fib.c
+++ b/net/decnet/dn_fib.c
@@ -298,7 +298,8 @@ struct dn_fib_info *dn_fib_create_info(const struct rtmsg *r, struct nlattr *att
int type = nla_type(attr);
if (type) {
- if (type > RTAX_MAX || nla_len(attr) < 4)
+ if (type > RTAX_MAX || type == RTAX_CC_ALGO ||
+ nla_len(attr) < 4)
goto err_inval;
fi->fib_metrics[type-1] = nla_get_u32(attr);
diff --git a/net/decnet/dn_table.c b/net/decnet/dn_table.c
index 86e3807..75c20a9 100644
--- a/net/decnet/dn_table.c
+++ b/net/decnet/dn_table.c
@@ -273,7 +273,8 @@ static inline size_t dn_fib_nlmsg_size(struct dn_fib_info *fi)
size_t payload = NLMSG_ALIGN(sizeof(struct rtmsg))
+ nla_total_size(4) /* RTA_TABLE */
+ nla_total_size(2) /* RTA_DST */
- + nla_total_size(4); /* RTA_PRIORITY */
+ + nla_total_size(4) /* RTA_PRIORITY */
+ + nla_total_size(TCP_CA_NAME_MAX); /* RTAX_CC_ALGO */
/* space for nested metrics */
payload += nla_total_size((RTAX_MAX * nla_total_size(4)));
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index f99f41b..d2b7b55 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -360,7 +360,8 @@ static inline size_t fib_nlmsg_size(struct fib_info *fi)
+ nla_total_size(4) /* RTA_TABLE */
+ nla_total_size(4) /* RTA_DST */
+ nla_total_size(4) /* RTA_PRIORITY */
- + nla_total_size(4); /* RTA_PREFSRC */
+ + nla_total_size(4) /* RTA_PREFSRC */
+ + nla_total_size(TCP_CA_NAME_MAX); /* RTAX_CC_ALGO */
/* space for nested metrics */
payload += nla_total_size((RTAX_MAX * nla_total_size(4)));
@@ -859,7 +860,16 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
if (type > RTAX_MAX)
goto err_inval;
- val = nla_get_u32(nla);
+ if (type == RTAX_CC_ALGO) {
+ char tmp[TCP_CA_NAME_MAX];
+
+ nla_strlcpy(tmp, nla, sizeof(tmp));
+ val = tcp_ca_get_key_by_name(tmp);
+ if (val == TCP_CA_UNSPEC)
+ goto err_inval;
+ } else {
+ val = nla_get_u32(nla);
+ }
if (type == RTAX_ADVMSS && val > 65535 - 40)
val = 65535 - 40;
if (type == RTAX_MTU && val > 65535 - 15)
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index b2d1838..0998ac6 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -30,6 +30,7 @@
#include <linux/slab.h>
#include <net/ipv6.h>
+#include <net/tcp.h>
#include <net/ndisc.h>
#include <net/addrconf.h>
@@ -650,10 +651,22 @@ static int fib6_commit_metrics(struct dst_entry *dst,
int type = nla_type(nla);
if (type) {
+ u32 val;
+
if (type > RTAX_MAX)
return -EINVAL;
+ if (type == RTAX_CC_ALGO) {
+ char tmp[TCP_CA_NAME_MAX];
+
+ nla_strlcpy(tmp, nla, sizeof(tmp));
+ val = tcp_ca_get_key_by_name(tmp);
+ if (val == TCP_CA_UNSPEC)
+ return -EINVAL;
+ } else {
+ val = nla_get_u32(nla);
+ }
- mp[type - 1] = nla_get_u32(nla);
+ mp[type - 1] = val;
}
}
return 0;
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index c910831..818c99a 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -2534,7 +2534,8 @@ static inline size_t rt6_nlmsg_size(void)
+ nla_total_size(4) /* RTA_OIF */
+ nla_total_size(4) /* RTA_PRIORITY */
+ RTAX_MAX * nla_total_size(4) /* RTA_METRICS */
- + nla_total_size(sizeof(struct rta_cacheinfo));
+ + nla_total_size(sizeof(struct rta_cacheinfo))
+ + nla_total_size(TCP_CA_NAME_MAX); /* RTAX_CC_ALGO */
}
static int rt6_fill_node(struct net *net,
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 2/4] net: tcp: add key management to congestion control
From: Daniel Borkmann @ 2014-12-05 15:24 UTC (permalink / raw)
To: davem; +Cc: hannes, fw, netdev
In-Reply-To: <1417793092-6263-1-git-send-email-dborkman@redhat.com>
For a per-route congestion control possibility, our aim is to store
a unique u32 key identifier into dst metrics, which can then be
mapped into a tcp_congestion_ops struct. We argue that having a
RTAX key entry is the most simple, generic and easy way to manage,
and also keeps the memory footprint of dst entries lower on 64 bit
than with storing a pointer directly, for example. Having a unique
key id also allows for decoupling actual TCP congestion control
module management from the FIB layer, i.e. we don't have to care
about expensive module refcounting inside the FIB at this point.
We first thought of using an IDR store for the realization, which
takes over dynamic assignment of unused key space and also performs
the key to pointer mapping in RCU. While doing so, we stumbled upon
the issue that due to the nature of dynamic key distribution, it
just so happens, arguably in very rare occasions, that excessive
module loads and unloads can lead to a possible reuse of previously
used key space. Thus, previously stale keys in the dst metric are
now being reassigned to a different congestion control algorithm,
which might lead to unexpected behaviour. One way to resolve this
would have been to walk FIBs on the actually rare occasion of a
module unload and reset the metric keys for each FIB in each netns,
but that's just very costly.
Therefore, we argue a better solution is to reuse the unique
congestion control algorithm name member and map that into u32 key
space through jhash. For that, we split the flags attribute (as it
currently uses 2 bits only anyway) into two u32 attributes, flags
and key, so that we can keep the cacheline boundary of 2 cachelines
on x86_64 and cache the precalculated key at registration time for
the fast path. On average we might expect 2 - 4 modules being loaded
worst case perhaps 15, so a key collision possibility is extremely
low, and guaranteed collision-free on LE/BE for all in-tree modules.
Overall this results in much simpler code, and all without the
overhead of an IDR. Due to the deterministic nature, modules can
now be unloaded, the congestion control algorithm for a specific
but unloaded key will fall back to the default one, and on module
reload time it will switch back to the expected algorithm
transparently.
Joint work with Florian Westphal.
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
include/net/inet_connection_sock.h | 3 +-
include/net/tcp.h | 9 +++++-
net/ipv4/tcp_cong.c | 63 ++++++++++++++++++++++++++++++++++++--
3 files changed, 71 insertions(+), 4 deletions(-)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 848e85c..5976bde 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -98,7 +98,8 @@ struct inet_connection_sock {
const struct tcp_congestion_ops *icsk_ca_ops;
const struct inet_connection_sock_af_ops *icsk_af_ops;
unsigned int (*icsk_sync_mss)(struct sock *sk, u32 pmtu);
- __u8 icsk_ca_state;
+ __u8 icsk_ca_state:7,
+ icsk_ca_dst_locked:1;
__u8 icsk_retransmits;
__u8 icsk_pending;
__u8 icsk_backoff;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index f50f29faf..135b70c 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -787,6 +787,8 @@ enum tcp_ca_ack_event_flags {
#define TCP_CA_MAX 128
#define TCP_CA_BUF_MAX (TCP_CA_NAME_MAX*TCP_CA_MAX)
+#define TCP_CA_UNSPEC 0
+
/* Algorithm can be set on socket without CAP_NET_ADMIN privileges */
#define TCP_CONG_NON_RESTRICTED 0x1
/* Requires ECN/ECT set on all packets */
@@ -794,7 +796,8 @@ enum tcp_ca_ack_event_flags {
struct tcp_congestion_ops {
struct list_head list;
- unsigned long flags;
+ u32 key;
+ u32 flags;
/* initialize private data (optional) */
void (*init)(struct sock *sk);
@@ -841,6 +844,10 @@ u32 tcp_reno_ssthresh(struct sock *sk);
void tcp_reno_cong_avoid(struct sock *sk, u32 ack, u32 acked);
extern struct tcp_congestion_ops tcp_reno;
+struct tcp_congestion_ops *tcp_ca_find_key(u32 key);
+u32 tcp_ca_get_key_by_name(const char *name);
+char *tcp_ca_get_name_by_key(u32 key, char *buffer);
+
static inline bool tcp_ca_needs_ecn(const struct sock *sk)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
diff --git a/net/ipv4/tcp_cong.c b/net/ipv4/tcp_cong.c
index 38f2f8a..8fd348c 100644
--- a/net/ipv4/tcp_cong.c
+++ b/net/ipv4/tcp_cong.c
@@ -13,6 +13,7 @@
#include <linux/types.h>
#include <linux/list.h>
#include <linux/gfp.h>
+#include <linux/jhash.h>
#include <net/tcp.h>
static DEFINE_SPINLOCK(tcp_cong_list_lock);
@@ -31,6 +32,19 @@ static struct tcp_congestion_ops *tcp_ca_find(const char *name)
return NULL;
}
+/* Simple linear search, not much in here. */
+struct tcp_congestion_ops *tcp_ca_find_key(u32 key)
+{
+ struct tcp_congestion_ops *e;
+
+ list_for_each_entry_rcu(e, &tcp_cong_list, list) {
+ if (e->key == key)
+ return e;
+ }
+
+ return NULL;
+}
+
/*
* Attach new congestion control algorithm to the list
* of available options.
@@ -45,9 +59,12 @@ int tcp_register_congestion_control(struct tcp_congestion_ops *ca)
return -EINVAL;
}
+ ca->key = jhash(ca->name, sizeof(ca->name), strlen(ca->name));
+
spin_lock(&tcp_cong_list_lock);
- if (tcp_ca_find(ca->name)) {
- pr_notice("%s already registered\n", ca->name);
+ if (ca->key == TCP_CA_UNSPEC || tcp_ca_find_key(ca->key)) {
+ pr_notice("%s already registered or non-unique key\n",
+ ca->name);
ret = -EEXIST;
} else {
list_add_tail_rcu(&ca->list, &tcp_cong_list);
@@ -70,9 +87,48 @@ void tcp_unregister_congestion_control(struct tcp_congestion_ops *ca)
spin_lock(&tcp_cong_list_lock);
list_del_rcu(&ca->list);
spin_unlock(&tcp_cong_list_lock);
+
+ /* Wait for outstanding readers to complete before the
+ * module gets removed entirely.
+ *
+ * A try_module_get() should fail by now as our module is
+ * in "going" state since no refs are held anymore and
+ * module_exit() handler being called.
+ */
+ synchronize_rcu();
}
EXPORT_SYMBOL_GPL(tcp_unregister_congestion_control);
+u32 tcp_ca_get_key_by_name(const char *name)
+{
+ const struct tcp_congestion_ops *ca;
+ u32 key;
+
+ rcu_read_lock();
+ ca = tcp_ca_find(name);
+ key = ca ? ca->key : TCP_CA_UNSPEC;
+ rcu_read_unlock();
+
+ return key;
+}
+EXPORT_SYMBOL_GPL(tcp_ca_get_key_by_name);
+
+char *tcp_ca_get_name_by_key(u32 key, char *buffer)
+{
+ const struct tcp_congestion_ops *ca;
+ char *ret = NULL;
+
+ rcu_read_lock();
+ ca = tcp_ca_find_key(key);
+ if (ca)
+ ret = strncpy(buffer, ca->name,
+ TCP_CA_NAME_MAX);
+ rcu_read_unlock();
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(tcp_ca_get_name_by_key);
+
/* Assign choice of congestion control. */
void tcp_assign_congestion_control(struct sock *sk)
{
@@ -256,6 +312,9 @@ int tcp_set_congestion_control(struct sock *sk, const char *name)
struct tcp_congestion_ops *ca;
int err = 0;
+ if (icsk->icsk_ca_dst_locked)
+ return -EPERM;
+
rcu_read_lock();
ca = tcp_ca_find(name);
--
1.7.11.7
^ permalink raw reply related
* [PATCH net-next 0/4] net: allow setting congctl via routing table
From: Daniel Borkmann @ 2014-12-05 15:24 UTC (permalink / raw)
To: davem; +Cc: hannes, fw, netdev
This is the second part of our work and allows for setting the congestion
control algorithm via routing table. For details, please see individual
patches.
Joint work with Florian Westphal, suggested by Hannes Frederic Sowa.
Thanks!
Daniel Borkmann (4):
net: tcp: refactor reinitialization of congestion control
net: tcp: add key management to congestion control
net: tcp: add RTAX_CC_ALGO fib handling
net: tcp: add per route congestion control
include/net/inet_connection_sock.h | 3 +-
include/net/tcp.h | 22 +++++++++-
include/uapi/linux/rtnetlink.h | 2 +
net/core/rtnetlink.c | 15 ++++++-
net/decnet/dn_fib.c | 3 +-
net/decnet/dn_table.c | 3 +-
net/ipv4/fib_semantics.c | 14 +++++-
net/ipv4/tcp_cong.c | 87 ++++++++++++++++++++++++++++++++------
net/ipv4/tcp_ipv4.c | 2 +
net/ipv4/tcp_minisocks.c | 30 +++++++++++--
net/ipv4/tcp_output.c | 21 +++++++++
net/ipv6/ip6_fib.c | 15 ++++++-
net/ipv6/route.c | 3 +-
net/ipv6/tcp_ipv6.c | 2 +
14 files changed, 196 insertions(+), 26 deletions(-)
--
1.7.11.7
^ permalink raw reply
* [PATCH net-next 4/4] net: tcp: add per route congestion control
From: Daniel Borkmann @ 2014-12-05 15:24 UTC (permalink / raw)
To: davem; +Cc: hannes, fw, netdev
In-Reply-To: <1417793092-6263-1-git-send-email-dborkman@redhat.com>
This work adds the possibility to define a per route/destination congestion
control algorithm. Generally, this opens up the possibility for a machine
with different links to enforce specific congestion control algorithms with
optimal strategies for each of them based on their network characteristics,
even transparently for a single application listening on all links.
For our specific use case, this additionally facilitates deployment of DCTCP,
for example, applications can easily serve internal traffic/dsts in DCTCP and
external one with CUBIC. Other scenarios would also allow for utilizing e.g.
long living, low priority background flows for certain destinations/routes
while still being able for normal traffic to utilize the default congestion
control algorithm. We also thought about a per netns setting (where different
defaults are possible), but given its actually a link specific property, we
argue that a per route/destination setting is the most natural and flexible.
The administrator can utilize this through ip-route(8) by appending "congctl
[lock] <name>", where <name> denotes the name of a loaded congestion control
algorithm and the optional lock parameter allows to enforce the given algorithm
so that applications in user space would not be allowed to overwrite that
algorithm for that destination.
The dst metric lookups are being done when a dst entry is already available
in order to avoid a costly lookup and still before the algorithms are being
initialized, thus overhead is very low when the feature is not being used.
While the client side would need to drop the current reference on the module,
on server side this can actually even be avoided as we just got a flat-copied
socket clone.
In case a dst metric contains a key to a congestion control module that has
just been removed from the kernel, it will fallback to the default congestion
control discipline for those and not reassign anything. Should at a later
point in time that module be reinserted into the kernel, then this algorithm
will be used again as keys are deterministic/static.
Joint work with Florian Westphal.
Suggested-by: Hannes Frederic Sowa <hannes@stressinduktion.org>
Signed-off-by: Florian Westphal <fw@strlen.de>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
include/net/tcp.h | 6 ++++++
net/ipv4/tcp_ipv4.c | 2 ++
net/ipv4/tcp_minisocks.c | 30 ++++++++++++++++++++++++++----
net/ipv4/tcp_output.c | 21 +++++++++++++++++++++
net/ipv6/tcp_ipv6.c | 2 ++
5 files changed, 57 insertions(+), 4 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 95bb237..b8fdc6b 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -448,6 +448,7 @@ int tcp_v4_conn_request(struct sock *sk, struct sk_buff *skb);
struct sock *tcp_create_openreq_child(struct sock *sk,
struct request_sock *req,
struct sk_buff *skb);
+void tcp_ca_openreq_child(struct sock *sk, const struct dst_entry *dst);
struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
struct request_sock *req,
struct dst_entry *dst);
@@ -636,6 +637,11 @@ static inline u32 tcp_rto_min_us(struct sock *sk)
return jiffies_to_usecs(tcp_rto_min(sk));
}
+static inline bool tcp_ca_dst_locked(const struct dst_entry *dst)
+{
+ return dst_metric_locked(dst, RTAX_CC_ALGO);
+}
+
/* Compute the actual receive window we are currently advertising.
* Rcv_nxt can be after the window if our peer push more data
* than the offered window.
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 33f5ff0..ed94fdc 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1340,6 +1340,8 @@ struct sock *tcp_v4_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
}
sk_setup_caps(newsk, dst);
+ tcp_ca_openreq_child(newsk, dst);
+
tcp_sync_mss(newsk, dst_mtu(dst));
newtp->advmss = dst_metric_advmss(dst);
if (tcp_sk(sk)->rx_opt.user_mss &&
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index 63d2680..bc9216d 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -399,6 +399,32 @@ static void tcp_ecn_openreq_child(struct tcp_sock *tp,
tp->ecn_flags = inet_rsk(req)->ecn_ok ? TCP_ECN_OK : 0;
}
+void tcp_ca_openreq_child(struct sock *sk, const struct dst_entry *dst)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+ u32 ca_key = dst_metric(dst, RTAX_CC_ALGO);
+ bool ca_got_dst = false;
+
+ if (ca_key != TCP_CA_UNSPEC) {
+ const struct tcp_congestion_ops *ca;
+
+ rcu_read_lock();
+ ca = tcp_ca_find_key(ca_key);
+ if (likely(ca && try_module_get(ca->owner))) {
+ icsk->icsk_ca_dst_locked = tcp_ca_dst_locked(dst);
+ icsk->icsk_ca_ops = ca;
+ ca_got_dst = true;
+ }
+ rcu_read_unlock();
+ }
+
+ if (!ca_got_dst && !try_module_get(icsk->icsk_ca_ops->owner))
+ tcp_assign_congestion_control(sk);
+
+ tcp_set_ca_state(sk, TCP_CA_Open);
+}
+EXPORT_SYMBOL_GPL(tcp_ca_openreq_child);
+
/* This is not only more efficient than what we used to do, it eliminates
* a lot of code duplication between IPv4/IPv6 SYN recv processing. -DaveM
*
@@ -451,10 +477,6 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
newtp->snd_cwnd = TCP_INIT_CWND;
newtp->snd_cwnd_cnt = 0;
- if (!try_module_get(newicsk->icsk_ca_ops->owner))
- tcp_assign_congestion_control(newsk);
-
- tcp_set_ca_state(newsk, TCP_CA_Open);
tcp_init_xmit_timers(newsk);
__skb_queue_head_init(&newtp->out_of_order_queue);
newtp->write_seq = newtp->pushed_seq = treq->snt_isn + 1;
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index f5bd4bd..5eb5676 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2916,6 +2916,25 @@ struct sk_buff *tcp_make_synack(struct sock *sk, struct dst_entry *dst,
}
EXPORT_SYMBOL(tcp_make_synack);
+static void tcp_ca_dst_init(struct sock *sk, const struct dst_entry *dst)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+ const struct tcp_congestion_ops *ca;
+ u32 ca_key = dst_metric(dst, RTAX_CC_ALGO);
+
+ if (ca_key == TCP_CA_UNSPEC)
+ return;
+
+ rcu_read_lock();
+ ca = tcp_ca_find_key(ca_key);
+ if (likely(ca && try_module_get(ca->owner))) {
+ module_put(icsk->icsk_ca_ops->owner);
+ icsk->icsk_ca_dst_locked = tcp_ca_dst_locked(dst);
+ icsk->icsk_ca_ops = ca;
+ }
+ rcu_read_unlock();
+}
+
/* Do all connect socket setups that can be done AF independent. */
static void tcp_connect_init(struct sock *sk)
{
@@ -2941,6 +2960,8 @@ static void tcp_connect_init(struct sock *sk)
tcp_mtup_init(sk);
tcp_sync_mss(sk, dst_mtu(dst));
+ tcp_ca_dst_init(sk, dst);
+
if (!tp->window_clamp)
tp->window_clamp = dst_metric(dst, RTAX_WINDOW);
tp->advmss = dst_metric_advmss(dst);
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index d06af89..20f44fe 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1199,6 +1199,8 @@ static struct sock *tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
inet_csk(newsk)->icsk_ext_hdr_len = (newnp->opt->opt_nflen +
newnp->opt->opt_flen);
+ tcp_ca_openreq_child(newsk, dst);
+
tcp_sync_mss(newsk, dst_mtu(dst));
newtp->advmss = dst_metric_advmss(dst);
if (tcp_sk(sk)->rx_opt.user_mss &&
--
1.7.11.7
^ permalink raw reply related
* [PATCH iproute2 -next] ip: route: add congestion control setting
From: Daniel Borkmann @ 2014-12-05 15:28 UTC (permalink / raw)
To: stephen; +Cc: hannes, fw, netdev
This patch adds configuration and dumping of congestion control metric
for ip route, f.e.: ip route add <dst> dev <dev> congctl [lock] <name>
Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
Stephen, this patch is already rebased on top of Florian's
ECN patch [1]. Thanks!
[1] http://patchwork.ozlabs.org/patch/407729/
include/linux/rtnetlink.h | 2 ++
ip/iproute.c | 24 +++++++++++++++++++++---
man/man8/ip-route.8.in | 25 ++++++++++++++++++++++++-
3 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/include/linux/rtnetlink.h b/include/linux/rtnetlink.h
index ae23d94..0c68a1a 100644
--- a/include/linux/rtnetlink.h
+++ b/include/linux/rtnetlink.h
@@ -390,6 +390,8 @@ enum {
#define RTAX_INITRWND RTAX_INITRWND
RTAX_QUICKACK,
#define RTAX_QUICKACK RTAX_QUICKACK
+ RTAX_CC_ALGO,
+#define RTAX_CC_ALGO RTAX_CC_ALGO
__RTAX_MAX
};
diff --git a/ip/iproute.c b/ip/iproute.c
index 5a496a9..18f7de7 100644
--- a/ip/iproute.c
+++ b/ip/iproute.c
@@ -53,6 +53,7 @@ static const char *mx_names[RTAX_MAX+1] = {
[RTAX_RTO_MIN] = "rto_min",
[RTAX_INITRWND] = "initrwnd",
[RTAX_QUICKACK] = "quickack",
+ [RTAX_CC_ALGO] = "congctl",
};
static void usage(void) __attribute__((noreturn));
@@ -80,8 +81,7 @@ static void usage(void)
fprintf(stderr, " [ window NUMBER] [ cwnd NUMBER ] [ initcwnd NUMBER ]\n");
fprintf(stderr, " [ ssthresh NUMBER ] [ realms REALM ] [ src ADDRESS ]\n");
fprintf(stderr, " [ rto_min TIME ] [ hoplimit NUMBER ] [ initrwnd NUMBER ]\n");
- fprintf(stderr, " [ features FEATURES ]\n");
- fprintf(stderr, " [ quickack BOOL ]\n");
+ fprintf(stderr, " [ features FEATURES ] [ quickack BOOL ] [ congctl NAME ]\n");
fprintf(stderr, "TYPE := [ unicast | local | broadcast | multicast | throw |\n");
fprintf(stderr, " unreachable | prohibit | blackhole | nat ]\n");
fprintf(stderr, "TABLE_ID := [ local | main | default | all | NUMBER ]\n");
@@ -545,10 +545,12 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
fprintf(fp, " %s", mx_names[i]);
else
fprintf(fp, " metric %d", i);
+
if (mxlock & (1<<i))
fprintf(fp, " lock");
+ if (i != RTAX_CC_ALGO)
+ val = *(unsigned*)RTA_DATA(mxrta[i]);
- val = *(unsigned*)RTA_DATA(mxrta[i]);
switch (i) {
case RTAX_FEATURES:
print_rtax_features(fp, val);
@@ -573,6 +575,10 @@ int print_route(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
fprintf(fp, " %gs", val/1e3);
else
fprintf(fp, " %ums", val);
+ break;
+ case RTAX_CC_ALGO:
+ fprintf(fp, " %s", (char *)RTA_DATA(mxrta[i]));
+ break;
}
}
}
@@ -925,6 +931,18 @@ static int iproute_modify(int cmd, unsigned flags, int argc, char **argv)
if (quickack != 1 && quickack != 0)
invarg("\"quickack\" value should be 0 or 1\n", *argv);
rta_addattr32(mxrta, sizeof(mxbuf), RTAX_QUICKACK, quickack);
+ } else if (matches(*argv, "congctl") == 0) {
+ char cc[16];
+ NEXT_ARG();
+ memset(cc, 0, sizeof(cc));
+ if (strcmp(*argv, "lock") == 0) {
+ mxlock |= (1<<RTAX_CC_ALGO);
+ NEXT_ARG();
+ }
+ strncpy(cc, *argv, sizeof(cc) - 1);
+ if (strlen(cc) == 0)
+ invarg("\"conctl\" value must be a algorithm name\n", *argv);
+ rta_addattr_l(mxrta, sizeof(mxbuf), RTAX_CC_ALGO, cc, strlen(cc));
} else if (matches(*argv, "rttvar") == 0) {
unsigned win;
NEXT_ARG();
diff --git a/man/man8/ip-route.8.in b/man/man8/ip-route.8.in
index 89960c1..6e4b94c 100644
--- a/man/man8/ip-route.8.in
+++ b/man/man8/ip-route.8.in
@@ -116,7 +116,9 @@ replace " } "
.B features
.IR FEATURES " ] [ "
.B quickack
-.IR BOOL " ]"
+.IR BOOL " ] [ "
+.B congctl
+.IR NAME " ]"
.ti -8
.IR TYPE " := [ "
@@ -433,6 +435,27 @@ sysctl is set to 0.
Enable or disable quick ack for connections to this destination.
.TP
+.BI congctl " NAME " "(3.19+ only)"
+.TP
+.BI "congctl lock" " NAME " "(3.19+ only)"
+Sets a specific TCP congestion control algorithm only for a given destination.
+If not specified, Linux keeps the current global default TCP congestion control
+algorithm, or the one set from the application. If the modifier
+.B lock
+is not used, an application may nevertheless overwrite the suggested congestion
+control algorithm for that destination. If the modifier
+.B lock
+is used, then an application is not allowed to overwrite the specified congestion
+control algorithm for that destination, thus it will be enforced/guaranteed to
+use the proposed algorithm. Should a congestion control module be unloaded, the
+specified congestion control algorithm will fall back to the current global
+default on connection establishment. In case the same congestion control module
+will be reloaded at a later point in time into the kernel and the congctl route
+attribute has not been modified until then, new connections for that destination
+will make use of it again. Note that the kernel will not try to autoload non-present
+congestion control modules.
+
+.TP
.BI advmss " NUMBER " "(2.3.15+ only)"
the MSS ('Maximal Segment Size') to advertise to these
destinations when establishing TCP connections. If it is not given,
--
1.9.3
^ permalink raw reply related
* Re: [patch net-next 1/2] net: sched: cls_basic: fix error path in basic_change()
From: John Fastabend @ 2014-12-05 15:29 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, jhs
In-Reply-To: <1417791023-28124-1-git-send-email-jiri@resnulli.us>
On 12/05/2014 06:50 AM, Jiri Pirko wrote:
> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> ---
> net/sched/cls_basic.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
> index 7cf0a62..5aed341 100644
> --- a/net/sched/cls_basic.c
> +++ b/net/sched/cls_basic.c
> @@ -178,10 +178,9 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
> return -EINVAL;
> }
>
> - err = -ENOBUFS;
> fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
> - if (fnew == NULL)
> - goto errout;
> + if (!fnew)
> + return -ENOBUFS;
>
> tcf_exts_init(&fnew->exts, TCA_BASIC_ACT, TCA_BASIC_POLICE);
> err = -EINVAL;
>
Nice catch, thanks!
Reviewed-by: John Fastabend <john.r.fastabend@intel.com>
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH net-next] tcp: refine TSO autosizing
From: Neal Cardwell @ 2014-12-05 15:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Yuchung Cheng, Nandita Dukkipati
In-Reply-To: <1417788937.4322.21.camel@edumazet-glaptop2.roam.corp.google.com>
On Fri, Dec 5, 2014 at 9:15 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Commit 95bd09eb2750 ("tcp: TSO packets automatic sizing") tried to
> control TSO size, but did this at the wrong place (sendmsg() time)
>
> At sendmsg() time, we might have a pessimistic view of flow rate,
> and we end up building very small skbs (with 2 MSS per skb).
>
> This is bad because :
>
> - It sends small TSO packets even in Slow Start where rate quickly
> increases.
> - It tends to make socket write queue very big, increasing tcp_ack()
> processing time, but also increasing memory needs, not necessarily
> accounted for, as fast clones overhead is currently ignored.
> - Lower GRO efficiency and more ACK packets.
>
> Servers with a lot of small lived connections suffer from this.
>
> Lets instead fill skbs as much as possible (64KB of payload), but split
> them at xmit time, when we have a precise idea of the flow rate.
> skb split is actually quite efficient.
Nice. I definitely agree this is the right direction.
However, from my experience testing a variant of this approach, this
kind of late decision about packet size was sometimes causing
performance shortfalls on long-RTT, medium-bandwidth paths unless
tcp_tso_should_defer() was also modified to use the new/smaller packet
size goal.
The issue is that tcp_tso_should_defer() uses tp->xmit_size_goal_segs
as a yardstick, and says, "hey, if cwnd and rwin allow us to send
tp->xmit_size_goal_segs * tp->mss_cache then let's go ahead and send
it now."
But if we remove the sendmsg-time autosizing logic that was tuning
tp->xmit_size_goal_segs, then tcp_tso_should_defer() is now going to
be waiting to try to accumulate permission to send a big skb with
tp->xmit_size_goal_segs (e.g. ~40) MSS in it.
In my tests I was able to fix this issue by making
tcp_tso_should_defer() use the latest size goal instead of
tp->xmit_size_goal_segs.
So, how about making the rate-based TSO autosizing goal (stored in
"segs" in this patch) at the top of tcp_write_xmit()? Then we could
pass in that segment goal to tcp_tso_should_defer() for use instead of
tp->xmit_size_goal_segs in deciding whether we have a big enough chunk
to send now. Similarly, that segment goal could be passed in to
tcp_mss_split_point instead of sk->sk_gso_max_segs.
(The autosizing calculation could be in a helper function to keep
tcp_write_xmit() manageable.)
neal
^ permalink raw reply
* Re: [patch net-next 1/2] net: sched: cls_basic: fix error path in basic_change()
From: Eric Dumazet @ 2014-12-05 16:03 UTC (permalink / raw)
To: John Fastabend; +Cc: Jiri Pirko, netdev, davem, jhs
In-Reply-To: <5481CF4C.2060607@gmail.com>
On Fri, 2014-12-05 at 07:29 -0800, John Fastabend wrote:
> On 12/05/2014 06:50 AM, Jiri Pirko wrote:
> > Signed-off-by: Jiri Pirko <jiri@resnulli.us>
> > ---
> > net/sched/cls_basic.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
> > index 7cf0a62..5aed341 100644
> > --- a/net/sched/cls_basic.c
> > +++ b/net/sched/cls_basic.c
> > @@ -178,10 +178,9 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
> > return -EINVAL;
> > }
> >
> > - err = -ENOBUFS;
> > fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
> > - if (fnew == NULL)
> > - goto errout;
> > + if (!fnew)
> > + return -ENOBUFS;
> >
> > tcf_exts_init(&fnew->exts, TCA_BASIC_ACT, TCA_BASIC_POLICE);
> > err = -EINVAL;
> >
>
> Nice catch, thanks!
>
> Reviewed-by: John Fastabend <john.r.fastabend@intel.com>
Sorry, but this looks a cosmetic change, right ?
If it is a fix, we'd like a 'Fixes: ...' tag.
^ permalink raw reply
* [PATCH net-next] macvlan: allow setting LRO independently of lower device
From: Michal Kubecek @ 2014-12-05 16:05 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev, linux-kernel
Since commit fbe168ba91f7 ("net: generic dev_disable_lro() stacked
device handling"), dev_disable_lro() zeroes NETIF_F_LRO feature flag
first for a macvlan device and then for its lower device. As an attempt
to set NETIF_F_LRO to zero is ignored, dev_disable_lro() issues a
warning and taints kernel.
Allowing NETIF_F_LRO to be set independently of the lower device
consists of three parts:
- add the flag to hw_features to allow toggling it
- allow setting it to 0 even if lower device has the flag set
- add the flag to MACVLAN_FEATURES to restore copying from lower
device on macvlan creation
Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
drivers/net/macvlan.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 9538674..10604db 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -747,7 +747,7 @@ static struct lock_class_key macvlan_netdev_addr_lock_key;
#define MACVLAN_FEATURES \
(NETIF_F_SG | NETIF_F_ALL_CSUM | NETIF_F_HIGHDMA | NETIF_F_FRAGLIST | \
- NETIF_F_GSO | NETIF_F_TSO | NETIF_F_UFO | \
+ NETIF_F_GSO | NETIF_F_TSO | NETIF_F_UFO | NETIF_F_LRO | \
NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_GRO | NETIF_F_RXCSUM | \
NETIF_F_HW_VLAN_CTAG_FILTER | NETIF_F_HW_VLAN_STAG_FILTER)
@@ -784,6 +784,7 @@ static int macvlan_init(struct net_device *dev)
(lowerdev->state & MACVLAN_STATE_MASK);
dev->features = lowerdev->features & MACVLAN_FEATURES;
dev->features |= ALWAYS_ON_FEATURES;
+ dev->hw_features |= NETIF_F_LRO;
dev->vlan_features = lowerdev->vlan_features & MACVLAN_FEATURES;
dev->gso_max_size = lowerdev->gso_max_size;
dev->iflink = lowerdev->ifindex;
@@ -936,15 +937,15 @@ static netdev_features_t macvlan_fix_features(struct net_device *dev,
netdev_features_t features)
{
struct macvlan_dev *vlan = netdev_priv(dev);
+ netdev_features_t lowerdev_features = vlan->lowerdev->features;
netdev_features_t mask;
features |= NETIF_F_ALL_FOR_ALL;
features &= (vlan->set_features | ~MACVLAN_FEATURES);
mask = features;
- features = netdev_increment_features(vlan->lowerdev->features,
- features,
- mask);
+ lowerdev_features &= (features | ~NETIF_F_LRO);
+ features = netdev_increment_features(lowerdev_features, features, mask);
features |= ALWAYS_ON_FEATURES;
features &= ~NETIF_F_NETNS_LOCAL;
--
1.8.4.5
^ permalink raw reply related
* [PATCH net-next] openvswitch: set correct protocol on route lookup
From: Jiri Benc @ 2014-12-05 16:24 UTC (permalink / raw)
To: netdev; +Cc: dev, Pravin Shelar, Wenyu Zhang
Respect what the caller passed to ovs_tunnel_get_egress_info.
Fixes: 8f0aad6f35f7e ("openvswitch: Extend packet attribute for egress tunnel info")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
net/openvswitch/vport.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/openvswitch/vport.c b/net/openvswitch/vport.c
index e771a46933e5..9584526c0778 100644
--- a/net/openvswitch/vport.c
+++ b/net/openvswitch/vport.c
@@ -600,7 +600,7 @@ int ovs_tunnel_get_egress_info(struct ovs_tunnel_info *egress_tun_info,
fl.saddr = tun_key->ipv4_src;
fl.flowi4_tos = RT_TOS(tun_key->ipv4_tos);
fl.flowi4_mark = skb_mark;
- fl.flowi4_proto = IPPROTO_GRE;
+ fl.flowi4_proto = ipproto;
rt = ip_route_output_key(net, &fl);
if (IS_ERR(rt))
--
1.8.3.1
^ permalink raw reply related
* Re: [patch net-next 1/2] net: sched: cls_basic: fix error path in basic_change()
From: John Fastabend @ 2014-12-05 16:34 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Jiri Pirko, netdev, davem, jhs
In-Reply-To: <1417795431.15618.6.camel@edumazet-glaptop2.roam.corp.google.com>
On 12/05/2014 08:03 AM, Eric Dumazet wrote:
> On Fri, 2014-12-05 at 07:29 -0800, John Fastabend wrote:
>> On 12/05/2014 06:50 AM, Jiri Pirko wrote:
>>> Signed-off-by: Jiri Pirko <jiri@resnulli.us>
>>> ---
>>> net/sched/cls_basic.c | 5 ++---
>>> 1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
>>> index 7cf0a62..5aed341 100644
>>> --- a/net/sched/cls_basic.c
>>> +++ b/net/sched/cls_basic.c
>>> @@ -178,10 +178,9 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
>>> return -EINVAL;
>>> }
>>>
>>> - err = -ENOBUFS;
>>> fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
>>> - if (fnew == NULL)
>>> - goto errout;
>>> + if (!fnew)
>>> + return -ENOBUFS;
>>>
>>> tcf_exts_init(&fnew->exts, TCA_BASIC_ACT, TCA_BASIC_POLICE);
>>> err = -EINVAL;
>>>
>>
>> Nice catch, thanks!
>>
>> Reviewed-by: John Fastabend <john.r.fastabend@intel.com>
>
> Sorry, but this looks a cosmetic change, right ?
>
> If it is a fix, we'd like a 'Fixes: ...' tag.
>
>
>
oops, you are right. fnew is null, free'ing the null value
is no issue at all from the error path. And if we are going
to start converting the use of
if (foo == NULL)
to
if (!foo)
in ./net/sched that is OK but not really worth the noise in
my opinion there are a lot of these in the code for quiet
sometime.
Sorry for the noise. I misread it as fixing a free on some
non-null value, which is not the case.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH net-next 0/4] net: allow setting congctl via routing table
From: Dave Taht @ 2014-12-05 16:35 UTC (permalink / raw)
To: Daniel Borkmann
Cc: davem@davemloft.net, Hannes Frederic Sowa, Florian Westphal,
netdev@vger.kernel.org
In-Reply-To: <1417793092-6263-1-git-send-email-dborkman@redhat.com>
On Fri, Dec 5, 2014 at 7:24 AM, Daniel Borkmann <dborkman@redhat.com> wrote:
> This is the second part of our work and allows for setting the congestion
> control algorithm via routing table. For details, please see individual
> patches.
>
> Joint work with Florian Westphal, suggested by Hannes Frederic Sowa.
>
> Thanks!
>
> Daniel Borkmann (4):
> net: tcp: refactor reinitialization of congestion control
> net: tcp: add key management to congestion control
> net: tcp: add RTAX_CC_ALGO fib handling
> net: tcp: add per route congestion control
Very interesting. Have you tried something other than dctcp here
(e.g. westwood or lp?)
Have you considered the case where the route changes underneath
you from one device to another?
Example, here I am routing everything through eth0, where I
would want cubic, probably...
root@ganesha:~/git/tinc# ip route
default via 172.26.16.1 dev eth0 proto babel onlink
69.181.216.0/22 via 172.26.16.1 dev eth0 proto babel onlink
169.254.0.0/16 dev eth0 scope link metric 1000
172.26.16.0/24 dev eth0 proto kernel scope link src 172.26.16.177
172.26.16.1 via 172.26.16.1 dev eth0 proto babel onlink
172.26.16.112 via 172.26.16.112 dev eth0 proto babel onlink
172.26.17.0/24 via 172.26.16.1 dev eth0 proto babel onlink
172.26.17.3 via 172.26.16.1 dev eth0 proto babel onlink
172.26.17.227 via 172.26.16.1 dev eth0 proto babel onlink
192.168.7.0/30 dev eth1 proto kernel scope link src 192.168.7.1 metric 1
192.168.7.2 via 172.26.16.112 dev eth0 proto babel onlink
And I pull the plug, and everything flips over to wlan0,
where I might want westwood (or something saner than
that. It might be nice to have a per-device cc default
algorithm...)
root@ganesha:~/git/tinc# ip route
default via 172.26.17.224 dev wlan0 proto babel onlink
69.181.216.0/22 via 172.26.17.224 dev wlan0 proto babel onlink
169.254.0.0/16 dev eth0 scope link metric 1000
172.26.16.0/24 dev eth0 proto kernel scope link src 172.26.16.177
172.26.16.1 via 172.26.17.227 dev wlan0 proto babel onlink
172.26.16.112 via 172.26.17.227 dev wlan0 proto babel onlink
172.26.17.0/24 via 172.26.17.224 dev wlan0 proto babel onlink
172.26.17.3 via 172.26.17.227 dev wlan0 proto babel onlink
172.26.17.227 via 172.26.17.227 dev wlan0 proto babel onlink
192.168.7.0/30 dev eth1 proto kernel scope link src 192.168.7.1 metric 1
192.168.7.2 via 172.26.17.227 dev wlan0 proto babel onlink
--
Dave Täht
the quest for a fq-friendly vpn with no head of line blocking continues!
https://plus.google.com/u/0/107942175615993706558/posts/QWPWLoGMtrm
^ 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